Saved report bug fixes

CHANGELOG.md

@@ -24,6 +24,10 @@ Changes can also be flagged with a GitHub label for tracking purposes. The URL o
 ### Added
 - Added cache-clearing methods to the `DBCache` model to allow deleting cache entries [#5629](https://github.com/ethyca/fides/pull/5629)
 
+### Fixed
+- Fixed issue where the custom report "reset" button was not working as expected [#5649](https://github.com/ethyca/fides/pull/5649)
+- Fixed column ordering issue in the Data Map report [#5649](https://github.com/ethyca/fides/pull/5649)
+- Fixed issue where the Data Map report filter dialog was missing an Accordion item label [#5649](https://github.com/ethyca/fides/pull/5649)
 
 
 ## [2.52.0](https://github.com/ethyca/fides/compare/2.51.2...2.52.0)

clients/admin-ui/cypress/e2e/datamap-report.cy.ts

@@ -323,6 +323,15 @@ describe("Data map report table", () => {
     it("should filter the table by making a selection", () => {
       cy.getByTestId("filter-multiple-systems-btn").click();
       cy.getByTestId("datamap-report-filter-modal").should("be.visible");
+      cy.getByTestId("filter-modal-accordion-button")
+        .eq(0)
+        .should("have.text", "Data use");
+      cy.getByTestId("filter-modal-accordion-button")
+        .eq(1)
+        .should("have.text", "Data categories");
+      cy.getByTestId("filter-modal-accordion-button")
+        .eq(2)
+        .should("have.text", "Data subject");
       cy.getByTestId("filter-modal-accordion-button").eq(1).click();
       cy.getByTestId("filter-modal-checkbox-tree-categories").should(
         "be.visible",
@@ -389,14 +398,15 @@ describe("Data map report table", () => {
       cy.get("#toast-datamap-report-toast")
         .should("be.visible")
         .should("have.attr", "data-status", "success");
-      cy.getByTestId("custom-reports-trigger")
-        .should("contain.text", "My Custom Report")
-        .click();
+      cy.getByTestId("custom-reports-trigger").should(
+        "contain.text",
+        "My Custom Report",
+      );
       cy.getByTestId("fidesTable").within(() => {
         // reordering applied to report
         cy.get("thead th").eq(2).should("contain.text", "Legal name");
         // column visibility applied to report
-        cy.get("thead th").eq(4).should("not.contain.text", "Data subject");
+        cy.getByTestId("column-data_subjects").should("not.exist");
       });
       cy.getByTestId("group-by-menu").should(
         "contain.text",
@@ -442,10 +452,36 @@ describe("Data map report table", () => {
       cy.getByTestId("custom-reports-reset-button").click();
       cy.getByTestId("apply-report-button").click();
       cy.getByTestId("custom-reports-popover").should("not.be.visible");
+
       cy.getByTestId("custom-reports-trigger").should(
         "contain.text",
         "Reports",
       );
+      cy.getByTestId("fidesTable").within(() => {
+        // reordering reverted
+        cy.get("thead th").eq(2).should("contain.text", "Data categories");
+        // column visibility restored
+        cy.getByTestId("column-data_subjects").should("exist");
+      });
+      cy.getByTestId("group-by-menu").should("contain.text", "Group by system");
+      cy.getByTestId("more-menu").click();
+      cy.getByTestId("edit-columns-btn").click();
+      cy.get("button#data_subjects").should(
+        "have.attr",
+        "aria-checked",
+        "true",
+      );
+      cy.getByTestId("column-settings-close-button").click();
+      cy.getByTestId("filter-multiple-systems-btn").click();
+      cy.getByTestId("datamap-report-filter-modal")
+        .should("be.visible")
+        .within(() => {
+          cy.getByTestId("filter-modal-accordion-button").eq(0).click();
+          cy.getByTestId("checkbox-Analytics").within(() => {
+            cy.get("[data-checked]").should("not.exist");
+          });
+          cy.getByTestId("standard-dialog-close-btn").click();
+        });
     });
     it("should allow the user cancel a report selection", () => {
       cy.wait("@getCustomReportsMinimal");

clients/admin-ui/src/features/datamap/reporting/DatamapReportFilterModal.tsx

@@ -148,7 +148,7 @@ export const DatamapReportFilterModal = ({
       data-testid="datamap-report-filter-modal"
     >
       <Accordion allowToggle>
-        <FilterModalAccordionItem label={columnNameMap.data_use}>
+        <FilterModalAccordionItem label={columnNameMap.data_uses}>
           <CheckboxTree
             nodes={dataUseNodes}
             selected={checkedUses}

clients/admin-ui/src/features/datamap/reporting/DatamapReportTable.tsx

@@ -60,7 +60,11 @@ import {
 import { CustomReportTemplates } from "../../common/custom-reports/CustomReportTemplates";
 import { DATAMAP_LOCAL_STORAGE_KEYS, DEFAULT_COLUMN_NAMES } from "./constants";
 import { DatamapReportWithCustomFields as DatamapReport } from "./datamap-report";
-import { useDatamapReport } from "./datamap-report-context";
+import {
+  DEFAULT_COLUMN_FILTERS,
+  DEFAULT_COLUMN_VISIBILITY,
+  useDatamapReport,
+} from "./datamap-report-context";
 import {
   getDatamapReportColumns,
   getDefaultColumn,
@@ -223,14 +227,6 @@ export const DatamapReportTable = () => {
     ],
   );
 
-  useEffect(() => {
-    if (datamapReport?.items?.length) {
-      const columnIDs = Object.keys(datamapReport.items[0]);
-      setColumnOrder(getColumnOrder(groupBy, columnIDs));
-    }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [groupBy, datamapReport]);
-
   const {
     isOpen: isColumnSettingsOpen,
     onOpen: onColumnSettingsOpen,
@@ -306,6 +302,20 @@ export const DatamapReportTable = () => {
     },
   });
 
+  useEffect(() => {
+    if (groupBy && !!tableInstance) {
+      if (tableInstance.getState().columnOrder.length === 0) {
+        const tableColumnIds = tableInstance.getAllColumns().map((c) => c.id);
+        setColumnOrder(getColumnOrder(groupBy, tableColumnIds));
+      } else {
+        setColumnOrder(
+          getColumnOrder(groupBy, tableInstance.getState().columnOrder),
+        );
+      }
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [groupBy, tableInstance]);
+
   useEffect(() => {
     // changing the groupBy should wait until the data is loaded to update the grouping
     const newGrouping = getGrouping(groupBy);
@@ -345,12 +355,41 @@ export const DatamapReportTable = () => {
 
   const handleSavedReport = (
     savedReport: CustomReportResponse | null,
-    resetForm: (
+    resetColumnNameForm: (
       nextState?: Partial<FormikState<Record<string, string>>> | undefined,
     ) => void,
   ) => {
+    if (!savedReport && !savedCustomReportId) {
+      return;
+    }
     if (!savedReport) {
-      setSavedCustomReportId("");
+      try {
+        setSavedCustomReportId("");
+
+        /* NOTE: we can't just use tableInstance.reset() here because it will reset the table to the initial state, which is likely to include report settings that were saved in the user's local storage. Instead, we need to reset each individual setting to its default value. */
+
+        // reset column visibility (must happen before updating order)
+        setColumnVisibility(DEFAULT_COLUMN_VISIBILITY);
+        tableInstance.toggleAllColumnsVisible(true);
+        tableInstance.setColumnVisibility(DEFAULT_COLUMN_VISIBILITY);
+
+        // reset column order (must happen prior to updating groupBy)
+        setColumnOrder([]);
+        tableInstance.setColumnOrder([]);
+
+        // reset groupBy and filters (will automatically update the tableinstance)
+        setGroupBy(DATAMAP_GROUPING.SYSTEM_DATA_USE);
+        setSelectedFilters(DEFAULT_COLUMN_FILTERS);
+
+        // reset column names
+        setColumnNameMapOverrides({});
+        resetColumnNameForm({ values: {} });
+      } catch (error: any) {
+        toast({
+          status: "error",
+          description: "There was a problem resetting the report.",
+        });
+      }
       return;
     }
     try {
@@ -369,8 +408,8 @@ export const DatamapReportTable = () => {
         );
 
         if (savedGroupBy) {
+          // No need to manually update the tableInstance here; setting the groupBy will trigger the useEffect to update the grouping.
           setGroupBy(savedGroupBy);
-          tableInstance.setGrouping(getGrouping(savedGroupBy));
         }
         if (savedFilters) {
           setSelectedFilters(savedFilters);
@@ -394,7 +433,7 @@ export const DatamapReportTable = () => {
           },
         );
         setColumnNameMapOverrides(columnNameMap);
-        resetForm({ values: columnNameMap });
+        resetColumnNameForm({ values: columnNameMap });
       }
       setSavedCustomReportId(savedReport.id);
       toast({

clients/admin-ui/src/features/datamap/reporting/datamap-report-context.tsx

@@ -12,6 +12,17 @@ import { DATAMAP_GROUPING } from "~/types/api";
 import { DatamapReportFilterSelections } from "../types";
 import { COLUMN_IDS, DATAMAP_LOCAL_STORAGE_KEYS } from "./constants";
 
+export const DEFAULT_COLUMN_VISIBILITY = {
+  [COLUMN_IDS.SYSTEM_UNDECLARED_DATA_CATEGORIES]: false,
+  [COLUMN_IDS.DATA_USE_UNDECLARED_DATA_CATEGORIES]: false,
+};
+
+export const DEFAULT_COLUMN_FILTERS = {
+  dataUses: [],
+  dataSubjects: [],
+  dataCategories: [],
+};
+
 interface DatamapReportContextProps {
   savedCustomReportId: string;
   setSavedCustomReportId: Dispatch<SetStateAction<string>>;
@@ -51,11 +62,7 @@ export const DatamapReportProvider = ({
   const [selectedFilters, setSelectedFilters] =
     useLocalStorage<DatamapReportFilterSelections>(
       DATAMAP_LOCAL_STORAGE_KEYS.FILTERS,
-      {
-        dataUses: [],
-        dataSubjects: [],
-        dataCategories: [],
-      },
+      DEFAULT_COLUMN_FILTERS,
     );
 
   const [columnOrder, setColumnOrder] = useLocalStorage<string[]>(
@@ -65,10 +72,7 @@ export const DatamapReportProvider = ({
 
   const [columnVisibility, setColumnVisibility] = useLocalStorage<
     Record<string, boolean>
-  >(DATAMAP_LOCAL_STORAGE_KEYS.COLUMN_VISIBILITY, {
-    [COLUMN_IDS.SYSTEM_UNDECLARED_DATA_CATEGORIES]: false,
-    [COLUMN_IDS.DATA_USE_UNDECLARED_DATA_CATEGORIES]: false,
-  });
+  >(DATAMAP_LOCAL_STORAGE_KEYS.COLUMN_VISIBILITY, DEFAULT_COLUMN_VISIBILITY);
 
   const [columnSizing, setColumnSizing] = useLocalStorage<
     Record<string, number>

clients/admin-ui/src/features/datamap/reporting/utils.ts

@@ -12,17 +12,22 @@ export const getGrouping = (groupBy?: DATAMAP_GROUPING) => {
   }
 };
 
-export const getColumnOrder = (
-  groupBy: DATAMAP_GROUPING,
-  columnIDs: string[],
-) => {
+export const getPrefixColumns = (groupBy: DATAMAP_GROUPING) => {
   let columnOrder: string[] = [];
   if (DATAMAP_GROUPING.SYSTEM_DATA_USE === groupBy) {
     columnOrder = [COLUMN_IDS.SYSTEM_NAME, COLUMN_IDS.DATA_USE];
   }
   if (DATAMAP_GROUPING.DATA_USE_SYSTEM === groupBy) {
     columnOrder = [COLUMN_IDS.DATA_USE, COLUMN_IDS.SYSTEM_NAME];
   }
+  return columnOrder;
+};
+
+export const getColumnOrder = (
+  groupBy: DATAMAP_GROUPING,
+  columnIDs: string[],
+) => {
+  let columnOrder: string[] = getPrefixColumns(groupBy);
   columnOrder = columnOrder.concat(
     columnIDs.filter(
       (columnID) =>
@@ -31,14 +36,3 @@ export const getColumnOrder = (
   );
   return columnOrder;
 };
-
-export const getPrefixColumns = (groupBy: DATAMAP_GROUPING) => {
-  let columnOrder: string[] = [];
-  if (DATAMAP_GROUPING.SYSTEM_DATA_USE === groupBy) {
-    columnOrder = [COLUMN_IDS.SYSTEM_NAME, COLUMN_IDS.DATA_USE];
-  }
-  if (DATAMAP_GROUPING.DATA_USE_SYSTEM === groupBy) {
-    columnOrder = [COLUMN_IDS.DATA_USE, COLUMN_IDS.SYSTEM_NAME];
-  }
-  return columnOrder;
-};