From c5a85ad0ebe41b807bf41976a96113f4172e4cbb Mon Sep 17 00:00:00 2001 From: mindsers Date: Mon, 20 Jul 2026 23:00:32 +0200 Subject: [PATCH] feat(templates): allow deleting custom event templates Add a delete action for event templates from the template view page, guarded server-side. System templates (day-off, freeform) are refused because they are looked up by key at runtime, and templates still linked to events are refused because Event.templateId is ON DELETE SET NULL and removing them would orphan those events out of key-filtered queries. Whole-template removal is manager-only; the button is hidden for system templates and disabled while events reference the template. Also fix a missing React key warning in the template edit parts table (bare fragment inside a list) surfaced while testing. --- app/features/events/index.server.ts | 3 +- .../server/duplicate-template.server.test.ts | 184 ++++++++++++++++ .../server/duplicate-template.server.ts | 91 ++++++++ .../server/event-templates.server.test.ts | 200 +++++------------- .../events/server/event-templates.server.ts | 109 +++------- .../part-role-labels.integration.test.ts | 2 +- .../routes/congregation/templates/edit.tsx | 6 +- .../routes/congregation/templates/view.tsx | 82 ++++++- app/i18n/messages/en.json | 3 + app/i18n/messages/fr.json | 3 + 10 files changed, 439 insertions(+), 244 deletions(-) create mode 100644 app/features/events/server/duplicate-template.server.test.ts create mode 100644 app/features/events/server/duplicate-template.server.ts diff --git a/app/features/events/index.server.ts b/app/features/events/index.server.ts index 09b57f3c..ce399cea 100644 --- a/app/features/events/index.server.ts +++ b/app/features/events/index.server.ts @@ -1,10 +1,11 @@ // Public server-only surface of the events feature. export { getNextDaysOffs } from './server/days-off.server' +export { duplicateTemplate } from './server/duplicate-template.server' export { + deleteTemplate, deleteTemplatePart, deleteTemplateServicePart, - duplicateTemplate, getTemplateById, getTemplates, isTemplateResponsible, diff --git a/app/features/events/server/duplicate-template.server.test.ts b/app/features/events/server/duplicate-template.server.test.ts new file mode 100644 index 00000000..4e2d4180 --- /dev/null +++ b/app/features/events/server/duplicate-template.server.test.ts @@ -0,0 +1,184 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('~/shared/infra/db.server', () => ({ + unscopedDb: { + eventTemplate: { findFirst: vi.fn(), create: vi.fn() }, + templatePartAllowedRole: { createMany: vi.fn() }, + templateServicePartAllowedRole: { createMany: vi.fn() }, + }, +})) + +const { duplicateTemplate } = await import('./duplicate-template.server') +const { unscopedDb: db } = await import('~/shared/infra/db.server') + +beforeEach(() => { + vi.resetAllMocks() +}) + +describe('duplicateTemplate', () => { + it('returns null when source template not found', async () => { + vi.mocked(db.eventTemplate.findFirst).mockResolvedValue(null as never) + + const result = await duplicateTemplate(db, 99, 1) + + expect(result).toBeNull() + expect(vi.mocked(db.eventTemplate.create)).not.toHaveBeenCalled() + }) + + // System templates are looked up by `key` at runtime. Duplicating them would + // produce a row with an untethered `-copy-` suffix; the UI hides the + // action but this is the server-side belt-and-suspenders check. + it('returns null when the source is a system template', async () => { + vi.mocked(db.eventTemplate.findFirst).mockResolvedValue({ + id: 1, + key: 'day-off', + name: 'Absence', + parts: [], + serviceParts: [], + } as never) + + const result = await duplicateTemplate(db, 1, 1) + + expect(result).toBeNull() + expect(vi.mocked(db.eventTemplate.create)).not.toHaveBeenCalled() + }) + + it('copies allowed-role lists from source parts and service roles to the duplicate', async () => { + const source = { + id: 5, + name: 'Reunion', + key: 'midweek', + description: '', + weekDay: 2, + isRecurring: true, + parts: [ + { + id: 10, + name: 'Discours', + section: '', + track: '', + order: 1, + durationMin: 30, + allowExternalSpeaker: false, + allowedRoles: [ + { roleId: 100, asKind: 'speaker' }, + { roleId: 200, asKind: 'reader' }, + ], + }, + { + id: 11, + name: 'Cantique', + section: '', + track: '', + order: 2, + durationMin: 5, + allowExternalSpeaker: false, + allowedRoles: [], + }, + ], + serviceParts: [ + { id: 20, name: 'Son', key: 'sono', allowedRoles: [{ roleId: 300 }] }, + { id: 21, name: 'Stage', key: 'stage', allowedRoles: [] }, + ], + } + vi.mocked(db.eventTemplate.findFirst).mockResolvedValue(source as never) + + const duplicated = { + id: 99, + name: 'Reunion (copie)', + parts: [ + { id: 510, order: 1 }, + { id: 511, order: 2 }, + ], + serviceParts: [ + { id: 520, name: 'Son' }, + { id: 521, name: 'Stage' }, + ], + } + vi.mocked(db.eventTemplate.create).mockResolvedValue(duplicated as never) + vi.mocked(db.templatePartAllowedRole.createMany).mockResolvedValue({ count: 2 } as never) + vi.mocked(db.templateServicePartAllowedRole.createMany).mockResolvedValue({ count: 1 } as never) + + await duplicateTemplate(db, 5, 7) + + // Speaker role for the first part + expect(vi.mocked(db.templatePartAllowedRole.createMany)).toHaveBeenCalledWith({ + data: [{ partId: 510, roleId: 100, asKind: 'speaker', congregationId: 7 }], + skipDuplicates: true, + }) + // Reader role for the first part + expect(vi.mocked(db.templatePartAllowedRole.createMany)).toHaveBeenCalledWith({ + data: [{ partId: 510, roleId: 200, asKind: 'reader', congregationId: 7 }], + skipDuplicates: true, + }) + // Service-role allowed-roles for the first service role + expect(vi.mocked(db.templateServicePartAllowedRole.createMany)).toHaveBeenCalledWith({ + data: [{ servicePartId: 520, roleId: 300, congregationId: 7 }], + skipDuplicates: true, + }) + // Empty lists are skipped + expect(vi.mocked(db.templatePartAllowedRole.createMany)).toHaveBeenCalledTimes(2) + expect(vi.mocked(db.templateServicePartAllowedRole.createMany)).toHaveBeenCalledTimes(1) + }) + + // A duplicated template must carry the source's per-part role labels; without + // this, admins who clone a template lose their custom labels silently. + it('copies speakerLabel and readerLabel from source parts to the duplicate (Layer 4)', async () => { + const source = { + id: 5, + name: 'Reunion', + key: 'midweek', + description: '', + weekDay: 2, + isRecurring: true, + parts: [ + { + id: 10, + name: 'Bible reading', + section: '', + track: '', + order: 1, + durationMin: 5, + allowExternalSpeaker: false, + // Distinct sentinels per part so an ordering regression in the copy + // loop (swapping parts[0] and parts[1]) fails visibly. + speakerLabel: 'STUDENT-SENTINEL-P1', + readerLabel: null, + allowedRoles: [], + }, + { + id: 11, + name: 'Return visit', + section: '', + track: '', + order: 2, + durationMin: 10, + allowExternalSpeaker: false, + speakerLabel: 'STUDENT-SENTINEL-P2', + readerLabel: 'HOUSEHOLDER-SENTINEL-P2', + allowedRoles: [], + }, + ], + serviceParts: [], + } + vi.mocked(db.eventTemplate.findFirst).mockResolvedValue(source as never) + vi.mocked(db.eventTemplate.create).mockResolvedValue({ + id: 99, + name: 'Reunion (copie)', + parts: [], + serviceParts: [], + } as never) + + await duplicateTemplate(db, 5, 7) + + const createCall = vi.mocked(db.eventTemplate.create).mock.calls[0][0] as { + data: { parts: { create: Array<{ speakerLabel: string | null; readerLabel: string | null }> } } + } + const createdParts = createCall.data.parts.create + expect(createdParts[0]).toMatchObject({ speakerLabel: 'STUDENT-SENTINEL-P1', readerLabel: null }) + expect(createdParts[1]).toMatchObject({ + speakerLabel: 'STUDENT-SENTINEL-P2', + readerLabel: 'HOUSEHOLDER-SENTINEL-P2', + }) + }) +}) diff --git a/app/features/events/server/duplicate-template.server.ts b/app/features/events/server/duplicate-template.server.ts new file mode 100644 index 00000000..3c95b8b1 --- /dev/null +++ b/app/features/events/server/duplicate-template.server.ts @@ -0,0 +1,91 @@ +import { isSystemTemplate } from '~/features/events/model/event-template.type' +import type { TransactionClient } from '~/shared/infra/db.server' + +export async function duplicateTemplate(db: TransactionClient, templateId: number, congregationId: number) { + const source = await db.eventTemplate.findFirst({ + where: { id: templateId, congregationId }, + include: { + parts: { + orderBy: { order: 'asc' }, + include: { allowedRoles: true }, + }, + serviceParts: { include: { allowedRoles: true } }, + }, + }) + if (!source) return null + + // System templates are looked up by key at runtime — duplicating them just + // clutters the list with an untethered `-copy-` row. The UI hides the + // Duplicate button; this is the server-side match. + if (isSystemTemplate(source.key)) return null + + const duplicated = await db.eventTemplate.create({ + data: { + name: `${source.name} (copie)`, + key: `${source.key}-copy-${Date.now()}`, + description: source.description, + weekDay: source.weekDay, + isRecurring: source.isRecurring, + startTime: source.startTime, + endTime: source.endTime, + congregationId, + parts: { + create: source.parts.map(part => ({ + name: part.name, + section: part.section, + track: part.track, + order: part.order, + durationMin: part.durationMin, + allowExternalSpeaker: part.allowExternalSpeaker, + speakerLabel: part.speakerLabel, + readerLabel: part.readerLabel, + congregationId, + })), + }, + serviceParts: { + create: source.serviceParts.map(role => ({ + name: role.name, + key: `${role.key}-copy-${Date.now()}`, + congregationId, + })), + }, + }, + include: { + parts: { orderBy: { order: 'asc' } }, + serviceParts: { orderBy: { name: 'asc' } }, + }, + }) + + const sourcePartsByOrder = new Map(source.parts.map(p => [p.order, p])) + const sourceServicePartsByName = new Map(source.serviceParts.map(r => [r.name, r])) + + for (const newPart of duplicated.parts) { + const sourcePart = sourcePartsByOrder.get(newPart.order) + if (!sourcePart) continue + const speakerRoleIds = sourcePart.allowedRoles.filter(r => r.asKind === 'speaker').map(r => r.roleId) + const readerRoleIds = sourcePart.allowedRoles.filter(r => r.asKind === 'reader').map(r => r.roleId) + if (speakerRoleIds.length > 0) { + await db.templatePartAllowedRole.createMany({ + data: speakerRoleIds.map(roleId => ({ partId: newPart.id, roleId, asKind: 'speaker', congregationId })), + skipDuplicates: true, + }) + } + if (readerRoleIds.length > 0) { + await db.templatePartAllowedRole.createMany({ + data: readerRoleIds.map(roleId => ({ partId: newPart.id, roleId, asKind: 'reader', congregationId })), + skipDuplicates: true, + }) + } + } + + for (const newRole of duplicated.serviceParts) { + const sourceRole = sourceServicePartsByName.get(newRole.name) + if (!sourceRole || sourceRole.allowedRoles.length === 0) continue + await db.templateServicePartAllowedRole.createMany({ + data: sourceRole.allowedRoles.map(r => ({ servicePartId: newRole.id, roleId: r.roleId, congregationId })), + skipDuplicates: true, + }) + } + + return duplicated +} diff --git a/app/features/events/server/event-templates.server.test.ts b/app/features/events/server/event-templates.server.test.ts index 79eac5fd..223d430c 100644 --- a/app/features/events/server/event-templates.server.test.ts +++ b/app/features/events/server/event-templates.server.test.ts @@ -2,7 +2,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('~/shared/infra/db.server', () => ({ unscopedDb: { - eventTemplate: { findMany: vi.fn(), findFirst: vi.fn(), update: vi.fn(), create: vi.fn() }, + eventTemplate: { findMany: vi.fn(), findFirst: vi.fn(), update: vi.fn(), create: vi.fn(), delete: vi.fn() }, + event: { count: vi.fn() }, templatePart: { create: vi.fn(), update: vi.fn(), delete: vi.fn() }, templateServicePart: { create: vi.fn(), update: vi.fn(), delete: vi.fn() }, templateResponsible: { upsert: vi.fn(), deleteMany: vi.fn(), findFirst: vi.fn() }, @@ -35,7 +36,7 @@ const { setTemplateResponsible, removeTemplateResponsible, isTemplateResponsible, - duplicateTemplate, + deleteTemplate, } = await import('./event-templates.server') const { unscopedDb: db } = await import('~/shared/infra/db.server') const allowedRoles = await import('~/features/events/server/allowed-roles.server') @@ -410,170 +411,69 @@ describe('upsertTemplateServicePart audit firing', () => { }) }) -describe('duplicateTemplate', () => { - it('returns null when source template not found', async () => { +describe('deleteTemplate', () => { + it('deletes a custom template with no linked events and returns its name', async () => { + vi.mocked(db.eventTemplate.findFirst).mockResolvedValue({ + key: 'midweek', + name: 'Réunion de semaine', + _count: { events: 0 }, + } as never) + vi.mocked(db.eventTemplate.delete).mockResolvedValue({ id: 7 } as never) + + const result = await deleteTemplate(db, 7, 1) + + expect(result).toEqual({ ok: true, name: 'Réunion de semaine' }) + expect(db.eventTemplate.delete).toHaveBeenCalledWith({ + where: { id_congregationId: { id: 7, congregationId: 1 } }, + }) + }) + + it('returns not-found and never deletes when the template does not exist', async () => { vi.mocked(db.eventTemplate.findFirst).mockResolvedValue(null as never) - const result = await duplicateTemplate(db, 99, 1) + const result = await deleteTemplate(db, 999, 1) - expect(result).toBeNull() - expect(vi.mocked(db.eventTemplate.create)).not.toHaveBeenCalled() + expect(result).toEqual({ ok: false, reason: 'not-found' }) + expect(db.eventTemplate.delete).not.toHaveBeenCalled() }) - // System templates are looked up by `key` at runtime. Duplicating them would - // produce a row with an untethered `-copy-` suffix; the UI hides the - // action but this is the server-side belt-and-suspenders check. - it('returns null when the source is a system template', async () => { + // System templates (day-off, freeform) are looked up by key at runtime; the + // server refuses to remove them even if a stale form POSTs the intent. + it('refuses to delete a system template', async () => { vi.mocked(db.eventTemplate.findFirst).mockResolvedValue({ - id: 1, key: 'day-off', - name: 'Absence', - parts: [], - serviceParts: [], + name: 'Jour de congé', + _count: { events: 0 }, } as never) - const result = await duplicateTemplate(db, 1, 1) + const result = await deleteTemplate(db, 3, 1) - expect(result).toBeNull() - expect(vi.mocked(db.eventTemplate.create)).not.toHaveBeenCalled() + expect(result).toEqual({ ok: false, reason: 'system' }) + expect(db.eventTemplate.delete).not.toHaveBeenCalled() }) - it('copies allowed-role lists from source parts and service roles to the duplicate', async () => { - const source = { - id: 5, - name: 'Reunion', + // Event.templateId is ON DELETE SET NULL: removing an in-use template would + // orphan its events (templateId → NULL) and drop them from key-filtered + // queries. The writer blocks it and surfaces the count instead. + it('refuses to delete a template still linked to events and reports the count', async () => { + vi.mocked(db.eventTemplate.findFirst).mockResolvedValue({ key: 'midweek', - description: '', - weekDay: 2, - isRecurring: true, - parts: [ - { - id: 10, - name: 'Discours', - section: '', - track: '', - order: 1, - durationMin: 30, - allowExternalSpeaker: false, - allowedRoles: [ - { roleId: 100, asKind: 'speaker' }, - { roleId: 200, asKind: 'reader' }, - ], - }, - { - id: 11, - name: 'Cantique', - section: '', - track: '', - order: 2, - durationMin: 5, - allowExternalSpeaker: false, - allowedRoles: [], - }, - ], - serviceParts: [ - { id: 20, name: 'Son', key: 'sono', allowedRoles: [{ roleId: 300 }] }, - { id: 21, name: 'Stage', key: 'stage', allowedRoles: [] }, - ], - } - vi.mocked(db.eventTemplate.findFirst).mockResolvedValue(source as never) - - const duplicated = { - id: 99, - name: 'Reunion (copie)', - parts: [ - { id: 510, order: 1 }, - { id: 511, order: 2 }, - ], - serviceParts: [ - { id: 520, name: 'Son' }, - { id: 521, name: 'Stage' }, - ], - } - vi.mocked(db.eventTemplate.create).mockResolvedValue(duplicated as never) - vi.mocked(db.templatePartAllowedRole.createMany).mockResolvedValue({ count: 2 } as never) - vi.mocked(db.templateServicePartAllowedRole.createMany).mockResolvedValue({ count: 1 } as never) - - await duplicateTemplate(db, 5, 7) - - // Speaker role for the first part - expect(vi.mocked(db.templatePartAllowedRole.createMany)).toHaveBeenCalledWith({ - data: [{ partId: 510, roleId: 100, asKind: 'speaker', congregationId: 7 }], - skipDuplicates: true, - }) - // Reader role for the first part - expect(vi.mocked(db.templatePartAllowedRole.createMany)).toHaveBeenCalledWith({ - data: [{ partId: 510, roleId: 200, asKind: 'reader', congregationId: 7 }], - skipDuplicates: true, - }) - // Service-role allowed-roles for the first service role - expect(vi.mocked(db.templateServicePartAllowedRole.createMany)).toHaveBeenCalledWith({ - data: [{ servicePartId: 520, roleId: 300, congregationId: 7 }], - skipDuplicates: true, - }) - // Empty lists are skipped - expect(vi.mocked(db.templatePartAllowedRole.createMany)).toHaveBeenCalledTimes(2) - expect(vi.mocked(db.templateServicePartAllowedRole.createMany)).toHaveBeenCalledTimes(1) + name: 'Réunion de semaine', + _count: { events: 4 }, + } as never) + + const result = await deleteTemplate(db, 7, 1) + + expect(result).toEqual({ ok: false, reason: 'in-use', eventCount: 4 }) + expect(db.eventTemplate.delete).not.toHaveBeenCalled() }) - // A duplicated template must carry the source's per-part role labels; without - // this, admins who clone a template lose their custom labels silently. - it('copies speakerLabel and readerLabel from source parts to the duplicate (Layer 4)', async () => { - const source = { - id: 5, - name: 'Reunion', - key: 'midweek', - description: '', - weekDay: 2, - isRecurring: true, - parts: [ - { - id: 10, - name: 'Bible reading', - section: '', - track: '', - order: 1, - durationMin: 5, - allowExternalSpeaker: false, - // Distinct sentinels per part so an ordering regression in the copy - // loop (swapping parts[0] and parts[1]) fails visibly. - speakerLabel: 'STUDENT-SENTINEL-P1', - readerLabel: null, - allowedRoles: [], - }, - { - id: 11, - name: 'Return visit', - section: '', - track: '', - order: 2, - durationMin: 10, - allowExternalSpeaker: false, - speakerLabel: 'STUDENT-SENTINEL-P2', - readerLabel: 'HOUSEHOLDER-SENTINEL-P2', - allowedRoles: [], - }, - ], - serviceParts: [], - } - vi.mocked(db.eventTemplate.findFirst).mockResolvedValue(source as never) - vi.mocked(db.eventTemplate.create).mockResolvedValue({ - id: 99, - name: 'Reunion (copie)', - parts: [], - serviceParts: [], - } as never) + it('scopes the lookup to the given congregation', async () => { + vi.mocked(db.eventTemplate.findFirst).mockResolvedValue(null as never) - await duplicateTemplate(db, 5, 7) + await deleteTemplate(db, 7, 42) - const createCall = vi.mocked(db.eventTemplate.create).mock.calls[0][0] as { - data: { parts: { create: Array<{ speakerLabel: string | null; readerLabel: string | null }> } } - } - const createdParts = createCall.data.parts.create - expect(createdParts[0]).toMatchObject({ speakerLabel: 'STUDENT-SENTINEL-P1', readerLabel: null }) - expect(createdParts[1]).toMatchObject({ - speakerLabel: 'STUDENT-SENTINEL-P2', - readerLabel: 'HOUSEHOLDER-SENTINEL-P2', - }) + const args = vi.mocked(db.eventTemplate.findFirst).mock.calls[0][0] + expect(args?.where).toEqual({ id: 7, congregationId: 42 }) }) }) diff --git a/app/features/events/server/event-templates.server.ts b/app/features/events/server/event-templates.server.ts index e03662c3..f0a6675d 100644 --- a/app/features/events/server/event-templates.server.ts +++ b/app/features/events/server/event-templates.server.ts @@ -247,91 +247,38 @@ export function isTemplateResponsible( }) } -export async function duplicateTemplate(db: TransactionClient, templateId: number, congregationId: number) { - const source = await db.eventTemplate.findFirst({ - where: { id: templateId, congregationId }, - include: { - parts: { - orderBy: { order: 'asc' }, - include: { allowedRoles: true }, - }, - serviceParts: { include: { allowedRoles: true } }, - }, - }) - if (!source) return null - - // System templates are looked up by key at runtime — duplicating them just - // clutters the list with an untethered `-copy-` row. The UI hides the - // Duplicate button; this is the server-side match. - if (isSystemTemplate(source.key)) return null +export type DeleteTemplateResult = + | { ok: true; name: string } + | { ok: false; reason: 'not-found' | 'system' } + | { ok: false; reason: 'in-use'; eventCount: number } - const duplicated = await db.eventTemplate.create({ - data: { - name: `${source.name} (copie)`, - key: `${source.key}-copy-${Date.now()}`, - description: source.description, - weekDay: source.weekDay, - isRecurring: source.isRecurring, - startTime: source.startTime, - endTime: source.endTime, - congregationId, - parts: { - create: source.parts.map(part => ({ - name: part.name, - section: part.section, - track: part.track, - order: part.order, - durationMin: part.durationMin, - allowExternalSpeaker: part.allowExternalSpeaker, - speakerLabel: part.speakerLabel, - readerLabel: part.readerLabel, - congregationId, - })), - }, - serviceParts: { - create: source.serviceParts.map(role => ({ - name: role.name, - key: `${role.key}-copy-${Date.now()}`, - congregationId, - })), - }, - }, - include: { - parts: { orderBy: { order: 'asc' } }, - serviceParts: { orderBy: { name: 'asc' } }, - }, +export async function deleteTemplate( + db: TransactionClient, + templateId: number, + congregationId: number, +): Promise { + const template = await db.eventTemplate.findFirst({ + where: { id: templateId, congregationId }, + select: { key: true, name: true, _count: { select: { events: true } } }, }) + if (!template) return { ok: false, reason: 'not-found' } - const sourcePartsByOrder = new Map(source.parts.map(p => [p.order, p])) - const sourceServicePartsByName = new Map(source.serviceParts.map(r => [r.name, r])) + // System templates (day-off, freeform) back domain concepts looked up by + // key at runtime — createDayOff / createFreeformEvent would break if the row + // disappeared. Same guard as updateTemplate / duplicateTemplate. + if (isSystemTemplate(template.key)) return { ok: false, reason: 'system' } - for (const newPart of duplicated.parts) { - const sourcePart = sourcePartsByOrder.get(newPart.order) - if (!sourcePart) continue - const speakerRoleIds = sourcePart.allowedRoles.filter(r => r.asKind === 'speaker').map(r => r.roleId) - const readerRoleIds = sourcePart.allowedRoles.filter(r => r.asKind === 'reader').map(r => r.roleId) - if (speakerRoleIds.length > 0) { - await db.templatePartAllowedRole.createMany({ - data: speakerRoleIds.map(roleId => ({ partId: newPart.id, roleId, asKind: 'speaker', congregationId })), - skipDuplicates: true, - }) - } - if (readerRoleIds.length > 0) { - await db.templatePartAllowedRole.createMany({ - data: readerRoleIds.map(roleId => ({ partId: newPart.id, roleId, asKind: 'reader', congregationId })), - skipDuplicates: true, - }) - } - } + // Event.templateId is ON DELETE SET NULL: deleting a template still linked to + // events would orphan them (templateId → NULL), and downstream queries that + // filter on template.key silently drop null-template rows — the exact bug the + // 20260720* migration series had to repair. Refuse rather than orphan. + if (template._count.events > 0) return { ok: false, reason: 'in-use', eventCount: template._count.events } - for (const newRole of duplicated.serviceParts) { - const sourceRole = sourceServicePartsByName.get(newRole.name) - if (!sourceRole || sourceRole.allowedRoles.length === 0) continue - await db.templateServicePartAllowedRole.createMany({ - data: sourceRole.allowedRoles.map(r => ({ servicePartId: newRole.id, roleId: r.roleId, congregationId })), - skipDuplicates: true, - }) - } + // parts, serviceParts, responsibles and their allowed-role rows all cascade + // (onDelete: Cascade) from the template FK, so the single delete is enough. + await db.eventTemplate.delete({ + where: { id_congregationId: { id: templateId, congregationId } }, + }) - return duplicated + return { ok: true, name: template.name } } diff --git a/app/features/events/server/part-role-labels.integration.test.ts b/app/features/events/server/part-role-labels.integration.test.ts index e36b339f..98f639cb 100644 --- a/app/features/events/server/part-role-labels.integration.test.ts +++ b/app/features/events/server/part-role-labels.integration.test.ts @@ -8,9 +8,9 @@ import { PrismaPg } from '@prisma/adapter-pg' import { afterAll, beforeAll, describe, expect, it } from 'vitest' import { PrismaClient } from '~/database/generated/client' +import { duplicateTemplate } from '~/features/events/server/duplicate-template.server' import { applyTemplateToEvent } from '~/features/events/server/event-parts.server' import { generateEventsFromTemplate } from '~/features/events/server/event-template-generation.server' -import { duplicateTemplate } from '~/features/events/server/event-templates.server' const adapter = new PrismaPg({ connectionString: process.env.DB_RUNTIME_URL ?? process.env.DB_URL, diff --git a/app/features/settings/routes/congregation/templates/edit.tsx b/app/features/settings/routes/congregation/templates/edit.tsx index 757b81de..5a488b6c 100644 --- a/app/features/settings/routes/congregation/templates/edit.tsx +++ b/app/features/settings/routes/congregation/templates/edit.tsx @@ -2,7 +2,7 @@ import { parseWithZod } from '@conform-to/zod' import { closestCenter, DndContext, type DragEndEvent } from '@dnd-kit/core' import { SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable' import { Clock, Pencil, Plus, Trash2 } from 'lucide-react' -import { useState } from 'react' +import { Fragment, useState } from 'react' import { data, redirect, useFetcher } from 'react-router' import { commitSession, getSession } from '~/features/authentication/index.server' import { InlineDeleteDialog, isSystemTemplate, PartEditSheet, ServiceEditSheet, SortableRow } from '~/features/events' @@ -463,7 +463,7 @@ export default function TemplateEditPage({ loaderData }: Route.ComponentProps) { {partsBySection.map(group => ( - <> + {group.section && ( @@ -525,7 +525,7 @@ export default function TemplateEditPage({ loaderData }: Route.ComponentProps) { ))} - + ))} diff --git a/app/features/settings/routes/congregation/templates/view.tsx b/app/features/settings/routes/congregation/templates/view.tsx index 497a448d..5d881b41 100644 --- a/app/features/settings/routes/congregation/templates/view.tsx +++ b/app/features/settings/routes/congregation/templates/view.tsx @@ -1,8 +1,14 @@ -import { Calendar, CalendarOff, CalendarPlus, Clock, Copy, Pencil, UserCog } from 'lucide-react' -import { Form, Link, redirect } from 'react-router' +import { Calendar, CalendarOff, CalendarPlus, Clock, Copy, Pencil, Trash2, UserCog } from 'lucide-react' +import { useState } from 'react' +import { Form, Link, redirect, useFetcher } from 'react-router' import { commitSession, getSession } from '~/features/authentication/index.server' -import { dayLabel, isSystemTemplate } from '~/features/events' -import { duplicateTemplate, getTemplateById, isTemplateResponsible } from '~/features/events/index.server' +import { dayLabel, InlineDeleteDialog, isSystemTemplate } from '~/features/events' +import { + deleteTemplate, + duplicateTemplate, + getTemplateById, + isTemplateResponsible, +} from '~/features/events/index.server' import * as m from '~/i18n/paraglide/messages' import { currentAccountContext, @@ -39,26 +45,61 @@ export function loader({ params, context }: Route.LoaderArgs) { const template = await getTemplateById(db, templateId, currentUser.congregationId) if (!template) throw redirect('/settings/congregation/templates') - const responsible = await isTemplateResponsible(db, templateId, currentUser.id, currentUser.congregationId) + const [responsible, eventCount] = await Promise.all([ + isTemplateResponsible(db, templateId, currentUser.id, currentUser.congregationId), + db.event.count({ where: { templateId, congregationId: currentUser.congregationId } }), + ]) const canEdit = permissions.has(Permission.ProgramManager) || responsible != null + const isSystem = isSystemTemplate(template.key) + // Deleting a whole template is manager-only and never allowed for system + // rows; responsibles may edit content but not remove the template. + const canDelete = permissions.has(Permission.ProgramManager) && !isSystem logger.info(`Loading template view. User ID: ${currentUser.id}. Template: ${template.name}.`) - return { template, canEdit, isSystem: isSystemTemplate(template.key) } + return { template, canEdit, canDelete, eventCount, isSystem } }) } -export function action({ request, params, context }: Route.ActionArgs) { +export async function action({ request, params, context }: Route.ActionArgs) { const permissions = context.get(permissionsContext) const currentUser = context.get(currentAccountContext) const templateId = requireParamId(params.templateId, '/settings/congregation/templates') + const formData = await request.formData() + const intent = formData.get('intent') + // biome-ignore lint/complexity/noExcessiveCognitiveComplexity: handles duplicate + delete intents in a single scoped transaction return withScopeFromContext(context, async db => { const responsible = await isTemplateResponsible(db, templateId, currentUser.id, currentUser.congregationId) if (!permissions.has(Permission.ProgramManager) && !responsible) throw redirect('/settings/congregation/templates') const session = await getSession(request.headers.get('Cookie')) + + if (intent === 'delete') { + // Whole-template removal is manager-only; responsibles edit content only. + if (!permissions.has(Permission.ProgramManager)) { + throw redirect(`/settings/congregation/templates/${templateId}`) + } + const result = await deleteTemplate(db, templateId, currentUser.congregationId) + if (result.ok) { + session.flash('success', m.settings_template_view_delete_success({ name: result.name })) + logger.info(`Deleted template ${templateId}. User ID: ${currentUser.id}.`) + return redirect('/settings/congregation/templates', { + headers: { 'Set-Cookie': await commitSession(session) }, + }) + } + session.flash( + 'error', + result.reason === 'in-use' + ? m.settings_template_view_delete_in_use({ count: String(result.eventCount) }) + : m.settings_template_view_delete_error(), + ) + return redirect(`/settings/congregation/templates/${templateId}`, { + headers: { 'Set-Cookie': await commitSession(session) }, + }) + } + const copy = await duplicateTemplate(db, templateId, currentUser.congregationId) if (copy) { session.flash('success', m.settings_template_view_duplicate_success({ name: copy.name })) @@ -76,7 +117,9 @@ export function action({ request, params, context }: Route.ActionArgs) { } export default function TemplateViewPage({ loaderData }: Route.ComponentProps) { - const { template, canEdit, isSystem } = loaderData + const { template, canEdit, canDelete, eventCount, isSystem } = loaderData + const [confirmOpen, setConfirmOpen] = useState(false) + const deleteFetcher = useFetcher() return (
@@ -110,6 +153,21 @@ export default function TemplateViewPage({ loaderData }: Route.ComponentProps) { )} + {canDelete && ( + + )}
) } @@ -198,6 +256,14 @@ export default function TemplateViewPage({ loaderData }: Route.ComponentProps) { )} + + deleteFetcher.submit({ intent: 'delete' }, { method: 'post' })} + isDeleting={deleteFetcher.state !== 'idle'} + /> ) } diff --git a/app/i18n/messages/en.json b/app/i18n/messages/en.json index f59f3109..f528bd30 100644 --- a/app/i18n/messages/en.json +++ b/app/i18n/messages/en.json @@ -766,6 +766,9 @@ "settings_template_view_service_roles_title": "Service roles", "settings_template_view_duplicate_success": "Template duplicated: \"{name}\".", "settings_template_view_duplicate_error": "Unable to duplicate this template.", + "settings_template_view_delete_success": "Template deleted: \"{name}\".", + "settings_template_view_delete_error": "Unable to delete this template.", + "settings_template_view_delete_in_use": "This template is still used by {count} event(s) and cannot be deleted.", "settings_territories_card_overlays_meta_title": "Assembly map - Unitae", "settings_territories_card_overlays_title": "Assembly map", "settings_territories_card_overlays_subtitle": "Define the zones that make up your assembly's full preaching territory. Each printed territory card displays this map with a marker showing where the assigned territory sits.", diff --git a/app/i18n/messages/fr.json b/app/i18n/messages/fr.json index 870ab20e..28a95076 100644 --- a/app/i18n/messages/fr.json +++ b/app/i18n/messages/fr.json @@ -768,6 +768,9 @@ "settings_template_view_service_roles_title": "Rôles de service", "settings_template_view_duplicate_success": "Modèle dupliqué : « {name} ».", "settings_template_view_duplicate_error": "Impossible de dupliquer ce modèle.", + "settings_template_view_delete_success": "Modèle supprimé : « {name} ».", + "settings_template_view_delete_error": "Impossible de supprimer ce modèle.", + "settings_template_view_delete_in_use": "Ce modèle est encore utilisé par {count} évènement(s) et ne peut pas être supprimé.", "settings_territories_card_overlays_meta_title": "Carte de l'assemblée - Unitae", "settings_territories_card_overlays_title": "Carte de l'assemblée", "settings_territories_card_overlays_subtitle": "Définissez les zones qui composent le territoire complet de votre assemblée. Chaque fiche de territoire imprimée affiche cette carte avec un repère indiquant où se situe le territoire attribué.",