Skip to content

PB-984: add error feedback for all URL parameters#1067

Merged
ltkum merged 11 commits intodevelopfrom
feat-PB-984-error-feedback-for-all-url-parameters
Oct 3, 2024
Merged

PB-984: add error feedback for all URL parameters#1067
ltkum merged 11 commits intodevelopfrom
feat-PB-984-error-feedback-for-all-url-parameters

Conversation

@ltkum
Copy link
Contributor

@ltkum ltkum commented Sep 16, 2024

Issue : when modifying the URL directly, users received no feedback about what they did wrong when making mistakes, especially on a productive environment where most logs are deactivated.
Fix : we now give a feedback for every parameter that is wrongfully filled. We also added some extra cases for the layers, where we want to give a feedback on each individual layer, and for the time slider when everything is in order, but there are no layers supporting it.

Test link

@ltkum ltkum force-pushed the feat-PB-984-error-feedback-for-all-url-parameters branch from 25a93ef to 09defc2 Compare September 16, 2024 14:45
@ltkum ltkum force-pushed the feat-PB-984-error-feedback-for-all-url-parameters branch 6 times, most recently from a29b553 to 69f0aa8 Compare September 17, 2024 14:26
@cypress
Copy link

cypress bot commented Sep 17, 2024

web-mapviewer    Run #3466

Run Properties:  status check passed Passed #3466  •  git commit bf8e56eacb: PB-984: allow empty layer parameters in validation
Project web-mapviewer
Branch Review feat-PB-984-error-feedback-for-all-url-parameters
Run status status check passed Passed #3466
Run duration 05m 05s
Commit git commit bf8e56eacb: PB-984: allow empty layer parameters in validation
Committer Martin Künzi
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 21
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 211
View all changes introduced in this branch ↗︎

@ltkum ltkum marked this pull request as ready for review September 17, 2024 15:32
@ltkum ltkum requested review from ltshb and pakb September 17, 2024 15:32
ltshb
ltshb previously requested changes Sep 18, 2024
Copy link
Contributor

@ltshb ltshb left a comment

Choose a reason for hiding this comment

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

I did not find the ticket from the sprint ? Should we not concentrate on the sprint tickets based on our team discussions?

@ltkum ltkum force-pushed the feat-PB-984-error-feedback-for-all-url-parameters branch 2 times, most recently from 30c1941 to 0d40728 Compare September 20, 2024 08:35
@ltkum ltkum requested a review from ltshb September 20, 2024 08:36
@ltkum ltkum force-pushed the feat-PB-984-error-feedback-for-all-url-parameters branch 2 times, most recently from 7c8d595 to f8df7e6 Compare September 26, 2024 11:40
@ltkum ltkum force-pushed the feat-PB-984-error-feedback-for-all-url-parameters branch from f8df7e6 to c2c9480 Compare September 30, 2024 09:39
@ltkum ltkum requested a review from pakb September 30, 2024 10:16
@ltkum ltkum force-pushed the feat-PB-984-error-feedback-for-all-url-parameters branch from 0b30c0f to 66293c1 Compare September 30, 2024 12:09
Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

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

some more snake case to remove and we are good to go

@ltkum ltkum force-pushed the feat-PB-984-error-feedback-for-all-url-parameters branch 2 times, most recently from d670d32 to 08bd650 Compare October 1, 2024 16:05
@pakb pakb dismissed ltshb’s stale review October 1, 2024 17:04

He's on vacation and we want to go on with this PR

ltkum added 4 commits October 3, 2024 09:24
Issue : when modifying the URL directly, users received no feedback about what they did wrong when making mistakes, especially on a productive environment where most logs are deactivated.
Fix : we now give a feedback for every parameter that is wrongfully filled. We also added some extra cases for the layers, where we want to give a feedback on each individual layer, and for the time slider when everything is in order, but there are no layers supporting it.
 - Added a way to handle external layers in the acceptedValues for layers, sending an error feedback when users try to add an external layer which doesn't have a scheme
 - Small correction to the backgroundLayer acceptedValues, where we were not looking at the right place for information, causing the background layer to be void as soon as we tried to reload.

small adjustments
- We now return an object containing  a validation Boolean, errors and warnings to be pushed
- We need to implement the 'addErrors' and 'addWarnings' methods
- Simplification of the layers validation : we no longer give the full set of parameters, since the only thing that we check is the layer id / base url
ltkum added 7 commits October 3, 2024 09:24
- Added the store methods to handle the changes
- Also modified the warnings queue to follow the same logic as the error queue
…e rather than being a method of the AbstractParameterClass, as this couldn't be used in the basic storesync parameters
- we removed some artifacts from when error queues functions were a part of abstractParamConfig class
 - the timeslider, when reloading, would handle the parameter as a string, and would fail the Number.isInteger test. Also, when activating the timeslider through the button, it would trigger a URL change, and detect some parameters (crosshair and camera, for example) as being present but undefined, causing errors to appear.

small update : ensuring we compare sr  as a number

small update: featureInfo is case insensitive
 - Removed double ternary operator in LayerParamConfig for easier reading
 - Removed dirty ssssssnake case in errorQueues
 - Comments should be in better places, and/or explaining some not so evident parameters validations
Issue : the validation returned a falsy value when the layer parameter was set to an empty string. Also, setting the layer parameter to en empty string would give you the store content, as it was ignoring the parameter in those case

Fix : add a check for an empty string at the very beginning of the layer param validation, and added a check to ensure we can populate the layers with an empty string

removing some snakes
@ltkum ltkum force-pushed the feat-PB-984-error-feedback-for-all-url-parameters branch from 08bd650 to bf8e56e Compare October 3, 2024 07:24
@ltkum ltkum merged commit 02fe4d1 into develop Oct 3, 2024
@ltkum ltkum deleted the feat-PB-984-error-feedback-for-all-url-parameters branch October 3, 2024 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants