Merge pull request #1084 from geoadmin/fix-pb-951-not-store-swisssearch

PB-951: do not store swisssearch param in url

src/modules/menu/components/search/SearchBar.vue

@@ -9,7 +9,7 @@ const dispatcher = { dispatcher: 'SearchBar' }
 
 const store = useStore()
 
-const isPristine = ref(true)
+const isPristine = ref(true) // if search bar is not yet modified by the user
 const showResults = ref(false)
 const searchInput = ref(null)
 const searchValue = ref('')
@@ -20,16 +20,21 @@ const searchQuery = computed(() => store.state.search.query)
 const hasResults = computed(() => store.state.search.results.length > 0)
 const isPhoneMode = computed(() => store.getters.isPhoneMode)
 
-watch(hasResults, (newValue) => {
-    // if an entry has been selected from the list, do not show the list again
-    // because the list has been hidden by onEntrySelected. Also if the search bar is pristine (not
-    // yet modified by the user) we don't want to show the result (e.g. at startup with a swisssearch
-    // query param)
-    if (!selectedEntry.value && !isPristine.value) {
-        log.debug(`Search has result changed to ${newValue}, change the show result to ${newValue}`)
-        showResults.value = newValue
-    }
-})
+watch(
+    hasResults,
+    (newValue) => {
+        // if an entry has been selected from the list, do not show the list again
+        // because the list has been hidden by onEntrySelected.
+        if (!selectedEntry.value) {
+            log.debug(
+                `Search has result changed to ${newValue}, change the show result to ${newValue}`
+            )
+            showResults.value = newValue
+        }
+    },
+    // we need to run the watcher immediately to make sure the result list is displayed on the first load
+    { immediate: true }
+)
 
 watch(showResults, (newValue) => {
     if (newValue && isPhoneMode.value && store.state.ui.showMenu) {

src/router/storeSync/SearchParamConfig.class.js

@@ -2,6 +2,7 @@ import AbstractParamConfig, {
     STORE_DISPATCHER_ROUTER_PLUGIN,
 } from '@/router/storeSync/abstractParamConfig.class'
 
+const URL_PARAM_NAME = 'swisssearch'
 /**
  * The goal is to stop centering on the search when sharing a position. When we share a position,
  * both the center and the crosshair are sets.
@@ -11,20 +12,47 @@ import AbstractParamConfig, {
  * @param {String} urlParamValue The search param
  */
 function dispatchSearchFromUrl(to, store, urlParamValue) {
-    store.dispatch('setSearchQuery', {
-        query: urlParamValue,
-        shouldCenter: !(to.query.crosshair && to.query.center),
-        dispatcher: STORE_DISPATCHER_ROUTER_PLUGIN,
-    })
+    // avoiding dispatching the search query to the store when there is nothing to set. Not avoiding this makes the CI test very flaky
+    if (urlParamValue) {
+        store.dispatch('setSearchQuery', {
+            query: urlParamValue,
+            shouldCenter: !(to.query.crosshair && to.query.center),
+            dispatcher: STORE_DISPATCHER_ROUTER_PLUGIN,
+        })
+    }
+}
+
+/**
+ * This will remove the query param from the URL It is necessary to do this in vanilla JS because
+ * the router does not provide a way to remove a query without reloading the page which then removes
+ * the value from the store.
+ *
+ * @param {Object} key The key to remove from the URL
+ */
+function removeQueryParamFromHref(key) {
+    const [baseUrl, queryString] = window.location.href.split('?')
+    if (!queryString) {
+        return
+    }
+
+    const params = new URLSearchParams(queryString)
+    if (!params.has(key)) {
+        return
+    }
+    params.delete(key)
+
+    const newQueryString = params.toString()
+    const newUrl = newQueryString ? `${baseUrl}?${newQueryString}` : baseUrl
+    window.history.replaceState({}, document.title, newUrl)
 }
 
 export default class SearchParamConfig extends AbstractParamConfig {
     constructor() {
         super({
-            urlParamName: 'swisssearch',
-            mutationsToWatch: ['setSearchQuery'],
+            urlParamName: URL_PARAM_NAME,
+            mutationsToWatch: [],
             setValuesInStore: dispatchSearchFromUrl,
-            extractValueFromStore: (store) => store.state.search.query,
+            afterSetValuesInStore: () => removeQueryParamFromHref(URL_PARAM_NAME),
             keepInUrlWhenDefault: false,
             valueType: String,
             defaultValue: '',

src/router/storeSync/abstractParamConfig.class.js

@@ -27,6 +27,8 @@ export default class AbstractParamConfig {
      *   added to the URL even though its value is set to the default value of the param.
      * @param {NumberConstructor | StringConstructor | BooleanConstructor, ObjectConstructor} valueType
      * @param {Boolean | Number | String | null} defaultValue
+     * @param {Function} afterSetValuesInStore A function that will be called after the store values
+     *   have been set
      */
     constructor({
         urlParamName,
@@ -37,6 +39,7 @@ export default class AbstractParamConfig {
         valueType = String,
         defaultValue = null,
         validateUrlInput = null,
+        afterSetValuesInStore = null,
     } = {}) {
         this.urlParamName = urlParamName
         this.mutationsToWatch = mutationsToWatch
@@ -51,6 +54,7 @@ export default class AbstractParamConfig {
             this.defaultValue = false
         }
         this.validateUrlInput = validateUrlInput
+        this.afterSetValuesInStore = afterSetValuesInStore
     }
 
     /**
@@ -201,4 +205,16 @@ export default class AbstractParamConfig {
             }
         })
     }
+
+    /**
+     * Triggers an action after the store has been populated with the query value. Returns a promise
+     *
+     * @returns {Promise<any>}
+     */
+    async afterPopulateStore() {
+        if (!this.afterSetValuesInStore) {
+            return
+        }
+        return await this.afterSetValuesInStore()
+    }
 }

src/router/storeSync/storeSync.routerPlugin.js

@@ -122,6 +122,7 @@ function urlQueryWatcher(store, to, from) {
 
         const setValueInStore = async (paramConfig, store, value) => {
             await paramConfig.populateStoreWithQueryValue(to, store, value)
+            await paramConfig.afterPopulateStore()
         }
 
         if (

src/store/modules/search.store.js

@@ -146,14 +146,12 @@ const actions = {
         }
         commit('setSearchResults', { results, dispatcher: `${dispatcher}/setSearchQuery` })
     },
-    setSearchResults: ({ commit }, { results, dispatcher }) =>
-        commit('setSearchResults', { results, dispatcher }),
     /**
      * @param commit
      * @param dispatch
      * @param {SearchResult} entry
      */
-    selectResultEntry: ({ dispatch, getters, rootState }, { entry, dispatcher }) => {
+    selectResultEntry: async ({ dispatch, getters, rootState, commit }, { entry, dispatcher }) => {
         const dispatcherSelectResultEntry = `${dispatcher}/search.store/selectResultEntry`
         switch (entry.resultType) {
             case SearchResultTypes.LAYER:
@@ -168,6 +166,23 @@ const actions = {
                         dispatcher: dispatcherSelectResultEntry,
                     })
                 }
+                // launching a new search to get (potential) layer features
+                try {
+                    const resultIncludingLayerFeatures = await search({
+                        outputProjection: rootState.position.projection,
+                        queryString: state.query,
+                        lang: rootState.i18n.lang,
+                        layersToSearch: getters.visibleLayers,
+                    })
+                    if (resultIncludingLayerFeatures.length > state.results.length) {
+                        commit('setSearchResults', {
+                            results: resultIncludingLayerFeatures,
+                            ...dispatcher,
+                        })
+                    }
+                } catch (error) {
+                    log.error(`Search failed`, error)
+                }
                 break
             case SearchResultTypes.LOCATION:
                 zoomToEntry(entry, dispatch, dispatcher, dispatcherSelectResultEntry)
@@ -221,10 +236,6 @@ const actions = {
 
                 break
         }
-        dispatch('setSearchQuery', {
-            query: entry.sanitizedTitle,
-            dispatcher: dispatcherSelectResultEntry,
-        })
     },
 }
 

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

@@ -275,6 +275,8 @@ describe('Testing the feature selection', () => {
                 },
                 true
             )
+            cy.wait(['@routeChange', '@layers', '@topics', '@topic-ech'])
+
             const featureCountWithKml = DEFAULT_FEATURE_COUNT_RECTANGLE_SELECTION + 1
             cy.openMenuIfMobile()
             cy.get('[data-cy="menu-tray-tool-section"]:visible').click()
@@ -285,7 +287,16 @@ describe('Testing the feature selection', () => {
                 force: true,
             })
             cy.get('[data-cy="import-file-load-button"]:visible').click()
-            cy.get('[data-cy="import-file-close-button"]:visible').click()
+
+            cy.wait(['@icon-sets', '@icon-set-babs', '@icon-set-default'])
+
+            cy.get('[data-cy="file-input-text"]').should(
+                'have.value',
+                'external-kml-file.kml, 0.368 kb'
+            )
+            cy.get('[data-cy="import-file-close-button"]:visible').realClick()
+            cy.readStoreValue('state.layers.activeLayers.length').should('eq', 2)
+            cy.readStoreValue('getters.visibleLayers.length').should('eq', 2)
 
             cy.closeMenuIfMobile()
 
@@ -324,6 +335,7 @@ describe('Testing the feature selection', () => {
 
             cy.get('[data-cy="feature-list-load-more"]').as('loadMore').should('be.visible')
             cy.get('@loadMore').click()
+            cy.wait('@routeChange')
             cy.wait('@identify')
                 .its('request.query')
                 .should((query) => {
@@ -363,6 +375,7 @@ describe('Testing the feature selection', () => {
                 },
                 true
             )
+            cy.wait('@routeChange')
 
             cy.intercept('**identify**', {
                 fixture: 'features/features.fixture',
@@ -372,8 +385,7 @@ describe('Testing the feature selection', () => {
                 x: 30,
                 y: -80,
             })
-            cy.wait('@identifySingleFeature')
-            cy.wait(`@htmlPopup`)
+            cy.wait(['@identifySingleFeature', '@htmlPopup'])
 
             cy.get('@highlightedFeatures').should('be.visible')
             cy.get('@highlightedFeatures').find('[data-cy="feature-item"]').should('have.length', 1)
@@ -391,6 +403,16 @@ describe('Testing the feature selection', () => {
             })
             cy.wait('@emptyIdentify')
             cy.get('@highlightedFeatures').should('not.exist')
+
+            cy.get('@routeChange.all').should('have.length', 5)
+            cy.get('@layers.all').should('have.length', 1)
+            cy.get('@topics.all').should('have.length', 1)
+            cy.get('@topic-ech.all').should('have.length', 1)
+            cy.get('@htmlPopup.all').should('have.length', 101)
+            cy.get('@noMoreResults.all').should('have.length', 1)
+            cy.get('@identify.all').should('have.length', 2)
+            cy.get('@identifySingleFeature.all').should('have.length', 1)
+            cy.get('@emptyIdentify.all').should('have.length', 1)
         })
     })
 })

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

@@ -268,9 +268,9 @@ describe('Test on legacy param import', () => {
                 false
             )
             cy.readStoreValue('state.search.query').should('eq', '1530 Payerne')
-            cy.url().should('include', 'swisssearch=1530+Payerne')
+            cy.url().should('not.contain', 'swisssearch')
             cy.get('[data-cy="searchbar"]').click()
-            cy.get('[data-cy="search-results-locations"]').should('be.visible')
+            cy.get('[data-cy="search-results-locations"]').should('not.be.visible')
         })
         it('External WMS layer', () => {
             const layerName = 'OpenData-AV'