Replace newsletter confirmation with a welcome email#8071
Replace newsletter confirmation with a welcome email#8071gregory-boch-prisma wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
WalkthroughChangesThe newsletter implementation centralizes Brevo subscription and unsubscription logic, uses shared route handling across applications, adds a website unsubscribe flow, and updates documentation and success messages for immediate subscriptions. Newsletter Flow
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/docs/src/app/api/newsletter/README.md (1)
84-89: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the newsletter API docs
apps/docs/src/app/api/newsletter/README.md:84-89,124-141still has carry-over examples from the old handler: the 400 response should say"A valid email address is required", and the “Development Mode Debug Info” section should be removed since the shared route only returns a plain 500 error and nodebugpayload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/docs/src/app/api/newsletter/README.md` around lines 84 - 89, Update the newsletter API documentation examples in README.md: change the 400 response error text to “A valid email address is required,” and remove the “Development Mode Debug Info” section because the shared route returns only a plain 500 response without a debug payload.
🧹 Nitpick comments (4)
packages/ui/src/lib/newsletter-subscription.ts (2)
118-135: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAvoid reusing the Brevo API key as an HMAC secret.
createUnsubscribeTokensigns withapiKeyitself as the HMAC key. This works today (HMAC key-recovery is infeasible), but it couples an unsubscribe-token secret to your highest-privilege Brevo credential — anyone who can compute/verify tokens effectively needs the master API key, and you lose the ability to rotate one independently of the other. A dedicatedNEWSLETTER_UNSUBSCRIBE_SECRETenv var would keep blast radius smaller.♻️ Suggested direction
-async function createUnsubscribeToken(apiKey: string, email: string): Promise<string> { +async function createUnsubscribeToken(secret: string, email: string): Promise<string> { const key = await crypto.subtle.importKey( "raw", - new TextEncoder().encode(apiKey), + new TextEncoder().encode(secret), { name: "HMAC", hash: "SHA-256" }, false, ["sign"], );Since existing tokens are looked up by stored value (not recomputed), this change is backward compatible for already-issued tokens.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/lib/newsletter-subscription.ts` around lines 118 - 135, Update createUnsubscribeToken to use a dedicated NEWSLETTER_UNSUBSCRIBE_SECRET environment variable as the HMAC key instead of the Brevo apiKey. Validate that the dedicated secret is configured before importing the key, while preserving the existing token message and digest format.
137-153: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on outbound Brevo requests.
getContact,upsertContact,sendWelcomeEmail, and the unsubscribe lookup/update calls all rely on the barefetch/fetcherwith noAbortSignal.timeout(...). SincesubscribeToPrismaNewsletterchains up to three sequential calls, a slow Brevo response leaves the request hanging until the hosting platform's own function timeout rather than failing fast with a clear error.♻️ Suggested fix (repeat for each fetcher call)
- const response = await fetcher( - `${BREVO_API_BASE_URL}/contacts/${encodeURIComponent(email)}?identifierType=email_id`, - { headers: getBrevoHeaders(apiKey) }, - ); + const response = await fetcher( + `${BREVO_API_BASE_URL}/contacts/${encodeURIComponent(email)}?identifierType=email_id`, + { headers: getBrevoHeaders(apiKey), signal: AbortSignal.timeout(5_000) }, + );Also applies to: 201-225
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/lib/newsletter-subscription.ts` around lines 137 - 153, Introduce a shared timeout signal for outbound Brevo requests and pass it through every fetcher call in getContact, upsertContact, sendWelcomeEmail, and the unsubscribe lookup/update flow. Use AbortSignal.timeout with the established request timeout value, ensuring sequential newsletter operations fail fast instead of waiting for the platform timeout.packages/ui/src/lib/newsletter-subscription.test.ts (1)
76-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a source-preservation assertion to this resubscribe test.
This test resubscribes via
source: "blog"but never asserts what happens toattributes.SOURCE(it only checks the unsubscribe token is preserved). Given the PR's "preserve source ownership" goal, add a case where the existing contact has a different originalSOURCEand assert it isn't overwritten — this would have caught theupsertContactgap flagged innewsletter-subscription.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/lib/newsletter-subscription.test.ts` around lines 76 - 102, Extend the “keeps an existing unsubscribe token when a contact resubscribes” test around subscribeToPrismaNewsletter by adding an existing contact attributes.SOURCE value that differs from the requested "blog" source, then assert the contact update body preserves that original source while retaining the existing unsubscribe-token assertions.apps/docs/src/app/api/newsletter/README.md (1)
17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify list/template ID ownership and name the welcome template.
Step 1 tells the reader to "note your list ID," but step 3 says the shared helper owns the IDs — it's not obvious the "noting" step is purely a verification check against hardcoded values, not a config step. Also, no welcome template ID is named here, even though Troubleshooting (lines 120-121) explicitly calls out "incorrect/inactive welcome template ID" as a common failure — naming the expected template ID would make that section actionable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/docs/src/app/api/newsletter/README.md` around lines 17 - 19, Clarify step 1 to state that the list ID is being verified against the shared server helper’s hardcoded value, not configured manually. Update step 2 to name the expected newsletter welcome template ID and indicate it should be confirmed active, keeping the ownership statement in step 3 consistent with these verification-only steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/lib/newsletter-subscription.ts`:
- Around line 155-171: The upsertContact function overwrites an existing
contact’s SOURCE during resubscription. Update its attributes to reuse
contact?.attributes.SOURCE when present, otherwise fall back to the incoming
source; add a resubscribe test in
packages/ui/src/lib/newsletter-subscription.test.ts:76-102 using a different
source and assert the original SOURCE is preserved.
---
Outside diff comments:
In `@apps/docs/src/app/api/newsletter/README.md`:
- Around line 84-89: Update the newsletter API documentation examples in
README.md: change the 400 response error text to “A valid email address is
required,” and remove the “Development Mode Debug Info” section because the
shared route returns only a plain 500 response without a debug payload.
---
Nitpick comments:
In `@apps/docs/src/app/api/newsletter/README.md`:
- Around line 17-19: Clarify step 1 to state that the list ID is being verified
against the shared server helper’s hardcoded value, not configured manually.
Update step 2 to name the expected newsletter welcome template ID and indicate
it should be confirmed active, keeping the ownership statement in step 3
consistent with these verification-only steps.
In `@packages/ui/src/lib/newsletter-subscription.test.ts`:
- Around line 76-102: Extend the “keeps an existing unsubscribe token when a
contact resubscribes” test around subscribeToPrismaNewsletter by adding an
existing contact attributes.SOURCE value that differs from the requested "blog"
source, then assert the contact update body preserves that original source while
retaining the existing unsubscribe-token assertions.
In `@packages/ui/src/lib/newsletter-subscription.ts`:
- Around line 118-135: Update createUnsubscribeToken to use a dedicated
NEWSLETTER_UNSUBSCRIBE_SECRET environment variable as the HMAC key instead of
the Brevo apiKey. Validate that the dedicated secret is configured before
importing the key, while preserving the existing token message and digest
format.
- Around line 137-153: Introduce a shared timeout signal for outbound Brevo
requests and pass it through every fetcher call in getContact, upsertContact,
sendWelcomeEmail, and the unsubscribe lookup/update flow. Use
AbortSignal.timeout with the established request timeout value, ensuring
sequential newsletter operations fail fast instead of waiting for the platform
timeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 865b1501-f417-4aab-ac1d-abbb6aad44cc
📒 Files selected for processing (11)
apps/blog/src/app/api/newsletter/route.tsapps/docs/src/app/api/newsletter/README.mdapps/docs/src/app/api/newsletter/route.tsapps/site/src/app/api/newsletter/route.tsapps/site/src/app/api/newsletter/unsubscribe/route.tsapps/site/src/app/newsletter/newsletter-signup.tsxapps/site/src/app/newsletter/unsubscribe/page.tsxpackages/ui/src/components/newsletter.tsxpackages/ui/src/lib/newsletter-route.tspackages/ui/src/lib/newsletter-subscription.test.tspackages/ui/src/lib/newsletter-subscription.ts
|
Argos note: the single changed snapshot is unrelated to this PR's newsletter UI. Both reruns point to I did not approve the visual change. It needs an Argos reviewer to mark the cross-app capture as flaky/unrelated: https://app.argos-ci.com/developer-connections/web/builds/7932 |
2 participants
What
SOURCEattribution and record the current path inNEWSLETTER_SOURCERelated Console source marker: prisma/pdp-control-plane#4560.
Brevo
NEWSLETTER_UNSUBSCRIBE_TOKENandNEWSLETTER_SOURCEcontact attributes are liveVerification
Rollout
After deployment, verify one new public subscription and one repeated subscription in Brevo, then deactivate legacy DOI template 36.
Summary by CodeRabbit
/newsletter/unsubscribepage with token validation and status feedback (success/invalid/error).