Merge pull request #825 from geoadmin/bugfix-PB-447-broken-legacy-parser

PB-447: legacy parser not switching to map view

package.json

@@ -79,7 +79,7 @@
         "vue": "^3.4.21",
         "vue-chartjs": "^5.3.0",
         "vue-i18n": "^9.11.0",
-        "vue-router": "^4.3.0",
+        "vue-router": "^4.3.2",
         "vue3-social-sharing": "^1.0.3",
         "vuex": "^4.1.0"
     },

src/router/appLoadingManagement.routerPlugin.js

@@ -21,6 +21,7 @@ const dispatcher = { dispatcher: 'appLoadingManagement.routerPlugin' }
  * @param {Store} store
  */
 const appLoadingManagementRouterPlugin = (router, store) => {
+    log.debug('[APP LOADING MANAGEMENT ROUTER] entry in app loading management plugin')
     if (!store.state.app.isReady) {
         const isLegacyUrl = isLegacyParams(window?.location?.search)
 
@@ -58,6 +59,8 @@ const appLoadingManagementRouterPlugin = (router, store) => {
             }
         })
     }
+    log.debug('[APP LOADING MANAGEMENT ROUTER] exiting app loading management plugin')
+    return true
 }
 
 export default appLoadingManagementRouterPlugin

src/router/index.js

@@ -11,6 +11,7 @@ import {
     MAP_VIEW,
 } from '@/router/viewNames'
 import store from '@/store'
+import log from '@/utils/logging'
 import { parseQuery, stringifyQuery } from '@/utils/url-router'
 import EmbedView from '@/views/EmbedView.vue'
 import LegacyParamsView from '@/views/LegacyParamsView.vue'
@@ -69,6 +70,10 @@ const router = createRouter({
     stringifyQuery: stringifyQuery,
 })
 
+router.onError((error) => {
+    log.error('[Router error] :', error)
+})
+
 appLoadingManagementRouterPlugin(router, store)
 legacyPermalinkManagementRouterPlugin(router, store)
 storeSyncRouterPlugin(router, store)

src/router/legacyPermalinkManagement.routerPlugin.js

@@ -281,6 +281,12 @@ const legacyPermalinkManagementRouterPlugin = (router, store) => {
         )
         let unSubscribeStoreMutation = null
         const unsubscribeRouter = router.beforeEach(async (to, from) => {
+            log.debug(
+                `[Legacy URL] entry into the legacy router with parameters 'form' and 'to': `,
+                from,
+                to
+            )
+
             if (MAP_VIEWS.includes(to.name) && from === START_LOCATION) {
                 // Redirect to the LegacyParamsView until the app is ready and that the legacy
                 // params have been parsed and converted. This is needed in order to postpone the
@@ -307,18 +313,22 @@ const legacyPermalinkManagementRouterPlugin = (router, store) => {
                         router.replace(newRoute)
                     }
                 })
-
+                log.debug(
+                    `[Legacy URL]: redirect between embed legacy view or standard legacy view`
+                )
                 return {
                     name: legacyEmbed ? LEGACY_EMBED_PARAM_VIEW : LEGACY_PARAM_VIEW,
                     replace: true,
                 }
             }
 
             if (MAP_VIEWS.includes(to.name) && LEGACY_VIEWS.includes(from.name)) {
-                log.debug('[Legacy URL] leaving the legacy URL plugin')
+                log.info('[Legacy URL] Work is done, unsuscribing from the mutations')
                 unsubscribeRouter()
                 unSubscribeStoreMutation()
             }
+            log.debug('[Legacy URL] exiting the Legacy URL router.')
+            return true
         })
     }
 }

src/router/storeSync/storeSync.routerPlugin.js

@@ -7,7 +7,6 @@ import { MAP_VIEWS } from '@/router/viewNames'
 import log from '@/utils/logging'
 
 export const FAKE_URL_CALLED_AFTER_ROUTE_CHANGE = '/tell-cypress-route-has-changed'
-
 const watchedMutations = [
     ...new Set(storeSyncConfig.map((paramConfig) => paramConfig.mutationsToWatch).flat()),
 ]
@@ -39,7 +38,6 @@ const isRoutePushNeeded = (store, currentRoute) => {
 
 // flag to distinguish URL change originated by this module or by another source
 let routeChangeIsTriggeredByThisModule = false
-
 /**
  * Watch for store changes and reflect the changes in the URL query param if needed
  *
@@ -64,11 +62,15 @@ function storeMutationWatcher(store, mutation, router) {
             storeSyncConfig.forEach((paramConfig) =>
                 paramConfig.populateQueryWithStoreValue(query, store)
             )
-            log.info('[store sync router] Store has changed, rerouting app to query', query)
+            log.info(
+                '[store sync router] Store has changed, rerouting app to query',
+                query,
+                router.currentRoute.value.name
+            )
             routeChangeIsTriggeredByThisModule = true
             router
                 .push({
-                    name: router.currentRoute.name,
+                    name: router.currentRoute.value.name,
                     query,
                 })
                 .catch((error) => {
@@ -100,15 +102,16 @@ function storeMutationWatcher(store, mutation, router) {
  *   query changes), false to cancel the navigation or RouteLocationRaw to change url query
  *   parameter.
  */
-function urlQueryWatcher(store, to) {
+function urlQueryWatcher(store, to, from) {
+    log.debug(`[URL query watcher] entry in the url query watcher function`)
     if (routeChangeIsTriggeredByThisModule) {
         log.debug(`[Router store plugin] Url query watcher triggered by itself ignore it`, to)
         // Only sync route params when the route change has not been
         // triggered by the sync from store mutations watcher above.
         routeChangeIsTriggeredByThisModule = false
         return undefined
     }
-    log.debug(`[URL query watcher]`, to.query)
+    log.debug(`[URL query watcher] queries 'from' and 'to' are :`, from?.query, to.query)
     const pendingStoreDispatch = []
     let requireQueryUpdate = false
     const newQuery = { ...to.query }
@@ -160,6 +163,7 @@ function urlQueryWatcher(store, to) {
             requireQueryUpdate = true
         }
     })
+
     // Fake call to a URL so that Cypress can wait for route changes without waiting for arbitrary length of time
     if (IS_TESTING_WITH_CYPRESS) {
         Promise.all(pendingStoreDispatch).then(() => {
@@ -168,6 +172,7 @@ function urlQueryWatcher(store, to) {
             })
         })
     }
+
     if (requireQueryUpdate) {
         log.debug(`[URL query watcher] Update URL query to ${JSON.stringify(newQuery)}`)
         // NOTE: this rewrite of query currently don't work when navigating manually got the `/#/`
@@ -191,7 +196,42 @@ function urlQueryWatcher(store, to) {
  */
 const storeSyncRouterPlugin = (router, store) => {
     let unsubscribeStoreMutation = null
-    router.beforeEach((to) => {
+    router.beforeEach((to, from) => {
+        log.debug(
+            `[Router store plugin] Entering the store sync plugin with the following 'from' and 'to': `,
+            from,
+            to
+        )
+        // we define a return Value, so we can check across the function what its value is
+        let retVal = undefined
+
+        if (!MAP_VIEWS.includes(to.name)) {
+            // leaving MapView make sure to unsubscribe the store mutation
+            if (unsubscribeStoreMutation) {
+                log.info(
+                    `[Router store plugin] Leaving ${to.name}, unregister store mutation watcher`
+                )
+
+                unsubscribeStoreMutation()
+                retVal = undefined
+            }
+        }
+        if (MAP_VIEWS.includes(to.name) && store.state.app.isReady) {
+            // Synchronize the store with the url query only on MapView and when the application
+            // is ready
+            retVal = urlQueryWatcher(store, to, from)
+        }
+
+        log.debug('[Router store plugin] exiting navigation guard with the following value', retVal)
+        // Note we return undefined to validate the route, see Vue Router documentation
+        return retVal
+    })
+
+    // There were cases were this mutation subscription would trigger too early after a legacy query.
+    // This would cause the storeMutationWatcher to push a new route with its 'currentRoute' value,
+    // which was LEGACY. By moving this subscription to the after Each loop, we ensure the 'currentRoute'
+    // is always set to MAPVIEW, avoiding a lock of the viewer.
+    router.afterEach((to) => {
         if (MAP_VIEWS.includes(to.name) && !unsubscribeStoreMutation) {
             log.info('[Router store plugin] Entering MapView, register store mutation watcher')
             // listening to store mutation in order to update URL
@@ -208,23 +248,7 @@ const storeSyncRouterPlugin = (router, store) => {
                     storeMutationWatcher(store, mutation, router)
                 }
             })
-        } else if (!MAP_VIEWS.includes(to.name)) {
-            // leaving MapView make sure to unsubscribe the store mutation
-            if (unsubscribeStoreMutation) {
-                log.info(
-                    `[Router store plugin] Leaving ${to.name}, unregister store mutation watcher`
-                )
-                unsubscribeStoreMutation()
-            }
         }
-
-        if (MAP_VIEWS.includes(to.name) && store.state.app.isReady) {
-            // Synchronize the store with the url query only on MapView and when the application
-            // is ready
-            return urlQueryWatcher(store, to)
-        }
-        // Note we return undefined to validate the route, see Vue Router documentation
-        return undefined
     })
 }
 

src/store/modules/app.store.js

@@ -9,7 +9,7 @@ export default {
         isReady: false,
 
         /**
-         * Flag telling that the Map Module is ready. This is usefull for E2E testing which should
+         * Flag telling that the Map Module is ready. This is useful for E2E testing which should
          * not start before the Map Module is ready.
          */
         isMapReady: false,