Fix URL update not reflected in list hook (#350)

* Fix URL update not reflected in list hook

* Maintain query prefs on tag click

ui/v2/src/hooks/ListHook.tsx

@@ -95,6 +95,21 @@ export interface IListHookOptions {
   renderSelectedOptions?: (result: QueryHookResult<any, any>, selectedIds: Set<string>) => JSX.Element | undefined;
 }
 
+function updateFromQueryString(queryStr: string, setFilter: (value: React.SetStateAction<ListFilterModel>) => void, forageData?: any) {
+  const queryParams = queryString.parse(queryStr);
+  setFilter((f) => {
+    const newFilter = _.cloneDeep(f);
+    newFilter.configureFromQueryParameters(queryParams);
+
+    if (forageData) {
+      const forageParams = queryString.parse(forageData.filter);
+      newFilter.overridePrefs(queryParams, forageParams);      
+    }
+
+    return newFilter;
+  });
+}
+
 export class ListHook {
   public static useList(options: IListHookOptions): IListHookData {
     const [filter, setFilter] = useState<ListFilterModel>(new ListFilterModel(options.filterMode));
@@ -108,8 +123,7 @@ export class ListHook {
 
     const filterListImpl = getFilterListImpl(options.filterMode);
 
-    // Update the filter when the query parameters change
+    // Initialise from interface forage when loaded
-    // we want to use the local forage only when the location search has not been set
     useEffect(() => {
       function updateFromLocalForage(queryData: any) {
         const queryParams = queryString.parse(queryData.filter);
@@ -123,33 +137,46 @@ export class ListHook {
         });
       }
 
-      function updateFromQueryString(queryStr: string) {
+      function initialise() {
-        const queryParams = queryString.parse(queryStr);
+        forageInitialised.current = true;
-        setFilter((f) => {
+
-          const newFilter = _.cloneDeep(f);
+        let forageData: any;
-          newFilter.configureFromQueryParameters(queryParams);
+
-          return newFilter;
+        if (interfaceForage.data && interfaceForage.data.queries[options.filterMode]) {
-        });
+          forageData = interfaceForage.data.queries[options.filterMode];
+        }
+
+        if (!options.props!.location.search && forageData) {
+          // we have some data, try to load it
+          updateFromLocalForage(forageData);
+        } else {
+          // use query string instead - include the forageData to include the following
+          // preferences if not specified: displayMode, itemsPerPage, sortBy and sortDir
+          updateFromQueryString(options.props!.location.search, setFilter, forageData);
+        }
       }
 
       // don't use query parameters for sub-components
       if (!options.subComponent) {
-        // do this only once after local forage has been initialised
+        // initialise once when the forage is loaded
         if (!forageInitialised.current && !interfaceForage.loading) {
-          forageInitialised.current = true;
+          initialise();
-
+          return;
-          if (!options.props!.location.search && interfaceForage.data && interfaceForage.data.queries[options.filterMode]) {
-            let queryData = interfaceForage.data.queries[options.filterMode];
-            // we have some data, try to load it
-            updateFromLocalForage(queryData);
-          } else if (interfaceForage.data) {
-            // else fallback to query string
-            updateFromQueryString(options.props!.location.search);
-          }
         }
       }
     }, [interfaceForage.data, interfaceForage.loading, options.props, options.filterMode, options.subComponent]);
 
+    // Update the filter when the query parameters change
+    useEffect(() => {
+      // don't use query parameters for sub-components
+      if (!options.subComponent) {
+        // only update from the URL if the forage is initialised
+        if (forageInitialised.current) {
+          updateFromQueryString(options.props!.location.search, setFilter);
+        }
+      }
+    }, [options.props, options.filterMode, options.subComponent]);
+
     function getFilter() {
       if (!options.filterHook) {
         return filter;
@@ -178,8 +205,12 @@ export class ListHook {
         // don't update this until local forage is loaded
         if (forageInitialised.current) {
           const location = Object.assign({}, options.props.history.location);
-          location.search = filter.makeQueryParameters();
+          const includePrefs = true;
-          options.props.history.replace(location);
+          location.search = "?" + filter.makeQueryParameters(includePrefs);
+
+          if (location.search !== options.props.history.location.search) {
+            options.props.history.replace(location);
+          }
 
           setInterfaceForage((d) => {
             const dataClone = _.cloneDeep(d);
@@ -367,31 +398,42 @@ export class ListHook {
       }
     }
 
-    const template = (
+    function getTemplate() {
-      <div>
+      if (!options.subComponent && !forageInitialised.current) {
-        <ListFilter
+        return (
-          onChangePageSize={onChangePageSize}
+          <div>
-          onChangeQuery={onChangeQuery}
+            {!result.error ? <Spinner size={Spinner.SIZE_LARGE} /> : undefined}
-          onChangeSortDirection={onChangeSortDirection}
+            {result.error ? <h1>{result.error.message}</h1> : undefined}
-          onChangeSortBy={onChangeSortBy}
+          </div>
-          onChangeDisplayMode={onChangeDisplayMode}
+        )
-          onAddCriterion={onAddCriterion}
+      } else {
-          onRemoveCriterion={onRemoveCriterion}
+        return (
-          onSelectAll={onSelectAll}
+          <div>
-          onSelectNone={onSelectNone}
+            <ListFilter
-          zoomIndex={options.zoomable ? zoomIndex : undefined}
+              onChangePageSize={onChangePageSize}
-          onChangeZoom={options.zoomable ? onChangeZoom : undefined}
+              onChangeQuery={onChangeQuery}
-          otherOperations={otherOperations}
+              onChangeSortDirection={onChangeSortDirection}
-          filter={filter}
+              onChangeSortBy={onChangeSortBy}
-        />
+              onChangeDisplayMode={onChangeDisplayMode}
-        {options.renderSelectedOptions && selectedIds.size > 0 ? options.renderSelectedOptions(result, selectedIds) : undefined}
+              onAddCriterion={onAddCriterion}
-        {result.loading || (!options.subComponent && !forageInitialised.current) ? <Spinner size={Spinner.SIZE_LARGE} /> : undefined}
+              onRemoveCriterion={onRemoveCriterion}
-        {result.error ? <h1>{result.error.message}</h1> : undefined}
+              onSelectAll={onSelectAll}
-        {maybeRenderContent()}
+              onSelectNone={onSelectNone}
-        {maybeRenderPagination()}
+              zoomIndex={options.zoomable ? zoomIndex : undefined}
-      </div>
+              onChangeZoom={options.zoomable ? onChangeZoom : undefined}
-    );
+              otherOperations={otherOperations}
+              filter={filter}
+            />
+            {options.renderSelectedOptions && selectedIds.size > 0 ? options.renderSelectedOptions(result, selectedIds) : undefined}
+            {result.loading || (!options.subComponent && !forageInitialised.current) ? <Spinner size={Spinner.SIZE_LARGE} /> : undefined}
+            {result.error ? <h1>{result.error.message}</h1> : undefined}
+            {maybeRenderContent()}
+            {maybeRenderPagination()}
+          </div>
+        )
+      }
+    }
 
-    return { filter, template, options, onSelectChange };
+    return { filter, template: getTemplate(), options, onSelectChange };
   }
 }

ui/v2/src/models/list-filter/filter.ts

@@ -151,23 +151,27 @@ export class ListFilterModel {
     this.sortByOptions = [...this.sortByOptions, "created_at", "updated_at"];
   }
 
+  private setSortBy(sortBy: any) {
+    this.sortBy = sortBy;
+      
+    // parse the random seed if provided
+    const randomPrefix = "random_";
+    if (this.sortBy && this.sortBy.startsWith(randomPrefix)) {
+      let seedStr = this.sortBy.substring(randomPrefix.length);
+
+      this.sortBy = "random";
+      try {
+        this.randomSeed = Number.parseInt(seedStr);
+      } catch (err) {
+        // ignore
+      }
+    }
+  }
+
   public configureFromQueryParameters(rawParms: any) {
     const params = rawParms as IQueryParameters;
     if (params.sortby !== undefined) {
-      this.sortBy = params.sortby;
+      this.setSortBy(params.sortby);
-      
-      // parse the random seed if provided
-      const randomPrefix = "random_";
-      if (this.sortBy && this.sortBy.startsWith(randomPrefix)) {
-        let seedStr = this.sortBy.substring(randomPrefix.length);
-
-        this.sortBy = "random";
-        try {
-          this.randomSeed = Number.parseInt(seedStr);
-        } catch (err) {
-          // ignore
-        }
-      }
     }
     if (params.sortdir === "asc" || params.sortdir === "desc") {
       this.sortDirection = params.sortdir;
@@ -205,6 +209,26 @@ export class ListFilterModel {
     }
   }
 
+  public overridePrefs(original: any, override: any) {
+    const originalParams = original as IQueryParameters;
+    const overrideParams = override as IQueryParameters;
+
+    if (originalParams.sortby === undefined && overrideParams.sortby !== undefined) {
+      this.setSortBy(overrideParams.sortby);
+    }
+    if (originalParams.sortdir === undefined && overrideParams.sortdir !== undefined) {
+      if (overrideParams.sortdir === "asc" || overrideParams.sortdir === "desc") {
+        this.sortDirection = overrideParams.sortdir;
+      }
+    }
+    if (originalParams.disp === undefined && overrideParams.disp !== undefined) {
+      this.displayMode = parseInt(overrideParams.disp, 10);
+    }
+    if (originalParams.perPage === undefined && overrideParams.perPage !== undefined) {
+      this.itemsPerPage = Number(overrideParams.perPage);
+    }
+  }
+
   private setRandomSeed() {
     if (this.sortBy == "random") {
       // #321 - set the random seed if it is not set
@@ -227,7 +251,8 @@ export class ListFilterModel {
     return this.sortBy;
   }
 
-  public makeQueryParameters(): string {
+  // includePrefs includes displayMode, sortBy, sortDir and perPage
+  public makeQueryParameters(includePrefs?: boolean): string {
     const encodedCriteria: string[] = [];
     this.criteria.forEach((criterion) => {
       const encodedCriterion: any = {};
@@ -238,16 +263,18 @@ export class ListFilterModel {
       encodedCriteria.push(jsonCriterion);
     });
 
-
+    const result: any = {
-    const result = {
-      sortby: this.getSortBy(),
-      sortdir: this.sortDirection,
-      disp: this.displayMode,
       q: this.searchTerm,
       p: this.currentPage,
-      perPage: this.itemsPerPage,
       c: encodedCriteria,
     };
+
+    if (includePrefs) {
+      result.disp = this.displayMode;
+      result.perPage = this.itemsPerPage;
+      result.sortby = this.getSortBy();
+      result.sortdir = this.sortDirection;
+    }
     return queryString.stringify(result, {encode: false});
   }