From cdd87e5e89af85b5c311828d20d77639da756bea Mon Sep 17 00:00:00 2001 From: Radoslaw Szwajkowski Date: Sat, 14 Sep 2024 17:37:13 +0200 Subject: [PATCH] :bug: Support custom serializers in table hooks (#2079) Changes: 1. add persistence provider to feature level persistence (IFeaturePersistenceArgs) but not to table layer persistence (ITablePersistenceArgs). This simplifies the provider code (feature-specific providers). 2. remove meta-persistence target "default" - no target has the same effect. 3. when in persistence provider mode use default values when the deserialized value is nulish. This solves the problem of initialFilterValues being overwritten in target cards scenario (the hook is called more then once and useState() based logic fails to detect initial load). Using the newly added feature, store the filters selected in the Target step (Analysis wizard) inside react-hook form in the same way as other values. Resolves: https://issues.redhat.com/browse/MTA-3438 Signed-off-by: Radoslaw Szwajkowski Co-authored-by: Scott Dickerson Signed-off-by: DvoraShechter1 --- .../active-item/useActiveItemState.ts | 10 ++++- .../expansion/useExpansionState.ts | 14 +++++-- .../filtering/useFilterState.ts | 16 ++++++-- .../pagination/usePaginationState.ts | 12 ++++-- .../table-controls/sorting/useSortState.ts | 15 +++++-- client/src/app/hooks/table-controls/types.ts | 17 +++++++- .../table-controls/useTableControlState.ts | 36 +++++++++++----- client/src/app/hooks/usePersistentState.ts | 41 ++++++++++++++++++- .../analysis-wizard/analysis-wizard.tsx | 2 + .../applications/analysis-wizard/schema.ts | 2 + .../analysis-wizard/set-targets.tsx | 18 ++++++-- 11 files changed, 150 insertions(+), 33 deletions(-) diff --git a/client/src/app/hooks/table-controls/active-item/useActiveItemState.ts b/client/src/app/hooks/table-controls/active-item/useActiveItemState.ts index f0e3f3626f..54988da75d 100644 --- a/client/src/app/hooks/table-controls/active-item/useActiveItemState.ts +++ b/client/src/app/hooks/table-controls/active-item/useActiveItemState.ts @@ -1,5 +1,5 @@ import { parseMaybeNumericString } from "@app/utils/utils"; -import { IFeaturePersistenceArgs } from "../types"; +import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types"; import { usePersistentState } from "@app/hooks/usePersistentState"; /** @@ -76,7 +76,13 @@ export const useActiveItemState = < persistTo, key: "activeItem", } - : { persistTo }), + : isPersistenceProvider(persistTo) + ? { + persistTo: "provider", + serialize: persistTo.write, + deserialize: () => persistTo.read() as string | number | null, + } + : { persistTo: "state" }), }); return { activeItemId, setActiveItemId }; }; diff --git a/client/src/app/hooks/table-controls/expansion/useExpansionState.ts b/client/src/app/hooks/table-controls/expansion/useExpansionState.ts index ac3a873bbf..848e62d12b 100644 --- a/client/src/app/hooks/table-controls/expansion/useExpansionState.ts +++ b/client/src/app/hooks/table-controls/expansion/useExpansionState.ts @@ -1,6 +1,6 @@ import { usePersistentState } from "@app/hooks/usePersistentState"; import { objectKeys } from "@app/utils/utils"; -import { IFeaturePersistenceArgs } from "../types"; +import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types"; import { DiscriminatedArgs } from "@app/utils/type-utils"; /** @@ -93,7 +93,9 @@ export const useExpansionState = < ? { persistTo, keys: ["expandedCells"], - serialize: (expandedCellsObj) => { + serialize: ( + expandedCellsObj: Partial> + ) => { if (!expandedCellsObj || objectKeys(expandedCellsObj).length === 0) return { expandedCells: null }; return { expandedCells: JSON.stringify(expandedCellsObj) }; @@ -111,7 +113,13 @@ export const useExpansionState = < persistTo, key: "expandedCells", } - : { persistTo }), + : isPersistenceProvider(persistTo) + ? { + persistTo: "provider", + serialize: persistTo.write, + deserialize: () => persistTo.read() as TExpandedCells, + } + : { persistTo: "state" }), }); return { expandedCells, setExpandedCells }; }; diff --git a/client/src/app/hooks/table-controls/filtering/useFilterState.ts b/client/src/app/hooks/table-controls/filtering/useFilterState.ts index 2de43e1f7c..e52a43c28f 100644 --- a/client/src/app/hooks/table-controls/filtering/useFilterState.ts +++ b/client/src/app/hooks/table-controls/filtering/useFilterState.ts @@ -1,5 +1,5 @@ import { FilterCategory, IFilterValues } from "@app/components/FilterToolbar"; -import { IFeaturePersistenceArgs } from "../types"; +import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types"; import { usePersistentState } from "@app/hooks/usePersistentState"; import { serializeFilterUrlParams } from "./helpers"; import { deserializeFilterUrlParams } from "./helpers"; @@ -90,7 +90,6 @@ export const useFilterState = < "filters" >({ isEnabled: !!isFilterEnabled, - defaultValue: initialFilterValues, persistenceKeyPrefix, // Note: For the discriminated union here to work without TypeScript getting confused // (e.g. require the urlParams-specific options when persistTo === "urlParams"), @@ -99,12 +98,21 @@ export const useFilterState = < ? { persistTo, keys: ["filters"], + defaultValue: initialFilterValues, serialize: serializeFilterUrlParams, deserialize: deserializeFilterUrlParams, } : persistTo === "localStorage" || persistTo === "sessionStorage" - ? { persistTo, key: "filters" } - : { persistTo }), + ? { persistTo, key: "filters", defaultValue: initialFilterValues } + : isPersistenceProvider(persistTo) + ? { + persistTo: "provider", + serialize: persistTo.write, + deserialize: () => + persistTo.read() as IFilterValues, + defaultValue: isFilterEnabled ? args?.initialFilterValues ?? {} : {}, + } + : { persistTo: "state", defaultValue: initialFilterValues }), }); return { filterValues, setFilterValues }; }; diff --git a/client/src/app/hooks/table-controls/pagination/usePaginationState.ts b/client/src/app/hooks/table-controls/pagination/usePaginationState.ts index 6fd87c84b1..61f8b6e981 100644 --- a/client/src/app/hooks/table-controls/pagination/usePaginationState.ts +++ b/client/src/app/hooks/table-controls/pagination/usePaginationState.ts @@ -1,5 +1,5 @@ import { usePersistentState } from "@app/hooks/usePersistentState"; -import { IFeaturePersistenceArgs } from "../types"; +import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types"; import { DiscriminatedArgs } from "@app/utils/type-utils"; /** @@ -94,7 +94,7 @@ export const usePaginationState = < ? { persistTo, keys: ["pageNumber", "itemsPerPage"], - serialize: (state) => { + serialize: (state: Partial) => { const { pageNumber, itemsPerPage } = state || {}; return { pageNumber: pageNumber ? String(pageNumber) : undefined, @@ -116,7 +116,13 @@ export const usePaginationState = < persistTo, key: "pagination", } - : { persistTo }), + : isPersistenceProvider(persistTo) + ? { + persistTo: "provider", + serialize: persistTo.write, + deserialize: () => persistTo.read() as IActivePagination, + } + : { persistTo: "state" }), }); const { pageNumber, itemsPerPage } = paginationState || defaultValue; const setPageNumber = (num: number) => diff --git a/client/src/app/hooks/table-controls/sorting/useSortState.ts b/client/src/app/hooks/table-controls/sorting/useSortState.ts index fc583142e9..91d2840a1b 100644 --- a/client/src/app/hooks/table-controls/sorting/useSortState.ts +++ b/client/src/app/hooks/table-controls/sorting/useSortState.ts @@ -1,5 +1,5 @@ import { DiscriminatedArgs } from "@app/utils/type-utils"; -import { IFeaturePersistenceArgs } from ".."; +import { IFeaturePersistenceArgs, isPersistenceProvider } from ".."; import { usePersistentState } from "@app/hooks/usePersistentState"; /** @@ -96,7 +96,9 @@ export const useSortState = < ? { persistTo, keys: ["sortColumn", "sortDirection"], - serialize: (activeSort) => ({ + serialize: ( + activeSort: Partial | null> + ) => ({ sortColumn: activeSort?.columnKey || null, sortDirection: activeSort?.direction || null, }), @@ -113,7 +115,14 @@ export const useSortState = < persistTo, key: "sort", } - : { persistTo }), + : isPersistenceProvider(persistTo) + ? { + persistTo: "provider", + serialize: persistTo.write, + deserialize: () => + persistTo.read() as IActiveSort | null, + } + : { persistTo: "state" }), }); return { activeSort, setActiveSort }; }; diff --git a/client/src/app/hooks/table-controls/types.ts b/client/src/app/hooks/table-controls/types.ts index d2293c2348..38b8e00b8a 100644 --- a/client/src/app/hooks/table-controls/types.ts +++ b/client/src/app/hooks/table-controls/types.ts @@ -64,6 +64,17 @@ export type TableFeature = | "activeItem" | "columns"; +export interface PersistenceProvider { + write: (value: T) => void; + read: () => T; +} + +export const isPersistenceProvider = ( + persistTo?: PersistTarget | PersistenceProvider +): persistTo is PersistenceProvider => + !!(persistTo as PersistenceProvider)?.write && + !!(persistTo as PersistenceProvider)?.read; + /** * Identifier for where to persist state for a single table feature or for all table features. * - "state" (default) - Plain React state. Resets on component unmount or page reload. @@ -106,7 +117,7 @@ export type IFeaturePersistenceArgs< /** * Where to persist state for this feature. */ - persistTo?: PersistTarget; + persistTo?: PersistTarget | PersistenceProvider; }; export interface ColumnSetting { @@ -131,7 +142,9 @@ export type ITablePersistenceArgs< */ persistTo?: | PersistTarget - | Partial>; + | Partial< + Record> + >; }; /** diff --git a/client/src/app/hooks/table-controls/useTableControlState.ts b/client/src/app/hooks/table-controls/useTableControlState.ts index 7e9d1ca651..85b36543e2 100644 --- a/client/src/app/hooks/table-controls/useTableControlState.ts +++ b/client/src/app/hooks/table-controls/useTableControlState.ts @@ -1,7 +1,8 @@ import { + IFeaturePersistenceArgs, ITableControlState, + ITablePersistenceArgs, IUseTableControlStateArgs, - PersistTarget, TableFeature, } from "./types"; import { useFilterState } from "./filtering"; @@ -11,6 +12,21 @@ import { useActiveItemState } from "./active-item"; import { useExpansionState } from "./expansion"; import { useColumnState } from "./column/useColumnState"; +const getPersistTo = ({ + feature, + persistTo, +}: { + feature: TableFeature; + persistTo: ITablePersistenceArgs["persistTo"]; +}): { + persistTo: IFeaturePersistenceArgs["persistTo"]; +} => ({ + persistTo: + !persistTo || typeof persistTo === "string" + ? persistTo + : persistTo[feature], +}); + /** * Provides the "source of truth" state for all table features. * - State can be persisted in one or more configurable storage targets, either the same for the entire table or different targets per feature. @@ -41,31 +57,29 @@ export const useTableControlState = < TFilterCategoryKey, TPersistenceKeyPrefix > => { - const getPersistTo = (feature: TableFeature): PersistTarget | undefined => - !args.persistTo || typeof args.persistTo === "string" - ? args.persistTo - : args.persistTo[feature] || args.persistTo.default; - const filterState = useFilterState< TItem, TFilterCategoryKey, TPersistenceKeyPrefix - >({ ...args, persistTo: getPersistTo("filter") }); + >({ + ...args, + ...getPersistTo({ feature: "filter", persistTo: args.persistTo }), + }); const sortState = useSortState({ ...args, - persistTo: getPersistTo("sort"), + ...getPersistTo({ feature: "sort", persistTo: args.persistTo }), }); const paginationState = usePaginationState({ ...args, - persistTo: getPersistTo("pagination"), + ...getPersistTo({ persistTo: args.persistTo, feature: "pagination" }), }); const expansionState = useExpansionState({ ...args, - persistTo: getPersistTo("expansion"), + ...getPersistTo({ persistTo: args.persistTo, feature: "expansion" }), }); const activeItemState = useActiveItemState({ ...args, - persistTo: getPersistTo("activeItem"), + ...getPersistTo({ persistTo: args.persistTo, feature: "activeItem" }), }); const { columnNames, tableName, initialColumns } = args; diff --git a/client/src/app/hooks/usePersistentState.ts b/client/src/app/hooks/usePersistentState.ts index ccd50c452a..5dd6045fac 100644 --- a/client/src/app/hooks/usePersistentState.ts +++ b/client/src/app/hooks/usePersistentState.ts @@ -9,7 +9,15 @@ import { DisallowCharacters } from "@app/utils/type-utils"; type PersistToStateOptions = { persistTo?: "state" }; -type PersistToUrlParamsOptions< +type PersistToProvider = { + persistTo: "provider"; + defaultValue: TValue; + isEnabled?: boolean; + serialize: (params: TValue) => void; + deserialize: () => TValue; +}; + +export type PersistToUrlParamsOptions< TValue, TPersistenceKeyPrefix extends string, TURLParamKey extends string, @@ -33,6 +41,7 @@ export type UsePersistentStateOptions< | PersistToStateOptions | PersistToUrlParamsOptions | PersistToStorageOptions + | PersistToProvider ); export const usePersistentState = < @@ -92,7 +101,37 @@ export const usePersistentState = < ? { ...options, key: prefixKey(options.key) } : { ...options, isEnabled: false, key: "" } ), + provider: usePersistenceProvider( + isPersistenceProviderOptions(options) + ? options + : { + serialize: () => {}, + deserialize: () => defaultValue, + defaultValue, + isEnabled: false, + persistTo: "provider", + } + ), }; const [value, setValue] = persistence[persistTo || "state"]; return isEnabled ? [value, setValue] : [defaultValue, () => {}]; }; + +const usePersistenceProvider = ({ + serialize, + deserialize, + defaultValue, +}: PersistToProvider): [TValue, (val: TValue) => void] => { + // use default value if nulish value was deserialized + return [deserialize() ?? defaultValue, serialize]; +}; + +export const isPersistenceProviderOptions = < + TValue, + TPersistenceKeyPrefix extends string, + TURLParamKey extends string, +>( + o: Partial< + UsePersistentStateOptions + > +): o is PersistToProvider => o.persistTo === "provider"; diff --git a/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx b/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx index a432b85be9..05541b89b1 100644 --- a/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx +++ b/client/src/app/pages/applications/analysis-wizard/analysis-wizard.tsx @@ -166,6 +166,8 @@ export const AnalysisWizard: React.FC = ({ mode: "source-code-deps", formLabels: [], selectedTargets: [], + // defaults will be passed as initialFilterValues to the table hook + targetFilters: undefined, selectedSourceLabels: [], withKnownLibs: "app", includedPackages: [], diff --git a/client/src/app/pages/applications/analysis-wizard/schema.ts b/client/src/app/pages/applications/analysis-wizard/schema.ts index 770dbff5a2..a1294e7f23 100644 --- a/client/src/app/pages/applications/analysis-wizard/schema.ts +++ b/client/src/app/pages/applications/analysis-wizard/schema.ts @@ -57,12 +57,14 @@ const useModeStepSchema = ({ export interface TargetsStepValues { formLabels: TargetLabel[]; selectedTargets: Target[]; + targetFilters?: Record; } const useTargetsStepSchema = (): yup.SchemaOf => { return yup.object({ formLabels: yup.array(), selectedTargets: yup.array(), + targetFilters: yup.object(), }); }; diff --git a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx index 75d5730647..3b255d375e 100644 --- a/client/src/app/pages/applications/analysis-wizard/set-targets.tsx +++ b/client/src/app/pages/applications/analysis-wizard/set-targets.tsx @@ -101,7 +101,7 @@ interface SetTargetsInternalProps { isLoading: boolean; isError: boolean; languageProviders: string[]; - initialFilters: string[]; + applicationProviders: string[]; } const SetTargetsInternal: React.FC = ({ @@ -109,7 +109,7 @@ const SetTargetsInternal: React.FC = ({ isLoading, isError, languageProviders, - initialFilters = [], + applicationProviders = [], }) => { const { t } = useTranslation(); @@ -177,13 +177,23 @@ const SetTargetsInternal: React.FC = ({ tableName: "target-cards", items: targets, idProperty: "name", - initialFilterValues: { name: initialFilters }, + initialFilterValues: { name: applicationProviders }, columnNames: { name: "name", }, isFilterEnabled: true, isPaginationEnabled: false, isLoading, + persistTo: { + filter: { + write(value) { + setValue("targetFilters", value as Record); + }, + read() { + return getValues().targetFilters; + }, + }, + }, filterCategories: [ { selectOptions: languageProviders?.map((language) => ({ @@ -281,7 +291,7 @@ export const SetTargets: React.FC = ({ applications }) => { return (