Skip to content

feat(docs): add registration page and update sidebar and account documentation page - #2689

Draft
Maciej D (mdanilowicz) wants to merge 1 commit into
mainfrom
feat/registration-recipes
Draft

Maciej D (mdanilowicz) wants to merge 1 commit into
mainfrom
feat/registration-recipes

Conversation

@mdanilowicz

Copy link
Copy Markdown
Contributor

Description

Type of change

ToDo's

Screenshots (if applicable)

Additional context

@patzick Patryk Tomczyk (patzick) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Code Review

Docs-only PR. Structure, frontmatter and sidebar match the existing login/wishlist recipes, all SchemaTypeTooltip type keys resolve against storeApiSchema.json, all Related Links targets exist, and the useUser().register() behaviour described (storefrontUrl injection, active && !doubleOptInRegistration gate, awaited refreshSessionContext(), no refreshCart(), isLoggedIn formula, rethrow on context failure) is accurate against packages/composables.

Six findings, all inline. One is a wrong technical claim (window.location.origin during SSR), the rest are accuracy/completeness gaps in a page whose value is being exactly right.

Note on CI: every check on this PR is green, but code-check.yml has paths-ignore: ["apps/**", "**.md"], so Build, Typecheck, lint and tests did not run on this diff — the passing lint check belongs to the Check PR title workflow. The new 493-line Vue-in-markdown page has therefore never been compiled by CI; please run the docs build locally before marking ready.

- The generated `billingAddress` type is `Schemas["CustomerAddress"]`, which requires `id` and `customerId`. Send placeholder values as both starter templates do, because you cannot know the ids of an address that does not exist yet.
- A registration with double opt-in enabled resolves successfully while leaving `isLoggedIn` false. Branch on the returned `doubleOptInRegistration` flag instead of assuming a session exists after the promise resolves.
- `refreshSessionContext()` rethrows after logging, so a failing `readContext get /context` rejects the `register()` promise even though the customer was already created. A retry then hits `VIOLATION::CUSTOMER_EMAIL_NOT_UNIQUE`.
- `getStorefrontUrl()` falls back to `window.location.origin`, which is empty during server-side rendering. Submit the form from the client, and set `devStorefrontUrl` when the Shopware domain differs from the origin your app runs on. An origin that is not a configured sales channel domain comes back as a constraint violation pointing at `/storefrontUrl`, which no field in your form owns.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

major — wrong SSR failure mode. getStorefrontUrl() is return devStorefrontUrl ?? window.location.origin ?? ""; (useInternationalization.ts:92-94). On the server window is not defined, so this throws a ReferenceError — it does not return an empty string. The advice that follows is right; only the stated mechanism is wrong.

Suggested: "…falls back to window.location.origin, which is not available during server-side rendering, so calling register() on the server throws."

}
);

// The generated body is a union, so accountType is a literal and vatIds can be

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor — the payload type is not a union. Omit<T, K> is Pick<T, Exclude<keyof T, K>>, which flattens Body & (private | business) into a single object type. On RegisterPayload the fields end up as accountType: "private" | "business", company: string | null | undefined, vatIds: [string, ...string[]] | null | undefined, so form.accountType = "business" and form.vatIds = [v] compile directly (verified with tsc 5.9 --strict).

Two consequences worth stating instead:

  • the computed models are a convenience for v-model + nullable fields, not a type requirement;
  • the discrimination the Types section describes is lost on RegisterPayload, so nothing stops you sending accountType: "business" without a company — that is enforced by the Store API, not the compiler.

| Step | Code | Store API | Type |
| -------------------------------- | ---------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- |
| Load salutation options | `fetchSalutations()` | `POST /salutation` | <SchemaTypeTooltip type-key='operations["readSalutation post /salutation"]["response"]' /> |
| Load countries and their states | `fetchCountries()` | `POST /country` | <SchemaTypeTooltip type-key='operations["readCountry post /country"]["response"]' /> |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor — the route depends on cacheableReads. Both composables switch to the GET variants when cacheableReads is set on the Shopware context:

  • useSalutations.ts:34-36readSalutationGet get /salutation
  • useCountries.ts:45-51readCountryGet get /country

So POST /salutation / POST /country in this table (and the two operations entries in the frontmatter) only describe the non-cacheable path. Worth listing both, as the rest of the page is precise about which route runs.


## Edge Cases

- The generated `billingAddress` type is `Schemas["CustomerAddress"]`, which requires `id` and `customerId`. Send placeholder values as both starter templates do, because you cannot know the ids of an address that does not exist yet.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor — only one template does this. templates/vue-starter-template/app/components/account/RegistrationForm.vue:51-53 sends id: "" / customerId: "", but templates/vue-demo-store/app/components/account/RegistrationForm.vue:43-50 omits both (its state is untyped) and registers fine. So the placeholders are a way to satisfy the generated CustomerAddress type, not something the Store API needs. Please reword to say that, and drop "both starter templates".

lastName: "",
email: "",
password: "",
company: "",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor — company placement diverges from both shipped forms. The example puts company at the top level of the body (which matches the business branch of the generated type), while vue-starter-template binds state.billingAddress.company and vue-demo-store binds state.billingAddress.company too. These are two different fields (customer vs. address). Since the recipe's job is to explain what Shopware does with the body, one sentence on which one the register route actually consumes — and why the templates use the address field — would prevent readers from getting a different result than the templates they copy from.

## Composables

- `useUser`: exposes `register`, which returns the created `Schemas["Customer"]`, plus `user`, `isLoggedIn`, `isCustomerSession`, `isGuestSession`, and `refreshUser` for the state you render afterwards.
- `useSalutations`: exposes `getSalutations` and `fetchSalutations`. It fetches the list on mount when nothing has been provided yet and shares it through the `swSalutations` injection, so several forms on one page issue one request.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit — the option lists are client-only. Both useSalutations and useCountries load via onMounted (useSalutations.ts:52, useCountries.ts:81), so getSalutations and getCountriesOptions are empty during SSR and the selects render with only the placeholder option on the server. Edge Cases covers SSR for storefrontUrl but not for these; a line here (or an explicit fetchSalutations()/fetchCountries() call in the example) would close the gap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants