#8860: show activated mod metadata in mod metadata form (#9026)

src/components/AsyncStateGate.tsx

@@ -21,7 +21,6 @@ import { getErrorMessage } from "@/errors/errorHelpers";
 import { type AsyncState, type FetchableAsyncState } from "@/types/sliceTypes";
 import { Button } from "react-bootstrap";
 import { isFetchableAsyncState } from "@/utils/asyncStateUtils";
-import { assertNotNullish } from "@/utils/nullishUtils";
 
 /**
  *  A standard error display for use with AsyncStateGate
@@ -96,8 +95,9 @@ function AsyncStateGate<Data>(
     throw error;
   }
 
-  assertNotNullish(data, "data should be defined"); // Type-only check
-  return <>{children({ data })}</>;
+  // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- at this point, the data can only be null/undefined if Data is explicitly set to be nullable
+  const dataResult = data as Data;
+  return <>{children({ data: dataResult })}</>;
 }
 
 export default AsyncStateGate;

src/pageEditor/tabs/MatchRulesSection.test.tsx

@@ -50,7 +50,6 @@ describe("MatchRulesSection", () => {
           ],
           selectors: ["#foo"],
         }),
-        setupRedux(dispatch) {},
       }).asFragment(),
     ).toMatchSnapshot();
   });

src/pageEditor/tabs/modMetadata/ModMetadataEditor.test.tsx

@@ -0,0 +1,109 @@
+/*
+ * 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 <http://www.gnu.org/licenses/>.
+ */
+
+import { render } from "@/pageEditor/testHelpers";
+import React from "react";
+import ModMetadataEditor from "@/pageEditor/tabs/modMetadata/ModMetadataEditor";
+import { formStateFactory } from "@/testUtils/factories/pageEditorFactories";
+import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice";
+import modComponentSlice from "@/store/modComponents/modComponentSlice";
+import {
+  activatedModComponentFactory,
+  modMetadataFactory,
+} from "@/testUtils/factories/modComponentFactories";
+import { type ModMetadata } from "@/types/modComponentTypes";
+import { screen } from "@testing-library/react";
+import { modDefinitionFactory } from "@/testUtils/factories/modDefinitionFactories";
+import { waitForEffect } from "@/testUtils/testHelpers";
+import { useOptionalModDefinition } from "@/modDefinitions/modDefinitionHooks";
+import { valueToAsyncState } from "@/utils/asyncStateUtils";
+import { type SemVerString } from "@/types/registryTypes";
+
+// :shrug: ideally would mock the definitions returned from the server. But that's more complicated than it's worth,
+// because you can't just mock the /api/registry/bricks/ endpoint due to the way our tests mock the background worker
+jest.mock("@/modDefinitions/modDefinitionHooks");
+
+const useOptionalModDefinitionMock = jest.mocked(useOptionalModDefinition);
+
+beforeEach(() => {
+  useOptionalModDefinitionMock.mockReset();
+});
+
+describe("ModMetadataEditor", () => {
+  it("should render latest version", async () => {
+    const modMetadata: ModMetadata = modMetadataFactory();
+    const formState = formStateFactory({ formStateConfig: { modMetadata } });
+
+    const modComponent = activatedModComponentFactory({
+      _recipe: modMetadata,
+    });
+
+    useOptionalModDefinitionMock.mockReturnValue(
+      valueToAsyncState(modDefinitionFactory({ metadata: modMetadata })),
+    );
+
+    render(<ModMetadataEditor />, {
+      setupRedux(dispatch) {
+        dispatch(
+          modComponentSlice.actions.UNSAFE_setModComponents([modComponent]),
+        );
+        dispatch(editorActions.addModComponentFormState(formState));
+        dispatch(editorActions.setActiveModId(formState.modMetadata!.id));
+      },
+    });
+
+    await waitForEffect();
+
+    expect(screen.getByLabelText("Name")).toHaveValue(modMetadata.name);
+    expect(screen.getByLabelText("Version")).toHaveValue(modMetadata.version);
+  });
+
+  it("should render if newer version", async () => {
+    const modMetadata: ModMetadata = modMetadataFactory();
+
+    const formState = formStateFactory({ formStateConfig: { modMetadata } });
+
+    const modComponent = activatedModComponentFactory({
+      _recipe: modMetadata,
+    });
+
+    useOptionalModDefinitionMock.mockReturnValue(
+      valueToAsyncState(
+        modDefinitionFactory({
+          metadata: {
+            ...modMetadata,
+            version: "999.999.999" as SemVerString,
+          },
+        }),
+      ),
+    );
+
+    render(<ModMetadataEditor />, {
+      setupRedux(dispatch) {
+        dispatch(
+          modComponentSlice.actions.UNSAFE_setModComponents([modComponent]),
+        );
+        dispatch(editorActions.addModComponentFormState(formState));
+        dispatch(editorActions.setActiveModId(formState.modMetadata!.id));
+      },
+    });
+
+    expect(screen.getByText("re-activate the mod")).toBeInTheDocument();
+    // Should be the version of the activated mod component, not the latest version
+    expect(screen.getByLabelText("Version")).toHaveValue(modMetadata.version);
+  });
+});

src/pageEditor/tabs/modMetadata/ModMetadataEditor.tsx

@@ -22,8 +22,6 @@ import {
   selectDirtyMetadataForModId,
 } from "@/pageEditor/store/editor/editorSelectors";
 import { Card, Container } from "react-bootstrap";
-import Loader from "@/components/Loader";
-import { getErrorMessage } from "@/errors/errorHelpers";
 import { actions } from "@/pageEditor/store/editor/editorSlice";
 import ErrorBoundary from "@/components/ErrorBoundary";
 import Effect from "@/components/Effect";
@@ -44,6 +42,8 @@ import cx from "classnames";
 import { assertNotNullish } from "@/utils/nullishUtils";
 import { type RegistryId } from "@/types/registryTypes";
 import { getActivateModHashRoute } from "@/extensionConsole/shared/routeHelpers";
+import { pick } from "lodash";
+import AsyncStateGate from "@/components/AsyncStateGate";
 
 // TODO: This should be yup.SchemaOf<ModMetadataFormState> but we can't set the `id` property to `RegistryId`
 // see: https://github.com/jquense/yup/issues/1183#issuecomment-749186432
@@ -99,11 +99,8 @@ const ModMetadataEditor: React.VoidFunctionComponent = () => {
 
   assertNotNullish(modId, "No active mod id");
 
-  const {
-    data: modDefinition,
-    isFetching,
-    error,
-  } = useOptionalModDefinition(modId);
+  const modDefinitionQuery = useOptionalModDefinition(modId);
+  const modDefinition = modDefinitionQuery.data;
 
   // Select a single mod component for the mod to check the activated version.
   // We rely on the assumption that every component in the mod has the same version.
@@ -117,15 +114,14 @@ const ModMetadataEditor: React.VoidFunctionComponent = () => {
     lt(activatedModVersion, latestModVersion);
 
   const dirtyMetadata = useSelector(selectDirtyMetadataForModId(modId));
-  const savedMetadata = modDefinition?.metadata;
-  const metadata = dirtyMetadata ?? savedMetadata;
+  // Prefer the metadata from the activated mod component
+  const currentMetadata =
+    dirtyMetadata ?? modDefinitionComponent?._recipe ?? modDefinition?.metadata;
 
-  const initialFormState: Partial<ModMetadataFormState> = {
-    id: metadata?.id,
-    name: metadata?.name,
-    version: metadata?.version,
-    description: metadata?.description,
-  };
+  const initialFormState: Partial<ModMetadataFormState> = pick(
+    currentMetadata,
+    ["id", "name", "version", "description"],
+  );
 
   const dispatch = useDispatch();
   const updateRedux = useCallback(
@@ -135,18 +131,6 @@ const ModMetadataEditor: React.VoidFunctionComponent = () => {
     [dispatch],
   );
 
-  if (isFetching || error) {
-    return (
-      <Container>
-        {isFetching ? (
-          <Loader />
-        ) : (
-          <div className="text-danger">{getErrorMessage(error)}</div>
-        )}
-      </Container>
-    );
-  }
-
   const renderBody: RenderBody = ({ values }) => (
     <IntegrationsSliceModIntegrationsContextAdapter>
       <Effect values={values} onChange={updateRedux} delayMillis={100} />
@@ -190,19 +174,23 @@ const ModMetadataEditor: React.VoidFunctionComponent = () => {
 
   return (
     <Container className={cx(styles.root, "max-750 ml-0")}>
-      <ErrorBoundary>
-        <Form
-          validationSchema={editModSchema}
-          initialValues={initialFormState}
-          onSubmit={() => {
-            console.error(
-              "The form's submit should not be called to save mod metadata. Use 'saveMod' from 'useSaveMod' instead.",
-            );
-          }}
-          renderBody={renderBody}
-          renderSubmit={() => null}
-        />
-      </ErrorBoundary>
+      <AsyncStateGate state={modDefinitionQuery}>
+        {() => (
+          <ErrorBoundary>
+            <Form
+              validationSchema={editModSchema}
+              initialValues={initialFormState}
+              onSubmit={() => {
+                console.error(
+                  "The form's submit should not be called to save mod metadata. Use 'saveMod' from 'useSaveMod' instead.",
+                );
+              }}
+              renderBody={renderBody}
+              renderSubmit={() => null}
+            />
+          </ErrorBoundary>
+        )}
+      </AsyncStateGate>
     </Container>
   );
 };

src/tsconfig.strictNullChecks.json

@@ -1426,6 +1426,7 @@
     "./pageEditor/tabs/logs/NavItemBadge.tsx",
     "./pageEditor/tabs/logs/useLogsBadgeState.ts",
     "./pageEditor/tabs/modMetadata/ModMetadataEditor.tsx",
+    "./pageEditor/tabs/modMetadata/ModMetadataEditor.test.tsx",
     "./pageEditor/tabs/modOptionsDefinitions/ModOptionsDefinitionEditor.test.tsx",
     "./pageEditor/tabs/modOptionsDefinitions/ModOptionsDefinitionEditor.tsx",
     "./pageEditor/tabs/modOptionsValues/ModOptionsValuesEditor.test.tsx",

src/types/modComponentTypes.ts

@@ -44,9 +44,10 @@ import { isRegistryId, isUUID } from "@/types/helpers";
  *
  * @see optionsSlice
  * @see ModComponentBase._recipe
+ * @see Metadata
  */
-// Don't export -- the use is clearer if it's always written as ModComponentBase[_recipe] property
-// XXX: Usage may be clearer, but the ergonomics of (ModMetadata | undefined) are terrible to handle with strict null checks
+// XXX: previously we didn't export because the usage was clearer as ModComponentBase[_recipe]. However, the ergonomics
+// of (ModMetadata | undefined) were bad to handle with strict null checks
 export type ModMetadata = Metadata & {
   /**
    * `undefined` for mods that were activated prior to the field being added