Skip to content

Commit

Permalink
🐛 Support custom serializers in table hooks (#2079)
Browse files Browse the repository at this point in the history
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 <rszwajko@redhat.com>
Co-authored-by: Scott Dickerson <sdickers@redhat.com>
  • Loading branch information
rszwajko and sjd78 authored Sep 14, 2024
1 parent 0d87a57 commit f389539
Show file tree
Hide file tree
Showing 11 changed files with 150 additions and 33 deletions.
Original file line number Diff line number Diff line change
@@ -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";

/**
Expand Down Expand Up @@ -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 };
};
14 changes: 11 additions & 3 deletions client/src/app/hooks/table-controls/expansion/useExpansionState.ts
Original file line number Diff line number Diff line change
@@ -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";

/**
Expand Down Expand Up @@ -93,7 +93,9 @@ export const useExpansionState = <
? {
persistTo,
keys: ["expandedCells"],
serialize: (expandedCellsObj) => {
serialize: (
expandedCellsObj: Partial<TExpandedCells<TColumnKey>>
) => {
if (!expandedCellsObj || objectKeys(expandedCellsObj).length === 0)
return { expandedCells: null };
return { expandedCells: JSON.stringify(expandedCellsObj) };
Expand All @@ -111,7 +113,13 @@ export const useExpansionState = <
persistTo,
key: "expandedCells",
}
: { persistTo }),
: isPersistenceProvider(persistTo)
? {
persistTo: "provider",
serialize: persistTo.write,
deserialize: () => persistTo.read() as TExpandedCells<TColumnKey>,
}
: { persistTo: "state" }),
});
return { expandedCells, setExpandedCells };
};
16 changes: 12 additions & 4 deletions client/src/app/hooks/table-controls/filtering/useFilterState.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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"),
Expand All @@ -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<TFilterCategoryKey>,
defaultValue: isFilterEnabled ? args?.initialFilterValues ?? {} : {},
}
: { persistTo: "state", defaultValue: initialFilterValues }),
});
return { filterValues, setFilterValues };
};
Original file line number Diff line number Diff line change
@@ -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";

/**
Expand Down Expand Up @@ -94,7 +94,7 @@ export const usePaginationState = <
? {
persistTo,
keys: ["pageNumber", "itemsPerPage"],
serialize: (state) => {
serialize: (state: Partial<IActivePagination>) => {
const { pageNumber, itemsPerPage } = state || {};
return {
pageNumber: pageNumber ? String(pageNumber) : undefined,
Expand All @@ -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) =>
Expand Down
15 changes: 12 additions & 3 deletions client/src/app/hooks/table-controls/sorting/useSortState.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DiscriminatedArgs } from "@app/utils/type-utils";
import { IFeaturePersistenceArgs } from "..";
import { IFeaturePersistenceArgs, isPersistenceProvider } from "..";
import { usePersistentState } from "@app/hooks/usePersistentState";

/**
Expand Down Expand Up @@ -96,7 +96,9 @@ export const useSortState = <
? {
persistTo,
keys: ["sortColumn", "sortDirection"],
serialize: (activeSort) => ({
serialize: (
activeSort: Partial<IActiveSort<TSortableColumnKey> | null>
) => ({
sortColumn: activeSort?.columnKey || null,
sortDirection: activeSort?.direction || null,
}),
Expand All @@ -113,7 +115,14 @@ export const useSortState = <
persistTo,
key: "sort",
}
: { persistTo }),
: isPersistenceProvider(persistTo)
? {
persistTo: "provider",
serialize: persistTo.write,
deserialize: () =>
persistTo.read() as IActiveSort<TSortableColumnKey> | null,
}
: { persistTo: "state" }),
});
return { activeSort, setActiveSort };
};
17 changes: 15 additions & 2 deletions client/src/app/hooks/table-controls/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,17 @@ export type TableFeature =
| "activeItem"
| "columns";

export interface PersistenceProvider<T> {
write: (value: T) => void;
read: () => T;
}

export const isPersistenceProvider = (
persistTo?: PersistTarget | PersistenceProvider<unknown>
): persistTo is PersistenceProvider<unknown> =>
!!(persistTo as PersistenceProvider<unknown>)?.write &&
!!(persistTo as PersistenceProvider<unknown>)?.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.
Expand Down Expand Up @@ -106,7 +117,7 @@ export type IFeaturePersistenceArgs<
/**
* Where to persist state for this feature.
*/
persistTo?: PersistTarget;
persistTo?: PersistTarget | PersistenceProvider<unknown>;
};

export interface ColumnSetting {
Expand All @@ -131,7 +142,9 @@ export type ITablePersistenceArgs<
*/
persistTo?:
| PersistTarget
| Partial<Record<TableFeature | "default", PersistTarget>>;
| Partial<
Record<TableFeature, PersistTarget | PersistenceProvider<unknown>>
>;
};

/**
Expand Down
36 changes: 25 additions & 11 deletions client/src/app/hooks/table-controls/useTableControlState.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {
IFeaturePersistenceArgs,
ITableControlState,
ITablePersistenceArgs,
IUseTableControlStateArgs,
PersistTarget,
TableFeature,
} from "./types";
import { useFilterState } from "./filtering";
Expand All @@ -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.
Expand Down Expand Up @@ -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<TSortableColumnKey, TPersistenceKeyPrefix>({
...args,
persistTo: getPersistTo("sort"),
...getPersistTo({ feature: "sort", persistTo: args.persistTo }),
});
const paginationState = usePaginationState<TPersistenceKeyPrefix>({
...args,
persistTo: getPersistTo("pagination"),
...getPersistTo({ persistTo: args.persistTo, feature: "pagination" }),
});
const expansionState = useExpansionState<TColumnKey, TPersistenceKeyPrefix>({
...args,
persistTo: getPersistTo("expansion"),
...getPersistTo({ persistTo: args.persistTo, feature: "expansion" }),
});
const activeItemState = useActiveItemState<TPersistenceKeyPrefix>({
...args,
persistTo: getPersistTo("activeItem"),
...getPersistTo({ persistTo: args.persistTo, feature: "activeItem" }),
});

const { columnNames, tableName, initialColumns } = args;
Expand Down
41 changes: 40 additions & 1 deletion client/src/app/hooks/usePersistentState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@ import { DisallowCharacters } from "@app/utils/type-utils";

type PersistToStateOptions = { persistTo?: "state" };

type PersistToUrlParamsOptions<
type PersistToProvider<TValue> = {
persistTo: "provider";
defaultValue: TValue;
isEnabled?: boolean;
serialize: (params: TValue) => void;
deserialize: () => TValue;
};

export type PersistToUrlParamsOptions<
TValue,
TPersistenceKeyPrefix extends string,
TURLParamKey extends string,
Expand All @@ -33,6 +41,7 @@ export type UsePersistentStateOptions<
| PersistToStateOptions
| PersistToUrlParamsOptions<TValue, TPersistenceKeyPrefix, TURLParamKey>
| PersistToStorageOptions<TValue>
| PersistToProvider<TValue>
);

export const usePersistentState = <
Expand Down Expand Up @@ -92,7 +101,37 @@ export const usePersistentState = <
? { ...options, key: prefixKey(options.key) }
: { ...options, isEnabled: false, key: "" }
),
provider: usePersistenceProvider<TValue>(
isPersistenceProviderOptions(options)
? options
: {
serialize: () => {},
deserialize: () => defaultValue,
defaultValue,
isEnabled: false,
persistTo: "provider",
}
),
};
const [value, setValue] = persistence[persistTo || "state"];
return isEnabled ? [value, setValue] : [defaultValue, () => {}];
};

const usePersistenceProvider = <TValue>({
serialize,
deserialize,
defaultValue,
}: PersistToProvider<TValue>): [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<TValue, TPersistenceKeyPrefix, TURLParamKey>
>
): o is PersistToProvider<TValue> => o.persistTo === "provider";
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ export const AnalysisWizard: React.FC<IAnalysisWizard> = ({
mode: "source-code-deps",
formLabels: [],
selectedTargets: [],
// defaults will be passed as initialFilterValues to the table hook
targetFilters: undefined,
selectedSourceLabels: [],
withKnownLibs: "app",
includedPackages: [],
Expand Down
2 changes: 2 additions & 0 deletions client/src/app/pages/applications/analysis-wizard/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,14 @@ const useModeStepSchema = ({
export interface TargetsStepValues {
formLabels: TargetLabel[];
selectedTargets: Target[];
targetFilters?: Record<string, string[]>;
}

const useTargetsStepSchema = (): yup.SchemaOf<TargetsStepValues> => {
return yup.object({
formLabels: yup.array(),
selectedTargets: yup.array(),
targetFilters: yup.object(),
});
};

Expand Down
Loading

0 comments on commit f389539

Please sign in to comment.