Fix link inclusion in reviewer emails

[FyreByrd]

Fixes #1405

Instead of having multiple possible email templates for each combination of artifacts, dynamically builds a list of artifact links from available relevant artifacts.

Summary by CodeRabbit

• Improvements
  • Review notification emails now show all product files in one consolidated file block (single placeholder) instead of multiple separate links.
  • Removed redundant review-template variants and aligned subject/body templates for consistency.
  • Change applied across English, Spanish (es-419) and French (fr-FR) translations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Consolidated review-related email templates across en-us, es-419, and fr-FR to use a single {{files}} placeholder; updated the email job executor to build and pass an HTML files fragment instead of separate artifact URL fields and simplified messageId selection to reviewProduct (+WithComment).

Changes

Cohort / File(s)	Summary
Locale templates src/lib/server/email-service/locales/en-us.json, src/lib/server/email-service/locales/es-419.json, src/lib/server/email-service/locales/fr-FR.json	Removed multiple review-specific keys and replaced explicit per-artifact HTML links with a unified {{files}} placeholder in review email body templates; removed corresponding subject keys for deleted variants.
Email executor src/lib/server/job-executors/email.ts	Replaced discrete artifact URL properties (apkUrl, pwaUrl, playListingUrl, assetPreviewUrl) with a single files HTML fragment built from available artifact URLs; messageId selection simplified to reviewProduct or reviewProductWithComment.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas to inspect closely:
  • src/lib/server/job-executors/email.ts — verify files HTML composition omits null/empty URLs, properly encodes/escapes URLs, and preserves intended ordering/labels.
  • Locale JSON files — ensure templates reference {{files}} correctly and no callers still expect removed subject/body keys.
  • Call sites of the email job executor — confirm no remaining dependencies on individual artifact URL properties.

Possibly related PRs

• Fix reviewer emails #1397 — modifies the same src/lib/server/job-executors/email.ts and related reviewer-email behavior; likely overlaps in artifact link formatting and messageId logic.

Suggested reviewers

• chrisvire

Poem

"I hopped through JSON, neat and spry,
Gathered links that scattered by.
One little placeholder now holds the keys,
Less hopping through anchors, more calm inboxes please.
— a tidy rabbit 🐇"

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix link inclusion in reviewer emails' clearly identifies the primary change: refactoring how artifact links are included in reviewer email templates.
Linked Issues check	✅ Passed	The PR successfully addresses issue #1405 by replacing multiple artifact-specific email templates with a dynamic link-building approach that includes only valid, relevant artifact links in reviewer emails.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue: locale file updates consolidate email templates and the executor implements dynamic link inclusion, with no unrelated modifications detected.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/review-email-links

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/lib/server/email-service/locales/en-us.json (1)

71-78: Remove deprecated email subject keys from locale files.

The keys reviewProductNoPlayListing, reviewAssetPackage, and reviewPwaProduct (lines 73, 75, 77) and their WithComment variants are no longer used. The email executor only ever uses reviewProduct or reviewProductWithComment for the messageId (see email.ts lines 104-105). These unused keys should be removed from all locale files (en-us.json, fr-FR.json, es-419.json) to reduce maintenance burden and prevent confusion.

🧹 Nitpick comments (3)

src/lib/server/email-service/locales/es-419.json (1)

35-36: Consider localizing the review email templates.

The reviewProduct and reviewProductWithComment templates are in English in the Spanish locale file. While this may be intentional, consider translating these templates to provide a consistent localized experience for Spanish-speaking users.

src/lib/server/email-service/locales/fr-FR.json (1)

35-36: Consider localizing the review email templates.

The reviewProduct and reviewProductWithComment templates are in English in the French locale file. While this may be intentional, consider translating these templates to provide a consistent localized experience for French-speaking users.

src/lib/server/job-executors/email.ts (1)

111-114: Consider handling the empty artifacts case for better user experience.

When all artifacts are null, the files string will be empty, resulting in the email displaying "Here are the product files to be reviewed:

". This message is misleading to reviewers when no files are actually available. Consider conditionally displaying a more informative message (e.g., "No files available for review") when the artifacts list is empty, either by adjusting the filter logic or updating the email template.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee9952c and 508c38d.

📒 Files selected for processing (4)

• src/lib/server/email-service/locales/en-us.json (1 hunks)
• src/lib/server/email-service/locales/es-419.json (1 hunks)
• src/lib/server/email-service/locales/fr-FR.json (1 hunks)
• src/lib/server/job-executors/email.ts (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: checks / build-and-test

🔇 Additional comments (2)

src/lib/server/email-service/locales/en-us.json (1)

35-36: LGTM! Template consolidation simplifies maintenance.

The refactoring to use a single {{files}} placeholder for dynamically-generated artifact links is clean and addresses the issue of incorrect links in republish emails.

src/lib/server/job-executors/email.ts (1)

98-103: LGTM! Artifact extraction is clean and type-safe.

The consolidation of artifact URLs into a single files object provides a clear, maintainable structure for dynamically building the email content.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/lib/server/job-executors/email.ts (1)

98-103: Dynamic files construction correctly filters missing artifacts and fixes broken links

The new files map plus the Object.entries(...).filter((e) => !!e[1]) pipeline ensures only artifacts with a real Url get rendered, so republish flows without an APK (or with a null/undefined URL) will no longer emit a broken APK link. The anchor generation now also uses properly quoted href attributes, which should render reliably in email clients. This looks aligned with the PR’s goal of only including valid links.

As an optional polish, you might consider mapping the internal keys (apk, pwa, play-listing, asset-preview) to more human‑friendly or localized labels for the link text, e.g. "APK", "PWA", "Google Play listing", "Asset preview", so end users don’t see raw type identifiers.

Also applies to: 111-114

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 745c38c and e04a964.

📒 Files selected for processing (1)

• src/lib/server/job-executors/email.ts (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: checks / build-and-test
• GitHub Check: checks / typecheck-lint
• GitHub Check: check / build-and-test

[chrisvire]

👍