Skip to content

feat(webhooks): show workflow-data callout on MRF webhook settings#9760

Open
yin-boop wants to merge 4 commits into
opengovsg:developfrom
yin-boop:webhooks-workflow-infobox
Open

feat(webhooks): show workflow-data callout on MRF webhook settings#9760
yin-boop wants to merge 4 commits into
opengovsg:developfrom
yin-boop:webhooks-workflow-infobox

Conversation

@yin-boop

Copy link
Copy Markdown

Problem

Admins can connect a webhook to an MRF form to send each submission to another tool in real time, but not every destination can do the same thing with it: only Plumber can use the multi-step workflow data across all steps — any other tool can only use the first step's data. Admins have no way to know this at the moment they configure a webhook, so they may wire up the wrong integration and be surprised when their tool can't act on later steps.

Solution

Adds an always-on informational callout at the top of the webhook settings, above the endpoint URL field, telling admins that if workflows are enabled, only Plumber can use data from all workflow steps while other tools can only use data from the first step, with a "Learn more" link to the simplified-modes FAQ. The callout is a neutral heads-up, not an error, and shows regardless of whether a workflow is configured yet or whether the entered URL is a Plumber link.

Visibility is gated on response mode alone: SettingsWebhooksPage computes showWorkflowInfobox = responseMode === Multirespondent and passes a single boolean into WebhooksSection. No feature flag is checked at this line — enable-mrf-webhooks is already guaranteed true on this code path by the page's existing enableWebhooks gate, and mrf-cutover governs form-creation defaults, a separate concern from webhook messaging.

Alternatives considered

  • We considered having WebhooksSection (or the callout itself) read useAdminFormSettings() directly, like its siblings WebhookUrlInput and RetryToggle do. We kept the visibility decision at the page instead: the page already owns the settings query and the response-mode branching, so the section stays a dumb layout component with no data lookups of its own, and the callout stays a prop-less, non-fetching component that tests in isolation.
  • An earlier draft (the webhooks-workflow-callout branch) gated the callout on mrf-cutover && MRF. We dropped the cutover flag: it couples the callout to the cutover rollout, while the capability distinction the copy describes is true regardless of cutover state.

Screenshots

Captured from the page stories in Storybook. The MRF "before" is unavailable via Storybook — the MRF story is introduced by this PR; on develop the same MRF form rendered the section without the callout, visually identical to the Storage row.

Page Before After
Settings → Webhooks (MRF form, enable-mrf-webhooks on) unavailable — see note above after MRF
Settings → Webhooks (Storage form — unchanged) before Storage after Storage

Breaking Changes

No - backwards compatible.

Tests

TC1: Callout shows on an MRF form

TC2: Callout hidden on a Storage-mode form

  • Open Settings → Webhooks on a Storage-mode form — webhook settings render with no callout

TC3: Callout is unconditional on workflow/URL

  • On the MRF form, confirm the callout shows before any workflow steps exist and does not change when a Plumber URL is entered

🤖 Generated with Claude Code

yin-boop and others added 4 commits July 15, 2026 12:17
Admins wiring a webhook on an MRF form have no signal that only
Plumber can act on data from every workflow step — other tools only
use the first step's fields. Add a self-contained info callout
(InlineMessage, markdown copy from a single i18n key) linking to
Plumber and the simplified-modes FAQ, so the heads-up lives next to
the endpoint URL where the integration decision is made.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Gate the workflow-data callout on response mode alone: the page
computes showWorkflowInfobox = (responseMode === Multirespondent) and
passes it into WebhooksSection, which renders the callout above the
endpoint URL input. No re-check of enable-mrf-webhooks (already
guaranteed by the page-level enableWebhooks gate) and no mrf-cutover
dependency (that flag governs form-creation defaults, not webhook
messaging). Page tests pin the two-quadrant visibility matrix — shown
for MRF, hidden for Storage — and an MrfMode story locks the MRF
state in Chromatic.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two stories now stand up identical GrowthBook provider boilerplate to
flip a feature flag on. Extract a withGrowthBookFeatures(...flags)
helper so each story declares just the flags it needs. No behaviour
change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Working docs assembled by prepare-for-review: the PR body (with
before/after Storybook captures) and the multi-axis review findings,
kept in-repo so reviewers can see the review disposition alongside
the change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@yin-boop
yin-boop requested a review from a team July 15, 2026 13:00
)
}

const showWorkflowInfobox =

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Why this gate checks only response mode: it deliberately does not re-check enableMrfWebhooks — this code path is unreachable unless that flag already passed the page-level enableWebhooks check above, so re-checking would be dead logic. It also does not gate on mrf-cutover (an earlier draft did): that flag controls form-creation defaults app-wide, a separate concern from whether an MRF form's webhook settings should carry this heads-up. The capability distinction the copy describes is true regardless of cutover state.

🤖 Generated with Claude Code

@yin-boop

Copy link
Copy Markdown
Author

Review findings — webhooks-workflow-infobox vs develop

Multi-axis review (/review) of the 3-commit diff adding the webhook workflow-data callout, its MRF-only gate, and the two-quadrant visibility tests. Each axis ran as an independent reviewer; Standards/Spec findings were verified against false positives by independent per-finding verifiers (cutoff 70).

Standards

Clean. No documented-standard violations — i18n key placement and interpolation match retry.description exactly, tests follow the repo's composeStories pattern, and the story title prefix matches convention. The most borderline drop (scored 60 on independent verification, below cutoff):

  • The callout takes a new, untagged dependency on MRF_CUTOVER_FAQ_LINK, a constant WorkspacePage.tsx:53 explicitly plans to remove ("Remove this infobox (and MRF_CUTOVER_FAQ_LINK) once the cutover is complete"). The verifier confirmed the maintenance risk is real but the issue spec explicitly directs this link and acknowledges the constant's name is a misnomer while the linked content stays accurate. Cheap defusal if desired: rename the constant (e.g. SIMPLIFIED_MODES_FAQ_LINK) or amend the WorkspacePage TODO so cutover cleanup doesn't delete a constant this permanent callout still uses.

Spec

Clean. All eight acceptance criteria verified met against the diff: placement above the endpoint URL field, gate is responseMode === Multirespondent only (no mrf-cutover, no enableMrfWebhooks re-check), character-exact copy sourced from the workflowInfobox i18n key, correct Plumber/FAQ hrefs, info variant, two-quadrant page tests, component copy/link tests, and an MRF page story. Two spec items confirmed moot due to spec staleness rather than diff defects:

  • MRF_CUTOVER_FAQ_LINK already exists in packages/shared/constants/links.ts on develop, so no shared-package change was needed.
  • WebhookV1SchemaInfobox no longer exists anywhere in the repo (removed on develop in 62ea89447), so the "leave its gate untouched" criterion is satisfied vacuously.

One informational note dropped at 50 (below cutoff): converting the pre-existing StorageModeMrfCutoverOn story to the new withGrowthBookFeatures helper is a behaviour-preserving refactor the spec didn't request; the spec's out-of-scope list doesn't forbid it.

Architecture / Divergent (merged — both axes flagged the same two lines)

  1. showWorkflowInfobox prop-drilling diverges from the section's own pattern (WebhooksSection.tsx:7; also raised by Standards below threshold). Both siblings (WebhookUrlInput, RetryToggle) read useAdminFormSettings() themselves and React Query dedupes the fetch, so the section could derive the boolean locally, deleting the prop and the page-level computation; the story-based tests would pass unchanged. Orchestrator note: the issue spec and PRD explicitly mandate the current shape ("passes a single boolean into WebhooksSection, which keeps it a dumb layout component free of its own data lookups"), and a breadcrumb records it — so this finding re-litigates a spec decision. Acting on it is a spec change, not a fix.
  2. withGrowthBookFeatures helper deduped into the wrong home (SettingsWebhooksPage.stories.tsx:56; flagged by Architecture, Divergent, and Standards). The identical GrowthBook decorator boilerplate exists in at least three other story files (CreateFormModal, DuplicateFormModal, UseTemplateModal stories), and shared story helpers already live in src/utils/storybook.tsx. Hoisting the new helper there costs the same as defining it locally and prevents a fifth copy; migrating the other three files can stay a follow-up.

All other choices were examined by the Divergent axis and confirmed optimal: always-on callout rather than workflow-presence detection, single i18n key with markdown links and URL interpolation, reuse of InlineMessage variant="info" useMarkdown, and composeStories-based tests.

Summary

2 surviving findings (both Architecture+Divergent, cross-axis merged); Standards and Spec clean. Worst issue: the helper's placement (the one clearly actionable in-PR change), with the prop-drilling finding gated on whether the spec's structure decision should be revisited. 3 findings dropped below threshold (scores 60, 50, and two sub-threshold Standards duplicates of the judgement findings).

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.

1 participant