feat(booking-templates): per-company opt-in hiding of system templates (#2004)

* feat(booking-templates): per-company opt-in hiding of system templates Users cannot delete or hide the 26 standard konteringspaket, which clutter the settings panel and every template picker. Deletion stays off the table (shared global rows); instead a company can now hide individual system templates for itself only. - New booking_template_hidden table (insert=hide, delete=unhide), RLS gated on active company + write role; nothing hidden by default - POST/DELETE /api/settings/booking-templates/[id]/hide (system templates only; company/team templates keep their real delete path) - List route decorates rows with per-company is_hidden; pickers filter them out; the settings panel shows hidden ones in a collapsed restore section so hiding is never silent - Classified in full-archive-export exclusions (UI preference, not rakenskapsinformation) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PU1KN431c9gp5zKvFaa1NL * fix(booking-templates): idempotent re-hide, system-only RLS insert, hidden filter in bulk-book Skeptic + CodeRabbit findings on #2004, one pass: - hide upsert now passes ignoreDuplicates (DO NOTHING): the table has no UPDATE policy on purpose, so the DO UPDATE conflict arm turned a concurrent re-hide into an RLS 42501/500; pg test pins the conflict shape - bth_insert policy additionally requires the referenced template to be an active system template (migration is unmerged, edited in place); negative pg test for company templates - BulkBookDialog excludes templates hidden by the company (was reading the table directly and ignoring hides) - panel shows the failure toast when the hide/unhide fetch itself rejects - picker category chips built from the hidden-filtered list Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PU1KN431c9gp5zKvFaa1NL --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-28 12:12:47 +02:00
parent 5720632832
commit 57d4359d1a
15 changed files with 834 additions and 18 deletions
@@ -0,0 +1,198 @@
+/**
+ * Tests for POST/DELETE /api/settings/booking-templates/[id]/hide.
+ *
+ * Hiding is opt-in and per-company: a hide row is written for the ACTIVE
+ * company only, and only for system templates (company/team templates have a
+ * real delete path already). The tests lock in both properties, plus the
+ * idempotent unhide.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { NextResponse } from 'next/server'
+import { createMockRequest, parseJsonResponse } from '@/tests/helpers'
+
+const requireAuthMock = vi.fn()
+vi.mock('@/lib/auth/require-auth', () => ({
+  requireAuth: (...args: unknown[]) => requireAuthMock(...args),
+}))
+
+vi.mock('@/lib/company/context', () => ({
+  getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
+  requireCompanyId: vi.fn().mockResolvedValue('company-1'),
+}))
+
+const requireWriteMock = vi.fn()
+vi.mock('@/lib/auth/require-write', () => ({
+  requireWritePermission: (...args: unknown[]) => requireWriteMock(...args),
+}))
+
+import { POST, DELETE } from '../route'
+
+interface CapturedCall {
+  method: string
+  args: unknown[]
+}
+
+/** Chainable builder recording calls; resolves queued {data,error} per from(). */
+function createCapturingSupabase(results: { data?: unknown; error?: unknown }[]) {
+  const calls: CapturedCall[] = []
+  let idx = 0
+  const makeBuilder = () => {
+    const result = results[idx++] ?? { data: null, error: null }
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const b: any = {}
+    for (const m of ['select', 'eq', 'upsert', 'delete', 'maybeSingle']) {
+      b[m] = (...args: unknown[]) => {
+        calls.push({ method: m, args })
+        return b
+      }
+    }
+    b.then = (resolve: (v: unknown) => void) =>
+      resolve({ data: result.data ?? null, error: result.error ?? null })
+    return b
+  }
+  return {
+    supabase: {
+      from: (table: string) => {
+        calls.push({ method: 'from', args: [table] })
+        return makeBuilder()
+      },
+    },
+    calls,
+  }
+}
+
+const idParams = { params: Promise.resolve({ id: 'tpl-1' }) }
+
+const SYSTEM_TEMPLATE = {
+  data: { id: 'tpl-1', is_system: true, is_active: true },
+}
+
+beforeEach(() => {
+  vi.clearAllMocks()
+  requireWriteMock.mockResolvedValue({ ok: true })
+})
+
+function auth(supabase: unknown) {
+  requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase, error: null })
+}
+
+function req(method: 'POST' | 'DELETE') {
+  return createMockRequest('/api/settings/booking-templates/tpl-1/hide', { method })
+}
+
+describe('POST /api/settings/booking-templates/[id]/hide', () => {
+  it('returns 401 when not authenticated', async () => {
+    requireAuthMock.mockResolvedValue({
+      user: null,
+      supabase: {},
+      error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
+    })
+    expect((await POST(req('POST'), idParams)).status).toBe(401)
+  })
+
+  it('returns 403 for a viewer', async () => {
+    const { supabase } = createCapturingSupabase([])
+    auth(supabase)
+    requireWriteMock.mockResolvedValue({
+      ok: false,
+      response: NextResponse.json({ error: 'Forbidden' }, { status: 403 }),
+    })
+    expect((await POST(req('POST'), idParams)).status).toBe(403)
+  })
+
+  it('returns 404 when the template does not exist', async () => {
+    const { supabase, calls } = createCapturingSupabase([{ data: null }])
+    auth(supabase)
+    const { status } = await parseJsonResponse(await POST(req('POST'), idParams))
+    expect(status).toBe(404)
+    expect(calls.find((c) => c.method === 'upsert')).toBeUndefined()
+  })
+
+  it('returns 404 for a retired (inactive) template', async () => {
+    const { supabase, calls } = createCapturingSupabase([
+      { data: { id: 'tpl-1', is_system: true, is_active: false } },
+    ])
+    auth(supabase)
+    const { status } = await parseJsonResponse(await POST(req('POST'), idParams))
+    expect(status).toBe(404)
+    expect(calls.find((c) => c.method === 'upsert')).toBeUndefined()
+  })
+
+  it('returns 400 for a non-system template', async () => {
+    const { supabase, calls } = createCapturingSupabase([
+      { data: { id: 'tpl-1', is_system: false, is_active: true } },
+    ])
+    auth(supabase)
+    const { status } = await parseJsonResponse(await POST(req('POST'), idParams))
+    expect(status).toBe(400)
+    expect(calls.find((c) => c.method === 'upsert')).toBeUndefined()
+  })
+
+  it('returns 500 when the upsert fails', async () => {
+    const { supabase } = createCapturingSupabase([
+      SYSTEM_TEMPLATE,
+      { error: { message: 'boom' } },
+    ])
+    auth(supabase)
+    const { status } = await parseJsonResponse(await POST(req('POST'), idParams))
+    expect(status).toBe(500)
+  })
+
+  it('hides a system template for the active company on the happy path', async () => {
+    const { supabase, calls } = createCapturingSupabase([SYSTEM_TEMPLATE, { data: null }])
+    auth(supabase)
+    const { status } = await parseJsonResponse(await POST(req('POST'), idParams))
+    expect(status).toBe(200)
+    const upsert = calls.find((c) => c.method === 'upsert')
+    expect(upsert?.args[0]).toEqual({
+      template_id: 'tpl-1',
+      company_id: 'company-1',
+      hidden_by: 'user-1',
+    })
+    // ignoreDuplicates is load-bearing: the table has no UPDATE policy, so a
+    // DO UPDATE conflict arm would be rejected by RLS on a concurrent re-hide.
+    expect(upsert?.args[1]).toEqual({
+      onConflict: 'template_id,company_id',
+      ignoreDuplicates: true,
+    })
+  })
+})
+
+describe('DELETE /api/settings/booking-templates/[id]/hide', () => {
+  it('returns 401 when not authenticated', async () => {
+    requireAuthMock.mockResolvedValue({
+      user: null,
+      supabase: {},
+      error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
+    })
+    expect((await DELETE(req('DELETE'), idParams)).status).toBe(401)
+  })
+
+  it('returns 403 for a viewer', async () => {
+    const { supabase } = createCapturingSupabase([])
+    auth(supabase)
+    requireWriteMock.mockResolvedValue({
+      ok: false,
+      response: NextResponse.json({ error: 'Forbidden' }, { status: 403 }),
+    })
+    expect((await DELETE(req('DELETE'), idParams)).status).toBe(403)
+  })
+
+  it('returns 500 when the delete fails', async () => {
+    const { supabase } = createCapturingSupabase([{ error: { message: 'boom' } }])
+    auth(supabase)
+    const { status } = await parseJsonResponse(await DELETE(req('DELETE'), idParams))
+    expect(status).toBe(500)
+  })
+
+  it('unhides scoped to the active company, idempotently', async () => {
+    // Zero deleted rows is still success: unhide may race a double click.
+    const { supabase, calls } = createCapturingSupabase([{ data: null }])
+    auth(supabase)
+    const { status } = await parseJsonResponse(await DELETE(req('DELETE'), idParams))
+    expect(status).toBe(200)
+    const eqCalls = calls.filter((c) => c.method === 'eq').map((c) => c.args)
+    expect(eqCalls).toContainEqual(['template_id', 'tpl-1'])
+    expect(eqCalls).toContainEqual(['company_id', 'company-1'])
+  })
+})
@@ -0,0 +1,82 @@
+import { NextResponse } from 'next/server'
+import { withRouteContext } from '@/lib/api/with-route-context'
+import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-message'
+
+/**
+ * POST /api/settings/booking-templates/[id]/hide
+ *
+ * Hide a system template for the current company. Opt-in and per-company:
+ * nothing is hidden by default, and a hide row never affects any other
+ * company. Company/team templates are excluded on purpose: they already have
+ * a real delete path, and hiding them would just be a confusing second one.
+ *
+ * DELETE /api/settings/booking-templates/[id]/hide
+ *
+ * Unhide (restore) the template for the current company.
+ */
+export const POST = withRouteContext<{ params: Promise<{ id: string }> }>(
+  'booking_template.hide',
+  async (_request, ctx, { params }) => {
+    const { id } = await params
+    const { supabase, companyId, user } = ctx
+
+    // Only an existing, active SYSTEM template can be hidden. RLS on
+    // booking_template_hidden scopes the write to the active company; this
+    // check scopes it to the right kind of template.
+    const { data: template, error: templateError } = await supabase
+      .from('booking_template_library')
+      .select('id, is_system, is_active')
+      .eq('id', id)
+      .maybeSingle()
+
+    if (templateError) {
+      return NextResponse.json({ error: getUserErrorMessage(templateError) }, { status: 500 })
+    }
+    if (!template || !template.is_active) {
+      return NextResponse.json({ error: 'Template not found' }, { status: 404 })
+    }
+    if (!template.is_system) {
+      return NextResponse.json({ error: 'Only system templates can be hidden' }, { status: 400 })
+    }
+
+    // ignoreDuplicates makes the conflict arm DO NOTHING. The table has no
+    // UPDATE policy (insert-or-delete only), so a DO UPDATE arm would be
+    // rejected by RLS and turn a concurrent re-hide into a 500.
+    const { error } = await supabase
+      .from('booking_template_hidden')
+      .upsert(
+        { template_id: id, company_id: companyId, hidden_by: user.id },
+        { onConflict: 'template_id,company_id', ignoreDuplicates: true },
+      )
+
+    if (error) {
+      return NextResponse.json({ error: getUserErrorMessage(error) }, { status: 500 })
+    }
+
+    return NextResponse.json({ data: { success: true } })
+  },
+  { requireWrite: true },
+)
+
+export const DELETE = withRouteContext<{ params: Promise<{ id: string }> }>(
+  'booking_template.unhide',
+  async (_request, ctx, { params }) => {
+    const { id } = await params
+    const { supabase, companyId } = ctx
+
+    // Deleting a row that does not exist is a no-op success: unhide is
+    // idempotent, and the panel may race a double click.
+    const { error } = await supabase
+      .from('booking_template_hidden')
+      .delete()
+      .eq('template_id', id)
+      .eq('company_id', companyId)
+
+    if (error) {
+      return NextResponse.json({ error: getUserErrorMessage(error) }, { status: 500 })
+    }
+
+    return NextResponse.json({ data: { success: true } })
+  },
+  { requireWrite: true },
+)
@@ -0,0 +1,104 @@
+/**
+ * Tests for GET /api/settings/booking-templates.
+ *
+ * Focused on the is_hidden decoration: every row carries the per-company flag,
+ * a failed hidden lookup falls back to "nothing hidden" (showing extra
+ * templates is the safe direction), and hidden rows are still RETURNED so the
+ * settings panel can offer restore; filtering is the pickers' job.
+ */
+import { describe, it, expect, vi, beforeEach } from 'vitest'
+import { NextResponse } from 'next/server'
+import { createMockRequest, parseJsonResponse } from '@/tests/helpers'
+
+const requireAuthMock = vi.fn()
+vi.mock('@/lib/auth/require-auth', () => ({
+  requireAuth: (...args: unknown[]) => requireAuthMock(...args),
+}))
+
+vi.mock('@/lib/company/context', () => ({
+  getActiveCompanyId: vi.fn().mockResolvedValue('11111111-1111-4111-8111-111111111111'),
+  requireCompanyId: vi.fn().mockResolvedValue('11111111-1111-4111-8111-111111111111'),
+}))
+
+const requireWriteMock = vi.fn()
+vi.mock('@/lib/auth/require-write', () => ({
+  requireWritePermission: (...args: unknown[]) => requireWriteMock(...args),
+}))
+
+import { GET } from '../route'
+
+/** Chainable builder resolving queued {data,error} per from() in call order. */
+function createQueuedSupabase(results: { data?: unknown; error?: unknown }[]) {
+  let idx = 0
+  const makeBuilder = () => {
+    const result = results[idx++] ?? { data: null, error: null }
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    const b: any = {}
+    for (const m of ['select', 'eq', 'or', 'order', 'maybeSingle']) {
+      b[m] = () => b
+    }
+    b.then = (resolve: (v: unknown) => void) =>
+      resolve({ data: result.data ?? null, error: result.error ?? null })
+    return b
+  }
+  return { from: () => makeBuilder() }
+}
+
+const TEMPLATES = [
+  { id: 'tpl-1', name: 'Bankavgift', is_system: true },
+  { id: 'tpl-2', name: 'Eget uttag', is_system: true },
+]
+
+beforeEach(() => {
+  vi.clearAllMocks()
+})
+
+function auth(supabase: unknown) {
+  requireAuthMock.mockResolvedValue({ user: { id: 'user-1' }, supabase, error: null })
+}
+
+const req = () => createMockRequest('/api/settings/booking-templates', { method: 'GET' })
+
+describe('GET /api/settings/booking-templates', () => {
+  it('returns 401 when not authenticated', async () => {
+    requireAuthMock.mockResolvedValue({
+      user: null,
+      supabase: {},
+      error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
+    })
+    expect((await GET(req(), { params: Promise.resolve({}) })).status).toBe(401)
+  })
+
+  it('marks hidden templates but still returns them', async () => {
+    // from() order: companies, then library / usage / hidden.
+    const supabase = createQueuedSupabase([
+      { data: { team_id: null } },
+      { data: TEMPLATES },
+      { data: [] },
+      { data: [{ template_id: 'tpl-2' }] },
+    ])
+    auth(supabase)
+    const { status, body } = await parseJsonResponse<{
+      data: { id: string; is_hidden: boolean }[]
+    }>(await GET(req(), { params: Promise.resolve({}) }))
+    expect(status).toBe(200)
+    expect(body.data).toHaveLength(2)
+    expect(body.data.find((t) => t.id === 'tpl-1')?.is_hidden).toBe(false)
+    expect(body.data.find((t) => t.id === 'tpl-2')?.is_hidden).toBe(true)
+  })
+
+  it('falls back to nothing hidden when the hidden lookup fails', async () => {
+    const supabase = createQueuedSupabase([
+      { data: { team_id: null } },
+      { data: TEMPLATES },
+      { data: [] },
+      { error: { message: 'boom' } },
+    ])
+    auth(supabase)
+    const { status, body } = await parseJsonResponse<{
+      data: { is_hidden: boolean }[]
+    }>(await GET(req(), { params: Promise.resolve({}) }))
+    expect(status).toBe(200)
+    expect(body.data.every((t) => t.is_hidden === false)).toBe(true)
+  })
+})
@@ -73,7 +73,7 @@ export const GET = withRouteContext(
      ...(teamId && UUID_RE.test(teamId) ? [`team_id.eq.${teamId}`] : []),
    ].join(',')

-    const [templatesRes, usageRes] = await Promise.all([
+    const [templatesRes, usageRes, hiddenRes] = await Promise.all([
      supabase
        .from('booking_template_library')
        .select('*')
@@ -85,6 +85,10 @@ export const GET = withRouteContext(
        .from('booking_template_usage')
        .select('template_id, last_used_at')
        .eq('company_id', companyId),
+      supabase
+        .from('booking_template_hidden')
+        .select('template_id')
+        .eq('company_id', companyId),
    ])

    if (templatesRes.error) {
@@ -98,10 +102,20 @@ export const GET = withRouteContext(
      }
    }

+    // hidden lookup failing is also non-fatal: falling back to "nothing
+    // hidden" shows extra templates, which is the safe direction.
+    const hiddenIds = new Set<string>()
+    if (!hiddenRes.error && hiddenRes.data) {
+      for (const row of hiddenRes.data) {
+        hiddenIds.add(row.template_id)
+      }
+    }
+
    const templates = templatesRes.data ?? []
    const decorated = templates.map((t) => ({
      ...t,
      last_used_at: usageByTemplate.get(t.id) ?? null,
+      is_hidden: hiddenIds.has(t.id),
    }))

    // Stable-sort: templates with last_used_at come first (most-recent first).