From 983ae0c9e340e085a86ba5042d933e5e68e75b98 Mon Sep 17 00:00:00 2001 From: Misha Holtz <36575242+mnholtz@users.noreply.github.com> Date: Fri, 15 Dec 2023 10:43:52 -0800 Subject: [PATCH] #7111 Fix deactivation/delete actions for unsaved standalone mod components (#7121) * refactor rename useSaveExtension -> useSaveStandaloneModComponent * refactor rename saveExtension -> saveStandaloneModComponent * rename saveExtension -> saveModComponent * add upsertStandaloneModComponent to rtk query endpoints * update modal message * add isSavedOnMod via selector * create selector for isSavedOnCloud * introduce modal constants * refactor remove useDeactivateModComponent/useDeleteModComponent * refactor use useRemoveModComponent * change type of props for useRemoveModComponentFromStorage * refactor rename useRemoveModComponent -> useRemoveModComponentFromStorage * remove useDeactivate... useDelete... helper hooks * update comment * remove showConfiramtionModal prop * update callback dependency props * revert unused changes * change config prop --- src/components/ConfirmationModal.tsx | 13 +- src/pageEditor/hooks/useDeactivateMod.tsx | 35 +++--- ... useRemoveModComponentFromStorage.test.ts} | 53 ++++---- ...s => useRemoveModComponentFromStorage.tsx} | 116 ++++++++---------- ...on.ts => useSaveStandaloneModComponent.ts} | 4 +- .../sidebar/DynamicModComponentListItem.tsx | 52 +++++--- .../sidebar/modals/AddToRecipeModal.tsx | 7 +- .../sidebar/modals/CreateRecipeModal.tsx | 9 +- src/store/extensionsSelectors.ts | 12 ++ 9 files changed, 151 insertions(+), 150 deletions(-) rename src/pageEditor/hooks/{useRemoveModComponent.test.ts => useRemoveModComponentFromStorage.test.ts} (53%) rename src/pageEditor/hooks/{useRemoveModComponent.ts => useRemoveModComponentFromStorage.tsx} (57%) rename src/pageEditor/hooks/{useSaveExtension.ts => useSaveStandaloneModComponent.ts} (95%) diff --git a/src/components/ConfirmationModal.tsx b/src/components/ConfirmationModal.tsx index de5bf656be..1edfb6734e 100644 --- a/src/components/ConfirmationModal.tsx +++ b/src/components/ConfirmationModal.tsx @@ -25,7 +25,7 @@ import React, { import { Modal, Button } from "react-bootstrap"; import { type ButtonVariant } from "react-bootstrap/types"; -type ModalProps = { +export type ConfirmationModalProps = { title?: string; message: string | React.ReactElement; submitVariant?: ButtonVariant; @@ -34,7 +34,7 @@ type ModalProps = { }; type ModalContextProps = { - showConfirmation: (modalProps: ModalProps) => Promise; + showConfirmation: (modalProps: ConfirmationModalProps) => Promise; }; const initialModalState: ModalContextProps = { @@ -46,7 +46,7 @@ const initialModalState: ModalContextProps = { const ModalContext = createContext(initialModalState); const ConfirmationModal: React.FunctionComponent< - ModalProps & { + ConfirmationModalProps & { onCancel: () => void; onSubmit: () => void; onExited: () => void; @@ -87,14 +87,15 @@ const ConfirmationModal: React.FunctionComponent< type Callback = (submit: boolean) => void; -const DEFAULT_MODAL_PROPS: ModalProps = { +const DEFAULT_MODAL_PROPS: ConfirmationModalProps = { message: "Are you sure?", }; export const ModalProvider: React.FunctionComponent<{ children: React.ReactNode; }> = ({ children }) => { - const [modalProps, setModalProps] = useState(DEFAULT_MODAL_PROPS); + const [modalProps, setModalProps] = + useState(DEFAULT_MODAL_PROPS); const [callback, setCallback] = useState(); const [isModalVisible, setIsModalVisible] = useState(false); useEffect( @@ -106,7 +107,7 @@ export const ModalProvider: React.FunctionComponent<{ ); const showConfirmation = useCallback( - async (modalProps: ModalProps) => { + async (modalProps: ConfirmationModalProps) => { // Cancel any previous modal that was showing callback?.(false); diff --git a/src/pageEditor/hooks/useDeactivateMod.tsx b/src/pageEditor/hooks/useDeactivateMod.tsx index b4e5cdb49f..e073d5b114 100644 --- a/src/pageEditor/hooks/useDeactivateMod.tsx +++ b/src/pageEditor/hooks/useDeactivateMod.tsx @@ -15,9 +15,12 @@ * along with this program. If not, see . */ -import React, { useCallback } from "react"; +import { useCallback } from "react"; import { type RegistryId } from "@/types/registryTypes"; -import { useDeactivateModComponent } from "@/pageEditor/hooks/useRemoveModComponent"; +import { + DEACTIVATE_MOD_MODAL_PROPS, + useRemoveModComponentFromStorage, +} from "@/pageEditor/hooks/useRemoveModComponentFromStorage"; import { useDispatch, useSelector } from "react-redux"; import { selectExtensions } from "@/store/extensionsSelectors"; import { selectElements } from "@/pageEditor/slices/editorSelectors"; @@ -37,7 +40,7 @@ type Config = { */ function useDeactivateMod(): (useDeactivateConfig: Config) => Promise { const dispatch = useDispatch(); - const deactivateModComponent = useDeactivateModComponent(); + const removeModComponentFromStorage = useRemoveModComponentFromStorage(); const extensions = useSelector(selectExtensions); const elements = useSelector(selectElements); const { showConfirmation } = useModals(); @@ -45,20 +48,7 @@ function useDeactivateMod(): (useDeactivateConfig: Config) => Promise { return useCallback( async ({ modId, shouldShowConfirmation = true }) => { if (shouldShowConfirmation) { - const confirmed = await showConfirmation({ - title: "Deactivate Mod?", - message: ( - <> - This action will deactivate the mod and remove it from the Page - Editor. You can reactivate or delete mods from the{" "} - - PixieBrix Extension Console - - . - - ), - submitCaption: "Deactivate", - }); + const confirmed = await showConfirmation(DEACTIVATE_MOD_MODAL_PROPS); if (!confirmed) { return; @@ -72,9 +62,8 @@ function useDeactivateMod(): (useDeactivateConfig: Config) => Promise { ); await Promise.all( extensionIds.map(async (extensionId) => - deactivateModComponent({ + removeModComponentFromStorage({ extensionId, - shouldShowConfirmation: false, }), ), ); @@ -85,7 +74,13 @@ function useDeactivateMod(): (useDeactivateConfig: Config) => Promise { dispatch(actions.removeRecipeData(modId)); }, - [dispatch, elements, extensions, deactivateModComponent, showConfirmation], + [ + dispatch, + elements, + extensions, + useRemoveModComponentFromStorage, + showConfirmation, + ], ); } diff --git a/src/pageEditor/hooks/useRemoveModComponent.test.ts b/src/pageEditor/hooks/useRemoveModComponentFromStorage.test.ts similarity index 53% rename from src/pageEditor/hooks/useRemoveModComponent.test.ts rename to src/pageEditor/hooks/useRemoveModComponentFromStorage.test.ts index a1ae435934..2609c87f88 100644 --- a/src/pageEditor/hooks/useRemoveModComponent.test.ts +++ b/src/pageEditor/hooks/useRemoveModComponentFromStorage.test.ts @@ -17,10 +17,7 @@ import { renderHook } from "@/pageEditor/testHelpers"; import { removeExtensionsFromAllTabs } from "@/store/uninstallUtils"; -import { - useDeactivateModComponent, - useDeleteModComponent, -} from "./useRemoveModComponent"; +import { useRemoveModComponentFromStorage } from "./useRemoveModComponentFromStorage"; import { actions as editorActions } from "@/pageEditor/slices/editorSlice"; import { actions as extensionsActions } from "@/store/extensionsSlice"; import { clearDynamicElements } from "@/contentScript/messenger/api"; @@ -31,36 +28,32 @@ beforeEach(() => { jest.resetAllMocks(); }); -test("useRemoveModComponent", async () => { +test("useRemoveModComponentFromStorage", async () => { const extensionId = uuidSequence(1); - // eslint-disable-next-line unicorn/no-array-for-each -- Better readability in this case, and performance is not a concern here - [useDeactivateModComponent, useDeleteModComponent].forEach(async (hook) => { - const { - result: { current: removeExtension }, - getReduxStore, - } = renderHook(() => hook(), { - setupRedux(dispatch, { store }) { - jest.spyOn(store, "dispatch"); - }, - }); + const { + result: { current: removeExtension }, + getReduxStore, + } = renderHook(() => useRemoveModComponentFromStorage(), { + setupRedux(dispatch, { store }) { + jest.spyOn(store, "dispatch"); + }, + }); - await removeExtension({ - extensionId, - shouldShowConfirmation: false, - }); + await removeExtension({ + extensionId, + }); - const { dispatch } = getReduxStore(); + const { dispatch } = getReduxStore(); - expect(dispatch).toHaveBeenCalledWith( - editorActions.removeElement(extensionId), - ); - expect(dispatch).toHaveBeenCalledWith( - extensionsActions.removeExtension({ extensionId }), - ); - expect(clearDynamicElements).toHaveBeenCalledWith(expect.any(Object), { - uuid: extensionId, - }); - expect(removeExtensionsFromAllTabs).toHaveBeenCalledWith([extensionId]); + expect(dispatch).toHaveBeenCalledWith( + editorActions.removeElement(extensionId), + ); + expect(dispatch).toHaveBeenCalledWith( + extensionsActions.removeExtension({ extensionId }), + ); + expect(clearDynamicElements).toHaveBeenCalledWith(expect.any(Object), { + uuid: extensionId, }); + expect(removeExtensionsFromAllTabs).toHaveBeenCalledWith([extensionId]); }); diff --git a/src/pageEditor/hooks/useRemoveModComponent.ts b/src/pageEditor/hooks/useRemoveModComponentFromStorage.tsx similarity index 57% rename from src/pageEditor/hooks/useRemoveModComponent.ts rename to src/pageEditor/hooks/useRemoveModComponentFromStorage.tsx index b1477e4fea..b410fec7e6 100644 --- a/src/pageEditor/hooks/useRemoveModComponent.ts +++ b/src/pageEditor/hooks/useRemoveModComponentFromStorage.tsx @@ -18,8 +18,11 @@ import { type UUID } from "@/types/stringTypes"; import { useDispatch, useSelector } from "react-redux"; import { selectSessionId } from "@/pageEditor/slices/sessionSelectors"; -import { useModals } from "@/components/ConfirmationModal"; -import { useCallback } from "react"; +import { + type ConfirmationModalProps, + useModals, +} from "@/components/ConfirmationModal"; +import React, { useCallback } from "react"; import reportEvent from "@/telemetry/reportEvent"; import { Events } from "@/telemetry/events"; import notify from "@/utils/notify"; @@ -31,24 +34,64 @@ import { removeExtensionsFromAllTabs } from "@/store/uninstallUtils"; type Config = { extensionId: UUID; - shouldShowConfirmation?: boolean; + // Show a confirmation modal with the specified modal props before removing the mod component if defined + showConfirmationModal?: ConfirmationModalProps; +}; + +export const DELETE_STARTER_BRICK_MODAL_PROPS: ConfirmationModalProps = { + title: "Delete starter brick?", + message: "This action cannot be undone.", + 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: ( + <> + Any unsaved changes will be lost. You can reactivate or delete mods from + the{" "} + + PixieBrix Extension Console + + . + + ), + submitCaption: "Deactivate", }; /** * Returns a callback that removes a mod component from the Page Editor and Extension Storage. * - * For mod components (packaged inside a mod), this callback will effectively delete the mod component. - * For standalone mods, this callback will simply deactivate the mod and remove it from the Page Editor. + * 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. * - * Prefer using `useDeactivateModComponent` or `useDeleteModComponent` instead of exporting this hook. + * In both cases, unsaved changes will be lost. **/ -function _useRemoveModComponent(): (extensionId: UUID) => Promise { +export function useRemoveModComponentFromStorage(): ( + useRemoveConfig: Config, +) => Promise { const dispatch = useDispatch(); const sessionId = useSelector(selectSessionId); + const { showConfirmation } = useModals(); return useCallback( - async (extensionId) => { - console.debug(`pageEditor: remove extension with id ${extensionId}`); + async ({ extensionId, showConfirmationModal }) => { + console.debug(`pageEditor: remove mod component with id ${extensionId}`); + + if (showConfirmationModal) { + const confirm = await showConfirmation(showConfirmationModal); + + if (!confirm) { + return; + } + } reportEvent(Events.PAGE_EDITOR_REMOVE, { sessionId, @@ -81,59 +124,6 @@ function _useRemoveModComponent(): (extensionId: UUID) => Promise { }); } }, - [dispatch, sessionId], + [dispatch, sessionId, showConfirmation], ); } - -export const useDeactivateModComponent = (): (( - useRemoveConfig: Config, -) => Promise) => { - const removeModComponent = _useRemoveModComponent(); - const { showConfirmation } = useModals(); - - return useCallback( - async ({ extensionId, shouldShowConfirmation = true }) => { - if (shouldShowConfirmation) { - const confirm = await showConfirmation({ - title: "Deactivate Mod?", - message: - "This action will deactivate the mod and remove it from the Page Editor. You can reactivate or delete mods from the PixieBrix Extension Console.", - submitCaption: "Deactivate", - }); - - if (!confirm) { - return; - } - } - - await removeModComponent(extensionId); - }, - [removeModComponent, showConfirmation], - ); -}; - -export const useDeleteModComponent = (): (( - useRemoveConfig: Config, -) => Promise) => { - const removeModComponent = _useRemoveModComponent(); - const { showConfirmation } = useModals(); - - return useCallback( - async ({ extensionId, shouldShowConfirmation = true }) => { - if (shouldShowConfirmation) { - const confirm = await showConfirmation({ - title: "Delete starter brick?", - message: "This action cannot be undone.", - submitCaption: "Delete", - }); - - if (!confirm) { - return; - } - } - - await removeModComponent(extensionId); - }, - [removeModComponent, showConfirmation], - ); -}; diff --git a/src/pageEditor/hooks/useSaveExtension.ts b/src/pageEditor/hooks/useSaveStandaloneModComponent.ts similarity index 95% rename from src/pageEditor/hooks/useSaveExtension.ts rename to src/pageEditor/hooks/useSaveStandaloneModComponent.ts index 545e5de38c..609f1f70f1 100644 --- a/src/pageEditor/hooks/useSaveExtension.ts +++ b/src/pageEditor/hooks/useSaveStandaloneModComponent.ts @@ -29,7 +29,7 @@ type ExtensionSaver = { isSaving: boolean; }; -function useSaveExtension(): ExtensionSaver { +function useSaveStandaloneModComponent(): ExtensionSaver { const [isSaving, setIsSaving] = useState(false); const create = useUpsertFormElement(); const sessionId = useSelector(selectSessionId); @@ -67,4 +67,4 @@ function useSaveExtension(): ExtensionSaver { }; } -export default useSaveExtension; +export default useSaveStandaloneModComponent; diff --git a/src/pageEditor/sidebar/DynamicModComponentListItem.tsx b/src/pageEditor/sidebar/DynamicModComponentListItem.tsx index c5720bd42a..b963bf1fdb 100644 --- a/src/pageEditor/sidebar/DynamicModComponentListItem.tsx +++ b/src/pageEditor/sidebar/DynamicModComponentListItem.tsx @@ -44,13 +44,16 @@ import { selectElementIsDirty, } from "@/pageEditor/slices/editorSelectors"; import ActionMenu from "@/pageEditor/sidebar/ActionMenu"; -import useSaveExtension from "@/pageEditor/hooks/useSaveExtension"; +import useSaveStandaloneModComponent from "@/pageEditor/hooks/useSaveStandaloneModComponent"; import useResetExtension from "@/pageEditor/hooks/useResetExtension"; import { - useDeactivateModComponent, - useDeleteModComponent, -} from "@/pageEditor/hooks/useRemoveModComponent"; + useRemoveModComponentFromStorage, + DEACTIVATE_MOD_MODAL_PROPS, + DELETE_STANDALONE_MOD_COMPONENT_MODAL_PROPS, + DELETE_STARTER_BRICK_MODAL_PROPS, +} from "@/pageEditor/hooks/useRemoveModComponentFromStorage"; import useSaveRecipe from "@/pageEditor/hooks/useSaveRecipe"; +import { selectIsModComponentSavedOnCloud } from "@/store/extensionsSelectors"; type DynamicModComponentListItemProps = { modComponentFormState: ModComponentFormState; @@ -80,7 +83,10 @@ const DynamicModComponentListItem: React.FunctionComponent< const isRelativeOfActiveListItem = !isActive && (isChildOfActiveListItem || isSiblingOfActiveListItem); const isDirty = useSelector(selectElementIsDirty(modComponentFormState.uuid)); - + const isSavedOnCloud = useSelector( + selectIsModComponentSavedOnCloud(modComponentFormState.uuid), + ); + const removeModComponentFromStorage = useRemoveModComponentFromStorage(); const isButton = modComponentFormState.type === "menuItem"; const showOverlay = useCallback(async (uuid: UUID) => { @@ -91,38 +97,44 @@ const DynamicModComponentListItem: React.FunctionComponent< await disableOverlay(thisTab); }, []); - const { save: saveExtension, isSaving: isSavingExtension } = - useSaveExtension(); + const { + save: saveStandaloneModComponent, + isSaving: isSavingStandaloneModComponent, + } = useSaveStandaloneModComponent(); const resetExtension = useResetExtension(); - - const deleteModComponent = useDeleteModComponent(); - const deactivateStandaloneMod = useDeactivateModComponent(); const { save: saveRecipe, isSaving: isSavingRecipe } = useSaveRecipe(); + const deleteModComponent = async () => + removeModComponentFromStorage({ + extensionId: modComponentFormState.uuid, + showConfirmationModal: modId + ? DELETE_STARTER_BRICK_MODAL_PROPS + : DELETE_STANDALONE_MOD_COMPONENT_MODAL_PROPS, + }); + const deactivateModComponent = async () => + removeModComponentFromStorage({ + extensionId: modComponentFormState.uuid, + showConfirmationModal: DEACTIVATE_MOD_MODAL_PROPS, + }); + const onSave = async () => { if (modComponentFormState.recipe) { await saveRecipe(modComponentFormState.recipe?.id); } else { - await saveExtension(modComponentFormState); + await saveStandaloneModComponent(modComponentFormState); } }; const isSaving = modComponentFormState.recipe ? isSavingRecipe - : isSavingExtension; + : isSavingStandaloneModComponent; const onReset = async () => resetExtension({ extensionId: modComponentFormState.uuid }); - const onDelete = modId - ? async () => - deleteModComponent({ extensionId: modComponentFormState.uuid }) - : undefined; + const onDelete = modId || !isSavedOnCloud ? deleteModComponent : undefined; - const onDeactivate = onDelete - ? undefined - : async () => - deactivateStandaloneMod({ extensionId: modComponentFormState.uuid }); + const onDeactivate = onDelete ? undefined : deactivateModComponent; const onClone = async () => { dispatch(actions.cloneActiveExtension()); diff --git a/src/pageEditor/sidebar/modals/AddToRecipeModal.tsx b/src/pageEditor/sidebar/modals/AddToRecipeModal.tsx index adc10ec2d0..f450fb2b93 100644 --- a/src/pageEditor/sidebar/modals/AddToRecipeModal.tsx +++ b/src/pageEditor/sidebar/modals/AddToRecipeModal.tsx @@ -35,10 +35,10 @@ import Form, { import { object, string } from "yup"; import RadioItemListWidget from "@/components/form/widgets/radioItemList/RadioItemListWidget"; import { type RadioItem } from "@/components/form/widgets/radioItemList/radioItemListWidgetTypes"; -import { useDeactivateModComponent } from "@/pageEditor/hooks/useRemoveModComponent"; import { isSingleObjectBadRequestError } from "@/errors/networkErrorHelpers"; import { type RegistryId } from "@/types/registryTypes"; import { type ModComponentBase } from "@/types/modComponentTypes"; +import { useRemoveModComponentFromStorage } from "@/pageEditor/hooks/useRemoveModComponentFromStorage"; type FormState = { recipeId: RegistryId; @@ -63,7 +63,7 @@ const AddToRecipeModal: React.FC = () => { ); const recipeMetadatas = useSelector(selectInstalledRecipeMetadatas); const activeElement = useSelector(selectActiveElement); - const deactivateStandaloneMod = useDeactivateModComponent(); + const removeModComponentFromStorage = useRemoveModComponentFromStorage(); const recipeMetadataById = useMemo(() => { const result: Record = {}; @@ -104,9 +104,8 @@ const AddToRecipeModal: React.FC = () => { }), ); if (!keepLocalCopy) { - await deactivateStandaloneMod({ + await removeModComponentFromStorage({ extensionId: elementId, - shouldShowConfirmation: false, }); } diff --git a/src/pageEditor/sidebar/modals/CreateRecipeModal.tsx b/src/pageEditor/sidebar/modals/CreateRecipeModal.tsx index 28b7379225..3dbeae0ca1 100644 --- a/src/pageEditor/sidebar/modals/CreateRecipeModal.tsx +++ b/src/pageEditor/sidebar/modals/CreateRecipeModal.tsx @@ -62,7 +62,7 @@ import { inferConfiguredModIntegrations, inferRecipeOptions, } from "@/store/extensionsUtils"; -import { useDeactivateModComponent } from "@/pageEditor/hooks/useRemoveModComponent"; +import { useRemoveModComponentFromStorage } from "@/pageEditor/hooks/useRemoveModComponentFromStorage"; import useDeactivateMod from "@/pageEditor/hooks/useDeactivateMod"; import RegistryIdWidget from "@/components/form/widgets/RegistryIdWidget"; import { isSingleObjectBadRequestError } from "@/errors/networkErrorHelpers"; @@ -108,7 +108,7 @@ function useSaveCallbacks({ const dispatch = useDispatch(); const [createRecipe] = useCreateRecipeMutation(); const createExtension = useUpsertFormElement(); - const deactivateStandaloneMod = useDeactivateModComponent(); + const removeModComponentFromStorage = useRemoveModComponentFromStorage(); const deactivateMod = useDeactivateMod(); const editorFormElements = useSelector(selectElements); @@ -159,9 +159,8 @@ function useSaveCallbacks({ modId: newRecipe.metadata.id, }); if (!keepLocalCopy) { - await deactivateStandaloneMod({ + await removeModComponentFromStorage({ extensionId: activeElement.uuid, - shouldShowConfirmation: false, }); } @@ -176,7 +175,7 @@ function useSaveCallbacks({ createRecipe, dispatch, keepLocalCopy, - deactivateStandaloneMod, + removeModComponentFromStorage, ], ); diff --git a/src/store/extensionsSelectors.ts b/src/store/extensionsSelectors.ts index 1c626d08de..57c165a0fc 100644 --- a/src/store/extensionsSelectors.ts +++ b/src/store/extensionsSelectors.ts @@ -20,6 +20,7 @@ import { createSelector } from "@reduxjs/toolkit"; import { type UnresolvedModComponent } from "@/types/modComponentTypes"; import { type RegistryId } from "@/types/registryTypes"; import { isEmpty } from "lodash"; +import { type UUID } from "@/types/stringTypes"; export function selectExtensions({ options, @@ -34,6 +35,17 @@ export function selectExtensions({ return options.extensions; } +const isModComponentSavedOnCloudSelector = createSelector( + selectExtensions, + (state: ModComponentsRootState, modComponentId: UUID) => modComponentId, + (extensions, modComponentId) => + extensions.some((extension) => extension.id === modComponentId), +); + +export const selectIsModComponentSavedOnCloud = + (modComponentId: UUID) => (state: ModComponentsRootState) => + isModComponentSavedOnCloudSelector(state, modComponentId); + const extensionsForRecipeSelector = createSelector( selectExtensions, (state: ModComponentsRootState, recipeId: RegistryId) => recipeId,