Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with vendor selector behavior #4521

Merged
merged 9 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ The types of changes are:
### Changed
- Upgrade to use Fideslang `3.0.0` and remove associated concepts [#4502](https://github.com/ethyca/fides/pull/4502)

### Fixed
- Fixed issues with Compass vendor selector behavior [#4521](https://github.com/ethyca/fides/pull/4521)

## [2.26.0](https://github.com/ethyca/fides/compare/2.25.0...main)

### Added
Expand Down
50 changes: 23 additions & 27 deletions clients/admin-ui/cypress/e2e/systems-plus.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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");
});
Expand All @@ -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) => {
Expand All @@ -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();
Expand All @@ -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");
Expand All @@ -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");
});
Expand All @@ -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");
Expand Down
7 changes: 7 additions & 0 deletions clients/admin-ui/cypress/e2e/systems.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/*", {
Expand Down
41 changes: 26 additions & 15 deletions clients/admin-ui/src/features/common/form/inputs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -87,7 +88,14 @@ 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"
);
Expand All @@ -105,6 +113,9 @@ export const TextInput = forwardRef(
background="white"
focusBorderColor="primary.600"
/>
{inputRightElement ? (
<InputRightElement pr={2}>{inputRightElement}</InputRightElement>
) : null}
{isPassword ? (
<InputRightElement pr="2">
<IconButton
Expand Down Expand Up @@ -510,6 +521,7 @@ export const CustomTextInput = ({
disabled,
variant = "inline",
isRequired = false,
inputRightElement,
...props
}: CustomInputProps & StringField) => {
const [initialField, meta] = useField(props);
Expand All @@ -519,6 +531,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}>
Expand All @@ -528,13 +551,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}
Expand All @@ -558,13 +575,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}
Expand Down
21 changes: 11 additions & 10 deletions clients/admin-ui/src/features/system/SystemInformationForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
jpople marked this conversation as resolved.
Show resolved Hide resolved
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]
Expand Down Expand Up @@ -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}
Expand Down
Loading