Fix bugs with mod options pushed down to component form states (#9090)

* fix the bug and add tests * fix tests, undo one change with removed form states * undo more changes * prose nits --------- Co-authored-by: Ben Loe <[email protected]>
pixiebrix · Sep 5, 2024 · dee6ee4 · dee6ee4
1 parent ad8d224
commit dee6ee4
Show file tree

Hide file tree

Showing 3 changed files with 169 additions and 5 deletions.
diff --git a/src/pageEditor/store/editor/editorSlice.test.ts b/src/pageEditor/store/editor/editorSlice.test.ts
@@ -36,13 +36,19 @@ import { type OutputKey } from "@/types/runtimeTypes";
 import { defaultBrickConfig } from "@/bricks/util";
 import { validateRegistryId } from "@/types/helpers";
 
-import { uuidSequence } from "@/testUtils/factories/stringFactories";
+import {
+  autoUUIDSequence,
+  uuidSequence,
+} from "@/testUtils/factories/stringFactories";
 import { formStateFactory } from "@/testUtils/factories/pageEditorFactories";
 import { brickConfigFactory } from "@/testUtils/factories/brickFactories";
 import { integrationDependencyFactory } from "@/testUtils/factories/integrationFactories";
 import { toExpression } from "@/utils/expressionUtils";
 import { getMaxMigrationsVersion } from "@/store/migratePersistedState";
 import { migrations } from "@/store/editorMigrations";
+import { modMetadataFactory } from "@/testUtils/factories/modComponentFactories";
+import { setActiveModId } from "./editorSliceHelpers";
+import { castDraft } from "immer";
 
 function getTabState(
   state: EditorState,
@@ -259,3 +265,142 @@ describe("persistEditorConfig", () => {
     expect(persistEditorConfig.version).toBe(maxVersion);
   });
 });
+
+describe("Mod Options editing", () => {
+  let initialState: EditorState;
+  const modId = validateRegistryId("test/mod");
+  const modMetadata = modMetadataFactory({ id: modId });
+
+  const existingComponentId = autoUUIDSequence();
+
+  const existingComponent = formStateFactory({
+    formStateConfig: {
+      uuid: existingComponentId,
+      modMetadata,
+    },
+  });
+
+  beforeEach(() => {
+    initialState = {
+      ...editorSlice.getInitialState(),
+      modComponentFormStates: [existingComponent],
+    };
+    // Make the mod active
+    setActiveModId(castDraft(initialState), modId);
+    // Edit the mod options
+    initialState = editorSlice.reducer(
+      initialState,
+      actions.editModOptionsValues({ testOption: "initial value" }),
+    );
+  });
+
+  test("mod options are updated correctly", () => {
+    const updatedOptionsArgs = { testOption: "updated value" };
+    const stateAfterEdit = editorSlice.reducer(
+      initialState,
+      actions.editModOptionsValues(updatedOptionsArgs),
+    );
+
+    // Check if the existing component's options are updated
+    const updatedExistingComponent = stateAfterEdit.modComponentFormStates.find(
+      (component) => component.uuid === existingComponentId,
+    );
+    expect(updatedExistingComponent?.optionsArgs).toEqual(updatedOptionsArgs);
+  });
+
+  // Skipping this for now, because there is some weirdness around how "deleted" form states inside mods are handled
+  // eslint-disable-next-line jest/no-disabled-tests -- see above
+  test.skip("deleted mod component form states have their mod options updated correctly", () => {
+    // Add another component with the same mod metadata, and then delete it
+    const additionalComponentId = autoUUIDSequence();
+    const newFormState = formStateFactory({
+      formStateConfig: {
+        uuid: additionalComponentId,
+        modMetadata,
+        optionsArgs: { testOption: "initial value" },
+      },
+    });
+
+    // Add the additional component
+    const stateAfterAddition = editorSlice.reducer(
+      initialState,
+      actions.addModComponentFormState(newFormState),
+    );
+
+    expect(stateAfterAddition.modComponentFormStates).toHaveLength(2);
+
+    // Delete the additional component
+    const stateAfterDeletion = {
+      // Need the object to be extensible
+      ...editorSlice.reducer(
+        stateAfterAddition,
+        actions.removeModComponentFormState(additionalComponentId),
+      ),
+    };
+
+    expect(stateAfterDeletion.modComponentFormStates).toHaveLength(1);
+    expect(
+      stateAfterDeletion.deletedModComponentFormStatesByModId[modId],
+    ).toContainEqual(
+      expect.objectContaining({
+        uuid: additionalComponentId,
+        optionsArgs: { testOption: "initial value" },
+      }),
+    );
+
+    // Make the mod active again
+    setActiveModId(castDraft(stateAfterDeletion), modId);
+
+    // Edit mod options values
+    const updatedOptionsArgs = { testOption: "updated value" };
+    const stateAfterEdit = editorSlice.reducer(
+      stateAfterDeletion,
+      actions.editModOptionsValues(updatedOptionsArgs),
+    );
+
+    // Check if the existing component's options are updated
+    const updatedExistingComponent = stateAfterEdit.modComponentFormStates.find(
+      (component) => component.uuid === existingComponentId,
+    );
+    expect(updatedExistingComponent?.optionsArgs).toEqual(updatedOptionsArgs);
+
+    // Check whether the deleted component's options are updated
+    const updatedDeletedComponents =
+      stateAfterEdit.deletedModComponentFormStatesByModId[modId];
+    expect(updatedDeletedComponents).toBeDefined();
+    const updatedDeletedComponent = updatedDeletedComponents?.find(
+      (component) => component.uuid === additionalComponentId,
+    );
+    expect(updatedDeletedComponent?.optionsArgs).toEqual(updatedOptionsArgs);
+
+    // Ensure the state is marked as dirty for the existing component
+    expect(stateAfterEdit.dirty[existingComponentId]).toBe(true);
+
+    // Ensure the state is not marked as dirty for the deleted component
+    expect(stateAfterEdit.dirty[additionalComponentId]).toBeUndefined();
+  });
+
+  test("new component with existing mod metadata receives dirty mod options", () => {
+    const newComponentId = autoUUIDSequence();
+    const newComponent = formStateFactory({
+      formStateConfig: {
+        uuid: newComponentId,
+        modMetadata: modMetadataFactory({ id: modId }),
+      },
+    });
+
+    const state = editorSlice.reducer(
+      initialState,
+      actions.addModComponentFormState(newComponent),
+    );
+
+    expect(state.modComponentFormStates).toHaveLength(2);
+    const addedComponent = state.modComponentFormStates.find(
+      (component) => component.uuid === newComponentId,
+    );
+    expect(addedComponent).toBeDefined();
+    expect(addedComponent?.optionsArgs).toEqual({
+      testOption: "initial value",
+    });
+  });
+});
diff --git a/src/pageEditor/store/editor/editorSlice.ts b/src/pageEditor/store/editor/editorSlice.ts
@@ -86,6 +86,7 @@ import {
   inspectedTab,
 } from "@/pageEditor/context/connection";
 import { assertNotNullish } from "@/utils/nullishUtils";
+import { collectModOptions } from "@/store/modComponents/modComponentUtils";
 
 export const initialState: EditorState = {
   selectionSeq: 0,
@@ -318,6 +319,23 @@ export const editorSlice = createSlice({
     ) {
       const modComponentFormState =
         action.payload as Draft<ModComponentFormState>;
+
+      // Check if the new form state has modMetadata
+      if (modComponentFormState.modMetadata) {
+        const modId = modComponentFormState.modMetadata.id;
+
+        // Find existing activated mod components with the same mod id
+        const existingModComponents = state.modComponentFormStates.filter(
+          (formState) => formState.modMetadata?.id === modId,
+        );
+
+        // If there are existing components, collect their options, and assign
+        if (existingModComponents.length > 0) {
+          const collectedOptions = collectModOptions(existingModComponents);
+          modComponentFormState.optionsArgs = collectedOptions;
+        }
+      }
+
       state.modComponentFormStates.push(modComponentFormState);
       state.dirty[modComponentFormState.uuid] = true;
 
@@ -881,10 +899,7 @@ export const editorSlice = createSlice({
       const notDeletedFormStates = selectNotDeletedModComponentFormStates({
         editor: state,
       });
-      const modFormStates = notDeletedFormStates.filter(
-        (formState) => formState.modMetadata?.id === modId,
-      );
-      for (const formState of modFormStates) {
+      for (const formState of notDeletedFormStates) {
         formState.optionsArgs = action.payload;
         state.dirty[formState.uuid] = true;
       }

diff --git a/src/store/editorStorage.test.ts b/src/store/editorStorage.test.ts
@@ -51,6 +51,10 @@ const setReduxStorageMock = jest.mocked(setReduxStorage);
 const currentPersistenceVersion = getMaxMigrationsVersion(migrations);
 
 describe("draftModComponentStorage", () => {
+  beforeEach(() => {
+    jest.resetAllMocks();
+  });
+
   test("removes one active form state", async () => {
     const formState = formStateFactory();
     const brickConfigurationUIStates: Record<UUID, BrickConfigurationUIState> =