Add custom fields to system history (#4294)

Co-authored-by: Kelsey Thomas <[email protected]>
ethyca · Oct 23, 2023 · 712394a · 712394a
1 parent 72fdbf2
commit 712394a
Show file tree

Hide file tree

Showing 13 changed files with 309 additions and 105 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,7 @@ The types of changes are:
 ### Added
 - Added a `FidesPreferenceToggled` event to Fides.js to track when user preferences change without being saved [#4253](https://github.com/ethyca/fides/pull/4253)
 - Add AC Systems to the TCF Overlay under Vendor Consents section [#4266](https://github.com/ethyca/fides/pull/4266/)
+- Custom fields are now included in system history change tracking [#4294](https://github.com/ethyca/fides/pull/4294)
 
 ### Changed
 - Derive cookie storage info, privacy policy and legitimate interest disclosure URLs, and data retention data from the data map instead of directly from gvl.json [#4286](https://github.com/ethyca/fides/pull/4286)

diff --git a/clients/admin-ui/src/features/common/custom-fields/hooks.ts b/clients/admin-ui/src/features/common/custom-fields/hooks.ts
@@ -3,11 +3,10 @@ import { useCallback, useMemo } from "react";
 import { useFeatures } from "~/features/common/features";
 import { useAlert } from "~/features/common/hooks";
 import {
-  useDeleteCustomFieldMutation,
+  useBulkUpdateCustomFieldsMutation,
   useGetAllAllowListQuery,
   useGetCustomFieldDefinitionsByResourceTypeQuery,
   useGetCustomFieldsForResourceQuery,
-  useUpsertCustomFieldMutation,
 } from "~/features/plus/plus.slice";
 import { CustomFieldWithId, ResourceTypes } from "~/types/api";
 
@@ -47,20 +46,13 @@ export const useCustomFields = ({
     skip: queryFidesKey !== "" && !(isEnabled && queryFidesKey),
   });
 
-  // The `fixedCacheKey` options will ensure that components referencing the same resource will
-  // share mutation info. That won't be too useful, though, because `upsertCustomField` can issue
-  // multiple requests: one for each field associated with the resource.
-  const [upsertCustomFieldMutationTrigger, upsertCustomFieldMutationResult] =
-    useUpsertCustomFieldMutation({ fixedCacheKey: resourceFidesKey });
-  const [deleteCustomFieldMutationTrigger, deleteCustomFieldMutationResult] =
-    useDeleteCustomFieldMutation({ fixedCacheKey: resourceFidesKey });
+  const [bulkUpdateCustomFieldsMutationTrigger] =
+    useBulkUpdateCustomFieldsMutation();
 
   const isLoading =
     allAllowListQuery.isLoading ||
     customFieldDefinitionsQuery.isLoading ||
-    isCustomFieldIsLoading ||
-    upsertCustomFieldMutationResult.isLoading ||
-    deleteCustomFieldMutationResult.isLoading;
+    isCustomFieldIsLoading;
 
   const idToAllowListWithOptions = useMemo(
     () =>
@@ -116,13 +108,21 @@ export const useCustomFields = ({
    */
   const customFieldValues = useMemo(() => {
     const values: CustomFieldValues = {};
-    if (definitionIdToCustomField) {
-      definitionIdToCustomField.forEach((value, key) => {
-        values[key] = value.value.toString();
+    if (activeCustomFieldDefinition && definitionIdToCustomField) {
+      activeCustomFieldDefinition.forEach((value) => {
+        const customField = definitionIdToCustomField.get(value.id || "");
+        if (customField) {
+          if (!!value.allow_list_id && value.field_type === "string[]") {
+            values[customField.custom_field_definition_id] = customField.value;
+          } else {
+            values[customField.custom_field_definition_id] =
+              customField.value.toString();
+          }
+        }
       });
     }
     return values;
-  }, [definitionIdToCustomField]);
+  }, [activeCustomFieldDefinition, definitionIdToCustomField]);
 
   /**
    * Issue a batch of upsert and delete requests that will sync the form selections to the
@@ -134,8 +134,8 @@ export const useCustomFields = ({
         return;
       }
 
-      // When creating an resource, the fides key may have initially been blank. But by the time the
-      // form is submitted it must not be blank (not undefined, not an empty string).
+      // When creating a resource, the fides key may have initially been blank.
+      // But by the time the form is submitted it must not be blank (not undefined, not an empty string).
       const fidesKey =
         "fides_key" in formValues && formValues.fides_key !== ""
           ? formValues.fides_key
@@ -156,37 +156,38 @@ export const useCustomFields = ({
         return;
       }
 
-      try {
-        // This would be a lot simpler (and more efficient) if the API had an endpoint for updating
-        // all the metadata associated with a field, including deleting options that weren't passed.
-        await Promise.allSettled(
-          sortedCustomFieldDefinitionIds.map((definitionId) => {
-            const customField = definitionIdToCustomField.get(definitionId);
-            const value = customFieldValuesFromForm[definitionId];
-
-            if (
-              value === undefined ||
-              value === "" ||
-              (Array.isArray(value) && value.length === 0)
-            ) {
-              if (!customField?.id) {
-                return undefined;
-              }
-              const { id } = customField;
-
-              return deleteCustomFieldMutationTrigger({ id });
-            }
-
-            const body = {
-              custom_field_definition_id: definitionId,
-              resource_id: fidesKey,
-              id: customField?.id,
-              value,
-            };
+      const upsertList: Array<CustomFieldWithId> = [];
+      const deleteList: Array<string> = [];
+
+      sortedCustomFieldDefinitionIds.forEach((definitionId) => {
+        const customField = definitionIdToCustomField.get(definitionId);
+        const value = customFieldValuesFromForm[definitionId];
+
+        if (
+          value === undefined ||
+          value === "" ||
+          (Array.isArray(value) && value.length === 0)
+        ) {
+          if (customField?.id) {
+            deleteList.push(customField.id);
+          }
+        } else {
+          upsertList.push({
+            custom_field_definition_id: definitionId,
+            resource_id: fidesKey,
+            id: customField?.id,
+            value,
+          });
+        }
+      });
 
-            return upsertCustomFieldMutationTrigger(body);
-          })
-        );
+      try {
+        await bulkUpdateCustomFieldsMutationTrigger({
+          resource_type: resourceType,
+          resource_id: fidesKey,
+          upsert: upsertList,
+          delete: deleteList,
+        });
       } catch (e) {
         errorAlert(
           `One or more custom fields have failed to save, please try again.`
@@ -198,11 +199,11 @@ export const useCustomFields = ({
     [
       isEnabled,
       definitionIdToCustomField,
-      deleteCustomFieldMutationTrigger,
       errorAlert,
       resourceFidesKey,
       sortedCustomFieldDefinitionIds,
-      upsertCustomFieldMutationTrigger,
+      bulkUpdateCustomFieldsMutationTrigger,
+      resourceType,
     ]
   );
 

diff --git a/clients/admin-ui/src/features/plus/plus.slice.ts b/clients/admin-ui/src/features/plus/plus.slice.ts
@@ -196,7 +196,14 @@ const plusApi = baseApi.injectEndpoints({
       }),
       invalidatesTags: ["Custom Fields", "Datamap"],
     }),
-
+    bulkUpdateCustomFields: build.mutation<void, any>({
+      query: (params) => ({
+        url: `plus/custom-metadata/custom-field/bulk`,
+        method: "POST",
+        body: params,
+      }),
+      invalidatesTags: ["Custom Fields", "Datamap"],
+    }),
     getAllCustomFieldDefinitions: build.query<
       CustomFieldDefinitionWithId[],
       void
@@ -321,6 +328,7 @@ export const {
   useUpdateScanMutation,
   useUpsertAllowListMutation,
   useUpsertCustomFieldMutation,
+  useBulkUpdateCustomFieldsMutation,
   useGetAllCustomFieldDefinitionsQuery,
   useGetAllowListQuery,
   useGetAllDictionaryEntriesQuery,

diff --git a/clients/admin-ui/src/features/system/SystemFormTabs.tsx b/clients/admin-ui/src/features/system/SystemFormTabs.tsx
@@ -15,7 +15,7 @@ import { useSystemOrDatamapRoute } from "~/features/common/hooks/useSystemOrData
 import { DEFAULT_TOAST_PARAMS } from "~/features/common/toast";
 import ConnectionForm from "~/features/datastore-connections/system_portal_config/ConnectionForm";
 import PrivacyDeclarationStep from "~/features/system/privacy-declarations/PrivacyDeclarationStep";
-import { System, SystemResponse } from "~/types/api";
+import { SystemResponse } from "~/types/api";
 
 import SystemHistoryTable from "./history/SystemHistoryTable";
 import {
@@ -109,7 +109,7 @@ const SystemFormTabs = ({
     }
   }, [activeSystem]);
 
-  const handleSuccess = (system: System) => {
+  const handleSuccess = (system: SystemResponse) => {
     // show a save message if this is the first time the system was saved
     if (activeSystem === undefined) {
       setShowSaveMessage(true);

diff --git a/clients/admin-ui/src/features/system/SystemInformationForm.tsx b/clients/admin-ui/src/features/system/SystemInformationForm.tsx
@@ -51,7 +51,7 @@ import {
   useUpdateSystemMutation,
 } from "~/features/system/system.slice";
 import SystemFormInputGroup from "~/features/system/SystemFormInputGroup";
-import { ResourceTypes, System, SystemResponse } from "~/types/api";
+import { ResourceTypes, SystemResponse } from "~/types/api";
 
 import { DictSuggestionToggle } from "./dictionary-form/ToggleDictSuggestions";
 import { usePrivacyDeclarationData } from "./privacy-declarations/hooks";
@@ -80,7 +80,7 @@ const SystemHeading = ({ system }: { system?: SystemResponse }) => {
 };
 
 interface Props {
-  onSuccess: (system: System) => void;
+  onSuccess: (system: SystemResponse) => void;
   system?: SystemResponse;
   withHeader?: boolean;
   children?: React.ReactNode;
@@ -156,7 +156,9 @@ const SystemInformationForm = ({
     const systemBody = transformFormValuesToSystem(values);
 
     const handleResult = (
-      result: { data: {} } | { error: FetchBaseQueryError | SerializedError }
+      result:
+        | { data: SystemResponse }
+        | { error: FetchBaseQueryError | SerializedError }
     ) => {
       if (isErrorResult(result)) {
         const attemptedAction = isEditing ? "editing" : "creating";
@@ -172,7 +174,7 @@ const SystemInformationForm = ({
         toast.closeAll();
         // Reset state such that isDirty will be checked again before next save
         formikHelpers.resetForm({ values });
-        onSuccess(systemBody);
+        onSuccess(result.data);
         dispatch(setSuggestions("hiding"));
       }
     };

diff --git a/clients/admin-ui/src/features/system/history/SystemHistoryTable.tsx b/clients/admin-ui/src/features/system/history/SystemHistoryTable.tsx
@@ -23,7 +23,9 @@ import { SystemHistoryResponse } from "~/types/api";
 import { SystemResponse } from "~/types/api/models/SystemResponse";
 
 import {
+  alignPrivacyDeclarationCustomFields,
   alignPrivacyDeclarations,
+  alignSystemCustomFields,
   assignSystemNames,
   assignVendorLabels,
   describeSystemChange,
@@ -59,6 +61,9 @@ const SystemHistoryTable = ({ system }: Props) => {
     history = assignVendorLabels(history, dictionaryOptions);
     // Look up the system names for the source and destination fides_keys
     history = assignSystemNames(history, systems);
+    // Align custom fields
+    history = alignSystemCustomFields(history);
+    history = alignPrivacyDeclarationCustomFields(history);
 
     setSelectedHistory(history);
     setModalOpen(true);