Fix saveNewMod

end-to-end-tests/pageObjects/pageEditor/pageEditorPage.ts

@@ -138,9 +138,9 @@ export class PageEditorPage extends BasePageObject {
   /**
    * Save a new mod with the given name and optional description.
    *
-   * @param modName the name to use when saving the new mod
-   * @param description the optional description to use when saving the new mod
-   * @returns the modId of the saved mod
+   * @param currentModName the current name (not registry id) of the mod to save
+   * @param descriptionOverride the optional description override
+   * @returns the RegistryId of the saved mod
    */
   @ModifiesModFormState
   async saveNewMod({
@@ -155,9 +155,10 @@ export class PageEditorPage extends BasePageObject {
     const modListItem =
       this.modListingPanel.getModListItemByName(currentModName);
     await modListItem.select();
+    // Expect the mod metadata editor to be showing form for a mod that's never been saved before
     await expect(
-      this.modEditorPane.editMetadataTabPanel.getByText(
-        "Save the mod to assign an id",
+      this.modEditorPane.editMetadataTabPanel.getByPlaceholder(
+        "Save the mod to assign a Mod ID",
       ),
     ).toBeVisible();
     // eslint-disable-next-line playwright/no-wait-for-timeout -- The save button re-mounts several times so we need a slight delay here before playwright clicks
@@ -169,14 +170,17 @@ export class PageEditorPage extends BasePageObject {
     await expect(saveNewModModal).toBeVisible();
     await expect(saveNewModModal.getByText("Save new mod")).toBeVisible();
 
-    // Add a random uuid to the mod id to prevent test collisions
-    const registryIdInput = saveNewModModal.getByLabel("Mod ID");
+    // // Can't use getByLabel to target because the field is composed of multiple widgets
+    const registryIdInput = saveNewModModal.getByTestId("registryId-id-id");
     const currentId = await registryIdInput.inputValue();
+    // Add a random uuid to the mod id to prevent test collisions
     await registryIdInput.fill(`${currentId}-${uuidv4()}`);
 
     if (descriptionOverride) {
       // Update the mod description
-      const descriptionInput = saveNewModModal.getByLabel("Description");
+      // FIXME: determine why getByLabel is not working for the description field
+      // const descriptionInput = saveNewModModal.getByLabel("Description");
+      const descriptionInput = saveNewModModal.locator("#description");
       await descriptionInput.fill(descriptionOverride);
     }
 

src/auth/authSelectors.ts

@@ -16,6 +16,15 @@
  */
 
 import { type AuthRootState } from "./authTypes";
+import { LegacyUserRole } from "@/data/model/UserRole";
+import { compact } from "lodash";
+import { createSelector } from "@reduxjs/toolkit";
+
+const editorRoles = new Set<number>([
+  LegacyUserRole.admin,
+  LegacyUserRole.developer,
+  LegacyUserRole.manager,
+]);
 
 export const selectAuth = (state: AuthRootState) => state.auth;
 export const selectIsLoggedIn = (state: AuthRootState) =>
@@ -27,3 +36,22 @@ export const selectOrganizations = (state: AuthRootState) =>
   selectAuth(state).organizations;
 export const selectOrganization = (state: AuthRootState) =>
   selectAuth(state).organization;
+
+export const selectEditableScopes = createSelector(
+  selectAuth,
+  (state: AuthRootState["auth"]) => {
+    const { scope: userScope, organizations } = state;
+
+    const organizationScopes: string[] = compact(
+      organizations.map(({ scope, role }) => {
+        if (scope && editorRoles.has(role)) {
+          return scope;
+        }
+
+        return null;
+      }),
+    );
+
+    return [userScope, ...organizationScopes].filter((x) => x != null);
+  },
+);

src/components/form/widgets/RegistryIdWidget.tsx

@@ -18,29 +18,20 @@
 import { useField } from "formik";
 import React from "react";
 import { useSelector } from "react-redux";
-import { selectAuth } from "@/auth/authSelectors";
+import { selectEditableScopes, selectScope } from "@/auth/authSelectors";
 import SelectWidget, {
   type Option,
   type SelectWidgetOnChange,
 } from "@/components/form/widgets/SelectWidget";
-import { compact } from "lodash";
 import { type RegistryId } from "@/types/registryTypes";
 // eslint-disable-next-line no-restricted-imports -- TODO: Fix over time
 import { Form } from "react-bootstrap";
 import styles from "./RegistryIdWidget.module.scss";
 import { type StylesConfig } from "react-select";
-import { LegacyUserRole } from "@/data/model/UserRole";
-
 import { getScopeAndId } from "@/utils/registryUtils";
 import useAsyncEffect from "use-async-effect";
 import { assertNotNullish } from "@/utils/nullishUtils";
 
-const editorRoles = new Set<number>([
-  LegacyUserRole.admin,
-  LegacyUserRole.developer,
-  LegacyUserRole.manager,
-]);
-
 const emptyObject = {} as const;
 
 const RegistryIdWidget: React.VFC<{
@@ -49,23 +40,15 @@ const RegistryIdWidget: React.VFC<{
   selectStyles?: StylesConfig;
 }> = ({ name, id, selectStyles = emptyObject }) => {
   const [{ value }, , { setValue }] = useField<RegistryId>(name);
-  const { scope: userScope, organizations } = useSelector(selectAuth);
-  // XXX: We should eventually refactor RequireScope to pass the required (non-null) user scope down to children, or set a context value
+  const userScope = useSelector(selectScope);
+  const editableScopes = useSelector(selectEditableScopes);
+
   assertNotNullish(
     userScope,
-    "This widget should only be used inside RequireScope, userScope should be defined",
+    "RegistryIdWidget should only be used inside RequireScope",
   );
-  const organizationScopes: string[] = compact(
-    organizations.map(({ scope, role }) => {
-      if (scope && editorRoles.has(role)) {
-        return scope;
-      }
 
-      return null;
-    }),
-  );
-  const scopes: string[] = [userScope, ...organizationScopes];
-  const options: Option[] = scopes.map((scope) => ({
+  const options: Option[] = editableScopes.map((scope) => ({
     label: scope,
     value: scope,
   }));
@@ -101,7 +84,7 @@ const RegistryIdWidget: React.VFC<{
   return (
     <div className={styles.root}>
       <SelectWidget
-        // This doesn't impact formik because these widgets aren't connected to formik directly;
+        // Passing `name` doesn't impact formik because these widgets aren't connected to formik directly;
         // we need it for testing because the react-select element is hard to identify in tests - it
         // doesn't accept a top-level data-testid prop
         name={`${name}-scope`}
@@ -110,14 +93,14 @@ const RegistryIdWidget: React.VFC<{
         onChange={onChangeScope}
         options={options}
         className={styles.select}
-        styles={selectStyles}
       />
       <span> / </span>
       <Form.Control
         id={id}
         value={idValue}
         onChange={onChangeId}
         className={styles.idInput}
+        // Can't use getByLabel to target because the field is composed of multiple widgets
         data-testid={`registryId-${name}-id`}
       />
     </div>