Skip to content

PB-2027: fix drawing marker/text test #1512

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: fix-pb-2064-reportproblem-cypress
Choose a base branch
from
Open

PB-2027: fix drawing marker/text test #1512

wants to merge 5 commits into from

Conversation

@pakb
Copy link
Contributor

@pakb pakb commented Nov 20, 2025 •
edited
Loading

Fixing the code so that the first test of the drawing.cy.ts file passes

Brings in a simplification of the useFieldValidation composable (less two-ways communication)

Test link

@pakb pakb requested a review from sommerfe November 20, 2025 09:21
@cypress
Copy link

cypress bot commented Nov 20, 2025 •
edited
Loading

web-mapviewer    Run #6042

Run Properties:  status check failed Failed #6042  •  git commit 4c84aa6531: PB-2027: remove double call to validate
Project web-mapviewer
Branch Review feat-PB-2027-fix-marker-text-test
Run status status check failed Failed #6042
Run duration 16m 11s
Commit git commit 4c84aa6531: PB-2027: remove double call to validate
Committer Pascal Barth
View all properties for this run ↗︎
Test results
Tests that failed  Failures 65
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 19
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 122
View all changes introduced in this branch ↗︎

Tests for review

Failed  timeSlider.cy.ts • 3 failed tests • e2e/chrome/mobile

View Output

Test Artifacts
Cypress tests covering the time slider, its functionalities and its URL parameter > checking the time slider behavior, both on startup and during use > checks that the time slider is functional and behave correctly part 1 Test Replay Screenshots
Cypress tests covering the time slider, its functionalities and its URL parameter > checking the time slider behavior, both on startup and during use > checks that the time slider is functional and behave correctly with the timeSlider param at startup Test Replay Screenshots
Cypress tests covering the time slider, its functionalities and its URL parameter > checking the time slider behavior, both on startup and during use > behaves correctly when years are being entered in the input Test Replay Screenshots
Failed  layers.cy.ts • 1 failed test • e2e/chrome/mobile

View Output

Test Artifacts
An uncaught error was detected outside of a test Test Replay Screenshots
Failed  drawing.cy.ts • 11 failed tests • e2e/chrome/mobile

View Output

Test Artifacts
Drawing module tests > Drawing mode/tools > can create line / measurement, extend it, and delete the last node by right click / button, and make a polygon Test Replay Screenshots
Drawing module tests > Drawing mode/tools > can create line/polygons and edit them Test Replay Screenshots
Drawing module tests > KML management > deletes the drawing when confirming the delete modal Test Replay Screenshots
Drawing module tests > KML management > manages the KML layer in the layer list / URL params correctly Test Replay Screenshots
Drawing module tests > KML management > keeps the KML after a page reload, and creates a copy if it is then edited Test Replay Screenshots
Drawing module tests > KML management > manages the KML layer correctly if it comes attached with an adminId at startup Test Replay Screenshots
Drawing module tests > KML management > manages the KML layer correctly if it comes attached with an adminId at startup from a legacy URL Test Replay Screenshots
Drawing module tests > KML management > receives an empty KML and can use drawing mode Test Replay Screenshots
Drawing module tests > others > can export the drawing/profile in multiple formats Test Replay Screenshots
Drawing module tests > others > shows a profile of a line/measure coming from service-alti data Test Replay Screenshots
The first 10 failed tests are shown, see all 11 tests in Cypress Cloud.
Failed  legacyParamImport.cy.ts • 1 failed test • e2e/chrome/mobile

View Output

Test Artifacts
An uncaught error was detected outside of a test Test Replay Screenshots
Failed  3d/layers.cy.ts • 10 failed tests • e2e/chrome/mobile

View Output

Test Artifacts
Test of layer handling in 3D > add layer from search bar Test Replay Screenshots
Test of layer handling in 3D > sets the opacity to the value defined in the layers URL param or menu UI Test Replay Screenshots
Test of layer handling in 3D > sets the timestamp of a layer when specified in the layers URL param Test Replay Screenshots
Test of layer handling in 3D > reorders visible layers when corresponding buttons are pressed Test Replay Screenshots
Test of layer handling in 3D > add GeoJson layer with opacity from URL param Test Replay Screenshots
Test of layer handling in 3D > removes a layer from the visible layers when the "remove" button is pressed Test Replay Screenshots
Test of layer handling in 3D > uses the 3D configuration of a layer if one exists Test Replay Screenshots
Test of layer handling in 3D > add KML layer from drawing Test Replay Screenshots
Test of layer handling in 3D > Verify layer features in 2D and 3D Test Replay Screenshots
Test of layer handling in 3D > Verify a layer with EPSG:4326(WEBMERCATOR) bounding box in 2D and 3D Test Replay Screenshots

The first 5 failed specs are shown, see all 24 specs in Cypress Cloud.

sommerfe
sommerfe reviewed Nov 20, 2025
View reviewed changes
Copy link
Contributor

@sommerfe sommerfe left a comment

Choose a reason for hiding this comment

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

Image When opening the report problem dialog there is this error, and you can not type anything into the text or input boxes

pakb added 3 commits November 20, 2025 16:27
@pakb
PB-2027: only request icon in LARGE size, and resize them with the scale
eddfdd3 
icons were doubly scaled because the scale was applied on top of a smaller icon URL. Reverting back to what was the use-case before the TS migration, which is that icons are always requested in the same size to the backend, and are scaled on the map (could we get rid of the scaling on the backend?)
@pakb
PB-2027: fix code so that marker/text test passes
44d0580 
simplifying a bit the useFieldValidation composable, and trying to maintain reactivity of props given there, even if it is two components down (EmailInput calls TextInput which calls useFieldValidation)
@pakb
PB-2027: simplify the useFieldValidation composable
a898578 
removing most bi-directional links and having all the "config" in one big catch'em all bag
@pakb pakb force-pushed the feat-PB-2027-fix-marker-text-test branch from ed1b01c to a898578 Compare November 20, 2025 15:27
@pakb
Copy link
Contributor Author

pakb commented Nov 20, 2025

I've massively simplified the useFieldValidation component (and checked that the Inputs.cy.ts component test is all green), it should be in a better place to review once again

@pakb pakb requested a review from sommerfe November 20, 2025 15:39
sommerfe
sommerfe reviewed Nov 20, 2025
View reviewed changes
Copy link
Contributor

@sommerfe sommerfe left a comment

Choose a reason for hiding this comment

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

the validation for the email and textarea in the report problem dialog doesn't seem to work anymore

pakb added 2 commits November 21, 2025 11:46
@pakb
PB-2027: fix lost validation reactivity with inputs and their props
98f78c6 
Some props need to be transformed to ref so that the composable doesn't loose the reactivity on them.

Also changing the logic of the userFieldValidation "pristin" flag, to clarify what it was meant for (and removing its usage whenever the default behavior was the one in place)
@pakb
PB-2027: remove double call to validate
4c84aa6 
watchEffect will be called whenever the composable is set up, and then each time one of the props is changed (so the onMounted hook will create a double unnecessary call)
@pakb pakb requested a review from sommerfe November 21, 2025 11:55
sommerfe
sommerfe reviewed Nov 21, 2025
View reviewed changes
Copy link
Contributor

@sommerfe sommerfe left a comment

Choose a reason for hiding this comment

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

I noticed two things for the report problem inputs

  1. the validation markers are already there before submitting, this is a different behavior than what we have on prod
  2. when closing the report problem with the cross, the values are cleaned up but the validation for the form stays

@pakb
Copy link
Contributor Author

pakb commented Nov 24, 2025

I noticed two things for the report problem inputs

  1. the validation markers are already there before submitting, this is a different behavior than what we have on prod
  2. when closing the report problem with the cross, the values are cleaned up but the validation for the form stays

You might want to retry after cleaning your cache, that was the behavior before my latest fixes but it shouldn't be the case (and behave like it used to before)

@sommerfe
Copy link
Contributor

sommerfe commented Nov 24, 2025

I noticed two things for the report problem inputs

  1. the validation markers are already there before submitting, this is a different behavior than what we have on prod
  2. when closing the report problem with the cross, the values are cleaned up but the validation for the form stays

You might want to retry after cleaning your cache, that was the behavior before my latest fixes but it shouldn't be the case (and behave like it used to before)

Thanks for the hint, indeed the point 2 is not valid anymore.
But the point 1 is still like I said, I even tried in a different browser and i get the same behavior there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sommerfe sommerfe sommerfe left review comments

Assignees

No one assigned

Labels

feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@pakb @sommerfe