Fix issue with monitor results pagination (#5060)

ethyca · Jul 5, 2024 · b9367ad · b9367ad
1 parent e26c84c
commit b9367ad
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 58 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,6 +45,7 @@ The types of changes are:
 
 ### Fixed
 - Fixed intermittent connection issues with Redshift by increasing timeout and preferring SSL in test connections [#4981](https://github.com/ethyca/fides/pull/4981)
+- Fixed data detection & discovery results not displaying correctly across multiple pages[#5060](https://github.com/ethyca/fides/pull/5060)
 
 ### Developer Experience
 - Fixed various environmental issues when running Cypress tests locally [#5040](https://github.com/ethyca/fides/pull/5040)

diff --git a/clients/admin-ui/cypress/e2e/discovery-detection.cy.ts b/clients/admin-ui/cypress/e2e/discovery-detection.cy.ts
@@ -24,17 +24,20 @@ describe("discovery and detection", () => {
 
     describe("additions", () => {
       it("should show addition icon and 'Dataset' type", () => {
-        cy.getByTestId("row-test_dataset_1-col-name").within(() => {
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_1-col-name"
+        ).within(() => {
           cy.getByTestId("add-icon").should("exist");
         });
-        cy.getByTestId("row-test_dataset_1-col-type").should(
-          "contain",
-          "Dataset"
-        );
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_1-col-type"
+        ).should("contain", "Dataset");
       });
 
       it("should be able to monitor or ignore", () => {
-        cy.getByTestId("row-test_dataset_1-col-action").within(() => {
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_1-col-action"
+        ).within(() => {
           cy.getByTestId("action-Monitor").click();
           cy.wait("@confirmResource");
           cy.getByTestId("action-Ignore").click();
@@ -43,24 +46,27 @@ describe("discovery and detection", () => {
       });
 
       it("should navigate to detection view on click", () => {
-        cy.getByTestId("row-test_dataset_1").click();
+        cy.getByTestId("row-my_bigquery_monitor-test_dataset_1").click();
         cy.url().should("contain", "detection");
       });
     });
 
     describe("changes", () => {
       it("should show change icon and 'Dataset' type", () => {
-        cy.getByTestId("row-test_dataset_4-col-name").within(() => {
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_4-col-name"
+        ).within(() => {
           cy.getByTestId("change-icon").should("exist");
         });
-        cy.getByTestId("row-test_dataset_4-col-type").should(
-          "contain",
-          "Dataset"
-        );
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_4-col-type"
+        ).should("contain", "Dataset");
       });
 
       it("should be able to confirm or ignore", () => {
-        cy.getByTestId("row-test_dataset_4-col-action").within(() => {
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_4-col-action"
+        ).within(() => {
           cy.getByTestId("action-Confirm").click();
           cy.wait("@confirmResource");
           cy.getByTestId("action-Ignore").click();
@@ -69,24 +75,27 @@ describe("discovery and detection", () => {
       });
 
       it("should navigate to detection view on click", () => {
-        cy.getByTestId("row-test_dataset_4").click();
+        cy.getByTestId("row-my_bigquery_monitor-test_dataset_4").click();
         cy.url().should("contain", "detection");
       });
     });
 
     describe("removals", () => {
       it("should show removal icon and 'Dataset' type", () => {
-        cy.getByTestId("row-test_dataset_3-col-name").within(() => {
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_3-col-name"
+        ).within(() => {
           cy.getByTestId("remove-icon").should("exist");
         });
-        cy.getByTestId("row-test_dataset_3-col-type").should(
-          "contain",
-          "Dataset"
-        );
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_3-col-type"
+        ).should("contain", "Dataset");
       });
 
       it("should only be able to ignore", () => {
-        cy.getByTestId("row-test_dataset_3-col-action").within(() => {
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_3-col-action"
+        ).within(() => {
           cy.getByTestId("action-Confirm").should("not.exist");
           cy.getByTestId("action-Monitor").should("not.exist");
           cy.getByTestId("action-Ignore").click();
@@ -95,24 +104,27 @@ describe("discovery and detection", () => {
       });
 
       it("should navigate to detection view on click", () => {
-        cy.getByTestId("row-test_dataset_3").click();
+        cy.getByTestId("row-my_bigquery_monitor-test_dataset_3").click();
         cy.url().should("contain", "detection");
       });
     });
 
     describe("classifications", () => {
       it("should show classification icon and 'Classification' type", () => {
-        cy.getByTestId("row-test_dataset_2-col-name").within(() => {
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_2-col-name"
+        ).within(() => {
           cy.getByTestId("classify-icon").should("exist");
         });
-        cy.getByTestId("row-test_dataset_2-col-type").should(
-          "contain",
-          "Classification"
-        );
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_2-col-type"
+        ).should("contain", "Classification");
       });
 
       it("should be able to confirm or ignore", () => {
-        cy.getByTestId("row-test_dataset_2-col-action").within(() => {
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_2-col-action"
+        ).within(() => {
           cy.getByTestId("action-Confirm").click();
           cy.wait("@promoteResource");
           cy.getByTestId("action-Ignore").click();
@@ -121,7 +133,7 @@ describe("discovery and detection", () => {
       });
 
       it("should navigate to discovery view on row click", () => {
-        cy.getByTestId("row-test_dataset_2").click();
+        cy.getByTestId("row-my_bigquery_monitor-test_dataset_2").click();
         cy.url().should("contain", "discovery");
       });
     });
@@ -137,14 +149,13 @@ describe("discovery and detection", () => {
       });
 
       it("should show datasets", () => {
-        cy.getByTestId("row-test_dataset_1-col-type").should(
-          "contain",
-          "Dataset"
-        );
+        cy.getByTestId(
+          "row-my_bigquery_monitor-test_dataset_1-col-type"
+        ).should("contain", "Dataset");
       });
 
       it("should navigate to a table view on click", () => {
-        cy.getByTestId("row-test_dataset_1").click();
+        cy.getByTestId("row-my_bigquery_monitor-test_dataset_1").click();
         cy.url().should("contain", "test_dataset_1");
         cy.getByTestId("results-breadcrumb").should(
           "contain",
@@ -173,15 +184,19 @@ describe("discovery and detection", () => {
 
       it("should show columns for tables with changes", () => {
         cy.getByTestId("name-header").should("contain", "Table name");
-        cy.getByTestId("row-consent-reports-20").should("exist");
-        cy.getByTestId("row-consent-reports-21").should("not.exist");
+        cy.getByTestId("row-my_bigquery_monitor-consent-reports-20").should(
+          "exist"
+        );
+        cy.getByTestId("row-my_bigquery_monitor-consent-reports-21").should(
+          "not.exist"
+        );
         cy.getByTestId("full-schema-toggle").within(() => {
           cy.get("span").should("not.have.attr", "data-checked");
         });
       });
 
       it("should navigate to field view on row click", () => {
-        cy.getByTestId("row-consent-reports-20").click();
+        cy.getByTestId("row-my_bigquery_monitor-consent-reports-20").click();
         cy.url().should("contain", "consent-reports-20");
         cy.getByTestId("results-breadcrumb").should(
           "contain",
@@ -193,27 +208,29 @@ describe("discovery and detection", () => {
         it("should be able to toggle showing full schema", () => {
           cy.getByTestId("full-schema-toggle").click();
           cy.wait("@getAllDetectionTables");
-          cy.getByTestId("row-consent-reports-21-col-type").should(
-            "contain",
-            "--"
-          );
-          cy.getByTestId("row-consent-reports-22-col-type").should(
-            "contain",
-            "--"
-          );
+          cy.getByTestId(
+            "row-my_bigquery_monitor-consent-reports-21-col-type"
+          ).should("contain", "--");
+          cy.getByTestId(
+            "row-my_bigquery_monitor-consent-reports-22-col-type"
+          ).should("contain", "--");
         });
 
         it("should allow muted tables to be unmuted", () => {
           cy.getByTestId("full-schema-toggle").click();
-          cy.getByTestId("row-consent-reports-21-col-actions").within(() => {
+          cy.getByTestId(
+            "row-my_bigquery_monitor-consent-reports-21-col-actions"
+          ).within(() => {
             cy.getByTestId("action-Monitor").click();
             cy.wait("@unmuteResource");
           });
         });
 
         it("should allow monitored tables to be muted", () => {
           cy.getByTestId("full-schema-toggle").click();
-          cy.getByTestId("row-consent-reports-22-col-actions").within(() => {
+          cy.getByTestId(
+            "row-my_bigquery_monitor-consent-reports-22-col-actions"
+          ).within(() => {
             cy.getByTestId("action-Ignore").click();
             cy.wait("@ignoreResource");
           });
@@ -236,7 +253,7 @@ describe("discovery and detection", () => {
       });
 
       it("should not allow navigation via row clicking", () => {
-        cy.getByTestId("row-User_geography").click();
+        cy.getByTestId("row-my_bigquery_monitor-User_geography").click();
         cy.url().should("not.contain", "User_geography");
       });
     });
@@ -253,14 +270,12 @@ describe("discovery and detection", () => {
 
       it("should show columns for classifications", () => {
         cy.getByTestId(
-          "row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1-col-type"
+          "row-my_bigquery_monitor-test_dataset_1-col-type"
         ).should("contain", "Classification");
       });
 
       it("should navigate to table view on row click", () => {
-        cy.getByTestId(
-          "row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1"
-        ).click();
+        cy.getByTestId("row-my_bigquery_monitor-test_dataset_1").click();
         cy.url().should("contain", "test_dataset_1");
         cy.getByTestId("results-breadcrumb").should(
           "contain",
@@ -285,9 +300,7 @@ describe("discovery and detection", () => {
       });
 
       it("should navigate to field view on row click", () => {
-        cy.getByTestId(
-          "row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20"
-        ).click();
+        cy.getByTestId("row-my_bigquery_monitor-consent-reports-20").click();
         cy.url().should("contain", "consent-reports-20");
         cy.getByTestId("results-breadcrumb").should(
           "contain",
@@ -328,7 +341,7 @@ describe("discovery and detection", () => {
 
       it("should not allow navigation via row clicking", () => {
         cy.getByTestId(
-          "row-my_bigquery_monitor.prj-bigquery-418515.test_dataset_1.consent-reports-20.User_geography-col-name"
+          "row-my_bigquery_monitor-User_geography-col-name"
         ).click();
         cy.url().should("not.contain", "User_geography");
       });

diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/tables/ActivityTable.tsx b/clients/admin-ui/src/features/data-discovery-and-detection/tables/ActivityTable.tsx
@@ -23,6 +23,7 @@ import {
 import { RelativeTimestampCell } from "~/features/common/table/v2/cells";
 import { useGetMonitorResultsQuery } from "~/features/data-discovery-and-detection/discovery-detection.slice";
 import ResultStatusCell from "~/features/data-discovery-and-detection/tables/ResultStatusCell";
+import getResourceRowName from "~/features/data-discovery-and-detection/utils/getResourceRowName";
 import { Database, DiffStatus, StagedResource } from "~/types/api";
 
 import DetectionItemAction from "../DetectionItemActions";
@@ -159,7 +160,7 @@ const ActivityTable: React.FC<ActivityTableProps> = ({
     getCoreRowModel: getCoreRowModel(),
     getGroupedRowModel: getGroupedRowModel(),
     getExpandedRowModel: getExpandedRowModel(),
-    getRowId: (row) => row.name ?? row.urn,
+    getRowId: getResourceRowName,
     columns: resourceColumns,
     manualPagination: true,
     data,

diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/tables/DetectionResultTable.tsx b/clients/admin-ui/src/features/data-discovery-and-detection/tables/DetectionResultTable.tsx
@@ -24,6 +24,7 @@ import useDiscoveryRoutes from "~/features/data-discovery-and-detection/hooks/us
 import { DiscoveryMonitorItem } from "~/features/data-discovery-and-detection/types/DiscoveryMonitorItem";
 import { StagedResourceType } from "~/features/data-discovery-and-detection/types/StagedResourceType";
 import { findResourceType } from "~/features/data-discovery-and-detection/utils/findResourceType";
+import getResourceRowName from "~/features/data-discovery-and-detection/utils/getResourceRowName";
 import { DiffStatus, StagedResource } from "~/types/api";
 
 import SearchInput from "../SearchInput";
@@ -149,7 +150,7 @@ const DetectionResultTable = ({ resourceUrn }: MonitorResultTableProps) => {
     getCoreRowModel: getCoreRowModel(),
     getGroupedRowModel: getGroupedRowModel(),
     getExpandedRowModel: getExpandedRowModel(),
-    getRowId: (row) => row.name ?? row.urn,
+    getRowId: getResourceRowName,
     columns: resourceColumns,
     manualPagination: true,
     data,

diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryResultTable.tsx b/clients/admin-ui/src/features/data-discovery-and-detection/tables/DiscoveryResultTable.tsx
@@ -26,6 +26,7 @@ import DiscoveryTableBulkActions from "~/features/data-discovery-and-detection/t
 import { DiscoveryMonitorItem } from "~/features/data-discovery-and-detection/types/DiscoveryMonitorItem";
 import { StagedResourceType } from "~/features/data-discovery-and-detection/types/StagedResourceType";
 import { findResourceType } from "~/features/data-discovery-and-detection/utils/findResourceType";
+import getResourceRowName from "~/features/data-discovery-and-detection/utils/getResourceRowName";
 import { DiffStatus, StagedResource } from "~/types/api";
 
 import SearchInput from "../SearchInput";
@@ -143,7 +144,7 @@ const DiscoveryResultTable = ({ resourceUrn }: MonitorResultTableProps) => {
     state: {
       rowSelection,
     },
-    getRowId: (row) => row.urn,
+    getRowId: getResourceRowName,
     data,
   });
 

diff --git a/clients/admin-ui/src/features/data-discovery-and-detection/utils/getResourceRowName.ts b/clients/admin-ui/src/features/data-discovery-and-detection/utils/getResourceRowName.ts
@@ -0,0 +1,6 @@
+import { StagedResource } from "~/types/api";
+
+const getResourceRowName = (row: StagedResource) =>
+  `${row.monitor_config_id}-${row.name ?? row.urn}`;
+
+export default getResourceRowName;