Closes #3785 #3778 #3779 #3788 by kappu72 · Pull Request #3898 · geosolutions-it/MapStore2

kappu72 · 2019-07-02T14:41:34Z

Description

With this pull a new Layer filter concept is added.
The user is able to set up a filter at layer level that will be used in any context (FG, QB, WMS Plugin)
see #3785 issue.
It also closes some minor bugs

Issues

Implement Layer Filter Concept #3785
Bug with attribute filter in Feature Grid #3778
Problem with Feature Grid filter state -1- #3779
Problem with Feature Grid filter state -2- #3788
Please check if the PR fulfills these requirements
The commit message follows our guidelines: https://github.com/geosolutions-it/MapStore2/blob/master/CONTRIBUTING.md
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

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

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

What is the current behavior? (You can also link to an open issue here)
It's not possible to define a filter at layer level

What is the new behavior?
It become possible to define a filter at layer level

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

Yes, and I documented them in migration notes
No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

…eosolutions-it#3788

offtherailz

Bugs

0 values are not valid as filters: (open map 10154 to test it)
Sometimes the spatial filter is not applied
When I reopen the filter I can't see the spatial filter anymore on the map (See the gif at the end)
No way to understand if the layer filter failed for some reason until close the query panel (See the gif at the end)
When layer error, the button to edit filter should be shown anyway in the toolbar, to allow editing and so fix the problem.
See that geometry filter breaks the history somehow (geometry is now shown on reload and can not be restored). --> (See the gif at the end)

Suggestions

Labels:
- instead of "search" tooltip in query panel there should be an "apply filter"
- instead of "open query panel" there should be a tooltip like "Filter Layer"
- Logic is not too much intuitive (see the gif). You have to apply, save and close. 3 clicks. Ideal is to edit and see changes in real-time, than save or discard (closing anyway the query panel). Even if you need an apply button, save and discard should close the panel automatically. Restore last saved filter is not needed, IMHO

Things noticed, out of the PR

Spatial Filter do not work with the types layer. I think this is a problem with the specific layer, also on dev (See the gif above)

http://gs-stable.geo-solutions.it/geoserver/wms?SERVICE=WMS&VERSION=1.3.0&REQUEST=GetMap&FORMAT=image%2Fpng&TRANSPARENT=true&LAYERS=mapstore%3ATypes&SRS=EPSG%3A3857&CRS=EPSG%3A3857&TILED=true&authkey=76b7c6d5-2e6b-42b6-aee0-c0b425b253b4&CQL_FILTER=(INTERSECTS(Point%2C%20Polygon((-8697922.144578222%205448431.310834297%2C%20-8697922.144578222%204572768.714799318%2C%20-7890747.1258867625%204572768.714799318%2C%20-7890747.1258867625%205448431.310834297%2C%20-8697922.144578222%205448431.310834297))))&WIDTH=256&HEIGHT=256&BBOX=-7514065.628545966%2C5009377.085697312%2C-6261721.357121638%2C6261721.35712164

Cross layer filter do not work with the types layer (Same as above)

offtherailz · 2019-07-02T14:54:28Z

web/client/translations/data.en-US

            "toolTrashLayerTooltip": "Remove selected layer",
            "toolTrashLayersTooltip": "Remove selected layers",
            "toolFeaturesGridTooltip": "Open Attribute Table",
+            "layerFilterTooltip": "Open Query Builder",


I suggest "Filter Layer", and instead of "Search", "Apply Filter"

offtherailz · 2019-07-02T16:16:05Z

web/client/actions/filterHistory.js

+const RESTORE_CURRENT_SAVED_FILTER = 'FILTER_HISTORY:RESTORE_CURRENT_SAVED_FILTER';
+const APPLY_FILTER = 'FILTER_HISTORY:APPLY_FILTER';
+const OPEN_QUERY_BUILDER = 'LAYER_FILTER:OPEN_QUERY_BUILDER';
+


You should document a little bit the actions to explain their role. So you can find better names for actions

web/client/epics/layerfilter.js

offtherailz · 2019-07-02T16:32:10Z

web/client/actions/filterHistory.js

+const STORE_CURRENT_APPLIED_FILTER = 'FILTER_HISTORY:STORE_CURRENT_APPLIED_FILTER';
+const RESTORE_CURRENT_SAVED_FILTER = 'FILTER_HISTORY:RESTORE_CURRENT_SAVED_FILTER';
+const APPLY_FILTER = 'FILTER_HISTORY:APPLY_FILTER';
+const OPEN_QUERY_BUILDER = 'LAYER_FILTER:OPEN_QUERY_BUILDER';


These actions could be part of the queryform reducer and actions. FilterHistory is anyway a bad name. See my comment about history, anyway

I prefer to keep separate, queryform reducer/actions are already enough long files full of actions.
As suggested I have modified the name.

coveralls · 2019-07-02T16:51:46Z

Coverage increased (+0.008%) to 81.938% when pulling a4833a1 on kappu72:i#3785 into ce9f78b on geosolutions-it:master.

offtherailz

Bugs

The discard button now doesn't enable anymore, whatever I do
The first button should be "Apply", not search (change icon and tooltip)

I have 4 icons, Apply (changes)|Save (changes)|Discard (changes)|Reset (filter)

The button should be 'Discard', not "restore last saved filter" (change icon and tooltip)

Discard is new. It discards the changes we do and restore the previous saved filter if there is one or an empty filter. It enables everytime we make a change to the filter or we Apply|Reset it and we have a previous filter that was Saved in the QB or when we start with empty (it send us back to empty in this case). It updates map content immediately.

When 'reset' with an existing filter it should update map content immediately and it needs "Save" Instead we have immediately I have confirm modal and it it saves on confirm.

Reset is the same we have now. It kills the filter and reset the Filtered Layer to be a normal layer. All its content is shown. It enables once we have saved at least once a valid filter, it makes us start from empty filter all over again. It needs Save to exit the QB if we started without an emtpy filter, otherwise discard is enough. It updates map content immediately.

Tooltips bug is very annoying in this context (in other context, like timeline, we take into account disabled by replicating disabled condition to add tooltipId property). I suggest to create a new enhancer called buttonTooltip that combines tooltip with a branch, that automatically removes the enhancement when disabled = true, if a property named noTooltipWhenDisabled is true. Then apply this instead of tooltip to the ToolbarButton. We can make noTooltipWhenDisabled true by default in the future if it doesn't cause any problem to the other toolbars.
Sometime, after reset, I see this tooltip that doesn't makes too much sense
Please @kappu72 make you sure that the discard button follows the specification, I couldn't test it because it doesn't work. Verify every point in #3785 before to PR.
Typo:

Error management

When the filter for some reason doesn't work, we have a "!" in the right corner. Maybe we should also show some suggestion or notification. If we try to exist with a filter that returned error, we can not (we can only discard). This is good, anyway we should have a different message, IMHO. @simboss what do you think?

Suggestions

Save (and maybe also discard, with confirm modal) should close the panel immediately. Manually close the panel after save is annoying.
I can apply, then edit the query form again, the save button is still enabled with the last applied filter. This can be misleading

offtherailz · 2019-07-03T07:49:15Z

web/client/actions/filterPersistence.js

+
+const APPLY_FILTER = 'FILTER_PERSISTENCE:APPLY_FILTER';
+/**
+ * It opens the queary panel to be used as layer filter query builder


I think this action is proper of filter persistance. Therefore at least the file should be named layerFilter

web/client/epics/layerfilter.js

kappu72 · 2019-07-04T09:27:49Z

Minutes of call with Lorenzo about his comments.
We needs to clarify if discard btn should be active only after a filter is changed (apply btn pressed) or after any changes to the filter builder ui.
The reset btn has just to reset filter and not to store it.
Save btn has to store the filter but not to close the panel.

offtherailz

Once a valid filter is created the Apply icons is enabled.

Selecting the Layer filter target layer and operation is valid filter but the apply button do not enable
If I save a filter with one attribute row and/or spatial filter, then I delete all filter rows and / or reset ROI, the apply button is disabled.
Save should be allowed only after apply
When I open the Feature Grid I have this in error log

PluginsUtils.js:310 Uncaught TypeError: text.substr is not a function
    at nextToken (parser.js:113)
    at tokenize (parser.js:137)
    at read (parser.js:362)
    at SwitchMapSubscriber.eval [as project] (wfsquery.js:186)
    at SwitchMapSubscriber._next (switchMap.js:88)
    at SwitchMapSubscriber.Subscriber.next (Subscriber.js:89)
    at FilterSubscriber._next (filter.js:88)
    at FilterSubscriber.Subscriber.next (Subscriber.js:89)
    at Subject.next (Subject.js:55)
    at eval (createEpicMiddleware.js:66)
    at eval (index.js:12)
    at SafeSubscriber.dispatch [as _next] (applyMiddleware.js:37)
    at SafeSubscriber.__tryOrUnsub (Subscriber.js:234)
    at SafeSubscriber.next (Subscriber.js:183)
    at Subscriber._next (Subscriber.js:125)
    at Subscriber.next (Subscriber.js:89)

I've discussed with Simone. Discard is ok for now. We can improve it by using.
Icon of filter must be the one designed with the layer
Icon of discard must be an 'undo icon

offtherailz

Error state is not enough visible. The tooltip should be visible without need to hover the mouse and/or the exclamation mark should be in red (or some notification should clearely notify the error).
With some combination the buttons are not enabled correctly
Reset do not ask confirm. User anyway can undo if you saved your canges.
Some strings need a review

            "changedFilter": "Filter Changed",
            "discardbtn": "Discard",
            "changedFilterAlert": "Save or discard actual filter?",
            "resetFilter": "Remove filter",
            "resetFilterAlert": "It is not possible to undo a filter reset action. Do you confirm?", <-- this do not make sense and is not used
            "confirmReset": "Confirm",
            "loadingError": "WMS rendering failed due to filter configuration, try to remove or fix it.", <-- Layer rendering failed because of the filter change. Please fix the filter or reset it.
            "changedFilterWithErrorAlert": "Filter is causing WMS render failure, try to fix or discard it" <-- The current filter caused an error during rendering. Please fix or reset your changes before to close.

@tdipisa these are bugs but not so blocking. Please tell us if you want to merge this PR and fix these later or wait for these fixes.

web/client/components/misc/enhancers/buttonTooltip.jsx

offtherailz · 2019-07-11T07:29:29Z

So as @tdipisa asked, I'm going to merge. @kappu72 if you already worked on fixes, please do a separate pull request to add them.

* 3523 manage exponential number coord editor (#3907) * Fixed exponential problem with input type number for coordiante entry * 'e' and 'E' chars are disabled for coordinate entry * both on aeronautical or decimal * updated doc * restored onkeyDown prop, and added more tests to check that is called * removed comment * Fix #3908 Time sync support for FeatureGrid (#3909) * Fix for #3845. Restored Identify and other plugins doc (#3921) * FIx #3785 Implemented Layer Filter (#3898) * Closes #3785 #3778 #3779 #3788 * Fixed lint error and added layerfilter tests * Fixes on Lorenzo's review * Added comments and fixed query tooltip string * Fixesx after Lorenzo's second review * Fixess on Lorenzo's last review * Removed console.log * Fixed btntooltip * use protocol-relative URL for CartoDB provider (#3938) * Fix 3945 zoom to feature for point now zooms correctly (#3946) * Fix 3945 zoom to feature for point now zooms correctly * fix max zoom default * added a todo for max zoom customization * fix #3915 wrong overflow in feature info settings (#3941) * Fix Glich of query panels roi styles (#3948) * Fix Glith of query panels roi styles * remove onmount double call in feature editor * Fix 3942 with correct positioning of tutorial for GFI step (#3943) * Fix 3942 with correct positioning of tutorial for GFI step * changed implementation * Fix #3928 widget builder color selector box shows wrong labels (#3937) * Fix dashboard autoreload (#3951) * Fix thematic maps color labels (#3952) * Fixes #3955 filter layer minor issues (#3956) * Fix fr-FR translation for options menu (#3961) * Fix #3913 GFI window does not close when opening the catalog (#3913) (#3944) * Fix #3913 GFI window does not close when opening the catalog (#3913) fix #3913 * move the epic to their more appropriate file * Fix #3910 Text annotations - wrong text align in preview (#3954) fix #3910 * Fix link on quickstart page (#3968) * Add infoFormat to Identify plugin documentation (#3966) * Add infoFormat to Identify plugin documentation * changed documentation message * Fix 3960 Template editor of feature info settings has slow response on typing (#3965) * add internal state to feature info editor to reduce on change calls * add default state in feature info editor component * Added missing Croatian translations & fix typos (#3959) * Added missing Croatian translations & fix typos * Fix typo * Added missing strings that were not added automatically by devs * Fix #3957 Avoid showing the MapStore version number (2) in translations (#3971) fix #3957 * #3950: the whole group containing the annotations layer disappears in 3d mode, also if it contains other layers (#3974) * #3863: improved Print plugin documentation with info on printing usin… (#3975) * #3863: improved Print plugin documentation with info on printing using custom scales * #3863: fixed typo * Fix #3940 Integrate the tooltip for metadata with missing params (#3973) * Fix #3940 Integrate the tooltip for metadata with missing params fix #3940 * sort the templateMetadata in translation * Fix #3906 the 'back' button does not undo the annotation creation (#3967) * Fix #3906 the 'back' button does not undo the annotation creation fix #3906 * improve test coverage * Fix 3976 Search Plugin improved (#3977) * Fix 3976 Search Plugin improved * sort is correct * maxRsults is configurable and limits the results size * add test to check results sorting * Update web/client/epics/search.js Co-Authored-By: Lorenzo Natali <offtherailz@gmail.com> * Update web/client/epics/__tests__/search-test.js Co-Authored-By: Lorenzo Natali <offtherailz@gmail.com> * #3972: fixed print of vector layers for solid dash stroke (#3978) * update style of annotation text symbolizer to avoid horizontal scrollbar (#3979) * #3962: removed old examples (#3986) * fix #3969 annotation description field is not clickable in safari (#3982) * #3784 Activate Sync Map by default (#3984) * Fix #3958 hide layer related buttons when removing a layer from the map (#3980) * Fix WidgetLegend preview (#3994) * Fix WidgetLegend preview * Update web/client/components/widgets/enhancers/legendWidget.js * Ewsterrenburg vietnamese translations (#3999) * Add Vietnamese translation * Removed erroneous comma * Fixing tests * removed console.log * Update featuregrid.js * Fix 3985 add catalog autosearch (#3988) * Fix 3895 catalog autosearch * Fix reducer default, fix epic for autosearch * test was hard to provide since it is using a thunk * anyway it has been tested so i removed the test for epic * added missing default in catalog reducer * #3989: fixed the broken examples (#4003) * Fix #3934 Cross layer filter: some wfs requests fails (#3991) * Fix 3817 map info url (#4000) * Wip 3817 mapinfo on map loading * Wip GetFeatureInfo after GetFeature * wip * moved queryparam logic in a separate epic * changed main comp to fetch new epic * added whitelist * add error handling for map info from url * added a generic api for queryparam actions * added possibility to restrict list of queriable layers to perform GFI * added possibility to override some request param for specific layers * * Added documentation * fix doc link * fix german translations * fix others translations * Fix documentation and make whitelist configurable * update doc * Save filterLayer in layer object (#4010) * Fix Print tool with vendor param (CQL_FILTER) (#4006) * Fix 2749 search urls for cross layer filter are compared ignoring "dirty" chars (#4004) * Fix 2749 search urls for cross layer filter are compared ignoring "dirty" url * fix test and check for dirty chars * Fix 4007 hide filter layer when no layers are present in TOC (#4008) * Fix 4007 hide filter layer whne no layers are present * even if a group is present with no layers * Fix tests * Fixed issues due to sameURL check in #4004 (#4013) * Fix #3805 User edit password field retain value across users (#4016) On editing user, the form use to retain password field across editing session. This commit fix and enable password field reset on close edit dialog * #3929 Cross layer filter cannot be disabled when filled (#3998) #3929 Cross layer filter cannot be disabled when filled * refix #2953 to work along #3929 * [docs] Update User Guide (#3995) * [docs] Update TOC section * [docs] filtering layers * [docs] Search tool updates * [docs] metadata template for CSW services * [docs] custom tooltip in layer settings * [docs] updating 'quering objects' section with coordinates editor * [docs] updating annotations section * [docs] layer filter persistence * [docs] crs selector section (#4012) * #3622: some fixes to involved documentations (#4018) * Some updates to the CRS selector documentation (#4019) * #3817 Fixing documentation (#4023) * #3817 fixing documentation * Update docs/developer-guide/map-query-parameters.md Co-Authored-By: mbarto <maurobartolomeoli@gmail.com> * Update docs/developer-guide/map-query-parameters.md Co-Authored-By: mbarto <maurobartolomeoli@gmail.com> * Update .travis.yml (#4027) * Remove IN PROGRESS (#4026) * Close #3025 Add boundary for draggable dialog (#4025) * Close #3025 Add boundary for draggable dialog * simplification of dialog boundary solution * make snapshot dialog draggable

Closes geosolutions-it#3785 geosolutions-it#3778 geosolutions-it#3779 g…

109d92c

…eosolutions-it#3788

kappu72 requested a review from offtherailz July 2, 2019 14:41

kappu72 added this to the 2019.02.00 milestone Jul 2, 2019

Fixed lint error and added layerfilter tests

a2abae5

offtherailz requested changes Jul 2, 2019

View reviewed changes

kappu72 added 2 commits July 3, 2019 06:11

Fixes on Lorenzo's review

1f622eb

Added comments and fixed query tooltip string

f291df3

offtherailz requested changes Jul 3, 2019

View reviewed changes

Fixesx after Lorenzo's second review

a07fdab

kappu72 marked this pull request as ready for review July 4, 2019 14:48

kappu72 requested a review from offtherailz July 4, 2019 15:45

offtherailz requested changes Jul 5, 2019

View reviewed changes

kappu72 added 4 commits July 10, 2019 16:19

Fixess on Lorenzo's last review

9bef3ee

Merged master

1588426

Removed console.log

ba18ef4

Fixed btntooltip

a4833a1

offtherailz reviewed Jul 10, 2019

View reviewed changes

web/client/components/misc/enhancers/buttonTooltip.jsx Outdated Show resolved Hide resolved

offtherailz merged commit b3b2b02 into geosolutions-it:master Jul 11, 2019

Conversation

kappu72 commented Jul 2, 2019 • edited by tdipisa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues

Uh oh!

offtherailz left a comment • edited by kappu72 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Bugs

Suggestions

Things noticed, out of the PR

Uh oh!

offtherailz Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

offtherailz Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

offtherailz Jul 2, 2019

Choose a reason for hiding this comment

Uh oh!

kappu72 Jul 3, 2019

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jul 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

offtherailz left a comment • edited by kappu72 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Bugs

Error management

Suggestions

Uh oh!

offtherailz Jul 3, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kappu72 commented Jul 4, 2019

Uh oh!

offtherailz left a comment • edited by kappu72 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

offtherailz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

offtherailz commented Jul 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kappu72 commented Jul 2, 2019 •

edited by tdipisa

Loading

offtherailz left a comment •

edited by kappu72

Loading

coveralls commented Jul 2, 2019 •

edited

Loading

offtherailz left a comment •

edited by kappu72

Loading

offtherailz left a comment •

edited by kappu72

Loading