From d8972e72d37c177fec7e391b5e02c18b297dd175 Mon Sep 17 00:00:00 2001 From: Maduranga Siriwardena Date: Fri, 31 Jul 2026 17:35:57 +0530 Subject: [PATCH] Round-trip the prompt action type through the flow builder The prompt action's type was write-only in the console: serialization emitted prompts[].action.type from the button element's actionType, but the load path never read it back and FlowNodeAction did not even declare the field. A flow definition authored outside the builder therefore lost its action type on the first save, and the Action selector showed the wrong option for it. Restore the type onto the element when a definition is loaded, so the property panel can display it and serialization emits it again. The element wins when it already carries a type, because an unwired button holds the only copy: a prompt action is emitted only once the button has a nextNode. Clear only the type the selector owns when the author picks a plain action. SUBMIT and REJECT are documented prompt action types with no option in the dropdown, and blanket clearing discarded them. Key the Sign out option on SIGN_OUT_CONFIRM rather than a UI-only SIGN_OUT sentinel, so the value selected in the editor and the value in the flow definition are one vocabulary, and label it Sign Out Action to match the Submit Form and Trigger Action options beside it. Refs #4299 --- .../ButtonExtendedProperties.tsx | 14 +- .../ButtonExtendedProperties.test.tsx | 18 +++ .../src/features/flows/models/responses.ts | 6 + .../__tests__/flowToCanvasTransformer.test.ts | 121 ++++++++++++++++++ .../flows/utils/flowToCanvasTransformer.ts | 7 + frontend/packages/i18n/src/locales/en-US.ts | 2 +- 6 files changed, 163 insertions(+), 5 deletions(-) diff --git a/frontend/apps/console/src/features/flows/components/resource-property-panel/extended-properties/ButtonExtendedProperties.tsx b/frontend/apps/console/src/features/flows/components/resource-property-panel/extended-properties/ButtonExtendedProperties.tsx index cc5da513a4..6ca0e077a7 100644 --- a/frontend/apps/console/src/features/flows/components/resource-property-panel/extended-properties/ButtonExtendedProperties.tsx +++ b/frontend/apps/console/src/features/flows/components/resource-property-panel/extended-properties/ButtonExtendedProperties.tsx @@ -28,13 +28,17 @@ import {ActionEventTypes, PromptActionTypes} from '@/features/flows/models/eleme * button's `eventType`; `SignOut` is a submit button that additionally raises * the `SIGN_OUT_CONFIRM` prompt action, so one selection maps onto two fields. * + * `SignOut` deliberately carries the same literal that is persisted as + * `prompts[].action.type`, so the value selected here and the value in the flow + * definition are one vocabulary rather than a UI-only alias. + * * The remaining `ActionEventTypes` (navigate, cancel, reset, back) are handled * by the SDK renderers but deliberately not offered here. */ const ACTION_OPTIONS = { Submit: 'SUBMIT', Trigger: 'TRIGGER', - SignOut: 'SIGN_OUT', + SignOut: PromptActionTypes.SignOutConfirm, } as const; type ActionOption = (typeof ACTION_OPTIONS)[keyof typeof ACTION_OPTIONS]; @@ -75,8 +79,10 @@ function ButtonExtendedProperties({resource, onChange}: ButtonExtendedProperties onChange('eventType', nextAction, resource); // Clearing keeps the button from silently staying a sign-out confirmation - // after the author picks a plain action. - if (element?.actionType) { + // after the author picks a plain action. Only the type this selector owns is + // cleared, so an action type it does not model (e.g. REJECT, authored in the + // flow definition directly) is left untouched rather than discarded. + if (element?.actionType === PromptActionTypes.SignOutConfirm) { onChange('actionType', '', resource); } }; @@ -135,7 +141,7 @@ function ButtonExtendedProperties({resource, onChange}: ButtonExtendedProperties {t('flows:core.buttonExtendedProperties.action.trigger', 'Trigger Action')} - {t('flows:core.buttonExtendedProperties.action.signOut', 'Trigger Signout')} + {t('flows:core.buttonExtendedProperties.action.signOut', 'Sign Out Action')} diff --git a/frontend/apps/console/src/features/flows/components/resource-property-panel/extended-properties/__tests__/ButtonExtendedProperties.test.tsx b/frontend/apps/console/src/features/flows/components/resource-property-panel/extended-properties/__tests__/ButtonExtendedProperties.test.tsx index f9afb2cc9e..995ab9e966 100644 --- a/frontend/apps/console/src/features/flows/components/resource-property-panel/extended-properties/__tests__/ButtonExtendedProperties.test.tsx +++ b/frontend/apps/console/src/features/flows/components/resource-property-panel/extended-properties/__tests__/ButtonExtendedProperties.test.tsx @@ -276,6 +276,24 @@ describe('ButtonExtendedProperties', () => { expect(mockOnChange).toHaveBeenCalledWith('actionType', '', resource); }); + it('should preserve an action type the selector does not model', async () => { + // REJECT is a valid prompt action type with no option here. Clearing it would silently + // discard a type authored directly in the flow definition. + const user = userEvent.setup(); + const resource = createMockResource({ + actionType: 'REJECT', + eventType: 'SUBMIT', + } as Partial); + + render(); + + await user.click(screen.getByRole('combobox')); + await user.click(screen.getByRole('option', {name: 'flows:core.buttonExtendedProperties.action.trigger'})); + + expect(mockOnChange).toHaveBeenCalledWith('eventType', 'TRIGGER', resource); + expect(mockOnChange).not.toHaveBeenCalledWith('actionType', '', resource); + }); + it('should not clear the action type when it was never set', async () => { const user = userEvent.setup(); const resource = createMockResource({eventType: 'TRIGGER'} as Partial); diff --git a/frontend/apps/console/src/features/flows/models/responses.ts b/frontend/apps/console/src/features/flows/models/responses.ts index 65e09dfa64..52e454394b 100644 --- a/frontend/apps/console/src/features/flows/models/responses.ts +++ b/frontend/apps/console/src/features/flows/models/responses.ts @@ -151,6 +151,12 @@ export interface FlowNodeAction { * ID of the next node to navigate to */ nextNode: string; + /** + * Semantic action type forwarded to the next node's executor (e.g. `SUBMIT`, `REJECT`, + * `SIGN_OUT_CONFIRM`). Restored onto the element as `actionType` on load so a definition + * authored outside the builder keeps its type when the flow is saved again. + */ + type?: string; /** * Executor configuration for actions that trigger executors */ diff --git a/frontend/apps/console/src/features/flows/utils/__tests__/flowToCanvasTransformer.test.ts b/frontend/apps/console/src/features/flows/utils/__tests__/flowToCanvasTransformer.test.ts index 73b913292b..ea14969644 100644 --- a/frontend/apps/console/src/features/flows/utils/__tests__/flowToCanvasTransformer.test.ts +++ b/frontend/apps/console/src/features/flows/utils/__tests__/flowToCanvasTransformer.test.ts @@ -21,6 +21,7 @@ import VisualFlowConstants from '../../constants/VisualFlowConstants'; import type {FlowDefinitionResponse, FlowNode} from '../../models/responses'; import {StaticStepTypes, StepTypes} from '../../models/steps'; import {transformFlowToCanvas} from '../flowToCanvasTransformer'; +import {transformReactFlow} from '../reactFlowTransformer'; describe('flowToCanvasTransformer', () => { const createBaseFlowData = (nodes: FlowNode[]): FlowDefinitionResponse => ({ @@ -462,6 +463,89 @@ describe('flowToCanvasTransformer', () => { }); }); + it("should restore the prompt action's type onto the element", () => { + // A definition authored outside the builder carries the type only on the prompt action. + // Without restoring it the property panel cannot show it and serialization drops it. + const flowData = createBaseFlowData([ + { + id: 'prompt-node', + type: 'PROMPT', + meta: { + components: [{id: 'action_confirm', type: 'ACTION', label: 'Sign out'}], + }, + prompts: [{action: {ref: 'action_confirm', type: 'SIGN_OUT_CONFIRM', nextNode: 'next-node'}}], + layout: {position: {x: 0, y: 0}, size: {width: 300, height: 200}}, + }, + ]); + + const result = transformFlowToCanvas(flowData); + + const component = result.nodes[0].data.components?.[0] as Record | undefined; + expect(component?.actionType).toBe('SIGN_OUT_CONFIRM'); + }); + + it('should restore the type when the element carries a cleared action type', () => { + // Clearing the Action selector writes an empty string rather than dropping the key, and + // cleanComponents keeps it, so a definition saved after a clear carries actionType: ''. + // That means no type, so it must not block the backfill. + const flowData = createBaseFlowData([ + { + id: 'prompt-node', + type: 'PROMPT', + meta: { + components: [{id: 'action_confirm', type: 'ACTION', actionType: '', label: 'Sign out'}], + }, + prompts: [{action: {ref: 'action_confirm', type: 'SIGN_OUT_CONFIRM', nextNode: 'next-node'}}], + layout: {position: {x: 0, y: 0}, size: {width: 300, height: 200}}, + }, + ]); + + const result = transformFlowToCanvas(flowData); + + const component = result.nodes[0].data.components?.[0] as Record | undefined; + expect(component?.actionType).toBe('SIGN_OUT_CONFIRM'); + }); + + it('should keep an action type already on the element over the prompt action', () => { + // The element is the authoring surface, so an unwired button that still carries a type + // must not have it overwritten by a stale prompt action. + const flowData = createBaseFlowData([ + { + id: 'prompt-node', + type: 'PROMPT', + meta: { + components: [{id: 'action_confirm', type: 'ACTION', actionType: 'REJECT', label: 'No'}], + }, + prompts: [{action: {ref: 'action_confirm', type: 'SIGN_OUT_CONFIRM', nextNode: 'next-node'}}], + layout: {position: {x: 0, y: 0}, size: {width: 300, height: 200}}, + }, + ]); + + const result = transformFlowToCanvas(flowData); + + const component = result.nodes[0].data.components?.[0] as Record | undefined; + expect(component?.actionType).toBe('REJECT'); + }); + + it('should leave the element without an action type when the prompt action has none', () => { + const flowData = createBaseFlowData([ + { + id: 'prompt-node', + type: 'PROMPT', + meta: { + components: [{id: 'submit-btn', type: 'ACTION', label: 'Submit'}], + }, + prompts: [{action: {ref: 'submit-btn', nextNode: 'next-node'}}], + layout: {position: {x: 0, y: 0}, size: {width: 300, height: 200}}, + }, + ]); + + const result = transformFlowToCanvas(flowData); + + const component = result.nodes[0].data.components?.[0] as Record | undefined; + expect(component).not.toHaveProperty('actionType'); + }); + it('should normalize INPUT element properties', () => { const flowData = createBaseFlowData([ { @@ -679,5 +763,42 @@ describe('flowToCanvasTransformer', () => { expect(result.edges).toHaveLength(0); }); }); + + describe('Action Type Round Trip', () => { + const signOutFlow = (): FlowDefinitionResponse => + createBaseFlowData([ + { + id: 'prompt_confirm', + type: 'PROMPT', + meta: { + components: [{id: 'action_confirm', type: 'ACTION', label: 'Sign out'}], + }, + prompts: [{action: {ref: 'action_confirm', type: 'SIGN_OUT_CONFIRM', nextNode: 'session_signout'}}], + layout: {position: {x: 0, y: 0}, size: {width: 300, height: 200}}, + }, + { + id: 'session_signout', + type: 'TASK_EXECUTION', + executor: {name: 'SessionSignOutExecutor'}, + layout: {position: {x: 400, y: 0}, size: {width: 200, height: 100}}, + }, + ]); + + it('should keep the action type when a definition is loaded and serialized again', () => { + // Opening a flow in the builder and saving it must not change its meaning. Before the type + // was restored onto the element, this round trip silently dropped it and a sign-out flow + // regressed into an endless confirmation loop. + const canvas = transformFlowToCanvas(signOutFlow()); + + const saved = transformReactFlow({edges: canvas.edges, nodes: canvas.nodes}); + + const prompt = saved.nodes.find((node) => node.id === 'prompt_confirm'); + expect(prompt?.prompts?.[0].action).toMatchObject({ + ref: 'action_confirm', + nextNode: 'session_signout', + type: 'SIGN_OUT_CONFIRM', + }); + }); + }); }); }); diff --git a/frontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.ts b/frontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.ts index b6a8c34026..3afe21be6f 100644 --- a/frontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.ts +++ b/frontend/apps/console/src/features/flows/utils/flowToCanvasTransformer.ts @@ -123,6 +123,13 @@ function restoreButtonAction( if (matchingAction) { return { ...component, + // Backfill the prompt action's type onto the element, which is where the property panel reads + // it from and where serialization projects it back out of. Only when the element does not + // already carry one: an unwired button keeps its type on the element alone, since a prompt + // action is only emitted once the button has a nextNode. Clearing the selector writes an empty + // string rather than dropping the key, and that empty string is persisted, so treat it as no + // type rather than as a type worth preserving. + ...(matchingAction.type && !component.actionType ? {actionType: matchingAction.type} : {}), action: { type: matchingAction.executor ? 'EXECUTOR' : 'NEXT', onSuccess: matchingAction.nextNode, diff --git a/frontend/packages/i18n/src/locales/en-US.ts b/frontend/packages/i18n/src/locales/en-US.ts index 43a184cf76..44acf66475 100644 --- a/frontend/packages/i18n/src/locales/en-US.ts +++ b/frontend/packages/i18n/src/locales/en-US.ts @@ -3821,7 +3821,7 @@ const translations = { 'core.buttonExtendedProperties.action.label': 'Action', 'core.buttonExtendedProperties.action.submit': 'Submit Form', 'core.buttonExtendedProperties.action.trigger': 'Trigger Action', - 'core.buttonExtendedProperties.action.signOut': 'Trigger Signout', + 'core.buttonExtendedProperties.action.signOut': 'Sign Out Action', 'core.buttonExtendedProperties.action.hint': 'What happens when the button is activated', 'core.buttonExtendedProperties.startIcon.label': 'Start Icon', 'core.buttonExtendedProperties.startIcon.placeholder': 'Enter icon path (e.g., assets/images/icons/icon.svg)',