Skip to content

PB-1318 : delete KML definitively#1224

Closed
sami-nouidri-swisstopo wants to merge 6 commits intodevelopfrom
task-PB-1318-delete-kml
Closed

PB-1318 : delete KML definitively#1224
sami-nouidri-swisstopo wants to merge 6 commits intodevelopfrom
task-PB-1318-delete-kml

Conversation

@sami-nouidri-swisstopo
Copy link
Contributor

@sami-nouidri-swisstopo sami-nouidri-swisstopo commented Jan 28, 2025

KML deletion is now done by sending a request to the pre-existing DELETE route in the service-kml backend.

Test link

@cypress
Copy link

cypress bot commented Jan 28, 2025

web-mapviewer    Run #4413

Run Properties:  status check failed Failed #4413  •  git commit 1ae23ba41b: PB-1318 : addressed concerns of collision by changing ID generation pattern
Project web-mapviewer
Branch Review task-PB-1318-delete-kml
Run status status check failed Failed #4413
Run duration 05m 49s
Commit git commit 1ae23ba41b: PB-1318 : addressed concerns of collision by changing ID generation pattern
Committer Sami Nouidri
View all properties for this run ↗︎

Test results
Tests that failed  Failures 1
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 232
View all changes introduced in this branch ↗︎

Tests for review

Failed  tests/cypress/tests-e2e/drawing.cy.js • 1 failed test • e2e/electron/mobile

View Output

Test Artifacts
Drawing module tests > Drawing mode/tools > can create marker/icons and edit them Test Replay Screenshots

Sami Nouidri added 4 commits January 30, 2025 14:15
The functions 'deleteDrawing()' and 'deleteKml' have been added to fully delete
the kml, using the existing DELETE route. A back-end URL override has been added
for service-kml as well, although it doesn't seem to work for now.
@sami-nouidri-swisstopo sami-nouidri-swisstopo marked this pull request as ready for review January 30, 2025 13:18
@sami-nouidri-swisstopo sami-nouidri-swisstopo changed the title PB-1318 : started work on routines to delete KML using service-kml PB-1318 : delete KML definitively Jan 30, 2025
Copy link
Contributor

@ltkum ltkum left a comment

Choose a reason for hiding this comment

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

A few things to change, but otherwise looks good.

$ Outdated
# modified: src/utils/components/DropdownButton.vue
# modified: tests/cypress/tests-e2e/drawing.cy.js
# modified: tests/cypress/tests-e2e/reportProblem.cy.js
#
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like this file is the result of a stdout to a file, and also looks like it's not the correct branch / PR. I would remove that.

* @returns {Promise<void>}
*/
export const deleteKml = (id, adminId) => {
log.info('base url : ', kmlBaseUrl)
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a developement log which stayed in the code. You should remove this.

Copy link
Contributor

@ltkum ltkum left a comment

Choose a reason for hiding this comment

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

LGTM :)

201,
kmlMetadataTemplate({
id: `${randomIntBetween(1000, 9999)}_fileId`,
id: `${Date.now()}_${randomIntBetween(1000, 9999)}_fileId`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I like it

sami-nouidri-swisstopo pushed a commit that referenced this pull request Feb 5, 2025
…erarchy

Please refer to PR #1224 to view the original git history and review
@sami-nouidri-swisstopo
Copy link
Contributor Author

As the map viewer has been refactored as part of the modularization effort, the changes have been manually rebased here : #1230

sami-nouidri-swisstopo pushed a commit that referenced this pull request Feb 6, 2025
…erarchy

Please refer to PR #1224 to view the original git history and review
@sami-nouidri-swisstopo sami-nouidri-swisstopo deleted the task-PB-1318-delete-kml branch February 7, 2025 12:58
sami-nouidri-swisstopo pushed a commit that referenced this pull request Feb 7, 2025
Please refer to PR #1224 for original git history and review
sami-nouidri-swisstopo pushed a commit that referenced this pull request Feb 12, 2025
Please refer to PR #1224 for original git history and review

pick 357653f PB-1318 : adjusted cypress tests to wait for variables to be set
pick 231ae54 PB-1318 : fixed wrong request type for cypress tests
sami-nouidri-swisstopo pushed a commit that referenced this pull request Feb 13, 2025
Please refer to PR #1224 for original git history and review
sami-nouidri-swisstopo pushed a commit that referenced this pull request Feb 14, 2025
Please refer to PR #1224 for original git history and review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants