From 739aab9b30ad0312c6fe4edde522f6103729c4b8 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Wed, 23 Oct 2024 16:15:50 -0400 Subject: [PATCH] #9323: deduplicate Page Editor save helpers (2/2) (#9332) --- .../add-button-starter-brick-to-mod.yaml | 35 ++-- ...add-context-menu-starter-brick-to-mod.diff | 99 +++++---- .../add-quick-bar-starter-brick-to-mod.diff | 89 +++++--- ...dd-sidebar-panel-starter-brick-to-mod.diff | 76 ++++--- .../add-trigger-starter-brick-to-mod.diff | 98 +++------ src/__mocks__/@/store/deactivateModHelpers.ts | 17 +- .../pages/mods/utils/useReactivateMod.test.ts | 10 +- .../hooks/updateReduxForSavedModDefinition.ts | 95 +++++++++ .../hooks/useBuildAndValidateMod.test.ts | 70 ++++--- .../hooks/useBuildAndValidateMod.ts | 70 +++---- .../useCheckModStarterBrickInvariants.ts | 2 +- src/pageEditor/hooks/useClearModChanges.ts | 23 ++- .../hooks/useClearModComponentChanges.ts | 1 + .../useCompareModComponentCounts.test.ts | 62 ------ .../hooks/useCompareModComponentCounts.ts | 2 +- .../hooks/useCreateModFromMod.test.ts | 69 ++++--- src/pageEditor/hooks/useCreateModFromMod.ts | 145 ++++++------- .../hooks/useCreateModFromModComponent.ts | 111 ++++------ .../hooks/useCreateModFromUnsavedMod.ts | 164 +++++---------- src/pageEditor/hooks/useDeactivateMod.tsx | 13 +- .../useRegisterDraftModInstanceOnAllFrames.ts | 2 +- src/pageEditor/hooks/useSaveMod.test.ts | 65 ++++-- src/pageEditor/hooks/useSaveMod.ts | 180 +++++++--------- .../useUpsertModComponentFormState.test.ts | 84 -------- .../hooks/useUpsertModComponentFormState.ts | 191 ----------------- .../modListingPanel/ModComponentListItem.tsx | 3 +- .../modListingPanel/ModSidebarListItems.tsx | 8 +- src/pageEditor/modListingPanel/common.ts | 11 - .../modListingPanel/filterSidebarItems.ts | 8 +- .../modListingPanel/modals/CreateModModal.tsx | 7 + src/pageEditor/panes/save/saveHelpers.test.ts | 90 ++++---- src/pageEditor/panes/save/saveHelpers.ts | 133 ++++++------ src/pageEditor/starterBricks/base.ts | 35 +--- ...ForMod.test.ts => editorSelectors.test.ts} | 2 +- .../store/editor/editorSelectors.ts | 64 +++++- src/pageEditor/store/editor/editorSlice.ts | 39 ++-- ...CleanComponentsAndDirtyFormStatesForMod.ts | 67 ------ .../ModOptionsValuesEditor.tsx | 24 +-- src/pageEditor/utils.ts | 42 +++- src/sidebar/DefaultContent.test.tsx | 13 +- src/store/modComponents/modComponentSlice.ts | 194 ++++++------------ .../modComponents/modComponentUtils.test.ts | 14 +- src/store/modComponents/modComponentUtils.ts | 2 +- src/store/modComponents/modInstanceUtils.ts | 4 +- .../sessionChangesListenerMiddleware.ts | 2 +- src/telemetry/events.ts | 2 - 46 files changed, 1067 insertions(+), 1470 deletions(-) create mode 100644 src/pageEditor/hooks/updateReduxForSavedModDefinition.ts delete mode 100644 src/pageEditor/hooks/useUpsertModComponentFormState.test.ts delete mode 100644 src/pageEditor/hooks/useUpsertModComponentFormState.ts rename src/pageEditor/store/editor/{selectGetCleanComponentsAndDirtyFormStatesForMod.test.ts => editorSelectors.test.ts} (99%) delete mode 100644 src/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod.ts diff --git a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-button-starter-brick-to-mod.yaml b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-button-starter-brick-to-mod.yaml index c2f2cba46d..48060879f7 100644 --- a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-button-starter-brick-to-mod.yaml +++ b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-button-starter-brick-to-mod.yaml @@ -21,7 +21,7 @@ definitions: extensionPoint: kind: extensionPoint definition: - type: trigger + type: menuItem reader: - '@pixiebrix/document-metadata' - '@pixiebrix/document-context' @@ -32,14 +32,15 @@ definitions: urlPatterns: [] selectors: [] allFrames: true - trigger: load - background: true - reportMode: once - showErrors: true + containerSelector: ul:has(> * > a:contains('navigation')) + targetMode: document + attachMode: once + position: append + template:
  • {{{ caption }}}
    • extensionPoint2: kind: extensionPoint definition: - type: menuItem + type: trigger reader: - '@pixiebrix/document-metadata' - '@pixiebrix/document-context' @@ -50,17 +51,11 @@ definitions: urlPatterns: [] selectors: [] allFrames: true - containerSelector: ul:has(> * > a:contains('navigation')) - targetMode: document - attachMode: once - position: append - template:
  • {{{ caption }}}
    • + trigger: load + background: true + reportMode: once + showErrors: true extensionPoints: - - label: My pbx.vercel.app trigger - config: - action: [] - id: extensionPoint - services: {} - label: My pbx.vercel.app button config: caption: Action @@ -71,5 +66,13 @@ extensionPoints: permissions: origins: [] permissions: [] + id: extensionPoint + services: {} + - label: My pbx.vercel.app trigger + config: + action: [] + permissions: + origins: [] + permissions: [] id: extensionPoint2 services: {} \ No newline at end of file diff --git a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-context-menu-starter-brick-to-mod.diff b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-context-menu-starter-brick-to-mod.diff index 6c6d17313d..107485c4e7 100644 --- a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-context-menu-starter-brick-to-mod.diff +++ b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-context-menu-starter-brick-to-mod.diff @@ -2,86 +2,95 @@ Index: add-context-menu-starter-brick-to-mod =================================================================== --- add-context-menu-starter-brick-to-mod +++ add-context-menu-starter-brick-to-mod -@@ -1,75 +1,101 @@ +@@ -1,78 +1,107 @@ apiVersion: v3 definitions: extensionPoint: definition: - background: true ++ contexts: ++ - all ++ documentUrlPatterns: ++ - '*://*/*' ++ isAvailable: ++ allFrames: true ++ matchPatterns: ++ - '*://*/*' ++ selectors: [] ++ urlPatterns: [] ++ reader: ++ - '@pixiebrix/document-metadata' ++ - '@pixiebrix/document-context' ++ - element: '@pixiebrix/html/element' ++ targetMode: eventTarget ++ type: contextMenu ++ kind: extensionPoint ++ extensionPoint2: ++ definition: + attachMode: once + containerSelector: ul:has(> * > a:contains('navigation')) isAvailable: allFrames: true matchPatterns: - https://pbx.vercel.app/* selectors: [] urlPatterns: [] + position: append reader: - '@pixiebrix/document-metadata' - '@pixiebrix/document-context' - element: '@pixiebrix/html/element' - reportMode: once - showErrors: true - trigger: load - type: trigger + targetMode: document + template:
  • {{{ caption }}}
    • + type: menuItem kind: extensionPoint - extensionPoint2: +- extensionPoint2: ++ extensionPoint5: definition: - attachMode: once - containerSelector: ul:has(> * > a:contains('navigation')) + background: true isAvailable: allFrames: true matchPatterns: - https://pbx.vercel.app/* selectors: [] urlPatterns: [] - position: append reader: - '@pixiebrix/document-metadata' - '@pixiebrix/document-context' - element: '@pixiebrix/html/element' - targetMode: document - template:
  • {{{ caption }}}
    • - type: menuItem + reportMode: once + showErrors: true + trigger: load + type: trigger kind: extensionPoint -+ extensionPoint3: -+ definition: -+ contexts: -+ - all -+ documentUrlPatterns: -+ - '*://*/*' -+ isAvailable: -+ allFrames: true -+ matchPatterns: -+ - '*://*/*' -+ selectors: [] -+ urlPatterns: [] -+ reader: -+ - '@pixiebrix/document-metadata' -+ - '@pixiebrix/document-context' -+ - element: '@pixiebrix/html/element' -+ targetMode: eventTarget -+ type: contextMenu -+ kind: extensionPoint extensionPoints: - config: action: [] - id: extensionPoint - label: My pbx.vercel.app trigger - services: {} - - config: - action: [] ++ onSuccess: true ++ title: Context menu item ++ id: extensionPoint ++ label: Context menu item ++ permissions: ++ origins: [] ++ permissions: [] ++ services: {} ++ - config: ++ action: [] caption: Action dynamicCaption: false onSuccess: true synchronous: false - id: extensionPoint2 +- id: extensionPoint ++ id: extensionPoint2 label: My pbx.vercel.app button -+ services: {} -+ - config: -+ action: [] -+ onSuccess: true -+ title: Context menu item -+ id: extensionPoint3 -+ label: Context menu item + permissions: + origins: [] + permissions: [] + services: {} + - config: + action: [] +- id: extensionPoint2 ++ id: extensionPoint5 + label: My pbx.vercel.app trigger permissions: origins: [] permissions: [] diff --git a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-quick-bar-starter-brick-to-mod.diff b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-quick-bar-starter-brick-to-mod.diff index 673485daf6..c0c7043469 100644 --- a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-quick-bar-starter-brick-to-mod.diff +++ b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-quick-bar-starter-brick-to-mod.diff @@ -2,48 +2,48 @@ Index: add-quick-bar-starter-brick-to-mod =================================================================== --- add-quick-bar-starter-brick-to-mod +++ add-quick-bar-starter-brick-to-mod -@@ -19,83 +19,109 @@ - type: trigger - kind: extensionPoint - extensionPoint2: +@@ -1,107 +1,136 @@ + apiVersion: v3 + definitions: + extensionPoint: definition: - attachMode: once - containerSelector: ul:has(> * > a:contains('navigation')) + contexts: + - all + documentUrlPatterns: + - '*://*/*' isAvailable: allFrames: true matchPatterns: - - https://pbx.vercel.app/* + - '*://*/*' selectors: [] urlPatterns: [] - position: append reader: - '@pixiebrix/document-metadata' - '@pixiebrix/document-context' - element: '@pixiebrix/html/element' - targetMode: document - template:
  • {{{ caption }}}
    • - type: menuItem + targetMode: eventTarget + type: contextMenu kind: extensionPoint - extensionPoint3: + extensionPoint2: definition: - contexts: - - all - documentUrlPatterns: - - '*://*/*' + attachMode: once + containerSelector: ul:has(> * > a:contains('navigation')) isAvailable: allFrames: true matchPatterns: - - '*://*/*' + - https://pbx.vercel.app/* selectors: [] urlPatterns: [] + position: append reader: - '@pixiebrix/document-metadata' - '@pixiebrix/document-context' - element: '@pixiebrix/html/element' - targetMode: eventTarget - type: contextMenu + targetMode: document + template:
  • {{{ caption }}}
    • + type: menuItem kind: extensionPoint -+ extensionPoint4: ++ extensionPoint3: + definition: + contexts: + - all @@ -63,11 +63,34 @@ Index: add-quick-bar-starter-brick-to-mod + targetMode: eventTarget + type: quickBar + kind: extensionPoint + extensionPoint5: + definition: + background: true + isAvailable: + allFrames: true + matchPatterns: + - https://pbx.vercel.app/* + selectors: [] + urlPatterns: [] + reader: + - '@pixiebrix/document-metadata' + - '@pixiebrix/document-context' + - element: '@pixiebrix/html/element' + reportMode: once + showErrors: true + trigger: load + type: trigger + kind: extensionPoint extensionPoints: - config: action: [] + onSuccess: true + title: Context menu item id: extensionPoint - label: My pbx.vercel.app trigger + label: Context menu item + permissions: + origins: [] + permissions: [] services: {} - config: action: [] @@ -77,23 +100,27 @@ Index: add-quick-bar-starter-brick-to-mod synchronous: false id: extensionPoint2 label: My pbx.vercel.app button + permissions: + origins: [] + permissions: [] services: {} - config: action: [] - onSuccess: true - title: Context menu item - id: extensionPoint3 - label: Context menu item -+ services: {} -+ - config: -+ action: [] -+ title: Quick Bar item -+ id: extensionPoint4 -+ label: Quick Bar item + id: extensionPoint5 + label: My pbx.vercel.app trigger permissions: origins: [] permissions: [] services: {} ++ - config: ++ action: [] ++ title: Quick Bar item ++ id: extensionPoint3 ++ label: Quick Bar item ++ permissions: ++ origins: [] ++ permissions: [] ++ services: {} kind: recipe metadata: description: Created by playwright test for adding starter bricks to a mod diff --git a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-sidebar-panel-starter-brick-to-mod.diff b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-sidebar-panel-starter-brick-to-mod.diff index 7407a2900a..33fb5e2dbd 100644 --- a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-sidebar-panel-starter-brick-to-mod.diff +++ b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-sidebar-panel-starter-brick-to-mod.diff @@ -2,28 +2,28 @@ Index: add-sidebar-panel-starter-brick-to-mod =================================================================== --- add-sidebar-panel-starter-brick-to-mod +++ add-sidebar-panel-starter-brick-to-mod -@@ -39,89 +39,141 @@ +@@ -21,116 +21,171 @@ kind: extensionPoint - extensionPoint3: + extensionPoint2: definition: - contexts: - - all - documentUrlPatterns: - - '*://*/*' + attachMode: once + containerSelector: ul:has(> * > a:contains('navigation')) isAvailable: allFrames: true matchPatterns: - - '*://*/*' + - https://pbx.vercel.app/* selectors: [] urlPatterns: [] + position: append reader: - '@pixiebrix/document-metadata' - '@pixiebrix/document-context' - element: '@pixiebrix/html/element' - targetMode: eventTarget - type: contextMenu + targetMode: document + template:
  • {{{ caption }}}
    • + type: menuItem kind: extensionPoint - extensionPoint4: + extensionPoint3: definition: contexts: - all @@ -43,7 +43,7 @@ Index: add-sidebar-panel-starter-brick-to-mod targetMode: eventTarget type: quickBar kind: extensionPoint -+ extensionPoint5: ++ extensionPoint4: + definition: + customEvent: null + debounce: @@ -62,11 +62,34 @@ Index: add-sidebar-panel-starter-brick-to-mod + trigger: load + type: actionPanel + kind: extensionPoint + extensionPoint5: + definition: + background: true + isAvailable: + allFrames: true + matchPatterns: + - https://pbx.vercel.app/* + selectors: [] + urlPatterns: [] + reader: + - '@pixiebrix/document-metadata' + - '@pixiebrix/document-context' + - element: '@pixiebrix/html/element' + reportMode: once + showErrors: true + trigger: load + type: trigger + kind: extensionPoint extensionPoints: - config: action: [] + onSuccess: true + title: Context menu item id: extensionPoint - label: My pbx.vercel.app trigger + label: Context menu item + permissions: + origins: [] + permissions: [] services: {} - config: action: [] @@ -76,20 +99,27 @@ Index: add-sidebar-panel-starter-brick-to-mod synchronous: false id: extensionPoint2 label: My pbx.vercel.app button + permissions: + origins: [] + permissions: [] services: {} - config: action: [] - onSuccess: true - title: Context menu item - id: extensionPoint3 - label: Context menu item + id: extensionPoint5 + label: My pbx.vercel.app trigger + permissions: + origins: [] + permissions: [] services: {} - config: action: [] title: Quick Bar item - id: extensionPoint4 + id: extensionPoint3 label: Quick Bar item -+ services: {} + permissions: + origins: [] + permissions: [] + services: {} + - config: + body: + - config: @@ -120,12 +150,12 @@ Index: add-sidebar-panel-starter-brick-to-mod + id: '@pixiebrix/document' + rootMode: document + heading: Sidebar Panel -+ id: extensionPoint5 ++ id: extensionPoint4 + label: Sidebar Panel - permissions: - origins: [] - permissions: [] - services: {} ++ permissions: ++ origins: [] ++ permissions: [] ++ services: {} kind: recipe metadata: description: Created by playwright test for adding starter bricks to a mod diff --git a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-trigger-starter-brick-to-mod.diff b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-trigger-starter-brick-to-mod.diff index 20a24498d7..4673319e2f 100644 --- a/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-trigger-starter-brick-to-mod.diff +++ b/end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts-snapshots/Add-starter-brick-to-mod/add-trigger-starter-brick-to-mod.diff @@ -2,74 +2,61 @@ Index: add-trigger-starter-brick-to-mod =================================================================== --- add-trigger-starter-brick-to-mod +++ add-trigger-starter-brick-to-mod -@@ -66,114 +66,119 @@ - allFrames: true - matchPatterns: - - '*://*/*' - selectors: [] - urlPatterns: [] - reader: - - '@pixiebrix/document-metadata' +@@ -91,80 +91,88 @@ - '@pixiebrix/document-context' - element: '@pixiebrix/html/element' - - '@pixiebrix/selection' - targetMode: eventTarget - type: quickBar - kind: extensionPoint - extensionPoint5: - definition: - customEvent: null - debounce: - leading: false - trailing: true - waitMillis: 250 - isAvailable: - allFrames: true - matchPatterns: - - '*://*/*' - selectors: [] - urlPatterns: [] - reader: - - '@pixiebrix/document-metadata' - - '@pixiebrix/document-context' + reportMode: once + showErrors: true trigger: load - type: actionPanel + type: trigger kind: extensionPoint extensionPoints: - config: action: [] + onSuccess: true + title: Context menu item id: extensionPoint - label: My pbx.vercel.app trigger + label: Context menu item + permissions: + origins: [] + permissions: [] services: {} - config: action: [] -+ id: extensionPoint -+ label: My pbx.vercel.app trigger -+ permissions: -+ origins: [] -+ permissions: [] -+ services: {} -+ - config: -+ action: [] caption: Action dynamicCaption: false onSuccess: true synchronous: false id: extensionPoint2 label: My pbx.vercel.app button + permissions: + origins: [] + permissions: [] services: {} - config: action: [] - onSuccess: true - title: Context menu item - id: extensionPoint3 - label: Context menu item + id: extensionPoint5 + label: My pbx.vercel.app trigger + permissions: + origins: [] + permissions: [] services: {} - config: action: [] ++ id: extensionPoint5 ++ label: My pbx.vercel.app trigger ++ permissions: ++ origins: [] ++ permissions: [] ++ services: {} ++ - config: ++ action: [] title: Quick Bar item - id: extensionPoint4 + id: extensionPoint3 label: Quick Bar item + permissions: + origins: [] + permissions: [] services: {} - config: body: @@ -101,27 +88,6 @@ Index: add-trigger-starter-brick-to-mod id: '@pixiebrix/document' rootMode: document heading: Sidebar Panel - id: extensionPoint5 + id: extensionPoint4 label: Sidebar Panel -- permissions: -- origins: [] -- permissions: [] - services: {} - kind: recipe - metadata: - description: Created by playwright test for adding starter bricks to a mod - id: >- - @extension-e2e-test-unaffiliated/new-mod-00000000-0000-0000-0000-000000000000 - name: New Mod - version: 1.0.0 - options: - schema: - properties: {} - type: object - uiSchema: - ui:order: - - '*' - variables: - schema: - properties: {} - type: object + permissions: diff --git a/src/__mocks__/@/store/deactivateModHelpers.ts b/src/__mocks__/@/store/deactivateModHelpers.ts index 491b853112..26f9d0f4fa 100644 --- a/src/__mocks__/@/store/deactivateModHelpers.ts +++ b/src/__mocks__/@/store/deactivateModHelpers.ts @@ -15,5 +15,20 @@ * along with this program. If not, see . */ -export const deactivateMod = jest.fn(); +import { actions as modComponentActions } from "@/store/modComponents/modComponentSlice"; +import type { RegistryId } from "@/types/registryTypes"; +import type { UUID } from "@/types/stringTypes"; +import type { Dispatch } from "react"; + +export const deactivateMod = jest.fn( + async ( + modId: RegistryId, + _modComponentIds: UUID[], + dispatch: Dispatch, + ) => { + // Keep the call to dispatch, but leave off reading/writing to the Page Editor storage and runtime side effects + dispatch(modComponentActions.removeModById(modId)); + }, +); + export const removeModDataAndInterfaceFromAllTabs = jest.fn(); diff --git a/src/extensionConsole/pages/mods/utils/useReactivateMod.test.ts b/src/extensionConsole/pages/mods/utils/useReactivateMod.test.ts index 1b8b04321d..b97c81fbc6 100644 --- a/src/extensionConsole/pages/mods/utils/useReactivateMod.test.ts +++ b/src/extensionConsole/pages/mods/utils/useReactivateMod.test.ts @@ -24,14 +24,14 @@ import { defaultModDefinitionFactory } from "@/testUtils/factories/modDefinition import { selectActivatedModComponents } from "@/store/modComponents/modComponentSelectors"; beforeEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); }); test("deactivates mod components", async () => { const modDefinition = defaultModDefinitionFactory(); const { - result: { current: reactivate }, + result: { current: reactivateMod }, act, getReduxStore, } = renderHook(() => useReactivateMod(), { @@ -50,7 +50,7 @@ test("deactivates mod components", async () => { getReduxStore().getState() as ModComponentsRootState, )[0]; - await act(async () => reactivate(modDefinition)); + await act(async () => reactivateMod(modDefinition)); expect(deactivateMod).toHaveBeenCalledWith( modDefinition.metadata.id, @@ -65,7 +65,7 @@ test("dispatches activate mod action", async () => { const modDefinition = defaultModDefinitionFactory(); const { - result: { current: reactivate }, + result: { current: reactivateMod }, act, } = renderHook(() => useReactivateMod(), { setupRedux(dispatch) { @@ -79,7 +79,7 @@ test("dispatches activate mod action", async () => { }, }); - await act(async () => reactivate(modDefinition)); + await act(async () => reactivateMod(modDefinition)); expect(modComponentActions.activateMod).toHaveBeenCalled(); }); diff --git a/src/pageEditor/hooks/updateReduxForSavedModDefinition.ts b/src/pageEditor/hooks/updateReduxForSavedModDefinition.ts new file mode 100644 index 0000000000..030956ea0f --- /dev/null +++ b/src/pageEditor/hooks/updateReduxForSavedModDefinition.ts @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2024 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import type { ModComponentFormState } from "@/pageEditor/starterBricks/formStateTypes"; +import type { ModComponentBase } from "@/types/modComponentTypes"; +import { actions as modComponentActions } from "@/store/modComponents/modComponentSlice"; +import collectExistingConfiguredDependenciesForMod from "@/integrations/util/collectExistingConfiguredDependenciesForMod"; +import { collectModOptionsArgs } from "@/store/modComponents/modComponentUtils"; +import type { AnyAction, ThunkDispatch } from "@reduxjs/toolkit"; +import type { ModDefinition } from "@/types/modDefinitionTypes"; +import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; +import mapModDefinitionToModMetadata from "@/modDefinitions/util/mapModDefinitionToModMetadata"; +import type { RegistryId } from "@/types/registryTypes"; +import { getDraftModComponentId } from "@/pageEditor/utils"; +import type { EditorRootState } from "@/pageEditor/store/editor/pageEditorTypes"; +import type { ModComponentsRootState } from "@/store/modComponents/modComponentTypes"; + +/** + * Update Redux for a saved mod definition. + * @param modIdToReplace if provided, the mod to replace in the Page Editor. + * @param modDefinition the mod definition to save + * @param draftModComponents mod components for determining options args and configured dependencies. If + * modIdToReplace is provided, the component ids must correspond to the mod ids for the new mod instance. + * @param isReactivate true if an activated mod is being reactivated + */ +function updateReduxForSavedModDefinition({ + modIdToReplace, + modDefinition, + draftModComponents, + isReactivate, +}: { + modIdToReplace?: RegistryId; + modDefinition: ModDefinition; + draftModComponents: Array; + isReactivate: boolean; +}) { + return async ( + dispatch: ThunkDispatch< + EditorRootState & ModComponentsRootState, + unknown, + AnyAction + >, + ): Promise => { + const modMetadata = mapModDefinitionToModMetadata(modDefinition); + + // Activate/re-activate the mod + dispatch(modComponentActions.removeModById(modDefinition.metadata.id)); + + dispatch( + modComponentActions.activateMod({ + modDefinition, + modComponentIds: modIdToReplace + ? draftModComponents.map((x) => getDraftModComponentId(x)) + : undefined, + configuredDependencies: collectExistingConfiguredDependenciesForMod( + modDefinition, + draftModComponents, + ), + optionsArgs: collectModOptionsArgs(draftModComponents), + screen: "pageEditor", + isReactivate, + }), + ); + + // Must dispatch updateModMetadataOnModComponentFormStates first to so the form states are associated with the new + // mod id. Otherwise, any other actions won't be able to find the form states. + dispatch( + editorActions.updateModMetadataOnModComponentFormStates({ + modId: modIdToReplace ?? modMetadata.id, + modMetadata, + }), + ); + + dispatch(editorActions.markModAsCleanById(modMetadata.id)); + + // Refresh mod availability on page, but don't await because it's not required for save flow + void dispatch(editorActions.checkAvailableActivatedModComponents()); + }; +} + +export default updateReduxForSavedModDefinition; diff --git a/src/pageEditor/hooks/useBuildAndValidateMod.test.ts b/src/pageEditor/hooks/useBuildAndValidateMod.test.ts index 9bdff0949e..13eec50bf0 100644 --- a/src/pageEditor/hooks/useBuildAndValidateMod.test.ts +++ b/src/pageEditor/hooks/useBuildAndValidateMod.test.ts @@ -105,34 +105,32 @@ describe("useBuildAndValidateMod", () => { }), ); - // Collect the dirty form states for any changed mod components - const modComponentFormStates: ModComponentFormState[] = []; + const starterBricks = selectStarterBricks(modDefinition); - if (dirtyModComponentCount > 0) { - const starterBricks = selectStarterBricks(modDefinition); + // Collect the dirty form states for any changed mod components + const dirtyModComponentFormStates: ModComponentFormState[] = []; - for (let i = 0; i < dirtyModComponentCount; i++) { - const starterBrick = starterBricks[i]!; - // Mock this lookup for the adapter call that follows - jest.mocked(lookupStarterBrick).mockResolvedValue(starterBrick); + for (let i = 0; i < dirtyModComponentCount; i++) { + const starterBrick = starterBricks[i]!; + // Mock this lookup for the adapter call that follows + jest.mocked(lookupStarterBrick).mockResolvedValue(starterBrick); - // Mod was activated, so get the mod component from state - const modComponent = state.activatedModComponents[i]!; + // Mod was activated, so get the mod component from state + const modComponent = state.activatedModComponents[i]!; - // Load the adapter for this mod component - const { fromModComponent } = adapter(starterBrick.definition.type); + // Load the adapter for this mod component + const { fromModComponent } = adapter(starterBrick.definition.type); - // Use the adapter to convert to FormState - // eslint-disable-next-line no-await-in-loop -- This is much easier to read than a large Promise.all() block - const modComponentFormState = (await fromModComponent( - modComponent, - )) as ModComponentFormState; + // Use the adapter to convert to FormState + // eslint-disable-next-line no-await-in-loop -- This is much easier to read than a large Promise.all() block + const modComponentFormState = (await fromModComponent( + modComponent, + )) as ModComponentFormState; - // Edit the label - modComponentFormState.label = `New Label ${i}`; + // Edit the label + modComponentFormState.label = `New Label ${i}`; - modComponentFormStates.push(modComponentFormState); - } + dirtyModComponentFormStates.push(modComponentFormState); } const { result } = renderHook(() => useBuildAndValidateMod(), { @@ -148,17 +146,19 @@ describe("useBuildAndValidateMod", () => { }); await hookAct(async () => { - const newMod = await result.current.buildAndValidateMod({ - sourceMod: modDefinition, - // Only pass in the unchanged clean mod components - cleanModComponents: state.activatedModComponents.slice( - dirtyModComponentCount, - ), - dirtyModComponentFormStates: modComponentFormStates, + const actualModDefinition = await result.current.buildAndValidateMod({ + sourceModDefinition: modDefinition, + draftModComponents: [ + // `buildAndValidate` now preserves mod component order. So order of dirty vs. clean must match the + // construction for expectedModDefinition + ...dirtyModComponentFormStates, + // Only pass in the unchanged clean mod components + ...state.activatedModComponents.slice(dirtyModComponentCount), + ], }); // Update the source mod with the expected label changes - const updatedMod = produce(modDefinition, (draft) => { + const expectedModDefinition = produce(modDefinition, (draft) => { for (const [index, starterBrick] of draft.extensionPoints .slice(0, dirtyModComponentCount) .entries()) { @@ -167,8 +167,8 @@ describe("useBuildAndValidateMod", () => { }); // Compare results - expect(normalizeModDefinition(newMod)).toStrictEqual( - normalizeModDefinition(updatedMod), + expect(normalizeModDefinition(actualModDefinition)).toStrictEqual( + normalizeModDefinition(expectedModDefinition), ); }); }, @@ -208,9 +208,11 @@ describe("useBuildAndValidateMod", () => { await hookAct(async () => { await expect( result.current.buildAndValidateMod({ - sourceMod: activatedModDefinition, - cleanModComponents: state.activatedModComponents.slice(1), - dirtyModComponentFormStates: [dirtyFormState1], + sourceModDefinition: activatedModDefinition, + draftModComponents: [ + ...state.activatedModComponents.slice(1), + dirtyFormState1, + ], }), ).rejects.toThrow("Mod save failed due to data integrity error"); }); diff --git a/src/pageEditor/hooks/useBuildAndValidateMod.ts b/src/pageEditor/hooks/useBuildAndValidateMod.ts index e5e076a6c2..78e149eb63 100644 --- a/src/pageEditor/hooks/useBuildAndValidateMod.ts +++ b/src/pageEditor/hooks/useBuildAndValidateMod.ts @@ -23,62 +23,61 @@ import reportEvent from "@/telemetry/reportEvent"; import { useCallback } from "react"; import { Events } from "@/telemetry/events"; import { BusinessError } from "@/errors/businessErrors"; -import { useDispatch } from "react-redux"; import useCheckModStarterBrickInvariants from "@/pageEditor/hooks/useCheckModStarterBrickInvariants"; import useCompareModComponentCounts from "@/pageEditor/hooks/useCompareModComponentCounts"; -import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; import { type JsonObject } from "type-fest"; import { type UnsavedModDefinition } from "@/types/modDefinitionTypes"; -import { isEmpty } from "lodash"; type UseBuildAndValidateModReturn = { - buildAndValidateMod: ( - modParts: Partial, - ) => Promise; + buildAndValidateMod: (modParts: ModParts) => Promise; }; +/** + * Error that a mod save failed due to data integrity check failures. + */ +export class DataIntegrityError extends BusinessError { + override name = "DataIntegrityError"; + + constructor() { + super("Mod save failed due to data integrity error"); + } +} + function useBuildAndValidateMod(): UseBuildAndValidateModReturn { - const dispatch = useDispatch(); const compareModComponentCountsToModDefinition = useCompareModComponentCounts(); const checkModStarterBrickInvariants = useCheckModStarterBrickInvariants(); const buildAndValidateMod = useCallback( async ({ - sourceMod, - cleanModComponents = [], - dirtyModComponentFormStates: existingDirtyModComponentFormStates = [], + sourceModDefinition, + draftModComponents, dirtyModOptionsDefinition, dirtyModMetadata, - }: Partial) => { - if ( - isEmpty(cleanModComponents) && - isEmpty(existingDirtyModComponentFormStates) - ) { - throw new Error("Error saving mod - no mod components found to save"); + }: ModParts) => { + if (draftModComponents.length === 0) { + throw new Error("Expected mod components to save"); } - const dirtyModComponentFormStates = [ - ...existingDirtyModComponentFormStates, - ]; - - const newMod = buildNewMod({ - sourceMod, - cleanModComponents, - dirtyModComponentFormStates, + const newModDefinition = buildNewMod({ + sourceModDefinition, + draftModComponents, dirtyModOptionsDefinition, dirtyModMetadata, }); - if (sourceMod) { + if (sourceModDefinition) { const modComponentDefinitionCountsMatch = compareModComponentCountsToModDefinition( - newMod, - sourceMod.metadata.id, + newModDefinition, + sourceModDefinition.metadata.id, ); const modComponentStarterBricksMatch = - await checkModStarterBrickInvariants(newMod, sourceMod.metadata.id); + await checkModStarterBrickInvariants( + newModDefinition, + sourceModDefinition.metadata.id, + ); if ( !modComponentDefinitionCountsMatch || @@ -88,25 +87,18 @@ function useBuildAndValidateMod(): UseBuildAndValidateModReturn { // See discussion: https://github.com/pixiebrix/pixiebrix-extension/pull/7629/files#r1492864349 reportEvent(Events.PAGE_EDITOR_MOD_SAVE_ERROR, { // Metadata is an object, but doesn't extend JsonObject so typescript doesn't like it - modMetadata: newMod.metadata as unknown as JsonObject, + modMetadata: newModDefinition.metadata as unknown as JsonObject, modComponentDefinitionCountsMatch, modComponentStarterBricksMatch, }); - dispatch(editorActions.showSaveDataIntegrityErrorModal()); - throw new BusinessError( - "Mod save failed due to data integrity error", - ); + throw new DataIntegrityError(); } } - return newMod; + return newModDefinition; }, - [ - checkModStarterBrickInvariants, - compareModComponentCountsToModDefinition, - dispatch, - ], + [checkModStarterBrickInvariants, compareModComponentCountsToModDefinition], ); return { buildAndValidateMod }; diff --git a/src/pageEditor/hooks/useCheckModStarterBrickInvariants.ts b/src/pageEditor/hooks/useCheckModStarterBrickInvariants.ts index 7cf1256aa8..c24969246e 100644 --- a/src/pageEditor/hooks/useCheckModStarterBrickInvariants.ts +++ b/src/pageEditor/hooks/useCheckModStarterBrickInvariants.ts @@ -19,7 +19,6 @@ import { type UnsavedModDefinition } from "@/types/modDefinitionTypes"; import { useCallback } from "react"; import { useSelector } from "react-redux"; import { isInnerDefinitionRegistryId } from "@/types/helpers"; -import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod"; import { isStarterBrickDefinitionLike, type StarterBrickDefinitionLike, @@ -33,6 +32,7 @@ import { import produce from "immer"; import { buildModComponents } from "@/pageEditor/panes/save/saveHelpers"; import { adapterForComponent } from "@/pageEditor/starterBricks/adapter"; +import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/editorSelectors"; function useCheckModStarterBrickInvariants(): ( unsavedModDefinition: UnsavedModDefinition, diff --git a/src/pageEditor/hooks/useClearModChanges.ts b/src/pageEditor/hooks/useClearModChanges.ts index 06c61844f0..380dadcd4b 100644 --- a/src/pageEditor/hooks/useClearModChanges.ts +++ b/src/pageEditor/hooks/useClearModChanges.ts @@ -17,11 +17,11 @@ import { useCallback } from "react"; import { type RegistryId } from "@/types/registryTypes"; -import { actions } from "@/pageEditor/store/editor/editorSlice"; +import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; import { useModals } from "@/components/ConfirmationModal"; import { useDispatch, useSelector } from "react-redux"; import useClearModComponentChanges from "@/pageEditor/hooks/useClearModComponentChanges"; -import { selectModComponentFormStates } from "@/pageEditor/store/editor/editorSelectors"; +import { selectGetModComponentFormStatesByModId } from "@/pageEditor/store/editor/editorSelectors"; /** * Hook that returns a callback to clear unsaved mod changes for a given mod id. @@ -31,7 +31,9 @@ function useClearModChanges(): (modId: RegistryId) => Promise { const { showConfirmation } = useModals(); const dispatch = useDispatch(); const clearModComponentChanges = useClearModComponentChanges(); - const modComponentFormStates = useSelector(selectModComponentFormStates); + const getModComponentFormStatesByModId = useSelector( + selectGetModComponentFormStatesByModId, + ); return useCallback( async (modId: RegistryId) => { @@ -46,23 +48,22 @@ function useClearModChanges(): (modId: RegistryId) => Promise { } await Promise.all( - modComponentFormStates - .filter((x) => x.modMetadata.id === modId) - .map(async (modComponentFormState) => + getModComponentFormStatesByModId(modId).map( + async (modComponentFormState) => clearModComponentChanges({ modComponentId: modComponentFormState.uuid, shouldShowConfirmation: false, }), - ), + ), ); - dispatch(actions.clearMetadataAndOptionsChangesForMod(modId)); - dispatch(actions.clearDeletedModComponentFormStatesForMod(modId)); - dispatch(actions.setActiveModId(modId)); + dispatch(editorActions.markModAsCleanById(modId)); + + dispatch(editorActions.setActiveModId(modId)); }, [ dispatch, - modComponentFormStates, + getModComponentFormStatesByModId, clearModComponentChanges, showConfirmation, ], diff --git a/src/pageEditor/hooks/useClearModComponentChanges.ts b/src/pageEditor/hooks/useClearModComponentChanges.ts index 3cd8c71a68..7bd7b63f90 100644 --- a/src/pageEditor/hooks/useClearModComponentChanges.ts +++ b/src/pageEditor/hooks/useClearModComponentChanges.ts @@ -65,6 +65,7 @@ function useClearModComponentChanges(): ( modComponentId, }); + // Avoid showing stale traces void clearModComponentTraces(modComponentId); try { diff --git a/src/pageEditor/hooks/useCompareModComponentCounts.test.ts b/src/pageEditor/hooks/useCompareModComponentCounts.test.ts index bb669b01ed..5f86b9c4c9 100644 --- a/src/pageEditor/hooks/useCompareModComponentCounts.test.ts +++ b/src/pageEditor/hooks/useCompareModComponentCounts.test.ts @@ -266,68 +266,6 @@ describe("useCompareModComponentCounts", () => { expect(result.current(unsavedModDefinition, modMetadata.id)).toBe(false); }); - it("should return false for 0 clean and 1 unmatching form state and 0 in definition", () => { - const modMetadata = modMetadataFactory(); - const modDefinition = modDefinitionFactory({ - metadata: modMetadata, - extensionPoints: [], - }); - - const formState = formStateFactory({ - formStateConfig: { - modMetadata, - }, - }); - - const { result } = renderHook(() => useCompareModComponentCounts(), { - setupRedux(dispatch) { - dispatch( - modComponentActions.activateMod({ - modDefinition, - screen: "extensionConsole", - isReactivate: false, - }), - ); - dispatch(editorActions.addModComponentFormState(formState)); - }, - }); - - expect(result.current(modDefinition, modMetadata.id)).toBe(false); - }); - - it("should return false for 0 clean and 1 unmatching form state and 2 in definition", () => { - const modMetadata = modMetadataFactory(); - const activatedModDefinition = modDefinitionFactory({ - metadata: modMetadata, - extensionPoints: [], - }); - const unsavedModDefinition = modDefinitionFactory({ - metadata: modMetadata, - extensionPoints: array(modComponentDefinitionFactory, 2), - }); - - const formState = formStateFactory({ - formStateConfig: { - modMetadata, - }, - }); - - const { result } = renderHook(() => useCompareModComponentCounts(), { - setupRedux(dispatch) { - dispatch( - modComponentActions.activateMod({ - modDefinition: activatedModDefinition, - screen: "extensionConsole", - isReactivate: false, - }), - ); - dispatch(editorActions.addModComponentFormState(formState)); - }, - }); - - expect(result.current(unsavedModDefinition, modMetadata.id)).toBe(false); - }); - it("should return false for 3 clean and 1 unmatching form state and 3 in definition", () => { const modMetadata = modMetadataFactory(); const modDefinition = modDefinitionFactory({ diff --git a/src/pageEditor/hooks/useCompareModComponentCounts.ts b/src/pageEditor/hooks/useCompareModComponentCounts.ts index f04293a0a8..d0dd339d69 100644 --- a/src/pageEditor/hooks/useCompareModComponentCounts.ts +++ b/src/pageEditor/hooks/useCompareModComponentCounts.ts @@ -18,8 +18,8 @@ import { type UnsavedModDefinition } from "@/types/modDefinitionTypes"; import { useSelector } from "react-redux"; import { useCallback } from "react"; -import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod"; import { type RegistryId } from "@/types/registryTypes"; +import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/editorSelectors"; /** * @returns A function that compares the number of mod components in the redux state and the mod definition diff --git a/src/pageEditor/hooks/useCreateModFromMod.test.ts b/src/pageEditor/hooks/useCreateModFromMod.test.ts index 1a3506da4a..5fe11dd817 100644 --- a/src/pageEditor/hooks/useCreateModFromMod.test.ts +++ b/src/pageEditor/hooks/useCreateModFromMod.test.ts @@ -20,7 +20,7 @@ import { modComponentDefinitionFactory, modDefinitionFactory, } from "@/testUtils/factories/modDefinitionFactories"; -import { hookAct, renderHook, waitFor } from "@/pageEditor/testHelpers"; +import { hookAct, renderHook } from "@/pageEditor/testHelpers"; import useCreateModFromMod from "@/pageEditor/hooks/useCreateModFromMod"; import { modMetadataFactory } from "@/testUtils/factories/modComponentFactories"; import { actions as modComponentActions } from "@/store/modComponents/modComponentSlice"; @@ -30,6 +30,8 @@ import { array } from "cooky-cutter"; import useCompareModComponentCounts from "@/pageEditor/hooks/useCompareModComponentCounts"; import useCheckModStarterBrickInvariants from "@/pageEditor/hooks/useCheckModStarterBrickInvariants"; import { API_PATHS } from "@/data/service/urlPaths"; +import { timestampFactory } from "@/testUtils/factories/stringFactories"; +import { DataIntegrityError } from "@/pageEditor/hooks/useBuildAndValidateMod"; const reportEventMock = jest.mocked(reportEvent); jest.mock("@/telemetry/trace"); @@ -53,20 +55,20 @@ describe("useCreateModFromMod", () => { }); it("saves with no dirty changes", async () => { - const metadata = modMetadataFactory(); - const definition = modDefinitionFactory({ - metadata, + const sourceMetadata = modMetadataFactory(); + const sourceDefinition = modDefinitionFactory({ + metadata: sourceMetadata, }); appApiMock .onPost(API_PATHS.BRICKS) - .reply(200, { updated_at: "2024-01-01T00:00:00Z" }); + .reply(200, { updated_at: timestampFactory() }); - const { result } = renderHook(() => useCreateModFromMod(), { + const { result, act } = renderHook(() => useCreateModFromMod(), { setupRedux(dispatch) { dispatch( modComponentActions.activateMod({ - modDefinition: definition, + modDefinition: sourceDefinition, screen: "pageEditor", isReactivate: false, }), @@ -74,17 +76,14 @@ describe("useCreateModFromMod", () => { }, }); - await hookAct(async () => { - await result.current.createModFromMod(definition, metadata); + await act(async () => { + await result.current.createModFromMod( + sourceDefinition, + modMetadataFactory(), + ); }); expect(appApiMock.history.post).toHaveLength(1); - await waitFor(() => { - expect(reportEventMock).toHaveBeenCalledWith( - Events.STARTER_BRICK_ACTIVATE, - expect.any(Object), - ); - }); expect(reportEventMock).toHaveBeenCalledWith( Events.MOD_ACTIVATE, @@ -92,11 +91,11 @@ describe("useCreateModFromMod", () => { ); }); - it("does not throw an error if the mod fails the compareModComponentCounts check", async () => { + it("throws DataIntegrityError if mod fails the compareModComponentCounts check", async () => { compareModComponentCountsMock.mockReturnValue(() => false); - const modMetadata = modMetadataFactory(); - const activatedModDefinition = modDefinitionFactory({ - metadata: modMetadata, + const sourceMetadata = modMetadataFactory(); + const sourceModDefinition = modDefinitionFactory({ + metadata: sourceMetadata, extensionPoints: array(modComponentDefinitionFactory, 2), }); @@ -108,7 +107,7 @@ describe("useCreateModFromMod", () => { setupRedux(dispatch) { dispatch( modComponentActions.activateMod({ - modDefinition: activatedModDefinition, + modDefinition: sourceModDefinition, screen: "pageEditor", isReactivate: false, }), @@ -117,20 +116,22 @@ describe("useCreateModFromMod", () => { }); await hookAct(async () => { - await result.current.createModFromMod( - activatedModDefinition, - modMetadata, - ); + await expect( + result.current.createModFromMod( + sourceModDefinition, + modMetadataFactory(), + ), + ).rejects.toThrow(DataIntegrityError); }); expect(appApiMock.history.post).toHaveLength(0); }); - it("does not throw an error if the mod fails the checkModStarterBrickInvariants check", async () => { + it("throws DataIntegrityError if mod fails the checkModStarterBrickInvariants check", async () => { checkModStarterBrickInvariantsMock.mockReturnValue(async () => false); - const modMetadata = modMetadataFactory(); - const activatedModDefinition = modDefinitionFactory({ - metadata: modMetadata, + const sourceModMetadata = modMetadataFactory(); + const sourceModDefinition = modDefinitionFactory({ + metadata: sourceModMetadata, extensionPoints: array(modComponentDefinitionFactory, 2), }); @@ -142,7 +143,7 @@ describe("useCreateModFromMod", () => { setupRedux(dispatch) { dispatch( modComponentActions.activateMod({ - modDefinition: activatedModDefinition, + modDefinition: sourceModDefinition, screen: "pageEditor", isReactivate: false, }), @@ -151,10 +152,12 @@ describe("useCreateModFromMod", () => { }); await hookAct(async () => { - await result.current.createModFromMod( - activatedModDefinition, - modMetadata, - ); + await expect( + result.current.createModFromMod( + sourceModDefinition, + modMetadataFactory(), + ), + ).rejects.toThrow(DataIntegrityError); }); expect(appApiMock.history.post).toHaveLength(0); diff --git a/src/pageEditor/hooks/useCreateModFromMod.ts b/src/pageEditor/hooks/useCreateModFromMod.ts index ebb578e01d..3b44d9d62e 100644 --- a/src/pageEditor/hooks/useCreateModFromMod.ts +++ b/src/pageEditor/hooks/useCreateModFromMod.ts @@ -16,25 +16,27 @@ */ import { useCreateModDefinitionMutation } from "@/data/service/api"; -import collectExistingConfiguredDependenciesForMod from "@/integrations/util/collectExistingConfiguredDependenciesForMod"; import useDeactivateMod from "@/pageEditor/hooks/useDeactivateMod"; import { type ModMetadataFormState } from "@/pageEditor/store/editor/pageEditorTypes"; import { selectDirtyModOptionsDefinitions, + selectGetDraftModComponentsForMod, selectKeepLocalCopyOnCreateMod, } from "@/pageEditor/store/editor/editorSelectors"; -import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod"; -import { collectModOptions } from "@/store/modComponents/modComponentUtils"; import reportEvent from "@/telemetry/reportEvent"; import { type ModDefinition } from "@/types/modDefinitionTypes"; import { useCallback } from "react"; import { useDispatch, useSelector } from "react-redux"; import { Events } from "@/telemetry/events"; -import { actions as modComponentActions } from "@/store/modComponents/modComponentSlice"; import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; import useBuildAndValidateMod from "@/pageEditor/hooks/useBuildAndValidateMod"; -import { BusinessError } from "@/errors/businessErrors"; import { ensureModComponentFormStatePermissionsFromUserGesture } from "@/pageEditor/editorPermissionsHelpers"; +import { + isModComponentFormState, + mapModDefinitionUpsertResponseToModDefinition, +} from "@/pageEditor/utils"; +import { createPrivateSharing } from "@/utils/registryUtils"; +import updateReduxForSavedModDefinition from "@/pageEditor/hooks/updateReduxForSavedModDefinition"; type UseCreateModFromModReturn = { createModFromMod: ( @@ -49,102 +51,87 @@ type UseCreateModFromModReturn = { */ function useCreateModFromMod(): UseCreateModFromModReturn { const dispatch = useDispatch(); - const [createMod] = useCreateModDefinitionMutation(); + const [createModDefinitionOnServer] = useCreateModDefinitionMutation(); const deactivateMod = useDeactivateMod(); - const getCleanComponentsAndDirtyFormStatesForMod = useSelector( - selectGetCleanComponentsAndDirtyFormStatesForMod, + const getDraftModComponentsForMod = useSelector( + selectGetDraftModComponentsForMod, + ); + const dirtyModOptionsDefinitionsMap = useSelector( + selectDirtyModOptionsDefinitions, ); - const dirtyModOptions = useSelector(selectDirtyModOptionsDefinitions); const keepLocalCopy = useSelector(selectKeepLocalCopyOnCreateMod); const { buildAndValidateMod } = useBuildAndValidateMod(); const createModFromMod = useCallback( - // eslint-disable-next-line @typescript-eslint/promise-function-async -- permissions check must be called in the user gesture context, `async-await` can break the call chain - (modDefinition: ModDefinition, metadata: ModMetadataFormState) => { - const modId = modDefinition.metadata.id; - const { cleanModComponents, dirtyModComponentFormStates } = - getCleanComponentsAndDirtyFormStatesForMod(modId); + async ( + sourceModDefinition: ModDefinition, + newModMetadata: ModMetadataFormState, + ) => { + const sourceModId = sourceModDefinition.metadata.id; + + if (sourceModId === newModMetadata.id) { + throw new Error( + "Expected new mod ID to be different from source mod ID", + ); + } + + const draftModComponents = getDraftModComponentsForMod(sourceModId); return ensureModComponentFormStatePermissionsFromUserGesture( - dirtyModComponentFormStates, + draftModComponents.filter((x) => isModComponentFormState(x)), // eslint-disable-next-line promise/prefer-await-to-then -- permissions check must be called in the user gesture context, `async-await` can break the call chain ).then(async (hasPermissions) => { if (!hasPermissions) { return; } - // eslint-disable-next-line security/detect-object-injection -- new mod IDs are sanitized in the form validation - const dirtyModOptionsDefinition = dirtyModOptions[modId]; + const newModId = newModMetadata.id; - try { - const newModDefinition = await buildAndValidateMod({ - sourceMod: modDefinition, - cleanModComponents, - dirtyModComponentFormStates, - dirtyModOptionsDefinition, - dirtyModMetadata: metadata, - }); + const unsavedModDefinition = await buildAndValidateMod({ + sourceModDefinition, + draftModComponents, + // eslint-disable-next-line security/detect-object-injection -- new mod IDs are sanitized in the form validation + dirtyModOptionsDefinition: dirtyModOptionsDefinitionsMap[sourceModId], + dirtyModMetadata: newModMetadata, + }); - const upsertResponse = await createMod({ - modDefinition: newModDefinition, - organizations: [], - public: false, - }).unwrap(); - - const savedModDefinition: ModDefinition = { - ...newModDefinition, - sharing: { - public: upsertResponse.public, - organizations: upsertResponse.organizations, - }, - updated_at: upsertResponse.updated_at, - }; - - if (!keepLocalCopy) { - await deactivateMod({ modId, shouldShowConfirmation: false }); - } - - const modComponents = [ - ...dirtyModComponentFormStates, - ...cleanModComponents, - ]; - - dispatch( - modComponentActions.activateMod({ - modDefinition: savedModDefinition, - configuredDependencies: - collectExistingConfiguredDependenciesForMod( - savedModDefinition, - modComponents, - ), - optionsArgs: collectModOptions(modComponents), - screen: "pageEditor", - isReactivate: false, - }), - ); - dispatch( - editorActions.setActiveModId(savedModDefinition.metadata.id), - ); - dispatch(editorActions.checkAvailableActivatedModComponents()); - - reportEvent(Events.PAGE_EDITOR_MOD_CREATE, { - copiedFrom: modId, - modId: savedModDefinition.metadata.id, + const upsertResponse = await createModDefinitionOnServer({ + modDefinition: unsavedModDefinition, + ...createPrivateSharing(), + }).unwrap(); + + await updateReduxForSavedModDefinition({ + // In the future, could consider passing the source mod id here if keepLocalCopy is false so that Page + // Editor navigation state is preserved for the source mod form states + modIdToReplace: undefined, + modDefinition: mapModDefinitionUpsertResponseToModDefinition( + unsavedModDefinition, + upsertResponse, + ), + draftModComponents, + isReactivate: false, + })(dispatch); + + dispatch(editorActions.setActiveModId(newModId)); + + if (!keepLocalCopy) { + await deactivateMod({ + modId: sourceModDefinition.metadata.id, + shouldShowConfirmation: false, }); - } catch (error) { - if (error instanceof BusinessError) { - // Error is already handled by buildAndValidateMod. - } else { - throw error; - } // Other errors can be thrown during mod activation } + + reportEvent(Events.PAGE_EDITOR_MOD_CREATE, { + copiedFrom: sourceModId, + modId: newModId, + }); }); }, [ - getCleanComponentsAndDirtyFormStatesForMod, - dirtyModOptions, + getDraftModComponentsForMod, + dirtyModOptionsDefinitionsMap, buildAndValidateMod, - createMod, + createModDefinitionOnServer, keepLocalCopy, dispatch, deactivateMod, diff --git a/src/pageEditor/hooks/useCreateModFromModComponent.ts b/src/pageEditor/hooks/useCreateModFromModComponent.ts index 5de0fa7a0f..252380d7a8 100644 --- a/src/pageEditor/hooks/useCreateModFromModComponent.ts +++ b/src/pageEditor/hooks/useCreateModFromModComponent.ts @@ -19,20 +19,18 @@ import { ensureModComponentFormStatePermissionsFromUserGesture } from "@/pageEdi import { type ModMetadataFormState } from "@/pageEditor/store/editor/pageEditorTypes"; import { type ModComponentFormState } from "@/pageEditor/starterBricks/formStateTypes"; import reportEvent from "@/telemetry/reportEvent"; -import { uuidv4 } from "@/types/helpers"; -import produce from "immer"; import { useCallback } from "react"; import { Events } from "@/telemetry/events"; import { useCreateModDefinitionMutation } from "@/data/service/api"; import { useDispatch, useSelector } from "react-redux"; import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; -import useUpsertModComponentFormState from "@/pageEditor/hooks/useUpsertModComponentFormState"; -import { mapModDefinitionUpsertResponseToModMetadata } from "@/pageEditor/utils"; +import { mapModDefinitionUpsertResponseToModDefinition } from "@/pageEditor/utils"; import { selectKeepLocalCopyOnCreateMod } from "@/pageEditor/store/editor/editorSelectors"; import useDeleteDraftModComponent from "@/pageEditor/hooks/useDeleteDraftModComponent"; import useBuildAndValidateMod from "@/pageEditor/hooks/useBuildAndValidateMod"; -import { BusinessError } from "@/errors/businessErrors"; -import { type Nullishable } from "@/utils/nullishUtils"; +import { assertNotNullish, type Nullishable } from "@/utils/nullishUtils"; +import { createPrivateSharing } from "@/utils/registryUtils"; +import updateReduxForSavedModDefinition from "@/pageEditor/hooks/updateReduxForSavedModDefinition"; type UseCreateModFromModReturn = { createModFromComponent: ( @@ -42,96 +40,75 @@ type UseCreateModFromModReturn = { }; function useCreateModFromModComponent( - activeModComponent: Nullishable, + activeModComponentFormState: Nullishable, ): UseCreateModFromModReturn { const dispatch = useDispatch(); const keepLocalCopy = useSelector(selectKeepLocalCopyOnCreateMod); - const [createMod] = useCreateModDefinitionMutation(); - const upsertModComponentFormState = useUpsertModComponentFormState(); + const [createModDefinitionOnServer] = useCreateModDefinitionMutation(); const deleteDraftModComponent = useDeleteDraftModComponent(); const { buildAndValidateMod } = useBuildAndValidateMod(); const createModFromComponent = useCallback( ( modComponentFormState: ModComponentFormState, - modMetadata: ModMetadataFormState, + newModMetadata: ModMetadataFormState, // eslint-disable-next-line @typescript-eslint/promise-function-async -- permissions check must be called in the user gesture context, `async-await` can break the call chain ) => ensureModComponentFormStatePermissionsFromUserGesture( modComponentFormState, // eslint-disable-next-line promise/prefer-await-to-then -- permissions check must be called in the user gesture context, `async-await` can break the call chain ).then(async (hasPermissions) => { - if (!hasPermissions || !activeModComponent) { - return; - } - - const newModComponentFormState = produce( - activeModComponent, - (draft) => { - draft.uuid = uuidv4(); - }, + assertNotNullish( + activeModComponentFormState, + "Expected mod component to be selected", ); - try { - const newModDefinition = await buildAndValidateMod({ - dirtyModComponentFormStates: [newModComponentFormState], - dirtyModMetadata: modMetadata, - }); - - const upsertResponse = await createMod({ - modDefinition: newModDefinition, - organizations: [], - public: false, - }).unwrap(); + if (!hasPermissions) { + return; + } - const newModComponent = produce(newModComponentFormState, (draft) => { - draft.modMetadata = mapModDefinitionUpsertResponseToModMetadata( - newModDefinition, - upsertResponse, - ); - }); + const modId = newModMetadata.id; + const draftModComponents = [activeModComponentFormState]; - dispatch(editorActions.addModComponentFormState(newModComponent)); + const unsavedModDefinition = await buildAndValidateMod({ + draftModComponents, + dirtyModMetadata: newModMetadata, + }); - await upsertModComponentFormState({ - modComponentFormState: newModComponent, - options: { - // Permissions are already checked above - checkPermissions: false, - // Need to provide user feedback - notifySuccess: true, - reactivateEveryTab: true, - }, - modId: newModDefinition.metadata.id, - }); + const upsertResponse = await createModDefinitionOnServer({ + modDefinition: unsavedModDefinition, + ...createPrivateSharing(), + }).unwrap(); - if (!keepLocalCopy) { - // Delete the mod component from the source mod - await deleteDraftModComponent({ - modComponentId: activeModComponent.uuid, - }); - } + await updateReduxForSavedModDefinition({ + modDefinition: mapModDefinitionUpsertResponseToModDefinition( + unsavedModDefinition, + upsertResponse, + ), + // Safe to pass form state that has the old mod component ID because the form states are only used + // to determine mod option args and integration dependencies + draftModComponents, + isReactivate: false, + })(dispatch); - // Check the new component availability, so it's added to available components if needed - dispatch(editorActions.checkActiveModComponentAvailability()); + dispatch(editorActions.setActiveModId(modId)); - reportEvent(Events.PAGE_EDITOR_MOD_CREATE, { - modId: newModDefinition.metadata.id, + if (!keepLocalCopy) { + // Delete the mod component from the source mod + await deleteDraftModComponent({ + modComponentId: modComponentFormState.uuid, }); - } catch (error) { - if (error instanceof BusinessError) { - // Error is already handled by buildAndValidateMod. - } else { - throw error; - } // Other errors can be thrown during mod activation } + + reportEvent(Events.PAGE_EDITOR_MOD_CREATE, { + modId, + }); }), [ - activeModComponent, + activeModComponentFormState, buildAndValidateMod, - createMod, + createModDefinitionOnServer, dispatch, - upsertModComponentFormState, keepLocalCopy, deleteDraftModComponent, ], diff --git a/src/pageEditor/hooks/useCreateModFromUnsavedMod.ts b/src/pageEditor/hooks/useCreateModFromUnsavedMod.ts index ee0e4328da..6c0ce1ea22 100644 --- a/src/pageEditor/hooks/useCreateModFromUnsavedMod.ts +++ b/src/pageEditor/hooks/useCreateModFromUnsavedMod.ts @@ -18,22 +18,23 @@ import { ensureModComponentFormStatePermissionsFromUserGesture } from "@/pageEditor/editorPermissionsHelpers"; import { type ModMetadataFormState } from "@/pageEditor/store/editor/pageEditorTypes"; import reportEvent from "@/telemetry/reportEvent"; -import produce from "immer"; import { useCallback } from "react"; import { Events } from "@/telemetry/events"; import { useCreateModDefinitionMutation } from "@/data/service/api"; import { useDispatch, useSelector } from "react-redux"; import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; -import { mapModDefinitionUpsertResponseToModMetadata } from "@/pageEditor/utils"; +import { + isModComponentFormState, + mapModDefinitionUpsertResponseToModDefinition, +} from "@/pageEditor/utils"; import useBuildAndValidateMod from "@/pageEditor/hooks/useBuildAndValidateMod"; -import { BusinessError } from "@/errors/businessErrors"; import { type RegistryId } from "@/types/registryTypes"; -import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod"; -import { adapterForComponent } from "@/pageEditor/starterBricks/adapter"; -import { actions as modComponentActions } from "@/store/modComponents/modComponentSlice"; -import { isInnerDefinitionRegistryId } from "@/types/helpers"; -import { modComponentWithInnerDefinitions } from "@/pageEditor/starterBricks/base"; -import { selectDirtyModOptionsDefinitions } from "@/pageEditor/store/editor/editorSelectors"; +import { + selectDirtyModOptionsDefinitions, + selectGetDraftModComponentsForMod, +} from "@/pageEditor/store/editor/editorSelectors"; +import { createPrivateSharing } from "@/utils/registryUtils"; +import updateReduxForSavedModDefinition from "@/pageEditor/hooks/updateReduxForSavedModDefinition"; type UseCreateModFromUnsavedModReturn = { createModFromUnsavedMod: ( @@ -48,12 +49,14 @@ type UseCreateModFromUnsavedModReturn = { */ function useCreateModFromUnsavedMod(): UseCreateModFromUnsavedModReturn { const dispatch = useDispatch(); - const [createMod] = useCreateModDefinitionMutation(); + const [createModDefinitionOnServer] = useCreateModDefinitionMutation(); const { buildAndValidateMod } = useBuildAndValidateMod(); - const getCleanComponentsAndDirtyFormStatesForMod = useSelector( - selectGetCleanComponentsAndDirtyFormStatesForMod, + const getDraftModComponentsForMod = useSelector( + selectGetDraftModComponentsForMod, + ); + const dirtyModOptionsDefinitionMap = useSelector( + selectDirtyModOptionsDefinitions, ); - const dirtyModOptionsById = useSelector(selectDirtyModOptionsDefinitions); /** * Save a new, unsaved mod to the server. @@ -64,118 +67,57 @@ function useCreateModFromUnsavedMod(): UseCreateModFromUnsavedModReturn { const createModFromUnsavedMod = useCallback( ( unsavedModId: RegistryId, - modMetadata: ModMetadataFormState, + newModMetadata: ModMetadataFormState, // eslint-disable-next-line @typescript-eslint/promise-function-async -- permissions check must be called in the user gesture context, `async-await` can break the call chain ) => { - const { cleanModComponents, dirtyModComponentFormStates } = - getCleanComponentsAndDirtyFormStatesForMod(unsavedModId); - // eslint-disable-next-line security/detect-object-injection -- RegistryId - const dirtyModOptionsDefinition = dirtyModOptionsById[unsavedModId]; + const draftModComponents = getDraftModComponentsForMod(unsavedModId); + + const dirtyModOptionsDefinition = + dirtyModOptionsDefinitionMap[unsavedModId]; return ensureModComponentFormStatePermissionsFromUserGesture( - dirtyModComponentFormStates, + draftModComponents.filter((x) => isModComponentFormState(x)), // eslint-disable-next-line promise/prefer-await-to-then -- permissions check must be called in the user gesture context, `async-await` can break the call chain ).then(async (hasPermissions) => { if (!hasPermissions) { return; } - try { - const newModDefinition = await buildAndValidateMod({ - dirtyModComponentFormStates, - cleanModComponents, - dirtyModMetadata: modMetadata, - dirtyModOptionsDefinition, - }); - - const createResponse = await createMod({ - modDefinition: newModDefinition, - organizations: [], - public: false, - }).unwrap(); - - const newComponentFormStates = dirtyModComponentFormStates.map( - (dirtyModComponentFormState) => - produce(dirtyModComponentFormState, (draft) => { - draft.modMetadata = mapModDefinitionUpsertResponseToModMetadata( - newModDefinition, - createResponse, - ); - }), - ); - - for (const newComponentFormState of newComponentFormStates) { - const { selectModComponent, selectStarterBrickDefinition } = - adapterForComponent(newComponentFormState); - const starterBrickId = - newComponentFormState.starterBrick.metadata.id; - const hasInnerStarterBrick = - isInnerDefinitionRegistryId(starterBrickId); - let newModComponent = selectModComponent(newComponentFormState); - - // The Page Editor only supports editing inline Starter Brick definitions, not Starter Brick packages. - // Therefore, no logic is required here for Starter Brick registry packages. - if (hasInnerStarterBrick) { - // Starter brick has an inner definition and doesn't exist as a registry item - const { definition } = selectStarterBrickDefinition( - newComponentFormState, - ); - newModComponent = modComponentWithInnerDefinitions( - newModComponent, - definition, - ); - } - - dispatch( - modComponentActions.saveModComponent({ - modComponent: { - ...newModComponent, - updateTimestamp: createResponse.updated_at, - }, - }), - ); - - // TODO: remove use of setModComponentFormState: https://github.com/pixiebrix/pixiebrix-extension/issues/9323 - dispatch( - editorActions.setModComponentFormState({ - modComponentFormState: newComponentFormState, - includesNonFormikChanges: true, - dirty: false, - }), - ); - dispatch( - editorActions.markModComponentFormStateAsClean( - newComponentFormState.uuid, - ), - ); - } - - const newModId = newModDefinition.metadata.id; - dispatch( - editorActions.clearMetadataAndOptionsChangesForMod(newModId), - ); - dispatch( - editorActions.clearDeletedModComponentFormStatesForMod(newModId), - ); - dispatch(editorActions.setActiveModId(newModId)); - - reportEvent(Events.PAGE_EDITOR_MOD_CREATE, { - modId: newModDefinition.metadata.id, - }); - } catch (error) { - if (error instanceof BusinessError) { - // Error is already handled by buildAndValidateMod. - } else { - throw error; - } // Other errors can be thrown during mod activation - } + const newModId = newModMetadata.id; + + const unsavedModDefinition = await buildAndValidateMod({ + draftModComponents, + dirtyModMetadata: newModMetadata, + dirtyModOptionsDefinition, + }); + + const upsertResponse = await createModDefinitionOnServer({ + modDefinition: unsavedModDefinition, + ...createPrivateSharing(), + }).unwrap(); + + await updateReduxForSavedModDefinition({ + modIdToReplace: unsavedModId, + modDefinition: mapModDefinitionUpsertResponseToModDefinition( + unsavedModDefinition, + upsertResponse, + ), + draftModComponents, + isReactivate: false, + })(dispatch); + + dispatch(editorActions.setActiveModId(newModId)); + + reportEvent(Events.PAGE_EDITOR_MOD_CREATE, { + modId: newModId, + }); }); }, [ - getCleanComponentsAndDirtyFormStatesForMod, - dirtyModOptionsById, + getDraftModComponentsForMod, + dirtyModOptionsDefinitionMap, buildAndValidateMod, - createMod, + createModDefinitionOnServer, dispatch, ], ); diff --git a/src/pageEditor/hooks/useDeactivateMod.tsx b/src/pageEditor/hooks/useDeactivateMod.tsx index 0e73b93b00..2c0d98bda7 100644 --- a/src/pageEditor/hooks/useDeactivateMod.tsx +++ b/src/pageEditor/hooks/useDeactivateMod.tsx @@ -27,7 +27,8 @@ import { actions as modComponentActions } from "@/store/modComponents/modCompone import { isInnerDefinitionRegistryId } from "@/types/helpers"; import { removeModDataAndInterfaceFromAllTabs } from "@/store/deactivateModHelpers"; import { selectModInstanceMap } from "@/store/modComponents/modInstanceSelectors"; -import { selectGetDraftModComponentIdsForMod } from "@/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod"; +import { getDraftModComponentId } from "@/pageEditor/utils"; +import { selectGetDraftModComponentsForMod } from "@/pageEditor/store/editor/editorSelectors"; type Config = { modId: RegistryId; @@ -70,8 +71,8 @@ function useDeactivateMod(): (useDeactivateConfig: Config) => Promise { const dispatch = useDispatch(); const { showConfirmation } = useModals(); const modInstanceMap = useSelector(selectModInstanceMap); - const getDraftModComponentIds = useSelector( - selectGetDraftModComponentIdsForMod, + const getDraftModComponentsForMod = useSelector( + selectGetDraftModComponentsForMod, ); return useCallback( @@ -93,7 +94,9 @@ function useDeactivateMod(): (useDeactivateConfig: Config) => Promise { dispatch(editorActions.removeModById(modId)); await removeModDataAndInterfaceFromAllTabs( modId, - getDraftModComponentIds(modId), + getDraftModComponentsForMod(modId).map((x) => + getDraftModComponentId(x), + ), ); // Remove the activated mod instance from all tabs, if one exists @@ -106,7 +109,7 @@ function useDeactivateMod(): (useDeactivateConfig: Config) => Promise { ); } }, - [dispatch, showConfirmation, modInstanceMap, getDraftModComponentIds], + [dispatch, showConfirmation, modInstanceMap, getDraftModComponentsForMod], ); } diff --git a/src/pageEditor/hooks/useRegisterDraftModInstanceOnAllFrames.ts b/src/pageEditor/hooks/useRegisterDraftModInstanceOnAllFrames.ts index 04f11564d1..64d04b9a40 100644 --- a/src/pageEditor/hooks/useRegisterDraftModInstanceOnAllFrames.ts +++ b/src/pageEditor/hooks/useRegisterDraftModInstanceOnAllFrames.ts @@ -31,9 +31,9 @@ import { useSelector } from "react-redux"; import { selectActiveModComponentFormState, selectCurrentModId, + selectGetCleanComponentsAndDirtyFormStatesForMod, } from "@/pageEditor/store/editor/editorSelectors"; import { StarterBrickTypes } from "@/types/starterBrickTypes"; -import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod"; import { selectModInstanceMap } from "@/store/modComponents/modInstanceSelectors"; import type { ModInstance } from "@/types/modInstanceTypes"; import { mapModInstanceToActivatedModComponents } from "@/store/modComponents/modInstanceUtils"; diff --git a/src/pageEditor/hooks/useSaveMod.test.ts b/src/pageEditor/hooks/useSaveMod.test.ts index f0b0aefcde..442a076441 100644 --- a/src/pageEditor/hooks/useSaveMod.test.ts +++ b/src/pageEditor/hooks/useSaveMod.test.ts @@ -30,23 +30,40 @@ import { actions as editorActions, editorSlice, } from "@/pageEditor/store/editor/editorSlice"; -import type { EditablePackageMetadata } from "@/types/contract"; +import type { + EditablePackageMetadata, + PackageUpsertResponse, +} from "@/types/contract"; import modComponentSlice from "@/store/modComponents/modComponentSlice"; import { type UUID } from "@/types/stringTypes"; import { API_PATHS } from "@/data/service/urlPaths"; import { createNewUnsavedModMetadata } from "@/utils/modUtils"; import { formStateFactory } from "@/testUtils/factories/pageEditorFactories"; +import { createPrivateSharing } from "@/utils/registryUtils"; +import { timestampFactory } from "@/testUtils/factories/stringFactories"; const modId = validateRegistryId("@test/mod"); jest.mock("@/utils/notify"); jest.mock("@/contentScript/messenger/api"); -describe("useSaveMod", () => { - beforeEach(() => { - jest.clearAllMocks(); - }); +beforeEach(() => { + jest.clearAllMocks(); +}); + +function packageUpsertResponseFactory( + editablePackage: EditablePackageMetadata, +): PackageUpsertResponse { + return { + ...editablePackage, + ...createPrivateSharing(), + updated_at: timestampFactory(), + // OK to pass empty string because it's not relevant to the tests + config: "", + }; +} +describe("useSaveMod", () => { it("saves with no dirty changes", async () => { appApiMock.reset(); @@ -54,7 +71,7 @@ describe("useSaveMod", () => { name: modId, }); - const definition = defaultModDefinitionFactory({ + const modDefinition = defaultModDefinitionFactory({ metadata: { id: editablePackage.name, name: editablePackage.verbose_name!, @@ -66,20 +83,21 @@ describe("useSaveMod", () => { modDefinitionRegistry.register([ { id: modId, - ...definition, + ...modDefinition, }, ]); appApiMock.onGet(API_PATHS.BRICKS).reply(200, [editablePackage]); - appApiMock.onPut(API_PATHS.BRICK(editablePackage.id)).reply(200, {}); - appApiMock.onPut(API_PATHS.BRICK(editablePackage.id)).reply(200, {}); + appApiMock + .onPut(API_PATHS.BRICK(editablePackage.id)) + .reply(200, packageUpsertResponseFactory(editablePackage)); const { result, waitForEffect } = renderHook(() => useSaveMod(), { setupRedux(dispatch) { dispatch( modComponentSlice.actions.activateMod({ - modDefinition: definition, + modDefinition, screen: "pageEditor", isReactivate: false, }), @@ -93,8 +111,9 @@ describe("useSaveMod", () => { await result.current(modId); }); - expect(notify.success).toHaveBeenCalledWith("Saved mod"); + // Assert error first to assist with debugging failures expect(notify.error).not.toHaveBeenCalled(); + expect(notify.success).toHaveBeenCalledWith("Saved mod"); }); it("preserves original options if no dirty options", async () => { @@ -136,7 +155,7 @@ describe("useSaveMod", () => { const putMock = appApiMock .onPut(API_PATHS.BRICK(editablePackage.id)) - .reply(200, {}); + .reply(201, packageUpsertResponseFactory(editablePackage)); const { result, waitForEffect } = renderHook(() => useSaveMod(), { setupRedux(dispatch) { @@ -156,6 +175,8 @@ describe("useSaveMod", () => { await result.current(modId); }); + // Assert error first to assist with debugging failures + expect(notify.error).not.toHaveBeenCalled(); expect(notify.success).toHaveBeenCalledWith("Saved mod"); const yamlConfig = ( @@ -205,7 +226,7 @@ describe("useSaveMod", () => { const putMock = appApiMock .onPut(API_PATHS.BRICK(editablePackage.id)) - .reply(200, {}); + .reply(200, packageUpsertResponseFactory(editablePackage)); const { result, waitForEffect } = renderHook(() => useSaveMod(), { setupRedux(dispatch) { @@ -238,6 +259,8 @@ describe("useSaveMod", () => { await result.current(modId); }); + // Assert error first to assist with debugging failures + expect(notify.error).not.toHaveBeenCalled(); expect(notify.success).toHaveBeenCalledWith("Saved mod"); const yamlConfig = ( @@ -274,13 +297,15 @@ describe("useSaveMod", () => { setupRedux(dispatch, { store }) { jest.spyOn(store, "dispatch"); - const formState = formStateFactory({ - formStateConfig: { - modMetadata: temporaryModMetadata, - }, - }); - - dispatch(editorActions.addModComponentFormState(formState)); + dispatch( + editorActions.addModComponentFormState( + formStateFactory({ + formStateConfig: { + modMetadata: temporaryModMetadata, + }, + }), + ), + ); }, }, ); diff --git a/src/pageEditor/hooks/useSaveMod.ts b/src/pageEditor/hooks/useSaveMod.ts index 352367d456..f0f2fb5924 100644 --- a/src/pageEditor/hooks/useSaveMod.ts +++ b/src/pageEditor/hooks/useSaveMod.ts @@ -20,7 +20,7 @@ import { useDispatch, useSelector } from "react-redux"; import { selectDirtyModMetadata, selectDirtyModOptionsDefinitions, - selectGetDeletedComponentIdsForMod, + selectGetDraftModComponentsForMod, } from "@/pageEditor/store/editor/editorSelectors"; import { useGetEditablePackagesQuery, @@ -28,8 +28,6 @@ import { } from "@/data/service/api"; import notify from "@/utils/notify"; import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; -import modComponentSlice from "@/store/modComponents/modComponentSlice"; -import useUpsertModComponentFormState from "@/pageEditor/hooks/useUpsertModComponentFormState"; import { type RegistryId } from "@/types/registryTypes"; import { useAllModDefinitions } from "@/modDefinitions/modDefinitionHooks"; import { ensureModComponentFormStatePermissionsFromUserGesture } from "@/pageEditor/editorPermissionsHelpers"; @@ -37,14 +35,18 @@ import reportEvent from "@/telemetry/reportEvent"; import { Events } from "@/telemetry/events"; import type { EditablePackageMetadata } from "@/types/contract"; import type { ModDefinition } from "@/types/modDefinitionTypes"; -import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod"; -import useBuildAndValidateMod from "@/pageEditor/hooks/useBuildAndValidateMod"; +import useBuildAndValidateMod, { + DataIntegrityError, +} from "@/pageEditor/hooks/useBuildAndValidateMod"; import { reloadModsEveryTab } from "@/contentScript/messenger/api"; import { assertNotNullish } from "@/utils/nullishUtils"; import { isInnerDefinitionRegistryId } from "@/types/helpers"; -import { mapModDefinitionUpsertResponseToModMetadata } from "@/pageEditor/utils"; - -const { actions: modComponentActions } = modComponentSlice; +import { + isModComponentFormState, + mapModDefinitionUpsertResponseToModDefinition, +} from "@/pageEditor/utils"; +import updateReduxForSavedModDefinition from "@/pageEditor/hooks/updateReduxForSavedModDefinition"; +import { isSpecificError } from "@/errors/errorHelpers"; /** @internal */ export function isModEditable( @@ -57,9 +59,6 @@ export function isModEditable( return modId != null && editablePackages.some((x) => x.name === modId); } -const EMPTY_MOD_DEFINITIONS: ModDefinition[] = []; -const EMPTY_EDITABLE_PACKAGES: EditablePackageMetadata[] = []; - /** * Returns a callback to save a mod by id, accounting for changed components, * activation options definitions, and mod metadata. Also validates the saved @@ -67,149 +66,114 @@ const EMPTY_EDITABLE_PACKAGES: EditablePackageMetadata[] = []; */ function useSaveMod(): (modId: RegistryId) => Promise { const dispatch = useDispatch(); - const upsertModComponentFormState = useUpsertModComponentFormState(); const { - data: modDefinitions = EMPTY_MOD_DEFINITIONS, + data: modDefinitions, isLoading: isModDefinitionsLoading, error: modDefinitionsError, } = useAllModDefinitions(); - const { - data: editablePackages = EMPTY_EDITABLE_PACKAGES, - isLoading: isEditablePackagesLoading, - } = useGetEditablePackagesQuery(); - const [updateMod] = useUpdateModDefinitionMutation(); - const getCleanComponentsAndDirtyFormStatesForMod = useSelector( - selectGetCleanComponentsAndDirtyFormStatesForMod, + const { data: editablePackages, isLoading: isEditablePackagesLoading } = + useGetEditablePackagesQuery(); + const [updateModDefinitionOnServer] = useUpdateModDefinitionMutation(); + const getDraftModComponentsForMod = useSelector( + selectGetDraftModComponentsForMod, ); - const getDeletedComponentIdsForMod = useSelector( - selectGetDeletedComponentIdsForMod, - ); - const allDirtyModOptionsDefinitions = useSelector( + const dirtyModOptionsDefinitionsMap = useSelector( selectDirtyModOptionsDefinitions, ); - const allDirtyModMetadatas = useSelector(selectDirtyModMetadata); + const dirtyModMetadataMap = useSelector(selectDirtyModMetadata); + const { buildAndValidateMod } = useBuildAndValidateMod(); - const save = useCallback( + const saveMod = useCallback( async (modId: RegistryId): Promise => { if (isInnerDefinitionRegistryId(modId)) { dispatch(editorActions.showCreateModModal({ keepLocalCopy: false })); return false; } - const modDefinition = modDefinitions.find( - (mod) => mod.metadata.id === modId, + assertNotNullish( + modDefinitions, + "saveMod called without modDefinitions loaded", ); - if (modDefinition == null) { + assertNotNullish( + editablePackages, + "saveMod called without editablePackages loaded", + ); + + const sourceModDefinition = modDefinitions.find( + (x) => x.metadata.id === modId, + ); + if (sourceModDefinition == null) { notify.error({ message: - "You no longer have edit permissions for the mod. Please reload the Page Editor.", + "You no longer have access to this mod package. Please reload the Page Editor.", }); return false; } - if (!isModEditable(editablePackages, modDefinition)) { + if (!isModEditable(editablePackages, sourceModDefinition)) { dispatch(editorActions.showSaveAsNewModModal()); return false; } - const { cleanModComponents, dirtyModComponentFormStates } = - getCleanComponentsAndDirtyFormStatesForMod(modId); + const draftModComponents = getDraftModComponentsForMod(modId); // XXX: this might need to come before the confirmation modal in order to avoid timout if the user takes too // long to confirm? // Check permissions as early as possible void ensureModComponentFormStatePermissionsFromUserGesture( - dirtyModComponentFormStates, + draftModComponents.filter((x) => isModComponentFormState(x)), ); - // Dirty options/metadata or null if there are no staged changes. - // eslint-disable-next-line security/detect-object-injection -- mod IDs are sanitized in the form validation - const dirtyModOptionsDefinition = allDirtyModOptionsDefinitions[modId]; - // eslint-disable-next-line security/detect-object-injection -- mod IDs are sanitized in the form validation - const dirtyModMetadata = allDirtyModMetadatas[modId]; - - const newMod = await buildAndValidateMod({ - sourceMod: modDefinition, - cleanModComponents, - dirtyModComponentFormStates, - dirtyModOptionsDefinition, - dirtyModMetadata, + const unsavedModDefinition = await buildAndValidateMod({ + sourceModDefinition, + draftModComponents, + // eslint-disable-next-line security/detect-object-injection -- mod IDs are sanitized in the form validation + dirtyModOptionsDefinition: dirtyModOptionsDefinitionsMap[modId], + // eslint-disable-next-line security/detect-object-injection -- mod IDs are sanitized in the form validation + dirtyModMetadata: dirtyModMetadataMap[modId], }); const packageId = editablePackages.find( // Bricks endpoint uses "name" instead of id - (x) => x.name === newMod.metadata.id, + (x) => x.name === modId, )?.id; - assertNotNullish(packageId, "Package ID is required to upsert a mod"); - - const upsertResponse = await updateMod({ + assertNotNullish( packageId, - modDefinition: newMod, - }).unwrap(); - - const newModMetadata = mapModDefinitionUpsertResponseToModMetadata( - newMod, - upsertResponse, - ); - - // Don't push to cloud since we're saving it with the mod - await Promise.all( - dirtyModComponentFormStates.map(async (modComponentFormState) => - upsertModComponentFormState({ - modComponentFormState, - options: { - // Permissions were already checked earlier in the save function here - checkPermissions: false, - // Notified and reactivated once in safeSave below - notifySuccess: false, - reactivateEveryTab: false, - }, - modId: newModMetadata.id, - }), - ), + "Package ID is required to upsert a mod definition", ); - // Update the mod metadata on mod components in the options slice - dispatch(modComponentActions.updateModMetadata(newModMetadata)); - - dispatch( - editorActions.updateModMetadataOnModComponentFormStates(newModMetadata), - ); - - // Remove any deleted mod component form states from the mod components slice - for (const modComponentId of getDeletedComponentIdsForMod(modId)) { - dispatch(modComponentActions.removeModComponent({ modComponentId })); - } + const upsertResponse = await updateModDefinitionOnServer({ + packageId, + modDefinition: unsavedModDefinition, + }).unwrap(); - // Clear the dirty states - dispatch( - editorActions.clearMetadataAndOptionsChangesForMod(newModMetadata.id), - ); - dispatch( - editorActions.clearDeletedModComponentFormStatesForMod( - newModMetadata.id, + await updateReduxForSavedModDefinition({ + modIdToReplace: modId, + modDefinition: mapModDefinitionUpsertResponseToModDefinition( + unsavedModDefinition, + upsertResponse, ), - ); + draftModComponents, + isReactivate: true, + })(dispatch); reportEvent(Events.PAGE_EDITOR_MOD_UPDATE, { - modId: newMod.metadata.id, + modId, }); return true; }, [ - allDirtyModMetadatas, - allDirtyModOptionsDefinitions, + dirtyModMetadataMap, + dirtyModOptionsDefinitionsMap, + getDraftModComponentsForMod, buildAndValidateMod, dispatch, editablePackages, - getCleanComponentsAndDirtyFormStatesForMod, - getDeletedComponentIdsForMod, modDefinitions, - updateMod, - upsertModComponentFormState, + updateModDefinitionOnServer, ], ); @@ -217,7 +181,7 @@ function useSaveMod(): (modId: RegistryId) => Promise { async (modId: RegistryId) => { if (modDefinitionsError) { notify.error({ - message: "Error fetching mod definitions", + message: "Error loading mod definitions", error: modDefinitionsError, }); @@ -225,27 +189,37 @@ function useSaveMod(): (modId: RegistryId) => Promise { } if (isModDefinitionsLoading || isEditablePackagesLoading) { + notify.error({ + message: "Mod definitions not loaded yet. Try again.", + }); + return; } try { - const success = await save(modId); + const success = await saveMod(modId); if (success) { notify.success("Saved mod"); reloadModsEveryTab(); } } catch (error) { + if (isSpecificError(error, DataIntegrityError)) { + dispatch(editorActions.showSaveDataIntegrityErrorModal()); + return; + } + notify.error({ - message: "Failed saving mod", + message: "Error saving mod", error, }); } }, [ + dispatch, isEditablePackagesLoading, isModDefinitionsLoading, modDefinitionsError, - save, + saveMod, ], ); } diff --git a/src/pageEditor/hooks/useUpsertModComponentFormState.test.ts b/src/pageEditor/hooks/useUpsertModComponentFormState.test.ts deleted file mode 100644 index 9e6fc46e5a..0000000000 --- a/src/pageEditor/hooks/useUpsertModComponentFormState.test.ts +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright (C) 2024 PixieBrix, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -import { renderHook } from "@/pageEditor/testHelpers"; -import useUpsertModComponentFormState from "@/pageEditor/hooks/useUpsertModComponentFormState"; -import { formStateFactory } from "@/testUtils/factories/pageEditorFactories"; -import MockAdapter from "axios-mock-adapter"; -import axios from "axios"; -import { type ModComponentState } from "@/store/modComponents/modComponentTypes"; -import { API_PATHS } from "@/data/service/urlPaths"; -import { selectModInstances } from "@/store/modComponents/modInstanceSelectors"; - -const axiosMock = new MockAdapter(axios); -const defaultOptions = { - checkPermissions: false, - notifySuccess: true, - reactivateEveryTab: false, -}; - -describe("useUpsertModComponentFormState", () => { - let expectedUpdateDate: Date; - - beforeAll(() => { - expectedUpdateDate = new Date("2023-01-01"); - jest.useFakeTimers().setSystemTime(expectedUpdateDate); - }); - - beforeEach(() => { - axiosMock.onGet(API_PATHS.BRICKS).reply(200, []); - }); - - afterEach(() => { - axiosMock.reset(); - }); - - it("should save form state to redux", async () => { - const modComponentFormState = formStateFactory(); - - const { result, getReduxStore, waitForEffect } = renderHook(() => - useUpsertModComponentFormState(), - ); - await waitForEffect(); - - const upsertModComponentFormState = result.current; - await upsertModComponentFormState({ - modComponentFormState, - options: defaultOptions, - }); - - const modInstances = selectModInstances( - getReduxStore().getState() as { options: ModComponentState }, - ); - - expect(modInstances).toHaveLength(1); - - expect(modInstances[0]!).toEqual( - expect.objectContaining({ - modComponentIds: [modComponentFormState.uuid], - definition: expect.objectContaining({ - extensionPoints: [ - expect.objectContaining({ - id: modComponentFormState.starterBrick.metadata.id, - }), - ], - }), - updatedAt: expectedUpdateDate.toISOString(), - }), - ); - }); -}); diff --git a/src/pageEditor/hooks/useUpsertModComponentFormState.ts b/src/pageEditor/hooks/useUpsertModComponentFormState.ts deleted file mode 100644 index d11708f5b9..0000000000 --- a/src/pageEditor/hooks/useUpsertModComponentFormState.ts +++ /dev/null @@ -1,191 +0,0 @@ -/* - * Copyright (C) 2024 PixieBrix, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -import { editorSlice } from "@/pageEditor/store/editor/editorSlice"; -import { useDispatch, useSelector } from "react-redux"; -import { useCallback } from "react"; -import notify from "@/utils/notify"; -import { getErrorMessage } from "@/errors/errorHelpers"; -import reportEvent from "@/telemetry/reportEvent"; -import { Events } from "@/telemetry/events"; -import { modComponentWithInnerDefinitions } from "@/pageEditor/starterBricks/base"; -import modComponentSlice from "@/store/modComponents/modComponentSlice"; -import { selectSessionId } from "@/pageEditor/store/session/sessionSelectors"; -import { type ModComponentFormState } from "@/pageEditor/starterBricks/formStateTypes"; -import { isSingleObjectBadRequestError } from "@/errors/networkErrorHelpers"; -import { ensureModComponentFormStatePermissionsFromUserGesture } from "@/pageEditor/editorPermissionsHelpers"; -import { isInnerDefinitionRegistryId } from "@/types/helpers"; -import { type RegistryId } from "@/types/registryTypes"; -import { reloadModsEveryTab } from "@/contentScript/messenger/api"; -import { adapterForComponent } from "@/pageEditor/starterBricks/adapter"; -import { nowTimestamp } from "@/utils/timeUtils"; - -const { saveModComponent } = modComponentSlice.actions; -const { markModComponentFormStateAsClean } = editorSlice.actions; - -function selectErrorMessage(error: unknown): string { - // FIXME: should this logic be in getErrorMessage? - if (isSingleObjectBadRequestError(error)) { - return ( - // FIXME: won't the data on each property be an array? - error.response?.data.config?.toString() ?? - error.response?.statusText ?? - "No response from PixieBrix server" - ); - } - - return getErrorMessage(error); -} - -type SaveOptions = { - /** - * Should the permissions be checked before saving? - */ - checkPermissions: boolean; - /** - * Should the user be notified of the save? - */ - notifySuccess: boolean; - /** - * Should mods be reactivated on all tabs? - */ - reactivateEveryTab: boolean; -}; - -/** - * @returns errorMessage an error message, or null if no error occurred - */ -type SaveCallback = (config: { - modComponentFormState: ModComponentFormState; - options: SaveOptions; - modId?: RegistryId; -}) => Promise; - -function onStepError(error: unknown, step: string): string { - const message = selectErrorMessage(error); - console.warn("Error %s: %s", step, message, { error }); - const errorMessage = `Error ${step}: ${message}`; - notify.error({ message: errorMessage, error }); - - return errorMessage; -} - -/** - * Hook that returns a callback to save a Mod Component Form State in Redux. - */ -function useUpsertModComponentFormState(): SaveCallback { - // XXX: Some users have problems when saving from the Page Editor that seem to indicate the sequence of events doesn't - // occur in the correct order on slower (CPU or network?) machines. Therefore, await all promises. We also have to - // make `reactivate` behave deterministically if we're still having problems (right now it's implemented as a - // fire-and-forget notification). - - const dispatch = useDispatch(); - const sessionId = useSelector(selectSessionId); - - const saveModComponentFormState = useCallback( - async ( - modComponentFormState: ModComponentFormState, - options: SaveOptions, - modId?: RegistryId, - ): Promise => { - if (options.checkPermissions) { - // Good to prompt the creator for permissions if any is missing, but they're not actually required to save - void ensureModComponentFormStatePermissionsFromUserGesture( - modComponentFormState, - ); - } - - const { selectStarterBrickDefinition, selectModComponent } = - adapterForComponent(modComponentFormState); - - reportEvent(Events.PAGE_EDITOR_MOD_COMPONENT_UPDATE, { - sessionId, - type: modComponentFormState.starterBrick.definition.type, - modId, - }); - - try { - let modComponent = selectModComponent(modComponentFormState); - const updateTimestamp = nowTimestamp(); - - // The Page Editor only supports editing inline Starter Brick definitions, not Starter Brick packages - // Therefore, no logic is required here for Starter Brick registry packages. - if ( - isInnerDefinitionRegistryId( - modComponentFormState.starterBrick.metadata.id, - ) - ) { - const { definition } = selectStarterBrickDefinition( - modComponentFormState, - ); - modComponent = modComponentWithInnerDefinitions( - modComponent, - definition, - ); - } - - dispatch( - saveModComponent({ - modComponent: { - ...modComponent, - // Note that it is unfortunately the client's responsibility to make sure the `updateTimestamp` is the - // same in Redux as it is on the server. - updateTimestamp, - }, - }), - ); - - dispatch(markModComponentFormStateAsClean(modComponentFormState.uuid)); - } catch (error) { - return onStepError(error, "saving mod"); - } - - if (options.reactivateEveryTab) { - reloadModsEveryTab(); - } - - if (options.notifySuccess) { - notify.success("Saved mod"); - } - - return null; - }, - [dispatch, sessionId], - ); - - return useCallback( - async ({ modComponentFormState, options, modId }) => { - try { - return await saveModComponentFormState( - modComponentFormState, - options, - modId, - ); - } catch (error) { - console.error("Error saving mod", { error }); - notify.error({ - message: "Error saving mod", - error, - }); - return "Save error"; - } - }, - [saveModComponentFormState], - ); -} - -export default useUpsertModComponentFormState; diff --git a/src/pageEditor/modListingPanel/ModComponentListItem.tsx b/src/pageEditor/modListingPanel/ModComponentListItem.tsx index f7c569a287..f06364192c 100644 --- a/src/pageEditor/modListingPanel/ModComponentListItem.tsx +++ b/src/pageEditor/modListingPanel/ModComponentListItem.tsx @@ -16,10 +16,11 @@ */ import React from "react"; -import { isModComponentBase, type ModComponentSidebarItem } from "./common"; +import { type ModComponentSidebarItem } from "./common"; import DraftModComponentListItem from "./DraftModComponentListItem"; import ActivatedModComponentListItem from "./ActivatedModComponentListItem"; import { type UUID } from "@/types/stringTypes"; +import { isModComponentBase } from "@/pageEditor/utils"; type ModComponentListItemProps = { modComponentSidebarItem: ModComponentSidebarItem; diff --git a/src/pageEditor/modListingPanel/ModSidebarListItems.tsx b/src/pageEditor/modListingPanel/ModSidebarListItems.tsx index 49dfa57dc6..6d8e801d70 100644 --- a/src/pageEditor/modListingPanel/ModSidebarListItems.tsx +++ b/src/pageEditor/modListingPanel/ModSidebarListItems.tsx @@ -18,10 +18,7 @@ import styles from "./ModSidebarListItems.module.scss"; import React, { useMemo, useState } from "react"; import { Accordion, Button, FormControl, ListGroup } from "react-bootstrap"; -import { - getModComponentItemId, - type SidebarItem, -} from "@/pageEditor/modListingPanel/common"; +import { type SidebarItem } from "@/pageEditor/modListingPanel/common"; import ModListItem from "@/pageEditor/modListingPanel/ModListItem"; import arrangeSidebarItems from "@/pageEditor/modListingPanel/arrangeSidebarItems"; import { @@ -36,6 +33,7 @@ import { useSelector } from "react-redux"; import ModComponentListItem from "./ModComponentListItem"; import { useDebounce } from "use-debounce"; import filterSidebarItems from "@/pageEditor/modListingPanel/filterSidebarItems"; +import { getDraftModComponentId } from "@/pageEditor/utils"; const ModSidebarListItems: React.FunctionComponent = () => { const activeModComponentId = useSelector(selectActiveModComponentId); @@ -88,7 +86,7 @@ const ModSidebarListItems: React.FunctionComponent = () => { {modComponents.map((modComponentSidebarItem) => ( . */ -import { - getModComponentItemId, - type SidebarItem, -} from "@/pageEditor/modListingPanel/common"; +import { type SidebarItem } from "@/pageEditor/modListingPanel/common"; import { type RegistryId } from "@/types/registryTypes"; import { type UUID } from "@/types/stringTypes"; +import { getDraftModComponentId } from "@/pageEditor/utils"; type FilterSidebarItemsArgs = { sidebarItems: SidebarItem[]; @@ -54,7 +52,7 @@ export default function filterSidebarItems({ // Don't filter out mod item if any mod component is active, or any mod component label matches the query for (const modComponentItem of sidebarItem.modComponents) { if ( - getModComponentItemId(modComponentItem) === activeModComponentId || + getDraftModComponentId(modComponentItem) === activeModComponentId || caseInsensitiveIncludes(modComponentItem.label, filterText) ) { return true; diff --git a/src/pageEditor/modListingPanel/modals/CreateModModal.tsx b/src/pageEditor/modListingPanel/modals/CreateModModal.tsx index a3726c97bc..948f6bd860 100644 --- a/src/pageEditor/modListingPanel/modals/CreateModModal.tsx +++ b/src/pageEditor/modListingPanel/modals/CreateModModal.tsx @@ -63,6 +63,8 @@ import { assertNotNullish } from "@/utils/nullishUtils"; import useIsMounted from "@/hooks/useIsMounted"; import useCreateModFromUnsavedMod from "@/pageEditor/hooks/useCreateModFromUnsavedMod"; import { type ModComponentFormState } from "@/pageEditor/starterBricks/formStateTypes"; +import { isSpecificError } from "@/errors/errorHelpers"; +import { DataIntegrityError } from "@/pageEditor/hooks/useBuildAndValidateMod"; /** * Hook to get the initial form state for the Create Mod modal. @@ -215,6 +217,11 @@ const CreateModModalBody: React.FC = () => { hideModal(); } catch (error) { + if (isSpecificError(error, DataIntegrityError)) { + dispatch(editorActions.showSaveDataIntegrityErrorModal()); + return; + } + if (isSingleObjectBadRequestError(error) && error.response.data.config) { helpers.setStatus(error.response.data.config); return; diff --git a/src/pageEditor/panes/save/saveHelpers.test.ts b/src/pageEditor/panes/save/saveHelpers.test.ts index 6eeb72d1bd..76cf540c81 100644 --- a/src/pageEditor/panes/save/saveHelpers.test.ts +++ b/src/pageEditor/panes/save/saveHelpers.test.ts @@ -95,9 +95,8 @@ describe("buildNewMod", () => { // Call the function under test const newMod = buildNewMod({ - sourceMod: undefined, - cleanModComponents: [modComponent], - dirtyModComponentFormStates: [], + sourceModDefinition: undefined, + draftModComponents: [modComponent], }); expect(newMod.extensionPoints).toHaveLength(1); @@ -131,9 +130,8 @@ describe("buildNewMod", () => { // Call the function under test const newMod = buildNewMod({ - sourceMod: undefined, - cleanModComponents: [], - dirtyModComponentFormStates: [modComponentFormState], + sourceModDefinition: undefined, + draftModComponents: [modComponentFormState], }); expect(newMod.extensionPoints).toHaveLength(1); @@ -169,9 +167,8 @@ describe("buildNewMod", () => { // Call the function under test const newMod = buildNewMod({ - sourceMod: undefined, - cleanModComponents: modComponents, - dirtyModComponentFormStates: [], + sourceModDefinition: undefined, + draftModComponents: modComponents, }); expect(Object.keys(newMod.definitions!)).toStrictEqual([ @@ -208,9 +205,8 @@ describe("buildNewMod", () => { // Call the function under test const newMod = buildNewMod({ - sourceMod: undefined, - cleanModComponents: modComponents, - dirtyModComponentFormStates: [], + sourceModDefinition: undefined, + draftModComponents: modComponents, }); expect(Object.keys(newMod.definitions!)).toStrictEqual([ @@ -242,9 +238,8 @@ describe("buildNewMod", () => { // Call the function under test const newMod = buildNewMod({ - sourceMod: undefined, - cleanModComponents: modComponents, - dirtyModComponentFormStates: [], + sourceModDefinition: undefined, + draftModComponents: modComponents, }); expect(Object.keys(newMod.definitions!)).toStrictEqual(["extensionPoint"]); @@ -293,48 +288,47 @@ describe("buildNewMod", () => { }), ); - // Collect the dirty form states for any changed mod components - const modComponentFormStates: ModComponentFormState[] = []; + const starterBricks = selectStarterBricks(modDefinition); - if (dirtyModComponentCount > 0) { - const starterBricks = selectStarterBricks(modDefinition); + // Collect the dirty form states for any changed mod components + const dirtyModComponentFormStates: ModComponentFormState[] = []; - for (let i = 0; i < dirtyModComponentCount; i++) { - const starterBrick = starterBricks[i]!; - // Mock this lookup for the adapter call that follows - jest.mocked(lookupStarterBrick).mockResolvedValue(starterBrick); + for (let i = 0; i < dirtyModComponentCount; i++) { + const starterBrick = starterBricks[i]!; + // Mock this lookup for the adapter call that follows + jest.mocked(lookupStarterBrick).mockResolvedValue(starterBrick); - // Mod was activated, so get the mod component from state - const modComponent = state.activatedModComponents[i]!; + // Mod was activated, so get the mod component from state + const modComponent = state.activatedModComponents[i]!; - // Load the adapter for this mod component - const { fromModComponent } = adapter(starterBrick.definition.type); + // Load the adapter for this mod component + const { fromModComponent } = adapter(starterBrick.definition.type); - // Use the adapter to convert to FormState - // eslint-disable-next-line no-await-in-loop -- This is much easier to read than a large Promise.all() block - const modComponentFormState = (await fromModComponent( - modComponent, - )) as ModComponentFormState; + // Use the adapter to convert to FormState + // eslint-disable-next-line no-await-in-loop -- This is much easier to read than a large Promise.all() block + const modComponentFormState = (await fromModComponent( + modComponent, + )) as ModComponentFormState; - // Edit the label - modComponentFormState.label = `New Label ${i}`; + // Edit the label + modComponentFormState.label = `New Label ${i}`; - modComponentFormStates.push(modComponentFormState); - } + dirtyModComponentFormStates.push(modComponentFormState); } - // Call the function under test - const newMod = buildNewMod({ - sourceMod: modDefinition, - // Only pass in the unchanged clean mod components - cleanModComponents: state.activatedModComponents.slice( - dirtyModComponentCount, - ), - dirtyModComponentFormStates: modComponentFormStates, + const actualModDefinition = buildNewMod({ + sourceModDefinition: modDefinition, + draftModComponents: [ + // `buildAndValidate` now preserves mod component order. So order of dirty vs. clean must match the + // construction for expectedModDefinition + ...dirtyModComponentFormStates, + // Only pass in the unchanged clean mod components + ...state.activatedModComponents.slice(dirtyModComponentCount), + ], }); - // Update the source mod with the expected label changes - const updatedMod = produce(modDefinition, (draft) => { + // Directly update the source mod with the expected label changes + const expectedModDefinition = produce(modDefinition, (draft) => { for (const [index, starterBrick] of draft.extensionPoints .slice(0, dirtyModComponentCount) .entries()) { @@ -343,8 +337,8 @@ describe("buildNewMod", () => { }); // Compare results - expect(normalizeModDefinition(newMod)).toStrictEqual( - normalizeModDefinition(updatedMod), + expect(normalizeModDefinition(actualModDefinition)).toStrictEqual( + normalizeModDefinition(expectedModDefinition), ); }, ); diff --git a/src/pageEditor/panes/save/saveHelpers.ts b/src/pageEditor/panes/save/saveHelpers.ts index d6cdcf2cf6..45f1951b80 100644 --- a/src/pageEditor/panes/save/saveHelpers.ts +++ b/src/pageEditor/panes/save/saveHelpers.ts @@ -27,7 +27,7 @@ import { PACKAGE_REGEX, validateRegistryId, } from "@/types/helpers"; -import { compact, sortBy } from "lodash"; +import { compact, uniqBy } from "lodash"; import { produce } from "immer"; import { type ModComponentFormState } from "@/pageEditor/starterBricks/formStateTypes"; import { @@ -58,6 +58,10 @@ import { import { isInnerDefinitionEqual } from "@/starterBricks/starterBrickUtils"; import { adapterForComponent } from "@/pageEditor/starterBricks/adapter"; import { mapModComponentBaseToModComponentDefinition } from "@/store/modComponents/modInstanceUtils"; +import { + getDraftModComponentId, + isModComponentFormState, +} from "@/pageEditor/utils"; /** * Generate a new registry id from an existing registry id by adding/replacing the scope. @@ -96,9 +100,14 @@ function deleteUnusedStarterBrickDefinitions( } export type ModParts = { - sourceMod?: ModDefinition; - cleanModComponents: SerializedModComponent[]; - dirtyModComponentFormStates: ModComponentFormState[]; + /** + * Mod components to save. These can be dirty or clean. + */ + draftModComponents: Array; + /** + * The original mod definition, if it exists. Undefined if this is a new mod. + */ + sourceModDefinition?: ModDefinition; /** * Dirty/new options to save. Undefined if there are no changes. */ @@ -123,6 +132,29 @@ const emptyModDefinition: UnsavedModDefinition = { variables: emptyModVariablesDefinitionFactory(), }; +function mapModComponentFormStateToModComponentBase( + modComponentFormState: ModComponentFormState, +): ModComponentBase { + const { selectModComponent, selectStarterBrickDefinition } = + adapterForComponent(modComponentFormState); + + const unsavedModComponent = selectModComponent(modComponentFormState); + + if (isInnerDefinitionRegistryId(unsavedModComponent.extensionPointId)) { + const starterBrickConfig = selectStarterBrickDefinition( + modComponentFormState, + ); + unsavedModComponent.definitions = { + [unsavedModComponent.extensionPointId]: { + kind: DefinitionKinds.STARTER_BRICK, + definition: starterBrickConfig.definition, + } satisfies StarterBrickDefinitionLike, + }; + } + + return unsavedModComponent; +} + /** * Create a copy of `sourceMod` (if provided) with given mod metadata, mod options, and mod components. * @@ -130,86 +162,63 @@ const emptyModDefinition: UnsavedModDefinition = { * only handles the starter brick if it's an inner definition * * @param sourceMod the original mod definition, or undefined for new mods - * @param cleanModComponents the mod's unchanged, activated mod components - * @param dirtyModComponentFormStates the mod's component form states (i.e., submitted via Formik) - * @param dirtyModOptions the mod's options form state, or nullish if there are no dirty options + * @param draftModComponents the activated mod components/form states to save. Must exclude deleted components + * @param dirtyModOptionsDefinition the mod's option definition form state, or nullish if there are no dirty options * @param dirtyModMetadata the mod's metadata form state, or nullish if there is no dirty mod metadata */ export function buildNewMod({ - sourceMod, - cleanModComponents, - dirtyModComponentFormStates, + sourceModDefinition, + draftModComponents, dirtyModOptionsDefinition, dirtyModMetadata, }: ModParts): UnsavedModDefinition { // If there's no source mod, then we're creating a new one, so we // start with an empty mod definition that will be filled in const unsavedModDefinition: UnsavedModDefinition = - sourceMod ?? emptyModDefinition; + sourceModDefinition ?? emptyModDefinition; return produce(unsavedModDefinition, (draft: UnsavedModDefinition): void => { - if (dirtyModOptionsDefinition) { - draft.options = normalizeModOptionsDefinition(dirtyModOptionsDefinition); + if (draftModComponents.length === 0) { + throw new Error("No mod components to save"); } - if (dirtyModMetadata) { - draft.metadata = dirtyModMetadata; - } - - const versionedItems = [ - ...cleanModComponents, - ...dirtyModComponentFormStates, - ]; - // We need to handle the unlikely edge-case of zero mod components here, hence the null-coalesce - const itemsApiVersion = - versionedItems[0]?.apiVersion ?? unsavedModDefinition.apiVersion; - const badApiVersion = versionedItems.find( - (item) => item.apiVersion !== itemsApiVersion, - )?.apiVersion; - - if (badApiVersion) { + if ( + draftModComponents.some( + (x) => x.apiVersion !== unsavedModDefinition.apiVersion, + ) + ) { throw new Error( - `Mod bricks have inconsistent API Versions (${itemsApiVersion}/${badApiVersion}). All bricks in a mod must have the same API Version.`, + "Runtime API version mismatch between mod definition and mod component definitions. Edit the mod in the the Workshop.", ); } - if (itemsApiVersion !== unsavedModDefinition.apiVersion) { - throw new Error( - `Mod uses API Version ${unsavedModDefinition.apiVersion}, but it's bricks have version ${itemsApiVersion}. Please use the Workshop to edit this mod.`, - ); + if ( + uniqBy( + draftModComponents.map((x) => getDraftModComponentId(x)), + (x) => x, + ).length !== draftModComponents.length + ) { + throw new Error("One or more duplicate mod component ids found"); } - const unsavedModComponents: ModComponentBase[] = - dirtyModComponentFormStates.map((modComponentFormState) => { - const { selectModComponent, selectStarterBrickDefinition } = - adapterForComponent(modComponentFormState); - - const unsavedModComponent = selectModComponent(modComponentFormState); - - if (isInnerDefinitionRegistryId(unsavedModComponent.extensionPointId)) { - const starterBrickConfig = selectStarterBrickDefinition( - modComponentFormState, - ); - unsavedModComponent.definitions = { - [unsavedModComponent.extensionPointId]: { - kind: DefinitionKinds.STARTER_BRICK, - definition: starterBrickConfig.definition, - } satisfies StarterBrickDefinitionLike, - }; - } + if (dirtyModOptionsDefinition) { + draft.options = normalizeModOptionsDefinition(dirtyModOptionsDefinition); + } - return unsavedModComponent; - }); + if (dirtyModMetadata) { + draft.metadata = dirtyModMetadata; + } - const { innerDefinitions, modComponents } = buildModComponents([ - ...cleanModComponents, - ...unsavedModComponents, - ]); + const { innerDefinitions, modComponents: extensionPoints } = + buildModComponents( + draftModComponents.map((draftModComponent) => + isModComponentFormState(draftModComponent) + ? mapModComponentFormStateToModComponentBase(draftModComponent) + : draftModComponent, + ), + ); - // This sorting is mostly for test ergonomics for easier equality assertions when - // things stay in the same order in this array. The clean/dirty mod components - // split/recombination logic causes things to get out of order in the result. - draft.extensionPoints = sortBy(modComponents, (x) => x.id); + draft.extensionPoints = extensionPoints; draft.definitions = innerDefinitions; // Delete any extra starter brick definitions that might have crept in diff --git a/src/pageEditor/starterBricks/base.ts b/src/pageEditor/starterBricks/base.ts index eb5cf4e38e..31fea489a7 100644 --- a/src/pageEditor/starterBricks/base.ts +++ b/src/pageEditor/starterBricks/base.ts @@ -19,9 +19,8 @@ import { DefinitionKinds, INNER_SCOPE, type Metadata, - type RegistryId, } from "@/types/registryTypes"; -import { castArray, cloneDeep, isEmpty } from "lodash"; +import { castArray, isEmpty } from "lodash"; import { assertStarterBrickDefinitionLike, type StarterBrickDefinitionLike, @@ -35,7 +34,6 @@ import type React from "react"; import { createSitePattern, SITES_PATTERN } from "@/permissions/patterns"; import { type Except } from "type-fest"; import { - isInnerDefinitionRegistryId, normalizeSemVerString, uuidv4, validateRegistryId, @@ -49,11 +47,10 @@ import { type ModComponentBase, type ModMetadata, } from "@/types/modComponentTypes"; -import { type SafeString, type UUID } from "@/types/stringTypes"; +import { type UUID } from "@/types/stringTypes"; import { isExpression } from "@/utils/expressionUtils"; import { isNullOrBlank } from "@/utils/stringUtils"; import { deepPickBy, freeze } from "@/utils/objectUtils"; -import { freshIdentifier } from "@/utils/variableUtils"; import { type BaseFormState, type BaseModComponentState, @@ -323,34 +320,6 @@ export function baseSelectStarterBrick( }; } -export function modComponentWithInnerDefinitions( - modComponent: ModComponentBase, - starterBrickDefinition: StarterBrickDefinitionProp, -): ModComponentBase { - if (isInnerDefinitionRegistryId(modComponent.extensionPointId)) { - const starterBrick = freshIdentifier( - DEFAULT_STARTER_BRICK_VAR as SafeString, - Object.keys(modComponent.definitions ?? {}), - ); - - const result = cloneDeep(modComponent); - result.definitions = { - ...result.definitions, - [starterBrick]: { - kind: DefinitionKinds.STARTER_BRICK, - definition: starterBrickDefinition, - }, - }; - - // XXX: we need to fix the type of ModComponentBase.extensionPointId to support variable names - result.extensionPointId = starterBrick as RegistryId; - - return result; - } - - return modComponent; -} - /** * Remove object entries undefined and empty-string values. * diff --git a/src/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod.test.ts b/src/pageEditor/store/editor/editorSelectors.test.ts similarity index 99% rename from src/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod.test.ts rename to src/pageEditor/store/editor/editorSelectors.test.ts index 5acd0213f0..7b512fc458 100644 --- a/src/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod.test.ts +++ b/src/pageEditor/store/editor/editorSelectors.test.ts @@ -26,12 +26,12 @@ import { } from "@/testUtils/factories/modComponentFactories"; import { modComponentToFormState } from "@/pageEditor/starterBricks/adapter"; import { formStateFactory } from "@/testUtils/factories/pageEditorFactories"; -import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod"; import { type InnerDefinitionRef, DefinitionKinds, } from "@/types/registryTypes"; import { starterBrickDefinitionFactory } from "@/testUtils/factories/modDefinitionFactories"; +import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/editorSelectors"; let starterBrickCount = 0; function newStarterBrickId(): InnerDefinitionRef { diff --git a/src/pageEditor/store/editor/editorSelectors.ts b/src/pageEditor/store/editor/editorSelectors.ts index 26061c4723..6b98407c20 100644 --- a/src/pageEditor/store/editor/editorSelectors.ts +++ b/src/pageEditor/store/editor/editorSelectors.ts @@ -22,7 +22,7 @@ import { ModalKey, type RootState, } from "@/pageEditor/store/editor/pageEditorTypes"; -import { flatMap, isEmpty, sortBy, uniqBy } from "lodash"; +import { flatMap, isEmpty, memoize, sortBy, uniqBy } from "lodash"; import { DataPanelTabKey } from "@/pageEditor/tabs/editTab/dataPanel/dataPanelTypes"; import { type BrickPipelineUIState, @@ -64,6 +64,14 @@ export const selectModComponentFormStates = ({ }: EditorRootState): EditorState["modComponentFormStates"] => editor.modComponentFormStates; +export const selectGetModComponentFormStatesByModId = createSelector( + selectModComponentFormStates, + (formStates) => + memoize((modId: RegistryId) => + formStates.filter((formState) => formState.modMetadata.id === modId), + ), +); + export const selectActiveModComponentFormState = createSelector( selectActiveModComponentId, selectModComponentFormStates, @@ -128,7 +136,7 @@ export const selectErrorState = ({ editor }: EditorRootState) => ({ editorError: editor.error ? deserializeError(editor.error) : null, }); -export const selectIsModComponentDirtyById = ({ editor }: EditorRootState) => +const selectIsModComponentDirtyById = ({ editor }: EditorRootState) => editor.dirty; /** @internal */ @@ -136,14 +144,6 @@ export const selectDeletedComponentFormStatesByModId = ({ editor, }: EditorRootState) => editor.deletedModComponentFormStatesByModId; -export const selectGetDeletedComponentIdsForMod = - ({ editor }: EditorRootState) => - (modId: RegistryId) => - // eslint-disable-next-line security/detect-object-injection -- RegistryId - (editor.deletedModComponentFormStatesByModId[modId] ?? []).map( - (formState) => formState.uuid, - ); - const selectAllDeletedModComponentIds = ({ editor }: EditorRootState) => new Set( flatMap(editor.deletedModComponentFormStatesByModId).map( @@ -581,3 +581,47 @@ export const selectFirstModComponentFormStateForActiveMod = createSelector( (formState, activeModId) => formState.find((x) => x.modMetadata.id === activeModId), ); + +export const selectGetCleanComponentsAndDirtyFormStatesForMod = createSelector( + selectNotDeletedActivatedModComponents, + selectNotDeletedModComponentFormStates, + selectIsModComponentDirtyById, + (activatedModComponents, formStates, isDirtyByComponentId) => + // Memoize because method constructs a fresh object reference + memoize((modId: RegistryId) => { + const dirtyModComponentFormStates = formStates.filter( + (formState) => + formState.modMetadata.id === modId && + isDirtyByComponentId[formState.uuid], + ); + + const cleanModComponents = activatedModComponents.filter( + (modComponent) => + modComponent.modMetadata.id === modId && + !dirtyModComponentFormStates.some( + (formState) => formState.uuid === modComponent.id, + ), + ); + + return { + cleanModComponents, + dirtyModComponentFormStates, + }; + }), +); + +export const selectGetDraftModComponentsForMod = createSelector( + selectGetCleanComponentsAndDirtyFormStatesForMod, + (getCleanComponentsAndDirtyFormStatesForMod) => + // Memoize because method constructs a fresh object reference + memoize((modId: RegistryId) => { + const { cleanModComponents, dirtyModComponentFormStates } = + getCleanComponentsAndDirtyFormStatesForMod(modId); + + // Return a consistent order so mod component order is stable on save + return sortBy( + [...cleanModComponents, ...dirtyModComponentFormStates], + (x) => x.label, + ); + }), +); diff --git a/src/pageEditor/store/editor/editorSlice.ts b/src/pageEditor/store/editor/editorSlice.ts index aff3b6cbdd..78d7dbe47b 100644 --- a/src/pageEditor/store/editor/editorSlice.ts +++ b/src/pageEditor/store/editor/editorSlice.ts @@ -87,7 +87,7 @@ import { inspectedTab, } from "@/pageEditor/context/connection"; import { assertNotNullish } from "@/utils/nullishUtils"; -import { collectModOptions } from "@/store/modComponents/modComponentUtils"; +import { collectModOptionsArgs } from "@/store/modComponents/modComponentUtils"; /** @internal */ export const initialState: EditorState = { @@ -461,23 +461,14 @@ export const editorSlice = createSlice({ } }, - clearMetadataAndOptionsChangesForMod( - state, - action: PayloadAction, - ) { - const { payload: modId } = action; - delete state.dirtyModMetadataById[modId]; - delete state.dirtyModOptionsById[modId]; - }, - updateModMetadataOnModComponentFormStates( state, - action: PayloadAction, + action: PayloadAction<{ modId: RegistryId; modMetadata: ModMetadata }>, ) { - const modMetadata = action.payload; + const { modId, modMetadata } = action.payload; const modComponentFormStates = state.modComponentFormStates.filter( (modComponentFormState) => - modComponentFormState.modMetadata.id === modMetadata.id, + modComponentFormState.modMetadata.id === modId, ); // Technically this method should also update the deleted form states. But this reducer method is only called // when the mod is being saved, so those deleted form states will be removed anyway. @@ -486,12 +477,21 @@ export const editorSlice = createSlice({ } }, - clearDeletedModComponentFormStatesForMod( - state, - action: PayloadAction, - ) { + /** + * Mark a mod and all of its associated form states as clean. + * @see markModComponentFormStateAsClean + */ + markModAsCleanById(state, action: PayloadAction) { const modId = action.payload; + + for (const modComponentFormState of state.modComponentFormStates) { + modComponentFormState.installed = true; + state.dirty[modComponentFormState.uuid] = false; + } + delete state.deletedModComponentFormStatesByModId[modId]; + delete state.dirtyModMetadataById[modId]; + delete state.dirtyModOptionsById[modId]; }, /** @@ -555,7 +555,7 @@ export const editorSlice = createSlice({ // NOTE: we don't need to have logic here for optionsDefinition and variablesDefinition because those // are stored/owned at the mod-level in the Page Editor if (existingModFormStates.length > 0) { - modComponentFormState.optionsArgs = collectModOptions( + modComponentFormState.optionsArgs = collectModOptionsArgs( existingModFormStates, ); } @@ -608,7 +608,8 @@ export const editorSlice = createSlice({ }, /** - * Marks the form state as clean, i.e., it has no unsaved changes. + * Marks the form state as clean and corresponding to an activated mod component, i.e., it has no unsaved changes. + * @see markModAsCleanById */ markModComponentFormStateAsClean(state, action: PayloadAction) { const modComponentId = action.payload; diff --git a/src/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod.ts b/src/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod.ts deleted file mode 100644 index 7487369914..0000000000 --- a/src/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod.ts +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright (C) 2024 PixieBrix, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -import { createSelector } from "@reduxjs/toolkit"; -import type { RegistryId } from "@/types/registryTypes"; -import { - selectIsModComponentDirtyById, - selectNotDeletedModComponentFormStates, - selectNotDeletedActivatedModComponents, -} from "@/pageEditor/store/editor/editorSelectors"; -import { memoize } from "lodash"; - -export const selectGetCleanComponentsAndDirtyFormStatesForMod = createSelector( - selectNotDeletedActivatedModComponents, - selectNotDeletedModComponentFormStates, - selectIsModComponentDirtyById, - (activatedModComponents, formStates, isDirtyByComponentId) => - // Memoize because method constructs a fresh object reference - memoize((modId: RegistryId) => { - const dirtyModComponentFormStates = formStates.filter( - (formState) => - formState.modMetadata.id === modId && - isDirtyByComponentId[formState.uuid], - ); - - const cleanModComponents = activatedModComponents.filter( - (modComponent) => - modComponent.modMetadata.id === modId && - !dirtyModComponentFormStates.some( - (formState) => formState.uuid === modComponent.id, - ), - ); - - return { - cleanModComponents, - dirtyModComponentFormStates, - }; - }), -); - -export const selectGetDraftModComponentIdsForMod = createSelector( - selectGetCleanComponentsAndDirtyFormStatesForMod, - (getCleanComponentsAndDirtyFormStatesForMod) => - // Memoize because method constructs a fresh object reference - memoize((modId: RegistryId) => { - const { cleanModComponents, dirtyModComponentFormStates } = - getCleanComponentsAndDirtyFormStatesForMod(modId); - return [ - ...cleanModComponents.map((modComponent) => modComponent.id), - ...dirtyModComponentFormStates.map((formState) => formState.uuid), - ]; - }), -); diff --git a/src/pageEditor/tabs/modOptionsValues/ModOptionsValuesEditor.tsx b/src/pageEditor/tabs/modOptionsValues/ModOptionsValuesEditor.tsx index 3b09b697e5..dbef3a7721 100644 --- a/src/pageEditor/tabs/modOptionsValues/ModOptionsValuesEditor.tsx +++ b/src/pageEditor/tabs/modOptionsValues/ModOptionsValuesEditor.tsx @@ -21,6 +21,7 @@ import { selectActiveModId, selectDirtyOptionsDefinitionsForModId, selectDirtyOptionValuesForModId, + selectGetDraftModComponentsForMod, } from "@/pageEditor/store/editor/editorSelectors"; import { useOptionalModDefinition } from "@/modDefinitions/modDefinitionHooks"; import genericOptionsFactory from "@/components/fields/schemaFields/genericOptionsFactory"; @@ -33,7 +34,7 @@ import Loader from "@/components/Loader"; import Alert from "@/components/Alert"; import { getErrorMessage } from "@/errors/errorHelpers"; import ErrorBoundary from "@/components/ErrorBoundary"; -import { collectModOptions } from "@/store/modComponents/modComponentUtils"; +import { collectModOptionsArgs } from "@/store/modComponents/modComponentUtils"; import useAsyncModOptionsValidationSchema from "@/hooks/useAsyncModOptionsValidationSchema"; import Effect from "@/components/Effect"; import { actions } from "@/pageEditor/store/editor/editorSlice"; @@ -42,7 +43,6 @@ import { DEFAULT_RUNTIME_API_VERSION } from "@/runtime/apiVersionOptions"; import ModIntegrationsContext from "@/mods/ModIntegrationsContext"; import { emptyModOptionsDefinitionFactory } from "@/utils/modUtils"; import { uniqBy } from "lodash"; -import { selectGetCleanComponentsAndDirtyFormStatesForMod } from "@/pageEditor/store/editor/selectGetCleanComponentsAndDirtyFormStatesForMod"; import { assertNotNullish } from "@/utils/nullishUtils"; const OPTIONS_FIELD_RUNTIME_CONTEXT: RuntimeContext = { @@ -71,11 +71,10 @@ const ModOptionsValuesContent: React.FC = () => { const modifiedOptionValues = useSelector( selectDirtyOptionValuesForModId(activeModId), ); - const getCleanComponentsAndDirtyFormStatesForMod = useSelector( - selectGetCleanComponentsAndDirtyFormStatesForMod, + const getDraftModComponentsForMod = useSelector( + selectGetDraftModComponentsForMod, ); - const { cleanModComponents, dirtyModComponentFormStates } = - getCleanComponentsAndDirtyFormStatesForMod(activeModId); + const draftModComponents = getDraftModComponentsForMod(activeModId); const optionsDefinition = useMemo(() => { if (dirtyModOptions) { @@ -102,24 +101,19 @@ const ModOptionsValuesContent: React.FC = () => { ); const initialValues = useMemo( - () => - modifiedOptionValues ?? - collectModOptions([ - ...cleanModComponents, - ...dirtyModComponentFormStates, - ]), - [cleanModComponents, dirtyModComponentFormStates, modifiedOptionValues], + () => modifiedOptionValues ?? collectModOptionsArgs(draftModComponents), + [draftModComponents, modifiedOptionValues], ); const integrationDependencies = useMemo( () => uniqBy( - [...cleanModComponents, ...dirtyModComponentFormStates].flatMap( + draftModComponents.flatMap( ({ integrationDependencies }) => integrationDependencies ?? [], ), ({ integrationId }) => integrationId, ), - [cleanModComponents, dirtyModComponentFormStates], + [draftModComponents], ); const updateRedux = useCallback( diff --git a/src/pageEditor/utils.ts b/src/pageEditor/utils.ts index 66371a5991..71cbc41785 100644 --- a/src/pageEditor/utils.ts +++ b/src/pageEditor/utils.ts @@ -25,10 +25,9 @@ import { isPipelineElement, } from "@/pageEditor/documentBuilder/documentBuilderTypes"; import ForEachElement from "@/bricks/transformers/controlFlow/ForEachElement"; -import { castArray, pick, pickBy } from "lodash"; +import { castArray, pickBy } from "lodash"; import { type AnalysisAnnotation } from "@/analysis/analysisTypes"; import { PIPELINE_BRICKS_FIELD_NAME } from "./consts"; -import { type ModMetadata } from "@/types/modComponentTypes"; import { type Brick } from "@/types/brickTypes"; import { sortedFields } from "@/components/fields/schemaFields/schemaFieldUtils"; import { castTextLiteralOrThrow } from "@/utils/expressionUtils"; @@ -38,16 +37,25 @@ import { CustomFormRenderer } from "@/bricks/renderers/customForm"; import MapValues from "@/bricks/transformers/controlFlow/MapValues"; import AddDynamicTextSnippet from "@/bricks/effects/AddDynamicTextSnippet"; import { type PackageUpsertResponse } from "@/types/contract"; -import { type UnsavedModDefinition } from "@/types/modDefinitionTypes"; +import { + type ModDefinition, + type UnsavedModDefinition, +} from "@/types/modDefinitionTypes"; +import type { ModComponentBase } from "@/types/modComponentTypes"; +import type { UUID } from "@/types/stringTypes"; +import type { ModComponentFormState } from "@/pageEditor/starterBricks/formStateTypes"; -export function mapModDefinitionUpsertResponseToModMetadata( +export function mapModDefinitionUpsertResponseToModDefinition( unsavedModDefinition: UnsavedModDefinition, response: PackageUpsertResponse, -): ModMetadata { +): ModDefinition { return { - ...unsavedModDefinition.metadata, - sharing: pick(response, ["public", "organizations"]), - ...pick(response, ["updated_at"]), + ...unsavedModDefinition, + sharing: { + public: response.public, + organizations: response.organizations, + }, + updated_at: response.updated_at, }; } @@ -245,3 +253,21 @@ export function selectPageEditorDimensions() { window.innerWidth > window.innerHeight ? "landscape" : "portrait", }; } + +export function isModComponentBase( + value: ModComponentBase | ModComponentFormState, +): value is ModComponentBase { + return "extensionPointId" in value; +} + +export function isModComponentFormState( + value: ModComponentBase | ModComponentFormState, +): value is ModComponentFormState { + return !isModComponentBase(value); +} + +export function getDraftModComponentId( + item: ModComponentBase | ModComponentFormState, +): UUID { + return isModComponentBase(item) ? item.id : item.uuid; +} diff --git a/src/sidebar/DefaultContent.test.tsx b/src/sidebar/DefaultContent.test.tsx index 3c6b372e13..189fe7f47f 100644 --- a/src/sidebar/DefaultContent.test.tsx +++ b/src/sidebar/DefaultContent.test.tsx @@ -19,10 +19,8 @@ import React from "react"; import { render, screen } from "@/sidebar/testHelpers"; import DefaultPanel from "./DefaultPanel"; import modComponentSlice from "@/store/modComponents/modComponentSlice"; -import { type ActivatedModComponent } from "@/types/modComponentTypes"; -import { modComponentFactory } from "@/testUtils/factories/modComponentFactories"; +import { activatedModComponentFactory } from "@/testUtils/factories/modComponentFactories"; import { appApiMock } from "@/testUtils/appApiMock"; -import { timestampFactory } from "@/testUtils/factories/stringFactories"; import { API_PATHS } from "@/data/service/urlPaths"; describe("renders DefaultPanel", () => { @@ -40,12 +38,9 @@ describe("renders DefaultPanel", () => { render(, { setupRedux(dispatch) { dispatch( - modComponentSlice.actions.saveModComponent({ - modComponent: { - ...(modComponentFactory() as ActivatedModComponent), - updateTimestamp: timestampFactory(), - }, - }), + modComponentSlice.actions.UNSAFE_setModComponents([ + activatedModComponentFactory(), + ]), ); }, }); diff --git a/src/store/modComponents/modComponentSlice.ts b/src/store/modComponents/modComponentSlice.ts index e9a790c48d..e51b99c01c 100644 --- a/src/store/modComponents/modComponentSlice.ts +++ b/src/store/modComponents/modComponentSlice.ts @@ -19,17 +19,11 @@ import { createSlice, type PayloadAction } from "@reduxjs/toolkit"; import { type Deployment } from "@/types/contract"; import reportEvent from "@/telemetry/reportEvent"; import { Events } from "@/telemetry/events"; -import { selectEventData } from "@/telemetry/deployments"; import { contextMenus } from "@/background/messenger/api"; -import { cloneDeep, partition } from "lodash"; -import { type Except } from "type-fest"; +import { cloneDeep, isEmpty, remove } from "lodash"; import { assertModComponentNotHydrated } from "@/runtime/runtimeUtils"; import { revertAll } from "@/store/commonActions"; -import { - type ActivatedModComponent, - type ModComponentBase, -} from "@/types/modComponentTypes"; -import { type Timestamp, type UUID } from "@/types/stringTypes"; +import { type ActivatedModComponent } from "@/types/modComponentTypes"; import { type ModDefinition } from "@/types/modDefinitionTypes"; import { type RegistryId } from "@/types/registryTypes"; import { type OptionsArgs } from "@/types/runtimeTypes"; @@ -37,12 +31,19 @@ import { type IntegrationDependency } from "@/integrations/integrationTypes"; import { initialState } from "@/store/modComponents/modComponentSliceInitialState"; import { mapModComponentDefinitionToActivatedModComponent } from "@/activation/mapModComponentDefinitionToActivatedModComponent"; import { isInnerDefinitionRegistryId } from "@/types/helpers"; +import type { UUID } from "@/types/stringTypes"; +import { assertNotNullish } from "@/utils/nullishUtils"; type ActivateModPayload = { /** * The mod definition to activate. */ modDefinition: ModDefinition; + /** + * If provided, use the provided the mod component ids instead of generating ids. For use in the Page Editor to + * maintain the same ids across saves. + */ + modComponentIds?: UUID[]; /** * Mod integration dependencies with configs filled in */ @@ -89,6 +90,7 @@ const modComponentSlice = createSlice({ { payload: { modDefinition, + modComponentIds, configuredDependencies, optionsArgs, deployment, @@ -103,21 +105,49 @@ const modComponentSlice = createSlice({ ); } - for (const modComponentDefinition of modDefinition.extensionPoints) { - // May be null from bad Workshop edit? - if (modComponentDefinition.id == null) { - throw new Error("modComponentDefinition.id is required"); - } + if ( + state.activatedModComponents.some( + (x) => x.modMetadata.id === modDefinition.metadata.id, + ) + ) { + throw new Error( + "Mod is already activated. Dispatch removeModById first.", + ); + } + + // Defensive checks against malformed information from the server + // Since 1.4.6 we're tracking the sharing information of mods + assertNotNullish( + modDefinition.sharing, + "modDefinition.sharing is required", + ); + assertNotNullish( + modDefinition.updated_at, + "modDefinition.updated_at is required", + ); - if (modDefinition.updated_at == null) { - // Since 1.4.8 we're tracking the updated_at timestamp of mods - throw new Error("updated_at is required"); - } + if (isEmpty(modDefinition.extensionPoints)) { + throw new Error("modDefinition has no components"); + } - if (modDefinition.sharing == null) { - // Since 1.4.6 we're tracking the sharing information of mods - throw new Error("sharing is required"); - } + if ( + modComponentIds != null && + modComponentIds.length !== modDefinition.extensionPoints.length + ) { + throw new Error( + "Mismatch between modDefinition component and modComponentIds count", + ); + } + + for (const [ + index, + modComponentDefinition, + ] of modDefinition.extensionPoints.entries()) { + // May be null from bad Workshop edit? + assertNotNullish( + modComponentDefinition.id, + "modComponentDefinition.id is required", + ); const activatedModComponent: ActivatedModComponent = mapModComponentDefinitionToActivatedModComponent({ @@ -128,12 +158,12 @@ const modComponentSlice = createSlice({ integrationDependencies: configuredDependencies ?? [], }); - assertModComponentNotHydrated(activatedModComponent); + // Force the mod component id as necessary + activatedModComponent.id = + // eslint-disable-next-line security/detect-object-injection -- number + modComponentIds?.[index] ?? activatedModComponent.id; - reportEvent( - Events.STARTER_BRICK_ACTIVATE, - selectEventData(activatedModComponent), - ); + assertModComponentNotHydrated(activatedModComponent); state.activatedModComponents.push(activatedModComponent); @@ -151,118 +181,10 @@ const modComponentSlice = createSlice({ }, /** - * Prefer using `useUpsertModComponentFormState` over calling this action directly. - * - * @see useUpsertModComponentFormState - */ - saveModComponent( - state, - { - payload, - }: PayloadAction<{ - modComponent: (ModComponentBase | ActivatedModComponent) & { - updateTimestamp: Timestamp; - }; - }>, - ) { - const { - modComponent: { - id, - apiVersion, - extensionPointId, - config, - definitions, - label, - optionsArgs, - integrationDependencies, - updateTimestamp, - modMetadata, - }, - } = payload; - - // Support both extensionId and id to keep the API consistent with the shape of the stored extension - if (id == null) { - throw new Error("id or extensionId is required"); - } - - if (extensionPointId == null) { - throw new Error("extensionPointId is required"); - } - - const modComponent: Except< - ActivatedModComponent, - "_serializedModComponentBrand" - > = { - id, - apiVersion, - extensionPointId, - modMetadata, - deploymentMetadata: undefined, - label, - definitions, - optionsArgs, - integrationDependencies, - config, - // We are unfortunately not rehydrating the createTimestamp properly from the server, so in most cases the - // createTimestamp saved in Redux won't match the timestamp on the server. This is OK for now because - // we don't use the exact value of createTimestamp for the time being. - // See https://github.com/pixiebrix/pixiebrix-extension/pull/7229 for more context - createTimestamp: updateTimestamp, - updateTimestamp, - active: true, - }; - - assertModComponentNotHydrated(modComponent); - - const index = state.activatedModComponents.findIndex((x) => x.id === id); - - if (index >= 0) { - // eslint-disable-next-line security/detect-object-injection -- array index from findIndex - state.activatedModComponents[index] = modComponent; - } else { - state.activatedModComponents.push(modComponent); - } - }, - - /** - * Update the mod metadata of all mod components associated with the given mod id. - */ - updateModMetadata( - state, - action: PayloadAction, - ) { - const metadata = action.payload; - for (const modComponent of state.activatedModComponents) { - if (modComponent.modMetadata.id === metadata?.id) { - modComponent.modMetadata = metadata; - } - } - }, - - /** - * Deactivate mod components associated with the given mod id + * Deactivate mod components associated with the given mod id, if any. Safe to call even if the mod is not activated. */ removeModById(state, { payload: modId }: PayloadAction) { - const [, modComponents] = partition( - state.activatedModComponents, - (x) => x.modMetadata.id === modId, - ); - - state.activatedModComponents = modComponents; - }, - - /** - * Deactivate a single mod component by id. Prefer removeModById - * @see removeModById - */ - removeModComponent( - state, - { payload: { modComponentId } }: PayloadAction<{ modComponentId: UUID }>, - ) { - // NOTE: removeModComponent doesn't delete the mod component/definition on the server. - state.activatedModComponents = state.activatedModComponents.filter( - (x) => x.id !== modComponentId, - ); + remove(state.activatedModComponents, (x) => x.modMetadata.id === modId); }, }, extraReducers(builder) { diff --git a/src/store/modComponents/modComponentUtils.test.ts b/src/store/modComponents/modComponentUtils.test.ts index 71f750ad8c..1278d72fe9 100644 --- a/src/store/modComponents/modComponentUtils.test.ts +++ b/src/store/modComponents/modComponentUtils.test.ts @@ -17,7 +17,7 @@ import { collectConfiguredIntegrationDependencies, - collectModOptions, + collectModOptionsArgs, findMaxIntegrationDependencyApiVersion, selectModComponentIntegrations, } from "@/store/modComponents/modComponentUtils"; @@ -34,13 +34,17 @@ import type { ModComponentBase } from "@/types/modComponentTypes"; describe("collectModOptions", () => { it("returns first option", () => { - expect(collectModOptions([{ optionsArgs: { foo: 42 } }])).toStrictEqual({ - foo: 42, - }); + expect(collectModOptionsArgs([{ optionsArgs: { foo: 42 } }])).toStrictEqual( + { + foo: 42, + }, + ); }); it("return blank object if not set", () => { - expect(collectModOptions([{ optionsArgs: undefined }])).toStrictEqual({}); + expect(collectModOptionsArgs([{ optionsArgs: undefined }])).toStrictEqual( + {}, + ); }); }); diff --git a/src/store/modComponents/modComponentUtils.ts b/src/store/modComponents/modComponentUtils.ts index 987a42a3c4..1c6771e243 100644 --- a/src/store/modComponents/modComponentUtils.ts +++ b/src/store/modComponents/modComponentUtils.ts @@ -33,7 +33,7 @@ import type { Schema } from "@/types/schemaTypes"; * Infer options from existing mod-component-like instances for reactivating a mod * @see activateMod */ -export function collectModOptions( +export function collectModOptionsArgs( modComponents: Array>, ): OptionsArgs { // For a given mod, all the components receive the same options during the diff --git a/src/store/modComponents/modInstanceUtils.ts b/src/store/modComponents/modInstanceUtils.ts index df30da9f65..057c2e9ed8 100644 --- a/src/store/modComponents/modInstanceUtils.ts +++ b/src/store/modComponents/modInstanceUtils.ts @@ -25,7 +25,7 @@ import { assertNotNullish } from "@/utils/nullishUtils"; import { uuidv4 } from "@/types/helpers"; import { collectIntegrationDependencies, - collectModOptions, + collectModOptionsArgs, selectModComponentIntegrations, } from "@/store/modComponents/modComponentUtils"; import { createPrivateSharing } from "@/utils/registryUtils"; @@ -172,7 +172,7 @@ export function mapActivatedModComponentsToModInstance( deploymentMetadata: firstComponent.deploymentMetadata ? migrateDeploymentMetadata(firstComponent.deploymentMetadata) : undefined, - optionsArgs: collectModOptions(modComponents), + optionsArgs: collectModOptionsArgs(modComponents), integrationsArgs: collectIntegrationDependencies(modComponents), updatedAt: firstComponent.updateTimestamp, definition: { diff --git a/src/store/sessionChanges/sessionChangesListenerMiddleware.ts b/src/store/sessionChanges/sessionChangesListenerMiddleware.ts index dcbedf61f1..09aab48578 100644 --- a/src/store/sessionChanges/sessionChangesListenerMiddleware.ts +++ b/src/store/sessionChanges/sessionChangesListenerMiddleware.ts @@ -28,7 +28,7 @@ sessionChangesListenerMiddleware.startListening({ editorActions.editModMetadata, editorActions.editModOptionsDefinitions, editorActions.editModOptionsValues, - editorActions.clearMetadataAndOptionsChangesForMod, + editorActions.markModAsCleanById, editorActions.removeModById, // Page Editor mod component actions diff --git a/src/telemetry/events.ts b/src/telemetry/events.ts index ba672f81ed..12c6b15d21 100644 --- a/src/telemetry/events.ts +++ b/src/telemetry/events.ts @@ -142,8 +142,6 @@ export const Events = { START_MOD_ACTIVATE: "StartInstallBlueprint", - STARTER_BRICK_ACTIVATE: "ExtensionActivate", - TOUR_START: "TourStart", TOUR_STEP: "TourStep", TOUR_END: "TourEnd",