Merge pull request #5127 from open-formulieren/bug/5040-delete-one-of…

…-multiple-logic-actions-in-same-trigger

Add unique key to Action component

src/openapi.yaml

@@ -8821,6 +8821,10 @@ components:
     LogicComponentAction:
       type: object
       properties:
+        uuid:
+          type: string
+          format: uuid
+          readOnly: true
         component:
           type: string
           title: Form.io component
@@ -8843,6 +8847,7 @@ components:
           $ref: '#/components/schemas/LogicActionPolymorphic'
       required:
       - action
+      - uuid
     LogicDescription:
       type: object
       properties:

src/openforms/forms/api/serializers/logic/action_serializers.py

@@ -1,3 +1,4 @@
+import uuid
 from datetime import date
 
 from django.utils.translation import gettext_lazy as _
@@ -133,6 +134,11 @@ class LogicActionPolymorphicSerializer(PolymorphicSerializer):
 @extend_schema_serializer(deprecate_fields=["form_step"])
 class LogicComponentActionSerializer(serializers.Serializer):
     # TODO: validate that the component is present on the form
+    uuid = serializers.UUIDField(
+        read_only=True,
+        default=uuid.uuid4,
+        label=_("UUID"),
+    )
     component = serializers.CharField(
         required=False,  # validated against the action.type
         allow_blank=True,

src/openforms/js/components/admin/form_design/FormLogic.stories.js

@@ -1,4 +1,4 @@
-import {fn} from '@storybook/test';
+import {expect, fn, userEvent, within} from '@storybook/test';
 
 import {
   FeatureFlagsDecorator,
@@ -161,6 +161,8 @@ export const FullFunctionality = {
               value: '',
             },
 
+            uuid: '',
+            _generatedId: '0',
             component: '',
             formStepUuid: null,
             variable: 'bar',
@@ -197,6 +199,8 @@ export const FullFunctionality = {
               },
             },
 
+            uuid: '',
+            _generatedId: '0',
             component: '',
             formStepUuid: null,
             variable: '',
@@ -209,3 +213,78 @@ export const FullFunctionality = {
     availableFormSteps: AVAILABLE_FORM_STEPS,
   },
 };
+
+export const DeletingOneOfMultipleActionsInSameTrigger = {
+  name: 'Deleting one of multiple actions in the same trigger',
+
+  args: {
+    logicRules: [
+      {
+        uuid: 'foo',
+        _generatedId: 'foo', // consumers should generate this, as it's used for the React key prop if no uuid exists
+        _logicType: 'simple',
+        form: 'http://localhost:8000/api/v2/forms/ae26e20c-f059-4fdf-bb82-afc377869bb5',
+        description: 'Sample rule',
+        _mayGenerateDescription: false,
+        order: 1,
+
+        jsonLogicTrigger: {
+          '==': [
+            {
+              var: 'foo',
+            },
+            'bar',
+          ],
+        },
+
+        isAdvanced: false,
+
+        actions: [
+          {
+            uuid: '',
+            _generatedId: '0',
+            component: '',
+            variable: 'foo',
+            formStepUuid: null,
+            action: {
+              type: 'variable',
+              value: 'First action',
+            },
+          },
+          {
+            uuid: '',
+            _generatedId: '1',
+            component: '',
+            variable: 'bar',
+            formStepUuid: null,
+            action: {
+              type: 'variable',
+              value: 'Second action',
+            },
+          },
+        ],
+      },
+    ],
+
+    availableFormVariables: AVAILABLE_FORM_VARIABLES,
+    availableFormSteps: AVAILABLE_FORM_STEPS,
+  },
+
+  play: async ({canvasElement}) => {
+    const canvas = within(canvasElement);
+
+    // Both actions should be present
+    expect(await canvas.findByText(/First action/)).toBeInTheDocument();
+    expect(await canvas.findByText(/Second action/)).toBeInTheDocument();
+
+    // Delete the first action
+    const deleteIcons = await canvas.findAllByTitle('Verwijderen');
+    expect(deleteIcons).toHaveLength(3);
+    await userEvent.click(deleteIcons[1]); // deleteIcons[0] is the delete icon for the entire rule
+    await userEvent.click(await canvas.findByRole('button', {name: 'Accepteren'}));
+
+    // First action should be removed, and the second should still be present
+    expect(canvas.queryByText(/First action/)).not.toBeInTheDocument();
+    expect(await canvas.findByText(/Second action/)).toBeInTheDocument();
+  },
+};

src/openforms/js/components/admin/form_design/logic/actions/ActionSet.js

@@ -6,11 +6,14 @@ import {useImmerReducer} from 'use-immer';
 
 import ButtonContainer from 'components/admin/forms/ButtonContainer';
 import useOnChanged from 'hooks/useOnChanged';
+import {getUniqueRandomString} from 'utils/random';
 
 import Action from './Action';
 import {ActionError} from './types';
 
 const EMPTY_ACTION = {
+  uuid: '',
+  _generatedId: '', // consumers should generate this, as it's used for the React key prop when uuid does not exist
   component: '',
   formStep: '',
   config: {},
@@ -52,7 +55,8 @@ const reducer = (draft, action) => {
       break;
     }
     case 'ACTION_ADDED': {
-      draft.actions.push(EMPTY_ACTION);
+      const newAction = {...EMPTY_ACTION, _generatedId: getUniqueRandomString()};
+      draft.actions.push(newAction);
       break;
     }
     case 'ACTION_DELETED': {
@@ -94,7 +98,7 @@ const ActionSet = ({name, actions, errors = [], onChange}) => {
     <>
       {state.actions.map((action, index) => (
         <Action
-          key={index}
+          key={action.uuid || action._generatedId}
           prefixText={index === 0 ? firstActionPrefix : extraActionPrefix}
           action={action}
           errors={errors[index] || {}}

src/openforms/submissions/logic/actions.py

@@ -32,6 +32,7 @@ class ActionDetails(TypedDict):
 
 
 class ActionDict(TypedDict):
+    uuid: str
     component: str
     variable: str
     form_step: str