From e59b710108c0050afbacea5471a77f7541183798 Mon Sep 17 00:00:00 2001
From: Jannik Stehle <50302941+JammingBen@users.noreply.github.com>
Date: Mon, 15 Jan 2024 11:25:34 +0100
Subject: [PATCH] refactor: register nav items via extension registry (#10329)

* refactor: register nav items via extension registry

Removes the nav items from the vuex navigation store module. It now uses the extension registry with the already existing `SidebarNavExtension` type for registering and retrieving  nav items.

* refactor: use app.{appId} as nav extension scope

* fix: route types
---
 .../src/components/Topbar/TopBar.vue          |  9 ++--
 packages/web-runtime/src/container/api.ts     | 25 +++++----
 .../src/container/application/classic.ts      |  7 ++-
 .../web-runtime/src/container/bootstrap.ts    | 27 +++++++---
 packages/web-runtime/src/helpers/navItems.ts  | 14 ++++-
 packages/web-runtime/src/index.ts             |  5 +-
 .../web-runtime/src/layouts/Application.vue   | 25 ++-------
 packages/web-runtime/src/store/index.ts       |  2 -
 packages/web-runtime/src/store/navigation.ts  | 54 -------------------
 .../mocks/store/defaultStoreMockOptions.ts    |  1 -
 10 files changed, 67 insertions(+), 102 deletions(-)
 delete mode 100644 packages/web-runtime/src/store/navigation.ts

diff --git a/packages/web-runtime/src/components/Topbar/TopBar.vue b/packages/web-runtime/src/components/Topbar/TopBar.vue
index 5be9f2cd128..574910ba672 100644
--- a/packages/web-runtime/src/components/Topbar/TopBar.vue
+++ b/packages/web-runtime/src/components/Topbar/TopBar.vue
@@ -56,11 +56,13 @@ import {
   useAuthStore,
   useCapabilityStore,
   useEmbedMode,
+  useExtensionRegistry,
   useRouter,
   useStore,
   useThemeStore
 } from '@ownclouders/web-pkg'
 import { isRuntimeRoute } from '../../router'
+import { getExtensionNavItems } from '../../helpers/navItems'
 
 export default {
   components: {
@@ -81,6 +83,7 @@ export default {
   setup(props) {
     const store = useStore()
     const capabilityStore = useCapabilityStore()
+    const extensionRegistry = useExtensionRegistry()
     const themeStore = useThemeStore()
     const { currentTheme } = storeToRefs(themeStore)
 
@@ -127,9 +130,9 @@ export default {
           if (app.type === 'extension') {
             // check if the extension has at least one navItem with a matching menuId
             return (
-              store.getters
-                .getNavItemsByExtension(app.id)
-                .filter((navItem) => isNavItemPermitted(permittedMenus, navItem)).length > 0 ||
+              getExtensionNavItems({ extensionRegistry, appId: app.id }).filter((navItem) =>
+                isNavItemPermitted(permittedMenus, navItem)
+              ).length > 0 ||
               (app.applicationMenu.enabled instanceof Function &&
                 app.applicationMenu.enabled(store, ability) &&
                 !permittedMenus.includes('user'))
diff --git a/packages/web-runtime/src/container/api.ts b/packages/web-runtime/src/container/api.ts
index 728c4a76516..9add81a3fb8 100644
--- a/packages/web-runtime/src/container/api.ts
+++ b/packages/web-runtime/src/container/api.ts
@@ -1,10 +1,10 @@
 import { RouteRecordRaw, Router } from 'vue-router'
 import clone from 'lodash-es/clone'
 import { RuntimeApi } from './types'
-import { ApiError } from '@ownclouders/web-pkg'
+import { ApiError, ExtensionRegistry, SidebarNavExtension } from '@ownclouders/web-pkg'
 import { get, isEqual, isObject, isArray, merge } from 'lodash-es'
 import { Module, Store } from 'vuex'
-import { App, Component, h } from 'vue'
+import { App, Component, computed, h } from 'vue'
 import { ApplicationTranslations, AppNavigationItem } from '@ownclouders/web-pkg'
 import type { Language } from 'vue3-gettext'
 
@@ -61,22 +61,25 @@ const announceRoutes = (applicationId: string, router: Router, routes: RouteReco
  * inject application specific navigation items into runtime
  *
  * @param applicationId
- * @param store
+ * @param extensionRegistry
  * @param navigationItems
  */
 const announceNavigationItems = (
   applicationId: string,
-  store: Store<unknown>,
+  extensionRegistry: ExtensionRegistry,
   navigationItems: AppNavigationItem[]
 ): void => {
   if (!isObject(navigationItems)) {
     throw new ApiError("navigationItems can't be blank")
   }
 
-  store.commit('SET_NAV_ITEMS_FROM_CONFIG', {
-    extension: applicationId,
-    navItems: navigationItems
-  })
+  const navExtensions = navigationItems.map((navItem) => ({
+    type: 'sidebarNav',
+    navItem,
+    scopes: [`app.${applicationId}`]
+  })) as SidebarNavExtension[]
+
+  extensionRegistry.registerExtensions(computed(() => navExtensions))
 }
 
 /**
@@ -213,7 +216,8 @@ export const buildRuntimeApi = ({
   store,
   router,
   gettext,
-  supportedLanguages
+  supportedLanguages,
+  extensionRegistry
 }: {
   applicationName: string
   applicationId: string
@@ -221,6 +225,7 @@ export const buildRuntimeApi = ({
   gettext: Language
   router: Router
   supportedLanguages: { [key: string]: string }
+  extensionRegistry: ExtensionRegistry
 }): RuntimeApi => {
   if (!applicationName) {
     throw new ApiError("applicationName can't be blank")
@@ -234,7 +239,7 @@ export const buildRuntimeApi = ({
     announceRoutes: (routes: RouteRecordRaw[]): void =>
       announceRoutes(applicationId, router, routes),
     announceNavigationItems: (navigationItems: AppNavigationItem[]): void =>
-      announceNavigationItems(applicationId, store, navigationItems),
+      announceNavigationItems(applicationId, extensionRegistry, navigationItems),
     announceTranslations: (appTranslations: ApplicationTranslations): void =>
       announceTranslations(supportedLanguages, gettext, appTranslations),
     announceStore: (applicationStore: Module<unknown, unknown>): void =>
diff --git a/packages/web-runtime/src/container/application/classic.ts b/packages/web-runtime/src/container/application/classic.ts
index d6e4e656998..9fddfd83813 100644
--- a/packages/web-runtime/src/container/application/classic.ts
+++ b/packages/web-runtime/src/container/application/classic.ts
@@ -119,19 +119,22 @@ export const convertClassicApplication = async ({
     throw new RuntimeError("appInfo.name can't be blank")
   }
 
+  const extensionRegistry = useExtensionRegistry({ configurationManager })
+
   const runtimeApi = buildRuntimeApi({
     applicationName,
     applicationId,
     store,
     router,
     gettext,
-    supportedLanguages
+    supportedLanguages,
+    extensionRegistry
   })
 
   await store.dispatch('registerApp', applicationScript.appInfo)
 
   if (applicationScript.extensions) {
-    useExtensionRegistry({ configurationManager }).registerExtensions(applicationScript.extensions)
+    extensionRegistry.registerExtensions(applicationScript.extensions)
   }
 
   return new ClassicApplication(runtimeApi, applicationScript, app)
diff --git a/packages/web-runtime/src/container/bootstrap.ts b/packages/web-runtime/src/container/bootstrap.ts
index 2139acfc58f..0cd6991e745 100644
--- a/packages/web-runtime/src/container/bootstrap.ts
+++ b/packages/web-runtime/src/container/bootstrap.ts
@@ -2,7 +2,7 @@ import { registerClient } from '../services/clientRegistration'
 import { RuntimeConfiguration } from './types'
 import { buildApplication, NextApplication } from './application'
 import { Store } from 'vuex'
-import { Router } from 'vue-router'
+import { RouteLocationRaw, Router, RouteRecordNormalized } from 'vue-router'
 import { App, computed } from 'vue'
 import { loadTheme } from '../helpers/theme'
 import OwnCloud from 'owncloud-sdk'
@@ -18,7 +18,9 @@ import {
   useAuthStore,
   AuthStore,
   useCapabilityStore,
-  CapabilityStore
+  CapabilityStore,
+  useExtensionRegistry,
+  ExtensionRegistry
 } from '@ownclouders/web-pkg'
 import { authService } from '../services/auth'
 import {
@@ -46,6 +48,7 @@ import { Resource } from '@ownclouders/web-client'
 import PQueue from 'p-queue'
 import { extractNodeId, extractStorageId } from '@ownclouders/web-client/src/helpers'
 import { storeToRefs } from 'pinia'
+import { getExtensionNavItems } from '../helpers/navItems'
 
 const getEmbedConfigFromQuery = (
   doesEmbedEnabledOptionExists: boolean
@@ -344,11 +347,21 @@ export const announceTheme = async ({
 export const announcePiniaStores = () => {
   const authStore = useAuthStore()
   const capabilityStore = useCapabilityStore()
+  const extensionRegistry = useExtensionRegistry({ configurationManager })
   const messagesStore = useMessages()
   const modalStore = useModals()
   const spacesStore = useSpacesStore()
   const userStore = useUserStore()
-  return { authStore, capabilityStore, messagesStore, modalStore, spacesStore, userStore }
+
+  return {
+    authStore,
+    capabilityStore,
+    extensionRegistry,
+    messagesStore,
+    modalStore,
+    spacesStore,
+    userStore
+  }
 }
 
 /**
@@ -539,10 +552,12 @@ export const announcePasswordPolicyService = ({ app }: { app: App }): void => {
  */
 export const announceDefaults = ({
   store,
-  router
+  router,
+  extensionRegistry
 }: {
   store: Store<unknown>
   router: Router
+  extensionRegistry: ExtensionRegistry
 }): void => {
   // set home route
   const appIds = store.getters.appIds
@@ -551,11 +566,11 @@ export const announceDefaults = ({
     defaultExtensionId = appIds[0]
   }
 
-  let route = router.getRoutes().find((r) => {
+  let route: RouteRecordNormalized | RouteLocationRaw = router.getRoutes().find((r) => {
     return r.path.startsWith(`/${defaultExtensionId}`) && r.meta?.entryPoint === true
   })
   if (!route) {
-    route = store.getters.getNavItemsByExtension(defaultExtensionId)[0]?.route
+    route = getExtensionNavItems({ extensionRegistry, appId: defaultExtensionId })[0]?.route
   }
   if (route) {
     router.addRoute({
diff --git a/packages/web-runtime/src/helpers/navItems.ts b/packages/web-runtime/src/helpers/navItems.ts
index eee66c48631..819fc4879bf 100644
--- a/packages/web-runtime/src/helpers/navItems.ts
+++ b/packages/web-runtime/src/helpers/navItems.ts
@@ -1,6 +1,18 @@
-import { AppNavigationItem } from '@ownclouders/web-pkg'
+import { AppNavigationItem, ExtensionRegistry, SidebarNavExtension } from '@ownclouders/web-pkg'
 
 export interface NavItem extends Omit<AppNavigationItem, 'name'> {
   name: string
   active: boolean
 }
+
+export const getExtensionNavItems = ({
+  extensionRegistry,
+  appId
+}: {
+  extensionRegistry: ExtensionRegistry
+  appId: string
+}) =>
+  extensionRegistry
+    .requestExtensions<SidebarNavExtension>('sidebarNav', [`app.${appId}`])
+    .map(({ navItem }) => navItem)
+    .filter((n) => n.enabled())
diff --git a/packages/web-runtime/src/index.ts b/packages/web-runtime/src/index.ts
index 3d63a74fc81..89c0754d92e 100644
--- a/packages/web-runtime/src/index.ts
+++ b/packages/web-runtime/src/index.ts
@@ -50,7 +50,8 @@ export const bootstrapApp = async (configurationPath: string): Promise<void> =>
   const app = createApp(pages.success)
   app.use(pinia)
 
-  const { authStore, capabilityStore, spacesStore, userStore } = announcePiniaStores()
+  const { authStore, capabilityStore, extensionRegistry, spacesStore, userStore } =
+    announcePiniaStores()
 
   app.provide('$router', router)
 
@@ -145,7 +146,7 @@ export const bootstrapApp = async (configurationPath: string): Promise<void> =>
   })
   announceCustomStyles({ runtimeConfiguration })
   announceCustomScripts({ runtimeConfiguration })
-  announceDefaults({ store, router })
+  announceDefaults({ store, router, extensionRegistry })
 
   app.use(router)
   app.use(store)
diff --git a/packages/web-runtime/src/layouts/Application.vue b/packages/web-runtime/src/layouts/Application.vue
index 01bb6627219..8c2f264d028 100644
--- a/packages/web-runtime/src/layouts/Application.vue
+++ b/packages/web-runtime/src/layouts/Application.vue
@@ -42,18 +42,13 @@
 <script lang="ts">
 import { mapGetters } from 'vuex'
 import orderBy from 'lodash-es/orderBy'
-import {
-  AppLoadingSpinner,
-  SidebarNavExtension,
-  useAuthStore,
-  useExtensionRegistry
-} from '@ownclouders/web-pkg'
+import { AppLoadingSpinner, useAuthStore, useExtensionRegistry } from '@ownclouders/web-pkg'
 import TopBar from '../components/Topbar/TopBar.vue'
 import MessageBar from '../components/MessageBar.vue'
 import SidebarNav from '../components/SidebarNav/SidebarNav.vue'
 import UploadInfo from '../components/UploadInfo.vue'
 import MobileNav from '../components/MobileNav.vue'
-import { NavItem } from '../helpers/navItems'
+import { NavItem, getExtensionNavItems } from '../helpers/navItems'
 import { LoadingIndicator } from '@ownclouders/web-pkg'
 import {
   useActiveApp,
@@ -67,7 +62,6 @@ import { useRouter } from 'vue-router'
 import { useGettext } from 'vue3-gettext'
 
 import '@uppy/core/dist/style.min.css'
-import { AppNavigationItem } from '@ownclouders/web-pkg'
 
 const MOBILE_BREAKPOINT = 640
 
@@ -92,13 +86,7 @@ export default defineComponent({
     const extensionRegistry = useExtensionRegistry()
 
     const extensionNavItems = computed(() =>
-      extensionRegistry
-        .requestExtensions<SidebarNavExtension>('sidebarNav', [
-          unref(activeApp),
-          `app.${unref(activeApp)}`
-        ])
-        .map(({ navItem }) => navItem)
-        .filter((n) => n.enabled())
+      getExtensionNavItems({ extensionRegistry, appId: unref(activeApp) })
     )
 
     // FIXME: we can convert to a single router-view without name (thus without the loop) and without this watcher when we release v6.0.0
@@ -141,14 +129,9 @@ export default defineComponent({
         return []
       }
 
-      const items = [
-        ...store.getters['getNavItemsByExtension'](unref(activeApp)),
-        ...unref(extensionNavItems)
-      ] as AppNavigationItem[]
-
       const { href: currentHref } = router.resolve(unref(route))
       return orderBy(
-        items.map((item) => {
+        unref(extensionNavItems).map((item) => {
           let active = typeof item.isActive !== 'function' || item.isActive()
 
           if (active) {
diff --git a/packages/web-runtime/src/store/index.ts b/packages/web-runtime/src/store/index.ts
index fdf04faab03..525174df559 100644
--- a/packages/web-runtime/src/store/index.ts
+++ b/packages/web-runtime/src/store/index.ts
@@ -1,7 +1,6 @@
 import ancestorMetaData from './ancestorMetaData'
 import apps from './apps'
 import config from './config'
-import navigation from './navigation'
 
 const runtime = {
   namespaced: true,
@@ -14,7 +13,6 @@ export default {
   modules: {
     apps,
     config,
-    navigation,
     runtime
   }
 }
diff --git a/packages/web-runtime/src/store/navigation.ts b/packages/web-runtime/src/store/navigation.ts
deleted file mode 100644
index 7c712651d18..00000000000
--- a/packages/web-runtime/src/store/navigation.ts
+++ /dev/null
@@ -1,54 +0,0 @@
-const state = {
-  // static nav items are set during extension loading and will not be modified later on
-  staticNavItems: {}
-}
-
-const mutations = {
-  /**
-   * Sets the nav items that are statically provided through extension configuration,
-   * i.e. the appInfo json.
-   *
-   * @param state
-   * @param extension
-   * @param navItems
-   * @constructor
-   */
-  SET_NAV_ITEMS_FROM_CONFIG(state, { extension, navItems }) {
-    const staticNavItems = state.staticNavItems
-    staticNavItems[extension] = navItems
-    state.staticNavItems = staticNavItems
-  }
-}
-
-const getters = {
-  /**
-   * Get all nav items that are associated with the given extension id.
-   *
-   * @param state
-   * @param getters
-   * @returns {function(*): *[]}
-   */
-  getNavItemsByExtension: (state) => (extension) => {
-    const staticNavItems = state.staticNavItems[extension] || []
-    return staticNavItems.filter((navItem) => {
-      if (!navItem.enabled) {
-        // when `enabled` callback not provided: count as enabled.
-        return true
-      }
-      try {
-        return navItem.enabled()
-      } catch (e) {
-        console.error('`enabled` callback on navItem ' + navItem.name + ' threw an error', e)
-        return false
-      }
-    })
-  }
-}
-
-const actions = {}
-export default {
-  state,
-  mutations,
-  getters,
-  actions
-}
diff --git a/packages/web-test-helpers/src/mocks/store/defaultStoreMockOptions.ts b/packages/web-test-helpers/src/mocks/store/defaultStoreMockOptions.ts
index 67de2d683cc..e1beba1c80f 100644
--- a/packages/web-test-helpers/src/mocks/store/defaultStoreMockOptions.ts
+++ b/packages/web-test-helpers/src/mocks/store/defaultStoreMockOptions.ts
@@ -5,7 +5,6 @@ export const defaultStoreMockOptions = {
   commit: jest.fn(),
   getters: {
     newFileHandlers: jest.fn(() => []),
-    getNavItemsByExtension: jest.fn(),
     apps: jest.fn(() => ({})),
     configuration: jest.fn().mockImplementation(() => ({
       options: {