Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Counter widget sends double requests, the first one has viewport as undefined #10674 #10683

Merged
merged 10 commits into from
Nov 27, 2024

Conversation

rowheat02
Copy link
Contributor

@rowheat02 rowheat02 commented Nov 18, 2024

Description

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

#10674

What is the current behavior?

#10674

What is the new behavior?

API call disabled when called without polygon, specifically filter XML text with undefined value

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes, and I documented them in migration notes
  • No

Other useful information

@rowheat02 rowheat02 added this to the 2024.02.02 milestone Nov 18, 2024
@rowheat02 rowheat02 self-assigned this Nov 18, 2024
@rowheat02 rowheat02 linked an issue Nov 18, 2024 that may be closed by this pull request
1 task
Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effective issue is this, and involves all the ogc filters with empty list of issues:

+    const filterString = filters?.length > 0
+        ? fb.filter(fb.and(
+            ...flatten(filters
+                .filter(filter => !!filter && (isString(filter) || isFilterValid(filter) && !filter.disabled))
+                .map(filter => isString(filter) ? [toFilter(read(filter))] : toOGCFilterParts(filter, ogcVersionOpt, nsPlaceholder)))
+        ))
+        : "";

The initial count with no geometry is due to the bbox that is not ready. Instead of filtering out invlid request, fix the invalid request as before (and add a test).

The initial request is anyway performed. Let's see if it is possible to avoid the request if the spatial filter should be present but is not....

If other widgets on map do not have the same issue, let's see why

Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no. With this solution we have a regression, the counter completely disappears if the sync is not available.
See this map attached:

map (10).json

This works on dev, and the counter is not visible anymore applying these changes saved in http://localhost:8081/#/viewer/51809

I suggest to :

git diff web/client/utils/FilterUtils.js
diff --git a/web/client/utils/FilterUtils.js b/web/client/utils/FilterUtils.js
index b34ca4d18..cca07a07f 100644
--- a/web/client/utils/FilterUtils.js
+++ b/web/client/utils/FilterUtils.js
@@ -1285,11 +1285,13 @@ export const mergeFiltersToOGC = (opts = {}, ...filters) =>  {
     });
     const toFilter = fromObject(fb);
 
-    const filterString = fb.filter(fb.and(
-        ...flatten(filters
-            .filter(filter => !!filter && (isString(filter) || isFilterValid(filter) && !filter.disabled))
-            .map(filter => isString(filter) ? [toFilter(read(filter))] : toOGCFilterParts(filter, ogcVersionOpt, nsPlaceholder)))
-    ));
+    const filterString = filters?.length > 0
+        ? fb.filter(fb.and(
+            ...flatten(filters
+                .filter(filter => !!filter && (isString(filter) || isFilterValid(filter) && !filter.disabled))
+                .map(filter => isString(filter) ? [toFilter(read(filter))] : toOGCFilterParts(filter, ogcVersionOpt, nsPlaceholder)))
+        ))
+        : "";
 

@tdipisa
Copy link
Member

tdipisa commented Nov 25, 2024

Moving the issue back in Ready @rowheat02

@rowheat02
Copy link
Contributor Author

Extra API call for counter and table widget fixed by filtering events checking mapSync and viewport.

@offtherailz offtherailz merged commit a174c99 into geosolutions-it:master Nov 27, 2024
5 checks passed
@tdipisa tdipisa added the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Nov 27, 2024
@tdipisa
Copy link
Member

tdipisa commented Nov 27, 2024

@ElenaGallo please test once in DEV and let us know for the backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch bug Widgets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Counter widget sends double requests, the first one has viewport as undefined
3 participants