feat(workflow-builder-redesign): person-framed approval control by bookwormsuf · Pull Request #9850 · opengovsg/FormSG

bookwormsuf · 2026-08-13T15:14:12Z

Problem

The approval control reads as a property of the step, not a job a person does. We want to tie the action of approving closer to the person involved in the step.

Solution

The toggle is reworded to tie to the person, the approval block moves above the fields it feeds into, and picking a Yes/No approval field now adds it to the fields list automatically. Storage is unchanged: approval_field (a Yes/No field id) plus edit (an array of field ids), exactly as today.

Reading order now follows the decision: decide whether this person approves, pick their decision field, then see that field in the fields list below. The read-only step card mirrors the same order so a saved step reads the same whether it's open or collapsed.

Auto-assign is deliberately one-way. Picking a field adds it to edit; nothing ever removes an id — not on toggle-off, not on clearing the field, not when switching the approval field from A to B. Adding is a convenience and is trivially undone. Removing would destroy a field the admin may have chosen by hand, in a part of the form they aren't looking at.

Important

Everything is behind useIsWorkflowBuilderRedesign() — the copy, the block reorder in both components, and the auto-assign behaviour. With the flag off, behaviour is byte-identical to before this PR.

Depends on #9849

Reviewers testing locally should merge or check out #9849 first. Without it, auto-save-on-card-switch discards edits to any already-saved step, so approval changes will appear to vanish for reasons unrelated to this PR. Two setValue(..., { shouldDirty: true }) calls in ApprovalsBlock are inert until that lands.

Alternatives considered

step_type schema column (FRM-2488). Shelved: it desyncs live production records without unlocking a new capability. Approval already has a home in the optional approval_field — presence means the step approves. The real problem was UI framing, not storage.
Locking the auto-added chip. Considered a lock icon plus tooltip so the approval field can't be removed from the fields list. Rejected because it needs a change to the shared ~components/Dropdown MultiSelect — MultiSelectItem's close button only takes a component-wide isDisabled, and ComboboxItem has no per-item locked field. Additive and low-risk, but it widens the blast radius app-wide for one ticket. The chip stays removable, with the existing "field not assigned to this person" validation as the backstop.
Auto-removing the field when approval is switched off. Three whole-rule options were weighed: always remove the outgoing field; track provenance and remove only what auto-add added; never remove. Provenance is correct per case but its tracking doesn't survive the card unmounting, so the same action would behave differently in a later session. Always-remove is simple but deletes a Yes/No field an admin may have added by hand as a plain question. Never-remove was chosen: the leftover is visible, one click to remove, and never destroys a deliberate choice. This also removes the need for a confirmation modal or an undo toast.
Question-form labels. The first draft phrased both labels as questions ("Is this person approving…?"). Reversed: a toggle's two states are on and off, not yes and no, so a question invites a "no" answer that does nothing. Both labels are now statements.
Transient helper text. Considered showing "the approval field is added here automatically" only just after the auto-add. Rejected — it explains a permanent state, so an admin returning later would see a chip they don't remember adding and no explanation. It also costs more to build than the persistent version.
Duplicating the JSX per flag branch. The first cut of the reorder wrote both blocks twice per component to swap their order. Replaced with a pair of named sections rendered in either sequence: same output for both flag states, and prop drift between the two copies is no longer possible.

Known gaps (not addressed here)

Empty Yes/No field state. A form with no Yes/No fields shows an empty dropdown rather than a hint to add one. Pre-existing on develop (there's no guard there either), so this PR doesn't introduce it — but moving approval above the fields makes it more prominent. Descoped to the empty-states workstream (FRM-2492).
nextEditFieldsForApproval is a shallow seam. It owns the add rule, but the redesign-flag guard and the never-remove invariant it documents are enforced by its caller. Raising it further was reviewed and judged out of scope here.
The "approval field must be in edit" rule lives in three places — the adder, the sibling block's re-validation, and the validate closure. No single module owns it. A hook owning both directions would fix it; larger than this PR.
Cross-step field overlap is unguarded. Another step's edit may contain this step's approval_field, so an earlier respondent could answer the approver's question. Pre-existing and only ever validated within a single step. Accepted as out of scope: this PR reframes one step's control, not cross-step field governance.

Screenshots

State	Before (flag off)	After (flag on)
Step card, expanded, approval off
Step card, expanded, approval on with field selected
Read-only step card, no approvals
Read-only step card, with approvals

Breaking Changes

No — backwards compatible. No schema change, no migration, no backend change. Existing saved steps already satisfy approval_field ∈ edit because the backend has always enforced it, so they load and validate unchanged. Behind a flag; revert is a straight revert.

Tests

TC1: person-framed copy and layout (flag ON)

On a non-first step, the approval control renders above the fields list
The toggle label reads as a statement about the person, not about the step
The Yes/No selector has a visible label and a required indicator
The first step shows no approval control at all
Collapse the step — the read-only card shows approval above the fields too
Collapse a step with approval off — the card shows no Approvals section at all, not "Approval not required in this step"
Select an approval field, save, then delete that Yes/No field from the form — the collapsed card still shows the Approvals section with the "field was deleted" error

TC2: auto-assign

Turn approval on, pick a Yes/No field → it appears in the fields list immediately, without saving
Helper text "The approval field is added here automatically." shows under the fields list
Pick a field that's already in the fields list → no duplicate chip

TC3: one-way rule (intended behaviour, not a bug)

Switch the approval field from A to B → B is added and A stays in the fields list
Turn approval off → the field stays in the fields list as a normal removable chip
Clear the approval field with the selector's clear control → the fields list is untouched
Turn approval off → the helper text disappears while the chip remains

TC4: validation

Turn approval on, select nothing, Save → "Please select a Yes/No field"
With approval on, remove the auto-added chip → the "not assigned to this person" error shows immediately, not only on Save
Re-add the field → the error clears immediately, without saving
Save a valid approval step, reopen it → toggle, selected field and chip are as left

TC5: flag OFF is unchanged

With the flag off: fields list sits above approval, old copy, tooltips present
With the flag off: picking an approval field does not add it to the fields list, and no helper text appears
Save a step with the flag on, then turn the flag off and reopen it → it renders in the old layout with its data intact

TC6: edge cases

Delete the Yes/No field after selecting it as the approval field → the selector clears, nothing crashes, the step still saves
Open a step saved before this change → it loads, renders and validates with no error

🤖 Generated with Claude Code

Reword the approval control to tie to the person rather than the step, behind useIsWorkflowBuilderRedesign(). Adds a label for the Yes/No selector (previously placeholder-only) and helper text for the fields list explaining the auto-add. Drops the approvals toggle tooltip and the fields-list tooltip under the flag; the fields-list tooltip key is removed rather than left orphaned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Approval now reads before the fields it feeds into, matching the intended reading order: decide whether this person approves, pick their decision field, then see it in the fields list below. Applied to both the edit form and the read-only step card so the two views agree. Only the sequence differs between flag states, so each section is built once and reordered rather than duplicating the subtree per branch. Output is byte-identical with the flag off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Selecting a Yes/No field as the approval field now adds it to the step's edit list automatically, removing the manual double-step the backend already required (approval_field must be in edit). Auto-add is one-way: nothing ever removes an id from edit, not on toggle-off, field-clear, or an A->B switch. The leftover stays as a normal removable chip. This is deliberate — auto-add is a convenience, auto-remove is a decision, and removing silently would destroy a choice the admin may have made by hand. Validation reads edit via getValues rather than a watched closure: QuestionsBlock triggers it synchronously before this component re-renders, so a closed-over value would be one change stale. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Addresses review findings on the auto-assign helper. The old helper was a Set union with one caller, so its interface was as complex as its implementation and the decisions that could actually break lived outside it — the redesign guard in the component, and the one-way invariant only in a comment. Its tests asserted Set semantics, not the rule. It now takes the whole change (current edit, incoming approval value, whether auto-assign applies) and returns the next edit list, so the invariant is testable at the interface. Covers clearing the field and the flag-off case, neither of which the previous shape could express. Also renamed: nothing reconciled, and the old name promised a symmetric API the module deliberately refuses to have. Moved to utils/ alongside isFirstStepByStepNumber, the convention for pure helpers in this tree. Reading edit via getValues inside the handler removes the last watched closure, which was the source of the stale-read bug. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Auto-save-on-switch only saves when formState.isDirty is true. The approval toggle's checked state is React state rather than RHF, so clearing approval_field is the only thing that can mark the form dirty when approval is switched off — without shouldDirty it stayed clean and the change was discarded on card switch. Same for the auto-assign write to `edit`, so a change to the fields list is never silently dropped either. NOTE: these are necessary but not sufficient. EditStepBlock reads formState.isDirty only inside the auto-save effect, never during render, so RHF never subscribes and isDirty is false on a freshly mounted card regardless. That is a pre-existing issue in the click-to-edit feature and is being raised separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Drops a plan-section reference that resolves to nothing in this repo, and tightens the helper docblock and the two ordering comments. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A step without approval showed an "Approvals" heading and an "Approval not required in this step" badge. The reorder moved that to the top of the card, so every non-approval step now opened with a section saying nothing. Hidden under the flag when the step has no approval field. Steps whose approval field was deleted still render, so the error stays visible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Updates the workflow builder UI (behind useIsWorkflowBuilderRedesign()) to frame approvals as a person’s action, reorder approval/fields blocks, and auto-add the chosen approval Yes/No field into the step’s edit list.

Changes:

Reworded i18n copy and added redesign-only helper/labels for approvals + auto-add explanation.
Reordered approval vs fields sections in both editable and read-only step cards under the redesign flag.
Added nextEditFieldsForApproval utility (+ tests) to implement one-way auto-assign of approval_field into edit.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
apps/frontend/src/i18n/locales/features/admin-form/sidebar/workflow/index.ts	Extends workflow locale typings for redesign-only helper/labels.
apps/frontend/src/i18n/locales/features/admin-form/sidebar/workflow/en-sg.ts	Updates redesign copy for approvals and adds auto-add helper text.
apps/frontend/src/features/admin-form/create/workflow/components/WorkflowContent/utils/nextEditFieldsForApproval.ts	Introduces one-way helper to add approval field into `edit`.
apps/frontend/src/features/admin-form/create/workflow/components/WorkflowContent/utils/nextEditFieldsForApproval.test.ts	Adds unit tests for the one-way auto-assign helper.
apps/frontend/src/features/admin-form/create/workflow/components/WorkflowContent/InactiveStepBlock/InactiveStepBlock.tsx	Reorders approvals/fields display in collapsed card under flag; hides empty approvals in redesign.
apps/frontend/src/features/admin-form/create/workflow/components/WorkflowContent/EditStepBlock/QuestionsBlock.tsx	Adds redesign helper text + triggers approval validation on `edit` changes.
apps/frontend/src/features/admin-form/create/workflow/components/WorkflowContent/EditStepBlock/EditStepBlock.tsx	Swaps Approvals/Questions section order under flag without duplicating subtrees.
apps/frontend/src/features/admin-form/create/workflow/components/WorkflowContent/EditStepBlock/ApprovalsBlock.tsx	Person-framed toggle copy, redesign-only selector label, and auto-add approval field into `edit`.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+            render={({ field: { value = '', onChange, ...rest } }) => {
+              const handleApprovalFieldChange = (newValue: string) => {
+                onChange(newValue)
+                setValue(
+                  FIELDS_TO_EDIT_NAME,
+                  nextEditFieldsForApproval({
+                    edit: getValues(FIELDS_TO_EDIT_NAME),
+                    approvalFieldId: newValue,
+                    isEnabled: isRedesign,
+                  }),
+                  { shouldDirty: true },
+                )
+              }


+            const handleFieldsChange = (newValue: string[]) => {
+              onChange(newValue)
+              if (isRedesign && selectedApprovalField) {
+                trigger(APPROVAL_FIELD_NAME)


bookwormsuf and others added 7 commits August 13, 2026 20:30

docs(workflow-builder-redesign): trim comments to what isn't obvious

872352d

Drops a plan-section reference that resolves to nothing in this repo, and tightens the helper docblock and the two ordering comments. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

bookwormsuf requested a lite review from Copilot August 13, 2026 16:25

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat(workflow-builder-redesign): person-framed approval control - #9850

feat(workflow-builder-redesign): person-framed approval control#9850
bookwormsuf wants to merge 7 commits into
developfrom
feat/approval-reframe

bookwormsuf commented Aug 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

bookwormsuf commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Depends on #9849

Alternatives considered

Known gaps (not addressed here)

Screenshots

Breaking Changes

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bookwormsuf commented Aug 13, 2026 •

edited

Loading