From d2d5b1fa56a0ef8ed1b44140658308342a462dae Mon Sep 17 00:00:00 2001 From: Scott J Dickerson Date: Fri, 17 Nov 2023 17:46:57 -0500 Subject: [PATCH] Refactor `WithUiId` handling to use hook `useWithUiId()` 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 | 53 +++++++----------- client/src/app/queries/issues.ts | 56 ++++++++++--------- client/src/app/utils/query-utils.ts | 33 +++++++++++ 8 files changed, 103 insertions(+), 65 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 2f2c071027..b48d8449fb 100644 --- a/client/src/app/api/models.ts +++ b/client/src/app/api/models.ts @@ -575,8 +575,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..13960729dd 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 do 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 7229f9015c..cd8a47ac92 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, @@ -216,7 +216,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..d0f2e15688 100644 --- a/client/src/app/queries/dependencies.ts +++ b/client/src/app/queries/dependencies.ts @@ -1,4 +1,3 @@ -import { useMemo } from "react"; import { useQuery } from "@tanstack/react-query"; import { AnalysisAppDependency, @@ -8,26 +7,21 @@ import { WithUiId, } from "@app/api/models"; import { getAppDependencies, getDependencies } from "@app/api/rest"; +import { useWithUiId } from "@app/utils/query-utils"; -export interface IDependenciesFetchState { +export const DependenciesQueryKey = "dependencies"; +export const AppDependenciesQueryKey = "appDependencies"; + +export interface IFetchDependenciesState { result: HubPaginatedResult>; isFetching: boolean; fetchError: unknown; refetch: () => void; } -export interface IAppDependenciesFetchState { - result: HubPaginatedResult; - isFetching: boolean; - fetchError: unknown; - refetch: () => void; -} - -export const DependenciesQueryKey = "dependencies"; -export const AppDependenciesQueryKey = "appDependencies"; export const useFetchDependencies = ( params: HubRequestParams = {} -): IDependenciesFetchState => { +): IFetchDependenciesState => { const { data, isLoading, error, refetch } = useQuery({ queryKey: [DependenciesQueryKey, params], queryFn: async () => await getDependencies(params), @@ -35,36 +29,29 @@ export const useFetchDependencies = ( 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(data?.data, (d) => `${d.name}/${d.provider}`); return { - result, + result: { + data: withUiId, + total: data?.total ?? 0, + params: data?.params ?? params, + }, isFetching: isLoading, fetchError: error, refetch, }; }; +export interface IFetchAppDependenciesState { + result: HubPaginatedResult; + isFetching: boolean; + fetchError: unknown; + refetch: () => void; +} + export const useFetchAppDependencies = ( params: HubRequestParams = {} -): IAppDependenciesFetchState => { +): IFetchAppDependenciesState => { 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..e6d93a35a1 100644 --- a/client/src/app/queries/issues.ts +++ b/client/src/app/queries/issues.ts @@ -2,11 +2,8 @@ import { useQuery } from "@tanstack/react-query"; import { AnalysisIssueReport, AnalysisRuleReport, - BaseAnalysisIssueReport, - BaseAnalysisRuleReport, HubPaginatedResult, HubRequestParams, - WithUiId, } from "@app/api/models"; import { getRuleReports, @@ -17,6 +14,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 +24,32 @@ 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 interface IFetchRuleReportsState { + result: HubPaginatedResult; + isFetching: boolean; + fetchError: unknown; + refetch: () => void; +} export const useFetchRuleReports = ( enabled: boolean, params: HubRequestParams = {} -) => { +): IFetchRuleReportsState => { const { data, 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(data?.data, (r) => `${r.ruleset}/${r.rule}`); return { - result: data || { data: [], total: 0, params }, + result: { + data: withUiId, + total: data?.total ?? 0, + params: data?.params ?? params, + }, isFetching: isLoading, fetchError: error, refetch, @@ -78,21 +71,32 @@ export const useFetchAppReports = (params: HubRequestParams = {}) => { }; }; +export interface IFetchIssueReportsState { + result: HubPaginatedResult; + isFetching: boolean; + fetchError: unknown; + refetch: () => void; +} + export const useFetchIssueReports = ( applicationId?: number, params: HubRequestParams = {} -) => { +): IFetchIssueReportsState => { const { data, 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(data?.data, (r) => `${r.ruleset}/${r.rule}`); return { - result: data || { data: [], total: 0, params }, + result: { + data: withUiId, + total: data?.total ?? 0, + params: data?.params ?? params, + }, 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; +};