PROD-1899 Move-admin-UI-away-from-using-unpaginated-system-and-datase…

…t-APIs-where-possible (#5135) Co-authored-by: Lucano Vera <lucanovera@ethyca.com> Co-authored-by: jpople <jeremy@ethyca.com>
ethyca · Jul 31, 2024 · 2de7500 · 2de7500
1 parent d5fb94e
commit 2de7500
Show file tree

Hide file tree

Showing 16 changed files with 53 additions and 573 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,6 +31,7 @@ The types of changes are:
 - Serve GVL languages as they are requested [#5112](https://github.com/ethyca/fides/pull/5112)
 - Changed text on system integrations tab to direct to new integration management [#5097](https://github.com/ethyca/fides/pull/5097)
 - Updates to consent experience styling [#5085](https://github.com/ethyca/fides/pull/5085)
+- Improve performance by removing the need to load every system into redux store [#5135](https://github.com/ethyca/fides/pull/5135)
 - Use the `user_id` from a Segment Trait instead of an `email` when deleting a user in Segment [#5004](https://github.com/ethyca/fides/pull/5004)
 - Moves some endpoints for property-specific messaging from OSS -> plus [#5069](https://github.com/ethyca/fides/pull/5069)
 - Text changes in monitor config table and form [#5142](https://github.com/ethyca/fides/pull/5142)

diff --git a/clients/admin-ui/cypress/e2e/routes.cy.ts b/clients/admin-ui/cypress/e2e/routes.cy.ts
@@ -16,7 +16,7 @@ describe("Routes", () => {
   describe("permissions", () => {
     beforeEach(() => {
       // For these tests, let's say we always have systems and connectors
-      cy.intercept("GET", "/api/v1/system", {
+      cy.intercept("GET", "/api/v1/system*", {
         fixture: "systems/systems.json",
       }).as("getSystems");
       cy.intercept("GET", "/api/v1/connection*", {
@@ -29,7 +29,6 @@ describe("Routes", () => {
       cy.assumeRole(RoleRegistryEnum.OWNER);
       cy.visit("/");
       cy.visit(ADD_SYSTEMS_ROUTE);
-      cy.wait("@getSystems");
       cy.getByTestId("add-systems");
       cy.visit("/privacy-requests");
       cy.getByTestId("privacy-requests");

diff --git a/clients/admin-ui/cypress/e2e/systems-classify.cy.ts b/clients/admin-ui/cypress/e2e/systems-classify.cy.ts
diff --git a/clients/admin-ui/cypress/e2e/user-management.cy.ts b/clients/admin-ui/cypress/e2e/user-management.cy.ts
@@ -371,7 +371,6 @@ describe("User management", () => {
         beforeEach(() => {
           cy.visit(`/user-management/profile/${USER_1_ID}`);
           cy.getByTestId("tab-Permissions").click();
-          cy.wait("@getSystems");
           cy.wait("@getUserManagedSystems");
         });
 
@@ -391,7 +390,6 @@ describe("User management", () => {
         beforeEach(() => {
           cy.visit(`/user-management/profile/${USER_1_ID}`);
           cy.getByTestId("tab-Permissions").click();
-          cy.wait("@getSystems");
           cy.wait("@getUserManagedSystems");
           cy.getByTestId("assign-systems-delete-table");
         });

diff --git a/clients/admin-ui/src/features/common/CommonSubscriptions.tsx b/clients/admin-ui/src/features/common/CommonSubscriptions.tsx
@@ -1,13 +1,13 @@
 import { useAppSelector } from "~/app/hooks";
 import { useGetHealthQuery } from "~/features/common/health.slice";
 import { useGetHealthQuery as useGetPlusHealthQuery } from "~/features/plus/plus.slice";
-import { useGetAllSystemsQuery } from "~/features/system/system.slice";
+import { useGetSystemsQuery } from "~/features/system";
 import { selectThisUsersScopes } from "~/features/user-management";
 
 const useCommonSubscriptions = () => {
   useGetHealthQuery();
   useGetPlusHealthQuery();
-  useGetAllSystemsQuery();
+  useGetSystemsQuery({ page: 1, size: 1 }); // used to preload systems count on selectSystemsCount
   useAppSelector(selectThisUsersScopes);
 };
 

diff --git a/clients/admin-ui/src/features/common/features/features.slice.ts b/clients/admin-ui/src/features/common/features/features.slice.ts
@@ -6,7 +6,7 @@ import { type RootState } from "~/app/store";
 import { selectHealth } from "~/features/common/health.slice";
 import { selectInitialConnections } from "~/features/datastore-connections";
 import { selectHealth as selectPlusHealth } from "~/features/plus/plus.slice";
-import { selectAllSystems } from "~/features/system";
+import { selectSystemsCount } from "~/features/system";
 import flagDefaults from "~/flags.json";
 
 import { configureFlags, flagsForEnv } from "./config";
@@ -146,7 +146,7 @@ export type Features = {
 export const useFeatures = (): Features => {
   const health = useAppSelector(selectHealth);
   const plusHealth = useAppSelector(selectPlusHealth);
-  const allSystems = useAppSelector(selectAllSystems);
+  const systemsCount = useAppSelector(selectSystemsCount);
   const initialConnections = useAppSelector(selectInitialConnections);
 
   const version = health?.version;
@@ -163,8 +163,6 @@ export const useFeatures = (): Features => {
 
   const tcf = plusHealth ? !!plusHealth.tcf.enabled : false;
 
-  const systemsCount = allSystems?.length ?? 0;
-
   const connectionsCount = initialConnections?.total ?? 0;
 
   const { flags } = useFlags();

diff --git a/clients/admin-ui/src/features/common/system-data-flow/DataFlowAccordionForm.tsx b/clients/admin-ui/src/features/common/system-data-flow/DataFlowAccordionForm.tsx
@@ -22,12 +22,10 @@ import {
 import { Form, Formik, FormikHelpers } from "formik";
 import React, { useEffect, useMemo, useState } from "react";
 
-import { useAppSelector } from "~/app/hooks";
 import {
   useGetAllSystemsQuery,
   useUpdateSystemMutation,
 } from "~/features/system";
-import { selectAllSystems } from "~/features/system/system.slice";
 import { DataFlow, System } from "~/types/api";
 
 const defaultInitialValues = {
@@ -53,8 +51,7 @@ export const DataFlowAccordionForm = ({
   const dataFlowSystemsModal = useDisclosure();
   const [updateSystemMutationTrigger] = useUpdateSystemMutation();
 
-  useGetAllSystemsQuery();
-  const systems = useAppSelector(selectAllSystems);
+  const { data: systems = [] } = useGetAllSystemsQuery();
 
   const initialDataFlows = useMemo(() => {
     let dataFlows = isIngress ? system.ingress : system.egress;

diff --git a/clients/admin-ui/src/features/config-wizard/ScanResults.tsx b/clients/admin-ui/src/features/config-wizard/ScanResults.tsx
@@ -20,19 +20,14 @@ import { useAPIHelper } from "~/features/common/hooks";
 import { useSystemOrDatamapRoute } from "~/features/common/hooks/useSystemOrDatamapRoute";
 import WarningModal from "~/features/common/modals/WarningModal";
 import { SystemsCheckboxTable } from "~/features/common/SystemsCheckboxTable";
-import {
-  setSystemsToClassify,
-  useUpsertSystemsMutation,
-} from "~/features/system";
+import { useUpsertSystemsMutation } from "~/features/system";
 import { System } from "~/types/api";
 
 import {
   changeStep,
   reset,
-  selectAddSystemsMethod,
   selectSystemsForReview,
 } from "./config-wizard.slice";
-import { SystemMethods } from "./types";
 
 const ALL_COLUMNS: ColumnMetadata[] = [
   { name: "Name", attribute: "name" },
@@ -55,7 +50,6 @@ const ScanResults = () => {
   const [selectedSystems, setSelectedSystems] = useState<System[]>(systems);
   const [selectedColumns, setSelectedColumns] =
     useState<ColumnMetadata[]>(ALL_COLUMNS);
-  const method = useAppSelector(selectAddSystemsMethod);
   const { handleError } = useAPIHelper();
 
   /**
@@ -77,15 +71,6 @@ const ScanResults = () => {
       return handleError(response.error);
     }
 
-    /*
-     * Eventually, all scanners will go through some sort of classify flow.
-     * But for now, only the data flow scanner does
-     */
-    if (method === SystemMethods.DATA_FLOW) {
-      dispatch(setSystemsToClassify(selectedSystems));
-      return navigateAndReset("/classify-systems");
-    }
-
     return navigateAndReset(systemOrDatamapRoute);
   };
 

diff --git a/clients/admin-ui/src/features/configure-consent/AddVendor.tsx b/clients/admin-ui/src/features/configure-consent/AddVendor.tsx
@@ -20,7 +20,10 @@ import {
   selectDictEntry,
   useGetAllDictionaryEntriesQuery,
 } from "~/features/plus/plus.slice";
-import { selectAllSystems, useCreateSystemMutation } from "~/features/system";
+import {
+  useCreateSystemMutation,
+  useLazyGetSystemsQuery,
+} from "~/features/system";
 import {
   selectLockedForGVL,
   selectSuggestions,
@@ -64,25 +67,30 @@ const AddVendor = ({
 
   const dispatch = useAppDispatch();
 
-  const systems = useAppSelector(selectAllSystems);
+  const [getSystemQueryTrigger] = useLazyGetSystemsQuery();
 
   const ValidationSchema = useMemo(
     () =>
       Yup.object().shape({
         name: Yup.string()
           .required()
           .label("Vendor name")
-          .test("is-unique", "", (value, context) => {
-            const takenSystemNames = systems.map((s) => s.name);
-            if (takenSystemNames.some((name) => name === value)) {
+          .test("is-unique", "", async (value, context) => {
+            const { data } = await getSystemQueryTrigger({
+              page: 1,
+              size: 10,
+              search: value,
+            });
+            const similarSystemNames = data?.items || [];
+            if (similarSystemNames.some((s) => s.name === value)) {
               return context.createError({
                 message: `You already have a vendor called "${value}". Please specify a unique name for this vendor.`,
               });
             }
             return true;
           }),
       }),
-    [systems]
+    [getSystemQueryTrigger]
   );
 
   // Subscribe and get dictionary values

diff --git a/clients/admin-ui/src/features/plus/plus.slice.ts b/clients/admin-ui/src/features/plus/plus.slice.ts
@@ -10,7 +10,6 @@ import {
 } from "~/features/dataset/dataset.slice";
 import { CreateSaasConnectionConfig } from "~/features/datastore-connections";
 import { CreateSaasConnectionConfigResponse } from "~/features/datastore-connections/types";
-import { selectSystemsToClassify } from "~/features/system";
 import {
   AllowList,
   AllowListUpdate,
@@ -507,15 +506,6 @@ export const selectDatasetClassifyInstances = createSelector(
   ({ data: instances }) => instances ?? emptyClassifyInstances
 );
 
-export const selectSystemClassifyInstances = createSelector(
-  [(state) => state, selectSystemsToClassify],
-  (state, systems) =>
-    plusApi.endpoints.getAllClassifyInstances.select({
-      resource_type: GenerateTypes.SYSTEMS,
-      fides_keys: systems?.map((s) => s.fides_key),
-    })(state)?.data ?? emptyClassifyInstances
-);
-
 const emptyClassifyInstanceMap: Map<string, ClassifyInstanceResponseValues> =
   new Map();
 
@@ -537,11 +527,6 @@ export const selectDatasetClassifyInstanceMap = createSelector(
   (instances) => instancesToMap(instances)
 );
 
-export const selectSystemClassifyInstanceMap = createSelector(
-  selectSystemClassifyInstances,
-  (instances) => instancesToMap(instances)
-);
-
 /**
  * This is the root of ClassifyInstance selectors that parallel the dataset's structure. These used
  * the cached getClassifyDataset response state, which is a query using  the "active" dataset's