From f5581c78c843d3a87fa72d364655968941564ecb Mon Sep 17 00:00:00 2001
From: Christoph Reiter <reiter.christoph@gmail.com>
Date: Thu, 28 Apr 2022 11:09:46 +0200
Subject: [PATCH] app-shell: allow trailing paths in the router

In case the path ending with the activity ID has more trailing path elements, we don't fall
back to the default route but just ignore them (and save them in the router state).

This allows passing in extra information via the path without breaking the routing,
and in case we want to forward the routing to the activities in the future we can use
the extra path elements we store for that.

For this to work with our current logic we have to stop comparing path strings and compare
the resulting computed state everywhere instead.
---
 packages/app-shell/src/app-shell.js           | 17 +++-
 .../app-shell/src/dbp-app-shell-welcome.js    |  2 +-
 packages/app-shell/src/router.js              | 80 ++++++++++---------
 3 files changed, 55 insertions(+), 44 deletions(-)

diff --git a/packages/app-shell/src/app-shell.js b/packages/app-shell/src/app-shell.js
index d110230d..95ea1045 100644
--- a/packages/app-shell/src/app-shell.js
+++ b/packages/app-shell/src/app-shell.js
@@ -62,6 +62,7 @@ export class AppShell extends ScopedElementsMixin(DBPLitElement) {
         this._roles = [];
         this._i18n = createInstance();
         this.lang = this._i18n.language;
+        this._extra = [];
 
         this.matomoUrl = '';
         this.matomoSiteId = -1;
@@ -179,6 +180,7 @@ export class AppShell extends ScopedElementsMixin(DBPLitElement) {
                     return {
                         lang: this.lang,
                         component: '',
+                        extra: [],
                     };
                 },
             },
@@ -191,18 +193,20 @@ export class AppShell extends ScopedElementsMixin(DBPLitElement) {
                             return {
                                 lang: params.lang,
                                 component: '',
+                                extra: [],
                             };
                         },
                     },
                     {
                         name: 'mainRoute',
-                        path: '/:component',
+                        path: ['/:component', '/:component/(.*)'],
                         action: (context, params) => {
-                            // remove the additional parameters added by Keycloak
-                            let componentTag = params.component.toLowerCase().replace(/&.+/, '');
+                            let componentTag = params.component.toLowerCase();
+                            let extra = params[0] ? params[0].split('/') : [];
                             return {
                                 lang: params.lang,
                                 component: componentTag,
+                                extra: extra,
                             };
                         },
                     },
@@ -218,17 +222,20 @@ export class AppShell extends ScopedElementsMixin(DBPLitElement) {
                     let state = {
                         component: this.activeView,
                         lang: this.lang,
+                        extra: this._extra,
                     };
                     return state;
                 },
                 setState: (state) => {
                     this.updateLangIfChanged(state.lang);
                     this.switchComponent(state.component);
+                    this._extra = state.extra;
                 },
                 getDefaultState: () => {
                     return {
                         lang: 'de',
                         component: this.routes[0],
+                        extra: [],
                     };
                 },
             },
@@ -270,8 +277,10 @@ export class AppShell extends ScopedElementsMixin(DBPLitElement) {
     connectedCallback() {
         super.connectedCallback();
 
-        if (this.src) this.fetchMetadata(this.src);
         this.initRouter();
+        if (this.src) {
+            this.fetchMetadata(this.src);
+        }
     }
 
     /**
diff --git a/packages/app-shell/src/dbp-app-shell-welcome.js b/packages/app-shell/src/dbp-app-shell-welcome.js
index baea4604..d16078bf 100644
--- a/packages/app-shell/src/dbp-app-shell-welcome.js
+++ b/packages/app-shell/src/dbp-app-shell-welcome.js
@@ -61,7 +61,7 @@ class AppShellWelcome extends ScopedElementsMixin(LitElement) {
                 cursor: pointer;
                 text-decoration: none;
             }
-            
+
             h2 a {
                 white-space: nowrap;
             }
diff --git a/packages/app-shell/src/router.js b/packages/app-shell/src/router.js
index cbaefc12..4f9461e0 100644
--- a/packages/app-shell/src/router.js
+++ b/packages/app-shell/src/router.js
@@ -1,6 +1,10 @@
 import UniversalRouter from 'universal-router';
 import generateUrls from 'universal-router/generateUrls';
 
+function stateMatches(a, b) {
+    return JSON.stringify(a, Object.keys(a).sort()) === JSON.stringify(b, Object.keys(b).sort());
+}
+
 /**
  * A wrapper around UniversalRouter which adds history integration
  */
@@ -31,28 +35,24 @@ export class Router {
 
         window.addEventListener('popstate', (event) => {
             this.setStateFromCurrentLocation();
-            this.dispatchLocationChanged();
+            this._dispatchLocationChanged();
         });
     }
 
+    async _getStateForPath(pathname) {
+        let isBasePath = pathname.replace(/\/$/, '') === this.router.baseUrl.replace(/\/$/, '');
+        if (isBasePath) {
+            return this.getDefaultState();
+        }
+        return this.router.resolve({pathname: pathname});
+    }
+
     /**
      * In case something else has changed the location, update the app state accordingly.
      */
     setStateFromCurrentLocation() {
-        const oldPathName = location.pathname;
-
-        this.router
-            .resolve({pathname: oldPathName})
+        this._getStateForPath(location.pathname)
             .then((page) => {
-                const newPathname = this.getPathname(page);
-                // In case of a router redirect, set the new location
-                if (newPathname !== oldPathName) {
-                    const referrerUrl = location.href;
-                    window.history.replaceState({}, '', newPathname);
-                    this.dispatchLocationChanged(referrerUrl);
-                } else if (this.isBasePath(oldPathName)) {
-                    page = this.getDefaultState();
-                }
                 this.setState(page);
             })
             .catch((e) => {
@@ -61,10 +61,6 @@ export class Router {
             });
     }
 
-    isBasePath(pathname) {
-        return pathname.replace(/\/$/, '') === this.router.baseUrl.replace(/\/$/, '');
-    }
-
     /**
      * Update the router after some internal state change.
      */
@@ -72,18 +68,21 @@ export class Router {
         // Queue updates so we can call this multiple times when changing state
         // without it resulting in multiple location changes
         setTimeout(() => {
-            const newPathname = this.getPathname();
-            const oldPathname = location.pathname;
-            if (newPathname === oldPathname) return;
-
-            const defaultPathname = this.getPathname(this.getDefaultState());
-            if (newPathname === defaultPathname && this.isBasePath(oldPathname)) {
-                return;
-            }
-
-            const referrerUrl = location.href;
-            window.history.pushState({}, '', newPathname);
-            this.dispatchLocationChanged(referrerUrl);
+            this._getStateForPath(location.pathname)
+                .then((page) => {
+                    const newState = this.getState();
+                    // if the state has changed we update
+                    if (!stateMatches(newState, page)) {
+                        const newPathname = this.getPathname();
+                        const referrerUrl = location.href;
+                        window.history.pushState({}, '', newPathname);
+                        this._dispatchLocationChanged(referrerUrl);
+                    }
+                })
+                .catch((e) => {
+                    // In case we can't resolve the location, just leave things as is.
+                    // This happens when a user enters a wrong URL or when testing with karma.
+                });
         });
     }
 
@@ -93,14 +92,15 @@ export class Router {
      * @param {string} pathname
      */
     updateFromPathname(pathname) {
-        this.router
-            .resolve({pathname: pathname})
+        this._getStateForPath(pathname)
             .then((page) => {
-                if (location.pathname === pathname) return;
-                const referrerUrl = location.href;
-                window.history.pushState({}, '', pathname);
-                this.setState(page);
-                this.dispatchLocationChanged(referrerUrl);
+                const oldState = this.getState();
+                if (!stateMatches(oldState, page)) {
+                    const referrerUrl = location.href;
+                    window.history.pushState({}, '', pathname);
+                    this.setState(page);
+                    this._dispatchLocationChanged(referrerUrl);
+                }
             })
             .catch((err) => {
                 throw new Error(`Route not found: ${pathname}: ${err}`);
@@ -117,7 +117,9 @@ export class Router {
      */
     getPathname(partialState) {
         const currentState = this.getState();
-        if (partialState === undefined) partialState = {};
+        if (partialState === undefined) {
+            partialState = {};
+        }
         let combined = {...currentState, ...partialState};
 
         try {
@@ -128,7 +130,7 @@ export class Router {
         }
     }
 
-    dispatchLocationChanged(referrerUrl = '') {
+    _dispatchLocationChanged(referrerUrl = '') {
         // fire a locationchanged event
         window.dispatchEvent(
             new CustomEvent('locationchanged', {
-- 
GitLab