feat(modifyFieldsOnValue): enable field to modify itself (#1494)

**Issue number:** https://splunk.atlassian.net/browse/ADDON-76687 ### PR Type **What kind of change does this PR introduce?** * [x] Feature * [ ] Bug Fix * [ ] Refactoring (no functional or API changes) * [ ] Documentation Update * [ ] Maintenance (dependency updates, CI, etc.) ## Summary ### Changes Remove verification to block field from applying modification to itself after value change. It is not a risk to allow users to modify field itself as we should be concerned only for value. (it is still not a problem but seems like a bad approach to solving problems). It also enables circular dependencies in case where modifications are not about value. (modifying values still should work good but seems like a bad practise) ### User experience User can right now modify state of field by applying changes to currently edited field, it allows to create ie. much more useful labels and help messages referencing value change. ## Checklist If an item doesn't apply to your changes, leave it unchecked. * [ ] I have performed a self-review of this change according to the [development guidelines](https://splunk.github.io/addonfactory-ucc-generator/contributing/#development-guidelines) * [x] Tests have been added/modified to cover the changes [(testing doc)](https://splunk.github.io/addonfactory-ucc-generator/contributing/#build-and-test) * [ ] Changes are documented * [x] PR title and description follows the [contributing principles](https://splunk.github.io/addonfactory-ucc-generator/contributing/#pull-requests) --------- Co-authored-by: srv-rr-github-token <[email protected]>
splunk · Dec 5, 2024 · 3fd0501 · 3fd0501
1 parent 01c88aa
commit 3fd0501
Show file tree

Hide file tree

Showing 12 changed files with 341 additions and 55 deletions.
diff --git a/docs/entity/modifyFieldsOnValue.md b/docs/entity/modifyFieldsOnValue.md
@@ -4,11 +4,11 @@ This feature allows to specify conditions to modify other fields based on curren
 
 ### Modification Object Properties
 
-| Property                                            | Type   | Description                                                                                                         |
-| --------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------- |
-| fieldValue<span class="required-asterisk">\*</span> | string | Value that will trigger the update, put `[[any_other_value]]` to trigger update for any other values than specified |
-| mode                                                | string | Mode that adds possibility to use modification only on certain mode                                                 |
-| fieldsToModify                                      | array  | List of fields modifications that will be applied after com ponent value will match                                 |
+| Property                                            | Type   | Description                                                                                                                       |
+| --------------------------------------------------- | ------ | --------------------------------------------------------------------------------------------------------------------------------- |
+| fieldValue<span class="required-asterisk">\*</span> | string | Value of current field that will trigger the update. Put `[[any_other_value]]` to make update for any other value than specified. |
+| mode                                                | string | Mode that adds possibility to use modification only on certain mode. One of ( `create` / `edit` / `clone` / `config` )            |
+| fieldsToModify                                      | array  | List of fields modifications that will be applied after com ponent value will match.                                              |
 
 ### fieldsToModify Properties
 

diff --git a/splunk_add_on_ucc_framework/global_config_validator.py b/splunk_add_on_ucc_framework/global_config_validator.py
@@ -530,7 +530,7 @@ def _validate_groups(self) -> None:
                             f"Service {service['name']} uses group field {group_field} which is not defined in entity"
                         )
 
-    def _is_circular(
+    def _is_circular_modification(
         self,
         mods: List[Any],
         visited: Dict[str, str],
@@ -551,61 +551,73 @@ def _is_circular(
             # no more dependent modification fields
             visited[current_field] = DEAD_END
             return visited
-        else:
-            for influenced_field in current_field_mods["influenced_fields"]:
-                if influenced_field not in all_entity_fields:
-                    raise GlobalConfigValidatorException(
-                        f"""Modification in field '{current_field}' for not existing field '{influenced_field}'"""
-                    )
-                if influenced_field == current_field:
-                    raise GlobalConfigValidatorException(
-                        f"""Field '{current_field}' tries to modify itself"""
-                    )
+
+        if current_field in current_field_mods["influenced_fields_value_change"]:
+            # field can modify itself except "value" property
+            raise GlobalConfigValidatorException(
+                f"""Field '{current_field}' tries to modify itself value"""
+            )
+
+        for influenced_field in current_field_mods["influenced_fields"]:
+            if influenced_field not in all_entity_fields:
+                raise GlobalConfigValidatorException(
+                    f"""Modification in field '{current_field}' for not existing field '{influenced_field}'"""
+                )
+
+            if influenced_field in current_field_mods["influenced_fields_value_change"]:
                 if visited[influenced_field] == VISITING:
                     raise GlobalConfigValidatorException(
                         f"""Circular modifications for field '{influenced_field}' in field '{current_field}'"""
                     )
-                else:
-                    visited = self._is_circular(
-                        mods, visited, all_entity_fields, influenced_field
-                    )
+                # check next influenced by value change field
+                visited = self._is_circular_modification(
+                    mods, visited, all_entity_fields, influenced_field
+                )
+
         # All dependent modifications fields are dead_end
         visited[current_field] = DEAD_END
         return visited
 
-    def _check_if_circular(
+    def _check_if_circular_modification(
         self,
         all_entity_fields: List[Any],
         fields_with_mods: List[Any],
         modifications: List[Any],
     ) -> None:
         visited = {field: "not_visited" for field in all_entity_fields}
-
         for start_field in fields_with_mods:
             # DFS algorithm for all fields with modifications
-            visited = self._is_circular(
+            visited = self._is_circular_modification(
                 modifications, visited, all_entity_fields, start_field
             )
 
     @staticmethod
     def _get_mods_data_for_single_entity(
-        fields_with_mods: List[Any],
-        all_modifications: List[Any],
         entity: Dict[str, Any],
     ) -> List[Any]:
         """
-        Add modification entity data to lists and returns them
+        Get modification entity data as lists
         """
+        entity_modifications = []
         if "modifyFieldsOnValue" in entity:
+            influenced_fields_value_change = set()
             influenced_fields = set()
-            fields_with_mods.append(entity["field"])
             for mods in entity["modifyFieldsOnValue"]:
                 for mod in mods["fieldsToModify"]:
                     influenced_fields.add(mod["fieldId"])
-            all_modifications.append(
-                {"fieldId": entity["field"], "influenced_fields": influenced_fields}
+
+                    if (
+                        mod.get("value") is not None
+                    ):  # circular deps are not a problem if not about value
+                        influenced_fields_value_change.add(mod["fieldId"])
+            entity_modifications.append(
+                {
+                    "fieldId": entity["field"],
+                    "influenced_fields": influenced_fields,
+                    "influenced_fields_value_change": influenced_fields_value_change,
+                }
             )
-        return [fields_with_mods, all_modifications]
+        return entity_modifications
 
     @staticmethod
     def _get_all_entities(
@@ -638,11 +650,13 @@ def _get_all_modification_data(
 
         entities = self._get_all_entities(collections)
         for entity in entities:
-            self._get_mods_data_for_single_entity(
-                fields_with_mods, all_modifications, entity
-            )
             all_fields.append(entity["field"])
 
+            if "modifyFieldsOnValue" in entity:
+                fields_with_mods.append(entity["field"])
+                entity_mods = self._get_mods_data_for_single_entity(entity)
+                all_modifications.extend(entity_mods)
+
         return [fields_with_mods, all_modifications, all_fields]
 
     def _validate_field_modifications(self) -> None:
@@ -664,7 +678,7 @@ def _validate_field_modifications(self) -> None:
                 all_fields_config,
             ) = self._get_all_modification_data(tabs)
 
-            self._check_if_circular(
+            self._check_if_circular_modification(
                 all_fields_config, fields_with_mods_config, all_modifications_config
             )
 
@@ -678,7 +692,7 @@ def _validate_field_modifications(self) -> None:
                 all_fields_inputs,
             ) = self._get_all_modification_data(services)
 
-            self._check_if_circular(
+            self._check_if_circular_modification(
                 all_fields_inputs, fields_with_mods_inputs, all_modifications_inputs
             )
 

diff --git a/tests/unit/test_global_config_validator.py b/tests/unit/test_global_config_validator.py
@@ -337,7 +337,7 @@ def test_config_validation_modifications_on_change():
     [
         (
             "invalid_config_with_modification_for_field_itself.json",
-            "Field 'text1' tries to modify itself",
+            "Field 'text1' tries to modify itself value",
         ),
         (
             "invalid_config_with_modification_for_unexisiting_fields.json",

diff --git a/tests/unit/testdata/invalid_config_with_modification_circular_modifications.json b/tests/unit/testdata/invalid_config_with_modification_circular_modifications.json
@@ -53,7 +53,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text2",
-                                            "disabled": false
+                                            "value": "modification"
                                         }
                                     ]
                                 },
@@ -62,7 +62,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text3",
-                                            "disabled": true
+                                            "value": "modification"
                                         }
                                     ]
                                 }
@@ -79,7 +79,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text3",
-                                            "disabled": false
+                                            "value": "modification"
                                         }
                                     ]
                                 },
@@ -88,7 +88,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text4",
-                                            "disabled": true
+                                            "value": "modification"
                                         }
                                     ]
                                 }
@@ -111,7 +111,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text5",
-                                            "disabled": false
+                                            "value": "modification"
                                         }
                                     ]
                                 },
@@ -120,7 +120,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text5",
-                                            "disabled": true
+                                            "value": "modification"
                                         }
                                     ]
                                 }
@@ -137,7 +137,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text6",
-                                            "disabled": false
+                                            "value": "modification"
                                         }
                                     ]
                                 },
@@ -146,7 +146,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text6",
-                                            "disabled": true
+                                            "value": "modification"
                                         }
                                     ]
                                 }
@@ -163,7 +163,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text7",
-                                            "disabled": false
+                                            "value": "modification"
                                         }
                                     ]
                                 },
@@ -172,7 +172,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text7",
-                                            "disabled": true
+                                            "value": "modification"
                                         }
                                     ]
                                 }
@@ -189,7 +189,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text1",
-                                            "disabled": false
+                                            "value": "modification"
                                         }
                                     ]
                                 }

diff --git a/tests/unit/testdata/invalid_config_with_modification_for_field_itself.json b/tests/unit/testdata/invalid_config_with_modification_for_field_itself.json
@@ -53,7 +53,7 @@
                                     "fieldsToModify": [
                                         {
                                             "fieldId": "text1",
-                                            "disabled": false
+                                            "value": "value modification"
                                         }
                                     ]
                                 }

diff --git a/tests/unit/testdata/valid_config_with_modification_on_value_change.json b/tests/unit/testdata/valid_config_with_modification_on_value_change.json
@@ -54,6 +54,10 @@
                                         {
                                             "fieldId": "text2",
                                             "disabled": false
+                                        },
+                                        {
+                                            "fieldId": "text1",
+                                            "label": "change label for itself"
                                         }
                                     ]
                                 },

diff --git a/ui/src/components/BaseFormView/stories/BaseFormView.stories.tsx b/ui/src/components/BaseFormView/stories/BaseFormView.stories.tsx
@@ -1,6 +1,7 @@
 import React from 'react';
 import type { Meta, StoryObj } from '@storybook/react';
-import { fn } from '@storybook/test';
+import { fn, userEvent, within } from '@storybook/test';
+
 import BaseFormView from '../BaseFormView';
 import {
     PAGE_CONFIG_BOTH_OAUTH,
@@ -17,6 +18,8 @@ import {
     getGlobalConfigMockGroupsForConfigPage,
     getGlobalConfigMockModificationToGroupsConfig,
 } from '../BaseFormConfigMock';
+import { getGlobalConfigMockModificationToFieldItself } from '../tests/configMocks';
+import { invariant } from '../../../util/invariant';
 
 interface BaseFormStoriesProps extends BaseFormProps {
     config: GlobalConfig;
@@ -143,3 +146,31 @@ export const GroupModificationsConfig: Story = {
         platform: 'cloud',
     },
 };
+
+export const FieldModifyItself: Story = {
+    args: {
+        currentServiceState: {},
+        serviceName: 'account',
+        mode: 'create' as Mode,
+        page: 'configuration',
+        stanzaName: 'unknownStanza',
+        handleFormSubmit: fn(),
+        config: getGlobalConfigMockModificationToFieldItself(),
+    },
+};
+
+export const FieldModifyItselfAfterMods: Story = {
+    args: FieldModifyItself.args,
+    play: async ({ canvasElement }) => {
+        const canvas = within(canvasElement);
+
+        const modifyInputText = canvas
+            .getAllByRole('textbox')
+            .find((el) => el.getAttribute('value') === 'default value');
+
+        invariant(modifyInputText, 'modification input field should be defined');
+
+        await userEvent.clear(modifyInputText);
+        await userEvent.type(modifyInputText, 'modify itself');
+    },
+};
diff --git a/...iew/stories/__images__/BaseFormView-field-modify-itself-after-mods-chromium.png b/...iew/stories/__images__/BaseFormView-field-modify-itself-after-mods-chromium.png
diff --git a/...s/BaseFormView/stories/__images__/BaseFormView-field-modify-itself-chromium.png b/...s/BaseFormView/stories/__images__/BaseFormView-field-modify-itself-chromium.png
diff --git a/...onents/BaseFormView/BaseFormView.test.tsx → .../BaseFormView/tests/BaseFormView.test.tsx b/...onents/BaseFormView/BaseFormView.test.tsx → .../BaseFormView/tests/BaseFormView.test.tsx
@@ -2,16 +2,16 @@ import { render, screen } from '@testing-library/react';
 import React from 'react';
 import userEvent from '@testing-library/user-event';
 
-import { getGlobalConfigMock } from '../../mocks/globalConfigMock';
-import { setUnifiedConfig } from '../../util/util';
-import BaseFormView from './BaseFormView';
-import { getBuildDirPath } from '../../util/script';
-import mockCustomControlMockForTest from '../CustomControl/CustomControlMockForTest';
+import { getGlobalConfigMock } from '../../../mocks/globalConfigMock';
+import { getBuildDirPath } from '../../../util/script';
+import { setUnifiedConfig } from '../../../util/util';
 import {
     getGlobalConfigMockCustomControl,
     getGlobalConfigMockGroupsForInputPage,
     getGlobalConfigMockGroupsForConfigPage,
-} from './BaseFormConfigMock';
+} from '../BaseFormConfigMock';
+import mockCustomControlMockForTest from '../../CustomControl/CustomControlMockForTest';
+import BaseFormView from '../BaseFormView';
 
 const handleFormSubmit = jest.fn();