Fix issues with vendor selector behavior (#4521)

ethyca · Dec 19, 2023 · d03ce76 · d03ce76
1 parent 7e2b55e
commit d03ce76
Show file tree

Hide file tree

Showing 7 changed files with 232 additions and 197 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,11 +25,10 @@ The types of changes are:
 ### Fixed
 - Fixed incorrect Compass button behavior in system form [#4508](https://github.com/ethyca/fides/pull/4508)
 - Omit certain fields from system payload when empty  [#4508](https://github.com/ethyca/fides/pull/4525)
+- Fixed issues with Compass vendor selector behavior [#4521](https://github.com/ethyca/fides/pull/4521)
 
 ### Changed
 - Upgrade to use Fideslang `3.0.0` and remove associated concepts [#4502](https://github.com/ethyca/fides/pull/4502)
-
-### Changed
 - `fides.js` now sets `supportsOOB` to `false` [#4516](https://github.com/ethyca/fides/pull/4516)
 
 ## [2.26.0](https://github.com/ethyca/fides/compare/2.25.0...main)

diff --git a/clients/admin-ui/cypress/e2e/systems-plus.cy.ts b/clients/admin-ui/cypress/e2e/systems-plus.cy.ts
@@ -24,42 +24,44 @@ describe("System management with Plus features", () => {
     cy.intercept("GET", "/api/v1/system", {
       fixture: "systems/systems.json",
     }).as("getSystems");
+    cy.intercept({ method: "POST", url: "/api/v1/system*" }).as(
+      "postDictSystem"
+    );
   });
 
   describe("vendor list", () => {
     beforeEach(() => {
       stubVendorList();
-      cy.visit(`${SYSTEM_ROUTE}/configure/demo_analytics_system`);
-      cy.wait(["@getDictionaryEntries", "@getSystems", "@getSystem"]);
+      cy.visit(`${ADD_SYSTEMS_MANUAL_ROUTE}`);
+      cy.wait(["@getDictionaryEntries", "@getSystems"]);
     });
 
     it("can display the vendor list dropdown", () => {
-      cy.getSelectValueContainer("input-vendor_id");
+      cy.getSelectValueContainer("input-name");
     });
 
     it("contains type ahead dictionary entries", () => {
-      cy.getSelectValueContainer("input-vendor_id").type("A");
-      cy.get("#react-select-select-vendor_id-option-0").contains("Aniview LTD");
-      cy.get("#react-select-select-vendor_id-option-1").contains(
+      cy.getSelectValueContainer("input-name").type("A");
+      cy.get("#react-select-select-name-option-0").contains("Aniview LTD");
+      cy.get("#react-select-select-name-option-1").contains(
         "Anzu Virtual Reality LTD"
       );
     });
 
     it("can reset suggestions by clearing vendor input", () => {
-      cy.getSelectValueContainer("input-vendor_id").type("L{enter}");
+      cy.getSelectValueContainer("input-name").type("L{enter}");
       cy.getByTestId("input-legal_name").should("have.value", "LINE");
-      cy.getSelectValueContainer("input-vendor_id")
-        .siblings(".custom-select__indicators")
-        .find(".custom-select__clear-indicator");
+      cy.getByTestId("clear-btn").click();
+      cy.getByTestId("input-legal_name").should("be.empty");
     });
 
     it("can't refresh suggestions immediately after populating", () => {
-      cy.getSelectValueContainer("input-vendor_id").type("A{enter}");
+      cy.getSelectValueContainer("input-name").type("A{enter}");
       cy.getByTestId("refresh-suggestions-btn").should("be.disabled");
     });
 
     it("can refresh suggestions when editing a saved system", () => {
-      cy.getSelectValueContainer("input-vendor_id").type("A{enter}");
+      cy.getSelectValueContainer("input-name").type("A{enter}");
       cy.fixture("systems/dictionary-system.json").then((dictSystem) => {
         cy.fixture("systems/system.json").then((origSystem) => {
           cy.intercept(
@@ -75,11 +77,8 @@ describe("System management with Plus features", () => {
           ).as("getDictSystem");
         });
       });
-      cy.intercept({ method: "PUT", url: "/api/v1/system*" }).as(
-        "putDictSystem"
-      );
       cy.getByTestId("save-btn").click();
-      cy.wait("@putDictSystem");
+      cy.wait("@postDictSystem");
       cy.wait("@getDictSystem");
       cy.getByTestId("refresh-suggestions-btn").should("not.be.disabled");
     });
@@ -88,7 +87,7 @@ describe("System management with Plus features", () => {
     // the form to be mistakenly marked as dirty and the "unsaved changes"
     // modal to pop up incorrectly when switching tabs
     it("can switch between tabs after populating from dictionary", () => {
-      cy.getSelectValueContainer("input-vendor_id").type("Anzu{enter}");
+      cy.getSelectValueContainer("input-name").type("Anzu{enter}");
       // the form fetches the system again after saving, so update the intercept with dictionary values
       cy.fixture("systems/dictionary-system.json").then((dictSystem) => {
         cy.fixture("systems/system.json").then((origSystem) => {
@@ -105,11 +104,8 @@ describe("System management with Plus features", () => {
           ).as("getDictSystem");
         });
       });
-      cy.intercept({ method: "PUT", url: "/api/v1/system*" }).as(
-        "putDictSystem"
-      );
       cy.getByTestId("save-btn").click();
-      cy.wait("@putDictSystem");
+      cy.wait("@postDictSystem");
       cy.wait("@getDictSystem");
       cy.getByTestId("input-dpo").should("have.value", "[email protected]");
       cy.getByTestId("tab-Data uses").click();
@@ -119,15 +115,15 @@ describe("System management with Plus features", () => {
     });
 
     it("locks editing for a GVL vendor when TCF is enabled", () => {
-      cy.getSelectValueContainer("input-vendor_id").type("Aniview{enter}");
+      cy.getSelectValueContainer("input-name").type("Aniview{enter}");
       cy.getByTestId("locked-for-GVL-notice");
       cy.getByTestId("input-description").should("be.disabled");
     });
 
     it("does not allow changes to data uses when locked", () => {
-      cy.getSelectValueContainer("input-vendor_id").type("Aniview{enter}");
+      cy.getSelectValueContainer("input-name").type("Aniview{enter}");
       cy.getByTestId("save-btn").click();
-      cy.wait(["@putSystem", "@getSystem", "@getSystems"]);
+      cy.wait(["@postSystem", "@getSystem", "@getSystems"]);
       cy.getByTestId("tab-Data uses").click();
       cy.getByTestId("add-btn").should("not.exist");
       cy.getByTestId("delete-btn").should("not.exist");
@@ -136,7 +132,7 @@ describe("System management with Plus features", () => {
     });
 
     it("does not lock editing for a non-GVL vendor", () => {
-      cy.getSelectValueContainer("input-vendor_id").type("L{enter}");
+      cy.getSelectValueContainer("input-name").type("L{enter}");
       cy.getByTestId("locked-for-GVL-notice").should("not.exist");
       cy.getByTestId("input-description").should("not.be.disabled");
     });
@@ -156,9 +152,9 @@ describe("System management with Plus features", () => {
     });
 
     it("allows changes to data uses for non-GVL vendors", () => {
-      cy.getSelectValueContainer("input-vendor_id").type("L{enter}");
+      cy.getSelectValueContainer("input-name").type("L{enter}");
       cy.getByTestId("save-btn").click();
-      cy.wait(["@putSystem", "@getSystem", "@getSystems"]);
+      cy.wait(["@postSystem", "@getSystem", "@getSystems"]);
       cy.getByTestId("tab-Data uses").click();
       cy.getByTestId("add-btn");
       cy.getByTestId("delete-btn");

diff --git a/clients/admin-ui/cypress/e2e/systems.cy.ts b/clients/admin-ui/cypress/e2e/systems.cy.ts
@@ -84,6 +84,13 @@ describe("System management page", () => {
         }).as("getConnectionTypes");
       });
 
+      it("can't create a system with the same name as an existing system", () => {
+        cy.visit(ADD_SYSTEMS_MANUAL_ROUTE);
+        cy.getByTestId("input-name").type("Demo Analytics System");
+        cy.getByTestId("input-description").focus();
+        cy.getByTestId("error-name");
+      });
+
       it.skip("Can step through the flow", () => {
         cy.fixture("systems/system.json").then((system) => {
           cy.intercept("GET", "/api/v1/system/*", {

diff --git a/clients/admin-ui/src/features/common/form/inputs.tsx b/clients/admin-ui/src/features/common/form/inputs.tsx
@@ -64,6 +64,7 @@ export interface CustomInputProps {
   variant?: Variant;
   isRequired?: boolean;
   textColor?: string;
+  inputRightElement?: React.ReactNode;
 }
 
 // We allow `undefined` here and leave it up to each component that uses this field
@@ -87,7 +88,17 @@ export const Label = ({
 );
 
 export const TextInput = forwardRef(
-  ({ isPassword, ...props }: InputProps & { isPassword: boolean }, ref) => {
+  (
+    {
+      isPassword,
+      inputRightElement,
+      ...props
+    }: InputProps & {
+      isPassword: boolean;
+      inputRightElement?: React.ReactNode;
+    },
+    ref
+  ) => {
     const [type, setType] = useState<"text" | "password">(
       isPassword ? "password" : "text"
     );
@@ -105,6 +116,9 @@ export const TextInput = forwardRef(
           background="white"
           focusBorderColor="primary.600"
         />
+        {inputRightElement ? (
+          <InputRightElement pr={2}>{inputRightElement}</InputRightElement>
+        ) : null}
         {isPassword ? (
           <InputRightElement pr="2">
             <IconButton
@@ -510,6 +524,7 @@ export const CustomTextInput = ({
   disabled,
   variant = "inline",
   isRequired = false,
+  inputRightElement,
   ...props
 }: CustomInputProps & StringField) => {
   const [initialField, meta] = useField(props);
@@ -519,6 +534,17 @@ export const CustomTextInput = ({
 
   const isPassword = initialType === "password";
 
+  const innerInput = (
+    <TextInput
+      {...field}
+      isDisabled={disabled}
+      data-testid={`input-${field.name}`}
+      placeholder={placeholder}
+      isPassword={isPassword}
+      inputRightElement={inputRightElement}
+    />
+  );
+
   if (variant === "inline") {
     return (
       <FormControl isInvalid={isInvalid} isRequired={isRequired}>
@@ -528,13 +554,7 @@ export const CustomTextInput = ({
           ) : null}
           <Flex alignItems="center">
             <Flex flexDir="column" flexGrow={1} mr="2">
-              <TextInput
-                {...field}
-                isDisabled={disabled}
-                data-testid={`input-${field.name}`}
-                placeholder={placeholder}
-                isPassword={isPassword}
-              />
+              {innerInput}
               <ErrorMessage
                 isInvalid={isInvalid}
                 message={meta.error}
@@ -558,13 +578,7 @@ export const CustomTextInput = ({
             {tooltip ? <QuestionTooltip label={tooltip} /> : null}
           </Flex>
         ) : null}
-        <TextInput
-          {...field}
-          isDisabled={disabled}
-          data-testid={`input-${field.name}`}
-          placeholder={placeholder}
-          isPassword={isPassword}
-        />
+        {innerInput}
         <ErrorMessage
           isInvalid={isInvalid}
           message={meta.error}

diff --git a/clients/admin-ui/src/features/system/SystemInformationForm.tsx b/clients/admin-ui/src/features/system/SystemInformationForm.tsx
@@ -129,12 +129,17 @@ const SystemInformationForm = ({
         name: Yup.string()
           .required()
           .label("System name")
-          .notOneOf(
-            systems
-              .filter((s) => s.name !== initialValues.name)
-              .map((s) => s.name),
-            "System must have a unique name"
-          ),
+          .test("is-unique", "", (value, context) => {
+            const takenSystemNames = systems
+              .map((s) => s.name)
+              .filter((name) => name !== initialValues.name);
+            if (takenSystemNames.some((name) => name === value)) {
+              return context.createError({
+                message: `You already have a system called "${value}". Please specify a unique name for this system.`,
+              });
+            }
+            return true;
+          }),
         privacy_policy: Yup.string().min(1).url().nullable(),
       }),
     [systems, initialValues.name]
@@ -300,10 +305,6 @@ const SystemInformationForm = ({
             <SystemFormInputGroup heading="System details">
               {features.dictionaryService ? (
                 <VendorSelector
-                  fieldsSeparated={
-                    features.dictionaryService &&
-                    features.flags.separateVendorSelector
-                  }
                   options={dictionaryOptions}
                   onVendorSelected={handleVendorSelected}
                   isCreate={!passedInSystem}