fix(timetable): make ?day query the source of truth with validation and fallback

Per Phase A finding A6 — the previous three-watcher Pinia-store design had
no validation. Landing on /events/{e}/timetable?day=DOES_NOT_EXIST quietly
set store.activeDayId to that bogus value and showed an empty page.
Cross-org sub-event IDs were silently accepted (backend OrganisationScope
returned an empty perf list, so the UI looked broken without telling the
user).

New design (Session 4 follow-up Step 5):

- src/composables/timetable/useActiveDay.ts (NEW)
  - The URL `?day` is the source of truth; Pinia does NOT hold this value.
  - `activeDayId` is a computed: queryDay if it appears in `validIds`,
    else the first valid id, else null when the list is empty.
  - One corrective watcher (immediate:true, flush:'post') quietly rewrites
    the URL when `?day` is missing or invalid; runs after Vue settles and
    after validIds has been recomputed from a fresh fetch.
  - `setActiveDay(id)` is the user-driven entry point — calls replace().
  - Cross-org IDs are blocked transparently: OrganisationScope keeps them
    out of validIds, so they fail the .includes() check and fall back.

- src/stores/useTimetableStore.ts
  - Removed `activeDayId` state and `setActiveDay()` action; the store
    docstring now documents that day-state lives at the URL.

- src/pages/events/[id]/timetable/index.vue
  - Replaced the three watchers + onMounted bootstrap with one
    `useActiveDay({ queryDay, validIds, replace })` call. The day-change
    side-effect watcher (clear drag, deselect performance) stays.
  - VTabs binds dayIdRef + setActiveDay directly.

- tests/unit/pages/timetableDaySync.test.ts (NEW, 9 tests)
  - Valid ?day=X → activeDayId=X, no URL rewrite.
  - Missing / invalid / cross-org ?day → fallback + URL replaced once.
  - Empty validIds → activeDayId=null, URL untouched.
  - setActiveDay(id) → calls replace.
  - setActiveDay(null) → no-op.
  - External URL change (browser back) → activeDayId follows.
  - validIds populated AFTER mount → fallback fires correctly.

- tests/unit/stores/useTimetableStore.test.ts: assert that activeDayId
  and setActiveDay are GONE from the store surface.

Test count: 324 → 333.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-09 03:37:31 +02:00
parent 8d1cb39172
commit e99acbde95
5 changed files with 274 additions and 31 deletions

View File

@@ -0,0 +1,71 @@
import { computed, watch } from 'vue'
import type { Ref } from 'vue'
/**
* `?day` query param ↔ active sub-event id binding for the timetable page.
*
* The URL is the source of truth. The store does NOT hold this value.
*
* Behaviour (RFC v0.2 §6.2 / Session 4 follow-up Step 5):
* - If `?day=X` is in `validIds` → activeDayId = X.
* - If `?day=X` is missing or invalid → activeDayId = first valid id,
* and the URL is silently rewritten
* via `replace({day: firstValidId})`.
* - If `validIds` is empty (event has → activeDayId = null.
* no sub-events / data still loading)
*
* Cross-org sub-event IDs are blocked transparently: `OrganisationScope`
* on the backend never returns them, so they fail `validIds.includes(...)`
* and fall back to the first valid id from the user's own organisation.
*/
export interface UseActiveDayDeps {
/** Reactive read of `route.query.day`. Must coerce array → string outside. */
queryDay: Ref<string | null>
/** Reactive list of sub-event IDs the user is allowed to see for this event. */
validIds: Ref<string[]>
/** `router.replace` wrapper that updates ONLY the `day` query param. */
replace: (dayId: string) => void
}
export interface UseActiveDayReturn {
activeDayId: Ref<string | null>
setActiveDay: (id: string | null) => void
}
export function useActiveDay(deps: UseActiveDayDeps): UseActiveDayReturn {
const activeDayId = computed<string | null>(() => {
const ids = deps.validIds.value
if (ids.length === 0)
return null
const q = deps.queryDay.value
if (q && ids.includes(q))
return q
return ids[0]
})
// Single corrective watcher — quietly rewrites the URL when the query param
// is missing or invalid. immediate:true so a mount with `?day=null` (or
// an invalid value) corrects the URL on first paint instead of waiting
// for the next user interaction. flush:'post' so it runs after Vue settles
// and after validIds has been recomputed from a fresh fetch.
watch([() => deps.queryDay.value, () => deps.validIds.value], ([q, ids]) => {
if (ids.length === 0)
return
if (q === null || !ids.includes(q))
deps.replace(ids[0])
}, { flush: 'post', immediate: true })
function setActiveDay(id: string | null): void {
if (id === null)
return
if (id === deps.queryDay.value)
return
deps.replace(id)
}
return { activeDayId, setActiveDay }
}

View File

@@ -1,5 +1,5 @@
<script setup lang="ts">
import { computed, onMounted, ref, watch } from 'vue'
import { computed, ref, watch } from 'vue'
import EventTabsNav from '@/components/events/EventTabsNav.vue'
import AddPerformanceDialog from '@/components/timetable/AddPerformanceDialog.vue'
import EmptyDayState from '@/components/timetable/EmptyDayState.vue'
@@ -14,6 +14,7 @@ import Wachtrij from '@/components/timetable/Wachtrij.vue'
import { useEventChildren, useEventDetail } from '@/composables/api/useEvents'
import { useTimetable } from '@/composables/api/useTimetable'
import { useTimetableMutations } from '@/composables/api/useTimetableMutations'
import { useActiveDay } from '@/composables/timetable/useActiveDay'
import { useDragOrClick } from '@/composables/timetable/useDragOrClick'
import { useTimetableKeyboard } from '@/composables/timetable/useTimetableKeyboard'
import { generateIdempotencyKey } from '@/lib/idempotencyKey'
@@ -59,22 +60,31 @@ const dayOptions = computed(() => {
return (subEvents.value ?? []).map(c => ({ id: c.id, name: c.name, start_date: null }))
})
const dayIdRef = computed<string | null>(() => store.activeDayId)
// `?day` is the source of truth — Pinia derives from it, never owns it.
// Behaviour fully delegated to useActiveDay (RFC §6.2 / Session 4 follow-up
// Step 5). Cross-org IDs are blocked transparently: OrganisationScope never
// returns them, so they fail validIds.includes(…) and fall back.
watch(() => route.query.day, raw => {
const value = typeof raw === 'string' ? raw : null
if (value && value !== store.activeDayId)
store.setActiveDay(value)
}, { immediate: true })
const validSubEventIds = computed(() => dayOptions.value.map(o => o.id))
watch(dayOptions, opts => {
if (!store.activeDayId && opts.length > 0)
store.setActiveDay(opts[0].id)
const queryDay = computed<string | null>(() => {
const v = route.query.day
return typeof v === 'string' && v.length > 0 ? v : null
})
watch(() => store.activeDayId, id => {
if (id && id !== route.query.day)
router.replace({ query: { ...route.query, day: id } })
const { activeDayId: dayIdRef, setActiveDay } = useActiveDay({
queryDay,
validIds: validSubEventIds,
replace: id => {
void router.replace({ query: { ...route.query, day: id } })
},
})
// Side effects on day-change: clear drag snapshot + deselect.
watch(dayIdRef, () => {
store.endDrag()
store.selectPerformance(null)
})
const tt = useTimetable(orgId, eventId, dayIdRef)
@@ -377,11 +387,8 @@ const { announce } = useTimetableKeyboard({
},
})
onMounted(() => {
// Fire watch immediately to bind ?day → store.
if (typeof route.query.day === 'string')
store.setActiveDay(route.query.day)
})
// `?day` source-of-truth is wired via the dayIdRef computed + the
// corrective watcher above; no onMounted bootstrap needed.
</script>
<template>
@@ -390,9 +397,9 @@ onMounted(() => {
<div class="tt-page">
<div class="tt-page__toolbar">
<VTabs
:model-value="store.activeDayId"
:model-value="dayIdRef"
density="compact"
@update:model-value="v => store.setActiveDay(typeof v === 'string' ? v : null)"
@update:model-value="v => setActiveDay(typeof v === 'string' ? v : null)"
>
<VTab
v-for="d in dayOptions"

View File

@@ -15,14 +15,17 @@ import type {
* cache via useTimetable.ts — this store carries only the bits that
* multiple components on the canvas need to share:
*
* - Active sub-event id (synced ↔ ?day query)
* - Selected performance id (drives popover anchor + keyboard focus)
* - In-flight drag state + origin snapshot for rollback (RFC D7)
* - Status filter (chips above wachtrij + canvas dimming)
* - Free-text search (wachtrij filter)
*
* Active sub-event ("day") is intentionally NOT held here — the URL
* `?day` query param is the source of truth, derived in the page entry
* via a computed against the validated sub-event list. Keeping it out
* of Pinia avoids the desync class of bug between URL and store.
*/
export const useTimetableStore = defineStore('timetable', () => {
const activeDayId = ref<string | null>(null)
const selectedPerformanceId = ref<string | null>(null)
// Drag state — set by usePointerDrag handlers, consumed by mutation
@@ -51,10 +54,6 @@ export const useTimetableStore = defineStore('timetable', () => {
const searchQuery = ref('')
function setActiveDay(id: string | null): void {
activeDayId.value = id
}
function selectPerformance(id: string | null): void {
selectedPerformanceId.value = id
}
@@ -97,7 +96,6 @@ export const useTimetableStore = defineStore('timetable', () => {
const isDragging = computed(() => dragPerformanceId.value !== null)
return {
activeDayId,
selectedPerformanceId,
dragPerformanceId,
dragOriginSnapshot,
@@ -105,7 +103,6 @@ export const useTimetableStore = defineStore('timetable', () => {
statusFilter,
searchQuery,
isDragging,
setActiveDay,
selectPerformance,
startDrag,
updateDragGhost,

View File

@@ -0,0 +1,165 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { defineComponent, h, nextTick, ref } from 'vue'
import { mount } from '@vue/test-utils'
import { useActiveDay } from '@/composables/timetable/useActiveDay'
/**
* The `?day` source-of-truth contract (Session 4 follow-up Step 5).
*
* useActiveDay is mounted inside a host component so the watcher actually
* runs (composables that use watch() need an active component instance).
*/
interface HostExposed {
activeDayId: () => string | null
setActiveDay: (id: string | null) => void
setQueryDay: (id: string | null) => void
setValidIds: (ids: string[]) => void
replaceMock: ReturnType<typeof vi.fn>
}
function mountSync(initialQuery: string | null, initialValidIds: string[]) {
const queryDay = ref<string | null>(initialQuery)
const validIds = ref<string[]>(initialValidIds)
const replaceMock = vi.fn((id: string) => {
queryDay.value = id
})
const host = defineComponent({
setup(_, { expose }) {
const { activeDayId, setActiveDay } = useActiveDay({
queryDay,
validIds,
replace: replaceMock,
})
expose({
activeDayId: () => activeDayId.value,
setActiveDay,
setQueryDay: (v: string | null) => { queryDay.value = v },
setValidIds: (v: string[]) => { validIds.value = v },
replaceMock,
})
return () => h('div')
},
})
const wrapper = mount(host)
return { wrapper, vm: wrapper.vm as unknown as HostExposed, replaceMock }
}
describe('useActiveDay — ?day source-of-truth', () => {
beforeEach(() => {
vi.clearAllMocks()
})
afterEach(() => {
vi.useRealTimers()
})
it('valid ?day=X returns X without rewriting the URL', async () => {
const { vm, replaceMock } = mountSync('day_2', ['day_1', 'day_2', 'day_3'])
await nextTick()
expect(vm.activeDayId()).toBe('day_2')
expect(replaceMock).not.toHaveBeenCalled()
})
it('missing ?day → fallback to first valid id + URL replaced once', async () => {
const { vm, replaceMock } = mountSync(null, ['day_1', 'day_2', 'day_3'])
await nextTick()
await nextTick()
expect(vm.activeDayId()).toBe('day_1')
expect(replaceMock).toHaveBeenCalledTimes(1)
expect(replaceMock).toHaveBeenCalledWith('day_1')
})
it('invalid ?day=DOES_NOT_EXIST → fallback + URL corrected', async () => {
const { vm, replaceMock } = mountSync('day_bogus', ['day_1', 'day_2'])
await nextTick()
await nextTick()
expect(vm.activeDayId()).toBe('day_1')
expect(replaceMock).toHaveBeenCalledWith('day_1')
})
it('cross-org ?day (id absent from validIds) → fallback to first valid', async () => {
// Backend OrganisationScope keeps the cross-org sub-event out of the
// returned list; useActiveDay treats it identically to "doesn't exist".
const { vm, replaceMock } = mountSync('day_other_org', ['day_a'])
await nextTick()
await nextTick()
expect(vm.activeDayId()).toBe('day_a')
expect(replaceMock).toHaveBeenCalledWith('day_a')
})
it('empty validIds → activeDayId is null and URL is not touched', async () => {
const { vm, replaceMock } = mountSync('day_1', [])
await nextTick()
await nextTick()
expect(vm.activeDayId()).toBeNull()
expect(replaceMock).not.toHaveBeenCalled()
})
it('setActiveDay(id) calls replace with the new id', async () => {
const { vm, replaceMock } = mountSync('day_1', ['day_1', 'day_2'])
await nextTick()
vm.setActiveDay('day_2')
await nextTick()
expect(replaceMock).toHaveBeenLastCalledWith('day_2')
expect(vm.activeDayId()).toBe('day_2')
})
it('setActiveDay(null) is a no-op', async () => {
const { vm, replaceMock } = mountSync('day_1', ['day_1', 'day_2'])
await nextTick()
replaceMock.mockClear()
vm.setActiveDay(null)
await nextTick()
expect(replaceMock).not.toHaveBeenCalled()
})
it('external URL change (browser back) propagates to activeDayId', async () => {
const { vm } = mountSync('day_1', ['day_1', 'day_2'])
await nextTick()
expect(vm.activeDayId()).toBe('day_1')
vm.setQueryDay('day_2')
await nextTick()
expect(vm.activeDayId()).toBe('day_2')
})
it('validIds populated AFTER mount triggers fallback if ?day was missing', async () => {
const { vm, replaceMock } = mountSync(null, [])
await nextTick()
expect(vm.activeDayId()).toBeNull()
expect(replaceMock).not.toHaveBeenCalled()
vm.setValidIds(['day_1', 'day_2'])
await nextTick()
await nextTick()
expect(vm.activeDayId()).toBe('day_1')
expect(replaceMock).toHaveBeenCalledWith('day_1')
})
})

View File

@@ -41,11 +41,14 @@ describe('useTimetableStore', () => {
expect(store.isStatusVisible(ArtistEngagementStatus.CONFIRMED)).toBe(true)
})
it('setActiveDay updates and selectPerformance maps to id', () => {
it('selectPerformance maps to id and clears on null', () => {
// activeDayId / setActiveDay intentionally REMOVED from the store
// (Step 5: ?day URL is the source of truth, page derives via computed).
const store = useTimetableStore()
store.setActiveDay('day_1')
expect(store.activeDayId).toBe('day_1')
expect((store as unknown as { activeDayId?: string }).activeDayId).toBeUndefined()
expect((store as unknown as { setActiveDay?: unknown }).setActiveDay).toBeUndefined()
store.selectPerformance('p1')
expect(store.selectedPerformanceId).toBe('p1')
store.selectPerformance(null)