From 652ce8914bc166201b0cd8f1da1fd3d79078dd06 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Wed, 13 Nov 2024 10:52:13 -0500 Subject: [PATCH] Hook cleanup --- .../src/pageEditor/hooks/useSaveMod.test.ts | 30 +------------------ .../src/pageEditor/hooks/useSaveMod.ts | 23 +++++--------- .../modals/SaveModVersionModal.tsx | 24 +++++---------- .../pageEditor/store/editor/editorSlice.ts | 6 ++-- .../store/editor/pageEditorTypes/index.ts | 2 +- 5 files changed, 20 insertions(+), 65 deletions(-) diff --git a/applications/browser-extension/src/pageEditor/hooks/useSaveMod.test.ts b/applications/browser-extension/src/pageEditor/hooks/useSaveMod.test.ts index e1357f5a00..ccd55aa9bb 100644 --- a/applications/browser-extension/src/pageEditor/hooks/useSaveMod.test.ts +++ b/applications/browser-extension/src/pageEditor/hooks/useSaveMod.test.ts @@ -15,7 +15,7 @@ * along with this program. If not, see . */ import { renderHook } from "@/pageEditor/testHelpers"; -import useSaveMod, { isModEditable } from "@/pageEditor/hooks/useSaveMod"; +import useSaveMod from "@/pageEditor/hooks/useSaveMod"; import { validateRegistryId } from "@/types/helpers"; import { appApiMock } from "@/testUtils/appApiMock"; import { editablePackageMetadataFactory } from "@/testUtils/factories/registryFactories"; @@ -43,7 +43,6 @@ import { timestampFactory } from "@/testUtils/factories/stringFactories"; import { propertiesToSchema } from "@/utils/schemaUtils"; import { act } from "@testing-library/react"; import { waitForEffect } from "@/testUtils/testHelpers"; -import { array } from "cooky-cutter"; const modId = validateRegistryId("@test/mod"); @@ -336,30 +335,3 @@ describe("useSaveMod", () => { expect(notify.error).not.toHaveBeenCalled(); }); }); - -describe("isModEditable", () => { - test("returns true if mod is in editable packages", () => { - const mod = defaultModDefinitionFactory(); - const editablePackages = [ - editablePackageMetadataFactory(), - editablePackageMetadataFactory({ - name: mod.metadata.id, - }), - ]; - - expect(isModEditable(editablePackages, mod)).toBe(true); - }); - - test("returns false if mod is not in editable packages", () => { - const mod = defaultModDefinitionFactory(); - const editablePackages = array(editablePackageMetadataFactory, 1)(); - - expect(isModEditable(editablePackages, mod)).toBe(false); - }); - - test("returns false if mod is null", () => { - const editablePackages = array(editablePackageMetadataFactory, 1)(); - - expect(isModEditable(editablePackages, null)).toBe(false); - }); -}); diff --git a/applications/browser-extension/src/pageEditor/hooks/useSaveMod.ts b/applications/browser-extension/src/pageEditor/hooks/useSaveMod.ts index c4ec0c64d7..80f2652ed6 100644 --- a/applications/browser-extension/src/pageEditor/hooks/useSaveMod.ts +++ b/applications/browser-extension/src/pageEditor/hooks/useSaveMod.ts @@ -22,23 +22,10 @@ import notify from "@/utils/notify"; import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; import { type RegistryId } from "@/types/registryTypes"; import { useAllModDefinitions } from "@/modDefinitions/modDefinitionHooks"; -import type { EditablePackageMetadata } from "@/types/contract"; -import type { ModDefinition } from "@/types/modDefinitionTypes"; -import { assertNotNullish, type Nullishable } from "@/utils/nullishUtils"; +import { assertNotNullish } from "@/utils/nullishUtils"; import { isInnerDefinitionRegistryId } from "@/types/helpers"; import { type AppDispatch } from "@/pageEditor/store/store"; -/** @internal */ -export function isModEditable( - editablePackages: EditablePackageMetadata[], - // Nullishable because the user might lose access to the mod while they were editing it (the mod or a mod component) - // See https://github.com/pixiebrix/pixiebrix-extension/issues/2813 - modDefinition: Nullishable, -): boolean { - const modId = modDefinition?.metadata.id; - return modId != null && editablePackages.some((x) => x.name === modId); -} - /** * Returns a callback to show the appropriate save modal based on whether: * - The mod have been saved yet @@ -107,7 +94,11 @@ function useSaveMod(): (modId: RegistryId) => Promise { return; } - if (!isModEditable(editablePackages, sourceModDefinition)) { + // Registry edit endpoints use package surrogate id + const packageId = editablePackages.find((x) => x.name === sourceModId) + ?.id; + + if (packageId == null) { dispatch(editorActions.showSaveAsNewModModal()); return; @@ -115,7 +106,7 @@ function useSaveMod(): (modId: RegistryId) => Promise { dispatch( editorActions.showSaveModVersionModal({ - modId: sourceModId, + packageId, sourceModDefinition, }), ); diff --git a/applications/browser-extension/src/pageEditor/modListingPanel/modals/SaveModVersionModal.tsx b/applications/browser-extension/src/pageEditor/modListingPanel/modals/SaveModVersionModal.tsx index 55d1b20c11..195592b81a 100644 --- a/applications/browser-extension/src/pageEditor/modListingPanel/modals/SaveModVersionModal.tsx +++ b/applications/browser-extension/src/pageEditor/modListingPanel/modals/SaveModVersionModal.tsx @@ -58,10 +58,7 @@ import updateReduxForSavedModDefinition from "@/pageEditor/hooks/updateReduxForS import reportEvent from "@/telemetry/reportEvent"; import { Events } from "@/telemetry/events"; import { isNullOrBlank } from "@/utils/stringUtils"; -import { - useGetEditablePackagesQuery, - useUpdateModDefinitionMutation, -} from "@/data/service/api"; +import { useUpdateModDefinitionMutation } from "@/data/service/api"; import { type AppDispatch } from "@/pageEditor/store/store"; type SaveVersionFormValues = { @@ -120,7 +117,6 @@ const SaveModVersionModalBody: React.VFC<{ onHide: () => void }> = ({ const dispatch = useDispatch(); const isMounted = useIsMounted(); - const { data: editablePackages = [] } = useGetEditablePackagesQuery(); const [updateModDefinitionOnServer] = useUpdateModDefinitionMutation(); const getDraftModComponentsForMod = useSelector( @@ -136,7 +132,8 @@ const SaveModVersionModalBody: React.VFC<{ onHide: () => void }> = ({ modalData, "SaveModVersionModalBody rendered without modal data set", ); - const { modId, sourceModDefinition } = modalData; + const { packageId, sourceModDefinition } = modalData; + const modId = sourceModDefinition.metadata.id; const formSchema = useFormSchema(modId); const initialFormState = useInitialFormState(modId); @@ -161,16 +158,6 @@ const SaveModVersionModalBody: React.VFC<{ onHide: () => void }> = ({ dirtyModVariablesDefinition: draftModState.variablesDefinition, }); - const packageId = editablePackages.find( - // Bricks endpoint uses "name" instead of id - (x) => x.name === modId, - )?.id; - - assertNotNullish( - packageId, - "Package ID is required to upsert a mod definition", - ); - const upsertResponse = await updateModDefinitionOnServer({ packageId, modDefinition: unsavedModDefinition, @@ -193,6 +180,10 @@ const SaveModVersionModalBody: React.VFC<{ onHide: () => void }> = ({ }); onHide(); + + notify.success({ + message: "Saved mod", + }); } catch (error) { if (isSpecificError(error, DataIntegrityError)) { dispatch(editorActions.showSaveDataIntegrityErrorModal()); @@ -229,6 +220,7 @@ const SaveModVersionModalBody: React.VFC<{ onHide: () => void }> = ({ label="Message" id="save-mod-modal-message" as="textarea" + placeholder="Fix bug XYZ... Added features XYZ..." description="A short description of the changes to the mod. If you provide a message, you must increment the version." showUntouchedErrors /> diff --git a/applications/browser-extension/src/pageEditor/store/editor/editorSlice.ts b/applications/browser-extension/src/pageEditor/store/editor/editorSlice.ts index 9d9e6302a4..dd4b1a098b 100644 --- a/applications/browser-extension/src/pageEditor/store/editor/editorSlice.ts +++ b/applications/browser-extension/src/pageEditor/store/editor/editorSlice.ts @@ -431,16 +431,16 @@ export const editorSlice = createSlice({ { payload, }: PayloadAction<{ - modId: RegistryId; + packageId: UUID; sourceModDefinition: ModDefinition; }>, ) { - const { modId, sourceModDefinition } = payload; + const { packageId, sourceModDefinition } = payload; state.visibleModal = { type: ModalKey.SAVE_MOD_VERSION, data: { - modId, + packageId, // Cast required due to draft/immutable shenanigans with RJSF uiSchema sourceModDefinition: sourceModDefinition as Draft, }, diff --git a/applications/browser-extension/src/pageEditor/store/editor/pageEditorTypes/index.ts b/applications/browser-extension/src/pageEditor/store/editor/pageEditorTypes/index.ts index a70f9f38a3..9991f573b8 100644 --- a/applications/browser-extension/src/pageEditor/store/editor/pageEditorTypes/index.ts +++ b/applications/browser-extension/src/pageEditor/store/editor/pageEditorTypes/index.ts @@ -86,7 +86,7 @@ export type ModalDefinition = | { type: ModalKey.SAVE_AS_NEW_MOD; data: EmptyObject } | { type: ModalKey.SAVE_MOD_VERSION; - data: { modId: RegistryId; sourceModDefinition: ModDefinition }; + data: { packageId: UUID; sourceModDefinition: ModDefinition }; } | { type: ModalKey.CREATE_MOD;