fix: namespace setting when current one is not in results (#3388)

Fixes an issue with projects in personal namespaces where the current namespace would not be selectable. The same issue would also exist for new groups which do not appear on the first page of results. Other fixes: * Fix focus border on the namespace selector * Fix label and input id linking. Now clicking on the label focuses on the select element (can then press up or down to select a namespace).
SwissDataScienceCenter · Nov 4, 2024 · 047ee92 · 047ee92
1 parent 09b3938
commit 047ee92
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 15 deletions.
diff --git a/client/src/features/ProjectPageV2/ProjectPageContent/Settings/ProjectSettings.tsx b/client/src/features/ProjectPageV2/ProjectPageContent/Settings/ProjectSettings.tsx
@@ -117,13 +117,14 @@ function ProjectSettingsEditForm({ project }: ProjectPageSettingsProps) {
     handleSubmit,
     watch,
     register,
+    reset,
   } = useForm<Required<ProjectV2Metadata>>({
     defaultValues: {
-      description: project.description,
+      description: project.description ?? "",
       name: project.name,
       namespace: project.namespace,
       visibility: project.visibility,
-      keywords: project.keywords,
+      keywords: project.keywords ?? [],
     },
   });
   const currentNamespace = watch("namespace");
@@ -133,7 +134,7 @@ function ProjectSettingsEditForm({ project }: ProjectPageSettingsProps) {
   const { notifications } = useContext(AppContext);
   const [areKeywordsDirty, setKeywordsDirty] = useState(false);
 
-  const [updateProject, { isLoading, error, isSuccess }] =
+  const [updateProject, { isLoading, error, isSuccess, data: updatedProject }] =
     usePatchProjectsByProjectIdMutation();
 
   const isUpdating = isLoading;
@@ -150,6 +151,19 @@ function ProjectSettingsEditForm({ project }: ProjectPageSettingsProps) {
     },
     [project, updateProject]
   );
+
+  useEffect(() => {
+    if (isSuccess && updatedProject != null) {
+      reset({
+        description: updatedProject.description ?? "",
+        name: updatedProject.name,
+        namespace: updatedProject.namespace,
+        visibility: updatedProject.visibility,
+        keywords: updatedProject.keywords ?? [],
+      });
+    }
+  }, [isSuccess, reset, updatedProject]);
+
   useEffect(() => {
     if (isSuccess && redirectAfterUpdate) {
       if (notifications && currentName)
@@ -190,6 +204,7 @@ function ProjectSettingsEditForm({ project }: ProjectPageSettingsProps) {
               name="namespace"
               control={control}
               entityName="project"
+              ensureNamespace={project.namespace}
               errors={errors}
             />
           }

diff --git a/...nt/src/features/dataConnectorsV2/components/DataConnectorModal/DataConnectorModalBody.tsx b/...nt/src/features/dataConnectorsV2/components/DataConnectorModal/DataConnectorModalBody.tsx
@@ -325,7 +325,7 @@ export function DataConnectorMount({
       </div>
 
       <div className="mb-3">
-        <Label className="form-label" for="namespace">
+        <Label className="form-label" for="namespace-input">
           Owner
         </Label>
 
@@ -341,6 +341,7 @@ export function DataConnectorMount({
                 className={cx(errors.namespace && "is-invalid")}
                 data-cy={"data-controller-namespace-input"}
                 id="namespace"
+                inputId="namespace-input"
                 onChange={(e) => {
                   field.onChange(e);
                   onFieldValueChange("namespace", e?.slug ?? "");

diff --git a/client/src/features/projectsV2/fields/ProjectNamespaceFormField.tsx b/client/src/features/projectsV2/fields/ProjectNamespaceFormField.tsx
@@ -41,9 +41,11 @@ import {
   projectV2Api,
   useGetNamespacesQuery,
   useLazyGetNamespacesQuery,
+  useGetNamespacesByNamespaceSlugQuery,
 } from "../api/projectV2.enhanced-api";
 import type { GenericFormFieldProps } from "./formField.types";
 import styles from "./ProjectNamespaceFormField.module.scss";
+import { skipToken } from "@reduxjs/toolkit/query";
 
 type ResponseNamespaces = GetNamespacesApiResponse["namespaces"];
 type ResponseNamespace = ResponseNamespaces[number];
@@ -105,6 +107,7 @@ function CustomMenuList({
         {props.children}
         {hasMore && (
           <div className={cx("d-grid")}>
+            {/* TODO: Make this button accessible. At the moment, it cannot be used from keyboard-only navigation. */}
             <Button
               className={cx("rounded-0", "rounded-bottom")}
               color="secondary"
@@ -128,6 +131,7 @@ function CustomMenuList({
 interface NamespaceSelectorProps {
   currentNamespace?: string;
   hasMore?: boolean;
+  inputId: string;
   isFetchingMore?: boolean;
   namespaces: ResponseNamespaces;
   onChange?: (newValue: SingleValue<ResponseNamespace>) => void;
@@ -137,6 +141,7 @@ interface NamespaceSelectorProps {
 function NamespaceSelector({
   currentNamespace,
   hasMore,
+  inputId,
   isFetchingMore,
   namespaces,
   onChange,
@@ -157,6 +162,7 @@ function NamespaceSelector({
 
   return (
     <Select
+      inputId={inputId}
       options={namespaces}
       value={currentValue}
       unstyled
@@ -175,8 +181,13 @@ function NamespaceSelector({
 }
 
 const selectClassNames: ClassNamesConfig<ResponseNamespace, false> = {
-  control: ({ menuIsOpen }) =>
-    cx(menuIsOpen ? "rounded-top" : "rounded", "border", styles.control),
+  control: ({ menuIsOpen, isFocused }) =>
+    cx(
+      menuIsOpen ? "rounded-top" : "rounded",
+      "border",
+      isFocused && "border-primary-subtle",
+      styles.control
+    ),
   dropdownIndicator: () => cx("pe-3"),
   menu: () => cx("bg-white", "rounded-bottom", "border", "border-top-0"),
   menuList: () => cx("d-grid"),
@@ -197,20 +208,26 @@ const selectClassNames: ClassNamesConfig<ResponseNamespace, false> = {
     cx("d-flex", "flex-column", "flex-sm-row", "column-gap-3", "px-3"),
 };
 
+interface ProjectNamespaceFormFieldProps<T extends FieldValues>
+  extends GenericFormFieldProps<T> {
+  ensureNamespace?: string;
+}
+
 export default function ProjectNamespaceFormField<T extends FieldValues>({
   control,
   entityName,
+  ensureNamespace,
   errors,
   name,
-}: GenericFormFieldProps<T>) {
+}: ProjectNamespaceFormFieldProps<T>) {
   // Handle forced refresh
   const dispatch = useAppDispatch();
   const refetch = useCallback(() => {
     dispatch(projectV2Api.util.invalidateTags(["Namespace"]));
   }, [dispatch]);
   return (
     <div className="mb-3">
-      <Label className="form-label" for={`${entityName}-namespace`}>
+      <Label className="form-label" for={`${entityName}-namespace-input`}>
         Namespace
         <RefreshNamespaceButton refresh={refetch} />
       </Label>
@@ -226,7 +243,9 @@ export default function ProjectNamespaceFormField<T extends FieldValues>({
               aria-describedby={`${entityName}NamespaceHelp`}
               className={cx(errors.namespace && "is-invalid")}
               data-cy={`${entityName}-namespace-input`}
+              ensureNamespace={ensureNamespace}
               id={`${entityName}-namespace`}
+              inputId={`${entityName}-namespace-input`}
               onChange={(newValue) => field.onChange(newValue?.slug)}
             />
           );
@@ -251,20 +270,35 @@ export default function ProjectNamespaceFormField<T extends FieldValues>({
 interface ProjectNamespaceControlProps {
   className: string;
   "data-cy": string;
+  ensureNamespace?: string;
   id: string;
+  inputId: string;
   onChange: (newValue: SingleValue<ResponseNamespace>) => void;
   value?: string;
 }
 
-export function ProjectNamespaceControl(props: ProjectNamespaceControlProps) {
-  const { className, id, onChange, value } = props;
-  const dataCy = props["data-cy"];
+export function ProjectNamespaceControl({
+  className,
+  ensureNamespace,
+  id,
+  inputId,
+  onChange,
+  value,
+  "data-cy": dataCy,
+}: ProjectNamespaceControlProps) {
   const {
     data: namespacesFirstPage,
     isError,
     isFetching,
     requestId,
   } = useGetNamespacesQuery({ params: { minimum_role: "editor" } });
+  const {
+    data: specificNamespace,
+    isError: specificNamespaceIsError,
+    requestId: specificNamespaceRequestId,
+  } = useGetNamespacesByNamespaceSlugQuery(
+    ensureNamespace ? { namespaceSlug: ensureNamespace } : skipToken
+  );
 
   const [
     { data: allNamespaces, fetchedPages, hasMore, currentRequestId },
@@ -302,7 +336,7 @@ export function ProjectNamespaceControl(props: ProjectNamespaceControlProps) {
     if (userNamespace != null && !value) {
       onChange(userNamespace);
     }
-  }, [onChange, namespacesFirstPage, value]);
+  }, [namespacesFirstPage, onChange, value]);
 
   useEffect(() => {
     if (namespacesFirstPage == null) {
@@ -344,6 +378,22 @@ export function ProjectNamespaceControl(props: ProjectNamespaceControlProps) {
     currentRequestId,
   ]);
 
+  useEffect(() => {
+    if (specificNamespace == null || allNamespaces == null) {
+      return;
+    }
+    const hasNamespace = allNamespaces.find(
+      ({ slug }) => slug === specificNamespace.slug
+    );
+    if (hasNamespace) {
+      return;
+    }
+    setState((prevState) => {
+      const namespaces = [specificNamespace, ...(prevState.data ?? [])];
+      return { ...prevState, data: namespaces };
+    });
+  }, [allNamespaces, specificNamespace, specificNamespaceRequestId]);
+
   if (isFetching) {
     return (
       <div className={cx(className, "form-control")} id={id}>
@@ -353,7 +403,7 @@ export function ProjectNamespaceControl(props: ProjectNamespaceControlProps) {
     );
   }
 
-  if (!allNamespaces || isError) {
+  if (!allNamespaces || isError || specificNamespaceIsError) {
     return (
       <div className={className} id={id}>
         <ErrorAlert>
@@ -368,6 +418,7 @@ export function ProjectNamespaceControl(props: ProjectNamespaceControlProps) {
       <NamespaceSelector
         currentNamespace={value}
         hasMore={hasMore}
+        inputId={inputId}
         isFetchingMore={namespacesPageResult.isFetching}
         namespaces={allNamespaces}
         onChange={onChange}

diff --git a/tests/cypress/e2e/projectV2.spec.ts b/tests/cypress/e2e/projectV2.spec.ts
@@ -227,7 +227,11 @@ describe("Edit v2 project", () => {
   });
 
   it("changes project namespace", () => {
-    fixtures.readProjectV2().updateProjectV2().listManyNamespaceV2();
+    fixtures
+      .readProjectV2()
+      .updateProjectV2()
+      .listManyNamespaceV2()
+      .readUserV2Namespace();
     cy.getDataCy("dashboard-project-list")
       .contains("a", "test 2 v2-project")
       .should("be.visible")
@@ -237,14 +241,15 @@ describe("Edit v2 project", () => {
     cy.getDataCy("project-settings-edit").should("be.visible").click();
     // Fetch the second page of namespaces
     cy.wait("@listNamespaceV2");
+    cy.wait("@readUserV2Namespace");
     cy.findReactSelectOptions("project-namespace-input", "namespace-select");
     cy.get("button").contains("Fetch more").click();
     // Need to click away so the dropdown option selection works
     cy.getDataCy("project-name-input").click();
     cy.wait("@listNamespaceV2");
     cy.findReactSelectOptions("project-namespace-input", "namespace-select")
       // Pick an element from the second page of results
-      .eq(25)
+      .contains("test-25-group-v2")
       .click();
     fixtures.readProjectV2({
       fixture: "projectV2/update-projectV2-metadata.json",

diff --git a/tests/cypress/fixtures/namespaceV2/namespaceV2-user1.json b/tests/cypress/fixtures/namespaceV2/namespaceV2-user1.json
@@ -0,0 +1,6 @@
+{
+  "id": "AB0134CD43Z3JF9DX88B7XB405N0",
+  "slug": "user1-uuid",
+  "created_by": "user1-uuid",
+  "namespace_kind": "user"
+}
diff --git a/tests/cypress/support/renkulab-fixtures/namespaceV2.ts b/tests/cypress/support/renkulab-fixtures/namespaceV2.ts
@@ -53,6 +53,10 @@ interface ListManyNamespacesArgs extends NameOnlyFixture {
   numberOfNamespaces?: number;
 }
 
+interface UserNamespaceV2Args extends SimpleFixture {
+  username?: string;
+}
+
 function generateGroups(numberOfGroups: number, start: number) {
   const groups = [];
   for (let i = 0; i < numberOfGroups; ++i) {
@@ -304,6 +308,21 @@ export function NamespaceV2<T extends FixturesConstructor>(Parent: T) {
       return this;
     }
 
+    readUserV2Namespace(args?: UserNamespaceV2Args) {
+      const {
+        fixture = "namespaceV2/namespaceV2-user1.json",
+        name = "readUserV2Namespace",
+        username = "user1-uuid",
+      } = args ?? {};
+      const response = { fixture };
+      cy.intercept(
+        "GET",
+        `/ui-server/api/data/namespaces/${username}`,
+        response
+      ).as(name);
+      return this;
+    }
+
     updateGroupV2(args?: GroupV2Args) {
       const {
         fixture = "groupV2/update-groupV2-metadata.json",