From 615a114f33fa3ef22108bbc165e21152ece3eed7 Mon Sep 17 00:00:00 2001
From: "bert.hausmans" <bert@hausmans.nl>
Date: Sat, 16 May 2026 21:07:57 +0200
Subject: [PATCH] fix(gui-v2): breadcrumb navigation via router.push + button
 type + void logout

- FIX A (IMPORTANT): PrimeVue Breadcrumb ignores `route` key; map non-last
  items with `command: () => router.push(item.to)` for real client-side nav
- FIX B: add type="button" to all 6 native <button> chrome elements
- FIX C: authStore.logout() bare call matches project no-void pattern
- FIX D: document param-route edge case in toBreadcrumbItems
- FIX E: regression test asserts command+push on non-last, no command on last,
  no `route` key on any item

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 .../src/components-v2/layout/AppTopbar.vue    | 38 ++++++++--
 .../layout/__tests__/AppTopbar.spec.ts        | 76 ++++++++++++++++++-
 apps/app/src/composables/useBreadcrumb.ts     |  6 +-
 3 files changed, 108 insertions(+), 12 deletions(-)

diff --git a/apps/app/src/components-v2/layout/AppTopbar.vue b/apps/app/src/components-v2/layout/AppTopbar.vue
index dc3a501a..4da4dc39 100644
--- a/apps/app/src/components-v2/layout/AppTopbar.vue
+++ b/apps/app/src/components-v2/layout/AppTopbar.vue
@@ -37,6 +37,7 @@ import Menu from 'primevue/menu'
 import OverlayBadge from 'primevue/overlaybadge'
 import Popover from 'primevue/popover'
 import { computed, ref } from 'vue'
+import { useRouter } from 'vue-router'
 import type { MenuItem } from 'primevue/menuitem'
 import Icon from '@/components/Icon.vue'
 import { useBreadcrumb } from '@/composables/useBreadcrumb'
@@ -45,11 +46,12 @@ import { useShellUiStore } from '@/stores/useShellUiStore'
 import { computeOrgGradient } from '@/utils/v2/gradient'
 
 // ---------------------------------------------------------------------------
-// Stores
+// Stores / router
 // ---------------------------------------------------------------------------
 
 const shell = useShellUiStore()
 const authStore = useAuthStore()
+const router = useRouter()
 
 // ---------------------------------------------------------------------------
 // Breadcrumb — route-driven via useBreadcrumb()
@@ -59,14 +61,27 @@ const { items: breadcrumbItems } = useBreadcrumb()
 
 /**
  * Map BreadcrumbItem[] → PrimeVue MenuItem[].
- * Non-last items get a `route` for router-link rendering.
- * Last item (current) has no route/command — plain label only.
+ *
+ * The installed PrimeVue Breadcrumb (BreadcrumbItem.vue) renders
+ * `<a :href="item.url || '#'">` and calls `item.command` on click.
+ * It does NOT honour a `route` key — router-link is never invoked.
+ *
+ * Fix: non-last items navigate via `command: () => router.push(item.to)` (client-side,
+ * no full reload, no href="#"). Last/current item has no `to` from useBreadcrumb()
+ * → no command → non-interactive.
  */
 const breadcrumbModel = computed<MenuItem[]>(() =>
-  breadcrumbItems.value.map(item => ({
-    label: item.label,
-    ...(item.to !== undefined ? { route: item.to } : {}),
-  })),
+  breadcrumbItems.value.map(item => {
+    const base: MenuItem = { label: item.label }
+
+    if (item.to !== undefined) {
+      base.command = () => {
+        router.push(item.to!)
+      }
+    }
+
+    return base
+  }),
 )
 
 // ---------------------------------------------------------------------------
@@ -214,7 +229,9 @@ const userMenuItems = computed<MenuItem[]>(() => [
       {
         label: 'Sign out',
         icon: 'tabler-logout',
-        command: () => authStore.logout(),
+        command: () => {
+          authStore.logout()
+        },
       },
     ],
   },
@@ -235,6 +252,7 @@ const userMenuItems = computed<MenuItem[]>(() => [
         .icon-btn: 38x38, rounded, transparent bg, fg-muted color, hover bg-hover
       -->
       <button
+        type="button"
         class="lg:hidden inline-flex h-[38px] w-[38px] items-center justify-center rounded-[var(--p-border-radius)] border-0 bg-transparent text-[var(--p-text-muted-color)] transition-colors duration-150 hover:bg-[var(--p-content-hover-background)] hover:text-[var(--p-text-color)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-[var(--p-primary-color)] focus-visible:outline-offset-1"
         aria-label="Open menu"
         @click="shell.setMobileOpen(true)"
@@ -295,6 +313,7 @@ const userMenuItems = computed<MenuItem[]>(() => [
       -->
       <button
         v-if="authStore.currentOrganisation"
+        type="button"
         class="ws-mobile-btn-shadow lg:hidden inline-flex h-[38px] w-[38px] flex-shrink-0 items-center justify-center rounded-[var(--p-border-radius)] border-0 text-[12px] font-bold text-white"
         :style="{
           background: `linear-gradient(135deg, ${mobileOrgGradient[0]}, ${mobileOrgGradient[1]})`,
@@ -332,6 +351,7 @@ const userMenuItems = computed<MenuItem[]>(() => [
         .icon-btn: 38x38, inline-flex, centered, rounded, transparent, fg-muted, hover bg-hover
       -->
       <button
+        type="button"
         class="inline-flex h-[38px] w-[38px] items-center justify-center rounded-[var(--p-border-radius)] border-0 bg-transparent text-[var(--p-text-muted-color)] transition-colors duration-150 hover:bg-[var(--p-content-hover-background)] hover:text-[var(--p-text-color)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-[var(--p-primary-color)] focus-visible:outline-offset-1"
         :aria-label="densityAriaLabel"
         @click="toggleDensity"
@@ -344,6 +364,7 @@ const userMenuItems = computed<MenuItem[]>(() => [
 
       <!-- Theme toggle. -->
       <button
+        type="button"
         class="inline-flex h-[38px] w-[38px] items-center justify-center rounded-[var(--p-border-radius)] border-0 bg-transparent text-[var(--p-text-muted-color)] transition-colors duration-150 hover:bg-[var(--p-content-hover-background)] hover:text-[var(--p-text-color)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-[var(--p-primary-color)] focus-visible:outline-offset-1"
         :aria-label="themeAriaLabel"
         @click="toggleTheme"
@@ -364,6 +385,7 @@ const userMenuItems = computed<MenuItem[]>(() => [
         severity="danger"
       >
         <button
+          type="button"
           class="inline-flex h-[38px] w-[38px] items-center justify-center rounded-[var(--p-border-radius)] border-0 bg-transparent text-[var(--p-text-muted-color)] transition-colors duration-150 hover:bg-[var(--p-content-hover-background)] hover:text-[var(--p-text-color)] focus-visible:outline focus-visible:outline-2 focus-visible:outline-[var(--p-primary-color)] focus-visible:outline-offset-1"
           aria-label="Notifications"
           @click="toggleNotifPopover"
diff --git a/apps/app/src/components-v2/layout/__tests__/AppTopbar.spec.ts b/apps/app/src/components-v2/layout/__tests__/AppTopbar.spec.ts
index eb183a2b..e4907549 100644
--- a/apps/app/src/components-v2/layout/__tests__/AppTopbar.spec.ts
+++ b/apps/app/src/components-v2/layout/__tests__/AppTopbar.spec.ts
@@ -9,9 +9,14 @@
  *  2. Theme toggle click → shell.setTheme with flipped value (light→dark, dark→light)
  *  3. Density toggle click → shell.setDensity with flipped value
  *  4. User-menu Sign out command → authStore.logout called
+ *  5. Breadcrumb model mapping:
+ *     - non-last items carry a `command` that calls router.push with the item's `to`
+ *     - last item has NO `command` (non-interactive) and NO `route` key (FIX A regression)
  *
  * useBreadcrumb() calls useRoute() internally. We provide a minimal
  * vue-router mock via vi.mock so the composable has a route to call.
+ * useRoute is exposed as a vi.fn() so individual tests can override the
+ * matched records without re-importing the module.
  */
 
 import { createPinia, setActivePinia } from 'pinia'
@@ -22,13 +27,21 @@ import { useAuthStore } from '@/stores/useAuthStore'
 
 // ---------------------------------------------------------------------------
 // Mock vue-router (useBreadcrumb calls useRoute internally)
+// vi.hoisted() is required so these vi.fn() instances are initialised
+// before the vi.mock() factory runs (vi.mock is hoisted to top of file by
+// the Vitest transform, but const declarations are not).
 // ---------------------------------------------------------------------------
 
+const { mockRouterPush, mockUseRoute } = vi.hoisted(() => ({
+  mockRouterPush: vi.fn(),
+  mockUseRoute: vi.fn(() => ({ matched: [] as unknown[] })),
+}))
+
 vi.mock('vue-router', () => ({
-  useRoute: () => ({
-    matched: [],
+  useRoute: mockUseRoute,
+  useRouter: () => ({
+    push: mockRouterPush,
   }),
-  useRouter: () => ({}),
 }))
 
 // ---------------------------------------------------------------------------
@@ -86,6 +99,11 @@ function mountTopbar() {
 describe('AppTopbar', () => {
   beforeEach(() => {
     setActivePinia(createPinia())
+    mockRouterPush.mockReset()
+    mockUseRoute.mockReset()
+
+    // Default: empty matched array (no breadcrumb items)
+    mockUseRoute.mockReturnValue({ matched: [] })
   })
 
   // -------------------------------------------------------------------------
@@ -221,4 +239,56 @@ describe('AppTopbar', () => {
 
     expect(logoutSpy).toHaveBeenCalledOnce()
   })
+
+  // -------------------------------------------------------------------------
+  // FIX A regression: breadcrumb model mapping uses command+router.push,
+  // NOT the `route` key that this PrimeVue Breadcrumb version ignores.
+  //
+  // BreadcrumbItem.vue renders <a :href="item.url || '#'"> and calls
+  // item.command on click — it never reads `route`. This test would have
+  // caught the broken mapping that set `route` instead of `command`.
+  // -------------------------------------------------------------------------
+
+  it('breadcrumb model: non-last items have command that calls router.push; last item has no command and no route key', async () => {
+    // Provide two matched records so we get a non-last item (with `to`) and
+    // a last item (current, no `to`).
+    mockUseRoute.mockReturnValue({
+      matched: [
+        { meta: { breadcrumb: 'Dashboard' }, name: 'dashboard', path: '/dashboard' },
+        { meta: { breadcrumb: 'Events' }, name: 'events', path: '/events' },
+      ],
+    })
+
+    const wrapper = mountTopbar()
+
+    const breadcrumbStub = wrapper.findComponent({ name: 'Breadcrumb' })
+
+    expect(breadcrumbStub.exists()).toBe(true)
+
+    const model = breadcrumbStub.props('model') as Array<{
+      label: string
+      command?: () => void
+      route?: unknown
+    }>
+
+    // Should have two items produced by toBreadcrumbItems
+    expect(model).toHaveLength(2)
+
+    const [firstItem, lastItem] = model as [typeof model[0], typeof model[0]]
+
+    // Non-last item: must carry `command`, must NOT carry `route`
+    expect(firstItem.label).toBe('Dashboard')
+    expect(typeof firstItem.command).toBe('function')
+    expect('route' in firstItem).toBe(false)
+
+    // Invoking command must call router.push with the item's resolved `to` path
+    firstItem.command!()
+    expect(mockRouterPush).toHaveBeenCalledOnce()
+    expect(mockRouterPush).toHaveBeenCalledWith('/dashboard')
+
+    // Last/current item: must have NO command (non-interactive), NO `route` key
+    expect(lastItem.label).toBe('Events')
+    expect(lastItem.command).toBeUndefined()
+    expect('route' in lastItem).toBe(false)
+  })
 })
diff --git a/apps/app/src/composables/useBreadcrumb.ts b/apps/app/src/composables/useBreadcrumb.ts
index f2b31e95..94a90b64 100644
--- a/apps/app/src/composables/useBreadcrumb.ts
+++ b/apps/app/src/composables/useBreadcrumb.ts
@@ -55,7 +55,11 @@ export function toBreadcrumbItems(matched: readonly BreadcrumbRouteRecord[]): Br
     if (isLast)
       return { label }
 
-    // Prefer path; fall back to name when path is absent/empty
+    // Prefer path; fall back to name when path is absent/empty.
+    // NOTE: a non-last record whose `path` is a param template (e.g. `/events/:id`)
+    // would yield an unresolved-template `to` — acceptable in foundation scope because
+    // param routes are normally the last (current, no-`to`) segment.
+    // TODO TECH-WS-GUI-REDESIGN: resolve param paths if a non-leaf param route ever needs a crumb link
     const to: RouteLocationRaw
       = record.path
         ? record.path