diff --git a/end-to-end-tests/tests/pageEditor/moveCopyModComponent.spec.ts b/end-to-end-tests/tests/pageEditor/moveOrCopyModComponent.spec.ts similarity index 100% rename from end-to-end-tests/tests/pageEditor/moveCopyModComponent.spec.ts rename to end-to-end-tests/tests/pageEditor/moveOrCopyModComponent.spec.ts diff --git a/src/analysis/analysisVisitors/varAnalysis/varAnalysis.ts b/src/analysis/analysisVisitors/varAnalysis/varAnalysis.ts index c3fc6db9ed..69870549ef 100644 --- a/src/analysis/analysisVisitors/varAnalysis/varAnalysis.ts +++ b/src/analysis/analysisVisitors/varAnalysis/varAnalysis.ts @@ -57,6 +57,7 @@ import { getOutputReference, isOutputKey } from "@/runtime/runtimeTypes"; import { assertNotNullish } from "@/utils/nullishUtils"; import { BusinessError } from "@/errors/businessErrors"; import { adapterForComponent } from "@/pageEditor/starterBricks/adapter"; +import { isInnerDefinitionRegistryId } from "@/types/helpers"; export const INVALID_VARIABLE_GENERIC_MESSAGE = "Invalid variable name"; @@ -305,11 +306,13 @@ async function setOptionsVars( formState: ModComponentFormState, contextVars: VarMap, ): Promise { - if (formState.modMetadata == null) { + const modId = formState.modMetadata.id; + + // Mod is unsaved, so definition won't be in the registry + if (isInnerDefinitionRegistryId(modId)) { return; } - const modId = formState.modMetadata.id; const mod = await modRegistry.lookup(modId); const optionsSchema = mod?.options?.schema; if (isEmpty(optionsSchema)) { diff --git a/src/components/fields/schemaFields/integrations/integrationDependencyFieldUtils.test.ts b/src/components/fields/schemaFields/integrations/integrationDependencyFieldUtils.test.ts index 19934fb331..fcc3efa7ad 100644 --- a/src/components/fields/schemaFields/integrations/integrationDependencyFieldUtils.test.ts +++ b/src/components/fields/schemaFields/integrations/integrationDependencyFieldUtils.test.ts @@ -31,7 +31,10 @@ import { validateOutputKey } from "@/runtime/runtimeTypes"; import { toExpression } from "@/utils/expressionUtils"; import { normalizeAvailability } from "@/bricks/available"; import { StarterBrickTypes } from "@/types/starterBrickTypes"; -import { emptyModVariablesDefinitionFactory } from "@/utils/modUtils"; +import { + createNewUnsavedModMetadata, + emptyModVariablesDefinitionFactory, +} from "@/utils/modUtils"; describe("selectVariables", () => { test("selects nothing when no services used", () => { @@ -224,7 +227,7 @@ describe("selectVariables", () => { permissions: emptyPermissionsFactory(), optionsArgs: {}, variablesDefinition: emptyModVariablesDefinitionFactory(), - modMetadata: undefined, + modMetadata: createNewUnsavedModMetadata({ modName: "Document Mod" }), modComponent: { brickPipeline: [ { diff --git a/src/pageEditor/hooks/useAddNewModComponent.ts b/src/pageEditor/hooks/useAddNewModComponent.ts index 29cb604fa1..3795c91240 100644 --- a/src/pageEditor/hooks/useAddNewModComponent.ts +++ b/src/pageEditor/hooks/useAddNewModComponent.ts @@ -89,19 +89,20 @@ function useAddNewModComponent(modMetadata?: ModMetadata): AddNewModComponent { setInsertingStarterBrickType(null); } - const url = await getCurrentInspectedURL(); - const metadata = internalStarterBrickMetaFactory(); - const initialFormState = fromNativeElement(url, metadata, element); + const initialFormState = fromNativeElement({ + url: await getCurrentInspectedURL(), + starterBrickMetadata: internalStarterBrickMetaFactory(), + modMetadata: + modMetadata ?? + createNewUnsavedModMetadata({ + modName: generateFreshModName(), + }), + element, + }); initialFormState.modComponent.brickPipeline = getExampleBrickPipeline(starterBrickType); - initialFormState.modMetadata = - modMetadata ?? - createNewUnsavedModMetadata({ - modName: generateFreshModName(), - }); - return initialFormState as ModComponentFormState; }, [ diff --git a/src/pageEditor/hooks/useClearModChanges.ts b/src/pageEditor/hooks/useClearModChanges.ts index 80e1f712b9..8decb667aa 100644 --- a/src/pageEditor/hooks/useClearModChanges.ts +++ b/src/pageEditor/hooks/useClearModChanges.ts @@ -47,7 +47,7 @@ function useClearModChanges(): (modId: RegistryId) => Promise { await Promise.all( modComponentFormStates - .filter((x) => x.modMetadata?.id === modId) + .filter((x) => x.modMetadata.id === modId) .map(async (modComponentFormState) => clearModComponentChanges({ modComponentId: modComponentFormState.uuid, diff --git a/src/pageEditor/hooks/useMigrateStandaloneComponentsToMods.test.ts b/src/pageEditor/hooks/useMigrateStandaloneComponentsToMods.test.ts deleted file mode 100644 index 9bd6afded4..0000000000 --- a/src/pageEditor/hooks/useMigrateStandaloneComponentsToMods.test.ts +++ /dev/null @@ -1,277 +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 useMigrateStandaloneComponentsToMods from "@/pageEditor/hooks/useMigrateStandaloneComponentsToMods"; -import { actions as modComponentActions } from "@/store/modComponents/modComponentSlice"; -import { - activatedModComponentFactory, - modMetadataFactory, -} from "@/testUtils/factories/modComponentFactories"; -import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; -import { formStateFactory } from "@/testUtils/factories/pageEditorFactories"; -import { autoUUIDSequence } from "@/testUtils/factories/stringFactories"; - -describe("useMigrateStandaloneComponentsToMods", () => { - test("given empty form states and components, does nothing", () => { - const { getReduxStore } = renderHook( - () => { - useMigrateStandaloneComponentsToMods(); - }, - { - setupRedux(dispatch, { store }) { - jest.spyOn(store, "dispatch"); - }, - }, - ); - - const { dispatch } = getReduxStore(); - - expect(dispatch).not.toHaveBeenCalled(); - }); - - test("given only activated components, does nothing", () => { - const { getReduxStore } = renderHook( - () => { - useMigrateStandaloneComponentsToMods(); - }, - { - setupRedux(dispatch, { store }) { - jest.spyOn(store, "dispatch"); - dispatch( - modComponentActions.UNSAFE_setModComponents([ - activatedModComponentFactory(), - activatedModComponentFactory(), - activatedModComponentFactory(), - ]), - ); - }, - }, - ); - - const { dispatch } = getReduxStore(); - - expect(dispatch).not.toHaveBeenCalled(); - }); - - test("given unsaved form state, does not migrate", () => { - const { getReduxStore } = renderHook( - () => { - useMigrateStandaloneComponentsToMods(); - }, - { - setupRedux(dispatch, { store }) { - jest.spyOn(store, "dispatch"); - dispatch(editorActions.addModComponentFormState(formStateFactory())); - }, - }, - ); - - const { dispatch } = getReduxStore(); - - expect(dispatch).not.toHaveBeenCalled(); - }); - - test("given activated component with no mod metadata and a form state, logs a warning and does nothing", () => { - const standaloneComponent = activatedModComponentFactory(); - delete standaloneComponent._recipe; - - const consoleWarnSpy = jest.spyOn(console, "warn"); - - const { getReduxStore } = renderHook( - () => { - useMigrateStandaloneComponentsToMods(); - }, - { - setupRedux(dispatch, { store }) { - jest.spyOn(store, "dispatch"); - dispatch( - modComponentActions.UNSAFE_setModComponents([standaloneComponent]), - ); - dispatch( - editorActions.addModComponentFormState( - formStateFactory({ - formStateConfig: { uuid: standaloneComponent.id }, - }), - ), - ); - }, - }, - ); - - expect(consoleWarnSpy).toHaveBeenCalledTimes(1); - expect(consoleWarnSpy).toHaveBeenCalledWith( - "Found activated mod component without mod metadata", - standaloneComponent, - ); - - const { dispatch } = getReduxStore(); - - expect(dispatch).not.toHaveBeenCalled(); - }); - - test("given activated component with mod metadata, syncs the form state", () => { - const modMetadata = modMetadataFactory(); - const standaloneComponent = activatedModComponentFactory({ - _recipe: modMetadata, - }); - const formState = formStateFactory({ - formStateConfig: { uuid: standaloneComponent.id }, - }); - - const { getReduxStore } = renderHook( - () => { - useMigrateStandaloneComponentsToMods(); - }, - { - setupRedux(dispatch, { store }) { - jest.spyOn(store, "dispatch"); - dispatch( - modComponentActions.UNSAFE_setModComponents([standaloneComponent]), - ); - dispatch(editorActions.addModComponentFormState(formState)); - }, - }, - ); - - const { dispatch } = getReduxStore(); - - expect(dispatch).toHaveBeenCalledTimes(1); - expect(dispatch).toHaveBeenCalledWith( - editorActions.syncModComponentFormState({ - ...formState, - modMetadata, - }), - ); - }); - - test("given various components and form states, migrates standalone form states correctly", async () => { - // Mod 1 has components with _recipe but form states have not been migrated - const modMetadata1 = modMetadataFactory(); - // Need to extract the id generation out of factories to prevent overlaps between components and form states - const componentId1a = autoUUIDSequence(); - const modComponent1a = activatedModComponentFactory({ - id: componentId1a, - _recipe: modMetadata1, - }); - const componentId1b = autoUUIDSequence(); - const modComponent1b = activatedModComponentFactory({ - id: componentId1b, - _recipe: modMetadata1, - }); - const formState1b = formStateFactory({ - formStateConfig: { uuid: componentId1b }, - }); - const componentId1c = autoUUIDSequence(); - const modComponent1c = activatedModComponentFactory({ - id: componentId1c, - _recipe: modMetadata1, - }); - const formState1c = formStateFactory({ - formStateConfig: { uuid: componentId1c }, - }); - - const standaloneComponentId = autoUUIDSequence(); - const standaloneComponent = activatedModComponentFactory({ - id: standaloneComponentId, - }); - delete standaloneComponent._recipe; - - // Mod 2 has no form states - const modMetadata2 = modMetadataFactory(); - const componentId2 = autoUUIDSequence(); - const modComponent2 = activatedModComponentFactory({ - id: componentId2, - _recipe: modMetadata2, - }); - - // Mod 3 has two components with form states created with the mod metadata already - const modMetadata3 = modMetadataFactory(); - const componentId3a = autoUUIDSequence(); - const modComponent3a = activatedModComponentFactory({ - id: componentId3a, - _recipe: modMetadata3, - }); - const formState3a = formStateFactory({ - formStateConfig: { - uuid: componentId3a, - modMetadata: modMetadata3, - }, - }); - const componentId3b = autoUUIDSequence(); - const modComponent3b = activatedModComponentFactory({ - id: componentId3b, - _recipe: modMetadata3, - }); - const formState3b = formStateFactory({ - formStateConfig: { - uuid: componentId3b, - modMetadata: modMetadata3, - }, - }); - - // Form state 4 is a newly created form state with no mod component or mod - const componentId4 = autoUUIDSequence(); - const formState4 = formStateFactory({ - formStateConfig: { uuid: componentId4 }, - }); - - const { getReduxStore } = renderHook( - () => { - useMigrateStandaloneComponentsToMods(); - }, - { - setupRedux(dispatch, { store }) { - jest.spyOn(store, "dispatch"); - dispatch( - modComponentActions.UNSAFE_setModComponents([ - modComponent1a, - modComponent1b, - modComponent1c, - standaloneComponent, - modComponent2, - modComponent3a, - modComponent3b, - ]), - ); - dispatch(editorActions.addModComponentFormState(formState1b)); - dispatch(editorActions.addModComponentFormState(formState1c)); - dispatch(editorActions.addModComponentFormState(formState3a)); - dispatch(editorActions.addModComponentFormState(formState3b)); - dispatch(editorActions.addModComponentFormState(formState4)); - }, - }, - ); - - const { dispatch } = getReduxStore(); - - // Only Mod 1's form states should be migrated - expect(dispatch).toHaveBeenCalledTimes(2); - expect(dispatch).toHaveBeenCalledWith( - editorActions.syncModComponentFormState({ - ...formState1b, - modMetadata: modMetadata1, - }), - ); - expect(dispatch).toHaveBeenCalledWith( - editorActions.syncModComponentFormState({ - ...formState1c, - modMetadata: modMetadata1, - }), - ); - }); -}); diff --git a/src/pageEditor/hooks/useMigrateStandaloneComponentsToMods.ts b/src/pageEditor/hooks/useMigrateStandaloneComponentsToMods.ts deleted file mode 100644 index d712e9d301..0000000000 --- a/src/pageEditor/hooks/useMigrateStandaloneComponentsToMods.ts +++ /dev/null @@ -1,64 +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 { useDispatch, useSelector } from "react-redux"; -import { selectModComponentFormStates } from "@/pageEditor/store/editor/editorSelectors"; -import { selectActivatedModComponents } from "@/store/modComponents/modComponentSelectors"; -import { useEffect } from "react"; -import { actions } from "@/pageEditor/store/editor/editorSlice"; - -/** - * Note: This must be run underneath the PersistGate component in the React component tree - */ -export default function useMigrateStandaloneComponentsToMods() { - const dispatch = useDispatch(); - const formStates = useSelector(selectModComponentFormStates); - const activatedModComponents = useSelector(selectActivatedModComponents); - - useEffect(() => { - const standaloneComponentFormStates = formStates.filter( - (formState) => formState.modMetadata == null, - ); - - for (const formState of standaloneComponentFormStates) { - const activatedModComponent = activatedModComponents.find( - ({ id }) => id === formState.uuid, - ); - - if (activatedModComponent == null) { - // We shouldn't touch "unsaved" form states that do not have a corresponding activated mod component - return; - } - - if (activatedModComponent._recipe == null) { - console.warn( - "Found activated mod component without mod metadata", - activatedModComponent, - ); - } else { - dispatch( - // Spread the previous form state here, original is not mutable, so we can't set the modMetadata directly - actions.syncModComponentFormState({ - ...formState, - modMetadata: activatedModComponent._recipe, - }), - ); - } - } - // eslint-disable-next-line -- Only need to run this migration once - }, []); -} diff --git a/src/pageEditor/hooks/useRemoveModComponentFromStorage.tsx b/src/pageEditor/hooks/useRemoveModComponentFromStorage.tsx index 375c5c4d8b..27f4dd2a53 100644 --- a/src/pageEditor/hooks/useRemoveModComponentFromStorage.tsx +++ b/src/pageEditor/hooks/useRemoveModComponentFromStorage.tsx @@ -44,13 +44,6 @@ export const DELETE_STARTER_BRICK_MODAL_PROPS: ConfirmationModalProps = { submitCaption: "Delete", }; -export const DELETE_STANDALONE_MOD_COMPONENT_MODAL_PROPS: ConfirmationModalProps = - { - title: "Delete mod?", - message: "This action cannot be undone.", - submitCaption: "Delete", - }; - export const DEACTIVATE_MOD_MODAL_PROPS: ConfirmationModalProps = { title: "Deactivate Mod?", message: ( @@ -69,10 +62,8 @@ export const DEACTIVATE_MOD_MODAL_PROPS: ConfirmationModalProps = { /** * Returns a callback that removes a mod component from the Page Editor and Mod Component Storage. * - * For mod components packaged inside a mod and standalone mod components not saved on the cloud, this callback will effectively delete the mod component. - * For saved standalone mods, this callback will simply deactivate the mod and remove it from the Page Editor. - * - * In both cases, unsaved changes will be lost. + * For mod components packaged inside a mod, this callback will effectively delete the mod component. Any unsaved + * changes will be lost. */ export function useRemoveModComponentFromStorage(): ( useRemoveConfig: Config, diff --git a/src/pageEditor/layout/EditorLayout.tsx b/src/pageEditor/layout/EditorLayout.tsx index f60eb30a11..8d2d25fa72 100644 --- a/src/pageEditor/layout/EditorLayout.tsx +++ b/src/pageEditor/layout/EditorLayout.tsx @@ -34,7 +34,6 @@ import { selectIsStaleSession } from "@/store/sessionChanges/sessionChangesSelec import StaleSessionPane from "@/pageEditor/panes/StaleSessionPane"; import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; import { usePreviousValue } from "@/hooks/usePreviousValue"; -import useMigrateStandaloneComponentsToMods from "@/pageEditor/hooks/useMigrateStandaloneComponentsToMods"; import { RestrictedFeatures } from "@/auth/featureFlags"; const EditorLayout: React.FunctionComponent = () => { @@ -47,15 +46,6 @@ const EditorLayout: React.FunctionComponent = () => { const url = useCurrentInspectedUrl(); - /** - * Migrate form states for activated standalone mod components. We are - * running it here because it's the top-most component underneath - * the redux-persist PersistGate. - * - * @since 2.0.8 - */ - useMigrateStandaloneComponentsToMods(); - const currentPane = useMemo( () => isRestricted ? ( diff --git a/src/pageEditor/modListingPanel/ActivatedModComponentListItem.tsx b/src/pageEditor/modListingPanel/ActivatedModComponentListItem.tsx index 724f49031a..7a778ef05f 100644 --- a/src/pageEditor/modListingPanel/ActivatedModComponentListItem.tsx +++ b/src/pageEditor/modListingPanel/ActivatedModComponentListItem.tsx @@ -77,7 +77,7 @@ const ActivatedModComponentListItem: React.FunctionComponent<{ ); const isActive = activeModComponentFormState?.uuid === modComponent.id; // Get the selected mod id, or the mod id of the selected mod component - const modId = activeModId ?? activeModComponentFormState?.modMetadata?.id; + const modId = activeModId ?? activeModComponentFormState?.modMetadata.id; // Set the alternate background if this item isn't active, but either its mod or another item in its mod is active const hasActiveModBackground = !isActive && modId && modComponent._recipe?.id === modId; diff --git a/src/pageEditor/modListingPanel/DraftModComponentListItem.tsx b/src/pageEditor/modListingPanel/DraftModComponentListItem.tsx index 422e1b4271..3c89c265ba 100644 --- a/src/pageEditor/modListingPanel/DraftModComponentListItem.tsx +++ b/src/pageEditor/modListingPanel/DraftModComponentListItem.tsx @@ -16,7 +16,7 @@ */ import styles from "./Entry.module.scss"; -import React, { useCallback, useMemo } from "react"; +import React, { useCallback } from "react"; import { actions } from "@/pageEditor/store/editor/editorSlice"; import { useDispatch, useSelector } from "react-redux"; import { ListGroup } from "react-bootstrap"; @@ -47,11 +47,8 @@ import ModComponentActionMenu from "@/pageEditor/modListingPanel/ModComponentAct import useClearModComponentChanges from "@/pageEditor/hooks/useClearModComponentChanges"; import { useRemoveModComponentFromStorage, - DEACTIVATE_MOD_MODAL_PROPS, - DELETE_STANDALONE_MOD_COMPONENT_MODAL_PROPS, DELETE_STARTER_BRICK_MODAL_PROPS, } from "@/pageEditor/hooks/useRemoveModComponentFromStorage"; -import { selectIsModComponentSavedOnCloud } from "@/store/modComponents/modComponentSelectors"; import { inspectedTab } from "@/pageEditor/context/connection"; import { StarterBrickTypes } from "@/types/starterBrickTypes"; @@ -78,9 +75,9 @@ const DraftModComponentListItem: React.FunctionComponent< const isActive = activeModComponentFormState?.uuid === modComponentFormState.uuid; - const modId = modComponentFormState.modMetadata?.id; - const isSiblingOfActiveListItem = activeModComponentFormState?.modMetadata?.id - ? modId === activeModComponentFormState?.modMetadata?.id + const modId = modComponentFormState.modMetadata.id; + const isSiblingOfActiveListItem = activeModComponentFormState?.modMetadata.id + ? modId === activeModComponentFormState?.modMetadata.id : false; const isChildOfActiveListItem = modId === activeModId; const isRelativeOfActiveListItem = @@ -88,9 +85,6 @@ const DraftModComponentListItem: React.FunctionComponent< const isDirty = useSelector( selectModComponentIsDirty(modComponentFormState.uuid), ); - const isSavedOnCloud = useSelector( - selectIsModComponentSavedOnCloud(modComponentFormState.uuid), - ); const removeModComponentFromStorage = useRemoveModComponentFromStorage(); const isButton = modComponentFormState.starterBrick.definition.type === @@ -109,36 +103,12 @@ const DraftModComponentListItem: React.FunctionComponent< const deleteModComponent = async () => removeModComponentFromStorage({ modComponentId: modComponentFormState.uuid, - showConfirmationModal: modId - ? DELETE_STARTER_BRICK_MODAL_PROPS - : DELETE_STANDALONE_MOD_COMPONENT_MODAL_PROPS, - }); - - const deactivateModComponent = async () => - removeModComponentFromStorage({ - modComponentId: modComponentFormState.uuid, - showConfirmationModal: DEACTIVATE_MOD_MODAL_PROPS, + showConfirmationModal: DELETE_STARTER_BRICK_MODAL_PROPS, }); - const onSave = useMemo(() => { - if (modComponentFormState.modMetadata == null) { - return async () => { - dispatch(actions.setActiveModComponentId(modComponentFormState.uuid)); - dispatch(actions.showCreateModModal({ keepLocalCopy: false })); - }; - } - - // eslint-disable-next-line unicorn/no-useless-undefined -- Code clarity, implicit returns are bad - return undefined; - }, [dispatch, modComponentFormState.modMetadata, modComponentFormState.uuid]); - const onClearChanges = async () => clearModComponentChanges({ modComponentId: modComponentFormState.uuid }); - const onDelete = modId || !isSavedOnCloud ? deleteModComponent : undefined; - - const onDeactivate = onDelete ? undefined : deactivateModComponent; - return ( )} - {isDirty && - // Don't show the dirty icon and save button at the same time - !onSave && ( - - - - )} + {isDirty && ( + + + + )} { dispatch( // Duplicate the mod component in the same mod @@ -212,27 +180,12 @@ const DraftModComponentListItem: React.FunctionComponent< onClearChanges={ modComponentFormState.installed ? onClearChanges : undefined } - onMoveToMod={ - modComponentFormState.modMetadata - ? async () => { - dispatch( - actions.showMoveCopyToModModal({ moveOrCopy: "move" }), - ); - } - : undefined - } - onCopyToMod={ - modComponentFormState.modMetadata - ? async () => { - dispatch( - actions.showMoveCopyToModModal({ moveOrCopy: "copy" }), - ); - } - : undefined - } - // TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/9242, remove standalone mod component actions - onSave={onSave} - onDelete={onDelete} + onMoveToMod={async () => { + dispatch(actions.showMoveCopyToModModal({ moveOrCopy: "move" })); + }} + onCopyToMod={async () => { + dispatch(actions.showMoveCopyToModModal({ moveOrCopy: "copy" })); + }} /> ); diff --git a/src/pageEditor/modListingPanel/ModComponentActionMenu.tsx b/src/pageEditor/modListingPanel/ModComponentActionMenu.tsx index ad370a483a..4b79ae3443 100644 --- a/src/pageEditor/modListingPanel/ModComponentActionMenu.tsx +++ b/src/pageEditor/modListingPanel/ModComponentActionMenu.tsx @@ -20,7 +20,6 @@ import { faClone, faFileExport, faHistory, - faTimes, faTrash, } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; @@ -28,7 +27,6 @@ import styles from "./ActionMenu.module.scss"; import EllipsisMenu, { type EllipsisMenuItem, } from "@/components/ellipsisMenu/EllipsisMenu"; -import SaveButton from "@/pageEditor/modListingPanel/SaveButton"; type OptionalAction = (() => Promise) | undefined; @@ -36,28 +34,22 @@ type ActionMenuProps = { labelRoot: string; isDirty: boolean; isActive: boolean; - onDelete: OptionalAction; - onDuplicate: OptionalAction; + onDelete: () => Promise; + onDuplicate: () => Promise; onClearChanges: OptionalAction; - onMoveToMod: OptionalAction; - onCopyToMod: OptionalAction; - // TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/9242, remove standalone mod component actions - onSave: OptionalAction; - onDeactivate: OptionalAction; + onMoveToMod: () => Promise; + onCopyToMod: () => Promise; }; const ModComponentActionMenu: React.FC = ({ isActive, labelRoot, isDirty, - onDelete = null, - onDuplicate = null, + onDelete, + onDuplicate, onClearChanges = null, - onMoveToMod = null, - onCopyToMod = null, - // Standalone Mod Component Actions - onSave = null, - onDeactivate = null, + onMoveToMod, + onCopyToMod, }) => { const menuItems: EllipsisMenuItem[] = [ { @@ -72,7 +64,6 @@ const ModComponentActionMenu: React.FC = ({ title: "Duplicate", icon: , action: onDuplicate, - hide: !onDuplicate, }, { title: "Move to mod", @@ -84,7 +75,6 @@ const ModComponentActionMenu: React.FC = ({ /> ), action: onMoveToMod, - hide: !onMoveToMod, }, { title: "Copy to mod", @@ -96,31 +86,16 @@ const ModComponentActionMenu: React.FC = ({ /> ), action: onCopyToMod, - hide: !onCopyToMod, }, { title: "Delete", icon: , action: onDelete, - hide: !onDelete, - }, - { - title: "Deactivate", - icon: , - action: onDeactivate, - hide: !onDeactivate, }, ]; return (
- {onSave != null && ( - - )} {isActive && ( = ({ // Set the alternate background if a mod component in this mod is active const hasModBackground = - activeModComponentFormState?.modMetadata?.id === modId; + activeModComponentFormState?.modMetadata.id === modId; const dirtyName = useSelector(selectDirtyMetadataForModId(modId))?.name; const name = dirtyName ?? savedName ?? "Loading..."; diff --git a/src/pageEditor/modListingPanel/__snapshots__/DraftModComponentListItem.test.tsx.snap b/src/pageEditor/modListingPanel/__snapshots__/DraftModComponentListItem.test.tsx.snap index bbb16e29d2..cdfa444258 100644 --- a/src/pageEditor/modListingPanel/__snapshots__/DraftModComponentListItem.test.tsx.snap +++ b/src/pageEditor/modListingPanel/__snapshots__/DraftModComponentListItem.test.tsx.snap @@ -32,30 +32,16 @@ exports[`DraftModComponentListItem renders active element 1`] = ` > Element 1 + + +
- -
+ />
- - - -
-
    -
  • -
      -
    • -
      -
      - ▶ -
      -
      - - - - - {} - - 2 keys - - -
        -
      • - - - "bar" - -
      • -
      • - - - 32 - -
      • -
      -
    • -
    • -
      -
      - ▶ -
      -
      - - - - - {} - - 2 keys - - -
        -
      • - - - "bar" - -
      • -
      • - - - 32 - -
      • -
      -
    • -
    • -
      -
      - ▶ -
      -
      - - - - - {} - - 2 keys - - -
        -
      • - - - "bar" - -
      • -
      • - - - 32 - -
      • -
      -
    • -
    -
  • -
-
- - -`; diff --git a/src/pageEditor/tabs/modMetadata/ModMetadataEditor.test.tsx b/src/pageEditor/tabs/modMetadata/ModMetadataEditor.test.tsx index 1792e6ebd6..30ef5a1870 100644 --- a/src/pageEditor/tabs/modMetadata/ModMetadataEditor.test.tsx +++ b/src/pageEditor/tabs/modMetadata/ModMetadataEditor.test.tsx @@ -62,7 +62,7 @@ describe("ModMetadataEditor", () => { modComponentSlice.actions.UNSAFE_setModComponents([modComponent]), ); dispatch(editorActions.addModComponentFormState(formState)); - dispatch(editorActions.setActiveModId(formState.modMetadata!.id)); + dispatch(editorActions.setActiveModId(formState.modMetadata.id)); }, }); @@ -98,7 +98,7 @@ describe("ModMetadataEditor", () => { modComponentSlice.actions.UNSAFE_setModComponents([modComponent]), ); dispatch(editorActions.addModComponentFormState(formState)); - dispatch(editorActions.setActiveModId(formState.modMetadata!.id)); + dispatch(editorActions.setActiveModId(formState.modMetadata.id)); }, }); diff --git a/src/pageEditor/utils.ts b/src/pageEditor/utils.ts index 0c2b9650ce..6338b17a18 100644 --- a/src/pageEditor/utils.ts +++ b/src/pageEditor/utils.ts @@ -30,7 +30,10 @@ import ForEachElement from "@/bricks/transformers/controlFlow/ForEachElement"; import { castArray, pick, pickBy } from "lodash"; import { type AnalysisAnnotation } from "@/analysis/analysisTypes"; import { PIPELINE_BRICKS_FIELD_NAME } from "./consts"; -import { type ModComponentBase } from "@/types/modComponentTypes"; +import { + type ModComponentBase, + type ModMetadata, +} from "@/types/modComponentTypes"; import { type UUID } from "@/types/stringTypes"; import { type RegistryId } from "@/types/registryTypes"; import { type Brick } from "@/types/brickTypes"; @@ -47,7 +50,7 @@ import { type UnsavedModDefinition } from "@/types/modDefinitionTypes"; export function mapModDefinitionUpsertResponseToModMetadata( unsavedModDefinition: UnsavedModDefinition, response: PackageUpsertResponse, -): ModComponentBase["_recipe"] { +): ModMetadata { return { ...unsavedModDefinition.metadata, sharing: pick(response, ["public", "organizations"]), @@ -68,7 +71,7 @@ export function getModId( ): RegistryId | undefined { return isModComponentBase(modComponentOrFormState) ? modComponentOrFormState._recipe?.id - : modComponentOrFormState.modMetadata?.id; + : modComponentOrFormState.modMetadata.id; } /** diff --git a/src/store/editorMigrations.test.ts b/src/store/editorMigrations.test.ts index 591a118f48..f8d8cf0461 100644 --- a/src/store/editorMigrations.test.ts +++ b/src/store/editorMigrations.test.ts @@ -24,6 +24,7 @@ import { type EditorStateV6, type EditorStateV7, type EditorStateV8, + type EditorStateV9, } from "@/pageEditor/store/editor/pageEditorTypes"; import { cloneDeep, mapValues, omit } from "lodash"; import { @@ -40,13 +41,17 @@ import { uuidSequence, } from "@/testUtils/factories/stringFactories"; import { modMetadataFactory } from "@/testUtils/factories/modComponentFactories"; -import { validateRegistryId } from "@/types/helpers"; +import { + isInnerDefinitionRegistryId, + validateRegistryId, +} from "@/types/helpers"; import { type BaseFormStateV1, type BaseFormStateV2, type BaseFormStateV3, type BaseFormStateV4, type BaseFormStateV5, + type BaseFormStateV6, type BaseModComponentStateV1, type BaseModComponentStateV2, } from "@/pageEditor/store/editor/baseFormStateTypes"; @@ -59,6 +64,7 @@ import { migrateEditorStateV5, migrateEditorStateV6, migrateEditorStateV7, + migrateEditorStateV8, } from "@/store/editorMigrations"; import { type FactoryConfig } from "cooky-cutter/dist/define"; import { StarterBrickTypes } from "@/types/starterBrickTypes"; @@ -286,6 +292,12 @@ const initialStateV8: EditorStateV8 & PersistedState = { }, }; +const initialStateV9: EditorStateV9 & PersistedState = { + ...cloneDeep(initialStateV8), + modComponentFormStates: [], + deletedModComponentFormStatesByModId: {}, +}; + function unmigrateServices( integrationDependencies: IntegrationDependencyV2[] = [], ): IntegrationDependencyV1[] { @@ -420,6 +432,15 @@ function unmigrateFormStateV5toV4(formState: BaseFormStateV5): BaseFormStateV4 { return omit(formState, "variablesDefinition"); } +function unmigrateFormStateV6toV5(formState: BaseFormStateV6): BaseFormStateV5 { + return { + ...formState, + modMetadata: isInnerDefinitionRegistryId(formState.modMetadata.id) + ? undefined + : formState.modMetadata, + }; +} + function unmigrateEditorStateV5toV4( state: EditorStateV5 & PersistedState, ): EditorStateV4 & PersistedState { @@ -471,13 +492,27 @@ function unmigrateEditorStateV8toV7( }; } +function unmigrateEditorStateV9toV8( + state: EditorStateV9 & PersistedState, +): EditorStateV8 & PersistedState { + return { + ...state, + modComponentFormStates: state.modComponentFormStates.map((formState) => + unmigrateFormStateV6toV5(formState), + ), + }; +} + type SimpleFactory = (override?: FactoryConfig) => T; -const formStateFactoryV5: SimpleFactory = (override) => +const formStateFactoryV6: SimpleFactory = (override) => formStateFactory({ formStateConfig: override as FactoryConfig, }); +const formStateFactoryV5: SimpleFactory = (override) => + unmigrateFormStateV6toV5(formStateFactoryV6()); + const formStateFactoryV4: SimpleFactory = (override) => unmigrateFormStateV5toV4(formStateFactoryV5()); @@ -700,4 +735,33 @@ describe("editor state migrations", () => { ); }); }); + + describe("migrateEditorState V8 to V9", () => { + it("migrates empty state", () => { + expect(migrateEditorStateV8(initialStateV8)).toStrictEqual( + initialStateV9, + ); + }); + + it("adds missing modMetadata", () => { + const expectedEditorStateV9: EditorStateV9 & PersistedState = { + ...initialStateV9, + modComponentFormStates: [formStateFactoryV6()], + }; + const unmigrated = unmigrateEditorStateV9toV8(expectedEditorStateV9); + expect(migrateEditorStateV8(unmigrated)).toStrictEqual({ + ...expectedEditorStateV9, + modComponentFormStates: [ + { + ...expectedEditorStateV9.modComponentFormStates[0], + modMetadata: expect.objectContaining({ + // Won't match exactly because the naming scheme is different between the factory and generation + id: expect.stringMatching(/@internal\/mod\/\S+/), + name: expect.toBeString(), + }), + }, + ], + }); + }); + }); }); diff --git a/src/store/editorMigrations.ts b/src/store/editorMigrations.ts index 2929e76ad1..c0d874c085 100644 --- a/src/store/editorMigrations.ts +++ b/src/store/editorMigrations.ts @@ -23,6 +23,7 @@ import { type BaseFormStateV3, type BaseFormStateV4, type BaseFormStateV5, + type BaseFormStateV6, type BaseModComponentStateV1, type BaseModComponentStateV2, } from "@/pageEditor/store/editor/baseFormStateTypes"; @@ -40,13 +41,17 @@ import { type EditorStateV6, type EditorStateV7, type EditorStateV8, + type EditorStateV9, } from "@/pageEditor/store/editor/pageEditorTypes"; import { type ModComponentFormState } from "@/pageEditor/starterBricks/formStateTypes"; import { produce } from "immer"; import { DataPanelTabKey } from "@/pageEditor/tabs/editTab/dataPanel/dataPanelTypes"; import { makeInitialDataTabState } from "@/pageEditor/store/editor/uiState"; import { type BrickConfigurationUIState } from "@/pageEditor/store/editor/uiStateTypes"; -import { emptyModVariablesDefinitionFactory } from "@/utils/modUtils"; +import { + createNewUnsavedModMetadata, + emptyModVariablesDefinitionFactory, +} from "@/utils/modUtils"; export const migrations: MigrationManifest = { // Redux-persist defaults to version: -1; Initialize to positive-1-indexed @@ -60,6 +65,7 @@ export const migrations: MigrationManifest = { 6: (state: EditorStateV5 & PersistedState) => migrateEditorStateV5(state), 7: (state: EditorStateV6 & PersistedState) => migrateEditorStateV6(state), 8: (state: EditorStateV7 & PersistedState) => migrateEditorStateV7(state), + 9: (state: EditorStateV8 & PersistedState) => migrateEditorStateV8(state), }; export function migrateIntegrationDependenciesV1toV2( @@ -249,3 +255,18 @@ export function migrateEditorStateV7( } }) as EditorState & PersistedState; } + +export function migrateEditorStateV8( + state: EditorStateV8 & PersistedState, +): EditorStateV9 & PersistedState { + return produce(state, (draft) => { + // Don't need to also loop over deletedModComponentFormStatesByModId, because there can't be any standalone + // mod components in there. (Standalone mods when deleted are removed from the editor state entirely.) + for (const formState of draft.modComponentFormStates) { + (formState as BaseFormStateV6).modMetadata ??= + createNewUnsavedModMetadata({ + modName: formState.label, + }); + } + }) as EditorState & PersistedState; +} diff --git a/src/store/modComponents/modComponentMigrations.test.ts b/src/store/modComponents/modComponentMigrations.test.ts index 65cb823052..c00220b4e5 100644 --- a/src/store/modComponents/modComponentMigrations.test.ts +++ b/src/store/modComponents/modComponentMigrations.test.ts @@ -52,6 +52,7 @@ import type { IntegrationDependencyV2, } from "@/integrations/integrationTypes"; import type { FactoryConfig } from "cooky-cutter/dist/define"; +import { array } from "cooky-cutter"; const testUserScope = "@test-user"; @@ -118,21 +119,18 @@ describe("migrateStandaloneComponentsToMods", () => { it("returns only mod components when userScope is null", () => { const modMetadata = modMetadataFactory(); - const modComponents = [ - activatedModComponentFactory({ - _recipe: modMetadata, - }), - activatedModComponentFactory({ - _recipe: modMetadata, - }), - activatedModComponentFactory({ - _recipe: modMetadata, - }), - ]; - const standaloneComponents = [ - activatedModComponentFactory(), - activatedModComponentFactory(), - ]; + const modComponents = array( + activatedModComponentFactory, + 3, + )({ + _recipe: modMetadata, + }); + const standaloneComponents = array( + activatedModComponentFactory, + 2, + )({ + _recipe: undefined, + }); expect( migrateStandaloneComponentsToMods( @@ -144,21 +142,20 @@ describe("migrateStandaloneComponentsToMods", () => { it("converts standalone components correctly", () => { const modMetadata = modMetadataFactory(); - const modComponents = [ - activatedModComponentFactory({ - _recipe: modMetadata, - }), - activatedModComponentFactory({ - _recipe: modMetadata, - }), - activatedModComponentFactory({ - _recipe: modMetadata, - }), - ]; - const standaloneComponents = [ - activatedModComponentFactory(), - activatedModComponentFactory(), - ]; + const modComponents = array( + activatedModComponentFactory, + 3, + )({ + _recipe: modMetadata, + }); + + const standaloneComponents = array( + activatedModComponentFactory, + 2, + )({ + _recipe: undefined, + }); + const migratedStandaloneComponents = standaloneComponents.map((component) => createModMetadataForStandaloneComponent(component, testUserScope), ); @@ -390,8 +387,12 @@ describe("inferModComponentStateVersion", () => { // type, and have at least one "standalone" mod component present (no _recipe). const state: ModComponentStateV3 = { extensions: [ - activatedFactoryV2(), - activatedFactoryV2(), + activatedFactoryV2({ + _recipe: undefined, + }), + activatedFactoryV2({ + _recipe: undefined, + }), activatedFactoryV2({ _recipe: modMetadataFactory(), }), diff --git a/src/store/modComponents/modComponentSelectors.ts b/src/store/modComponents/modComponentSelectors.ts index c2410e33f9..baa200d2de 100644 --- a/src/store/modComponents/modComponentSelectors.ts +++ b/src/store/modComponents/modComponentSelectors.ts @@ -20,7 +20,6 @@ import { createSelector } from "@reduxjs/toolkit"; import { type ActivatedModComponent } from "@/types/modComponentTypes"; import { type RegistryId } from "@/types/registryTypes"; import { isEmpty, memoize } from "lodash"; -import { type UUID } from "@/types/stringTypes"; export function selectActivatedModComponents({ options, @@ -35,17 +34,6 @@ export function selectActivatedModComponents({ return options.activatedModComponents; } -const isModComponentSavedOnCloudSelector = createSelector( - selectActivatedModComponents, - (_state: ModComponentsRootState, modComponentId: UUID) => modComponentId, - (modComponents, modComponentId) => - modComponents.some((modComponent) => modComponent.id === modComponentId), -); - -export const selectIsModComponentSavedOnCloud = - (modComponentId: UUID) => (state: ModComponentsRootState) => - isModComponentSavedOnCloudSelector(state, modComponentId); - export const selectGetModComponentsForMod = createSelector( selectActivatedModComponents, (activatedModComponents) => diff --git a/src/store/sessionChanges/sessionChangesListenerMiddleware.ts b/src/store/sessionChanges/sessionChangesListenerMiddleware.ts index a474fc2581..34a41465c0 100644 --- a/src/store/sessionChanges/sessionChangesListenerMiddleware.ts +++ b/src/store/sessionChanges/sessionChangesListenerMiddleware.ts @@ -34,9 +34,7 @@ sessionChangesListenerMiddleware.startListening({ actions.editModOptionsDefinitions, actions.editModOptionsValues, actions.clearMetadataAndOptionsChangesForMod, - actions.addModComponentFormStateToMod, actions.addModComponentFormState, - actions.removeModComponentFormStateFromMod, actions.removeModData, modComponentSlice.actions.removeModComponent, diff --git a/src/testUtils/factories/modComponentFactories.ts b/src/testUtils/factories/modComponentFactories.ts index 783a506bc0..61ce129ad7 100644 --- a/src/testUtils/factories/modComponentFactories.ts +++ b/src/testUtils/factories/modComponentFactories.ts @@ -52,7 +52,7 @@ export const modComponentRefFactory = define({ /** * Factory for a mod component ref from a standalone mod component. * @deprecated standalone mod components are deprecated - * @since 2.0.6 provides a internal mod id instead of `undefined` + * @since 2.0.6 provides an internal mod id instead of `undefined` */ export const standaloneModComponentRefFactory = define({ // Don't repeat UUIDs across contexts @@ -113,7 +113,8 @@ export const modComponentFactory = define({ apiVersion: "v3" as ApiVersion, extensionPointId: (n: number) => validateRegistryId(`test/starter-brick-${n}`), - _recipe: undefined, + // @since 2.1.4 includes mod metadata + _recipe: modMetadataFactory, _deployment: undefined, label: "Test label", integrationDependencies(): IntegrationDependency[] { diff --git a/src/testUtils/factories/pageEditorFactories.ts b/src/testUtils/factories/pageEditorFactories.ts index 511fd8bad2..7993408293 100644 --- a/src/testUtils/factories/pageEditorFactories.ts +++ b/src/testUtils/factories/pageEditorFactories.ts @@ -56,7 +56,10 @@ import { type BaseModComponentState } from "@/pageEditor/store/editor/baseFormSt import { assertNotNullish } from "@/utils/nullishUtils"; import { type Permissions } from "webextension-polyfill"; import { validateOutputKey } from "@/runtime/runtimeTypes"; -import { emptyModVariablesDefinitionFactory } from "@/utils/modUtils"; +import { + createNewUnsavedModMetadata, + emptyModVariablesDefinitionFactory, +} from "@/utils/modUtils"; const baseModComponentStateFactory = define({ brickPipeline: () => pipelineFactory(), @@ -75,12 +78,15 @@ const internalFormStateFactory = define({ apiVersion: "v3" as ApiVersion, uuid: uuidSequence, installed: true, - optionsArgs: () => ({}) as OptionsArgs, + optionsArgs() { + return {} as OptionsArgs; + }, variablesDefinition: () => emptyModVariablesDefinitionFactory(), integrationDependencies(): IntegrationDependency[] { return []; }, - modMetadata: undefined, + modMetadata: (n: number) => + createNewUnsavedModMetadata({ modName: `Unsaved Mod ${n}` }), permissions(): Permissions.Permissions { return { permissions: [], @@ -133,14 +139,15 @@ export const triggerFormStateFactory = ( override?: FactoryConfig, pipelineOverride?: BrickPipeline, ) => { - const defaultProps = trigger.fromNativeElement( - "https://test.com", - metadataFactory({ + const defaultProps = trigger.fromNativeElement({ + url: "https://test.com", + modMetadata: createNewUnsavedModMetadata({ modName: "Unsaved Mod" }), + starterBrickMetadata: metadataFactory({ id: (n: number) => validateRegistryId(`test/extension-point-${n}`), name: (n: number) => `Extension Point ${n}`, }), - null, - ); + element: null, + }); return formStateFactory({ formStateConfig: { @@ -155,15 +162,15 @@ export const sidebarPanelFormStateFactory = ( override?: FactoryConfig, pipelineOverride?: BrickPipeline, ): SidebarFormState => { - const defaultProps = sidebar.fromNativeElement( - "https://test.com", - metadataFactory({ + const defaultProps = sidebar.fromNativeElement({ + url: "https://test.com", + modMetadata: createNewUnsavedModMetadata({ modName: "Unsaved Mod" }), + starterBrickMetadata: metadataFactory({ id: (n: number) => validateRegistryId(`test/extension-point-${n}`), name: (n: number) => `Extension Point ${n}`, }), - // TypeScript complains if the 3rd positional argument is left off - undefined as never, - ); + element: null, + }); return formStateFactory({ formStateConfig: { @@ -178,14 +185,15 @@ export const contextMenuFormStateFactory = ( override?: FactoryConfig, pipelineOverride?: BrickPipeline, ) => { - const defaultProps = contextMenu.fromNativeElement( - "https://test.com", - metadataFactory({ + const defaultProps = contextMenu.fromNativeElement({ + url: "https://test.com", + modMetadata: createNewUnsavedModMetadata({ modName: "Unsaved Mod" }), + starterBrickMetadata: metadataFactory({ id: (n: number) => validateRegistryId(`test/extension-point-${n}`), name: (n: number) => `Extension Point ${n}`, }), - null, - ); + element: null, + }); return formStateFactory({ formStateConfig: { @@ -200,14 +208,15 @@ export const quickbarFormStateFactory = ( override?: FactoryConfig, pipelineOverride?: BrickPipeline, ) => { - const defaultProps = quickBar.fromNativeElement( - "https://test.com", - metadataFactory({ + const defaultProps = quickBar.fromNativeElement({ + url: "https://test.com", + modMetadata: createNewUnsavedModMetadata({ modName: "Unsaved Mod" }), + starterBrickMetadata: metadataFactory({ id: (n: number) => validateRegistryId(`test/extension-point-${n}`), name: (n: number) => `Extension Point ${n}`, }), - null, - ); + element: null, + }); return formStateFactory({ formStateConfig: { @@ -222,18 +231,19 @@ export const menuItemFormStateFactory = ( override?: FactoryConfig, pipelineOverride?: BrickPipeline, ) => { - const defaultTriggerProps = menuItem.fromNativeElement( - "https://test.com", - metadataFactory({ + const defaultTriggerProps = menuItem.fromNativeElement({ + url: "https://test.com", + modMetadata: createNewUnsavedModMetadata({ modName: "Unsaved Mod" }), + starterBrickMetadata: metadataFactory({ id: (n: number) => validateRegistryId(`test/extension-point-${n}`), name: (n: number) => `Extension Point ${n}`, }), - { + element: { item: { caption: "Caption for test", }, } as ButtonSelectionResult, - ); + }); return formStateFactory({ formStateConfig: {