From 03ee68cfb11888e208a795ca6115d28e8676c5c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morten=20H=C3=B8fft?= <6759267+MortenHofft@users.noreply.github.com> Date: Wed, 23 Oct 2024 14:29:11 +0200 Subject: [PATCH] fix: statemanagement issues. closes #645 #646 #647 #648 --- packages/gbif-org/src/contexts/filter.tsx | 106 ++++++++++-------- .../filterAdapter/filter2predicate.test.ts | 4 +- .../filterAdapter/filter2predicate.tsx | 4 +- .../filterAdapter/filter2v1.tsx | 12 +- .../filterAdapter/useFilterParams.ts | 19 ++-- .../dataManagement/filterAdapter/v12filter.ts | 6 +- packages/gbif-org/src/utils/hash.tsx | 16 +++ packages/gbif-org/src/utils/querystring.tsx | 21 +++- 8 files changed, 117 insertions(+), 71 deletions(-) create mode 100644 packages/gbif-org/src/utils/hash.tsx diff --git a/packages/gbif-org/src/contexts/filter.tsx b/packages/gbif-org/src/contexts/filter.tsx index d6d1fae5b..33dac3342 100644 --- a/packages/gbif-org/src/contexts/filter.tsx +++ b/packages/gbif-org/src/contexts/filter.tsx @@ -8,21 +8,31 @@ import get from 'lodash/get'; import isEqual from 'react-fast-compare'; import hash from 'object-hash'; -export const FilterContext = React.createContext(undefined); +export const FilterContext = React.createContext({ + setField: () => ({}), + setFullField: () => ({}), + setFilter: () => ({}), + add: () => ({}), + remove: () => ({}), + toggle: () => ({}), + negateField: () => ({}), + filter: { must: {} }, + filterHash: '', +}); export type FilterType = { must?: Record; - must_not?: Record; + mustNot?: Record; }; export type FilterContextType = { - setFilter: (filter: FilterType | undefined) => FilterType | undefined; - setField: (field: string, value: any[], isNegated?: boolean) => void; - setFullField: (field: string, must: any[], mustNot: any[]) => void; - add: (field: string, value: any, isNegated?: boolean) => void; - remove: (field: string, value: any, isNegated?: boolean) => void; - toggle: (field: string, value: any, isNegated?: boolean) => void; - negateField: (field: string, isNegated?: boolean) => void; + setFilter: (filter: FilterType) => FilterType; + setField: (field: string, value: any[], isNegated?: boolean) => FilterType; + setFullField: (field: string, must: any[], mustNot: any[]) => FilterType; + add: (field: string, value: any, isNegated?: boolean) => FilterType; + remove: (field: string, value: any, isNegated?: boolean) => FilterType; + toggle: (field: string, value: any, isNegated?: boolean) => FilterType; + negateField: (field: string, isNegated?: boolean) => FilterType; filter: FilterType; filterHash: string; }; @@ -30,24 +40,24 @@ export type FilterContextType = { export function FilterProvider({ filter: controlledFilter, onChange: controlledOnChange, children }: { filter?: FilterType; onChange?: (filter: FilterType) => void; children: React.ReactNode }) { const [currentFilter, onChange] = useUncontrolledProp(controlledFilter, {}, controlledOnChange); - const setFilter = (filter: FilterType | undefined) => { + const setFilter = (filter: FilterType): FilterType => { if (isEqual(filter, currentFilter)) { - return; + return currentFilter; } if (typeof filter === 'object') { filter = cleanUpFilter(cloneDeep(filter)); - if (isEmpty(filter.must)) delete filter.must; - if (isEmpty(filter.must_not)) delete filter.must_not; - if (Object.keys(filter).length === 0) filter = undefined; + if (isEmpty(filter?.must)) delete filter.must; + if (isEmpty(filter?.mustNot)) delete filter.mustNot; + if (Object.keys(filter).length === 0) filter = {}; } onChange(filter || {}); return filter; }; - const setField = (field: string, value: any[], isNegated?: boolean) => { + const setField = (field: string, value: any[], isNegated?: boolean): FilterType => { const filter = currentFilter ? cloneDeep(currentFilter) : {}; - const type = isNegated ? 'must_not' : 'must'; - setFilter({ + const type = isNegated ? 'mustNot' : 'must'; + return setFilter({ ...filter, [type]: { ...filter[type], @@ -56,31 +66,31 @@ export function FilterProvider({ filter: controlledFilter, onChange: controlledO }); }; - const setFullField = (field: string, must: any[], mustNot: any[]) => { + const setFullField = (field: string, must: any[], mustNot: any[]): FilterType => { const filter = currentFilter ? cloneDeep(currentFilter) : {}; const result = setFilter({ ...filter, must: { ...filter.must, - [field]: mustNot, - }, - must_not: { - ...filter.must_not, [field]: must, }, + mustNot: { + ...filter.mustNot, + [field]: mustNot, + }, }); return result; }; - const negateField = (field: string, isNegated?: boolean) => { + const negateField = (field: string, isNegated?: boolean): FilterType => { const filter = currentFilter ? cloneDeep(currentFilter) : {}; - let must = get(filter, `must.${field}`, []); - let mustNot = get(filter, `must_not.${field}`, []); - let value = [...must, ...mustNot]; + const must = get(filter, `must.${field}`, []); + const mustNot = get(filter, `mustNot.${field}`, []); + const value = [...must, ...mustNot]; const uniqValues = uniqWith(value, isEqual); - const typeToSet = isNegated ? 'must_not' : 'must'; - const typeToRemove = !isNegated ? 'must_not' : 'must'; - setFilter({ + const typeToSet = isNegated ? 'mustNot' : 'must'; + const typeToRemove = !isNegated ? 'mustNot' : 'must'; + return setFilter({ ...filter, [typeToSet]: { ...filter[typeToSet], @@ -93,45 +103,39 @@ export function FilterProvider({ filter: controlledFilter, onChange: controlledO }); }; - const add = (field: string, value: any, isNegated?: boolean) => { - const type = isNegated ? 'must_not' : 'must'; + const add = (field: string, value: any, isNegated?: boolean): FilterType => { + const type = isNegated ? 'mustNot' : 'must'; let values = get(currentFilter, `${type}.${field}`, []); values = values.concat(value); values = uniqWith(values, isEqual); - setField(field, values, isNegated); + return setField(field, values, isNegated); }; - const remove = (field: string, value: any, isNegated?: boolean) => { - const type = isNegated ? 'must_not' : 'must'; + const remove = (field: string, value: any, isNegated?: boolean): FilterType => { + const type = isNegated ? 'mustNot' : 'must'; let values = get(currentFilter, `${type}.${field}`, []); values = values.filter((e) => !isEqual(e, value)); - setField(field, values, isNegated); + return setField(field, values, isNegated); }; - const toggle = (field: string, value: any, isNegated?: boolean) => { - const type = isNegated ? 'must_not' : 'must'; - let values = get(currentFilter, `${type}.${field}`, []); + const toggle = (field: string, value: any, isNegated?: boolean): FilterType => { + const type = isNegated ? 'mustNot' : 'must'; + const values = get(currentFilter, `${type}.${field}`, []); if (values.some((e) => isEqual(e, value))) { - remove(field, value, isNegated); + return remove(field, value, isNegated); } else { - add(field, value, isNegated); + return add(field, value, isNegated); } }; - const cleanUpFilter = (filter: FilterType) => { - const must = pickBy(get(filter, 'must', {}), (x) => !isEmpty(x)); - const must_not = pickBy(get(filter, 'must_not', {}), (x) => !isEmpty(x)); - return { must, must_not }; - }; - const hashObj = { must: currentFilter?.must || {}, - must_not: currentFilter?.must_not || {}, + mustNot: currentFilter?.mustNot || {}, }; const filterHash = hash(hashObj); const contextValue = { setField, // updates a single field - setFullField, // updates a single field both must and must_not. Ugly hack as I couldn't get it to work begint to calls. The problem is that the filter isn't updated between the two calls in the event loop and hence the first update is ignored + setFullField, // updates a single field both must and mustNot. Ugly hack as I couldn't get it to work begint to calls. The problem is that the filter isn't updated between the two calls in the event loop and hence the first update is ignored setFilter, // updates the filter as a whole add, remove, @@ -148,3 +152,9 @@ export function FilterProvider({ filter: controlledFilter, onChange: controlledO ); } + +export const cleanUpFilter = (filter: FilterType): FilterType => { + const must = pickBy(get(filter, 'must', {}), (x) => !isEmpty(x)); + const mustNot = pickBy(get(filter, 'mustNot', {}), (x) => !isEmpty(x)); + return { must, mustNot }; +}; \ No newline at end of file diff --git a/packages/gbif-org/src/dataManagement/filterAdapter/filter2predicate.test.ts b/packages/gbif-org/src/dataManagement/filterAdapter/filter2predicate.test.ts index 115d2b9f6..a4d53565b 100644 --- a/packages/gbif-org/src/dataManagement/filterAdapter/filter2predicate.test.ts +++ b/packages/gbif-org/src/dataManagement/filterAdapter/filter2predicate.test.ts @@ -233,8 +233,8 @@ test('it can do pre transformations of the filter', () => { ...filter.must, customField: ['always_there'], }, - must_not: { - ...filter.must_not, + mustNot: { + ...filter.mustNot, }, }; }, diff --git a/packages/gbif-org/src/dataManagement/filterAdapter/filter2predicate.tsx b/packages/gbif-org/src/dataManagement/filterAdapter/filter2predicate.tsx index b40531eec..1e1662b31 100644 --- a/packages/gbif-org/src/dataManagement/filterAdapter/filter2predicate.tsx +++ b/packages/gbif-org/src/dataManagement/filterAdapter/filter2predicate.tsx @@ -32,10 +32,10 @@ export function filter2predicate(filter: FilterType | undefined | null, filterCo if (filterConfig?.preFilterTransform) { filter = filterConfig?.preFilterTransform(filter); } - const { must, must_not } = filter; + const { must, mustNot } = filter; const positive = getPredicates({ filters: must, filterConfig }); - const negated = getPredicates({ filters: must_not, filterConfig }).map((p):Predicate => ({ type: PredicateType.Not, predicate: p })); + const negated = getPredicates({ filters: mustNot, filterConfig }).map((p):Predicate => ({ type: PredicateType.Not, predicate: p })); const predicates = positive.concat(negated); diff --git a/packages/gbif-org/src/dataManagement/filterAdapter/filter2v1.tsx b/packages/gbif-org/src/dataManagement/filterAdapter/filter2v1.tsx index f5230e1de..08503932a 100644 --- a/packages/gbif-org/src/dataManagement/filterAdapter/filter2v1.tsx +++ b/packages/gbif-org/src/dataManagement/filterAdapter/filter2v1.tsx @@ -25,23 +25,23 @@ export function filter2v1( if (filterConfig?.preFilterTransform) { filter = filterConfig?.preFilterTransform(filter); } - const { must, must_not } = filter; + const { must, mustNot } = filter; - let composedFilter: { [key: string]: ValuesType } = {}; - let errors: ErrorType[] = []; + const composedFilter: { [key: string]: ValuesType } = {}; + const errors: ErrorType[] = []; if (must) Object.entries(must) .filter(([, values]) => values) .forEach(([filterName, values]) => { const fieldFilter = getField({ filterName, values, filterConfig, errors }); - if (fieldFilter) composedFilter[fieldFilter.name] = fieldFilter.values; + if (fieldFilter?.values) composedFilter[fieldFilter.name] = fieldFilter.values; }); // Negation support removed as discussed in https://github.com/gbif/hosted-portals/issues/209 // Previous version in https://github.com/gbif/gbif-web/blob/8720cf9c9df0df089c4f54462d0b12c1696fffd1/packages/react-components/src/dataManagement/filterAdapter/filter2v1.js#L22 - if (must_not) { - const negatedFields = Object.entries(must_not).filter(([, values]) => values); + if (mustNot) { + const negatedFields = Object.entries(mustNot).filter(([, values]) => values); if (negatedFields.length > 0) { errors.push({ errorType: 'UNSUPPORTED_NEGATION', diff --git a/packages/gbif-org/src/dataManagement/filterAdapter/useFilterParams.ts b/packages/gbif-org/src/dataManagement/filterAdapter/useFilterParams.ts index 0cf97d896..815a70333 100644 --- a/packages/gbif-org/src/dataManagement/filterAdapter/useFilterParams.ts +++ b/packages/gbif-org/src/dataManagement/filterAdapter/useFilterParams.ts @@ -1,11 +1,12 @@ import { useCallback, useState, useEffect } from 'react'; import { filter2v1 } from '.'; -import { FilterType } from '../../contexts/filter'; -import { FilterConfigType, FieldType } from './filter2predicate'; +import { cleanUpFilter, FilterType } from '../../contexts/filter'; +import { FilterConfigType } from './filter2predicate'; import isPlainObject from 'lodash/isPlainObject'; import { useSearchParams } from 'react-router-dom'; import { asStringParams, ParamQuery, parseParams } from '@/utils/querystring'; import v12filter from './v12filter'; +import objectHash from 'object-hash'; // function v12filter(query: any, filterConfig: FilterConfigType): FilterType { // const filter = {}; @@ -45,17 +46,19 @@ export function useFilterParams({ filterConfig }: { filterConfig: FilterConfigTy // Field names can change according to the configuration const setFilter = useCallback( (nextFilter: FilterType) => { - const { filter, errors } = filter2v1(nextFilter, filterConfig); + if (objectHash(cleanUpFilter(nextFilter)) === objectHash(cleanUpFilter(filter))) { + return; + } + const { filter: v1Filter, errors } = filter2v1(nextFilter, filterConfig); if (errors) { - debugger; // if we cannot serialize the filter to version 1 API, then just serialize the json and put it in the filter param setQuery({ ...emptyQuery, filter: nextFilter }); } else { - setQuery({ ...emptyQuery, ...filter }); + setQuery({ ...emptyQuery, ...v1Filter }); } // setQuery({q: 'sdf', country: ['DK', 'DE', 'SE']}); }, - [filterConfig, emptyQuery] + [filterConfig, emptyQuery, filter, setQuery] ); // Transform the query from the url to the naming the consumer prefers. @@ -78,8 +81,8 @@ function useQueryParams() { const [query, setQuery] = useState({}); // useCallback to to setsearchparams, but before doing so it should turn everything into string or array of strings - const updateQuery = useCallback((query: any) => { - const stringParams = asStringParams(query); + const updateQuery = useCallback((nextQuery: any) => { + const stringParams = asStringParams(nextQuery); setSearchParams(stringParams); }, [setSearchParams]); diff --git a/packages/gbif-org/src/dataManagement/filterAdapter/v12filter.ts b/packages/gbif-org/src/dataManagement/filterAdapter/v12filter.ts index 4d51c0e6d..dfe7146f2 100644 --- a/packages/gbif-org/src/dataManagement/filterAdapter/v12filter.ts +++ b/packages/gbif-org/src/dataManagement/filterAdapter/v12filter.ts @@ -13,7 +13,7 @@ export default function v12filter(query: ParamQuery, filterConfig: FilterConfigT const fields = filterConfig?.fields; if (!fields) return {}; - let reverseMap: { [key: string]: string } = Object.keys(fields).reduce( + const reverseMap: { [key: string]: string } = Object.keys(fields).reduce( (prev: { [key: string]: string }, fieldName) => { const from = fields[fieldName]?.defaultKey || fieldName; const to = fieldName; @@ -35,11 +35,11 @@ export default function v12filter(query: ParamQuery, filterConfig: FilterConfigT //if range type then transform values if (v1Types.includes('range')) { arrayValue = arrayValue.map((val: string): object => { - const parts = val.split(','); + const parts = (val + '').split(','); if (parts.length === 1) { return { type: 'equals', value: parts[0] }; } else { - let range : {type: string, value: {gte?: number | string, lte?: number | string}} = { + const range : {type: string, value: {gte?: number | string, lte?: number | string}} = { type: 'range', value: {}, }; diff --git a/packages/gbif-org/src/utils/hash.tsx b/packages/gbif-org/src/utils/hash.tsx new file mode 100644 index 000000000..0180b1bc8 --- /dev/null +++ b/packages/gbif-org/src/utils/hash.tsx @@ -0,0 +1,16 @@ +import stringify from 'fast-json-stable-stringify'; + +export function hash(obj: unknown) { + return strToHash(stringify(obj)); +} + +export const strToHash = function (str : string) { + let hash = 0; + if (str.length === 0) return hash; + for (let i = 0; i < str.length; i++) { + const chr = str.charCodeAt(i); + hash = ((hash << 5) - hash) + chr; + hash |= 0; // Convert to 32bit integer + } + return hash; +}; \ No newline at end of file diff --git a/packages/gbif-org/src/utils/querystring.tsx b/packages/gbif-org/src/utils/querystring.tsx index 509d24ef9..b793ba443 100644 --- a/packages/gbif-org/src/utils/querystring.tsx +++ b/packages/gbif-org/src/utils/querystring.tsx @@ -55,6 +55,23 @@ function tryParse(value: string): string | number | JSON { } catch (e) { // if JSON.parse fails, it is not a JSON string } - const parsed = parseFloat(jsonValue); - return isNaN(parsed) ? jsonValue : parsed; + if (isValidFloat(value)) { + return parseFloat(value); + } + return jsonValue; } + +function isValidFloat(str: string): boolean { + // First, parse the string as a float + const parsed = parseFloat(str); + + // Check if the parsed result is NaN (not a number) + if (isNaN(parsed)) { + return false; + } + // chat that the original str does not contain letters or characters beyond 0-9 and punctuation . + if (/[^0-9.]/.test(str)) { + return false; + } + return true; +} \ No newline at end of file