Skip to content

Commit e95fb22

Browse files
chore(plans): close out welcome-notification (PR #99)
Closeout: flip to done, tick validations (8 new tests + 310/310 API tests, lint + type-check clean), fill Notes (encodeURIComponent defensive slug encoding, fire-and-forget spy timing works because the promise is queued synchronously, no new env needed since CFP_SITE_HOST already in place) + Follow-ups (Slack #welcome bump when Slack DM lands, email verification reminder via bounce webhook, A/B copy if marketing wants). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 64c517a commit e95fb22

1 file changed

Lines changed: 18 additions & 9 deletions

File tree

plans/welcome-notification.md

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
---
2-
status: in-progress
2+
status: done
33
depends: []
44
specs:
55
- specs/api/auth.md
66
issues: [43]
7+
pr: 99
78
---
89

910
# Plan: Welcome notification on fresh OAuth signup
@@ -84,12 +85,12 @@ Wiring-level test: the existing `github-oauth.test.ts` covers the `create-fresh`
8485

8586
## Validation
8687

87-
- [ ] Three Notifier impls (interface + LoggingNotifier + EmailNotifier) all have the new method.
88-
- [ ] EmailNotifier sends with the right Resend payload (subject + text + html + to).
89-
- [ ] Welcome template HTML-escapes `fullName`.
90-
- [ ] OAuth `create-fresh` path fires the notifier (verified via spy in github-oauth.test.ts).
91-
- [ ] Existing OAuth tests continue to pass (the notifier call is fire-and-forget; doesn't change response shape).
92-
- [ ] `npm run type-check && npm run lint && npm test` clean.
88+
- [x] Three Notifier impls (interface + LoggingNotifier + EmailNotifier) all have `notifyWelcomeOnSignup`.
89+
- [x] EmailNotifier sends with the right Resend payload (subject + text + html + to).
90+
- [x] Welcome template HTML-escapes `fullName` (test asserts `<script>` becomes `&lt;script&gt;`).
91+
- [x] OAuth `create-fresh` path fires the notifier via spy assertion in github-oauth.test.ts — payload matches `{ email, fullName, slug }`.
92+
- [x] Existing OAuth tests pass (15 pre-existing + 1 new = 16 in that file).
93+
- [x] 310/310 API tests pass; `npm run type-check && npm run lint` clean.
9394

9495
## Risks / unknowns
9596

@@ -100,8 +101,16 @@ Wiring-level test: the existing `github-oauth.test.ts` covers the `create-fresh`
100101

101102
## Notes
102103

103-
*(filled at done time)*
104+
One-shot implementation across two commits (plan + impl/tests).
105+
106+
Surprises:
107+
108+
- **`encodeURIComponent` for the profile-link slug.** Slugs are `[a-z0-9-]` per the spec so they never need encoding today, but the template uses `encodeURIComponent` defensively. Test asserts that a hypothetical slug with spaces produces `name%20with%20spaces` in the href, which gives the template the same robustness as the existing interest/filled renderers.
109+
- **Spy timing in the OAuth test.** Fire-and-forget worried me — would the spy assertion race the route handler's return? But `void notifier.notifyWelcomeOnSignup(...)` synchronously invokes the function (queuing the promise) before the next statement runs. By the time the route handler resolves and the test awaits the response, the spy has been called at least once. The Resend HTTPS send happens later in the promise chain; we don't await it.
110+
- **No new env needed.** The notifier already had a `siteHost` from `CFP_SITE_HOST` (added in #82's email-notifier wiring); the welcome template reuses it for the profile/projects/help-wanted links. Zero new config.
104111

105112
## Follow-ups
106113

107-
*(filled at done time)*
114+
- **Slack #welcome bump.** Once Slack DM lands (#95), the welcome path could DM the new user with a #welcome channel invite link. Out of scope here; tracked alongside the Slack notifier work.
115+
- **Email verification reminder.** If the GitHub primary email later fails delivery (Resend bounces it), today the user has no recovery path beyond editing their profile. Tied to the bounce-webhook follow-up from #82.
116+
- **A/B-able copy.** The body is currently hardcoded. If marketing ever wants to experiment, lift into a small key/value content table in a future plan. *Deferred* — premature for v1.

0 commit comments

Comments
 (0)