🌱 Refactor WithUiId handling to use hook useWithUiId() (#1555)

Following up on #1554, create hook `useWithUiId()` to handle injecting UI id to objects. Any object `T` going will to the hook will come out as a `WithUiId<T>` object. Tables using the UI id have been adjusted to use the Constant `UI_UNIQUE_ID` as the `WithUiId<T>` table data `idProperty`. All uses of `WithUiId<T>` are now handled the same way. --------- Signed-off-by: Scott J Dickerson <[email protected]>
konveyor · Dec 14, 2023 · b654645 · b654645
1 parent 1a2f183
commit b654645
Show file tree

Hide file tree

Showing 8 changed files with 104 additions and 74 deletions.
diff --git a/client/src/app/Constants.ts b/client/src/app/Constants.ts
@@ -24,6 +24,12 @@ export const isRWXSupported = ENV.RWX_SUPPORTED === "true";
 
 export const DEFAULT_SELECT_MAX_HEIGHT = 200;
 
+/**
+ * The name of the client generated id field inserted in a object marked with mixin type
+ * `WithUiId`.
+ */
+export const UI_UNIQUE_ID = "_ui_unique_id";
+
 // Colors
 
 // t('colors.red')

diff --git a/client/src/app/api/models.ts b/client/src/app/api/models.ts
@@ -578,8 +578,14 @@ export interface BaseAnalysisIssueReport extends AnalysisIssuesCommonFields {
   files: number;
 }
 
-// After fetching from the hub, we inject a unique id composed of ruleset+rule for convenience
+/**
+ * Mark an object as having a unique client generated id field.  Use this type if
+ * an objects from hub does not have a single field with a unique key AND the object
+ * is to be used in a table.  Our table handlers assume a single field with a unique
+ * value across all objects in a set to properly handle row selections.
+ */
 export type WithUiId<T> = T & { _ui_unique_id: string };
+
 export type AnalysisRuleReport = WithUiId<BaseAnalysisRuleReport>;
 export type AnalysisIssueReport = WithUiId<BaseAnalysisIssueReport>;
 

diff --git a/client/src/app/hooks/table-controls/DOCS.md b/client/src/app/hooks/table-controls/DOCS.md
@@ -514,7 +514,9 @@ Table columns are identified by unique keys which are statically inferred from t
 
 #### Item IDs
 
-Item objects must contain some unique identifier which is either a string or number. The property key of this identifier is a required config argument called `idProperty`, which will usually be `"id"`. If no unique identifier is present in the API data, an artificial one can be injected before passing the data into these hooks, which can be done in the useQuery `select` callback (see instances where we have used `"_ui_unique_id"`). Any state which keeps track of something by item (i.e. by row) makes use of `item[idProperty]` as an identifier. Examples of this include selected rows, expanded rows and active rows. Valid `idProperty` values are also enforced by TypeScript generics; if an `idProperty` is provided that is not a property on the `TItem` type, you should get a type error.
+Item objects must contain some unique identifier which is either a string or number. The property key of this identifier is a required config argument called `idProperty`, which will usually be `"id"`. If no unique identifier is present in the API data, an artificial one can be injected before passing the data into these hooks. This can be done in the useQuery `select` callback (see instances where we have used `"_ui_unique_id"`). Another option is to use the query hook `useWithUiId()` on the react-query fetched data. Since `select` modified data is not part of the query cache, it does not matter if transforms are done in react-query, `useWithUiId` hook, or other means.
+
+Any state which keeps track of something by item (i.e. by row) makes use of `item[idProperty]` as an identifier. Examples of this include selected rows, expanded rows and active rows. Valid `idProperty` values are also enforced by TypeScript generics. If an `idProperty` is provided that is not a property on the `TItem` type, you should get a type error.
 
 > ⚠️ TECH DEBT NOTE: Things specific to `useQuery` and `_ui_unique_id` here are Konveyor-specific notes that should be removed after moving this to table-batteries.
 

diff --git a/client/src/app/pages/dependencies/dependencies.tsx b/client/src/app/pages/dependencies/dependencies.tsx
@@ -18,7 +18,7 @@ import {
   useTableControlProps,
   getHubRequestParams,
 } from "@app/hooks/table-controls";
-import { TablePersistenceKeyPrefix } from "@app/Constants";
+import { TablePersistenceKeyPrefix, UI_UNIQUE_ID } from "@app/Constants";
 import { SimplePagination } from "@app/components/SimplePagination";
 import {
   ConditionalTableBody,
@@ -97,7 +97,7 @@ export const Dependencies: React.FC = () => {
 
   const tableControls = useTableControlProps({
     ...tableControlState, // Includes filterState, sortState and paginationState
-    idProperty: "_ui_unique_id",
+    idProperty: UI_UNIQUE_ID,
     currentPageItems,
     totalItemCount,
     isLoading: isFetching,

diff --git a/client/src/app/pages/issues/issues-table.tsx b/client/src/app/pages/issues/issues-table.tsx
@@ -33,7 +33,7 @@ import { useSelectionState } from "@migtools/lib-ui";
 
 import { AppPlaceholder } from "@app/components/AppPlaceholder";
 import { OptionWithValue, SimpleSelect } from "@app/components/SimpleSelect";
-import { TablePersistenceKeyPrefix } from "@app/Constants";
+import { TablePersistenceKeyPrefix, UI_UNIQUE_ID } from "@app/Constants";
 import { useFetchIssueReports, useFetchRuleReports } from "@app/queries/issues";
 import {
   FilterType,
@@ -226,7 +226,7 @@ export const IssuesTable: React.FC<IIssuesTableProps> = ({ mode }) => {
 
   const tableControls = useTableControlProps({
     ...tableControlState, // Includes filterState, sortState and paginationState
-    idProperty: "_ui_unique_id",
+    idProperty: UI_UNIQUE_ID,
     currentPageItems: currentPageReports,
     totalItemCount: totalReportCount,
     isLoading,

diff --git a/client/src/app/queries/dependencies.ts b/client/src/app/queries/dependencies.ts
@@ -1,70 +1,46 @@
-import { useMemo } from "react";
 import { useQuery } from "@tanstack/react-query";
 import {
-  AnalysisAppDependency,
   AnalysisDependency,
   HubPaginatedResult,
   HubRequestParams,
   WithUiId,
 } from "@app/api/models";
 import { getAppDependencies, getDependencies } from "@app/api/rest";
-
-export interface IDependenciesFetchState {
-  result: HubPaginatedResult<WithUiId<AnalysisDependency>>;
-  isFetching: boolean;
-  fetchError: unknown;
-  refetch: () => void;
-}
-export interface IAppDependenciesFetchState {
-  result: HubPaginatedResult<AnalysisAppDependency>;
-  isFetching: boolean;
-  fetchError: unknown;
-  refetch: () => void;
-}
+import { useWithUiId } from "@app/utils/query-utils";
 
 export const DependenciesQueryKey = "dependencies";
 export const AppDependenciesQueryKey = "appDependencies";
 
-export const useFetchDependencies = (
-  params: HubRequestParams = {}
-): IDependenciesFetchState => {
-  const { data, isLoading, error, refetch } = useQuery({
+export const useFetchDependencies = (params: HubRequestParams = {}) => {
+  const {
+    data: dependencies,
+    isLoading,
+    error,
+    refetch,
+  } = useQuery({
     queryKey: [DependenciesQueryKey, params],
     queryFn: async () => await getDependencies(params),
     onError: (error) => console.log("error, ", error),
     keepPreviousData: true,
   });
 
-  const result = useMemo(() => {
-    if (!data) {
-      return { data: [], total: 0, params };
-    }
-
-    const syntheticData: WithUiId<AnalysisDependency>[] = data.data.map(
-      (dep) => ({
-        ...dep,
-        _ui_unique_id: `${dep.name}/${dep.provider}`,
-      })
-    );
-
-    return {
-      data: syntheticData,
-      total: data.total,
-      params: data.params,
-    };
-  }, [data, params]);
-
+  const withUiId = useWithUiId(
+    dependencies?.data,
+    (d) => `${d.name}/${d.provider}`
+  );
   return {
-    result,
+    result: {
+      data: withUiId,
+      total: dependencies?.total ?? 0,
+      params: dependencies?.params ?? params,
+    } as HubPaginatedResult<WithUiId<AnalysisDependency>>,
     isFetching: isLoading,
     fetchError: error,
     refetch,
   };
 };
 
-export const useFetchAppDependencies = (
-  params: HubRequestParams = {}
-): IAppDependenciesFetchState => {
+export const useFetchAppDependencies = (params: HubRequestParams = {}) => {
   const { data, isLoading, error, refetch } = useQuery({
     queryKey: [AppDependenciesQueryKey, params],
     queryFn: async () => await getAppDependencies(params),

diff --git a/client/src/app/queries/issues.ts b/client/src/app/queries/issues.ts
@@ -2,8 +2,6 @@ import { useQuery } from "@tanstack/react-query";
 import {
   AnalysisIssueReport,
   AnalysisRuleReport,
-  BaseAnalysisIssueReport,
-  BaseAnalysisRuleReport,
   HubPaginatedResult,
   HubRequestParams,
   WithUiId,
@@ -17,6 +15,7 @@ import {
   getIssueReports,
   getIssue,
 } from "@app/api/rest";
+import { useWithUiId } from "@app/utils/query-utils";
 
 export const RuleReportsQueryKey = "rulereports";
 export const AppReportsQueryKey = "appreports";
@@ -26,37 +25,33 @@ export const IssuesQueryKey = "issues";
 export const IssueQueryKey = "issue";
 export const IncidentsQueryKey = "incidents";
 
-const injectUiUniqueIds = <
-  T extends BaseAnalysisRuleReport | BaseAnalysisIssueReport,
->(
-  result: HubPaginatedResult<T>
-): HubPaginatedResult<WithUiId<T>> => {
-  // There is no single unique id property on some of the hub's composite report objects.
-  // We need to create one for table hooks to work.
-  const processedData = result.data.map(
-    (baseReport): WithUiId<T> => ({
-      ...baseReport,
-      _ui_unique_id: `${baseReport.ruleset}/${baseReport.rule}`,
-    })
-  );
-  return { ...result, data: processedData };
-};
-
 export const useFetchRuleReports = (
   enabled: boolean,
   params: HubRequestParams = {}
 ) => {
-  const { data, isLoading, error, refetch } = useQuery({
+  const {
+    data: ruleReport,
+    isLoading,
+    error,
+    refetch,
+  } = useQuery({
     queryKey: [RuleReportsQueryKey, params],
     queryFn: () => getRuleReports(params),
     onError: (error) => console.log("error, ", error),
     keepPreviousData: true,
-    select: (result): HubPaginatedResult<AnalysisRuleReport> =>
-      injectUiUniqueIds(result),
     enabled,
   });
+
+  const withUiId = useWithUiId(
+    ruleReport?.data,
+    (r) => `${r.ruleset}/${r.rule}`
+  );
   return {
-    result: data || { data: [], total: 0, params },
+    result: {
+      data: withUiId,
+      total: ruleReport?.total ?? 0,
+      params: ruleReport?.params ?? params,
+    } as HubPaginatedResult<WithUiId<AnalysisRuleReport>>,
     isFetching: isLoading,
     fetchError: error,
     refetch,
@@ -82,17 +77,29 @@ export const useFetchIssueReports = (
   applicationId?: number,
   params: HubRequestParams = {}
 ) => {
-  const { data, isLoading, error, refetch } = useQuery({
+  const {
+    data: issueReport,
+    isLoading,
+    error,
+    refetch,
+  } = useQuery({
     enabled: applicationId !== undefined,
     queryKey: [IssueReportsQueryKey, applicationId, params],
     queryFn: () => getIssueReports(applicationId, params),
     onError: (error) => console.log("error, ", error),
     keepPreviousData: true,
-    select: (result): HubPaginatedResult<AnalysisIssueReport> =>
-      injectUiUniqueIds(result),
   });
+
+  const withUiId = useWithUiId(
+    issueReport?.data,
+    (r) => `${r.ruleset}/${r.rule}`
+  );
   return {
-    result: data || { data: [], total: 0, params },
+    result: {
+      data: withUiId,
+      total: issueReport?.total ?? 0,
+      params: issueReport?.params ?? params,
+    } as HubPaginatedResult<AnalysisIssueReport>,
     isFetching: isLoading,
     fetchError: error,
     refetch,

diff --git a/client/src/app/utils/query-utils.ts b/client/src/app/utils/query-utils.ts
@@ -0,0 +1,33 @@
+import { useMemo } from "react";
+import { UI_UNIQUE_ID } from "@app/Constants";
+import { WithUiId } from "@app/api/models";
+
+/**
+ * Make a shallow copy of `data` and insert a new `UI_UNIQUE_ID` field in each element
+ * with the output of the `generator` function.  This hook allows generating the needed
+ * UI id field for any object that does not already have a unique id field so the object
+ * can be used with our table selection handlers.
+ *
+ * @returns A shallow copy of `T` with an added `UI_UNIQUE_ID` field.
+ */
+export const useWithUiId = <T>(
+  /** Source data to modify. */
+  data: T[] | undefined,
+  /** Generate the unique id for a specific `T`. */
+  generator: (item: T) => string
+): WithUiId<T>[] => {
+  const result = useMemo(() => {
+    if (!data || data.length === 0) {
+      return [];
+    }
+
+    const dataWithUiId: WithUiId<T>[] = data.map((item) => ({
+      ...item,
+      [UI_UNIQUE_ID]: generator(item),
+    }));
+
+    return dataWithUiId;
+  }, [data, generator]);
+
+  return result;
+};