Merge pull request #649 from geoadmin/bug-drawing-e2e-ci

Fix race condition when exporting/saving kml

adr/2024_01_31_drawing_icon_size.md

@@ -91,8 +91,8 @@ With table and following the openlayer scale formulas our Icon size should be
 | --------- | ------------- | ----------- | -------------- |
 | small     | 0.5           | 0.75        | 24             |
 | medium    | 0.75          | 1.125       | 36             |
-| big       | 1             | 1.5         | 48             |
-| bigger    | 1.25          | 1.875       | 60             |
+| large     | 1             | 1.5         | 48             |
+| xlarge    | 1.25          | 1.875       | 60             |
 
 \* `kml_scale` := `ol_scale` \* `size` / 32 => `ol_scale` \* 1.5
 
@@ -102,5 +102,5 @@ With table and following the openlayer scale formulas our Icon size should be
 | ------ | ------------ | --------- |
 | small  | 1            | 1         |
 | medium | 1.5          | 1.5       |
-| big    | 2            | 2         |
-| bigger | -            | 2.5       |
+| large  | 2            | 2         |
+| xlarge | -            | 2.5       |

src/api/features/EditableFeature.class.js

@@ -2,6 +2,7 @@ import { Icon as olIcon } from 'ol/style'
 
 import { extractOlFeatureGeodesicCoordinates } from '@/api/features/features.api'
 import SelectableFeature from '@/api/features/SelectableFeature.class'
+import { DEFAULT_ICON_URL_PARAMS } from '@/api/icon.api'
 import { allStylingColors, allStylingSizes, MEDIUM, RED } from '@/utils/featureStyleUtils'
 
 /** @enum */
@@ -152,7 +153,7 @@ export default class EditableFeature extends SelectableFeature {
         // as well as to use browser cache more efficiently we get all the
         // icons at the default scale of 1 (48x48px) and do the scaling
         // on the client
-        return this._icon?.generateURL(this.fillColor)
+        return this._icon?.generateURL(this.fillColor, DEFAULT_ICON_URL_PARAMS.scale)
     }
 
     generateOpenlayersIcon() {
@@ -162,6 +163,7 @@ export default class EditableFeature extends SelectableFeature {
                   crossOrigin: 'Anonymous',
                   anchor: this.icon.anchor,
                   scale: this.iconSizeScale,
+                  size: DEFAULT_ICON_URL_PARAMS.size,
               })
             : null
     }

src/api/icon.api.js

@@ -4,6 +4,20 @@ import { API_SERVICES_BASE_URL } from '@/config'
 import { RED } from '@/utils/featureStyleUtils'
 import log from '@/utils/logging'
 
+/**
+ * Default Icon parameters for the URL.
+ *
+ * NOTE: The size should match the received size for the scale from the backend. It is needed to
+ * avoid race condition when exporting/saving KML. Openlayer requires the size to compute the
+ * scale.
+ *
+ * TODO: take the default size from the backend icon API
+ */
+export const DEFAULT_ICON_URL_PARAMS = {
+    scale: 1,
+    size: [48, 48],
+}
+
 /**
  * Collection of icons belonging to the same "category" (or set).
  *
@@ -139,7 +153,7 @@ export class DrawingIcon {
      * @param {Number} iconScale The scale to use for this icon's URL
      * @returns {String} A full URL to this icon on the service-icons backend
      */
-    generateURL(iconColor = RED, iconScale = 1) {
+    generateURL(iconColor = RED, iconScale = DEFAULT_ICON_URL_PARAMS.scale) {
         const rgb = iconColor.rgb.slice(0, 3)
         return this._imageTemplateURL
             .replace('{icon_set_name}', this._iconSetName)

tests/cypress/support/drawing.js

@@ -2,12 +2,16 @@ import pako from 'pako'
 
 import { EditableFeatureTypes } from '@/api/features/EditableFeature.class'
 import { BREAKPOINT_PHONE_WIDTH } from '@/config'
+import { GREEN, RED } from '@/utils/featureStyleUtils'
 import { randomIntBetween } from '@/utils/numberUtils'
 
 export const addIconFixtureAndIntercept = () => {
-    cy.intercept(`**/api/icons/sets/default/icons/*@*.png`, {
+    cy.intercept(`**/api/icons/sets/default/icons/**${RED.rgbString}.png`, {
         fixture: 'service-icons/placeholder.png',
     }).as('icon-default')
+    cy.intercept(`**/api/icons/sets/default/icons/**${GREEN.rgbString}.png`, {
+        fixture: 'service-icons/placeholder.png',
+    }).as('icon-default-green')
     cy.intercept(`**/api/icons/sets/babs/icons/*@*.png`, {
         fixture: 'service-icons/placeholder.png',
     }).as('icon-babs')
@@ -208,3 +212,34 @@ export function kmlMetadataTemplate(data) {
     }
     return metadata
 }
+
+/**
+ * Wait until all defaults red icons have been loaded
+ *
+ * This wait is required when checking the KML XML content, because openlayer requires the icon size
+ * from the icon request in order to compute the icon scale in KML, if the icon is not loaded when
+ * saving the KML, openlayer uses a default size of 64 pixel which defer from our icon size of 48
+ * pixel.
+ *
+ * So before doing an action that would change the icon size, we need to make sure that all icons
+ * have been already loaded to avoid any race condition.
+ *
+ * This wait is only needed in cypress as in real life the save has a debouncing of at least 2
+ * seconds which ensure that we have the icons.
+ */
+Cypress.Commands.add('waitOnAllIconsDefault', () => {
+    cy.get('@icon-set-default').then((interception) => {
+        cy.wait(Array(interception.response.body.items.length).fill('@icon-default'))
+    })
+})
+
+/**
+ * Wait until all defaults green icons have been loaded
+ *
+ * @see waitOnAllIconsDefault for more infos
+ */
+Cypress.Commands.add('waitOnAllIconsDefaultGreen', () => {
+    cy.get('@icon-set-default').then((interception) => {
+        cy.wait(Array(interception.response.body.items.length).fill('@icon-default-green'))
+    })
+})

tests/cypress/tests-e2e/drawing.cy.js

@@ -10,6 +10,7 @@ import {
 } from 'tests/cypress/support/drawing'
 
 import { EditableFeatureTypes } from '@/api/features/EditableFeature.class'
+import { DEFAULT_ICON_URL_PARAMS } from '@/api/icon.api'
 import LayerTypes from '@/api/layers/LayerTypes.enum'
 import { API_SERVICE_KML_BASE_URL, DEFAULT_PROJECTION } from '@/config'
 import { WGS84 } from '@/utils/coordinates/coordinateSystems'
@@ -35,7 +36,7 @@ const isNonEmptyArray = (value) => {
 const KML_STYLE_RED = 'ff0000ff'
 const KML_STYLE_BLACK = 'ff000000'
 
-const DEFAULT_ICON_URL_SCALE = '1x'
+const DEFAULT_ICON_URL_SCALE = `${DEFAULT_ICON_URL_PARAMS.scale}x`
 
 describe('Drawing module tests', () => {
     context('Drawing mode/tools', () => {
@@ -53,14 +54,13 @@ describe('Drawing module tests', () => {
         beforeEach(() => {
             cy.goToDrawing()
         })
-
         it('can create marker/icons and edit them', () => {
             // it should load all icon sets as soon as we enter the drawing module
             cy.wait('@icon-sets')
             cy.wait('@icon-set-default')
 
             cy.clickDrawingTool(EditableFeatureTypes.MARKER)
-            cy.get('[data-cy="ol-map"]').click()
+            cy.get('[data-cy="ol-map"]:visible').click()
 
             cy.wait('@post-kml')
 
@@ -71,10 +71,10 @@ describe('Drawing module tests', () => {
                 .should('include', `${RED.rgbString}.png`)
 
             // clicking on the "Edit icon" button
-            cy.get('[data-cy="drawing-style-marker-button"]').click()
+            cy.get('[data-cy="drawing-style-marker-button"]:visible').click()
             // opening up the icon set selector
             cy.get(
-                '[data-cy="drawing-style-icon-set-button"] [data-cy="dropdown-main-button"]'
+                '[data-cy="drawing-style-icon-set-button"] [data-cy="dropdown-main-button"]:visible'
             ).click()
             // the list of icon sets should contain all backend's possibilities
             cy.get(`[data-cy="dropdown-item-default"]`).should('be.visible')
@@ -91,29 +91,22 @@ describe('Drawing module tests', () => {
             ).should('not.exist')
             // going back to the default icon set
             cy.get(
-                '[data-cy="drawing-style-icon-set-button"] [data-cy="dropdown-main-button"]'
+                '[data-cy="drawing-style-icon-set-button"] [data-cy="dropdown-main-button"]:visible'
             ).click()
-            cy.get('[data-cy="dropdown-item-default"]').click()
+            cy.get('[data-cy="dropdown-item-default"]:visible').click()
+            cy.get('[data-cy="dropdown-item-default"]').should('not.be.visible')
             // color selector should be back
             cy.get(
                 '[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-color-select-box"]'
             ).should('be.visible')
-            // closing the icon set selector (we've not selected another icon, so no update of the KML should have occurred)
-            cy.get(
-                '[data-cy="drawing-style-icon-set-button"] [data-cy="dropdown-main-button"]'
-            ).click()
-
-            // creating intercepts for all icon requests
-            cy.intercept(`**/api/icons/sets/default/icons/**${GREEN.rgbString}.png`, {
-                fixture: 'service-icons/placeholder.png',
-            }).as('icon-default-green')
 
             // changing icon list's color to green
             cy.get(
-                `[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-color-select-box"] [data-cy="color-selector-${GREEN.name}"]`
+                `[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-color-select-box"] [data-cy="color-selector-${GREEN.name}"]:visible`
             ).click()
             // it should load all icons with the green color
-            cy.wait('@icon-default-green')
+            cy.waitOnAllIconsDefaultGreen()
+
             // the color of the marker already placed on the map must switch to green
             cy.wait('@update-kml').then((interception) => {
                 cy.checkKMLRequest(interception, [
@@ -125,7 +118,7 @@ describe('Drawing module tests', () => {
 
             // opening up the icon size selector
             cy.get(
-                '[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-size-selector"] [data-cy="dropdown-main-button"]'
+                '[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-size-selector"] [data-cy="dropdown-main-button"]:visible'
             ).click()
             // all sizes should be represented
             allStylingSizes.forEach((size) => {
@@ -137,14 +130,14 @@ describe('Drawing module tests', () => {
             cy.get(
                 `[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-size-selector"] [data-cy="dropdown-item-${LARGE.label}"]`
             ).click()
-            // icons should be reloaded as large green
-            cy.wait('@icon-default-green')
             // the existing icon on the map must be updated to large and green
             cy.wait('@update-kml').then((interception) => {
                 cy.checkKMLRequest(interception, [
                     new RegExp(
                         `<IconStyle><scale>${LARGE.iconScale * LEGACY_ICON_XML_SCALE_FACTOR}</scale>`
                     ),
+                    new RegExp(`<Icon>.*?<gx:w>48</gx:w>.*?</Icon>`),
+                    new RegExp(`<Icon>.*?<gx:h>48</gx:h>.*?</Icon>`),
                     new RegExp(
                         `<href>https?://.*/api/icons/sets/default/icons/001-marker@${DEFAULT_ICON_URL_SCALE}-${GREEN.rgbString}.png</href>`
                     ),
@@ -153,14 +146,14 @@ describe('Drawing module tests', () => {
 
             // opening up all icons of the current sets so that we may choose a new one
             cy.get(
-                '[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-toggle-all-icons-button"]'
+                '[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-toggle-all-icons-button"]:visible'
             ).click()
             // picking up the 4th icon of the set
             cy.fixture('service-icons/set-default.fixture.json').then((defaultIconSet) => {
                 const fourthIcon = defaultIconSet.items[3]
                 cy.get(
-                    `[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-icon-selector-${fourthIcon.name}"]`
-                ).click({ force: true })
+                    `[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-icon-selector-${fourthIcon.name}"]:visible`
+                ).click()
                 // the KML must be updated with the newly selected icon
                 cy.wait('@update-kml').then((interception) =>
                     cy.checkKMLRequest(interception, [
@@ -172,12 +165,12 @@ describe('Drawing module tests', () => {
             })
             // closing the icons
             cy.get(
-                '[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-toggle-all-icons-button"]'
-            ).click({ force: true })
+                '[data-cy="drawing-style-marker-popup"] [data-cy="drawing-style-toggle-all-icons-button"]:visible'
+            ).click()
             // closing the icon style popup
-            cy.get('[data-cy="drawing-style-popover"] [data-cy="close-popover-button"]').click({
-                force: true,
-            })
+            cy.get(
+                '[data-cy="drawing-style-popover"] [data-cy="close-popover-button"]:visible'
+            ).click()
 
             // changing/editing the title of this marker
             testTitleEdit()