feat(newsletter): manage subscription preferences (consolidates #78 + #126)#244
feat(newsletter): manage subscription preferences (consolidates #78 + #126)#244hugodemenez wants to merge 1 commit into
Conversation
Replace overlapping PR #78 and PR #126 work with a single implementation rebased on current beta: - Add Newsletter preference fields (weekly, monthly stats, renewals) - Dual-auth preferences API (Supabase session or signed email token) - Secure HMAC unsubscribe links across email templates and cron jobs - Enforce preferences in weekly summary and renewal notice crons - Shared preferences UI on /newsletter and dashboard settings - Email normalization migration for existing Newsletter rows Closes #75 Co-authored-by: Hugo Demenez <hugodemenez@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🤖 Automated Code ReviewPR Review: Newsletter PreferencesThis PR adds granular newsletter preferences (weekly summary, monthly stats, renewal notices), HMAC-signed unsubscribe/preferences links, a preferences API + UI, and email normalization. Overall direction is solid and it fixes a real security hole in the old unsubscribe flow. Critical / High Priority1. Welcome re-subscribe leaves preference flags disabled
2. The preference is exposed in the UI and stored in the DB, but no cron or sender checks it. Users can disable monthly stats with no effect — misleading UX and a compliance risk if this is marketed as a real opt-out. 3. Production secret fallback
4. Migration is destructive and non-transactional The migration deletes all Security (mostly improved)Good:
Notes:
Logic / ConsistencyRenewal notices vs weekly summary behave differently:
If renewal notices are product/transactional, that may be fine. If they’re marketing, this is inconsistent with weekly summary and with the preferences UI implying user control.
Code Quality / React / TypeScriptGood:
Minor issues:
Error HandlingGood: API routes return appropriate 400/401/403/500; JSON parse errors handled on POST. Gaps:
Performance
Summary
Recommendation: Fix the welcome upsert to reset preference flags (or use explicit defaults on re-subscribe), wire Blockers before merge: Items 1 and 2 above if monthly stats and re-subscribe flows are in scope for this release. Generated by Cursor CLI workflow — |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot did not run on this PR, so there is no automated review signal to approve against. Human review is recommended before merge given the signed-token auth and newsletter preference API changes. No additional reviewers were assigned (the PR author is the sole repository maintainer).
Sent by Cursor Approval Agent: Pull Request Approver
There was a problem hiding this comment.
Reviewed PR #244 against beta; base is already beta, so no rebase was needed.
Requesting changes before merge. The signed-unsubscribe direction is good, but the current PR still leaves some sent unsubscribe links unsigned, re-subscribe can produce an active row with every delivery flag disabled, and monthlyStatsEnabled is exposed without any enforcement path.
Per the daily-review instructions, I also spun up a repair agent. It pushed repair/pr-244-newsletter-preferences at aec3132b75fb28a1d263df815e6621d0d52392b2; bun run typecheck and focused lint passed there. That repair branch fixes the re-subscribe and signed-unsubscribe gaps it found, but monthly stats remains unresolved because no existing monthly-stats sender was present to wire up.
Sent by Cursor Automation: Coworker
| await prisma.newsletter.upsert({ | ||
| where: { email: record.email }, | ||
| where: { email }, | ||
| update: { isActive: true }, |
There was a problem hiding this comment.
Re-subscribing only sets isActive back to true. If this address previously used the new unsubscribe flow, the row also has weeklySummaryEnabled, monthlyStatsEnabled, and renewalNoticeEnabled set to false, so a new signup becomes active but still receives no mail. Please reset the granular flags to their defaults in this upsert update.
| import { parse } from "csv-parse" | ||
| import { render } from "@react-email/render" | ||
| import { | ||
| createNewsletterUnsubscribeUrl, |
There was a problem hiding this comment.
This helper is imported, but the real send paths below still build bare /api/email/unsubscribe?email=... links. With the new unsubscribe route requiring a signed token or logged-in session, those emailed links will fail for normal recipients; the bulk/test sends should use this helper too.
| <> | ||
| {renderToggle("newsletter-status", copy.statusLabel, "isActive")} | ||
| {renderToggle("weekly-summary", copy.weeklySummaryLabel, "weeklySummaryEnabled")} | ||
| {renderToggle("monthly-stats", copy.monthlyStatsLabel, "monthlyStatsEnabled")} |
There was a problem hiding this comment.
This exposes monthlyStatsEnabled as a user-facing opt-out, but I could not find a monthly-stats sender or query that reads this flag. Until it is enforced (or hidden until the sender exists), users can disable this preference with no effect.
2 participants
Summary
Consolidates the overlapping work from PR #78 and PR #126 (both targeting issue #75) into a single feature branch rebased on the latest
beta.Users can now manage newsletter subscription preferences instead of only unsubscribing entirely.
Problem
Issue #75: users could unsubscribe from emails but could not manage individual subscription types:
PR #78 and PR #126 both attempted to solve this with divergent approaches (different field names, auth models, and incomplete cron enforcement). Neither was merge-ready against current
beta.Solution
Database
NewsletterwithweeklySummaryEnabled,monthlyStatsEnabled, andrenewalNoticeEnabled(all defaulttrue)Preference management UI
/newsletter: functional preferences form (replaces "coming soon")/dashboard/settings: same form for authenticated usersToggles:
isActive)API —
GET/POST /api/email/preferencesDual authentication:
/newsletter?email=…&token=…(24h expiry)Uses shared
lib/prismaclient and strict boolean validation on POST.Security —
lib/newsletter-email.tsCentralized HMAC-SHA256 signing for:
?email=linksSecret:
NEWSLETTER_UNSUBSCRIBE_SECRET(falls back toCRON_SECRET; dev fallback in non-production).Cron / email enforcement
/api/cron): only sends to subscribers withisActive+weeklySummaryEnabledisActive+renewalNoticeEnabledisActiveAll email templates and admin newsletter sends now use signed unsubscribe URLs.
Supersedes
This PR replaces and closes:
@prisma/client, different field names, no cron enforcement, largepackage-lock.jsonchurn)monthlyStatsEnabledin form/API, committed generated prisma noise)Testing
bun run typecheckOPENAI_API_KEY=dummy bun run build/dashboard/settings/newsletter?email=…&token=…and persists togglesMigration notes
Run
bunx prisma db pushor apply migration20260624120000_newsletter_preferencesbefore deploy.Ensure
NEWSLETTER_UNSUBSCRIBE_SECRET(orCRON_SECRET) is set in production so signed links work consistently across instances.Closes #75