fix: statemanagement issues. closes #645 #646 #647 #648

gbif · Oct 23, 2024 · 03ee68c · 03ee68c
1 parent 8c3ae9d
commit 03ee68c
Show file tree

Hide file tree

Showing 8 changed files with 117 additions and 71 deletions.
diff --git a/packages/gbif-org/src/contexts/filter.tsx b/packages/gbif-org/src/contexts/filter.tsx
@@ -8,46 +8,56 @@ import get from 'lodash/get';
 import isEqual from 'react-fast-compare';
 import hash from 'object-hash';
 
-export const FilterContext = React.createContext<FilterContextType | undefined >(undefined);
+export const FilterContext = React.createContext<FilterContextType>({
+  setField: () => ({}),
+  setFullField: () => ({}),
+  setFilter: () => ({}),
+  add: () => ({}),
+  remove: () => ({}),
+  toggle: () => ({}),
+  negateField: () => ({}),
+  filter: { must: {} },
+  filterHash: '',
+});
 
 export type FilterType = {
   must?: Record<string, any[]>;
-  must_not?: Record<string, any[]>;
+  mustNot?: Record<string, any[]>;
 };
 
 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;
 };
 
 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
     </FilterContext.Provider>
   );
 }
+
+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 };
+};
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
+};
diff --git 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;
+}