Skip to content

test(login): cover the select field branch in every branding design - #935

Closed
fforootd wants to merge 1 commit into
mainfrom
claude/adoring-cannon-944372
Closed

fforootd wants to merge 1 commit into
mainfrom
claude/adoring-cannon-944372

Conversation

@fforootd

Copy link
Copy Markdown
Member

Summary

  • The Liquid field-rendering loop ({% for f in fields %} with a {% case f.type %} dispatch over checkbox / select / default input) is duplicated byte-identically across six templates: the built-in packages/components/src/orchestrator/templates/default.liquid plus the five ejectable branding designs under packages/config/defaults/branding/*/login.liquid. The duplication is deliberate — each design is self-contained so zitadel branding eject --design X writes a standalone file (ADR 037, feat: tenant login templates as editable config (ADR 037) #563).
  • The catalog parity test loops every design in BRANDING_DESIGNS, but its shared fixture declared only email (type email) and remember (type checkbox). The select arm — the one that renders <zl-select options='{{ f.validation.enum | selectOptions | json }}'> — was therefore never reached for the five branding designs; it was covered only through default.liquid's own tests. A hand-edit that broke one design's select branch shipped green.
  • Adds a required select field with a closed validation.enum to the fixture step and asserts, per design, that the rendered <zl-select> carries its localised label, its required flag, and options built from the enum. Existing email/checkbox assertions are unchanged.
  • The consumes its mandatory_gates marker test gains a duplicate check for the new field: a required select must not trip the safety net into appending a generic <zl-field> alongside it. That regression was previously guarded only in isolation in mandatory-gates.spec.ts; it is now proven per design through the real pipeline.

No shared partial was extracted — that is a separate open design question, and the six copies stay as they are.

Validation

  • moon run components:test — 33 files, 425 tests passed (32 in design-templates.spec.ts, up from 27).
  • moon run components:typecheck components:lint — clean; no new lint warnings attributable to the spec.
  • pnpm exec oxfmt --check packages/components/src/orchestrator/design-templates.spec.ts — formatted.
  • Mutation-tested to prove the assertion is not vacuous, restoring each template afterwards (git status clean, only the spec is modified):
    • dropped options='…' from minimal/login.liquid → exactly one failure, minimal only (expected null to deeply equal [ Array(2) ]);
    • deleted the whole {% when 'select' %} arm from split/login.liquid → exactly one failure, split only (expected null not to be null — the field falls through to the <zl-field> default arm).

Release notes / changeset

  • No changeset required — no shipped behavior changed. The PR is exclusively a *.spec.ts change (.changeset/README.md#decision-table, row 3), which is also why the title type is test.

Notes

  • The assertion parses the rendered HTML into a fragment and reads the decoded options attribute rather than string-matching it. The value passes through two escaping hops — Liquid runs with outputEscape: "escape", so the | json output is HTML-encoded before the sanitiser's serialiser re-encodes it — and a JSON.parse round-trip is both stronger and immune to that churn.
  • options is already on zlSelectManifest.attrs, so it survives the DOMPurify pass; the test would catch it if that ever changed.
  • Fixture naming (department, enum ["Engineering", "Support"]) matches the existing select fixture idiom in embedding.browser.spec.ts.
  • Setup note for anyone reproducing locally in a fresh worktree: moon run components:test fails with tsx/tsdown/orval: command not found until pnpm install --frozen-lockfile has been run.

The Liquid field-rendering loop with its `{% case f.type %}` dispatch is
duplicated byte-identically across six templates — the built-in
`default.liquid` plus the five ejectable branding designs, which are
deliberately self-contained so `zitadel branding eject --design X` writes a
standalone file (ADR 037).

The catalog parity test loops every design, but its shared fixture declared
only `email` and `remember`, so the `select` arm — the one that serialises
`f.validation.enum` into `<zl-select options=...>` — was never reached for
the five branding designs. It was covered only through `default.liquid`, so
a hand-edit that broke one design's select branch shipped green.

Add a required `select` field with a closed enum to the fixture and assert,
per design, that the rendered `<zl-select>` carries its localised label, its
required flag, and options built from the enum. The assertion parses the
HTML and reads the decoded attribute rather than string-matching, because
the value passes through two escaping hops (Liquid's `outputEscape` encodes
the `| json` output, then the serialiser re-encodes it).

The mandatory-gates test also gains a duplicate check for the new field: a
*required* select must not trip the safety net into appending a generic
`<zl-field>` alongside it. That regression was previously guarded only in
isolation in `mandatory-gates.spec.ts`; it is now proven per design through
the real pipeline.

No shared partial extracted — the six copies stay as they are.
Copilot AI lite review requested due to automatic review settings August 20, 2026 23:36
@vercel

vercel Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
nextgen Ready Ready Preview Aug 20, 2026 11:36pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
nextgen-docs Skipped Skipped Aug 20, 2026 11:36pm
nextgen-mock-zitadel Skipped Skipped Aug 20, 2026 11:36pm

Request Review

@zitadel-kanban zitadel-kanban Bot moved this to Inbox in Engineering Kanban Aug 20, 2026
@fforootd fforootd self-assigned this Aug 20, 2026
@vercel
vercel Bot temporarily deployed to Preview – nextgen August 20, 2026 23:36 Inactive

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@fforootd

Copy link
Copy Markdown
Member Author

relates to #398

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ No Changeset found

Latest commit: 000124d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

This branch was previously deployed

3 inactive deployments
Preview – nextgen — 000124df Deployed Aug 20, 2026 by vercel[bot]
Preview – nextgen-docs — 000124df Deployed Aug 20, 2026 by vercel[bot]
Preview – nextgen-mock-zitadel — 000124df Deployed Aug 20, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants