Privacy declaration with composite ID (#2905)

Co-authored-by: Kelsey Thomas <101993653+Kelsey-Ethyca@users.noreply.github.com>
ethyca · Mar 24, 2023 · 5f901e1 · 5f901e1
1 parent 9082c94
commit 5f901e1
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 48 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,7 @@ The types of changes are:
 ### Fixed
 
 * Restricted Contributors from being able to create Owners [#2888](https://github.com/ethyca/fides/pull/2888)
+* Allow multiple data uses as long as their processing activity name is different [#2905](https://github.com/ethyca/fides/pull/2905)
 
 ### Fixed
 * Allow for dynamic aspect ratio for logo on Privacy Center 404 [#2895](https://github.com/ethyca/fides/pull/2895)

diff --git a/clients/admin-ui/cypress/e2e/systems.cy.ts b/clients/admin-ui/cypress/e2e/systems.cy.ts
@@ -265,6 +265,7 @@ describe("System management page", () => {
 
     it("Can go directly to a system's edit page", () => {
       cy.visit("/systems/configure/fidesctl_system");
+      cy.wait("@getFidesctlSystem");
       cy.getByTestId("input-name").should("have.value", "Fidesctl System");
 
       cy.intercept("GET", "/api/v1/system/*", {
@@ -319,6 +320,7 @@ describe("System management page", () => {
       cy.wait(["@getDataCategories", "@getDataSubjects", "@getDataUses"]);
       cy.getByTestId("new-declaration-form").within(() => {
         cy.getByTestId("input-data_use").type(`${secondDataUse}{enter}`);
+        cy.getByTestId("input-name").type(`test-1{enter}`);
         cy.getByTestId("input-data_categories").type(`user.biometric{enter}`);
         cy.getByTestId("input-data_subjects").type(`anonymous{enter}`);
         cy.getByTestId("save-btn").click();
@@ -441,18 +443,19 @@ describe("System management page", () => {
       });
     });
 
-    it("warns when a data use is being added that is already used", () => {
+    it("warns when a data use and processing activity is being added that is already used", () => {
       cy.visit(SYSTEM_ROUTE);
       cy.getByTestId("system-fidesctl_system").within(() => {
         cy.getByTestId("more-btn").click();
         cy.getByTestId("edit-btn").click();
       });
-      // "improve.system" is already being used
+      // "improve.system" and "Store system data." are already being used
       cy.getByTestId("tab-Data uses").click();
       cy.getByTestId("add-btn").click();
       cy.wait(["@getDataCategories", "@getDataSubjects", "@getDataUses"]);
       cy.getByTestId("new-declaration-form").within(() => {
         cy.getByTestId("input-data_use").type(`improve.system{enter}`);
+        cy.getByTestId("input-name").type(`Store system data.{enter}`);
         cy.getByTestId("input-data_categories").type(`user.biometric{enter}`);
         cy.getByTestId("input-data_subjects").type(`anonymous{enter}`);
         cy.getByTestId("save-btn").click();
@@ -483,14 +486,36 @@ describe("System management page", () => {
       cy.getByTestId(`accordion-header-improve.system`);
       cy.getByTestId(`accordion-header-advertising`).click();
 
-      // try to change 'advertising' to 'improve.system'
+      // try to change 'advertising' to 'improve.system' and make their names match
       cy.getByTestId("advertising-form").within(() => {
         cy.getByTestId("input-data_use").type(`improve.system{enter}`);
+        cy.getByTestId("input-name").clear().type(`Store system data.{enter}`);
         cy.getByTestId("save-btn").click();
       });
       cy.getByTestId("toast-error-msg");
     });
 
+    it("can have multiple of the same data use if the names are different", () => {
+      cy.visit(SYSTEM_ROUTE);
+      cy.getByTestId("system-fidesctl_system").within(() => {
+        cy.getByTestId("more-btn").click();
+        cy.getByTestId("edit-btn").click();
+      });
+      // "improve.system" and "Store system data." are already being used
+      // use "improve.system" again but a different name
+      cy.getByTestId("tab-Data uses").click();
+      cy.getByTestId("add-btn").click();
+      cy.wait(["@getDataCategories", "@getDataSubjects", "@getDataUses"]);
+      cy.getByTestId("new-declaration-form").within(() => {
+        cy.getByTestId("input-data_use").type(`improve.system{enter}`);
+        cy.getByTestId("input-name").type(`A different description.{enter}`);
+        cy.getByTestId("input-data_categories").type(`user.biometric{enter}`);
+        cy.getByTestId("input-data_subjects").type(`anonymous{enter}`);
+        cy.getByTestId("save-btn").click();
+      });
+      cy.getByTestId("toast-success-msg");
+    });
+
     describe("delete privacy declaration", () => {
       beforeEach(() => {
         cy.fixture("systems/systems_with_data_uses.json").then((systems) => {

diff --git a/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationAccordion.tsx b/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationAccordion.tsx
@@ -8,34 +8,33 @@ import {
 } from "@fidesui/react";
 import { Form, Formik } from "formik";
 
-import { PrivacyDeclaration } from "~/types/api";
-
 import {
   DataProps,
   PrivacyDeclarationFormComponents,
   usePrivacyDeclarationForm,
   ValidationSchema,
 } from "./PrivacyDeclarationForm";
+import { PrivacyDeclarationWithId } from "./types";
 
 interface AccordionProps extends DataProps {
-  privacyDeclarations: PrivacyDeclaration[];
+  privacyDeclarations: PrivacyDeclarationWithId[];
   onEdit: (
-    oldDeclaration: PrivacyDeclaration,
-    newDeclaration: PrivacyDeclaration
+    oldDeclaration: PrivacyDeclarationWithId,
+    newDeclaration: PrivacyDeclarationWithId
   ) => Promise<boolean>;
-  onDelete: (declaration: PrivacyDeclaration) => Promise<boolean>;
+  onDelete: (declaration: PrivacyDeclarationWithId) => Promise<boolean>;
 }
 
 const PrivacyDeclarationAccordionItem = ({
   privacyDeclaration,
   onEdit,
   onDelete,
   ...dataProps
-}: { privacyDeclaration: PrivacyDeclaration } & Omit<
+}: { privacyDeclaration: PrivacyDeclarationWithId } & Omit<
   AccordionProps,
   "privacyDeclarations"
 >) => {
-  const handleEdit = (newValues: PrivacyDeclaration) =>
+  const handleEdit = (newValues: PrivacyDeclarationWithId) =>
     onEdit(privacyDeclaration, newValues);
 
   const { initialValues, renderHeader, handleSubmit } =
@@ -100,7 +99,7 @@ const PrivacyDeclarationAccordion = ({
         // The closest is 'data_use' but that is only enforced on the frontend and can change
         // This results in the "Saved" indicator not appearing if you change the 'data_use' in the form
         // The fix would be to enforce a key, either on the backend, or through a significant workaround on the frontend
-        key={dec.data_use}
+        key={dec.id}
         privacyDeclaration={dec}
         {...props}
       />

diff --git a/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationForm.tsx b/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationForm.tsx
@@ -27,6 +27,8 @@ import {
   PrivacyDeclaration,
 } from "~/types/api";
 
+import { PrivacyDeclarationWithId } from "./types";
+
 export const ValidationSchema = Yup.object().shape({
   data_categories: Yup.array(Yup.string())
     .min(1, "Must assign at least one data category")
@@ -46,6 +48,20 @@ const defaultInitialValues: PrivacyDeclaration = {
 
 type FormValues = typeof defaultInitialValues;
 
+const transformPrivacyDeclarationToHaveId = (
+  privacyDeclaration: PrivacyDeclaration
+) => ({
+  ...privacyDeclaration,
+  id: privacyDeclaration.name
+    ? `${privacyDeclaration.data_use} - ${privacyDeclaration.name}`
+    : privacyDeclaration.data_use,
+});
+
+export const transformPrivacyDeclarationsToHaveId = (
+  privacyDeclarations: PrivacyDeclaration[]
+): PrivacyDeclarationWithId[] =>
+  privacyDeclarations.map(transformPrivacyDeclarationToHaveId);
+
 export interface DataProps {
   allDataCategories: DataCategory[];
   allDataUses: DataUse[];
@@ -72,7 +88,7 @@ export const PrivacyDeclarationFormComponents = ({
     : [];
 
   const handleDelete = async () => {
-    await onDelete(initialValues);
+    await onDelete(transformPrivacyDeclarationToHaveId(initialValues));
     deleteModal.onClose();
   };
 
@@ -179,7 +195,9 @@ export const usePrivacyDeclarationForm = ({
       (du) => du.fides_key === initialValues.data_use
     )[0];
     if (thisDataUse) {
-      return thisDataUse.name;
+      return initialValues.name
+        ? `${thisDataUse.name} - ${initialValues.name}`
+        : thisDataUse.name;
     }
     return undefined;
   }, [allDataUses, initialValues]);
@@ -188,7 +206,10 @@ export const usePrivacyDeclarationForm = ({
     values: FormValues,
     formikHelpers: FormikHelpers<FormValues>
   ) => {
-    const success = await onSubmit(values, formikHelpers);
+    const success = await onSubmit(
+      transformPrivacyDeclarationToHaveId(values),
+      formikHelpers
+    );
     if (success) {
       // Reset state such that isDirty will be checked again before next save
       formikHelpers.resetForm({ values });
@@ -225,11 +246,11 @@ export const usePrivacyDeclarationForm = ({
 
 interface Props {
   onSubmit: (
-    values: PrivacyDeclaration,
+    values: PrivacyDeclarationWithId,
     formikHelpers: FormikHelpers<PrivacyDeclaration>
   ) => Promise<boolean>;
-  onDelete: (declaration: PrivacyDeclaration) => Promise<boolean>;
-  initialValues?: PrivacyDeclaration;
+  onDelete: (declaration: PrivacyDeclarationWithId) => Promise<boolean>;
+  initialValues?: PrivacyDeclarationWithId;
 }
 
 export const PrivacyDeclarationForm = ({

diff --git a/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationManager.tsx b/clients/admin-ui/src/features/system/privacy-declarations/PrivacyDeclarationManager.tsx
@@ -4,17 +4,24 @@ import { useMemo, useState } from "react";
 import { PrivacyDeclaration, System } from "~/types/api";
 
 import PrivacyDeclarationAccordion from "./PrivacyDeclarationAccordion";
-import { DataProps, PrivacyDeclarationForm } from "./PrivacyDeclarationForm";
-
-type FormValues = PrivacyDeclaration;
-
-const transformFormValuesToDeclaration = (
-  formValues: FormValues
-): PrivacyDeclaration => ({
-  ...formValues,
-  // Fill in an empty string for name because of https://github.com/ethyca/fideslang/issues/98
-  name: formValues.name ?? "",
-});
+import {
+  DataProps,
+  PrivacyDeclarationForm,
+  transformPrivacyDeclarationsToHaveId,
+} from "./PrivacyDeclarationForm";
+import { PrivacyDeclarationWithId } from "./types";
+
+const transformDeclarationForSubmission = (
+  formValues: PrivacyDeclarationWithId
+): PrivacyDeclaration => {
+  // Remove the id which is only a frontend artifact
+  const { id, ...values } = formValues;
+  return {
+    ...values,
+    // Fill in an empty string for name because of https://github.com/ethyca/fideslang/issues/98
+    name: values.name ?? "",
+  };
+};
 
 interface Props {
   system: System;
@@ -35,22 +42,24 @@ const PrivacyDeclarationManager = ({
 
   const [showNewForm, setShowNewForm] = useState(false);
   const [newDeclaration, setNewDeclaration] = useState<
-    PrivacyDeclaration | undefined
+    PrivacyDeclarationWithId | undefined
   >(undefined);
 
   const accordionDeclarations = useMemo(() => {
+    const declarations = transformPrivacyDeclarationsToHaveId(
+      system.privacy_declarations
+    );
     if (!newDeclaration) {
-      return system.privacy_declarations;
+      return declarations;
     }
-    return system.privacy_declarations.filter(
-      (pd) => pd.data_use !== newDeclaration.data_use
-    );
+    return declarations.filter((pd) => pd.id !== newDeclaration.id);
   }, [newDeclaration, system]);
 
   const checkAlreadyExists = (values: PrivacyDeclaration) => {
     if (
-      accordionDeclarations.filter((d) => d.data_use === values.data_use)
-        .length > 0
+      accordionDeclarations.filter(
+        (d) => d.data_use === values.data_use && d.name === values.name
+      ).length > 0
     ) {
       onCollision();
       return true;
@@ -59,36 +68,36 @@ const PrivacyDeclarationManager = ({
   };
 
   const handleSave = async (
-    updatedDeclarations: PrivacyDeclaration[],
+    updatedDeclarations: PrivacyDeclarationWithId[],
     isDelete?: boolean
   ) => {
     const transformedDeclarations = updatedDeclarations.map((d) =>
-      transformFormValuesToDeclaration(d)
+      transformDeclarationForSubmission(d)
     );
     const success = await onSave(transformedDeclarations, isDelete);
     return success;
   };
 
   const handleEditDeclaration = async (
-    oldDeclaration: PrivacyDeclaration,
-    updatedDeclaration: PrivacyDeclaration
+    oldDeclaration: PrivacyDeclarationWithId,
+    updatedDeclaration: PrivacyDeclarationWithId
   ) => {
     // Do not allow editing a privacy declaration to have the same data use as one that already exists
     if (
-      updatedDeclaration.data_use !== oldDeclaration.data_use &&
+      updatedDeclaration.id !== oldDeclaration.id &&
       checkAlreadyExists(updatedDeclaration)
     ) {
       return false;
     }
     // Because the data use can change, we also need a reference to the old declaration in order to
     // make sure we are replacing the proper one
     const updatedDeclarations = accordionDeclarations.map((dec) =>
-      dec.data_use === oldDeclaration.data_use ? updatedDeclaration : dec
+      dec.id === oldDeclaration.id ? updatedDeclaration : dec
     );
     return handleSave(updatedDeclarations);
   };
 
-  const saveNewDeclaration = async (values: PrivacyDeclaration) => {
+  const saveNewDeclaration = async (values: PrivacyDeclarationWithId) => {
     if (checkAlreadyExists(values)) {
       return false;
     }
@@ -104,14 +113,18 @@ const PrivacyDeclarationManager = ({
     setNewDeclaration(undefined);
   };
 
-  const handleDelete = async (declarationToDelete: PrivacyDeclaration) => {
-    const updatedDeclarations = system.privacy_declarations.filter(
-      (dec) => dec.data_use !== declarationToDelete.data_use
-    );
+  const handleDelete = async (
+    declarationToDelete: PrivacyDeclarationWithId
+  ) => {
+    const updatedDeclarations = transformPrivacyDeclarationsToHaveId(
+      system.privacy_declarations
+    ).filter((dec) => dec.id !== declarationToDelete.id);
     return handleSave(updatedDeclarations, true);
   };
 
-  const handleDeleteNew = async (declarationToDelete: PrivacyDeclaration) => {
+  const handleDeleteNew = async (
+    declarationToDelete: PrivacyDeclarationWithId
+  ) => {
     const success = await handleDelete(declarationToDelete);
     if (success) {
       setShowNewForm(false);

diff --git a/clients/admin-ui/src/features/system/privacy-declarations/types.ts b/clients/admin-ui/src/features/system/privacy-declarations/types.ts
@@ -0,0 +1,11 @@
+import { PrivacyDeclaration } from "~/types/api";
+
+/**
+ * This is because privacy declarations do not have an ID on the backend.
+ * It is very useful for React rendering to have a stable ID. We currently
+ * make this the composite of data_use - name, but even better may be to
+ * give it a UUID (or to have the backend actually enforce this!)
+ */
+export interface PrivacyDeclarationWithId extends PrivacyDeclaration {
+  id: string;
+}