Merge pull request #14784 from strapi/fix/settings-conditions-category

ConditionsModal: Fix selection of multiple items in different categories

packages/core/admin/admin/src/pages/SettingsPage/pages/Roles/EditPage/components/ConditionsModal/ActionRow/index.js

@@ -5,14 +5,9 @@ import { Box } from '@strapi/design-system/Box';
 import { Flex } from '@strapi/design-system/Flex';
 import { Typography } from '@strapi/design-system/Typography';
 import { MultiSelectNested } from '@strapi/design-system/Select';
-import upperFirst from 'lodash/upperFirst';
 import { useIntl } from 'react-intl';
-import styled from 'styled-components';
-import { rowHeight } from '../../Permissions/utils/constants';
 
-const FlexWrapper = styled(Flex)`
-  height: ${rowHeight};
-`;
+import { getNestedOptions, getSelectedValues, getNewStateFromChangedValues } from './utils/options';
 
 const ActionRow = ({
   arrayOfOptionsGroupedByCategory,
@@ -24,40 +19,13 @@ const ActionRow = ({
   value,
 }) => {
   const { formatMessage } = useIntl();
-  const options = arrayOfOptionsGroupedByCategory.reduce((arr, curr) => {
-    const [label, children] = curr;
-    const obj = {
-      label: upperFirst(label),
-      children: children.map((child) => ({
-        label: child.displayName,
-        value: child.id,
-      })),
-    };
 
-    return [...arr, obj];
-  }, []);
-
-  // Output: ['value1', 'value2']
-  const values = Object.values(value)
-    .map((x) =>
-      Object.entries(x)
-        .filter(([, value]) => value)
-        .map(([key]) => key)
-    )
-    .flat();
-
-  // ! Only expects arrayOfOpt to be [['default', obj]] - might break in future changes
   const handleChange = (val) => {
-    const [[, values]] = arrayOfOptionsGroupedByCategory;
-    const formattedValues = values.reduce(
-      (acc, curr) => ({ [curr.id]: val.includes(curr.id), ...acc }),
-      {}
-    );
-    onChange(name, formattedValues);
+    onChange(name, getNewStateFromChangedValues(arrayOfOptionsGroupedByCategory, val));
   };
 
   return (
-    <FlexWrapper as="li" background={isGrey ? 'neutral100' : 'neutral0'}>
+    <Flex as="li" background={isGrey ? 'neutral100' : 'neutral0'} paddingBottom={3} paddingTop={3}>
       <Flex paddingLeft={6} style={{ width: 180 }}>
         <Typography variant="sigma" textColor="neutral600">
           {formatMessage({
@@ -85,12 +53,12 @@ const ActionRow = ({
           id={name}
           customizeContent={(values) => `${values.length} currently selected`}
           onChange={handleChange}
-          value={values}
-          options={options}
+          value={getSelectedValues(value)}
+          options={getNestedOptions(arrayOfOptionsGroupedByCategory)}
           disabled={isFormDisabled || IS_DISABLED}
         />
       </Box>
-    </FlexWrapper>
+    </Flex>
   );
 };
 
@@ -103,4 +71,5 @@ ActionRow.propTypes = {
   value: PropTypes.object.isRequired,
   onChange: PropTypes.func.isRequired,
 };
+
 export default ActionRow;

packages/core/admin/admin/src/pages/SettingsPage/pages/Roles/EditPage/components/ConditionsModal/ActionRow/utils/options.js

@@ -0,0 +1,31 @@
+import upperFirst from 'lodash/upperFirst';
+
+const getSelectedValues = (rawValue) =>
+  Object.values(rawValue)
+    .map((x) =>
+      Object.entries(x)
+        .filter(([, value]) => value)
+        .map(([key]) => key)
+    )
+    .flat();
+
+const getNestedOptions = (options) =>
+  options.reduce((acc, [label, children]) => {
+    acc.push({
+      label: upperFirst(label),
+      children: children.map((child) => ({
+        label: child.displayName,
+        value: child.id,
+      })),
+    });
+
+    return acc;
+  }, []);
+
+const getNewStateFromChangedValues = (options, changedValues) =>
+  options
+    .map(([, values]) => values)
+    .flat()
+    .reduce((acc, curr) => ({ [curr.id]: changedValues.includes(curr.id), ...acc }), {});
+
+export { getNestedOptions, getSelectedValues, getNewStateFromChangedValues };

packages/core/admin/admin/src/pages/SettingsPage/pages/Roles/EditPage/components/ConditionsModal/ActionRow/utils/tests/options.test.js

@@ -0,0 +1,145 @@
+import { getSelectedValues, getNestedOptions, getNewStateFromChangedValues } from '../options';
+
+describe('ActionRow | utils | getSelectedValues', () => {
+  test('should reduce the default values to a flat array', () => {
+    const FIXTURE = {
+      something: {
+        'admin::billing-amount-under-10k': false,
+        'admin::billing-amount-above-20k': true,
+      },
+      default: {
+        'admin::is-creator': true,
+        'admin::has-same-role-as-creator': false,
+      },
+    };
+
+    expect(getSelectedValues(FIXTURE)).toStrictEqual([
+      'admin::billing-amount-above-20k',
+      'admin::is-creator',
+    ]);
+  });
+});
+
+describe('ActionRow | utils | getNestedOptions', () => {
+  test('should reduce the default values to a flat array', () => {
+    const FIXTURE = [
+      [
+        'default',
+        [
+          {
+            id: 'default:id:1',
+            displayName: 'default:displayName:1',
+            category: 'default:category',
+          },
+
+          {
+            id: 'default:id:2',
+            displayName: 'default:displayName:2',
+            category: 'default:category',
+          },
+        ],
+      ],
+
+      [
+        'something',
+        [
+          {
+            id: 'something:id:1',
+            displayName: 'something:displayName:1',
+            category: 'something:category',
+          },
+        ],
+      ],
+    ];
+
+    expect(getNestedOptions(FIXTURE)).toStrictEqual([
+      {
+        label: 'Default',
+        children: [
+          {
+            label: 'default:displayName:1',
+            value: 'default:id:1',
+          },
+
+          {
+            label: 'default:displayName:2',
+            value: 'default:id:2',
+          },
+        ],
+      },
+
+      {
+        label: 'Something',
+        children: [
+          {
+            label: 'something:displayName:1',
+            value: 'something:id:1',
+          },
+        ],
+      },
+    ]);
+  });
+});
+
+describe('ActionRow | utils | getNewStateFromChangedValues', () => {
+  const FIXTURE_OPTIONS = [
+    [
+      'default',
+      [
+        {
+          id: 'default:id:1',
+          displayName: 'default:displayName:1',
+          category: 'default:category',
+        },
+
+        {
+          id: 'default:id:2',
+          displayName: 'default:displayName:2',
+          category: 'default:category',
+        },
+      ],
+    ],
+
+    [
+      'something',
+      [
+        {
+          id: 'something:id:1',
+          displayName: 'something:displayName:1',
+          category: 'something:category',
+        },
+      ],
+    ],
+  ];
+  test('should generate false for all values if nothing was selected', () => {
+    expect(getNewStateFromChangedValues(FIXTURE_OPTIONS, [])).toStrictEqual({
+      'default:id:1': false,
+      'default:id:2': false,
+      'something:id:1': false,
+    });
+  });
+
+  test('should generate true for selected values', () => {
+    expect(getNewStateFromChangedValues(FIXTURE_OPTIONS, ['default:id:1'])).toStrictEqual({
+      'default:id:1': true,
+      'default:id:2': false,
+      'something:id:1': false,
+    });
+
+    expect(
+      getNewStateFromChangedValues(FIXTURE_OPTIONS, ['default:id:1', 'something:id:1'])
+    ).toStrictEqual({
+      'default:id:1': true,
+      'default:id:2': false,
+      'something:id:1': true,
+    });
+  });
+
+  test('should ignore unknown values', () => {
+    expect(getNewStateFromChangedValues(FIXTURE_OPTIONS, ['random:id:1'])).toStrictEqual({
+      'default:id:1': false,
+      'default:id:2': false,
+      'something:id:1': false,
+    });
+  });
+});