-
Notifications
You must be signed in to change notification settings - Fork 82
Conversation
Rework the webhook trigger flow (#4787) across the show panel and the Choose/Configure wizard: - Add a flat ghost button variant; move shadow out of the shared base so ghost buttons have no raised outline. - Picker: drop the "Popular" badge, add a trailing chevron per row. - Choose: show the trigger type as a chip with a "Change" button that opens the picker. - Configure: replace the native Response Type select with a Listbox carrying per-option descriptions; collapse Authentication and Response Options by default; let users type any Success/Error status code (default 201); drop the header back arrow for a clickable breadcrumb; label the commit button "Finish". - Show panel: use a secondary Edit button, drop the section separators, and summarise the response in one line with a link to the docs. Authentication still uses the existing modal and is unchanged here.
Resting panel:
- Make Authentication and Response collapsible, with an at-a-glance
auth count in the header ("none configured" / "N configured")
- Show the response type description and, when responding on
completion, the default success/error statuses as read-only inputs
with a note that job code can override them
- Replace prominent buttons with inline links ("Add authentication",
"Configure default response status") that deep-link straight into
the relevant Configure section
Edit wizard:
- Replace the multi-select credential dropdown with a row-per-method
picker: "Add" appends a row, each row dedupes against the others,
and a "Create a new authentication method" link opens the form
- Hide Response Options unless the webhook responds on completion
- Relay the create-auth modal's close event so Cancel works in the
new flow
Bring the cron trigger onto the redesigned inspector flow, matching the webhook pattern: a read-only show panel (humanized schedule + cron input source) and a Choose -> Configure edit wizard over a local draft that commits on Finish. Configure reuses the CronFieldBuilder dropdown for the schedule, which now auto-opens the raw cron expression field when Custom is selected and shows a plain-English description of the expression beneath it. Extract the shared TriggerTypeStep so the webhook and cron wizards no longer duplicate the picker step wiring.
Replace the per-type WebhookEditWizard and CronEditWizard with a single TriggerEditWizard that owns the draft and step machine and dispatches the Choose and Configure steps by trigger type. The initial step is `type ? 'choose' : 'picker'`, so supporting a nullable type later is a one-line change. Port kafka onto the new flow (KafkaConfigureStep + KafkaShowPanel, generalized TriggerChooseStep) so the legacy TriggerForm — and its now-orphaned WebhookAuthMethodModal — can be deleted. Kafka drops the Enabled toggle and Run button to match the webhook and cron panels. Guard useWebhookTrigger so it only requests trigger auth methods for webhook triggers, avoiding needless channel calls for cron and kafka.
Fold TriggerTypeStep back into TriggerPicker and route all three trigger types through the draft path, so picking a type holds until Finish — Cancel now reverts a kafka type switch instead of committing it immediately. Commit only the trigger fields that actually changed so a concurrent canvas edit to an untouched field is no longer clobbered, and stop a type change from silently re-enabling a disabled trigger. Resync the webhook auth-method rows when the selection resolves asynchronously. Extract shared EditFooter, ReadOnlyField, WizardFinishFooter and useCanEditWorkflow used across the show panels and configure steps, and add a trigger/ barrel for external consumers. Share the trigger test harness across the inspector test files, add a WebhookConfigureStep test, and collapse the repetitive cron and inspector suites with test.each.
Codecov Report
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.5%. Comparing base (84c0754) to head (e6065ba).
Additional details and impacted files
@@ Coverage Diff @@ ## main #4854 +/- ## ===================================== Coverage 90.5% 90.5% ===================================== Files 445 445 Lines 22721 22721 ===================================== + Hits 20560 20561 +1 + Misses 2161 2160 -1
☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.
🚀 New features to boost your workflow:
- ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Confirmed: no backend changes. The PR is a pure frontend refactor of the trigger inspector that calls pre-existing, already-secured Phoenix channel handlers (request_trigger_auth_methods / update_trigger_auth_methods in lib/lightning_web/channels/workflow_channel.ex:507,538 — both scope on socket.assigns.project.id and gate writes with authorize_edit_workflow).
Security Review ✅
- S0 (project scoping): N/A — no backend changes; the new TypeScript hooks call existing channel events (
request_trigger_auth_methods,update_trigger_auth_methods) whose handlers already scope queries tosocket.assigns.project.id(lib/lightning_web/channels/workflow_channel.ex:516-523,549-553). - S1 (authorization): N/A — no new web-layer actions; the unchanged
update_trigger_auth_methodshandler still gates onauthorize_edit_workflow(socket)(workflow_channel.ex:549) before any mutation. - S2 (audit trail): N/A — no new config-resource writes from server code; the existing handler still passes
actor: socket.assigns.current_userintoWebhookAuthMethods.update_trigger_auth_methods(workflow_channel.ex:558) which produces the audit entry.
@lmac-1
lmac-1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great, Frank! I have left some comments in the code re organisation and permissions. I do think some of these things need cleaning up before merging, so that we are in a great place for further design system work.
As a side conversation, as we are moving into a newer design system, I am wondering whether we should have some of these newer 'primitive' components in one place? e.g. breadcrumbs, buttons, badges. Then, as new epics come in, we can start to use these components rather than the legacy ones. Thoughts @theroinaochieng @stuartc ? Not for this PR, but something to clean up in future work or as a follow-on PR.
Some design niggles (not to block approval - perhaps something for Tyrell to review later in UAT, but just noting for reference):
Image 1. "On webhook call" vs "On a Schedule" - I feel like "Schedule" should be lower case so that it's consistent casing. Image 2. This lighter purple for "Change" gave me a feeling that it was inactive or disabled because of the lighter colour.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No read-only scenario tested here — worth adding a case with can_edit_workflow: false to cover the !trigger.type path mentioned in TriggerInspector comment.
- Move CronFieldBuilder and its test into trigger/; only the cron Configure step uses it - Replace hardcoded hex in WebhookUrlField with Tailwind emerald classes - Trim trigger/index.ts to the exports TriggerInspector actually consumes - Extract the duplicated sameIdSet into trigger/idSet.ts - Disable WebhookConfigureStep inputs when the workflow is read-only, to match the cron and kafka Configure steps - Drop the speculative typeless-trigger path from TriggerInspector and the wizard's initial step - Align Kafka Configure input borders with the webhook convention - Lowercase "On a schedule" for casing consistency with "On webhook call"
Diff the draft against an open-time baseline snapshot rather than the live trigger, so Finish writes only the fields the user actually changed. Comparing against the live trigger treated a collaborator's concurrent edit to an untouched field (e.g. toggling enabled on the canvas) as a local edit and reverted it on Finish; isDirty had the same false positive. Also commit the auth-method change before the local trigger write so a failed channel request leaves nothing partially committed and reports failure to keep the wizard open.
Fold WebhookChooseStep into TriggerChooseStep: the two were ~80% identical, differing only in the webhook URL field and exit affordance (footer Cancel vs. header back arrow). The merged step takes `type` plus the webhook-only URL props, and chooses its exit shape from whether `onBack` or `onCancel` is passed. Replace the three near-identical wizard footers (WizardFinishFooter plus the inline footers in the webhook Choose/Configure steps) with a single WizardFooter, parameterized by primary label, optional Cancel, and an optional validation error. This also removes the inconsistency where the webhook Configure step hand-rolled its footer instead of sharing one. No behavior change; the trigger inspector test suite is unchanged and passes.
Drop comments that restated the adjacent code and metadata that no longer aids the reader — Figma version references, issue-number tags, and "ported from / former TriggerForm" historical notes. Keep per-prop JSDoc, the functional doc blocks, and the rationale comments that explain non-obvious behaviour (draft baseline and auth-load guards, the server-side auth-modal relay, enabled-state preservation).
...ger-inspector-flow
midigofrank
commented
Jun 17, 2026
Thanks @lmac-1 , I have incorporated your changes, except for making global components. My intentions with these changes were to reuse existing components and not introduce new ones, I think this can come in as a follow up. Would you mind giving it another go?
@lmac-1
lmac-1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good Frank, thanks for applying the changes. I would feel more comfortable if another dev could give this a quickish glance/review before merging.
Uh oh!
There was an error while loading. Please reload this page.
Description
This PR redesigns the trigger inspector, replacing the old edit-in-place form with a read-only resting panel and a guided edit wizard, for webhook, cron, and kafka triggers.
Closes #4787
Closes #4797
Closes #4798
What changed for users:
Previously, selecting a trigger node dropped you straight into an editable form that wrote every change live, field by field. Now selecting a trigger shows a read-only "resting" panel that summarises its configuration, with an Edit button. Editing opens a wizard — Choose → (optionally Change the type via a picker) → Configure → Finish — and changes are held in a local draft, only persisted when you click Finish. Cancel discards them. So editing a trigger is now a deliberate, reviewable action rather than a live mutation.
The same flow applies to every trigger type:
The resting panels are read-only by design and drop the Enabled toggle and Run button — enabling/disabling lives on the canvas, and running lives in the header.
No backend changes — trigger schema and persistence are unchanged; this is a frontend/UX redesign in the collaborative editor.
Validation steps
kafka_triggers_enabled, repeat for a kafka trigger (connection/security/advanced config, validation messages).Additional notes for the reviewer
kafka_triggers_enabledis set; kafka is covered by unit tests and was browser-verified by temporarily forcing the flag on.TriggerFormandWebhookAuthMethodModal(and their tests). The unified wizard lives inassets/js/collaborative-editor/components/inspector/trigger/.AI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
You can read more details in our
Responsible AI Policy
Pre-submission checklist
/reviewwith Claude Code)
(e.g.,
:owner,:admin,:editor,:viewer)