feat(docs): add newsletter subscription documentation - #2685
Maciej D (mdanilowicz) wants to merge 1 commit into
Conversation
…ted documentation
Patryk Tomczyk (patzick)
left a comment
Patryk Tomczyk (patzick)
left a comment
There was a problem hiding this comment.
🔍 Code Review
✅ Verdict
Approve. Docs-only recipe, structurally consistent with login.md / wishlist.md, CI green, no conflicts. I verified every factual claim against the source: useNewsletter exports and per-instance newsletterStatus ref, the isNewsletterSubscriber / confirmationNeeded derivations, newsletterUnsubscribe returning void, getStorefrontUrl(), the auth schemes of all four routes (ApiKey vs ApiKey + ContextToken), the option semantics (direct/subscribe/unsubscribe/confirmSubscribe), the NewsletterStatus union, and the absence of any captcha/honeypot in the shipped forms. All accurate. Every SchemaTypeTooltip key resolves against apps/docs/src/.data/schema/storeApiSchema.json, and packages/composables/useNewsletter.html resolves via the [composable].paths.ts dynamic route.
📋 Findings
No blockers or majors. Five minors/nits left as inline comments — pick them up as you like:
newsletter.md:138— the subscribe response is reachable as a return value, but the doc never says so; that matters because it is the only status a guest ever gets.newsletter.md:145—UNSUBSCRIBE_KEYis listed without saying how it is used.newsletter.md:220—??onuser.value?.emaildiverges from the templates'|| "".newsletter.md:308/:318— "both starter templates" / "the templates" overstates:vue-starter-template-extendedhas neither page.newsletter.md:316— SSR consequence is understated (throws, not falls back).
📝 Other notes
The PR is still a draft and the description is the unmodified template — worth filling in before marking ready.
autofix: skipped (not my PR — re-run with --autofix to fix it here)
| | Step | Code | Store API | Type | | ||
| | ------------------------- | ---------------------------------------------------------------- | ------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | | ||
| | Subscribe an address | `newsletterSubscribe({ email, option: SUBSCRIBE_KEY })` | `POST /newsletter/subscribe` | <SchemaTypeTooltip type-key='operations["subscribeToNewsletter post /newsletter/subscribe"]["body"]' /> | | ||
| | Read the returned status | `newsletterStatus` | `POST /newsletter/subscribe` | <SchemaTypeTooltip type-key='operations["subscribeToNewsletter post /newsletter/subscribe"]["response"]' /> | |
There was a problem hiding this comment.
newsletterSubscribe also returns result.data ({ success, status }, typed as the operation response), not just writes into newsletterStatus. The doc declares NewsletterSubscribeResponse at line 168 but never shows that anything resolves with it.
This matters given line 320 ("A guest has no way to read a status. After a guest subscribe, the only status you have is the one returned by that single request") — a reader following that advice has no documented way to reach it. Worth a row or a sentence: const { status } = await newsletterSubscribe({ ... }).
|
|
||
| ## Composables | ||
|
|
||
| - `useNewsletter`: the whole feature except the confirmation call. Exposes `newsletterSubscribe`, `newsletterUnsubscribe`, `getNewsletterStatus`, `newsletterStatus`, `isNewsletterSubscriber`, `confirmationNeeded`, `SUBSCRIBE_KEY`, and `UNSUBSCRIBE_KEY`. `SUBSCRIBE_KEY` is `"subscribe"` and `UNSUBSCRIBE_KEY` is `"unsubscribe"`, the two `option` values the composable ships; the Store API also documents `direct` and `confirmSubscribe`. |
There was a problem hiding this comment.
UNSUBSCRIBE_KEY is exposed but the doc never says what to do with it, and the natural reading — that newsletterUnsubscribe sends it — is wrong: newsletterUnsubscribe(email) posts to /newsletter/unsubscribe with { email } only and never touches option.
The only way to use UNSUBSCRIBE_KEY is newsletterSubscribe({ email, option: UNSUBSCRIBE_KEY }), which goes to /newsletter/subscribe. One clause saying that would close the gap.
| // storefrontUrl is added by the composable, never by the form. | ||
| // The response status is written to newsletterStatus, so no reload here. | ||
| await newsletterSubscribe({ | ||
| email: user.value?.email ?? email.value, |
There was a problem hiding this comment.
?? only falls back on null/undefined, so an empty-string user.email is submitted as "" and the Store API rejects it. Both templates use user.value?.email || "" here (templates/vue-starter-template/app/pages/account/index.vue:24, templates/vue-demo-store/app/pages/account/index.vue).
Separately, in the logged-in branch the email.value fallback is unreachable — the email ref is only bound in the guest form — so || (or dropping the fallback in that branch) reads more honestly. Same on line 235.
|
|
||
| Subscribing changes nothing about the sales channel session. The recipient is identified by the email address in the body, not by `sw-context-token`, and `useNewsletter` calls neither `refreshSessionContext()` nor `refreshCart()` because no context value, price, or cart line depends on a newsletter subscription. | ||
|
|
||
| `getNewsletterStatus()` is the exception. `readNewsletterRecipient post /account/newsletter-recipient` is declared with the context token in the schema and returns an `AccountNewsletterRecipient`, the status of the customer behind the current session. Call it after login, as both starter templates do on the account overview. |
There was a problem hiding this comment.
"both starter templates" is imprecise — the two account pages that call getNewsletterStatus() are in vue-demo-store and vue-starter-template; vue-starter-template-extended is also a starter template and has neither. Naming the two templates avoids the ambiguity.
|
|
||
| ## Edge Cases | ||
|
|
||
| - `getStorefrontUrl()` reads `window.location.origin` unless `devStorefrontUrl` is configured, so trigger `newsletterSubscribe` from a client-side handler and set `devStorefrontUrl` for local development, where the origin is `localhost` and matches no sales channel domain. |
There was a problem hiding this comment.
Understates the consequence. getStorefrontUrl() is devStorefrontUrl ?? window.location.origin ?? "" — with no devStorefrontUrl, evaluating it on the server hits an undefined window and throws a ReferenceError; it does not fall back to "" (the ?? "" is dead in that path).
So calling newsletterSubscribe from useAsyncData/callOnce/setup-time code crashes SSR rather than degrading. Saying "throws during SSR" makes the "client-side handler" advice land.
|
|
||
| - `getStorefrontUrl()` reads `window.location.origin` unless `devStorefrontUrl` is configured, so trigger `newsletterSubscribe` from a client-side handler and set `devStorefrontUrl` for local development, where the origin is `localhost` and matches no sales channel domain. | ||
| - In a multi-domain or multi-language sales channel, `storefrontUrl` decides which domain the confirmation link points at. Subscribing from the wrong origin sends the customer to a domain that may not serve your confirmation route. | ||
| - No composable wraps `confirmNewsletter post /newsletter/confirm`. The templates handle it in `app/pages/newsletter-subscribe.vue` by reading `em` and `hash` from the query and calling `apiClient.invoke` directly. Without such a page, double opt-in subscriptions never activate. |
There was a problem hiding this comment.
"The templates" reads as all of them — only vue-demo-store and vue-starter-template ship app/pages/newsletter-subscribe.vue. Worth scoping the sentence to those two.
2 participants
Description
Type of change
ToDo's
Screenshots (if applicable)
Additional context