From e7225da149e62dbf3a388d7bd724b5c7f4d0a488 Mon Sep 17 00:00:00 2001 From: mindsers Date: Thu, 16 Jul 2026 00:17:40 +0200 Subject: [PATCH 1/4] feat(events): surface absence/assignment conflicts across the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layers three coordinated interventions so absence/assignment conflicts stop slipping through: - Blocking modal on the "mes absences" creation flow lists each overlapping assignment with the responsible to contact by name (falls back to a generic wording when there is none). - Amber "Conflit" / "N conflits" badge on every upcoming event card in the programme list, plus a `?hasConflicts=true` filter the dashboard card lands on. - Aggregated urgent-item card on the dashboard for template responsibles and `Permission.ProgramManager` holders — shows up to three absentee names and deep-links into the filtered event list. Also fixes a pre-existing ID-type bug in `checkDayOffConflict` and `refreshConflictFlags`: they mixed `UserAccount.id` (from `Event.createdById`) with `Member.id` (from `assigneeId` / `assistantId`), so the `hasConflict` invariant was only maintained by coincidence for users whose `Member.id` happened to equal their `UserAccount.id`. They now join through `Event.createdBy.memberId` and filter assignments consistently by `Member.id`; `days-off` callers pass `currentUser.member?.id` and skip the refresh for accounts with no linked member. Broadens the overlap lookup to untemplated events at the same time (excluding Off events themselves). Regression pins: - `refreshConflictFlags` filters part / service assignments by `memberId`, includes untemplated events, and excludes Off events. - `checkDayOffConflict` joins via `createdBy.memberId`. - `getResponsibleConflicts` self-heals when `hasConflict` clears on the underlying assignment; `ProgramManager` sees conflicts on untemplated events even without template responsibility. --- app/features/dashboard/routes/index.tsx | 58 +++-- .../server/dashboard.integration.test.ts | 137 +++++++++++- .../get-responsible-conflicts.server.test.ts | 203 ++++++++++++++++++ .../get-responsible-conflicts.server.ts | 86 ++++++++ .../dashboard/ui/build-urgent-items.test.ts | 81 ++++++- .../dashboard/ui/build-urgent-items.ts | 38 +++- .../events/routes/days-off/delete.tsx | 2 +- app/features/events/routes/days-off/new.tsx | 41 +++- app/features/events/routes/programs/list.tsx | 137 ++++++++---- .../events/server/days-off.server.test.ts | 41 ++-- app/features/events/server/days-off.server.ts | 26 ++- .../server/event-filters.server.test.ts | 24 +++ .../events/server/event-filters.server.ts | 18 ++ ...ist-user-conflicts-in-range.server.test.ts | 162 ++++++++++++++ .../list-user-conflicts-in-range.server.ts | 114 ++++++++++ .../programme-assignments.server.test.ts | 68 ++++++ .../server/programme-assignments.server.ts | 24 ++- .../events/ui/DayOffConflictModal.tsx | 64 ++++++ .../ui/day-off-conflict-helpers.test.ts | 31 +++ .../events/ui/day-off-conflict-helpers.ts | 8 + app/i18n/messages/en.json | 12 ++ app/i18n/messages/fr.json | 12 ++ 22 files changed, 1286 insertions(+), 101 deletions(-) create mode 100644 app/features/dashboard/server/get-responsible-conflicts.server.test.ts create mode 100644 app/features/dashboard/server/get-responsible-conflicts.server.ts create mode 100644 app/features/events/server/list-user-conflicts-in-range.server.test.ts create mode 100644 app/features/events/server/list-user-conflicts-in-range.server.ts create mode 100644 app/features/events/ui/DayOffConflictModal.tsx create mode 100644 app/features/events/ui/day-off-conflict-helpers.test.ts create mode 100644 app/features/events/ui/day-off-conflict-helpers.ts diff --git a/app/features/dashboard/routes/index.tsx b/app/features/dashboard/routes/index.tsx index 3c52146b..54dabe97 100644 --- a/app/features/dashboard/routes/index.tsx +++ b/app/features/dashboard/routes/index.tsx @@ -10,6 +10,7 @@ import { getUserTerritories, type TerritoryStatus, } from '~/features/dashboard/server/dashboard.server' +import { getResponsibleConflicts } from '~/features/dashboard/server/get-responsible-conflicts.server' import { buildUrgentItems } from '~/features/dashboard/ui/build-urgent-items' import { OnboardingChecklist } from '~/features/dashboard/ui/OnboardingChecklist' import * as m from '~/i18n/paraglide/messages' @@ -43,6 +44,7 @@ export function loader({ context }: Route.LoaderArgs) { const permissions = context.get(permissionsContext) const isAdmin = permissions.has(Permission.Admin) const isTerritoriesManager = permissions.has(Permission.TerritoriesManager) + const isProgramManager = permissions.has(Permission.ProgramManager) const canViewBoard = permissions.has(Permission.BoardViewer) // Member-bound queries (territories, programme assignments) need the linked @@ -54,25 +56,33 @@ export function loader({ context }: Route.LoaderArgs) { return safeQuery(label, currentUser.id, () => run(memberId)) } - const [territories, recentDocuments, unreadDocumentCount, absences, nextMeeting, dayoffConflict] = - await Promise.all([ - memberSafeQuery('territories', mid => getUserTerritories(db, mid)), - canViewBoard - ? safeQuery('documents', currentUser.id, () => - getRecentDocuments(db, currentUser.id, currentUser.congregationId), - ) - : Promise.resolve(null), - canViewBoard - ? safeQuery('unread-count', currentUser.id, () => - getUnreadDocumentCount(db, currentUser.id, currentUser.congregationId), - ) - : Promise.resolve(0), - safeQuery('absences', currentUser.id, () => - getUpcomingAbsences(db, currentUser.id, currentUser.congregationId), - ), - memberSafeQuery('next-meeting', mid => getNextMeeting(db, mid)), - memberSafeQuery('dayoff-conflict', mid => getConflictingAssignments(db, mid)), - ]) + const [ + territories, + recentDocuments, + unreadDocumentCount, + absences, + nextMeeting, + dayoffConflict, + responsibleConflicts, + ] = await Promise.all([ + memberSafeQuery('territories', mid => getUserTerritories(db, mid)), + canViewBoard + ? safeQuery('documents', currentUser.id, () => + getRecentDocuments(db, currentUser.id, currentUser.congregationId), + ) + : Promise.resolve(null), + canViewBoard + ? safeQuery('unread-count', currentUser.id, () => + getUnreadDocumentCount(db, currentUser.id, currentUser.congregationId), + ) + : Promise.resolve(0), + safeQuery('absences', currentUser.id, () => getUpcomingAbsences(db, currentUser.id, currentUser.congregationId)), + memberSafeQuery('next-meeting', mid => getNextMeeting(db, mid)), + memberSafeQuery('dayoff-conflict', mid => getConflictingAssignments(db, mid)), + safeQuery('responsible-conflicts', currentUser.id, () => + getResponsibleConflicts(db, currentUser.id, isProgramManager), + ), + ]) // Onboarding: count entities for admin checklist let onboarding = null @@ -99,6 +109,7 @@ export function loader({ context }: Route.LoaderArgs) { nextMeeting, absences, dayoffConflict, + responsibleConflicts, onboarding, isAdmin, isTerritoriesManager, @@ -135,6 +146,7 @@ export default function Dashboard({ loaderData }: Route.ComponentProps) { nextMeeting, absences, dayoffConflict, + responsibleConflicts, onboarding, isAdmin, isTerritoriesManager, @@ -148,7 +160,13 @@ export default function Dashboard({ loaderData }: Route.ComponentProps) { }) // Build urgent items from across features - const urgentItems = buildUrgentItems(territories, unreadDocumentCount, nextMeeting, dayoffConflict) + const urgentItems = buildUrgentItems( + territories, + unreadDocumentCount, + nextMeeting, + dayoffConflict, + responsibleConflicts, + ) return (
diff --git a/app/features/dashboard/server/dashboard.integration.test.ts b/app/features/dashboard/server/dashboard.integration.test.ts index da008ad6..ac88bf8d 100644 --- a/app/features/dashboard/server/dashboard.integration.test.ts +++ b/app/features/dashboard/server/dashboard.integration.test.ts @@ -3,7 +3,6 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest' import { PrismaClient } from '~/database/generated/client' import { EventKind } from '~/features/events/model/event-kind.type' import { TerritoryKind } from '~/features/territories/model/territory-kind.type' -import { PublisherType } from '~/shared/types/publisher-type' const adapter = new PrismaPg({ connectionString: process.env.DB_RUNTIME_URL ?? process.env.DB_URL, @@ -219,6 +218,7 @@ afterAll(async () => { const { getUserTerritories, getRecentDocuments, getUnreadDocumentCount, getNextMeeting, getConflictingAssignments } = await import('./dashboard.server') +const { getResponsibleConflicts } = await import('./get-responsible-conflicts.server') const { refreshConflictFlags } = await import('~/features/events/server/programme-assignments.server') // --- Tests --- @@ -669,4 +669,139 @@ describe('getConflictingAssignments (integration)', () => { }) } }) + + // Invariant pin (responsible side): the aggregated responsible-conflict + // card is correct only as long as the hasConflict flag remains authoritative. + // Once refreshConflictFlags (or any direct write) clears the flag, the + // responsible's card must disappear on the next read — no stale cache, no + // extra invalidation step. This mirrors the absentee-side pin at line 578. + it('getResponsibleConflicts drops the entry when hasConflict clears on the underlying assignment', async () => { + const setup = await withScope(congregationId, async tx => { + const eventKind = await tx.eventKind.findFirstOrThrow({ + where: { congregationId, key: { not: EventKind.Off } }, + }) + const template = await tx.programmeTemplate.create({ + data: { + name: 'Responsible Invariant Template', + key: `resp-invariant-template-${ts}`, + kindId: eventKind.id, + congregationId, + }, + }) + // Bob is the responsible for this template; Alice is the absentee. + await tx.programmeTemplateResponsible.create({ + data: { + templateId: template.id, + userId: bobAccountId, + congregationId, + }, + }) + const event = await tx.event.create({ + data: { + name: 'Responsible Invariant Event', + kindId: eventKind.id, + templateId: template.id, + startDate: new Date('2028-01-05T19:00:00Z'), + endDate: new Date('2028-01-05T21:00:00Z'), + createdById: aliceAccountId, + congregationId, + }, + }) + const part = await tx.programmePartAssignment.create({ + data: { + eventId: event.id, + assigneeId: aliceId, + name: 'Discours', + section: 'main', + order: 1, + hasConflict: true, + congregationId, + }, + }) + return { templateId: template.id, eventId: event.id, partId: part.id } + }) + + try { + // Bob is the responsible; he sees the outstanding conflict on his template. + const before = await withScope(congregationId, tx => getResponsibleConflicts(tx, bobAccountId, false)) + expect(before.count).toBe(1) + expect(before.absenteeNames).toEqual(['Alice Dupont']) + + // Simulate resolution: the underlying assignment is no longer in conflict + // (either the absence went away or the assignment was reassigned). + await withScope(congregationId, tx => + tx.programmePartAssignment.update({ + where: { id_congregationId: { id: setup.partId, congregationId } }, + data: { hasConflict: false }, + }), + ) + + const after = await withScope(congregationId, tx => getResponsibleConflicts(tx, bobAccountId, false)) + expect(after).toEqual({ count: 0, absenteeNames: [], additionalAbsenteesCount: 0 }) + } finally { + await withScope(congregationId, async tx => { + await tx.programmePartAssignment.delete({ + where: { id_congregationId: { id: setup.partId, congregationId } }, + }) + await tx.event.delete({ where: { id_congregationId: { id: setup.eventId, congregationId } } }) + await tx.programmeTemplateResponsible.deleteMany({ where: { templateId: setup.templateId } }) + await tx.programmeTemplate.delete({ + where: { id_congregationId: { id: setup.templateId, congregationId } }, + }) + }) + } + }) + + // A ProgramManager should see conflicts on events they don't own via a + // template responsibility — including untemplated events, which have no + // responsibles at all. This pins the "manager sees everything" branch of + // the filter (the non-manager path is covered by unit tests). + it('getResponsibleConflicts includes untemplated events for ProgramManager users', async () => { + const setup = await withScope(congregationId, async tx => { + const eventKind = await tx.eventKind.findFirstOrThrow({ + where: { congregationId, key: { not: EventKind.Off } }, + }) + const event = await tx.event.create({ + data: { + name: 'Untemplated Manager Event', + kindId: eventKind.id, + templateId: null, + startDate: new Date('2028-02-10T19:00:00Z'), + endDate: new Date('2028-02-10T21:00:00Z'), + createdById: aliceAccountId, + congregationId, + }, + }) + const part = await tx.programmePartAssignment.create({ + data: { + eventId: event.id, + assigneeId: aliceId, + name: 'Custom part', + section: 'main', + order: 1, + hasConflict: true, + congregationId, + }, + }) + return { eventId: event.id, partId: part.id } + }) + + try { + // Bob is neither a template responsible nor a manager — must see nothing. + const nonManager = await withScope(congregationId, tx => getResponsibleConflicts(tx, bobAccountId, false)) + expect(nonManager.count).toBe(0) + + // ProgramManager path — same query, isProgramManager=true — must include it. + const asManager = await withScope(congregationId, tx => getResponsibleConflicts(tx, bobAccountId, true)) + expect(asManager.count).toBe(1) + expect(asManager.absenteeNames).toEqual(['Alice Dupont']) + } finally { + await withScope(congregationId, async tx => { + await tx.programmePartAssignment.delete({ + where: { id_congregationId: { id: setup.partId, congregationId } }, + }) + await tx.event.delete({ where: { id_congregationId: { id: setup.eventId, congregationId } } }) + }) + } + }) }) diff --git a/app/features/dashboard/server/get-responsible-conflicts.server.test.ts b/app/features/dashboard/server/get-responsible-conflicts.server.test.ts new file mode 100644 index 00000000..f6201c93 --- /dev/null +++ b/app/features/dashboard/server/get-responsible-conflicts.server.test.ts @@ -0,0 +1,203 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('~/shared/infra/db.server', () => ({ + unscopedDb: { + programmePartAssignment: { findMany: vi.fn() }, + programmeServiceRoleAssignment: { findMany: vi.fn() }, + }, +})) + +const { getResponsibleConflicts } = await import('./get-responsible-conflicts.server') +const { unscopedDb: db } = await import('~/shared/infra/db.server') + +beforeEach(() => { + vi.resetAllMocks() + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([] as never) + vi.mocked(db.programmeServiceRoleAssignment.findMany).mockResolvedValue([] as never) +}) + +describe('getResponsibleConflicts', () => { + it('returns zero count with no names when no conflicts exist', async () => { + const result = await getResponsibleConflicts(db, 42, false) + expect(result).toEqual({ count: 0, absenteeNames: [], additionalAbsenteesCount: 0 }) + }) + + // Non-manager users only see conflicts on templates they are the + // responsible for. The join goes through + // event.template.responsibles.some.userId. + it('scopes the query to templates the user is responsible for (non-manager)', async () => { + const userId = 100 + await getResponsibleConflicts(db, userId, false) + + const partCall = vi.mocked(db.programmePartAssignment.findMany).mock.calls[0][0] + const partWhere = partCall?.where as Record + expect(partWhere.hasConflict).toBe(true) + expect(partWhere.event).toEqual({ + startDate: { gte: expect.any(Date) }, + template: { responsibles: { some: { userId } } }, + }) + }) + + it('scopes the service-role query with the same template filter (non-manager)', async () => { + const userId = 100 + await getResponsibleConflicts(db, userId, false) + + const serviceCall = vi.mocked(db.programmeServiceRoleAssignment.findMany).mock.calls[0][0] + const serviceWhere = serviceCall?.where as Record + expect(serviceWhere.event).toEqual({ + startDate: { gte: expect.any(Date) }, + template: { responsibles: { some: { userId } } }, + }) + }) + + // ProgramManager sees everything — including untemplated events (which + // have no responsible relation at all). + it('drops the template filter for ProgramManager users', async () => { + await getResponsibleConflicts(db, 100, true) + + const partCall = vi.mocked(db.programmePartAssignment.findMany).mock.calls[0][0] + const partWhere = partCall?.where as Record + expect(partWhere.event).toEqual({ startDate: { gte: expect.any(Date) } }) + expect(partWhere.event).not.toHaveProperty('template') + }) + + it('only considers upcoming events (startDate >= now)', async () => { + await getResponsibleConflicts(db, 100, true) + + const partCall = vi.mocked(db.programmePartAssignment.findMany).mock.calls[0][0] + const event = (partCall?.where as { event: { startDate: { gte: Date } } }).event + expect(event.startDate.gte.getTime()).toBeGreaterThan(Date.now() - 5000) + }) + + // Two rows for the same member on the same event (one row lists them as + // assignee, another where they're the assistant on a different part of + // the same event) collapse to one conflict. Otherwise a member appearing + // twice double-counts the badge. + it('dedupes by (memberId, eventId) when computing count', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + eventId: 1, + assigneeId: 100, + assignee: { firstname: 'Alice', lastname: 'Dupont' }, + assistantId: null, + assistant: null, + }, + { + eventId: 1, + assigneeId: null, + assignee: null, + assistantId: 100, + assistant: { firstname: 'Alice', lastname: 'Dupont' }, + }, + ] as never) + + const result = await getResponsibleConflicts(db, 999, true) + expect(result.count).toBe(1) + expect(result.absenteeNames).toEqual(['Alice Dupont']) + }) + + it('counts one conflict per (member × event) — same member on two events counts twice', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + eventId: 1, + assigneeId: 100, + assignee: { firstname: 'Alice', lastname: 'Dupont' }, + assistantId: null, + assistant: null, + }, + { + eventId: 2, + assigneeId: 100, + assignee: { firstname: 'Alice', lastname: 'Dupont' }, + assistantId: null, + assistant: null, + }, + ] as never) + + const result = await getResponsibleConflicts(db, 999, true) + expect(result.count).toBe(2) + expect(result.absenteeNames).toEqual(['Alice Dupont']) + }) + + it('merges names across part and service assignments (deduped)', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + eventId: 1, + assigneeId: 100, + assignee: { firstname: 'Alice', lastname: 'Dupont' }, + assistantId: null, + assistant: null, + }, + ] as never) + vi.mocked(db.programmeServiceRoleAssignment.findMany).mockResolvedValue([ + { eventId: 2, assigneeId: 100, assignee: { firstname: 'Alice', lastname: 'Dupont' } }, + ] as never) + + const result = await getResponsibleConflicts(db, 999, true) + expect(result.absenteeNames).toEqual(['Alice Dupont']) + expect(result.count).toBe(2) + }) + + it('caps absenteeNames at 3, sorted alphabetically; count still reflects all conflicts', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + eventId: 1, + assigneeId: 100, + assignee: { firstname: 'Charlie', lastname: 'C' }, + assistantId: null, + assistant: null, + }, + { + eventId: 2, + assigneeId: 200, + assignee: { firstname: 'Alice', lastname: 'A' }, + assistantId: null, + assistant: null, + }, + { + eventId: 3, + assigneeId: 300, + assignee: { firstname: 'Bob', lastname: 'B' }, + assistantId: null, + assistant: null, + }, + { + eventId: 4, + assigneeId: 400, + assignee: { firstname: 'Diane', lastname: 'D' }, + assistantId: null, + assistant: null, + }, + { + eventId: 5, + assigneeId: 500, + assignee: { firstname: 'Eve', lastname: 'E' }, + assistantId: null, + assistant: null, + }, + ] as never) + + const result = await getResponsibleConflicts(db, 999, true) + expect(result.absenteeNames).toEqual(['Alice A', 'Bob B', 'Charlie C']) + expect(result.count).toBe(5) + expect(result.additionalAbsenteesCount).toBe(2) + }) + + // A part row can carry both an assignee and an assistant — both are + // potentially the absentee, so both are enumerated. Downstream the + // responsible sees them and can click through for detail. + it('includes both assignee and assistant of a part row', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + eventId: 1, + assigneeId: 100, + assignee: { firstname: 'Speaker', lastname: 'One' }, + assistantId: 200, + assistant: { firstname: 'Reader', lastname: 'Two' }, + }, + ] as never) + + const result = await getResponsibleConflicts(db, 999, true) + expect(result.absenteeNames.sort()).toEqual(['Reader Two', 'Speaker One']) + }) +}) diff --git a/app/features/dashboard/server/get-responsible-conflicts.server.ts b/app/features/dashboard/server/get-responsible-conflicts.server.ts new file mode 100644 index 00000000..ad0a179d --- /dev/null +++ b/app/features/dashboard/server/get-responsible-conflicts.server.ts @@ -0,0 +1,86 @@ +import type { TransactionClient } from '~/shared/infra/db.server' +import { fullName } from '~/shared/utils/display-name' + +export interface ResponsibleConflictsSummary { + // Number of (member × event) conflict pairs. Powers the headline count. + count: number + // Up to MAX_ABSENTEE_NAMES unique absentee display names, sorted alphabetically. + absenteeNames: string[] + // How many unique absentees exist beyond the ones listed above. 0 when + // every unique absentee fits in `absenteeNames`. The label formatter + // uses this to append "(+N autres)". + additionalAbsenteesCount: number +} + +const MAX_ABSENTEE_NAMES = 3 + +// Aggregated counts of upcoming programme assignments flagged as +// day-off conflicts, scoped by who can see them: +// - non-manager: only events on templates where the user is the +// documented responsible (ProgrammeTemplateResponsible.userId). +// - ProgramManager: all events, including untemplated ones which have +// no responsible relation at all. +// Returns a count (unique member × event pairs) and up to three +// absentee names, sorted alphabetically, for at-a-glance triage on +// the dashboard. +export async function getResponsibleConflicts( + db: TransactionClient, + userId: number, + isProgramManager: boolean, +): Promise { + const now = new Date() + + const eventFilter = isProgramManager + ? { startDate: { gte: now } } + : { startDate: { gte: now }, template: { responsibles: { some: { userId } } } } + + const [partRows, serviceRows] = await Promise.all([ + db.programmePartAssignment.findMany({ + where: { hasConflict: true, event: eventFilter }, + select: { + eventId: true, + assigneeId: true, + assignee: { select: { firstname: true, lastname: true } }, + assistantId: true, + assistant: { select: { firstname: true, lastname: true } }, + }, + }), + db.programmeServiceRoleAssignment.findMany({ + where: { hasConflict: true, event: eventFilter }, + select: { + eventId: true, + assigneeId: true, + assignee: { select: { firstname: true, lastname: true } }, + }, + }), + ]) + + const pairs = new Set() + const nameByMemberId = new Map() + + const record = ( + memberId: number | null, + member: { firstname: string | null; lastname: string | null } | null, + eventId: number, + ) => { + if (memberId == null || member == null) return + pairs.add(`${memberId}:${eventId}`) + if (!nameByMemberId.has(memberId)) { + nameByMemberId.set(memberId, fullName(member)) + } + } + + for (const row of partRows) { + record(row.assigneeId, row.assignee, row.eventId) + record(row.assistantId, row.assistant, row.eventId) + } + for (const row of serviceRows) { + record(row.assigneeId, row.assignee, row.eventId) + } + + const sortedNames = [...nameByMemberId.values()].sort((a, b) => a.localeCompare(b)) + const absenteeNames = sortedNames.slice(0, MAX_ABSENTEE_NAMES) + const additionalAbsenteesCount = Math.max(0, sortedNames.length - MAX_ABSENTEE_NAMES) + + return { count: pairs.size, absenteeNames, additionalAbsenteesCount } +} diff --git a/app/features/dashboard/ui/build-urgent-items.test.ts b/app/features/dashboard/ui/build-urgent-items.test.ts index a46111fa..c8100697 100644 --- a/app/features/dashboard/ui/build-urgent-items.test.ts +++ b/app/features/dashboard/ui/build-urgent-items.test.ts @@ -10,6 +10,10 @@ vi.mock('~/i18n/paraglide/messages', () => ({ `${name} — ${eventName}`, dashboard_urgent_dayoff_conflict: ({ eventName }: { eventName: string }) => `Conflict with ${eventName}`, dashboard_urgent_unread_documents: ({ count }: { count: string }) => `${count} unread documents`, + dashboard_urgent_responsible_conflict_singular: ({ names }: { names: string }) => `1 responsible conflict: ${names}`, + dashboard_urgent_responsible_conflict_plural: ({ count, names }: { count: string; names: string }) => + `${count} responsible conflicts: ${names}`, + dashboard_urgent_responsible_conflict_extras: ({ count }: { count: string }) => ` (+${count} more)`, })) const { @@ -18,7 +22,9 @@ const { urgentPartAssignmentItems, urgentServiceRoleItems, urgentDayoffConflictItems, + urgentResponsibleConflictItems, urgentDocumentsItem, + formatResponsibleConflictLabel, } = await import('./build-urgent-items') beforeEach(() => { @@ -258,6 +264,60 @@ describe('urgentDayoffConflictItems', () => { }) }) +// --- urgentResponsibleConflictItems --- + +describe('formatResponsibleConflictLabel', () => { + it('uses the singular template with a single name when count is 1', () => { + const label = formatResponsibleConflictLabel({ + count: 1, + absenteeNames: ['Marie Dupont'], + additionalAbsenteesCount: 0, + }) + expect(label).toBe('1 responsible conflict: Marie Dupont') + }) + + it('uses the plural template with all names joined by comma', () => { + const label = formatResponsibleConflictLabel({ + count: 3, + absenteeNames: ['Alice A', 'Bob B', 'Charlie C'], + additionalAbsenteesCount: 0, + }) + expect(label).toBe('3 responsible conflicts: Alice A, Bob B, Charlie C') + }) + + it('appends "(+N more)" when there are additional unlisted absentees', () => { + const label = formatResponsibleConflictLabel({ + count: 5, + absenteeNames: ['Alice A', 'Bob B', 'Charlie C'], + additionalAbsenteesCount: 2, + }) + expect(label).toBe('5 responsible conflicts: Alice A, Bob B, Charlie C (+2 more)') + }) +}) + +describe('urgentResponsibleConflictItems', () => { + it('returns empty array when the summary is null', () => { + expect(urgentResponsibleConflictItems(null)).toEqual([]) + }) + + it('returns empty array when the summary count is zero', () => { + expect(urgentResponsibleConflictItems({ count: 0, absenteeNames: [], additionalAbsenteesCount: 0 })).toEqual([]) + }) + + it('returns one item with priority 1 and a deep-link to the filtered programme list', () => { + const items = urgentResponsibleConflictItems({ + count: 2, + absenteeNames: ['Marie D.', 'Jean P.'], + additionalAbsenteesCount: 0, + }) + expect(items).toHaveLength(1) + expect(items[0].priority).toBe(1) + expect(items[0].to).toBe('/programs?hasConflicts=true') + expect(items[0].label).toContain('Marie D.') + expect(items[0].key).toBe('responsible-conflicts') + }) +}) + // --- urgentDocumentsItem --- describe('urgentDocumentsItem', () => { @@ -282,12 +342,12 @@ describe('urgentDocumentsItem', () => { describe('buildUrgentItems', () => { it('returns empty array when all inputs are null/empty', () => { - expect(buildUrgentItems(null, null, null, null)).toEqual([]) + expect(buildUrgentItems(null, null, null, null, null)).toEqual([]) }) it('returns items sorted by priority', () => { const territories = [makeTerritory(1, 'T-1', 'overdue', new Date(2026, 3, 20))] - const items = buildUrgentItems(territories, 5, null, null) + const items = buildUrgentItems(territories, 5, null, null, null) expect(items[0].priority).toBeLessThan(items[1].priority) }) @@ -298,7 +358,7 @@ describe('buildUrgentItems', () => { makeTerritory(3, 'T-3', 'due-soon', new Date(2026, 4, 1)), makeTerritory(4, 'T-4', 'due-soon', new Date(2026, 4, 2)), ] - const items = buildUrgentItems(territories, 10, null, null) + const items = buildUrgentItems(territories, 10, null, null, null) expect(items).toHaveLength(3) }) @@ -310,7 +370,7 @@ describe('buildUrgentItems', () => { { id: 10, name: 'Discours', section: 'main', topic: '', order: 1, assignee: null, assistant: null }, ], }) - const items = buildUrgentItems(territories, null, meeting, null) + const items = buildUrgentItems(territories, null, meeting, null, null) expect(items[0].key).toContain('part-') expect(items[1].key).toContain('territory-overdue-') }) @@ -326,8 +386,19 @@ describe('buildUrgentItems', () => { serviceRoleAssignments: [{ id: 5, name: 'Son', assignee: null }], }) const conflict = makeConflict(7, 'Réunion de semaine', meetingDate) - const items = buildUrgentItems(null, null, meeting, conflict) + const items = buildUrgentItems(null, null, meeting, conflict, null) const priorities = items.map(i => i.priority) expect(priorities).toEqual([0, 2, 3]) }) + + it('includes the responsible-conflict card at priority 1 when the summary has conflicts', () => { + const items = buildUrgentItems(null, null, null, null, { + count: 3, + absenteeNames: ['Marie D.', 'Jean P.'], + additionalAbsenteesCount: 0, + }) + expect(items).toHaveLength(1) + expect(items[0].priority).toBe(1) + expect(items[0].key).toBe('responsible-conflicts') + }) }) diff --git a/app/features/dashboard/ui/build-urgent-items.ts b/app/features/dashboard/ui/build-urgent-items.ts index cf506eab..8e5ead81 100644 --- a/app/features/dashboard/ui/build-urgent-items.ts +++ b/app/features/dashboard/ui/build-urgent-items.ts @@ -1,10 +1,11 @@ -import { CalendarOff, FileText, MapPin, Mic } from 'lucide-react' +import { AlertTriangle, CalendarOff, FileText, MapPin, Mic } from 'lucide-react' import type { getConflictingAssignments, getNextMeeting, getUserTerritories, } from '~/features/dashboard/server/dashboard.server' +import type { ResponsibleConflictsSummary } from '~/features/dashboard/server/get-responsible-conflicts.server' import * as m from '~/i18n/paraglide/messages' import { THREE_DAYS_MS } from '~/shared/constants/limits' @@ -129,17 +130,52 @@ export function urgentDocumentsItem(unreadCount: number | null): UrgentItem[] { ] } +// Compose the human-readable label for the responsible-conflict card. +// Kept pure and exported so it can be unit-tested without JSX and reused +// if we ever surface the same summary elsewhere. +export function formatResponsibleConflictLabel(summary: ResponsibleConflictsSummary): string { + const namesJoined = summary.absenteeNames.join(', ') + const extras = + summary.additionalAbsenteesCount > 0 + ? m.dashboard_urgent_responsible_conflict_extras({ count: String(summary.additionalAbsenteesCount) }) + : '' + const names = namesJoined + extras + + if (summary.count === 1) { + return m.dashboard_urgent_responsible_conflict_singular({ names }) + } + return m.dashboard_urgent_responsible_conflict_plural({ count: String(summary.count), names }) +} + +export function urgentResponsibleConflictItems(summary: ResponsibleConflictsSummary | null): UrgentItem[] { + if (!summary || summary.count === 0) return [] + + return [ + { + key: 'responsible-conflicts', + label: formatResponsibleConflictLabel(summary), + to: '/programs?hasConflicts=true', + icon: AlertTriangle, + borderClass: 'border-l-amber-500 bg-amber-500/5', + iconClass: 'text-amber-600 dark:text-amber-400', + priority: 1, + }, + ] +} + export function buildUrgentItems( territories: Territories, unreadDocumentCount: number | null, nextMeeting: NextMeeting, dayoffConflict: DayoffConflict, + responsibleConflicts: ResponsibleConflictsSummary | null, ): UrgentItem[] { const items = [ ...urgentTerritoriesItems(territories), ...urgentPartAssignmentItems(nextMeeting), ...urgentServiceRoleItems(nextMeeting), ...urgentDayoffConflictItems(dayoffConflict), + ...urgentResponsibleConflictItems(responsibleConflicts), ...urgentDocumentsItem(unreadDocumentCount), ] items.sort((a, b) => a.priority - b.priority) diff --git a/app/features/events/routes/days-off/delete.tsx b/app/features/events/routes/days-off/delete.tsx index 8baa76f7..e3790134 100644 --- a/app/features/events/routes/days-off/delete.tsx +++ b/app/features/events/routes/days-off/delete.tsx @@ -76,7 +76,7 @@ export async function action({ request, params, context }: Route.ActionArgs) { const deletedEvent = await deleteDayOff( db, requireParamId(params.eventId, '/me/days-off'), - currentUser.id, + currentUser.member?.id ?? null, congregationId, ) diff --git a/app/features/events/routes/days-off/new.tsx b/app/features/events/routes/days-off/new.tsx index 80bf84c7..1f78f676 100644 --- a/app/features/events/routes/days-off/new.tsx +++ b/app/features/events/routes/days-off/new.tsx @@ -1,9 +1,11 @@ import { useState } from 'react' -import { Form, redirect } from 'react-router' +import { data, Form, redirect, useActionData, useNavigate } from 'react-router' import { commitSession, getSession } from '~/features/authentication/index.server' import { createDayOff } from '~/features/events/server/days-off.server' +import { listUserConflictsInRange } from '~/features/events/server/list-user-conflicts-in-range.server' +import { DayOffConflictModal } from '~/features/events/ui/DayOffConflictModal' import * as m from '~/i18n/paraglide/messages' -import { currentAccountContext, withScopeFromContext } from '~/shared/auth/route-context.server' +import { congregationContext, currentAccountContext, withScopeFromContext } from '~/shared/auth/route-context.server' import logger from '~/shared/infra/logger.server' import { Card, CardContent } from '~/shared/ui/card' import { useUnsavedChanges } from '~/shared/ui/hooks/use-unsaved-changes' @@ -20,22 +22,30 @@ export const meta: Route.MetaFunction = () => { export async function loader({ request, context }: Route.LoaderArgs) { const currentUser = context.get(currentAccountContext) + const congregation = context.get(congregationContext) const session = await getSession(request.headers.get('Cookie')) logger.info(`Loading personal Days Off form. User ID: ${currentUser.id}.`) return { user: currentUser, + timezone: congregation.timezone, error: session.get('error'), } } -export default function DaysOffPage() { +export default function DaysOffPage({ loaderData }: Route.ComponentProps) { const [startDate, setStartDate] = useState('') const { blocker, markDirty } = useUnsavedChanges() + const actionData = useActionData() + const navigate = useNavigate() + const [modalDismissed, setModalDismissed] = useState(false) const minimumEndDate = new Date(startDate.length > 0 ? startDate : new Date().toISOString().split('T')[0]) minimumEndDate.setDate(minimumEndDate.getDate() + 1) + const conflicts = actionData?.conflicts ?? null + const showModal = conflicts != null && conflicts.length > 0 && !modalDismissed + return (
@@ -75,6 +85,18 @@ export default function DaysOffPage() { + + {conflicts != null && ( + { + setModalDismissed(true) + navigate('/me/days-off') + }} + /> + )}
) } @@ -89,9 +111,10 @@ export async function action({ request, context }: Route.ActionArgs) { logger.info(`Creating new days off. User ID: ${currentUser.id}.`) const { congregationId } = currentUser + const memberId = currentUser.member?.id ?? null return withScopeFromContext(context, async db => { - const event = await createDayOff(db, currentUser.id, startDate, endDate, congregationId) + const event = await createDayOff(db, currentUser.id, memberId, startDate, endDate, congregationId) if (event == null) { session.flash('error', m.days_off_new_invalid_dates()) logger.info(`Failed to creating new days off. User ID: ${currentUser.id}.`) @@ -103,6 +126,16 @@ export async function action({ request, context }: Route.ActionArgs) { }) } + // Surface any pre-existing programme assignments that overlap this absence + // so the user is prompted to reach out before the meeting. When there are + // conflicts we skip the success flash — the modal is the confirmation. + const conflicts = memberId != null ? await listUserConflictsInRange(db, memberId, startDate, endDate) : [] + + if (conflicts.length > 0) { + logger.info(`Days off created with ${conflicts.length} conflict(s). User ID: ${currentUser.id}.`) + return data({ conflicts }) + } + session.flash('success', m.days_off_new_success()) logger.info(`Successfuly created new days off. User ID: ${currentUser.id}.`) diff --git a/app/features/events/routes/programs/list.tsx b/app/features/events/routes/programs/list.tsx index 104fa6a0..00d90c86 100644 --- a/app/features/events/routes/programs/list.tsx +++ b/app/features/events/routes/programs/list.tsx @@ -1,4 +1,13 @@ -import { CalendarOff, ChevronRight, FileDown, Loader2, MoreHorizontal, Trash2, UserCog } from 'lucide-react' +import { + AlertTriangle, + CalendarOff, + ChevronRight, + FileDown, + Loader2, + MoreHorizontal, + Trash2, + UserCog, +} from 'lucide-react' import { useEffect, useRef, useState } from 'react' import { Link, redirect, useFetcher } from 'react-router' import { EventKind } from '~/features/events/model/event-kind.type' @@ -69,13 +78,23 @@ export function loader({ request, context }: Route.LoaderArgs) { congregationId, NOT: { kind: { key: EventKind.Off } }, }, - include: { template: true, kind: true }, + include: { + template: true, + kind: true, + _count: { + select: { + partAssignments: { where: { hasConflict: true } }, + serviceRoleAssignments: { where: { hasConflict: true } }, + }, + }, + }, orderBy: { startDate: 'asc' }, }) const upcomingEvents = events.map(event => ({ ...event, canEdit: isProgramManager || (event.templateId != null && responsibleTemplateIdSet.has(event.templateId)), + conflictCount: event._count.partAssignments + event._count.serviceRoleAssignments, })) return { @@ -128,6 +147,75 @@ function eventTimeOrEmpty(date: Date, timezone: string): string { return time === '00:00' ? '' : time } +function ConflictBadge({ count }: { count: number }) { + const label = + count === 1 + ? m.programs_event_conflict_badge_singular() + : m.programs_event_conflict_badge_plural({ count: String(count) }) + return ( + + + ) +} + +function EventRow({ + event, + timezone, + selected, + onToggleSelection, +}: { + event: Event + timezone: string + selected: boolean + onToggleSelection: () => void +}) { + const weekday = formatEventDate(event.startDate, timezone, 'fr-FR', { weekday: 'long' }) + const time = eventTimeOrEmpty(new Date(event.startDate), timezone) + const isUpcoming = new Date(event.startDate) >= new Date() + const showConflictBadge = isUpcoming && event.conflictCount > 0 + const cardStyle = event.kind?.color ? { borderLeftColor: event.kind.color, borderLeftWidth: '4px' } : {} + const kindLabel = event.kind ? ` · ${event.kind.name}` : '' + const timeLabel = time ? ` · ${time}` : '' + + return ( +
+ {event.canEdit && ( + e.stopPropagation()} + /> + )} + + + +
+ {event.name} + + {weekday} + {timeLabel} + {kindLabel} + +
+
+ {showConflictBadge && } + +
+ + + +
+ ) +} + function WeekCheckbox({ events, selectedIds, @@ -310,42 +398,15 @@ export default function ProgramListPage({ loaderData }: Route.ComponentProps) {
- {events.map(event => { - const weekday = formatEventDate(event.startDate, timezone, 'fr-FR', { weekday: 'long' }) - const time = eventTimeOrEmpty(new Date(event.startDate), timezone) - - return ( -
- {event.canEdit && ( - toggleSelection(event.id)} - className="size-4 rounded border border-input accent-primary" - onClick={e => e.stopPropagation()} - /> - )} - - - -
- {event.name} - - {weekday} - {time ? ` · ${time}` : ''} - {event.kind ? ` · ${event.kind.name}` : ''} - -
- - - - -
- ) - })} + {events.map(event => ( + toggleSelection(event.id)} + /> + ))} ) })} diff --git a/app/features/events/server/days-off.server.test.ts b/app/features/events/server/days-off.server.test.ts index f22a5317..18a4ea79 100644 --- a/app/features/events/server/days-off.server.test.ts +++ b/app/features/events/server/days-off.server.test.ts @@ -19,51 +19,64 @@ beforeEach(() => { }) describe('createDayOff', () => { - it('retourne null quand startDate est null', async () => { - const result = await createDayOff(db, 1, null, new Date(2025, 3, 10), 1) + it('returns null when startDate is null', async () => { + const result = await createDayOff(db, 1, 1, null, new Date(2025, 3, 10), 1) expect(result).toBeNull() }) - it('retourne null quand endDate est null', async () => { - const result = await createDayOff(db, 1, new Date(2025, 3, 8), null, 1) + it('returns null when endDate is null', async () => { + const result = await createDayOff(db, 1, 1, new Date(2025, 3, 8), null, 1) expect(result).toBeNull() }) - it('retourne null quand startDate est undefined', async () => { - const result = await createDayOff(db, 1, undefined, new Date(2025, 3, 10), 1) + it('returns null when startDate is undefined', async () => { + const result = await createDayOff(db, 1, 1, undefined, new Date(2025, 3, 10), 1) expect(result).toBeNull() }) - it('retourne null quand startDate > endDate', async () => { - const result = await createDayOff(db, 1, new Date(2025, 3, 15), new Date(2025, 3, 10), 1) + it('returns null when startDate > endDate', async () => { + const result = await createDayOff(db, 1, 1, new Date(2025, 3, 15), new Date(2025, 3, 10), 1) expect(result).toBeNull() }) - it('crée un événement quand les dates sont valides', async () => { + it('creates an event when dates are valid', async () => { const fakeEvent = { id: 1, name: 'Absence' } vi.mocked(db.eventKind.findFirst).mockResolvedValue({ id: 5, key: 'off' } as never) vi.mocked(db.event.create).mockResolvedValue(fakeEvent as never) - const result = await createDayOff(db, 1, new Date(2025, 3, 8), new Date(2025, 3, 10), 1) + const result = await createDayOff(db, 1, 1, new Date(2025, 3, 8), new Date(2025, 3, 10), 1) expect(result).toEqual(fakeEvent) }) - it('crée un événement même quand startDate == endDate', async () => { + it('creates an event when startDate == endDate', async () => { const sameDate = new Date(2025, 3, 8) const fakeEvent = { id: 2, name: 'Absence' } vi.mocked(db.eventKind.findFirst).mockResolvedValue({ id: 5, key: 'off' } as never) vi.mocked(db.event.create).mockResolvedValue(fakeEvent as never) - const result = await createDayOff(db, 1, sameDate, sameDate, 1) + const result = await createDayOff(db, 1, 1, sameDate, sameDate, 1) expect(result).toEqual(fakeEvent) }) - it("crée l'événement même sans eventKind trouvé", async () => { + it('creates the event even when no eventKind is found', async () => { const fakeEvent = { id: 3, name: 'Absence' } vi.mocked(db.eventKind.findFirst).mockResolvedValue(null as never) vi.mocked(db.event.create).mockResolvedValue(fakeEvent as never) - const result = await createDayOff(db, 1, new Date(2025, 3, 8), new Date(2025, 3, 10), 1) + const result = await createDayOff(db, 1, 1, new Date(2025, 3, 8), new Date(2025, 3, 10), 1) expect(result).toEqual(fakeEvent) }) + + // With a null memberId (account without a linked Member — e.g. circuit + // overseer or admin), we skip the conflict-flag refresh entirely: there + // can be no assignments to flag. + it('skips refreshConflictFlags when memberId is null', async () => { + const fakeEvent = { id: 4, name: 'Absence' } + vi.mocked(db.eventKind.findFirst).mockResolvedValue({ id: 5, key: 'off' } as never) + vi.mocked(db.event.create).mockResolvedValue(fakeEvent as never) + + await createDayOff(db, 1, null, new Date(2025, 3, 8), new Date(2025, 3, 10), 1) + + expect(db.event.findMany).not.toHaveBeenCalled() + }) }) diff --git a/app/features/events/server/days-off.server.ts b/app/features/events/server/days-off.server.ts index ae00ff29..7872aaf4 100644 --- a/app/features/events/server/days-off.server.ts +++ b/app/features/events/server/days-off.server.ts @@ -19,9 +19,14 @@ export function getNextDaysOffs(db: TransactionClient, userId: number, congregat }) } +// `accountId` writes Event.createdBy. `memberId` (nullable — admin / circuit +// overseer accounts with no linked member) is what refreshConflictFlags needs +// to reconcile assignments; when it's null there can be no assignments to +// conflict, so we skip the refresh entirely. export async function createDayOff( db: TransactionClient, - userId: number, + accountId: number, + memberId: number | null, startDate: Date | null | undefined, endDate: Date | null | undefined, congregationId: number, @@ -41,27 +46,34 @@ export async function createDayOff( ...(eventKind ? { kind: { connect: { id: eventKind.id } } } : {}), startDate, endDate, - createdBy: { connect: { id: userId } }, + createdBy: { connect: { id: accountId } }, name: m.seed_event_kind_absence(), congregation: { connect: { id: congregationId } }, }, }) - // Update conflict flags on programme assignments overlapping this new day-off - await refreshConflictFlags(db, userId, startDate, endDate, congregationId) + if (memberId != null) { + await refreshConflictFlags(db, memberId, startDate, endDate, congregationId) + } return event } -export async function deleteDayOff(db: TransactionClient, eventId: number, userId: number, congregationId: number) { +export async function deleteDayOff( + db: TransactionClient, + eventId: number, + memberId: number | null, + congregationId: number, +) { const event = await db.event.delete({ where: { id_congregationId: { id: eventId, congregationId }, }, }) - // Refresh conflict flags — the absence is gone, so conflicts may be resolved - await refreshConflictFlags(db, userId, event.startDate, event.endDate, congregationId) + if (memberId != null) { + await refreshConflictFlags(db, memberId, event.startDate, event.endDate, congregationId) + } return event } diff --git a/app/features/events/server/event-filters.server.test.ts b/app/features/events/server/event-filters.server.test.ts index e492aa19..021f542c 100644 --- a/app/features/events/server/event-filters.server.test.ts +++ b/app/features/events/server/event-filters.server.test.ts @@ -95,4 +95,28 @@ describe('computeFilters', () => { expect(result.createdById).toBeUndefined() }) + + it('filters by hasConflicts when param is true', () => { + const params = new URLSearchParams({ hasConflicts: 'true' }) + const result = computeFilters(params) + + expect(result.OR).toEqual([ + { partAssignments: { some: { hasConflict: true } } }, + { serviceRoleAssignments: { some: { hasConflict: true } } }, + ]) + }) + + it('does not filter by hasConflicts when param is absent', () => { + const params = new URLSearchParams() + const result = computeFilters(params) + + expect(result.OR).toBeUndefined() + }) + + it('does not filter by hasConflicts when param is false', () => { + const params = new URLSearchParams({ hasConflicts: 'false' }) + const result = computeFilters(params) + + expect(result.OR).toBeUndefined() + }) }) diff --git a/app/features/events/server/event-filters.server.ts b/app/features/events/server/event-filters.server.ts index 6473e5ab..e3ee9e25 100644 --- a/app/features/events/server/event-filters.server.ts +++ b/app/features/events/server/event-filters.server.ts @@ -16,6 +16,7 @@ export function computeFilters(params: URLSearchParams): Prisma.EventWhereInput filters = applyDateRangeFilter(filters, params) filters = applyPublisherFilter(filters, params) + filters = applyHasConflictsFilter(filters, params) return filters } @@ -48,3 +49,20 @@ function applyPublisherFilter(filters: Prisma.EventWhereInput, params: URLSearch return filters } + +// `?hasConflicts=true` restricts the list to events that have at least one +// assignment flagged as a day-off conflict. Powers the deep-link from the +// responsible-conflict dashboard card. +function applyHasConflictsFilter(filters: Prisma.EventWhereInput, params: URLSearchParams): Prisma.EventWhereInput { + if (params.get('hasConflicts') !== 'true') { + return filters + } + + return { + ...filters, + OR: [ + { partAssignments: { some: { hasConflict: true } } }, + { serviceRoleAssignments: { some: { hasConflict: true } } }, + ], + } +} diff --git a/app/features/events/server/list-user-conflicts-in-range.server.test.ts b/app/features/events/server/list-user-conflicts-in-range.server.test.ts new file mode 100644 index 00000000..fc44a78d --- /dev/null +++ b/app/features/events/server/list-user-conflicts-in-range.server.test.ts @@ -0,0 +1,162 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('~/shared/infra/db.server', () => ({ + unscopedDb: { + programmePartAssignment: { findMany: vi.fn() }, + programmeServiceRoleAssignment: { findMany: vi.fn() }, + }, +})) + +const { listUserConflictsInRange } = await import('./list-user-conflicts-in-range.server') +const { unscopedDb: db } = await import('~/shared/infra/db.server') + +beforeEach(() => { + vi.resetAllMocks() + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([] as never) + vi.mocked(db.programmeServiceRoleAssignment.findMany).mockResolvedValue([] as never) +}) + +describe('listUserConflictsInRange', () => { + it('returns an empty array when the member has no conflicting assignments', async () => { + const result = await listUserConflictsInRange(db, 42, new Date(2026, 6, 1), new Date(2026, 6, 3)) + expect(result).toEqual([]) + }) + + // Filters must be by memberId (assigneeId / assistantId reference Member.id), + // hasConflict: true, and event range overlap. Anything else risks either + // showing stale conflicts or missing legitimate ones. + it('filters part assignments by memberId, hasConflict, and event overlap', async () => { + const memberId = 5000 + const start = new Date(2026, 6, 1) + const end = new Date(2026, 6, 3) + + await listUserConflictsInRange(db, memberId, start, end) + + const call = vi.mocked(db.programmePartAssignment.findMany).mock.calls[0][0] + const where = call?.where as Record + expect(where.hasConflict).toBe(true) + expect(where.OR).toEqual([{ assigneeId: memberId }, { assistantId: memberId }]) + expect(where.event).toEqual({ startDate: { lte: end }, endDate: { gte: start } }) + }) + + it('filters service-role assignments by memberId as assignee only', async () => { + const memberId = 5000 + await listUserConflictsInRange(db, memberId, new Date(2026, 6, 1), new Date(2026, 6, 3)) + + const call = vi.mocked(db.programmeServiceRoleAssignment.findMany).mock.calls[0][0] + const where = call?.where as Record + expect(where.hasConflict).toBe(true) + expect(where.assigneeId).toBe(memberId) + }) + + it('resolves the responsible name via accountDisplayName when a template responsible exists', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + name: 'Discours public', + event: { + startDate: new Date(2026, 6, 5), + template: { + responsibles: [{ user: { firstname: 'Jean', lastname: 'Dupont', member: null } }], + }, + }, + }, + ] as never) + + const result = await listUserConflictsInRange(db, 5000, new Date(2026, 6, 1), new Date(2026, 6, 10)) + + expect(result).toEqual([ + { eventDate: new Date(2026, 6, 5), assignmentName: 'Discours public', responsibleName: 'Jean Dupont' }, + ]) + }) + + // Prefers the linked Member's name over the account fallback fields, per + // accountDisplayName semantics — matches what shows up everywhere else in + // the app. + it("uses the responsible's linked Member name when present", async () => { + vi.mocked(db.programmeServiceRoleAssignment.findMany).mockResolvedValue([ + { + name: 'Micros', + event: { + startDate: new Date(2026, 6, 5), + template: { + responsibles: [ + { + user: { + firstname: 'account-first', + lastname: 'account-last', + member: { firstname: 'Pierre', lastname: 'Martin' }, + }, + }, + ], + }, + }, + }, + ] as never) + + const result = await listUserConflictsInRange(db, 5000, new Date(2026, 6, 1), new Date(2026, 6, 10)) + + expect(result[0].responsibleName).toBe('Pierre Martin') + }) + + it('returns responsibleName as null for untemplated events', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + name: 'Custom part', + event: { + startDate: new Date(2026, 6, 5), + template: null, + }, + }, + ] as never) + + const result = await listUserConflictsInRange(db, 5000, new Date(2026, 6, 1), new Date(2026, 6, 10)) + + expect(result[0].responsibleName).toBeNull() + }) + + // When a templated event has no responsible assigned yet, the UI still + // shows the generic fallback wording — the query surfaces null the same + // way as for untemplated events. + it('returns responsibleName as null when the template has no responsible assigned', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + name: 'Prière', + event: { + startDate: new Date(2026, 6, 5), + template: { responsibles: [] }, + }, + }, + ] as never) + + const result = await listUserConflictsInRange(db, 5000, new Date(2026, 6, 1), new Date(2026, 6, 10)) + + expect(result[0].responsibleName).toBeNull() + }) + + it('merges part + service conflicts and sorts by eventDate ascending', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + name: 'Later part', + event: { startDate: new Date(2026, 6, 10), template: null }, + }, + { + name: 'Earlier part', + event: { startDate: new Date(2026, 6, 2), template: null }, + }, + ] as never) + vi.mocked(db.programmeServiceRoleAssignment.findMany).mockResolvedValue([ + { + name: 'Middle service', + event: { startDate: new Date(2026, 6, 5), template: null }, + }, + ] as never) + + const result = await listUserConflictsInRange(db, 5000, new Date(2026, 6, 1), new Date(2026, 6, 15)) + + expect(result.map((r: { assignmentName: string }) => r.assignmentName)).toEqual([ + 'Earlier part', + 'Middle service', + 'Later part', + ]) + }) +}) diff --git a/app/features/events/server/list-user-conflicts-in-range.server.ts b/app/features/events/server/list-user-conflicts-in-range.server.ts new file mode 100644 index 00000000..d49355ac --- /dev/null +++ b/app/features/events/server/list-user-conflicts-in-range.server.ts @@ -0,0 +1,114 @@ +import type { TransactionClient } from '~/shared/infra/db.server' +import { accountDisplayName } from '~/shared/utils/display-name' + +export interface UserConflictInRange { + eventDate: Date + assignmentName: string + responsibleName: string | null +} + +// Read the assignments a member has on programme events that overlap the +// given date range and whose hasConflict flag is set. Used by the days-off +// create action to build the "you have assignments during this absence" +// modal. `responsibleName` resolves through accountDisplayName so the +// linked Member's name wins over the account fallback fields when both +// exist; untemplated events (or templates without a responsible) return +// null so the UI can show a generic fallback line. +export async function listUserConflictsInRange( + db: TransactionClient, + memberId: number, + startDate: Date, + endDate: Date, +): Promise { + const [partConflicts, serviceConflicts] = await Promise.all([ + db.programmePartAssignment.findMany({ + where: { + hasConflict: true, + OR: [{ assigneeId: memberId }, { assistantId: memberId }], + event: { startDate: { lte: endDate }, endDate: { gte: startDate } }, + }, + select: { + name: true, + event: { + select: { + startDate: true, + template: { + select: { + responsibles: { + select: { + user: { + select: { + firstname: true, + lastname: true, + member: { select: { firstname: true, lastname: true } }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }), + db.programmeServiceRoleAssignment.findMany({ + where: { + hasConflict: true, + assigneeId: memberId, + event: { startDate: { lte: endDate }, endDate: { gte: startDate } }, + }, + select: { + name: true, + event: { + select: { + startDate: true, + template: { + select: { + responsibles: { + select: { + user: { + select: { + firstname: true, + lastname: true, + member: { select: { firstname: true, lastname: true } }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }), + ]) + + const merged: UserConflictInRange[] = [...partConflicts, ...serviceConflicts].map(a => ({ + eventDate: a.event.startDate, + assignmentName: a.name, + responsibleName: resolveResponsibleName(a.event.template), + })) + + merged.sort((a, b) => a.eventDate.getTime() - b.eventDate.getTime()) + return merged +} + +type TemplateWithResponsibles = + | { + responsibles: Array<{ + user: { + firstname: string | null + lastname: string | null + member: { firstname: string; lastname: string } | null + } + }> + } + | null + | undefined + +function resolveResponsibleName(template: TemplateWithResponsibles): string | null { + const user = template?.responsibles?.[0]?.user + if (!user) return null + const name = accountDisplayName(user) + return name.length > 0 ? name : null +} diff --git a/app/features/events/server/programme-assignments.server.test.ts b/app/features/events/server/programme-assignments.server.test.ts index fe3d5edc..fbf8db73 100644 --- a/app/features/events/server/programme-assignments.server.test.ts +++ b/app/features/events/server/programme-assignments.server.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import { EventKind } from '~/features/events/model/event-kind.type' vi.mock('~/shared/infra/db.server', () => ({ unscopedDb: { @@ -42,6 +43,24 @@ describe('checkDayOffConflict', () => { const result = await checkDayOffConflict(db, 1, new Date(2026, 3, 14), new Date(2026, 3, 14), 1) expect(result).toBe(false) }) + + // Regression pin — the ID passed in is a Member.id (all call sites resolve + // participants via Member); day-off events store the creator's UserAccount.id + // in Event.createdById. Filtering by `createdBy: { memberId }` is the only + // shape that correctly joins the two. A prior version filtered by + // `createdById: memberId` which silently returned no results whenever the + // member's linked account.id differed from the member.id. + it('joins day-offs through Event.createdBy.memberId (not createdById)', async () => { + vi.mocked(db.event.findFirst).mockResolvedValue(null as never) + const memberId = 5000 + + await checkDayOffConflict(db, memberId, new Date(2026, 3, 14), new Date(2026, 3, 14), 1) + + const firstCall = vi.mocked(db.event.findFirst).mock.calls[0][0] + const where = firstCall?.where as Record + expect(where.createdBy).toEqual({ memberId }) + expect(where).not.toHaveProperty('createdById') + }) }) describe('assignPart', () => { @@ -396,4 +415,53 @@ describe('refreshConflictFlags', () => { expect(db.programmePartAssignment.updateMany).not.toHaveBeenCalled() }) + + // Regression pin — participants are Members (`assigneeId`, `assistantId` + // reference Member.id). Filtering with a UserAccount.id would silently miss + // every assignment whose Member.id differs from the assignee's account.id. + it('filters part and service assignments by memberId', async () => { + vi.mocked(db.event.findMany).mockResolvedValue([ + { id: 1, startDate: new Date(2026, 3, 14), endDate: new Date(2026, 3, 14) }, + ] as never) + vi.mocked(db.event.findFirst).mockResolvedValue({ id: 99 } as never) + vi.mocked(db.programmePartAssignment.updateMany).mockResolvedValue({ count: 1 } as never) + vi.mocked(db.programmeServiceRoleAssignment.updateMany).mockResolvedValue({ count: 0 } as never) + + const memberId = 5000 + await refreshConflictFlags(db, memberId, new Date(2026, 3, 13), new Date(2026, 3, 15), 1) + + const partCall = vi.mocked(db.programmePartAssignment.updateMany).mock.calls[0][0] + const partWhere = partCall?.where as Record + expect(partWhere.OR).toEqual([{ assigneeId: memberId }, { assistantId: memberId }]) + + const serviceCall = vi.mocked(db.programmeServiceRoleAssignment.updateMany).mock.calls[0][0] + const serviceWhere = serviceCall?.where as Record + expect(serviceWhere.assigneeId).toBe(memberId) + }) + + // Regression pin — untemplated events also carry assignments (added + // manually) and must participate in the hasConflict invariant. A prior + // `templateId: { not: null }` filter excluded them. + it('includes untemplated events (no templateId filter)', async () => { + vi.mocked(db.event.findMany).mockResolvedValue([] as never) + + await refreshConflictFlags(db, 5, new Date(2026, 3, 13), new Date(2026, 3, 15), 1) + + const call = vi.mocked(db.event.findMany).mock.calls[0][0] + const where = call?.where as Record + expect(where).not.toHaveProperty('templateId') + }) + + // The Off events themselves are just date ranges — they have no part or + // service assignments. Iterating over them is wasted work and semantically + // odd (an Off event isn't a programme event that can conflict with itself). + it('excludes Off events from the overlapping-events lookup', async () => { + vi.mocked(db.event.findMany).mockResolvedValue([] as never) + + await refreshConflictFlags(db, 5, new Date(2026, 3, 13), new Date(2026, 3, 15), 1) + + const call = vi.mocked(db.event.findMany).mock.calls[0][0] + const where = call?.where as Record + expect(where.kind).toEqual({ key: { not: EventKind.Off } }) + }) }) diff --git a/app/features/events/server/programme-assignments.server.ts b/app/features/events/server/programme-assignments.server.ts index 3cc4e199..7d548b45 100644 --- a/app/features/events/server/programme-assignments.server.ts +++ b/app/features/events/server/programme-assignments.server.ts @@ -218,9 +218,13 @@ export async function unassignServiceRole(db: TransactionClient, assignmentId: n return { assignment, previousAssigneeId: existing.assigneeId } } +// The `memberId` here is Member.id. Day-off events store the creator's +// UserAccount.id in Event.createdById, so we join through +// Event.createdBy.memberId to resolve the absence back to the same Member +// that carries assignments (assigneeId / assistantId reference Member). export async function checkDayOffConflict( db: TransactionClient, - userId: number, + memberId: number, startDate: Date, endDate: Date, congregationId: number, @@ -228,7 +232,7 @@ export async function checkDayOffConflict( const conflictingDayOff = await db.event.findFirst({ where: { congregationId, - createdById: userId, + createdBy: { memberId }, kind: { key: EventKind.Off }, startDate: { lte: endDate }, endDate: { gte: startDate }, @@ -240,16 +244,18 @@ export async function checkDayOffConflict( export async function refreshConflictFlags( db: TransactionClient, - userId: number, + memberId: number, startDate: Date, endDate: Date, congregationId: number, ) { - // Find all programme events overlapping with the given date range + // Find all programme events (templated OR not) overlapping the range. + // Off events themselves have no assignments and are excluded to avoid + // pointless iteration. const overlappingEvents = await db.event.findMany({ where: { congregationId, - templateId: { not: null }, + kind: { key: { not: EventKind.Off } }, startDate: { lte: endDate }, endDate: { gte: startDate }, }, @@ -257,23 +263,21 @@ export async function refreshConflictFlags( }) for (const event of overlappingEvents) { - const hasConflict = await checkDayOffConflict(db, userId, event.startDate, event.endDate, congregationId) + const hasConflict = await checkDayOffConflict(db, memberId, event.startDate, event.endDate, congregationId) - // Update part assignments where this user is assigned await db.programmePartAssignment.updateMany({ where: { eventId: event.id, congregationId, - OR: [{ assigneeId: userId }, { assistantId: userId }], + OR: [{ assigneeId: memberId }, { assistantId: memberId }], }, data: { hasConflict }, }) - // Update service role assignments where this user is assigned await db.programmeServiceRoleAssignment.updateMany({ where: { eventId: event.id, - assigneeId: userId, + assigneeId: memberId, congregationId, }, data: { hasConflict }, diff --git a/app/features/events/ui/DayOffConflictModal.tsx b/app/features/events/ui/DayOffConflictModal.tsx new file mode 100644 index 00000000..29400a9b --- /dev/null +++ b/app/features/events/ui/DayOffConflictModal.tsx @@ -0,0 +1,64 @@ +import type { UserConflictInRange } from '~/features/events/server/list-user-conflicts-in-range.server' +import { pickConflictModalTitle } from '~/features/events/ui/day-off-conflict-helpers' +import * as m from '~/i18n/paraglide/messages' +import { + AlertDialog, + AlertDialogAction, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from '~/shared/ui/alert-dialog' +import { formatEventDate } from '~/shared/utils/event-time' + +interface DayOffConflictModalProps { + conflicts: UserConflictInRange[] + timezone: string + open: boolean + onClose: () => void +} + +const ROW_DATE_OPTIONS: Intl.DateTimeFormatOptions = { weekday: 'short', day: 'numeric', month: 'short' } + +export function DayOffConflictModal({ conflicts, timezone, open, onClose }: DayOffConflictModalProps) { + const title = pickConflictModalTitle(conflicts.length, { + singular: m.days_off_conflict_modal_title_singular, + plural: count => m.days_off_conflict_modal_title_plural({ count }), + }) + + return ( + (!next ? onClose() : undefined)}> + + + {title} + {m.days_off_conflict_modal_intro()} + + +
    + {conflicts.map((c, idx) => { + const date = formatEventDate(c.eventDate, timezone, 'fr-FR', ROW_DATE_OPTIONS) + const text = + c.responsibleName != null + ? m.days_off_conflict_modal_row_named({ + assignment: c.assignmentName, + date, + responsible: c.responsibleName, + }) + : m.days_off_conflict_modal_row_fallback({ assignment: c.assignmentName, date }) + return ( + // Conflicts have no stable id — server returns an aggregate; index is fine + // because we never reorder within one modal instance. + // biome-ignore lint/suspicious/noArrayIndexKey: aggregate row has no stable id +
  • {text}
    • + ) + })} +
+ + + {m.days_off_conflict_modal_button()} + + + + ) +} diff --git a/app/features/events/ui/day-off-conflict-helpers.test.ts b/app/features/events/ui/day-off-conflict-helpers.test.ts new file mode 100644 index 00000000..bd3a8fae --- /dev/null +++ b/app/features/events/ui/day-off-conflict-helpers.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from 'vitest' +import { pickConflictModalTitle } from './day-off-conflict-helpers' + +describe('pickConflictModalTitle', () => { + it('picks the singular title for a single conflict', () => { + const result = pickConflictModalTitle(1, { + singular: () => 'singular-copy', + plural: n => `plural-copy-${n}`, + }) + expect(result).toBe('singular-copy') + }) + + it('picks the plural title for two or more conflicts, forwarding the count', () => { + const result = pickConflictModalTitle(3, { + singular: () => 'singular-copy', + plural: n => `plural-copy-${n}`, + }) + expect(result).toBe('plural-copy-3') + }) + + // Guarding zero is defensive — the modal only renders when there is at + // least one conflict, but the helper must be total to keep call sites + // typed and simple. + it('picks the plural title for zero conflicts (defensive)', () => { + const result = pickConflictModalTitle(0, { + singular: () => 'singular-copy', + plural: n => `plural-copy-${n}`, + }) + expect(result).toBe('plural-copy-0') + }) +}) diff --git a/app/features/events/ui/day-off-conflict-helpers.ts b/app/features/events/ui/day-off-conflict-helpers.ts new file mode 100644 index 00000000..8e98ddcd --- /dev/null +++ b/app/features/events/ui/day-off-conflict-helpers.ts @@ -0,0 +1,8 @@ +export interface ConflictTitleMessages { + singular: () => string + plural: (count: number) => string +} + +export function pickConflictModalTitle(count: number, messages: ConflictTitleMessages): string { + return count === 1 ? messages.singular() : messages.plural(count) +} diff --git a/app/i18n/messages/en.json b/app/i18n/messages/en.json index 7724d2b7..d28fd7e3 100644 --- a/app/i18n/messages/en.json +++ b/app/i18n/messages/en.json @@ -1302,6 +1302,18 @@ "days_off_new_end_date": "End date", "days_off_new_invalid_dates": "Unable to add this absence. The dates are invalid.", "days_off_new_success": "Absence added successfully.", + "days_off_conflict_modal_title_singular": "You have an assignment during this absence", + "days_off_conflict_modal_title_plural": "You have {count} assignments during this absence", + "days_off_conflict_modal_intro": "Your absence has been saved. Please let the responsibles know so they can arrange a replacement:", + "days_off_conflict_modal_row_named": "{assignment} on {date} — notify {responsible}", + "days_off_conflict_modal_row_fallback": "{assignment} on {date} — notify a programme responsible", + "days_off_conflict_modal_button": "Got it, I'll let them know", + "dashboard_urgent_responsible_conflict_singular": "1 assignment conflict: {names}", + "dashboard_urgent_responsible_conflict_plural": "{count} assignment conflicts: {names}", + "dashboard_urgent_responsible_conflict_extras": " (+{count} more)", + "programs_event_conflict_badge_singular": "Conflict", + "programs_event_conflict_badge_plural": "{count} conflicts", + "programs_event_conflict_badge_aria": "Assignment conflict: {count} absent participant(s)", "days_off_admin_meta_title": "Absences - Unitae", "days_off_admin_page_title": "Absences", "days_off_admin_page_subtitle": "List of all absences for the selected period.", diff --git a/app/i18n/messages/fr.json b/app/i18n/messages/fr.json index 4a02debf..8c006156 100644 --- a/app/i18n/messages/fr.json +++ b/app/i18n/messages/fr.json @@ -1304,6 +1304,18 @@ "days_off_new_end_date": "Date de fin", "days_off_new_invalid_dates": "Impossible d'ajouter cette absence. Les dates sont invalides.", "days_off_new_success": "Absence ajoutée avec succès.", + "days_off_conflict_modal_title_singular": "Vous avez une affectation pendant cette absence", + "days_off_conflict_modal_title_plural": "Vous avez {count} affectations pendant cette absence", + "days_off_conflict_modal_intro": "Votre absence est bien enregistrée. Pensez à prévenir les responsables pour qu'ils prévoient un remplaçant :", + "days_off_conflict_modal_row_named": "{assignment} le {date} — prévenir {responsible}", + "days_off_conflict_modal_row_fallback": "{assignment} le {date} — prévenir un responsable du programme", + "days_off_conflict_modal_button": "D'accord, je vais le prévenir", + "dashboard_urgent_responsible_conflict_singular": "1 conflit d'affectation : {names}", + "dashboard_urgent_responsible_conflict_plural": "{count} conflits d'affectation : {names}", + "dashboard_urgent_responsible_conflict_extras": " (+{count} autres)", + "programs_event_conflict_badge_singular": "Conflit", + "programs_event_conflict_badge_plural": "{count} conflits", + "programs_event_conflict_badge_aria": "Conflit d'affectation : {count} personne(s) absente(s)", "days_off_admin_meta_title": "Absences - Unitae", "days_off_admin_page_title": "Absences", "days_off_admin_page_subtitle": "Liste de toutes les absences sur la période sélectionnée.", From c262c5ce31f6b0f7129ab65197d1f2ca4037885c Mon Sep 17 00:00:00 2001 From: mindsers Date: Thu, 16 Jul 2026 01:17:34 +0200 Subject: [PATCH 2/4] refactor(events): address code review feedback on conflict awareness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code changes: - `applyHasConflictsFilter` now composes via `AND` so a caller (or future filter) can freely set its own top-level `OR` without either clause clobbering the other. - `resolveResponsibleName` surfaces every named responsible on a template, deterministically sorted, instead of arbitrarily picking the first row Prisma returns. - `ResponsibleConflictsSummary` swaps `additionalAbsenteesCount` for `totalAbsenteesCount`; the formatter derives `extras = total - shown` so the `shown + extras = total` invariant is unforgeable. - Days-off action now logs a distinguishing info line when the conflict check is skipped because the current account has no linked Member, so ops can tell that branch apart from "member exists, zero conflicts". Test additions: - `refreshConflictFlags` unit tests now assert on `data.hasConflict` for both directions (true → set, false → clear) and iterate over multiple overlapping events. - `deleteDayOff` gets first-time unit coverage: delete-and-return, refresh when `memberId` is provided, skip when it is `null`. - `getResponsibleConflicts` gets an explicit null-assignee row test so the `record()` skip guard is behaviourally pinned. - `event-filters` gets a combined-params test asserting hasConflicts composes cleanly with from/to and publisher filters. - `list-user-conflicts-in-range` gets a multi-responsible test pinning the alphabetical join. Comment cleanups: - Trimmed `list-user-conflicts-in-range.server.ts`, `days-off/new.tsx`, `get-responsible-conflicts.server.ts`, `build-urgent-items.ts`, and `event-filters.server.ts` to keep the *why* and drop restatements / caller names / speculative future-tense. - Replaced the "at line 578" reference in `dashboard.integration.test.ts` with a name-based reference so an insert above cannot silently invalidate the cross-ref. --- .../server/dashboard.integration.test.ts | 12 ++--- .../get-responsible-conflicts.server.test.ts | 32 +++++++++++- .../get-responsible-conflicts.server.ts | 24 ++++----- .../dashboard/ui/build-urgent-items.test.ts | 12 ++--- .../dashboard/ui/build-urgent-items.ts | 14 ++---- app/features/events/routes/days-off/new.tsx | 18 +++++-- .../events/server/days-off.server.test.ts | 41 ++++++++++++++- .../server/event-filters.server.test.ts | 39 +++++++++++++-- .../events/server/event-filters.server.ts | 17 +++++-- ...ist-user-conflicts-in-range.server.test.ts | 24 +++++++++ .../list-user-conflicts-in-range.server.ts | 26 ++++++---- .../programme-assignments.server.test.ts | 50 +++++++++++++++++-- 12 files changed, 242 insertions(+), 67 deletions(-) diff --git a/app/features/dashboard/server/dashboard.integration.test.ts b/app/features/dashboard/server/dashboard.integration.test.ts index ac88bf8d..e5d8f472 100644 --- a/app/features/dashboard/server/dashboard.integration.test.ts +++ b/app/features/dashboard/server/dashboard.integration.test.ts @@ -670,11 +670,11 @@ describe('getConflictingAssignments (integration)', () => { } }) - // Invariant pin (responsible side): the aggregated responsible-conflict - // card is correct only as long as the hasConflict flag remains authoritative. - // Once refreshConflictFlags (or any direct write) clears the flag, the - // responsible's card must disappear on the next read — no stale cache, no - // extra invalidation step. This mirrors the absentee-side pin at line 578. + // Invariant pin (responsible side): `getResponsibleConflicts` derives its + // result from the persisted `hasConflict` flag with no caching layer, so + // once the flag flips to `false` the responsible's card must vanish on + // the next read. Mirrors the absentee-side "refreshConflictFlags clears + // stale hasConflict and the alert disappears" pin earlier in this file. it('getResponsibleConflicts drops the entry when hasConflict clears on the underlying assignment', async () => { const setup = await withScope(congregationId, async tx => { const eventKind = await tx.eventKind.findFirstOrThrow({ @@ -737,7 +737,7 @@ describe('getConflictingAssignments (integration)', () => { ) const after = await withScope(congregationId, tx => getResponsibleConflicts(tx, bobAccountId, false)) - expect(after).toEqual({ count: 0, absenteeNames: [], additionalAbsenteesCount: 0 }) + expect(after).toEqual({ count: 0, absenteeNames: [], totalAbsenteesCount: 0 }) } finally { await withScope(congregationId, async tx => { await tx.programmePartAssignment.delete({ diff --git a/app/features/dashboard/server/get-responsible-conflicts.server.test.ts b/app/features/dashboard/server/get-responsible-conflicts.server.test.ts index f6201c93..e0bda54c 100644 --- a/app/features/dashboard/server/get-responsible-conflicts.server.test.ts +++ b/app/features/dashboard/server/get-responsible-conflicts.server.test.ts @@ -19,7 +19,7 @@ beforeEach(() => { describe('getResponsibleConflicts', () => { it('returns zero count with no names when no conflicts exist', async () => { const result = await getResponsibleConflicts(db, 42, false) - expect(result).toEqual({ count: 0, absenteeNames: [], additionalAbsenteesCount: 0 }) + expect(result).toEqual({ count: 0, absenteeNames: [], totalAbsenteesCount: 0 }) }) // Non-manager users only see conflicts on templates they are the @@ -180,7 +180,35 @@ describe('getResponsibleConflicts', () => { const result = await getResponsibleConflicts(db, 999, true) expect(result.absenteeNames).toEqual(['Alice A', 'Bob B', 'Charlie C']) expect(result.count).toBe(5) - expect(result.additionalAbsenteesCount).toBe(2) + expect(result.totalAbsenteesCount).toBe(5) + }) + + // A row can arrive with a null assigneeId when the assignment slot is + // held but no member is booked yet, or via an integrity issue where the + // related member could not be joined. Either way it must not corrupt the + // aggregation. This pin guards the `record()` null-skip at line 66. + it('silently skips rows whose assignee is null (no assignee booked or join failed)', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + eventId: 1, + assigneeId: null, + assignee: null, + assistantId: null, + assistant: null, + }, + { + eventId: 2, + assigneeId: 100, + assignee: { firstname: 'Alice', lastname: 'Dupont' }, + assistantId: null, + assistant: null, + }, + ] as never) + + const result = await getResponsibleConflicts(db, 999, true) + expect(result.count).toBe(1) + expect(result.absenteeNames).toEqual(['Alice Dupont']) + expect(result.totalAbsenteesCount).toBe(1) }) // A part row can carry both an assignee and an assistant — both are diff --git a/app/features/dashboard/server/get-responsible-conflicts.server.ts b/app/features/dashboard/server/get-responsible-conflicts.server.ts index ad0a179d..8fb0f8ba 100644 --- a/app/features/dashboard/server/get-responsible-conflicts.server.ts +++ b/app/features/dashboard/server/get-responsible-conflicts.server.ts @@ -6,23 +6,20 @@ export interface ResponsibleConflictsSummary { count: number // Up to MAX_ABSENTEE_NAMES unique absentee display names, sorted alphabetically. absenteeNames: string[] - // How many unique absentees exist beyond the ones listed above. 0 when - // every unique absentee fits in `absenteeNames`. The label formatter - // uses this to append "(+N autres)". - additionalAbsenteesCount: number + // Total number of distinct absentees, including those beyond the ones + // sampled in `absenteeNames`. Consumers derive the "(+N autres)" tail + // from `totalAbsenteesCount - absenteeNames.length` so the invariant + // `sampled + extras = total` cannot drift out of sync. + totalAbsenteesCount: number } const MAX_ABSENTEE_NAMES = 3 -// Aggregated counts of upcoming programme assignments flagged as -// day-off conflicts, scoped by who can see them: -// - non-manager: only events on templates where the user is the -// documented responsible (ProgrammeTemplateResponsible.userId). -// - ProgramManager: all events, including untemplated ones which have +// Scoping: +// - non-manager → only events on templates where the user is the documented +// responsible (ProgrammeTemplateResponsible.userId); +// - ProgramManager → all events, including untemplated ones which have // no responsible relation at all. -// Returns a count (unique member × event pairs) and up to three -// absentee names, sorted alphabetically, for at-a-glance triage on -// the dashboard. export async function getResponsibleConflicts( db: TransactionClient, userId: number, @@ -80,7 +77,6 @@ export async function getResponsibleConflicts( const sortedNames = [...nameByMemberId.values()].sort((a, b) => a.localeCompare(b)) const absenteeNames = sortedNames.slice(0, MAX_ABSENTEE_NAMES) - const additionalAbsenteesCount = Math.max(0, sortedNames.length - MAX_ABSENTEE_NAMES) - return { count: pairs.size, absenteeNames, additionalAbsenteesCount } + return { count: pairs.size, absenteeNames, totalAbsenteesCount: sortedNames.length } } diff --git a/app/features/dashboard/ui/build-urgent-items.test.ts b/app/features/dashboard/ui/build-urgent-items.test.ts index c8100697..53d5e9e2 100644 --- a/app/features/dashboard/ui/build-urgent-items.test.ts +++ b/app/features/dashboard/ui/build-urgent-items.test.ts @@ -271,7 +271,7 @@ describe('formatResponsibleConflictLabel', () => { const label = formatResponsibleConflictLabel({ count: 1, absenteeNames: ['Marie Dupont'], - additionalAbsenteesCount: 0, + totalAbsenteesCount: 1, }) expect(label).toBe('1 responsible conflict: Marie Dupont') }) @@ -280,7 +280,7 @@ describe('formatResponsibleConflictLabel', () => { const label = formatResponsibleConflictLabel({ count: 3, absenteeNames: ['Alice A', 'Bob B', 'Charlie C'], - additionalAbsenteesCount: 0, + totalAbsenteesCount: 3, }) expect(label).toBe('3 responsible conflicts: Alice A, Bob B, Charlie C') }) @@ -289,7 +289,7 @@ describe('formatResponsibleConflictLabel', () => { const label = formatResponsibleConflictLabel({ count: 5, absenteeNames: ['Alice A', 'Bob B', 'Charlie C'], - additionalAbsenteesCount: 2, + totalAbsenteesCount: 5, }) expect(label).toBe('5 responsible conflicts: Alice A, Bob B, Charlie C (+2 more)') }) @@ -301,14 +301,14 @@ describe('urgentResponsibleConflictItems', () => { }) it('returns empty array when the summary count is zero', () => { - expect(urgentResponsibleConflictItems({ count: 0, absenteeNames: [], additionalAbsenteesCount: 0 })).toEqual([]) + expect(urgentResponsibleConflictItems({ count: 0, absenteeNames: [], totalAbsenteesCount: 0 })).toEqual([]) }) it('returns one item with priority 1 and a deep-link to the filtered programme list', () => { const items = urgentResponsibleConflictItems({ count: 2, absenteeNames: ['Marie D.', 'Jean P.'], - additionalAbsenteesCount: 0, + totalAbsenteesCount: 2, }) expect(items).toHaveLength(1) expect(items[0].priority).toBe(1) @@ -395,7 +395,7 @@ describe('buildUrgentItems', () => { const items = buildUrgentItems(null, null, null, null, { count: 3, absenteeNames: ['Marie D.', 'Jean P.'], - additionalAbsenteesCount: 0, + totalAbsenteesCount: 2, }) expect(items).toHaveLength(1) expect(items[0].priority).toBe(1) diff --git a/app/features/dashboard/ui/build-urgent-items.ts b/app/features/dashboard/ui/build-urgent-items.ts index 8e5ead81..42c34709 100644 --- a/app/features/dashboard/ui/build-urgent-items.ts +++ b/app/features/dashboard/ui/build-urgent-items.ts @@ -130,16 +130,12 @@ export function urgentDocumentsItem(unreadCount: number | null): UrgentItem[] { ] } -// Compose the human-readable label for the responsible-conflict card. -// Kept pure and exported so it can be unit-tested without JSX and reused -// if we ever surface the same summary elsewhere. +// Exported (not inlined into `urgentResponsibleConflictItems`) so the +// composition can be unit-tested without JSX. export function formatResponsibleConflictLabel(summary: ResponsibleConflictsSummary): string { - const namesJoined = summary.absenteeNames.join(', ') - const extras = - summary.additionalAbsenteesCount > 0 - ? m.dashboard_urgent_responsible_conflict_extras({ count: String(summary.additionalAbsenteesCount) }) - : '' - const names = namesJoined + extras + const extraCount = Math.max(0, summary.totalAbsenteesCount - summary.absenteeNames.length) + const extras = extraCount > 0 ? m.dashboard_urgent_responsible_conflict_extras({ count: String(extraCount) }) : '' + const names = summary.absenteeNames.join(', ') + extras if (summary.count === 1) { return m.dashboard_urgent_responsible_conflict_singular({ names }) diff --git a/app/features/events/routes/days-off/new.tsx b/app/features/events/routes/days-off/new.tsx index 1f78f676..2a6bc7be 100644 --- a/app/features/events/routes/days-off/new.tsx +++ b/app/features/events/routes/days-off/new.tsx @@ -126,10 +126,20 @@ export async function action({ request, context }: Route.ActionArgs) { }) } - // Surface any pre-existing programme assignments that overlap this absence - // so the user is prompted to reach out before the meeting. When there are - // conflicts we skip the success flash — the modal is the confirmation. - const conflicts = memberId != null ? await listUserConflictsInRange(db, memberId, startDate, endDate) : [] + // When there are conflicts we skip the success flash — the modal is + // the confirmation. + let conflicts: Awaited> + if (memberId == null) { + // Accounts without a linked Member cannot own programme assignments, + // so there is nothing to check. Logged so ops can distinguish this + // branch from a genuine "no conflicts found". + logger.info( + `Days off created for account without linked Member; skipped conflict check. User ID: ${currentUser.id}.`, + ) + conflicts = [] + } else { + conflicts = await listUserConflictsInRange(db, memberId, startDate, endDate) + } if (conflicts.length > 0) { logger.info(`Days off created with ${conflicts.length} conflict(s). User ID: ${currentUser.id}.`) diff --git a/app/features/events/server/days-off.server.test.ts b/app/features/events/server/days-off.server.test.ts index 18a4ea79..43b06190 100644 --- a/app/features/events/server/days-off.server.test.ts +++ b/app/features/events/server/days-off.server.test.ts @@ -9,7 +9,7 @@ vi.mock('~/shared/infra/db.server', () => ({ }, })) -const { createDayOff } = await import('./days-off.server') +const { createDayOff, deleteDayOff } = await import('./days-off.server') const { unscopedDb: db } = await import('~/shared/infra/db.server') beforeEach(() => { @@ -80,3 +80,42 @@ describe('createDayOff', () => { expect(db.event.findMany).not.toHaveBeenCalled() }) }) + +describe('deleteDayOff', () => { + it('deletes the event and returns it', async () => { + const fakeEvent = { id: 42, startDate: new Date(2025, 3, 8), endDate: new Date(2025, 3, 10) } + vi.mocked(db.event.delete).mockResolvedValue(fakeEvent as never) + + const result = await deleteDayOff(db, 42, 1, 1) + + expect(result).toEqual(fakeEvent) + expect(db.event.delete).toHaveBeenCalledWith({ + where: { id_congregationId: { id: 42, congregationId: 1 } }, + }) + }) + + it('refreshes conflict flags over the deleted range when memberId is provided', async () => { + const startDate = new Date(2025, 3, 8) + const endDate = new Date(2025, 3, 10) + vi.mocked(db.event.delete).mockResolvedValue({ id: 42, startDate, endDate } as never) + + await deleteDayOff(db, 42, 1, 1) + + expect(db.event.findMany).toHaveBeenCalled() + }) + + // Mirror of createDayOff's null-memberId guard — an account without a + // linked Member cannot own any conflict-carrying assignments, so refresh + // is skipped rather than issuing a no-op query. + it('skips refreshConflictFlags when memberId is null', async () => { + vi.mocked(db.event.delete).mockResolvedValue({ + id: 42, + startDate: new Date(2025, 3, 8), + endDate: new Date(2025, 3, 10), + } as never) + + await deleteDayOff(db, 42, null, 1) + + expect(db.event.findMany).not.toHaveBeenCalled() + }) +}) diff --git a/app/features/events/server/event-filters.server.test.ts b/app/features/events/server/event-filters.server.test.ts index 021f542c..2f9f0927 100644 --- a/app/features/events/server/event-filters.server.test.ts +++ b/app/features/events/server/event-filters.server.test.ts @@ -100,9 +100,13 @@ describe('computeFilters', () => { const params = new URLSearchParams({ hasConflicts: 'true' }) const result = computeFilters(params) - expect(result.OR).toEqual([ - { partAssignments: { some: { hasConflict: true } } }, - { serviceRoleAssignments: { some: { hasConflict: true } } }, + expect(result.AND).toEqual([ + { + OR: [ + { partAssignments: { some: { hasConflict: true } } }, + { serviceRoleAssignments: { some: { hasConflict: true } } }, + ], + }, ]) }) @@ -110,13 +114,38 @@ describe('computeFilters', () => { const params = new URLSearchParams() const result = computeFilters(params) - expect(result.OR).toBeUndefined() + expect(result.AND).toBeUndefined() }) it('does not filter by hasConflicts when param is false', () => { const params = new URLSearchParams({ hasConflicts: 'false' }) const result = computeFilters(params) - expect(result.OR).toBeUndefined() + expect(result.AND).toBeUndefined() + }) + + // Combined-param pin: hasConflicts must compose cleanly with date + publisher + // filters. A regression that spread over `startDate` / `endDate` (or drops + // `createdById`) would slip through the single-param tests above. + it('preserves date and publisher filters when hasConflicts is applied', () => { + const params = new URLSearchParams({ + from: '2025-06-01', + to: '2025-06-30', + publisher: '42', + hasConflicts: 'true', + }) + const result = computeFilters(params) + + expect(result.startDate).toEqual({ lte: new Date('2025-06-30') }) + expect(result.endDate).toEqual({ gte: new Date('2025-06-01') }) + expect(result.createdById).toBe(42) + expect(result.AND).toEqual([ + { + OR: [ + { partAssignments: { some: { hasConflict: true } } }, + { serviceRoleAssignments: { some: { hasConflict: true } } }, + ], + }, + ]) }) }) diff --git a/app/features/events/server/event-filters.server.ts b/app/features/events/server/event-filters.server.ts index e3ee9e25..f9401bad 100644 --- a/app/features/events/server/event-filters.server.ts +++ b/app/features/events/server/event-filters.server.ts @@ -51,18 +51,25 @@ function applyPublisherFilter(filters: Prisma.EventWhereInput, params: URLSearch } // `?hasConflicts=true` restricts the list to events that have at least one -// assignment flagged as a day-off conflict. Powers the deep-link from the -// responsible-conflict dashboard card. +// assignment flagged as a day-off conflict. Nested under `AND` so a +// caller (or a future filter) can freely set its own top-level `OR` +// without either clause silently overwriting the other. function applyHasConflictsFilter(filters: Prisma.EventWhereInput, params: URLSearchParams): Prisma.EventWhereInput { if (params.get('hasConflicts') !== 'true') { return filters } + const existingAnd = Array.isArray(filters.AND) ? filters.AND : filters.AND ? [filters.AND] : [] return { ...filters, - OR: [ - { partAssignments: { some: { hasConflict: true } } }, - { serviceRoleAssignments: { some: { hasConflict: true } } }, + AND: [ + ...existingAnd, + { + OR: [ + { partAssignments: { some: { hasConflict: true } } }, + { serviceRoleAssignments: { some: { hasConflict: true } } }, + ], + }, ], } } diff --git a/app/features/events/server/list-user-conflicts-in-range.server.test.ts b/app/features/events/server/list-user-conflicts-in-range.server.test.ts index fc44a78d..723f56ca 100644 --- a/app/features/events/server/list-user-conflicts-in-range.server.test.ts +++ b/app/features/events/server/list-user-conflicts-in-range.server.test.ts @@ -133,6 +133,30 @@ describe('listUserConflictsInRange', () => { expect(result[0].responsibleName).toBeNull() }) + // A template can carry multiple responsibles; each named person must + // surface so the absentee knows who to reach. Sorted alphabetically so + // the modal reads the same across renders. + it('joins every responsible name when a template has more than one, sorted alphabetically', async () => { + vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ + { + name: 'Discours public', + event: { + startDate: new Date(2026, 6, 5), + template: { + responsibles: [ + { user: { firstname: 'Zoé', lastname: 'Petit', member: null } }, + { user: { firstname: 'Alain', lastname: 'Roux', member: null } }, + ], + }, + }, + }, + ] as never) + + const result = await listUserConflictsInRange(db, 5000, new Date(2026, 6, 1), new Date(2026, 6, 10)) + + expect(result[0].responsibleName).toBe('Alain Roux, Zoé Petit') + }) + it('merges part + service conflicts and sorts by eventDate ascending', async () => { vi.mocked(db.programmePartAssignment.findMany).mockResolvedValue([ { diff --git a/app/features/events/server/list-user-conflicts-in-range.server.ts b/app/features/events/server/list-user-conflicts-in-range.server.ts index d49355ac..fd9bed92 100644 --- a/app/features/events/server/list-user-conflicts-in-range.server.ts +++ b/app/features/events/server/list-user-conflicts-in-range.server.ts @@ -7,13 +7,10 @@ export interface UserConflictInRange { responsibleName: string | null } -// Read the assignments a member has on programme events that overlap the -// given date range and whose hasConflict flag is set. Used by the days-off -// create action to build the "you have assignments during this absence" -// modal. `responsibleName` resolves through accountDisplayName so the -// linked Member's name wins over the account fallback fields when both -// exist; untemplated events (or templates without a responsible) return -// null so the UI can show a generic fallback line. +// `responsibleName` resolves through `accountDisplayName` so a template +// responsible's linked Member name wins over the account fallback fields. +// Untemplated events (or templates with no responsible assigned) return +// `null` so the UI can show a generic fallback line. export async function listUserConflictsInRange( db: TransactionClient, memberId: number, @@ -106,9 +103,16 @@ type TemplateWithResponsibles = | null | undefined +// A template can have any number of responsibles. Every named responsible +// is surfaced so the absentee knows who to reach, deterministically ordered +// by display name so the modal reads the same across renders. function resolveResponsibleName(template: TemplateWithResponsibles): string | null { - const user = template?.responsibles?.[0]?.user - if (!user) return null - const name = accountDisplayName(user) - return name.length > 0 ? name : null + const responsibles = template?.responsibles ?? [] + if (responsibles.length === 0) return null + + const names = responsibles.map(r => accountDisplayName(r.user)).filter(name => name.length > 0) + if (names.length === 0) return null + + names.sort((a, b) => a.localeCompare(b)) + return names.join(', ') } diff --git a/app/features/events/server/programme-assignments.server.test.ts b/app/features/events/server/programme-assignments.server.test.ts index fbf8db73..f211c6e2 100644 --- a/app/features/events/server/programme-assignments.server.test.ts +++ b/app/features/events/server/programme-assignments.server.test.ts @@ -394,18 +394,36 @@ describe('unassignServiceRole', () => { }) describe('refreshConflictFlags', () => { - it('updates conflict flags for overlapping events', async () => { + it('writes hasConflict:true when the member has an overlapping day-off', async () => { vi.mocked(db.event.findMany).mockResolvedValue([ { id: 1, startDate: new Date(2026, 3, 14), endDate: new Date(2026, 3, 14) }, ] as never) - vi.mocked(db.event.findFirst).mockResolvedValue({ id: 99 } as never) + vi.mocked(db.event.findFirst).mockResolvedValue({ id: 99 } as never) // day-off found + vi.mocked(db.programmePartAssignment.updateMany).mockResolvedValue({ count: 1 } as never) + vi.mocked(db.programmeServiceRoleAssignment.updateMany).mockResolvedValue({ count: 0 } as never) + + await refreshConflictFlags(db, 5, new Date(2026, 3, 13), new Date(2026, 3, 15), 1) + + const partCall = vi.mocked(db.programmePartAssignment.updateMany).mock.calls[0][0] + const serviceCall = vi.mocked(db.programmeServiceRoleAssignment.updateMany).mock.calls[0][0] + expect(partCall?.data).toEqual({ hasConflict: true }) + expect(serviceCall?.data).toEqual({ hasConflict: true }) + }) + + it('writes hasConflict:false when the member no longer has an overlapping day-off', async () => { + vi.mocked(db.event.findMany).mockResolvedValue([ + { id: 1, startDate: new Date(2026, 3, 14), endDate: new Date(2026, 3, 14) }, + ] as never) + vi.mocked(db.event.findFirst).mockResolvedValue(null as never) // no day-off vi.mocked(db.programmePartAssignment.updateMany).mockResolvedValue({ count: 1 } as never) vi.mocked(db.programmeServiceRoleAssignment.updateMany).mockResolvedValue({ count: 0 } as never) await refreshConflictFlags(db, 5, new Date(2026, 3, 13), new Date(2026, 3, 15), 1) - expect(db.programmePartAssignment.updateMany).toHaveBeenCalled() - expect(db.programmeServiceRoleAssignment.updateMany).toHaveBeenCalled() + const partCall = vi.mocked(db.programmePartAssignment.updateMany).mock.calls[0][0] + const serviceCall = vi.mocked(db.programmeServiceRoleAssignment.updateMany).mock.calls[0][0] + expect(partCall?.data).toEqual({ hasConflict: false }) + expect(serviceCall?.data).toEqual({ hasConflict: false }) }) it('does nothing when no overlapping events', async () => { @@ -416,6 +434,30 @@ describe('refreshConflictFlags', () => { expect(db.programmePartAssignment.updateMany).not.toHaveBeenCalled() }) + // Every overlapping event must be reconciled independently. A regression + // that early-returned after the first iteration would leave later events + // stuck on stale flags. + it('iterates every overlapping event, updating each independently', async () => { + vi.mocked(db.event.findMany).mockResolvedValue([ + { id: 1, startDate: new Date(2026, 3, 14), endDate: new Date(2026, 3, 14) }, + { id: 2, startDate: new Date(2026, 3, 15), endDate: new Date(2026, 3, 15) }, + { id: 3, startDate: new Date(2026, 3, 16), endDate: new Date(2026, 3, 16) }, + ] as never) + vi.mocked(db.event.findFirst).mockResolvedValue({ id: 99 } as never) + vi.mocked(db.programmePartAssignment.updateMany).mockResolvedValue({ count: 1 } as never) + vi.mocked(db.programmeServiceRoleAssignment.updateMany).mockResolvedValue({ count: 0 } as never) + + await refreshConflictFlags(db, 5, new Date(2026, 3, 13), new Date(2026, 3, 17), 1) + + expect(vi.mocked(db.programmePartAssignment.updateMany).mock.calls).toHaveLength(3) + expect(vi.mocked(db.programmeServiceRoleAssignment.updateMany).mock.calls).toHaveLength(3) + + const eventIdsUpdated = vi + .mocked(db.programmePartAssignment.updateMany) + .mock.calls.map(([args]) => (args?.where as { eventId: number }).eventId) + expect(eventIdsUpdated).toEqual([1, 2, 3]) + }) + // Regression pin — participants are Members (`assigneeId`, `assistantId` // reference Member.id). Filtering with a UserAccount.id would silently miss // every assignment whose Member.id differs from the assignee's account.id. From e44ad6cfe4995389d352eb37b64991b4d7b1f035 Mon Sep 17 00:00:00 2001 From: mindsers Date: Thu, 16 Jul 2026 01:27:00 +0200 Subject: [PATCH 3/4] refactor(dashboard): gate responsible-conflicts card and simplify plumbing Security: - Gate the `getResponsibleConflicts` call on `ProgramViewer` (or `ProgramManager`). The card deep-links to `/programs?hasConflicts=true` which requires `ProgramViewer`; users without it now skip the query and the card is never rendered for them. Cleanups (no behavioural change): - Extract the shared event/template/responsibles select shape into a single `eventWithResponsiblesSelect` const in `list-user-conflicts-in-range.server.ts`; derive the private `EventWithResponsibles` type via `Prisma.EventGetPayload` so it stays welded to the schema. - Inline `formatResponsibleConflictLabel` into `urgentResponsibleConflictItems` (single call site); label formatting is now covered directly by the item builder's own tests. - Inline `pickConflictModalTitle` into `DayOffConflictModal` and delete the now-empty `day-off-conflict-helpers.ts` + its test file. - Collapse the `let conflicts` + `if/else` in `days-off/new.tsx` into a ternary, hoisting the null-member log line above it. --- app/features/dashboard/routes/index.tsx | 12 ++- .../dashboard/ui/build-urgent-items.test.ts | 58 +++++++------- .../dashboard/ui/build-urgent-items.ts | 21 ++--- app/features/events/routes/days-off/new.tsx | 12 +-- .../list-user-conflicts-in-range.server.ts | 80 ++++++------------- .../events/ui/DayOffConflictModal.tsx | 9 +-- .../ui/day-off-conflict-helpers.test.ts | 31 ------- .../events/ui/day-off-conflict-helpers.ts | 8 -- 8 files changed, 78 insertions(+), 153 deletions(-) delete mode 100644 app/features/events/ui/day-off-conflict-helpers.test.ts delete mode 100644 app/features/events/ui/day-off-conflict-helpers.ts diff --git a/app/features/dashboard/routes/index.tsx b/app/features/dashboard/routes/index.tsx index 54dabe97..83e0d420 100644 --- a/app/features/dashboard/routes/index.tsx +++ b/app/features/dashboard/routes/index.tsx @@ -46,6 +46,10 @@ export function loader({ context }: Route.LoaderArgs) { const isTerritoriesManager = permissions.has(Permission.TerritoriesManager) const isProgramManager = permissions.has(Permission.ProgramManager) const canViewBoard = permissions.has(Permission.BoardViewer) + // The responsible-conflict card deep-links to /programs?hasConflicts=true. + // Gate the query the same way so we don't hand a user a link to a page + // they cannot open — mirrors the canViewBoard pattern below. + const canViewPrograms = permissions.has(Permission.ProgramViewer) || isProgramManager // Member-bound queries (territories, programme assignments) need the linked // Member id; account-bound queries (documents/views) use the UserAccount id. @@ -79,9 +83,11 @@ export function loader({ context }: Route.LoaderArgs) { safeQuery('absences', currentUser.id, () => getUpcomingAbsences(db, currentUser.id, currentUser.congregationId)), memberSafeQuery('next-meeting', mid => getNextMeeting(db, mid)), memberSafeQuery('dayoff-conflict', mid => getConflictingAssignments(db, mid)), - safeQuery('responsible-conflicts', currentUser.id, () => - getResponsibleConflicts(db, currentUser.id, isProgramManager), - ), + canViewPrograms + ? safeQuery('responsible-conflicts', currentUser.id, () => + getResponsibleConflicts(db, currentUser.id, isProgramManager), + ) + : Promise.resolve(null), ]) // Onboarding: count entities for admin checklist diff --git a/app/features/dashboard/ui/build-urgent-items.test.ts b/app/features/dashboard/ui/build-urgent-items.test.ts index 53d5e9e2..26c383b2 100644 --- a/app/features/dashboard/ui/build-urgent-items.test.ts +++ b/app/features/dashboard/ui/build-urgent-items.test.ts @@ -24,7 +24,6 @@ const { urgentDayoffConflictItems, urgentResponsibleConflictItems, urgentDocumentsItem, - formatResponsibleConflictLabel, } = await import('./build-urgent-items') beforeEach(() => { @@ -266,35 +265,6 @@ describe('urgentDayoffConflictItems', () => { // --- urgentResponsibleConflictItems --- -describe('formatResponsibleConflictLabel', () => { - it('uses the singular template with a single name when count is 1', () => { - const label = formatResponsibleConflictLabel({ - count: 1, - absenteeNames: ['Marie Dupont'], - totalAbsenteesCount: 1, - }) - expect(label).toBe('1 responsible conflict: Marie Dupont') - }) - - it('uses the plural template with all names joined by comma', () => { - const label = formatResponsibleConflictLabel({ - count: 3, - absenteeNames: ['Alice A', 'Bob B', 'Charlie C'], - totalAbsenteesCount: 3, - }) - expect(label).toBe('3 responsible conflicts: Alice A, Bob B, Charlie C') - }) - - it('appends "(+N more)" when there are additional unlisted absentees', () => { - const label = formatResponsibleConflictLabel({ - count: 5, - absenteeNames: ['Alice A', 'Bob B', 'Charlie C'], - totalAbsenteesCount: 5, - }) - expect(label).toBe('5 responsible conflicts: Alice A, Bob B, Charlie C (+2 more)') - }) -}) - describe('urgentResponsibleConflictItems', () => { it('returns empty array when the summary is null', () => { expect(urgentResponsibleConflictItems(null)).toEqual([]) @@ -313,9 +283,35 @@ describe('urgentResponsibleConflictItems', () => { expect(items).toHaveLength(1) expect(items[0].priority).toBe(1) expect(items[0].to).toBe('/programs?hasConflicts=true') - expect(items[0].label).toContain('Marie D.') expect(items[0].key).toBe('responsible-conflicts') }) + + it('uses the singular label with a single name when count is 1', () => { + const items = urgentResponsibleConflictItems({ + count: 1, + absenteeNames: ['Marie Dupont'], + totalAbsenteesCount: 1, + }) + expect(items[0].label).toBe('1 responsible conflict: Marie Dupont') + }) + + it('uses the plural label with all names joined by comma', () => { + const items = urgentResponsibleConflictItems({ + count: 3, + absenteeNames: ['Alice A', 'Bob B', 'Charlie C'], + totalAbsenteesCount: 3, + }) + expect(items[0].label).toBe('3 responsible conflicts: Alice A, Bob B, Charlie C') + }) + + it('appends "(+N more)" when there are additional unlisted absentees', () => { + const items = urgentResponsibleConflictItems({ + count: 5, + absenteeNames: ['Alice A', 'Bob B', 'Charlie C'], + totalAbsenteesCount: 5, + }) + expect(items[0].label).toBe('5 responsible conflicts: Alice A, Bob B, Charlie C (+2 more)') + }) }) // --- urgentDocumentsItem --- diff --git a/app/features/dashboard/ui/build-urgent-items.ts b/app/features/dashboard/ui/build-urgent-items.ts index 42c34709..b4a898df 100644 --- a/app/features/dashboard/ui/build-urgent-items.ts +++ b/app/features/dashboard/ui/build-urgent-items.ts @@ -130,26 +130,21 @@ export function urgentDocumentsItem(unreadCount: number | null): UrgentItem[] { ] } -// Exported (not inlined into `urgentResponsibleConflictItems`) so the -// composition can be unit-tested without JSX. -export function formatResponsibleConflictLabel(summary: ResponsibleConflictsSummary): string { +export function urgentResponsibleConflictItems(summary: ResponsibleConflictsSummary | null): UrgentItem[] { + if (!summary || summary.count === 0) return [] + const extraCount = Math.max(0, summary.totalAbsenteesCount - summary.absenteeNames.length) const extras = extraCount > 0 ? m.dashboard_urgent_responsible_conflict_extras({ count: String(extraCount) }) : '' const names = summary.absenteeNames.join(', ') + extras - - if (summary.count === 1) { - return m.dashboard_urgent_responsible_conflict_singular({ names }) - } - return m.dashboard_urgent_responsible_conflict_plural({ count: String(summary.count), names }) -} - -export function urgentResponsibleConflictItems(summary: ResponsibleConflictsSummary | null): UrgentItem[] { - if (!summary || summary.count === 0) return [] + const label = + summary.count === 1 + ? m.dashboard_urgent_responsible_conflict_singular({ names }) + : m.dashboard_urgent_responsible_conflict_plural({ count: String(summary.count), names }) return [ { key: 'responsible-conflicts', - label: formatResponsibleConflictLabel(summary), + label, to: '/programs?hasConflicts=true', icon: AlertTriangle, borderClass: 'border-l-amber-500 bg-amber-500/5', diff --git a/app/features/events/routes/days-off/new.tsx b/app/features/events/routes/days-off/new.tsx index 2a6bc7be..c12ba88e 100644 --- a/app/features/events/routes/days-off/new.tsx +++ b/app/features/events/routes/days-off/new.tsx @@ -127,19 +127,15 @@ export async function action({ request, context }: Route.ActionArgs) { } // When there are conflicts we skip the success flash — the modal is - // the confirmation. - let conflicts: Awaited> + // the confirmation. Accounts without a linked Member cannot own + // programme assignments, so we skip the check with a log line so ops + // can distinguish that branch from a genuine empty result. if (memberId == null) { - // Accounts without a linked Member cannot own programme assignments, - // so there is nothing to check. Logged so ops can distinguish this - // branch from a genuine "no conflicts found". logger.info( `Days off created for account without linked Member; skipped conflict check. User ID: ${currentUser.id}.`, ) - conflicts = [] - } else { - conflicts = await listUserConflictsInRange(db, memberId, startDate, endDate) } + const conflicts = memberId == null ? [] : await listUserConflictsInRange(db, memberId, startDate, endDate) if (conflicts.length > 0) { logger.info(`Days off created with ${conflicts.length} conflict(s). User ID: ${currentUser.id}.`) diff --git a/app/features/events/server/list-user-conflicts-in-range.server.ts b/app/features/events/server/list-user-conflicts-in-range.server.ts index fd9bed92..5cd2901e 100644 --- a/app/features/events/server/list-user-conflicts-in-range.server.ts +++ b/app/features/events/server/list-user-conflicts-in-range.server.ts @@ -1,3 +1,4 @@ +import type { Prisma } from '~/database/generated/client' import type { TransactionClient } from '~/shared/infra/db.server' import { accountDisplayName } from '~/shared/utils/display-name' @@ -7,6 +8,28 @@ export interface UserConflictInRange { responsibleName: string | null } +// Shared between the two `findMany` calls so the query shape stays in sync. +const eventWithResponsiblesSelect = { + startDate: true, + template: { + select: { + responsibles: { + select: { + user: { + select: { + firstname: true, + lastname: true, + member: { select: { firstname: true, lastname: true } }, + }, + }, + }, + }, + }, + }, +} satisfies Prisma.EventSelect + +type EventWithResponsibles = Prisma.EventGetPayload<{ select: typeof eventWithResponsiblesSelect }> + // `responsibleName` resolves through `accountDisplayName` so a template // responsible's linked Member name wins over the account fallback fields. // Untemplated events (or templates with no responsible assigned) return @@ -26,26 +49,7 @@ export async function listUserConflictsInRange( }, select: { name: true, - event: { - select: { - startDate: true, - template: { - select: { - responsibles: { - select: { - user: { - select: { - firstname: true, - lastname: true, - member: { select: { firstname: true, lastname: true } }, - }, - }, - }, - }, - }, - }, - }, - }, + event: { select: eventWithResponsiblesSelect }, }, }), db.programmeServiceRoleAssignment.findMany({ @@ -56,26 +60,7 @@ export async function listUserConflictsInRange( }, select: { name: true, - event: { - select: { - startDate: true, - template: { - select: { - responsibles: { - select: { - user: { - select: { - firstname: true, - lastname: true, - member: { select: { firstname: true, lastname: true } }, - }, - }, - }, - }, - }, - }, - }, - }, + event: { select: eventWithResponsiblesSelect }, }, }), ]) @@ -90,23 +75,10 @@ export async function listUserConflictsInRange( return merged } -type TemplateWithResponsibles = - | { - responsibles: Array<{ - user: { - firstname: string | null - lastname: string | null - member: { firstname: string; lastname: string } | null - } - }> - } - | null - | undefined - // A template can have any number of responsibles. Every named responsible // is surfaced so the absentee knows who to reach, deterministically ordered // by display name so the modal reads the same across renders. -function resolveResponsibleName(template: TemplateWithResponsibles): string | null { +function resolveResponsibleName(template: EventWithResponsibles['template']): string | null { const responsibles = template?.responsibles ?? [] if (responsibles.length === 0) return null diff --git a/app/features/events/ui/DayOffConflictModal.tsx b/app/features/events/ui/DayOffConflictModal.tsx index 29400a9b..9fd1f624 100644 --- a/app/features/events/ui/DayOffConflictModal.tsx +++ b/app/features/events/ui/DayOffConflictModal.tsx @@ -1,5 +1,4 @@ import type { UserConflictInRange } from '~/features/events/server/list-user-conflicts-in-range.server' -import { pickConflictModalTitle } from '~/features/events/ui/day-off-conflict-helpers' import * as m from '~/i18n/paraglide/messages' import { AlertDialog, @@ -22,10 +21,10 @@ interface DayOffConflictModalProps { const ROW_DATE_OPTIONS: Intl.DateTimeFormatOptions = { weekday: 'short', day: 'numeric', month: 'short' } export function DayOffConflictModal({ conflicts, timezone, open, onClose }: DayOffConflictModalProps) { - const title = pickConflictModalTitle(conflicts.length, { - singular: m.days_off_conflict_modal_title_singular, - plural: count => m.days_off_conflict_modal_title_plural({ count }), - }) + const title = + conflicts.length === 1 + ? m.days_off_conflict_modal_title_singular() + : m.days_off_conflict_modal_title_plural({ count: conflicts.length }) return ( (!next ? onClose() : undefined)}> diff --git a/app/features/events/ui/day-off-conflict-helpers.test.ts b/app/features/events/ui/day-off-conflict-helpers.test.ts deleted file mode 100644 index bd3a8fae..00000000 --- a/app/features/events/ui/day-off-conflict-helpers.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { pickConflictModalTitle } from './day-off-conflict-helpers' - -describe('pickConflictModalTitle', () => { - it('picks the singular title for a single conflict', () => { - const result = pickConflictModalTitle(1, { - singular: () => 'singular-copy', - plural: n => `plural-copy-${n}`, - }) - expect(result).toBe('singular-copy') - }) - - it('picks the plural title for two or more conflicts, forwarding the count', () => { - const result = pickConflictModalTitle(3, { - singular: () => 'singular-copy', - plural: n => `plural-copy-${n}`, - }) - expect(result).toBe('plural-copy-3') - }) - - // Guarding zero is defensive — the modal only renders when there is at - // least one conflict, but the helper must be total to keep call sites - // typed and simple. - it('picks the plural title for zero conflicts (defensive)', () => { - const result = pickConflictModalTitle(0, { - singular: () => 'singular-copy', - plural: n => `plural-copy-${n}`, - }) - expect(result).toBe('plural-copy-0') - }) -}) diff --git a/app/features/events/ui/day-off-conflict-helpers.ts b/app/features/events/ui/day-off-conflict-helpers.ts deleted file mode 100644 index 8e98ddcd..00000000 --- a/app/features/events/ui/day-off-conflict-helpers.ts +++ /dev/null @@ -1,8 +0,0 @@ -export interface ConflictTitleMessages { - singular: () => string - plural: (count: number) => string -} - -export function pickConflictModalTitle(count: number, messages: ConflictTitleMessages): string { - return count === 1 ? messages.singular() : messages.plural(count) -} From 629b99d75deab85f4633e49d4d00d23aa18fa28c Mon Sep 17 00:00:00 2001 From: mindsers Date: Thu, 16 Jul 2026 02:04:45 +0200 Subject: [PATCH 4/4] polish(events): sharpen the day-off conflict modal copy and visuals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copy: - Title now leads with "Attention :" and names the conflict directly: "Attention : conflit avec {une affectation planifiée | {count} affectations planifiées}". - Intro softens the warning while still nudging action: "Votre absence est bien enregistrée. Elle chevauche cependant des affectations déjà prévues — pensez à prévenir au plus vite les responsables concernés…" - Fallback row uses the definite article (`le responsable du programme`). - Button becomes a commitment: "J'ai compris, je les préviens". Visuals: - Amber `AlertTriangle` badge stacked above title + description, all centered — replaces the desktop `AlertDialogMedia` layout that put the icon in a left column with the header body on the right. - Conflict list wraps in an amber-tinted panel with a bullet dot per row, so the list reads as a distinct block instead of drifting body text under the intro. English strings mirror the French tone shift. --- app/features/events/ui/DayOffConflictModal.tsx | 16 +++++++++++----- app/i18n/messages/en.json | 10 +++++----- app/i18n/messages/fr.json | 10 +++++----- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/app/features/events/ui/DayOffConflictModal.tsx b/app/features/events/ui/DayOffConflictModal.tsx index 9fd1f624..0ddfd08c 100644 --- a/app/features/events/ui/DayOffConflictModal.tsx +++ b/app/features/events/ui/DayOffConflictModal.tsx @@ -1,3 +1,4 @@ +import { AlertTriangle } from 'lucide-react' import type { UserConflictInRange } from '~/features/events/server/list-user-conflicts-in-range.server' import * as m from '~/i18n/paraglide/messages' import { @@ -6,7 +7,6 @@ import { AlertDialogContent, AlertDialogDescription, AlertDialogFooter, - AlertDialogHeader, AlertDialogTitle, } from '~/shared/ui/alert-dialog' import { formatEventDate } from '~/shared/utils/event-time' @@ -29,12 +29,15 @@ export function DayOffConflictModal({ conflicts, timezone, open, onClose }: DayO return ( (!next ? onClose() : undefined)}> - +
+
+ +
{title} {m.days_off_conflict_modal_intro()} - +
-
    +
      {conflicts.map((c, idx) => { const date = formatEventDate(c.eventDate, timezone, 'fr-FR', ROW_DATE_OPTIONS) const text = @@ -49,7 +52,10 @@ export function DayOffConflictModal({ conflicts, timezone, open, onClose }: DayO // Conflicts have no stable id — server returns an aggregate; index is fine // because we never reorder within one modal instance. // biome-ignore lint/suspicious/noArrayIndexKey: aggregate row has no stable id -
    • {text}
      • +
    • +
      • ) })}
    diff --git a/app/i18n/messages/en.json b/app/i18n/messages/en.json index d28fd7e3..de4bd94b 100644 --- a/app/i18n/messages/en.json +++ b/app/i18n/messages/en.json @@ -1302,12 +1302,12 @@ "days_off_new_end_date": "End date", "days_off_new_invalid_dates": "Unable to add this absence. The dates are invalid.", "days_off_new_success": "Absence added successfully.", - "days_off_conflict_modal_title_singular": "You have an assignment during this absence", - "days_off_conflict_modal_title_plural": "You have {count} assignments during this absence", - "days_off_conflict_modal_intro": "Your absence has been saved. Please let the responsibles know so they can arrange a replacement:", + "days_off_conflict_modal_title_singular": "Warning: this absence conflicts with a scheduled assignment", + "days_off_conflict_modal_title_plural": "Warning: this absence conflicts with {count} scheduled assignments", + "days_off_conflict_modal_intro": "Your absence is saved. It does overlap assignments that are already scheduled, though — please reach out to the responsibles below as soon as you can so a replacement can be arranged:", "days_off_conflict_modal_row_named": "{assignment} on {date} — notify {responsible}", - "days_off_conflict_modal_row_fallback": "{assignment} on {date} — notify a programme responsible", - "days_off_conflict_modal_button": "Got it, I'll let them know", + "days_off_conflict_modal_row_fallback": "{assignment} on {date} — notify the programme responsible", + "days_off_conflict_modal_button": "Got it, I'll notify them", "dashboard_urgent_responsible_conflict_singular": "1 assignment conflict: {names}", "dashboard_urgent_responsible_conflict_plural": "{count} assignment conflicts: {names}", "dashboard_urgent_responsible_conflict_extras": " (+{count} more)", diff --git a/app/i18n/messages/fr.json b/app/i18n/messages/fr.json index 8c006156..bcd77b18 100644 --- a/app/i18n/messages/fr.json +++ b/app/i18n/messages/fr.json @@ -1304,12 +1304,12 @@ "days_off_new_end_date": "Date de fin", "days_off_new_invalid_dates": "Impossible d'ajouter cette absence. Les dates sont invalides.", "days_off_new_success": "Absence ajoutée avec succès.", - "days_off_conflict_modal_title_singular": "Vous avez une affectation pendant cette absence", - "days_off_conflict_modal_title_plural": "Vous avez {count} affectations pendant cette absence", - "days_off_conflict_modal_intro": "Votre absence est bien enregistrée. Pensez à prévenir les responsables pour qu'ils prévoient un remplaçant :", + "days_off_conflict_modal_title_singular": "Attention : conflit avec une affectation planifiée", + "days_off_conflict_modal_title_plural": "Attention : conflit avec {count} affectations planifiées", + "days_off_conflict_modal_intro": "Votre absence est bien enregistrée. Elle chevauche cependant des affectations déjà prévues — pensez à prévenir au plus vite les responsables concernés pour qu'un remplacement puisse être organisé :", "days_off_conflict_modal_row_named": "{assignment} le {date} — prévenir {responsible}", - "days_off_conflict_modal_row_fallback": "{assignment} le {date} — prévenir un responsable du programme", - "days_off_conflict_modal_button": "D'accord, je vais le prévenir", + "days_off_conflict_modal_row_fallback": "{assignment} le {date} — prévenir le responsable du programme", + "days_off_conflict_modal_button": "J'ai compris, je les préviens", "dashboard_urgent_responsible_conflict_singular": "1 conflit d'affectation : {names}", "dashboard_urgent_responsible_conflict_plural": "{count} conflits d'affectation : {names}", "dashboard_urgent_responsible_conflict_extras": " (+{count} autres)",