Hook cleanup

applications/browser-extension/src/pageEditor/hooks/useSaveMod.test.ts

@@ -15,7 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 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);
-  });
-});

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<ModDefinition>,
-): 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,15 +94,19 @@ function useSaveMod(): (modId: RegistryId) => Promise<void> {
         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;
       }
 
       dispatch(
         editorActions.showSaveModVersionModal({
-          modId: sourceModId,
+          packageId,
           sourceModDefinition,
         }),
       );

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<AppDispatch>();
   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
       />

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<ModDefinition>,
         },

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;