feat(mrf-cutover): default DuplicateFormModal to MRF with storage-mode escape hatch (5/6)#9466
Conversation
| switch (responseMode) { | ||
| case FormResponseMode.Encrypt: | ||
| case FormResponseMode.Encrypt: { | ||
| const cutoverDefaultEmails = adminEmail ? [adminEmail] : [] |
There was a problem hiding this comment.
Post-cutover, the storage-mode duplication path is reached via the escape hatch, and CreateFormStorageModeScreen does not render an email recipients input — so emails on the wizard form is undefined. The pre-cutover dupe path used emails.filter(Boolean) which would crash here. Mirrored the exact emails ? emails.filter(Boolean) : cutoverDefaultEmails guard from CreateFormWizardProvider.tsx:131 so the two flows stay symmetric and a future reader can grep for one pattern.
Alternatives considered
- Default
emailsto[]: rejected — admin would receive no email notifications on the duplicated storage form, diverging from create-flow behaviour for no reason. - Add an email input to
CreateFormStorageModeScreen: rejected — out of scope for this issue and would change the Figma-specified UX.
d7024b6 to
4db88d8
Compare
8421393 to
ce0f849
Compare
| publicKey: keypair.publicKey, | ||
| workspaceId, | ||
| emails: emails.filter(Boolean), | ||
| emails: emails ? emails.filter(Boolean) : cutoverDefaultEmails, |
There was a problem hiding this comment.
Post-cutover, the storage-mode duplication path is reached via the escape hatch, and CreateFormStorageModeScreen does not render an email recipients input — so emails on the wizard form is undefined. The pre-cutover dupe path used emails.filter(Boolean) which would crash here. Mirrored the exact emails ? emails.filter(Boolean) : cutoverDefaultEmails guard from CreateFormWizardProvider.tsx:131 so the two flows stay symmetric and a future reader can grep for one pattern.
Alternatives considered
- Default
emailsto[]: rejected — admin would receive no email notifications on the duplicated storage form, diverging from create-flow behaviour for no reason. - Add an email input to
CreateFormStorageModeScreen: rejected — out of scope for this issue and would change the Figma-specified UX.
…e mode When mrf-cutover is on, the create-form details screen no longer offers a response-mode choice — Multirespondent is the implicit default. An "old version of FormSG" link composed from the user's beta flags navigates to a new storage-mode-only screen (title + create + back). Submitting routes through the existing storage-mode mutation. Closes #9453. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Wrap escape-hatch link in an info InlineMessage to match Figma - Storage-mode page header reads "Set up a Storage mode form" - Add storage-mode subtitle explaining that storage mode is outdated Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… spacing in cutover flow The MRF default screen previously showed both the escape-hatch infobox and the existing data-classification infobox, cluttering the layout. The cutover Figma drops the latter and uses 2.5rem rhythm around the escape-hatch infobox and between input/button on the storage-mode page. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4db88d8 to
9965cf9
Compare
Covers the dupe-flow contract introduced by #9454 ACs 1 and 6: the wizard defaults responseMode to Multirespondent regardless of the source form's mode, and the default submit fires the multirespondent dupe mutation. These tests are a regression net on top of the shared cutover behaviour added in #9453. Refs #9454 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Post-cutover, the escape-hatch storage screen has no email-recipients input, so the wizard form's `emails` field is undefined on submit and the existing `emails.filter(Boolean)` would crash. Mirror the `emails ? filter : cutoverDefaultEmails` guard used by the create flow (CreateFormWizardProvider.tsx) so the two paths stay symmetric. Refs #9454 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
No stories existed for DuplicateFormModal. Mirror the CreateFormModal.stories.tsx variants so Chromatic can baseline the post-cutover dupe modal: pre-cutover Default, MrfCutoverOn, and the three beta-flag copy variants (children, webhook v1, both). Closes AC8 for #9454. Refs #9454 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ce0f849 to
34806ce
Compare
feat(mrf-cutover): default UseTemplateModal to MRF with storage-mode escape hatch (6/6)
Resolve conflicts from create-form-modal-cutover commits now in develop: - 380351f (emails-undefined type fix): take develop's (emails ?? defaultEmails).filter(Boolean) in the three wizard providers (UseTemplate, CreateForm, DupeForm) — functionally identical to the branch's cutoverDefaultEmails guard but type-safe. - 3ea0349 (FormTitleInput abstraction): take develop's <FormTitleInput /> in CreateFormDetailsScreen/CreateFormStorageModeScreen; this branch made no changes to those files. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| </GrowthBookProvider> | ||
| ) | ||
|
|
||
| export const MrfCutoverOn = Template.bind({}) |
There was a problem hiding this comment.
issue (spec gap, confidence 100): no story opens the escape hatch, so the storage-mode page is never rendered or snapshotted.
What's happening
- Every story here (and in
UseTemplateModal.stories.tsx) is a bareTemplate.bind({})with noplayfunction. - The storage-mode screen (title input + create button) is only reachable by clicking the "old version of FormSG" link, which no story performs.
- So the cutover stories baseline only the MRF default screen and the beta-copy variants.
Why it matters
Issue #9454 explicitly asks for stories covering MRF default, escape-hatch open, beta-flag copy variants, and "clicking the link navigates to a storage-mode page (title input + create button)". The escape-hatch-open state and its Chromatic baseline are missing.
Suggestion
Add a story with a play function that clicks the escape-hatch link (via userEvent + within) so the storage-mode page renders and gets a Chromatic snapshot. Applies to both DuplicateFormModal and UseTemplateModal stories.
🤖 This comment was generated by an AI code review. Please verify before acting on it.
| expect(dupeStorageModeFormMutation.mutate).not.toHaveBeenCalled() | ||
| }) | ||
|
|
||
| it('fires the storage dupe mutation with [user.email] fallback after escape hatch + submit', async () => { |
There was a problem hiding this comment.
issue (spec gap, confidence 85): the "back to MRF default" path is untested.
What's happening
- This test (and the matching one in
UseTemplateWizardProvider.cutover.test.tsx) callsgoToStorageModeDetails()then submits — only the forward path. - Neither test calls
goToMrfDetails()(defined inCreateFormWizardProvider.tsx:64-67, which resetsresponseModeto Multirespondent and returns to theDetailsstep) nor asserts the step returns.
Why it matters
Issue #9454 lists "a back affordance returns the user to the MRF default screen" as acceptance criteria. The behaviour exists but is unpinned at the unit layer.
Suggestion
Add one assertion: after goToStorageModeDetails(), call goToMrfDetails() and expect result.current.currentStep to equal CreateFormFlowStates.Details.
🤖 This comment was generated by an AI code review. Please verify before acting on it.
| features: { [featureFlags.mrfCutover]: { defaultValue: true } }, | ||
| }) | ||
|
|
||
| const withCutover = (Story: StoryFn) => ( |
There was a problem hiding this comment.
suggestion (non-blocking, confidence 80): move the withCutover decorator into ~utils/storybook.
What's happening
This mrfCutoverOn = new GrowthBook(...) + withCutover block is duplicated verbatim in UseTemplateModal.stories.tsx, and the same construct appears inline in CreateFormModal.stories.tsx and SettingsWebhooksPage.stories.tsx.
Context
~utils/storybook.tsx is already the home for shared story decorators — fullScreenDecorator and LoggedInDecorator, both imported in this file, live there.
Suggestion
Export a mrfCutoverDecorator (or withGrowthbookFeature(flag)) from ~utils/storybook and have these two new files be its first consumers.
Benefits
- The flag setup changes in one place as the cutover evolves.
- Future modal/page stories get cutover scaffolding with one import.
Cleaning up the other two files is out of scope here — fine as a follow-up.
🤖 This comment was generated by an AI code review. Please verify before acting on it.
| DuplicateFormModalProps, | ||
| } from './DuplicateFormModal' | ||
|
|
||
| const getDashboardResponse = () => |
There was a problem hiding this comment.
polish (non-blocking, confidence 75): reuse the existing dashboard handler instead of an inline one.
The repo already exports getAdminForms.empty() (apps/frontend/src/mocks/msw/handlers/admin-form/form.ts), which serves the same /api/v3/admin/forms route returning [], typed as AdminDashboardFormMetaDto[] (the inline version here is untyped). It's already used this way in TransferOwnershipModal.stories.tsx.
Suggestion
Import getAdminForms and use getAdminForms.empty() in baseMsw and the variant msw arrays, dropping the local getDashboardResponse.
(Flagged below the review's auto-post bar — surfaced because you asked for all findings; safe to skip.)
🤖 This comment was generated by an AI code review. Please verify before acting on it.
Problem
Per the PRD: when admins duplicate an existing form, the duplicate flow currently inherits the source form's response mode, locking admins into storage mode whenever they copy a legacy form. Closes #9454.
Solution
Under the
mrf-cutoverflag:DuplicateFormModalto Multirespondent, regardless of the source form'sresponseMode.dupeStorageModeFormmutation path.DupeFormWizardProvider.cutover.test.tsxpinning the MRF default + dispatch under cutover.When the flag is off, the original duplicate flow renders unchanged.
Alternatives considered
We considered defaulting
emailsto[]so the duplicate form would simply have no notification recipients, but decided against it because the admin would receive no email notifications on the duplicated storage form — diverging from create-flow behaviour for no good reason. We also considered adding an email input toCreateFormStorageModeScreen, but that is out of scope for this issue and would change the Figma-specified UX.Breaking Changes
No - gated behind
mrf-cutoverfeature flag.Tests
TC1: storage-mode source → MRF default
TC2: escape hatch from duplicate flow
TC3: flag off
TC4: storage-mode template → MRF default
TC5: escape hatch from use-template flow
TC6: flag off