Fix issues with shape drawing on initialize (#860) (#871)

- Updates the Query model to set the right cql / filterTree on both itself and the LazyResult object right at construction time, preventing weirdness that could arise from the slight delay we have today - Removes a timeout effect that would inadvertantly use old data and cause the shapes to disappear from the map. - Updates the drawing / display component to useCallback so that we can ensure we're handling updates correctly Co-authored-by: Andrew Fiedler <[email protected]>
codice · Feb 6, 2024 · 3906b65 · 3906b65
1 parent 9694462
commit 3906b65
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 49 deletions.
diff --git a/...es/catalog-ui-search/src/main/webapp/component/visualization/maps/drawing-and-display.tsx b/...es/catalog-ui-search/src/main/webapp/component/visualization/maps/drawing-and-display.tsx
@@ -175,7 +175,7 @@ export const useDrawingAndDisplayModels = ({
   })
   useListenTo(
     (wreqr as any).vent,
-    'search:linedisplay search:polydisplay search:bboxdisplay search:circledisplay search:keyworddisplay',
+    'search:linedisplay search:polydisplay search:bboxdisplay search:circledisplay search:keyworddisplay search:areadisplay',
     (model: any) => {
       if (Array.isArray(model)) {
         setModels(
@@ -188,25 +188,23 @@ export const useDrawingAndDisplayModels = ({
       }
     }
   )
-  const updateFilterModels = React.useMemo(() => {
-    return () => {
-      const resultFilter = TypedUserInstance.getEphemeralFilter()
-      const extractedModels = [] as any[]
-      if (filterTree) {
-        extractModelsFromFilter({
-          filter: filterTree,
-          extractedModels,
-        })
-      }
-      if (resultFilter) {
-        extractModelsFromFilter({
-          filter: resultFilter,
-          extractedModels,
-        })
-      }
-      setFilterModels(extractedModels)
+  const updateFilterModels = React.useCallback(() => {
+    const resultFilter = TypedUserInstance.getEphemeralFilter()
+    const extractedModels = [] as any[]
+    if (filterTree) {
+      extractModelsFromFilter({
+        filter: filterTree,
+        extractedModels,
+      })
     }
-  }, [filterTree])
+    if (resultFilter) {
+      extractModelsFromFilter({
+        filter: resultFilter,
+        extractedModels,
+      })
+    }
+    setFilterModels(extractedModels)
+  }, [filterTree, models])
   React.useEffect(() => {
     updateFilterModels()
   }, [updateFilterModels])
@@ -249,14 +247,6 @@ export const useDrawingAndDisplayModels = ({
       setDrawingModels([])
     }
   }, [isDrawing])
-  React.useEffect(() => {
-    const timeoutId = window.setTimeout(() => {
-      updateFilterModels()
-    }, 1000)
-    return () => {
-      window.clearTimeout(timeoutId)
-    }
-  }, [])
   const callback = React.useMemo(() => {
     return () => {
       if (map) {

diff --git a/ui-frontend/packages/catalog-ui-search/src/main/webapp/js/model/Query.tsx b/ui-frontend/packages/catalog-ui-search/src/main/webapp/js/model/Query.tsx
@@ -130,8 +130,8 @@ export default Backbone.AssociatedModel.extend({
       isTransient: true,
     },
   ],
-  // override constructor slightly to ensure options are available on the self ref immediately
-  constructor(_attributes: any, options: any) {
+  // override constructor slightly to ensure options / attributes are available on the self ref immediately
+  constructor(attributes: any, options: any) {
     if (
       !options ||
       !options.transformDefaults ||
@@ -143,6 +143,7 @@ export default Backbone.AssociatedModel.extend({
         'Options for transformDefaults, transformFilterTree, transformSorts, and transformCount must be provided'
       )
     }
+    this._constructorAttributes = attributes || {}
     this.options = options
     return Backbone.AssociatedModel.apply(this, arguments)
   },
@@ -168,13 +169,27 @@ export default Backbone.AssociatedModel.extend({
     }
     return json
   },
-  /**
-   * Do not put filterTree in this, otherwise things that only set cql and are meant to rebuild the filter tree won't work (see initialize)
-   */
   defaults() {
+    const filterTree = this._constructorAttributes?.filterTree
+    let constructedFilterTree: FilterBuilderClass
+    let constructedCql = this._constructorAttributes?.cql || "anyText ILIKE '*'"
+    if (filterTree && typeof filterTree === 'string') {
+      constructedFilterTree = new FilterBuilderClass(JSON.parse(filterTree))
+    } else if (!filterTree || filterTree.id === undefined) {
+      // when we make drastic changes to filter tree it will be necessary to fall back to cql and reconstruct a filter tree that's compatible
+      constructedFilterTree = cql.read(constructedCql)
+      console.warn('migrating a filter tree to the latest structure')
+      // allow downstream projects to handle how they want to inform users of migrations
+      ;(wreqr as any).vent.trigger('filterTree:migration', {
+        search: this,
+      })
+    } else {
+      constructedFilterTree = new FilterBuilderClass(filterTree)
+    }
     return this.options.transformDefaults({
       originalDefaults: {
-        cql: "anyText ILIKE '*'",
+        cql: constructedCql,
+        filterTree: constructedFilterTree,
         associatedFormModel: undefined,
         excludeUnnecessaryAttributes: true,
         count: StartupDataStore.Configuration.getResultCount(),
@@ -189,7 +204,7 @@ export default Backbone.AssociatedModel.extend({
         // initialize this here so we can avoid creating spurious references to LazyQueryResults objects
         result: new QueryResponse({
           lazyResults: new LazyQueryResults({
-            filterTree: this.get('filterTree'),
+            filterTree: constructedFilterTree,
             sorts: [],
             sources: [],
             transformSorts: ({ originalSorts }) => {
@@ -252,21 +267,6 @@ export default Backbone.AssociatedModel.extend({
   initialize(attributes: any) {
     // @ts-expect-error ts-migrate(2769) FIXME: No overload matches this call.
     _.bindAll.apply(_, [this].concat(_.functions(this))) // underscore bindAll does not take array arg
-    const filterTree = this.get('filterTree')
-    if (filterTree && typeof filterTree === 'string') {
-      this.set('filterTree', new FilterBuilderClass(JSON.parse(filterTree)))
-    }
-    // when we make drastic changes to filter tree it will be necessary to fall back to cql and reconstruct a filter tree that's compatible
-    if (!filterTree || filterTree.id === undefined) {
-      this.set('filterTree', cql.read(this.get('cql'))) // reconstruct
-      console.warn('migrating a filter tree to the latest structure')
-      // allow downstream projects to handle how they want to inform users of migrations
-      ;(wreqr as any).vent.trigger('filterTree:migration', {
-        search: this,
-      })
-    } else {
-      this.set('filterTree', new FilterBuilderClass(filterTree)) // instantiate the class if everything is a-okay
-    }
     this._handleDeprecatedFederation(attributes)
     this.listenTo(
       this,