[thomasbeaudry]

Summary

An instance can disable a language, but the email surfaces ignored that: the template editors
offered every language the application ships, and the send pickers chose from the same list. An
admin could author copy in a language nothing would ever send, and the welcome template's
validation demanded a language no reader is offered.

Four commits, scoped so the changelog reads one entry each:

• fix(schemas) — Spanish added to both built-in templates (welcome and assignment). This changes
  what the API sends.
• fix(web) — the template editors and the welcome template's validation follow the instance's
  active languages.
• fix(web) — the assignment and welcome send pickers do the same. A template authored only in
  disabled languages keeps its own: the copy exists nowhere else, so scoping it away would leave
  the template unsendable.
• fix(web) — the active-language checkboxes on the settings page are listed by their translated
  name rather than the canonical language order.

Review notes

One deviation from the review's suggestion for the e2e placement. The new-user template editor is
behind isMailEnabled, and mail.spec.ts documents that it owns that flag for the whole suite, so
that assertion lives there rather than in admin-settings.spec.ts. To keep it stable against the
Spanish toggling in admin-settings.spec.ts, it asserts the picker offers no language outside the
set returned by a new ApiClient.findActiveLanguages(), read in the same test — a subset check
rather than an exact list. The group-editor and checkbox-order assertions are in
admin-settings.spec.ts as suggested, the former inside the serial active languages block.

Test plan

• pnpm lint — 34/34 tasks
• pnpm test:coverage — 412 files, 4732 tests, 100% statements/branches/functions/lines
• pnpm test:e2e — 293 passed. Two unrelated failures locally: record-file-deletion
  (local .env has STORAGE_ENABLED=false; CI generates from .env.template, which has it
  on) and subject-detail, which timed out under full parallel load and passes on its own.
• Each fix proven load-bearing by reverting it: 10 unit tests and all 3 new e2e tests fail
  without the change.
• tsc run against apps/web, packages/schemas and testing at each of the four commits.

🤖 Generated with Claude Code

[gdevenyi]

The lint-and-test check fails, so review is on hold.

@opendatacapture/web#lint stops at tsc. The new required activeLanguages prop is not passed in the existing unit tests:

• apps/web/src/components/GroupEmailTemplates/__tests__/EditTemplateDialog.test.tsx (line 35)
• apps/web/src/components/GroupEmailTemplates/__tests__/ViewDefaultTemplateDialog.test.tsx (lines 18, 23, 33, 40)

Please pass activeLanguages in these tests, run pnpm lint and pnpm test:coverage locally, and push. Review continues when CI is green.

Reviewed at commit a338e3b.

[joshunrau]

Thanks for this. Scoping the template editors to the active languages is the right fix. A few things before it can go in:

1. The send pickers still offer disabled languages, and one now offers Spanish for the first time. The built-in assignment template now includes Spanish, so the "send by email" language picker in AssignmentEmailForm.tsx (lines 58-62) offers Spanish even when Spanish is disabled. Before this PR it only offered English and French. Please intersect its options with setupStateQuery.data.activeLanguages, and handle a template authored only in disabled languages. Please also pass options={setupStateQuery.data.activeLanguages} to the welcome-email picker in routes/_app/admin/users/create.tsx:132. Otherwise an admin can send the welcome email in a language whose template the mail page no longer shows. Add a unit test for each.
2. The tests don't catch the scoping being removed. I removed languages={activeLanguages} from the create, edit and new-user editors and put the new-user validation back to every language, and all 29 tests in those three files still passed.
   • NewUserTemplateSection.test.tsx:102 passes either way, because the component fills Spanish and French from the defaults. Its description also says "English and French" but it passes ['en']. Blank an inactive language (es: '') with activeLanguages: ['en', 'fr'] so it fails without the change.
   • EditTemplateDialog.test.tsx:20 says a test there tells a scoped picker from an unscoped one, but there isn't one yet. Please add the picker assertion from ViewDefaultTemplateDialog.test.tsx:50 there, and in CreateTemplateForm.test.tsx and NewUserTemplateSection.test.tsx.
3. Stale comment: NewUserTemplateSection.tsx:40-41 still says every language must be filled in. It's every active language now.
4. The settings sort needs a test. Nothing in routes/_app/admin/__tests__/settings.test.tsx checks the checkbox order. In English the sorted order (English, French, Spanish) differs from the LANGUAGES order, so one assertion is enough.
5. E2E test. Every change needs one in testing/. The e2e instance is seeded with English and French only, so you can assert that the template-editor and send-form pickers don't offer Spanish. You'll need *-language locators in testing/src/pages/_app/group/email-templates.page.ts and testing/src/pages/_app/admin/mail.page.ts. admin-settings.spec.ts:100 turns Spanish on temporarily and Playwright runs in parallel locally, so put the assertion in that file's serial active languages block to keep it stable.
6. Commit history. PRs are merged with their commits intact and the changelog gets one entry per commit. Please split a338e3b into separately scoped commits, e.g.:
   • fix(schemas): add Spanish to the default email templates. This changes what the API sends, so it deserves its own changelog entry.
   • fix(web): scope email template languages to active languages, with the test fix from a1c3c0e folded in so every commit passes tsc.
   • A separate commit for the settings sort.

If you hand this to Claude Code, Opus 5 is the right size. The work spans components, a route, unit tests and the Playwright suite, and item 1 needs a judgment call on the fallback.

Reviewed at commit a1c3c0e.