From 3f179464548eb224d0ebfc54244d05c376b5072f Mon Sep 17 00:00:00 2001 From: Allison King Date: Tue, 20 Sep 2022 14:30:40 -0400 Subject: [PATCH] New fields on system forms (#1082) * Extend DescribeSystemsForm * Extend PrivacyDeclarationForm * Add joint controller and data protection impact assessment * Move system dependency field to abridged form * Prepare data protection impact assessment for payload * Add cypress tests * Extend ReviewSystemForm * Add tests for extended review form * Fix adding another declaration when name field is blank * Rename SuccessPage --> SystemRegisterSuccess * Pass system object through props * Update tests * Update changelog * Refactor ReviewSystemForm * Move config wizard system forms to system directory (#1097) * Move system forms to system directory * Refactor form layout components into its own file * Rename files from form --> step * Update changelog --- CHANGELOG.md | 2 + .../ctl/admin-ui/cypress/e2e/systems.cy.ts | 162 +++++++++++++++++- .../ctl/admin-ui/cypress/fixtures/system.json | 4 +- .../admin-ui/src/features/common/helpers.ts | 9 + .../config-wizard/ConfigWizardWalkthrough.tsx | 17 +- .../DescribeSystemStep.tsx} | 82 +++++++-- .../system/DescribeSystemsFormExtension.tsx | 80 +++++++++ .../src/features/system/ManualSystemFlow.tsx | 14 +- .../PrivacyDeclarationAccordion.tsx | 26 +-- .../PrivacyDeclarationFormExtension.tsx | 23 +++ .../PrivacyDeclarationStep.tsx} | 28 ++- .../system/ReviewSystemFormExtension.tsx | 69 ++++++++ .../ReviewSystemStep.tsx} | 75 +++----- .../SystemRegisterSuccess.tsx | 0 .../src/features/system/form-layout.tsx | 34 ++++ .../admin-ui/src/features/taxonomy/hooks.tsx | 7 +- 16 files changed, 512 insertions(+), 120 deletions(-) rename clients/ctl/admin-ui/src/features/{config-wizard/DescribeSystemsForm.tsx => system/DescribeSystemStep.tsx} (69%) create mode 100644 clients/ctl/admin-ui/src/features/system/DescribeSystemsFormExtension.tsx rename clients/ctl/admin-ui/src/features/{config-wizard => system}/PrivacyDeclarationAccordion.tsx (84%) create mode 100644 clients/ctl/admin-ui/src/features/system/PrivacyDeclarationFormExtension.tsx rename clients/ctl/admin-ui/src/features/{config-wizard/PrivacyDeclarationForm.tsx => system/PrivacyDeclarationStep.tsx} (94%) create mode 100644 clients/ctl/admin-ui/src/features/system/ReviewSystemFormExtension.tsx rename clients/ctl/admin-ui/src/features/{config-wizard/ReviewSystemForm.tsx => system/ReviewSystemStep.tsx} (57%) rename clients/ctl/admin-ui/src/features/{config-wizard => system}/SystemRegisterSuccess.tsx (100%) create mode 100644 clients/ctl/admin-ui/src/features/system/form-layout.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d672e5d92..9225d7808d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,12 +27,14 @@ The types of changes are: * New page to add a system via yaml [#1062](https://github.com/ethyca/fides/pull/1062) * Skeleton of page to add a system manually [#1068](https://github.com/ethyca/fides/pull/1068) * Refactor config wizard system forms to be reused for system management [#1072](https://github.com/ethyca/fides/pull/1072) + * Add additional optional fields to system management forms [#1082](https://github.com/ethyca/fides/pull/1082) * Delete a system through the UI [#1085](https://github.com/ethyca/fides/pull/1085) ### Changed * Changed behavior of `load_default_taxonomy` to append instead of upsert [#1040](https://github.com/ethyca/fides/pull/1040) * Changed behavior of adding privacy declarations to decouple the actions of the "add" and "next" buttons [#1086](https://github.com/ethyca/fides/pull/1086) +* Moved system related UI components from the `config-wizard` directory to the `system` directory [#1097](https://github.com/ethyca/fides/pull/1097) ### Fixed diff --git a/clients/ctl/admin-ui/cypress/e2e/systems.cy.ts b/clients/ctl/admin-ui/cypress/e2e/systems.cy.ts index be27475baa..520f2234c5 100644 --- a/clients/ctl/admin-ui/cypress/e2e/systems.cy.ts +++ b/clients/ctl/admin-ui/cypress/e2e/systems.cy.ts @@ -127,6 +127,12 @@ describe("System management page", () => { cy.intercept("GET", "/api/v1/data_use", { fixture: "data_uses.json", }).as("getDataUse"); + cy.intercept("GET", "/api/v1/system", { + fixture: "systems.json", + }).as("getSystems"); + cy.intercept("GET", "/api/v1/dataset", { fixture: "datasets.json" }).as( + "getDatasets" + ); }); it("Can step through the flow", () => { @@ -135,6 +141,7 @@ describe("System management page", () => { cy.visit("/system/new"); cy.getByTestId("manually-generate-btn").click(); cy.url().should("contain", "/system/new/configure"); + cy.wait("@getSystems"); cy.getByTestId("input-name").type(system.name); cy.getByTestId("input-fides_key").type(system.fides_key); cy.getByTestId("input-description").type(system.description); @@ -142,6 +149,11 @@ describe("System management page", () => { system.tags.forEach((tag) => { cy.getByTestId("input-tags").type(`${tag}{enter}`); }); + cy.getByTestId("input-system_dependencies").click(); + cy.getByTestId("input-system_dependencies").within(() => { + cy.contains("Demo Analytics System").click(); + }); + cy.getByTestId("confirm-btn").click(); cy.wait("@postSystem").then((interception) => { const { body } = interception.request; @@ -153,6 +165,8 @@ describe("System management page", () => { system_type: system.system_type, tags: system.tags, privacy_declarations: [], + third_country_transfers: [], + system_dependencies: ["demo_analytics_system"], }); }); @@ -183,9 +197,10 @@ describe("System management page", () => { cy.getByTestId("next-btn").click(); cy.wait("@putSystem").then((interception) => { const { body } = interception.request; - // eslint-disable-next-line @typescript-eslint/naming-convention - const { dataset_references, ...expected } = declaration; - expect(body.privacy_declarations[0]).to.eql(expected); + expect(body.privacy_declarations[0]).to.eql({ + ...declaration, + dataset_references: [], + }); }); // Now at the Review stage @@ -199,6 +214,9 @@ describe("System management page", () => { system.tags.forEach((tag) => { cy.getByTestId("review-System tags").contains(tag); }); + system.system_dependencies.forEach((dep) => { + cy.getByTestId("review-System dependencies").contains(dep); + }); // Open up the privacy declaration cy.getByTestId( "declaration-Analyze customer behaviour for improvements." @@ -216,6 +234,9 @@ describe("System management page", () => { cy.getByTestId("declaration-Data qualifier").contains( reviewDeclaration.data_qualifier ); + reviewDeclaration.dataset_references.forEach((dr) => { + cy.getByTestId("declaration-Dataset references").contains(dr); + }); cy.getByTestId("confirm-btn").click(); @@ -228,6 +249,141 @@ describe("System management page", () => { cy.url().should("match", /system$/); }); }); + + it("Can render and post extended form fields", () => { + const system = { + fides_key: "foo", + system_type: "cool system", + data_responsibility_title: "Sub-Processor", + organization_fides_key: "default_organization", + administrating_department: "department", + third_country_transfers: ["USA"], + joint_controller: { + name: "bob", + email: "bob@ethyca.com", + }, + data_protection_impact_assessment: { + is_required: true, + progress: "in progress", + link: "http://www.ethyca.com", + }, + }; + cy.visit("/system/new"); + cy.getByTestId("manually-generate-btn").click(); + // input required fields + cy.getByTestId("input-fides_key").type(system.fides_key); + cy.getByTestId("input-system_type").type(system.system_type); + + // now input extra fields + cy.getByTestId("input-data_responsibility_title").click(); + cy.getByTestId("input-data_responsibility_title").within(() => { + cy.contains(system.data_responsibility_title).click(); + }); + cy.getByTestId("input-administrating_department").type( + system.administrating_department + ); + cy.getByTestId("input-third_country_transfers").type( + "United States of America{enter}" + ); + cy.getByTestId("input-joint_controller.name").type( + system.joint_controller.name + ); + cy.getByTestId("input-joint_controller.email").type( + system.joint_controller.email + ); + cy.getByTestId( + "input-data_protection_impact_assessment.is_required" + ).within(() => { + cy.getByTestId("option-true").click(); + }); + cy.getByTestId("input-data_protection_impact_assessment.progress").type( + system.data_protection_impact_assessment.progress + ); + cy.getByTestId("input-data_protection_impact_assessment.link").type( + system.data_protection_impact_assessment.link + ); + + cy.getByTestId("confirm-btn").click(); + cy.wait("@postSystem").then((interception) => { + const { body } = interception.request; + expect(body).to.eql({ + name: "", + organization_fides_key: system.organization_fides_key, + fides_key: system.fides_key, + description: "", + system_type: system.system_type, + tags: [], + privacy_declarations: [], + third_country_transfers: ["USA"], + system_dependencies: [], + administrating_department: system.administrating_department, + data_responsibility_title: system.data_responsibility_title, + joint_controller: { + ...system.joint_controller, + address: "", + phone: "", + }, + data_protection_impact_assessment: + system.data_protection_impact_assessment, + }); + }); + + // Fill in the privacy declaration form + cy.wait("@getDataCategory"); + cy.wait("@getDataQualifier"); + cy.wait("@getDataSubject"); + cy.wait("@getDataUse"); + cy.wait("@getDatasets"); + cy.getByTestId("privacy-declaration-form"); + const declaration = { + name: "my declaration", + data_categories: ["user.biometric", "user.contact"], + data_use: "advertising", + data_subjects: ["citizen_voter", "consultant"], + dataset_references: ["demo_users_dataset_2"], + }; + cy.getByTestId("input-name").type(declaration.name); + declaration.data_categories.forEach((dc) => { + cy.getByTestId("input-data_categories").type(`${dc}{enter}`); + }); + cy.getByTestId("input-data_use").type(`${declaration.data_use}{enter}`); + declaration.data_subjects.forEach((ds) => { + cy.getByTestId("input-data_subjects").type(`${ds}{enter}`); + }); + cy.getByTestId("input-dataset_references").click(); + cy.getByTestId("input-dataset_references").within(() => { + cy.contains("Demo Users Dataset 2").click(); + }); + cy.getByTestId("add-btn").click(); + cy.getByTestId("next-btn").click(); + cy.wait("@putSystem").then((interception) => { + const { body } = interception.request; + expect(body.privacy_declarations[0]).to.eql(declaration); + }); + + // Now at the Review stage + cy.getByTestId("review-heading"); + cy.getByTestId("review-Data responsibility title").contains( + "Controller" + ); + cy.getByTestId("review-Administrating department").contains( + "Engineering" + ); + cy.getByTestId("review-Geographic location").contains("USA"); + cy.getByTestId("review-Geographic location").contains("CAN"); + cy.getByTestId("review-Joint controller").within(() => { + cy.getByTestId("review-Name").contains("Sally Controller"); + }); + cy.getByTestId("review-Data protection impact assessment").within( + () => { + cy.getByTestId("review-Is required").contains("Yes"); + cy.getByTestId("review-Progress").contains("Complete"); + cy.getByTestId("review-Link").contains( + "https://example.org/analytics_system_data_protection_impact_assessment" + ); + } + ); + }); }); }); diff --git a/clients/ctl/admin-ui/cypress/fixtures/system.json b/clients/ctl/admin-ui/cypress/fixtures/system.json index 1c6f8d5513..c8c7e49901 100644 --- a/clients/ctl/admin-ui/cypress/fixtures/system.json +++ b/clients/ctl/admin-ui/cypress/fixtures/system.json @@ -19,8 +19,8 @@ "dataset_references": ["demo_users_dataset"] } ], - "system_dependencies": null, - "joint_controller": null, + "system_dependencies": ["demo_marketing_system"], + "joint_controller": { "name": "Sally Controller" }, "third_country_transfers": ["USA", "CAN"], "administrating_department": "Engineering", "data_protection_impact_assessment": { diff --git a/clients/ctl/admin-ui/src/features/common/helpers.ts b/clients/ctl/admin-ui/src/features/common/helpers.ts index cd91d805e3..293a938899 100644 --- a/clients/ctl/admin-ui/src/features/common/helpers.ts +++ b/clients/ctl/admin-ui/src/features/common/helpers.ts @@ -73,3 +73,12 @@ export const parseError = ( return defaultError; }; + +/** + * Given an enumeration, create options out of its key and values + */ +export const enumToOptions = (e: { [s: number]: string }) => + Object.entries(e).map((entry) => ({ + value: entry[1], + label: entry[1], + })); diff --git a/clients/ctl/admin-ui/src/features/config-wizard/ConfigWizardWalkthrough.tsx b/clients/ctl/admin-ui/src/features/config-wizard/ConfigWizardWalkthrough.tsx index eb10167efe..67e27fc4aa 100644 --- a/clients/ctl/admin-ui/src/features/config-wizard/ConfigWizardWalkthrough.tsx +++ b/clients/ctl/admin-ui/src/features/config-wizard/ConfigWizardWalkthrough.tsx @@ -4,6 +4,10 @@ import React from "react"; import { useAppDispatch, useAppSelector } from "~/app/hooks"; import { CloseSolidIcon } from "~/features/common/Icon"; +import DescribeSystemStep from "~/features/system/DescribeSystemStep"; +import PrivacyDeclarationStep from "~/features/system/PrivacyDeclarationStep"; +import ReviewSystemStep from "~/features/system/ReviewSystemStep"; +import SystemRegisterSuccess from "~/features/system/SystemRegisterSuccess"; import { System } from "~/types/api"; import HorizontalStepper from "../common/HorizontalStepper"; @@ -19,12 +23,8 @@ import { setSystemToCreate, } from "./config-wizard.slice"; import { HORIZONTAL_STEPS, STEPS } from "./constants"; -import DescribeSystemsForm from "./DescribeSystemsForm"; import OrganizationInfoForm from "./OrganizationInfoForm"; -import PrivacyDeclarationForm from "./PrivacyDeclarationForm"; -import ReviewSystemForm from "./ReviewSystemForm"; import ScanResultsForm from "./ScanResultsForm"; -import SystemRegisterSuccess from "./SystemRegisterSuccess"; import ViewYourDataMapPage from "./ViewYourDataMapPage"; const ConfigWizardWalkthrough = () => { @@ -81,23 +81,26 @@ const ConfigWizardWalkthrough = () => { /> ) : null} {reviewStep === 1 && ( - )} {reviewStep === 2 && system && ( - )} {reviewStep === 3 && system && ( - dispatch(changeReviewStep())} + abridged /> )} {reviewStep === 4 && system && ( diff --git a/clients/ctl/admin-ui/src/features/config-wizard/DescribeSystemsForm.tsx b/clients/ctl/admin-ui/src/features/system/DescribeSystemStep.tsx similarity index 69% rename from clients/ctl/admin-ui/src/features/config-wizard/DescribeSystemsForm.tsx rename to clients/ctl/admin-ui/src/features/system/DescribeSystemStep.tsx index 5fd1f07fe9..5b87e20344 100644 --- a/clients/ctl/admin-ui/src/features/config-wizard/DescribeSystemsForm.tsx +++ b/clients/ctl/admin-ui/src/features/system/DescribeSystemStep.tsx @@ -7,14 +7,19 @@ import * as Yup from "yup"; import { CustomCreatableMultiSelect, + CustomMultiSelect, CustomTextInput, } from "~/features/common/form/inputs"; import { getErrorMessage, isErrorResult } from "~/features/common/helpers"; import { DEFAULT_ORGANIZATION_FIDES_KEY } from "~/features/organization"; -import { useCreateSystemMutation } from "~/features/system/system.slice"; +import DescribeSystemsFormExtension from "~/features/system/DescribeSystemsFormExtension"; +import { + useCreateSystemMutation, + useGetAllSystemsQuery, +} from "~/features/system/system.slice"; import { System } from "~/types/api"; -const initialValues: System = { +const initialValues = { description: "", fides_key: "", name: "", @@ -22,35 +27,71 @@ const initialValues: System = { tags: [], system_type: "", privacy_declarations: [], + data_responsibility_title: undefined, + administrating_department: "", + third_country_transfers: [], + system_dependencies: [], + joint_controller: { + name: "", + email: "", + address: "", + phone: "", + }, + data_protection_impact_assessment: { + is_required: "false", + progress: "", + link: "", + }, }; -type FormValues = typeof initialValues; +export type FormValues = typeof initialValues; const ValidationSchema = Yup.object().shape({ fides_key: Yup.string().required().label("System key"), system_type: Yup.string().required().label("System type"), }); +const transformFormValuesToSystem = (formValues: FormValues): System => { + const hasImpactAssessment = + formValues.data_protection_impact_assessment.is_required === "true"; + const impactAssessment = hasImpactAssessment + ? { ...formValues.data_protection_impact_assessment, is_required: true } + : undefined; + const hasJointController = + formValues.joint_controller !== initialValues.joint_controller; + const jointController = hasJointController + ? formValues.joint_controller + : undefined; + return { + ...formValues, + organization_fides_key: DEFAULT_ORGANIZATION_FIDES_KEY, + privacy_declarations: [], + data_protection_impact_assessment: impactAssessment, + joint_controller: jointController, + administrating_department: + formValues.administrating_department === "" + ? undefined + : formValues.administrating_department, + }; +}; + interface Props { onCancel: () => void; onSuccess: (system: System) => void; + abridged?: boolean; } -const DescribeSystemsForm = ({ onCancel, onSuccess }: Props) => { +const DescribeSystemStep = ({ onCancel, onSuccess, abridged }: Props) => { const [createSystem] = useCreateSystemMutation(); const [isLoading, setIsLoading] = useState(false); + const { data: systems } = useGetAllSystemsQuery(); + const systemOptions = systems + ? systems.map((s) => ({ label: s.name ?? s.fides_key, value: s.fides_key })) + : []; const toast = useToast(); const handleSubmit = async (values: FormValues) => { - const systemBody = { - description: values.description, - fides_key: values.fides_key, - name: values.name, - organization_fides_key: DEFAULT_ORGANIZATION_FIDES_KEY, - privacy_declarations: [], - system_type: values.system_type, - tags: values.tags, - }; + const systemBody = transformFormValuesToSystem(values); const handleResult = ( result: { data: {} } | { error: FetchBaseQueryError | SerializedError } @@ -67,7 +108,7 @@ const DescribeSystemsForm = ({ onCancel, onSuccess }: Props) => { }); } else { toast.closeAll(); - onSuccess(values); + onSuccess(systemBody); } }; @@ -86,7 +127,7 @@ const DescribeSystemsForm = ({ onCancel, onSuccess }: Props) => { onSubmit={handleSubmit} validationSchema={ValidationSchema} > - {({ dirty }) => ( + {({ dirty, values }) => (
@@ -139,6 +180,15 @@ const DescribeSystemsForm = ({ onCancel, onSuccess }: Props) => { } tooltip="Provide one or more tags to group the system. Tags are important as they allow you to filter and group systems for reporting and later review. Tags provide tremendous value as you scale - imagine you have thousands of systems, you’re going to thank us later for tagging!" /> + + {!abridged ? ( + + ) : null}