Opened 4 years ago
Closed 4 years ago
#20729 closed enhancement (fixed)
Improve validation with data filters enabled
Reported by: | ljdelight | Owned by: | GerdP |
---|---|---|---|
Priority: | normal | Milestone: | 21.05 |
Component: | Core validator | Version: | latest |
Keywords: | filter | Cc: |
Description
Users de-clutter their editing area by enabling filters to hide/disable objects, and these objects are sometimes not included when a user performs validation and are often hidden from view after validation actually occurs. This may skew the validation results without the user being aware of the issue.
This enhancement is to warn the user when validation is performed while filters are actively hiding/disabling objects. The dialog allows the user to persist their response as the saved preference key 'validator.vallidate_with_filters_action' (values are 'ask' (default), 'continue', and 'fail').
When the dialog should be shown to the user, here's a screenshot and the 'Help' button opens the JOSM Help Browser Validator page (https://wiki.openstreetmap.org/wiki/JOSM/Validator ):
When the 'error' selection is saved, the next time the user runs validation with active filters this error message is shown:
The 'validator.vallidate_with_filters_action' Preference can be configured by changing the radio selection under Edit > Settings > Data validator:
Attachments (12)
Change History (42)
by , 4 years ago
Attachment: | prompt-dialog.png added |
---|
by , 4 years ago
Attachment: | preferences-options.png added |
---|
by , 4 years ago
Attachment: | error-dialog.png added |
---|
by , 4 years ago
Attachment: | josm-validation-with-filters.patch added |
---|
follow-up: 6 comment:1 by , 4 years ago
comment:2 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → needinfo |
comment:4 by , 4 years ago
Component: | Core → Core validator |
---|---|
Keywords: | filter added; validator removed |
Milestone: | → 21.04 |
comment:5 by , 4 years ago
What is that special about active filter with the indication in the top right corner of Mapview compared to validation with incomplete data, some selection or on upload?
by , 4 years ago
Attachment: | validation-with-filter.png added |
---|
Validation done with the filter enabled, causing the 'way end node near other highway' result to not be identified.
by , 4 years ago
Attachment: | validation-no-filter.png added |
---|
by , 4 years ago
Attachment: | ticket20729-filter-changes-validation-results.osm added |
---|
comment:6 by , 4 years ago
Replying to GerdP:
I was not aware that filters have an influence on the results of the validator. I think this should not happen, but maybe it's a wanted feature? I rarely work with filters. Can you give an example to reproduce this problem?
Sure, I attached 'ticket20729-filter-changes-validation-results.osm' with two ways that are near but not connected. The filter 'highway: residential' enabled and hidden does change the validation results from 3 warnings to 2 warnings, screenshots follow.
Additionally, I have seen cases where the validation creates objects (in the validation layer) that are not visible to the user and the dialog would help there.
Validation shows 3 warnings without a filter:
Validation shows 2 warnings with a filter (disable checked):
comment:7 by , 4 years ago
OK, I see. The difference in the result is probably caused by the use of IPrimitive.isDrawable()
. The implementations for this method return false for the filtered objects and thus some tests don't "see" all data. This is probably not intended, but I have to dig deeper to be sure. I'll open new tickets for those cases which seem wrong.
comment:8 by , 4 years ago
comment:9 by , 4 years ago
The code in Multipolygon
also checks isDrawable
and some tests use this code.
comment:10 by , 4 years ago
This has some more ugly effects. If you have a filter activated that shows only natural=tree and load a file containing some multipolygons and then deactivate the filter the multipolygons are not rendered properly.
comment:11 by , 4 years ago
I think Multipolygon.load()
should not checkisDrawable()
. I assume isUsable()
was meant here and also in the other tests.
by , 4 years ago
Attachment: | 20729-replace-isDrawable.patch added |
---|
My proposal to fix the tests by replacing sDrawable()
by more appropriate methods
comment:12 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | needinfo → new |
+1. Sounds reasonable. Time will tell if isDrawable
was there for a good reason...
comment:14 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → needinfo |
@ljdelight: I think r17761 solves part of the problem.
Do you see other possible reasons why filters cause different test results? I looked for isDisabled and other methods which depend on the filter but found no more issues in core.
comment:15 by , 4 years ago
Hey @GerdP, your patch solved what I had noticed with the different test results.
The remaining issue is the case where the validation creates objects that are not visible to the user (comment 6 has the screenshots).
What are your thoughts on the dialog?
comment:16 by , 4 years ago
I see no reason to show a warning before the validation is executed now that the errors are fixed. Maybe just show a hint when the validation found problems with hidden objects? Or show a hint when such a problem is selected?
comment:17 by , 4 years ago
Summary: | [patch] Alert user if validation action is performed with data filters enabled → Improve validation with data filters enabled |
---|
by , 4 years ago
Attachment: | validation-has-hidden-results.png added |
---|
comment:18 by , 4 years ago
by , 4 years ago
Attachment: | josm-show-notification-on-hidden-objects.patch added |
---|
comment:20 by , 4 years ago
comment:21 by , 4 years ago
I think the text is too complex and missleading as the validation layer doesn't show markers for filtered objects.
What about Validation results contain elements hidden by a filter.
for the first line?
This also works for users that deactivate the "Validation errors" layer. Still, they may note problems when the "Zoom to problem" action shows an empty area.
by , 4 years ago
Attachment: | 20729-notification.patch added |
---|
sligtly simplified patch with different text
comment:23 by , 4 years ago
Considering the milestone:21.04 roadmap, we probably should no longer change i18n strings...
comment:25 by , 4 years ago
Hi, is there anything else needed on this ticket? It's in a need info. Thanks
comment:26 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | needinfo → assigned |
No, I'll commit my patch after release of 21.04 unless anybody suggests a better text.
comment:27 by , 4 years ago
What do you want to say with "review"? Is "disable" better?
Please disable active filters to see the hidden results.
comment:28 by , 4 years ago
My understanding is that the filters may stay activated, only if the checkbox below H is enabled nothing is rendered and "zoom to problem" might show an empty area.
comment:29 by , 4 years ago
Milestone: | 21.04 → 21.05 |
---|
I was not aware that filters have an influence on the results of the validator. I think this should not happen, but maybe it's a wanted feature? I rarely work with filters. Can you give an example to reproduce this problem?