From b654645c56e8d0f66f41d9deb78dc115c2e18d0e Mon Sep 17 00:00:00 2001 From: Scott Dickerson Date: Thu, 14 Dec 2023 12:50:02 -0500 Subject: [PATCH] :seedling: 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` object. Tables using the UI id have been adjusted to use the Constant `UI_UNIQUE_ID` as the `WithUiId` table data `idProperty`. All uses of `WithUiId` are now handled the same way. --------- Signed-off-by: Scott J Dickerson --- client/src/app/Constants.ts | 6 ++ client/src/app/api/models.ts | 8 ++- client/src/app/hooks/table-controls/DOCS.md | 4 +- .../app/pages/dependencies/dependencies.tsx | 4 +- client/src/app/pages/issues/issues-table.tsx | 4 +- client/src/app/queries/dependencies.ts | 60 ++++++------------- client/src/app/queries/issues.ts | 59 ++++++++++-------- client/src/app/utils/query-utils.ts | 33 ++++++++++ 8 files changed, 104 insertions(+), 74 deletions(-) create mode 100644 client/src/app/utils/query-utils.ts diff --git a/client/src/app/Constants.ts b/client/src/app/Constants.ts index 59d8974062..81954b6c0f 100644 --- 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 index 8ae4ffcaa1..e3107eff72 100644 --- 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 & { _ui_unique_id: string }; + export type AnalysisRuleReport = WithUiId; export type AnalysisIssueReport = WithUiId; diff --git a/client/src/app/hooks/table-controls/DOCS.md b/client/src/app/hooks/table-controls/DOCS.md index 1b409e1b12..ab19bef9fe 100644 --- 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 index 0c61a698cb..4c4d347656 100644 --- 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 index db2e487fc8..401e724eba 100644 --- 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 = ({ 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 index 6fecde893f..24b155f672 100644 --- 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>; - isFetching: boolean; - fetchError: unknown; - refetch: () => void; -} -export interface IAppDependenciesFetchState { - result: HubPaginatedResult; - 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[] = 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>, 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 index ac66630ee5..4a0cf53755 100644 --- 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 -): HubPaginatedResult> => { - // 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 => ({ - ...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 => - 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>, 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 => - 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, isFetching: isLoading, fetchError: error, refetch, diff --git a/client/src/app/utils/query-utils.ts b/client/src/app/utils/query-utils.ts new file mode 100644 index 0000000000..84c1590aba --- /dev/null +++ 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 = ( + /** Source data to modify. */ + data: T[] | undefined, + /** Generate the unique id for a specific `T`. */ + generator: (item: T) => string +): WithUiId[] => { + const result = useMemo(() => { + if (!data || data.length === 0) { + return []; + } + + const dataWithUiId: WithUiId[] = data.map((item) => ({ + ...item, + [UI_UNIQUE_ID]: generator(item), + })); + + return dataWithUiId; + }, [data, generator]); + + return result; +};