From 94feb2ba20994171ea40e17c31c262c90379251e Mon Sep 17 00:00:00 2001 From: mindsers Date: Sun, 23 Aug 2026 22:27:33 +0200 Subject: [PATCH 1/3] feat(territories): split the legacy campaign backfill into per-period campaigns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The campaigns migration collapsed all legacy type='campaign' attributions into one synthetic ended 'Campagne' per congregation. This follow-up data migration splits each synthetic into one campaign per date cluster (a gap of more than 30 days between an attribution's start and the latest end seen so far opens a new cluster), named by its period ('Campagne 03/2024'), repoints the attributions and drops the emptied synthetic. Synthetics are recognized by the backfill's exact stamps (name + activatedAt = startDate + endedAt = endDate), so campaigns that went through the real lifecycle — even ones named 'Campagne' — and single-cluster synthetics are untouched. Single data-modifying CTE chain (temp tables don't survive Prisma's statement execution); verified against seeded multi-cluster, decoy and single-cluster states on the dev database. --- .../migration.sql | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 app/database/migrations/20260823200000_split_legacy_campaign_backfill/migration.sql diff --git a/app/database/migrations/20260823200000_split_legacy_campaign_backfill/migration.sql b/app/database/migrations/20260823200000_split_legacy_campaign_backfill/migration.sql new file mode 100644 index 00000000..d8fc10a7 --- /dev/null +++ b/app/database/migrations/20260823200000_split_legacy_campaign_backfill/migration.sql @@ -0,0 +1,95 @@ +-- Follow-up to 20260823000000_add_publishing_campaigns: that backfill collapsed +-- every congregation's legacy `type = 'campaign'` attributions into ONE synthetic +-- ended campaign named 'Campagne'. Real history usually spans several distinct +-- drives, so split each synthetic campaign into one campaign per date cluster: +-- a gap of more than 30 days between an attribution's start and the latest end +-- seen so far opens a new cluster. Synthetic campaigns are recognized by the +-- exact stamps the backfill wrote (name 'Campagne', activatedAt = startDate, +-- endedAt = endDate) — a campaign that went through the real lifecycle never +-- has activation/end timestamps exactly equal to its day-granular bounds. +-- Single-cluster synthetics are left untouched (splitting would be a rename). +-- One data-modifying CTE chain: temp tables don't survive Prisma's statement +-- execution. + +WITH synth AS ( + SELECT id, "congregationId" + FROM "Campaign" + WHERE name = 'Campagne' + AND "activatedAt" = "startDate" + AND "endedAt" = "endDate" +), +attrs AS ( + SELECT a.id, + a."congregationId", + a."campaignId", + a."startDate", + COALESCE(a."endDate", a."startDate") AS eff_end + FROM "Attribution" a + JOIN synth s ON s.id = a."campaignId" +), +marked AS ( + SELECT attrs.*, + CASE + WHEN "startDate" > COALESCE( + MAX(eff_end) OVER ( + PARTITION BY "campaignId" + ORDER BY "startDate", id + ROWS BETWEEN UNBOUNDED PRECEDING AND 1 PRECEDING + ), + "startDate" + ) + INTERVAL '30 days' + THEN 1 ELSE 0 + END AS opens_cluster + FROM attrs +), +clustered AS ( + SELECT marked.*, + SUM(opens_cluster) OVER (PARTITION BY "campaignId" ORDER BY "startDate", id) AS cluster + FROM marked +), +-- Only campaigns that actually split into several clusters are rewritten. +bounds AS ( + SELECT "campaignId", + "congregationId", + cluster, + MIN("startDate") AS cluster_start, + MAX(eff_end) AS cluster_end + FROM clustered + WHERE "campaignId" IN ( + SELECT "campaignId" FROM clustered GROUP BY "campaignId" HAVING COUNT(DISTINCT cluster) > 1 + ) + GROUP BY "campaignId", "congregationId", cluster +), +-- One already-ended campaign per cluster, named by its period. Cluster bounds +-- within one congregation are disjoint (30-day gaps) and there is only one +-- synthetic campaign per congregation, so (congregationId, startDate, endDate) +-- uniquely identifies the created row for the repointing below. +created AS ( + INSERT INTO "Campaign" ("name", "startDate", "endDate", "activatedAt", "endedAt", "congregationId", "updatedAt") + SELECT 'Campagne ' || to_char(b.cluster_start, 'MM/YYYY'), + b.cluster_start, + b.cluster_end, + b.cluster_start, + b.cluster_end, + b."congregationId", + CURRENT_TIMESTAMP + FROM bounds b + RETURNING id, "congregationId", "startDate", "endDate" +), +repointed AS ( + UPDATE "Attribution" a + SET "campaignId" = c.id + FROM clustered lc + JOIN bounds b + ON b."campaignId" = lc."campaignId" AND b.cluster = lc.cluster + JOIN created c + ON c."congregationId" = b."congregationId" + AND c."startDate" = b.cluster_start + AND c."endDate" = b.cluster_end + WHERE a.id = lc.id + RETURNING lc."campaignId" AS old_campaign_id +) +-- The split synthetics are now empty (every attribution was repointed; the +-- backfill never paused anything) — drop them. +DELETE FROM "Campaign" +WHERE id IN (SELECT DISTINCT old_campaign_id FROM repointed); From 170cbff57c7e5c66417219ccfe97a684408bc361 Mon Sep 17 00:00:00 2001 From: mindsers Date: Mon, 24 Aug 2026 22:09:52 +0200 Subject: [PATCH 2/3] feat(events): move assignment eligibility onto parts, merge midweek talk kinds Trying the preset feature showed eligibility sits at the wrong level: two parts of the same kind can be done by completely different roles, so which roles may fill a slot belongs to the part, not to the kind. The preset keeps capability only (reader slot, slot labels, external-speaker rule, share message). - Drop PartPresetAllowedRole (schema, services, preset editor pickers) and the preset-wins resolution rule; part rows are the single source of truth - Part editors always show the role pickers, preset chosen or not - Migration materializes the previously effective eligibility into TemplatePartAllowedRole/EventPartAllowedRole so no part widens or narrows - With eligibility gone, nothing distinguishes the three seeded midweek talk kinds: merge spiritual-gems, spiritual-pearls and christian-life-talk into one midweek-talk kind ("Sujet VCM"), repointing existing parts - Archive format 2.6: no part-preset-allowed-roles file; importing a 2.5 archive folds the legacy kinds into midweek-talk and discards preset-level eligibility with a log line --- .../migration.sql | 74 ++++++++++++ app/database/schema.prisma | 20 ---- app/database/seed-marketing.ts | 2 +- app/features/events/index.ts | 1 - .../model/allowed-roles-resolution.test.ts | 56 --------- .../events/model/allowed-roles-resolution.ts | 57 ---------- .../events/model/part-preset-defaults.ts | 24 +--- app/features/events/model/part-preset.type.ts | 6 +- .../events/_edit-event-intents.server.ts | 7 +- .../events/schemas/part-preset.schema.test.ts | 22 ---- .../events/schemas/part-preset.schema.ts | 9 -- .../server/allowed-roles.queries.test.ts | 33 +----- .../events/server/allowed-roles.queries.ts | 59 ++-------- .../server/allowed-roles.server.test.ts | 107 ++---------------- .../events/server/allowed-roles.server.ts | 85 ++------------ .../events/server/event-parts.server.ts | 6 +- .../events/server/event-templates.server.ts | 4 +- .../events/server/part-presets.queries.ts | 5 +- .../events/server/part-presets.server.test.ts | 43 ------- .../events/server/part-presets.server.ts | 30 ----- .../server/seed-part-presets.server.test.ts | 8 +- .../events/server/seed-part-presets.server.ts | 11 +- .../server/seed-templates.server.test.ts | 3 +- .../events/server/seed-templates.server.ts | 8 +- app/features/events/ui/PartEditSheet.tsx | 59 +++++----- app/features/events/ui/PartPresetForm.tsx | 31 +---- .../routes/congregation/presets/edit.tsx | 19 +--- .../routes/congregation/presets/new.tsx | 16 +-- .../routes/congregation/templates/edit.tsx | 4 +- .../server/data-transfer.integration.test.ts | 4 - .../server/data-transfer.type.test.ts | 5 +- .../settings/server/data-transfer.type.ts | 12 +- .../server/export-congregation.server.ts | 7 -- .../server/import-congregation.server.ts | 9 +- .../server/import-part-presets.server.test.ts | 73 ++++++------ .../server/import-part-presets.server.ts | 70 ++++++++---- app/i18n/messages/en.json | 7 +- app/i18n/messages/fr.json | 7 +- app/shared/constants/constants.test.ts | 4 +- app/shared/constants/limits.ts | 6 +- scripts/check-tenant-scoping.ts | 1 - 41 files changed, 278 insertions(+), 736 deletions(-) create mode 100644 app/database/migrations/20260824000000_move_part_eligibility_off_presets/migration.sql delete mode 100644 app/features/events/model/allowed-roles-resolution.test.ts delete mode 100644 app/features/events/model/allowed-roles-resolution.ts diff --git a/app/database/migrations/20260824000000_move_part_eligibility_off_presets/migration.sql b/app/database/migrations/20260824000000_move_part_eligibility_off_presets/migration.sql new file mode 100644 index 00000000..823b6659 --- /dev/null +++ b/app/database/migrations/20260824000000_move_part_eligibility_off_presets/migration.sql @@ -0,0 +1,74 @@ +-- Eligibility (which roles may fill a slot) moves off the part kind and back +-- onto the parts themselves. Trying the preset feature showed the kind is the +-- wrong granularity: two parts of the same kind — two "Sujet VCM" — can +-- legitimately belong to different populations depending on where they sit in +-- the programme. The kind keeps capability only (reader slot, labels, +-- external-speaker rule, share message). +-- +-- Step 1 makes the change behaviour-neutral: under the old rule a kind with +-- roles configured for a slot decided that slot and the part's own rows lay +-- dormant. Materialize that effective answer into the part rows before the +-- preset rows disappear, so no existing part widens or narrows. + +DELETE FROM "TemplatePartAllowedRole" tpar +USING "TemplatePart" tp +WHERE tpar."partId" = tp."id" + AND EXISTS ( + SELECT 1 FROM "PartPresetAllowedRole" ppar + WHERE ppar."presetId" = tp."presetId" AND ppar."asKind" = tpar."asKind" + ); + +INSERT INTO "TemplatePartAllowedRole" ("partId", "roleId", "asKind", "congregationId") +SELECT tp."id", ppar."roleId", ppar."asKind", tp."congregationId" +FROM "TemplatePart" tp +JOIN "PartPresetAllowedRole" ppar ON ppar."presetId" = tp."presetId" +ON CONFLICT DO NOTHING; + +DELETE FROM "EventPartAllowedRole" epar +USING "EventPart" ep +WHERE epar."eventPartId" = ep."id" + AND EXISTS ( + SELECT 1 FROM "PartPresetAllowedRole" ppar + WHERE ppar."presetId" = ep."presetId" AND ppar."asKind" = epar."asKind" + ); + +INSERT INTO "EventPartAllowedRole" ("eventPartId", "roleId", "asKind", "congregationId") +SELECT ep."id", ppar."roleId", ppar."asKind", ep."congregationId" +FROM "EventPart" ep +JOIN "PartPresetAllowedRole" ppar ON ppar."presetId" = ep."presetId" +ON CONFLICT DO NOTHING; + +DROP TABLE "PartPresetAllowedRole"; + +-- Step 2: with eligibility gone, nothing distinguishes the three seeded +-- midweek talk kinds (Joyaux, Perles, Vie chrétienne) any more — merge them +-- into one "Sujet VCM" kind. Custom wording stored on the old rows (a rename +-- or an edited share message) is dropped; the merged kind starts on the +-- catalogue defaults. + +INSERT INTO "PartPreset" ("key", "scope", "hasReaderSlot", "allowExternalSpeaker", "isSystem", "congregationId", "updatedAt") +SELECT 'midweek-talk', 'part', false, true, true, c."id", CURRENT_TIMESTAMP +FROM "Congregation" c +ON CONFLICT ("key", "congregationId") DO NOTHING; + +UPDATE "TemplatePart" tp +SET "presetId" = np."id" +FROM "PartPreset" op, "PartPreset" np +WHERE tp."presetId" = op."id" + AND op."isSystem" = true + AND op."key" IN ('spiritual-gems', 'spiritual-pearls', 'christian-life-talk') + AND np."key" = 'midweek-talk' + AND np."congregationId" = op."congregationId"; + +UPDATE "EventPart" ep +SET "presetId" = np."id" +FROM "PartPreset" op, "PartPreset" np +WHERE ep."presetId" = op."id" + AND op."isSystem" = true + AND op."key" IN ('spiritual-gems', 'spiritual-pearls', 'christian-life-talk') + AND np."key" = 'midweek-talk' + AND np."congregationId" = op."congregationId"; + +DELETE FROM "PartPreset" +WHERE "isSystem" = true + AND "key" IN ('spiritual-gems', 'spiritual-pearls', 'christian-life-talk'); diff --git a/app/database/schema.prisma b/app/database/schema.prisma index a2f8d1d3..8bb9bdac 100644 --- a/app/database/schema.prisma +++ b/app/database/schema.prisma @@ -118,7 +118,6 @@ model Congregation { templateServicePartAllowedRoles TemplateServicePartAllowedRole[] eventServicePartAllowedRoles EventServicePartAllowedRole[] partPresets PartPreset[] - partPresetAllowedRoles PartPresetAllowedRole[] externalSpeakers ExternalSpeaker[] emergencyContacts EmergencyContact[] @@ -328,7 +327,6 @@ model Role { allowedForTemplateParts TemplatePartAllowedRole[] allowedForEventParts EventPartAllowedRole[] - allowedForPartPresets PartPresetAllowedRole[] allowedForTemplateServiceParts TemplateServicePartAllowedRole[] allowedForEventServiceParts EventServicePartAllowedRole[] allowedForBoardSections BoardSectionVisibilityRole[] @@ -1099,7 +1097,6 @@ model PartPreset { eventParts EventPart[] templateServiceParts TemplateServicePart[] eventServiceParts EventServicePart[] - allowedRoles PartPresetAllowedRole[] congregation Congregation @relation(fields: [congregationId], references: [id]) congregationId Int @@ -1112,23 +1109,6 @@ model PartPreset { @@index([congregationId]) } -// Which roles may be assigned to a part of this kind. Mirrors -// TemplatePartAllowedRole; `asKind` distinguishes the speaker and reader slots. -model PartPresetAllowedRole { - preset PartPreset @relation(fields: [presetId], references: [id], onDelete: Cascade) - presetId Int - role Role @relation(fields: [roleId], references: [id], onDelete: Cascade) - roleId Int - asKind String - - congregation Congregation @relation(fields: [congregationId], references: [id], onDelete: Cascade) - congregationId Int - - @@id([presetId, roleId, asKind]) - @@index([roleId]) - @@index([congregationId]) -} - model TemplatePartAllowedRole { part TemplatePart @relation(fields: [partId], references: [id], onDelete: Cascade) partId Int diff --git a/app/database/seed-marketing.ts b/app/database/seed-marketing.ts index 46f9efb2..e4536f41 100644 --- a/app/database/seed-marketing.ts +++ b/app/database/seed-marketing.ts @@ -541,7 +541,7 @@ async function cleanCongregationData(congregationId: number) { await prisma.eventTemplate.deleteMany({ where: { congregationId } }) // After the parts that point at them, before the congregation itself: the // FK to Congregation restricts, so leaving these behind makes the final - // congregation.delete fail. PartPresetAllowedRole cascades from here. + // congregation.delete fail. await prisma.partPreset.deleteMany({ where: { congregationId } }) await prisma.event.deleteMany({ where: { congregationId } }) await prisma.boardDynamicDocumentView.deleteMany({ diff --git a/app/features/events/index.ts b/app/features/events/index.ts index bb6ac412..80842cdb 100644 --- a/app/features/events/index.ts +++ b/app/features/events/index.ts @@ -1,6 +1,5 @@ // Public client-safe surface of the events feature. -export { partAllowedRolesToWrite, resolveAllowedRoleIds } from './model/allowed-roles-resolution' export { dayLabel, dayLabelShort } from './model/day-label' export { EventStatus } from './model/event-status.type' export { EventTemplateKey, isSystemTemplate } from './model/event-template.type' diff --git a/app/features/events/model/allowed-roles-resolution.test.ts b/app/features/events/model/allowed-roles-resolution.test.ts deleted file mode 100644 index 1006ff9f..00000000 --- a/app/features/events/model/allowed-roles-resolution.test.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { partAllowedRolesToWrite, resolveAllowedRoleIds } from './allowed-roles-resolution' - -describe('resolveAllowedRoleIds', () => { - it('uses the preset roles when it has any', () => { - expect(resolveAllowedRoleIds({ partRoleIds: [1, 2], presetRoleIds: [7, 8] })).toEqual([7, 8]) - }) - - it('falls back to the part when the preset has none', () => { - // Empty does not mean "nobody" — resolveEligibleUserIds reads an empty list - // as "any member". Treating an unconfigured preset as authoritative would - // silently hand every part using it the widest possible audience. - expect(resolveAllowedRoleIds({ partRoleIds: [1, 2], presetRoleIds: [] })).toEqual([1, 2]) - }) - - it('leaves an unrestricted part unrestricted', () => { - expect(resolveAllowedRoleIds({ partRoleIds: [], presetRoleIds: [] })).toEqual([]) - }) - - it('lets a preset narrow a part that had no restriction', () => { - expect(resolveAllowedRoleIds({ partRoleIds: [], presetRoleIds: [7] })).toEqual([7]) - }) - - it('cannot be used to remove a restriction the part carries', () => { - // The deliberate asymmetry with allowExternalSpeaker, where the preset can - // say no. Here "no roles" is the widest setting rather than the narrowest, - // so an empty preset can only ever mean "not configured". - expect(resolveAllowedRoleIds({ partRoleIds: [1], presetRoleIds: [] })).toEqual([1]) - }) - - it('does not merge the two sides', () => { - // A union would let a part quietly widen what its kind permits. - expect(resolveAllowedRoleIds({ partRoleIds: [1, 2], presetRoleIds: [3] })).toEqual([3]) - }) -}) - -describe('partAllowedRolesToWrite', () => { - it('passes both slots through when the part has no kind', () => { - expect( - partAllowedRolesToWrite({ partPresetId: null, allowedSpeakerRoleIds: [3], allowedReaderRoleIds: [4] }), - ).toEqual({ allowedSpeakerRoleIds: [3], allowedReaderRoleIds: [4] }) - }) - - it('passes an emptied selection through, so clearing still works', () => { - expect( - partAllowedRolesToWrite({ partPresetId: null, allowedSpeakerRoleIds: [], allowedReaderRoleIds: [] }), - ).toEqual({ allowedSpeakerRoleIds: [], allowedReaderRoleIds: [] }) - }) - - it('omits both slots when a kind owns eligibility', () => { - // Not [] — the part's own rows are the kind's fallback and must survive. - expect(partAllowedRolesToWrite({ partPresetId: 55, allowedSpeakerRoleIds: [], allowedReaderRoleIds: [] })).toEqual( - {}, - ) - }) -}) diff --git a/app/features/events/model/allowed-roles-resolution.ts b/app/features/events/model/allowed-roles-resolution.ts deleted file mode 100644 index 88503688..00000000 --- a/app/features/events/model/allowed-roles-resolution.ts +++ /dev/null @@ -1,57 +0,0 @@ -/** - * Which roles may be assigned to a slot: the preset's when it has any, - * otherwise the part's own. - * - * This is deliberately NOT the rule used for allowExternalSpeaker, where the - * preset wins even when it says no. The difference is what "empty" means. - * resolveEligibleUserIds reads an empty allowed-roles list as "any member" — - * the widest setting, not the narrowest. So an unconfigured preset is - * indistinguishable from one that permits everyone, and letting it win would - * silently hand every part using that kind the largest possible audience. - * - * The consequence, worth knowing: a preset can narrow eligibility or redefine - * it, but cannot be used to lift a restriction a part already carries. Removing - * one means clearing it on the part. - * - * It also makes adopting a kind safe on the read side — until roles are set on - * it, every part resolves to exactly the eligibility it had. Keeping that true - * on the write side is a separate problem, and the reason the part editor stops - * managing these rows once a kind is chosen: see partAllowedRolesToWrite. - * - * Named rather than positional because the two lists are the same type and the - * whole rule is which one wins; swapping them would compile and invert it. - */ -export function resolveAllowedRoleIds({ - partRoleIds, - presetRoleIds, -}: { - partRoleIds: number[] - presetRoleIds: number[] -}): number[] { - return presetRoleIds.length > 0 ? presetRoleIds : partRoleIds -} - -/** - * Which role slots the part editor actually managed. - * - * The editor hides its role pickers once a kind is chosen, and an unchecked - * checkbox submits nothing — so "the user cleared the selection" and "the - * picker was never rendered" arrive as the same empty field. The kind is what - * tells them apart: with one, eligibility belongs to the kind (see - * resolveAllowedRoleIds) and the part's own rows must be left alone, because - * they are what the kind falls back to while it restricts nobody. - * - * Omitting the keys rather than sending [] is the point — the services treat - * undefined as "not managed" and [] as "managed, and empty". - */ -export function partAllowedRolesToWrite(value: { - partPresetId: number | null - allowedSpeakerRoleIds: number[] - allowedReaderRoleIds: number[] -}): { allowedSpeakerRoleIds?: number[]; allowedReaderRoleIds?: number[] } { - if (value.partPresetId != null) return {} - return { - allowedSpeakerRoleIds: value.allowedSpeakerRoleIds, - allowedReaderRoleIds: value.allowedReaderRoleIds, - } -} diff --git a/app/features/events/model/part-preset-defaults.ts b/app/features/events/model/part-preset-defaults.ts index 62257a4c..4a83f37d 100644 --- a/app/features/events/model/part-preset-defaults.ts +++ b/app/features/events/model/part-preset-defaults.ts @@ -18,12 +18,10 @@ type MessageFn = (inputs?: Record, options?: { locale?: Locale }) const BUILT_IN_NAMES: Record = { [PartPresetKey.Prayer]: m.seed_preset_prayer, [PartPresetKey.Chairman]: m.seed_preset_chairman, - [PartPresetKey.SpiritualGems]: m.seed_preset_spiritual_gems, - [PartPresetKey.SpiritualPearls]: m.seed_preset_spiritual_pearls, + [PartPresetKey.MidweekTalk]: m.seed_preset_midweek_talk, [PartPresetKey.BibleReading]: m.seed_preset_bible_reading, [PartPresetKey.SchoolDemonstration]: m.seed_preset_school_demonstration, [PartPresetKey.SchoolTalk]: m.seed_preset_school_talk, - [PartPresetKey.ChristianLifeTalk]: m.seed_preset_christian_life_talk, [PartPresetKey.PublicTalk]: m.seed_preset_public_talk, [PartPresetKey.WatchtowerStudy]: m.seed_preset_watchtower_study, [PartPresetKey.CongregationBibleStudy]: m.seed_preset_congregation_bible_study, @@ -36,12 +34,10 @@ const BUILT_IN_NAMES: Record = { const BUILT_IN_SPEAKER_LABELS: Record = { [PartPresetKey.Prayer]: m.seed_preset_label_brother, [PartPresetKey.Chairman]: m.seed_preset_label_chairman, - [PartPresetKey.SpiritualGems]: m.seed_preset_label_speaker, - [PartPresetKey.SpiritualPearls]: m.seed_preset_label_conductor, + [PartPresetKey.MidweekTalk]: m.seed_preset_label_speaker, [PartPresetKey.BibleReading]: m.seed_preset_label_reader, [PartPresetKey.SchoolDemonstration]: m.seed_preset_label_publisher, [PartPresetKey.SchoolTalk]: m.seed_preset_label_speaker, - [PartPresetKey.ChristianLifeTalk]: m.seed_preset_label_speaker, [PartPresetKey.PublicTalk]: m.seed_preset_label_speaker, [PartPresetKey.WatchtowerStudy]: m.seed_preset_label_conductor, [PartPresetKey.CongregationBibleStudy]: m.seed_preset_label_conductor, @@ -60,18 +56,14 @@ export const SHARE_MESSAGES: Record> = { 'Bonjour {{assigneeFirstname}},\n\nTu as la prière le {{date}} à {{time}} ({{eventName}}).\n\n{{link}}', [PartPresetKey.Chairman]: 'Bonjour {{assigneeFirstname}},\n\nTu présides la réunion du {{date}} à {{time}}.\n\n{{link}}', - [PartPresetKey.SpiritualGems]: - 'Bonjour {{assigneeFirstname}},\n\nTu as un discours dans les Joyaux de la Parole de Dieu le {{date}} à {{time}}.\nSujet : {{topic}}\nDurée : {{duration}}\nNote : {{note}}\n\n{{link}}', - [PartPresetKey.SpiritualPearls]: - 'Bonjour {{assigneeFirstname}},\n\nTu conduis « Recherchons des perles spirituelles » le {{date}} à {{time}}.\nDurée : {{duration}}\nNote : {{note}}\n\n{{link}}', + [PartPresetKey.MidweekTalk]: + 'Bonjour {{assigneeFirstname}},\n\nTu as un sujet à la réunion Vie chrétienne et ministère le {{date}} à {{time}}.\nPartie : {{partName}}\nSujet : {{topic}}\nDurée : {{duration}}\nNote : {{note}}\n\n{{link}}', [PartPresetKey.BibleReading]: 'Bonjour {{assigneeFirstname}},\n\nTu as la lecture de la Bible le {{date}} à {{time}}.\nPassage : {{topic}}\nDurée : {{duration}}\nNote : {{note}}\n\n{{link}}', [PartPresetKey.SchoolDemonstration]: "Bonjour {{assigneeFirstname}},\n\nTu as un sujet de l'école le {{date}} à {{time}}.\nSujet : {{topic}}\nDurée : {{duration}}\nAvec : {{assistant}}\nNote : {{note}}\n\n{{link}}", [PartPresetKey.SchoolTalk]: "Bonjour {{assigneeFirstname}},\n\nTu as un discours de l'école le {{date}} à {{time}}.\nSujet : {{topic}}\nDurée : {{duration}}\nNote : {{note}}\n\n{{link}}", - [PartPresetKey.ChristianLifeTalk]: - 'Bonjour {{assigneeFirstname}},\n\nTu as un discours dans « Vie chrétienne » le {{date}} à {{time}}.\nSujet : {{topic}}\nDurée : {{duration}}\nNote : {{note}}\n\n{{link}}', [PartPresetKey.PublicTalk]: 'Bonjour {{assigneeFirstname}},\n\nTu donnes le discours public le {{date}} à {{time}}.\nThème : {{topic}}\nNote : {{note}}\n\n{{link}}', [PartPresetKey.WatchtowerStudy]: @@ -84,18 +76,14 @@ export const SHARE_MESSAGES: Record> = { 'Hi {{assigneeFirstname}},\n\nYou have the prayer on {{date}} at {{time}} ({{eventName}}).\n\n{{link}}', [PartPresetKey.Chairman]: 'Hi {{assigneeFirstname}},\n\nYou are chairing the meeting on {{date}} at {{time}}.\n\n{{link}}', - [PartPresetKey.SpiritualGems]: - "Hi {{assigneeFirstname}},\n\nYou have a talk in Treasures From God's Word on {{date}} at {{time}}.\nTopic: {{topic}}\nLength: {{duration}}\nNote: {{note}}\n\n{{link}}", - [PartPresetKey.SpiritualPearls]: - 'Hi {{assigneeFirstname}},\n\nYou are conducting Digging for Spiritual Gems on {{date}} at {{time}}.\nLength: {{duration}}\nNote: {{note}}\n\n{{link}}', + [PartPresetKey.MidweekTalk]: + 'Hi {{assigneeFirstname}},\n\nYou have a talk in the midweek meeting on {{date}} at {{time}}.\nPart: {{partName}}\nTopic: {{topic}}\nLength: {{duration}}\nNote: {{note}}\n\n{{link}}', [PartPresetKey.BibleReading]: 'Hi {{assigneeFirstname}},\n\nYou have the Bible reading on {{date}} at {{time}}.\nPassage: {{topic}}\nLength: {{duration}}\nNote: {{note}}\n\n{{link}}', [PartPresetKey.SchoolDemonstration]: 'Hi {{assigneeFirstname}},\n\nYou have a school demonstration on {{date}} at {{time}}.\nTopic: {{topic}}\nLength: {{duration}}\nWith: {{assistant}}\nNote: {{note}}\n\n{{link}}', [PartPresetKey.SchoolTalk]: 'Hi {{assigneeFirstname}},\n\nYou have a school talk on {{date}} at {{time}}.\nTopic: {{topic}}\nLength: {{duration}}\nNote: {{note}}\n\n{{link}}', - [PartPresetKey.ChristianLifeTalk]: - 'Hi {{assigneeFirstname}},\n\nYou have a talk in Living as Christians on {{date}} at {{time}}.\nTopic: {{topic}}\nLength: {{duration}}\nNote: {{note}}\n\n{{link}}', [PartPresetKey.PublicTalk]: 'Hi {{assigneeFirstname}},\n\nYou are giving the public talk on {{date}} at {{time}}.\nTheme: {{topic}}\nNote: {{note}}\n\n{{link}}', [PartPresetKey.WatchtowerStudy]: diff --git a/app/features/events/model/part-preset.type.ts b/app/features/events/model/part-preset.type.ts index b592eced..1cabded2 100644 --- a/app/features/events/model/part-preset.type.ts +++ b/app/features/events/model/part-preset.type.ts @@ -10,12 +10,12 @@ export enum PartPresetKey { Prayer = 'prayer', Chairman = 'chairman', - SpiritualGems = 'spiritual-gems', - SpiritualPearls = 'spiritual-pearls', + // One kind for every midweek-meeting talk (Joyaux, Perles, Vie chrétienne): + // eligibility lives on the parts, so nothing distinguished them any more. + MidweekTalk = 'midweek-talk', BibleReading = 'bible-reading', SchoolDemonstration = 'school-demonstration', SchoolTalk = 'school-talk', - ChristianLifeTalk = 'christian-life-talk', PublicTalk = 'public-talk', WatchtowerStudy = 'watchtower-study', CongregationBibleStudy = 'congregation-bible-study', diff --git a/app/features/events/routes/programs/events/_edit-event-intents.server.ts b/app/features/events/routes/programs/events/_edit-event-intents.server.ts index 29295b51..d3f25262 100644 --- a/app/features/events/routes/programs/events/_edit-event-intents.server.ts +++ b/app/features/events/routes/programs/events/_edit-event-intents.server.ts @@ -1,5 +1,4 @@ import { parseWithZod } from '@conform-to/zod' -import { partAllowedRolesToWrite } from '~/features/events' import { addPartSchema, addServiceSchema, @@ -114,7 +113,8 @@ async function handleAddPart( speakerLabel: partSpeakerLabel ?? null, readerLabel: partReaderLabel ?? null, presetId: partPresetId, - ...partAllowedRolesToWrite({ partPresetId, allowedSpeakerRoleIds, allowedReaderRoleIds }), + allowedSpeakerRoleIds, + allowedReaderRoleIds, congregationId, }, actorId, @@ -160,7 +160,8 @@ async function handleUpdatePart( speakerLabel: partSpeakerLabel ?? null, readerLabel: partReaderLabel ?? null, presetId: partPresetId, - ...partAllowedRolesToWrite({ partPresetId, allowedSpeakerRoleIds, allowedReaderRoleIds }), + allowedSpeakerRoleIds, + allowedReaderRoleIds, }, congregationId, actorId, diff --git a/app/features/events/schemas/part-preset.schema.test.ts b/app/features/events/schemas/part-preset.schema.test.ts index 21f5aade..ed1c8fac 100644 --- a/app/features/events/schemas/part-preset.schema.test.ts +++ b/app/features/events/schemas/part-preset.schema.test.ts @@ -84,25 +84,3 @@ describe('partPresetSchema', () => { expect(parsed.success && parsed.data.readerLabel).toBeNull() }) }) - -describe('partPresetSchema allowed roles', () => { - it('accepts a single role as a bare value', () => { - const parsed = partPresetSchema.safeParse({ ...base(), allowedSpeakerRoleIds: '4' }) - - expect(parsed.success && parsed.data.allowedSpeakerRoleIds).toEqual([4]) - }) - - it('accepts several roles', () => { - const parsed = partPresetSchema.safeParse({ ...base(), allowedReaderRoleIds: ['4', '9'] }) - - expect(parsed.success && parsed.data.allowedReaderRoleIds).toEqual([4, 9]) - }) - - it('treats no selection as unrestricted rather than invalid', () => { - // Empty means "any member" downstream — it is a legitimate choice, not an - // omission to reject. - const parsed = partPresetSchema.safeParse(base()) - - expect(parsed.success && parsed.data.allowedSpeakerRoleIds).toEqual([]) - }) -}) diff --git a/app/features/events/schemas/part-preset.schema.ts b/app/features/events/schemas/part-preset.schema.ts index 3075694e..bf636ea9 100644 --- a/app/features/events/schemas/part-preset.schema.ts +++ b/app/features/events/schemas/part-preset.schema.ts @@ -15,13 +15,6 @@ const checkboxField = z .optional() .transform(v => v === 'on') -// Same shape as the part form's role fields — a single value arrives as a -// string, several as an array, and nothing at all as an empty selection. -const roleIdsField = z.preprocess( - v => (Array.isArray(v) ? v : v == null || v === '' ? [] : [v]), - z.array(z.coerce.number().int().positive()), -) - export const partPresetSchema = z.object({ // Blank means "use the built-in name", which is why this is no longer // required: the placeholder in the form shows what blank will produce. @@ -53,8 +46,6 @@ export const partPresetSchema = z.object({ message: `Variable(s) inconnue(s) : ${unknown.map(name => `{{${name}}}`).join(', ')}. Disponibles : ${SHARE_VARIABLES.map(name => `{{${name}}}`).join(', ')}`, }) }), - allowedSpeakerRoleIds: roleIdsField.default([]), - allowedReaderRoleIds: roleIdsField.default([]), }) export type PartPresetFormValues = z.infer diff --git a/app/features/events/server/allowed-roles.queries.test.ts b/app/features/events/server/allowed-roles.queries.test.ts index 3fdd6bad..b5030b2d 100644 --- a/app/features/events/server/allowed-roles.queries.test.ts +++ b/app/features/events/server/allowed-roles.queries.test.ts @@ -2,9 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('~/shared/infra/db.server', () => ({ unscopedDb: { - eventPart: { findMany: vi.fn() }, eventPartAllowedRole: { findMany: vi.fn() }, - partPresetAllowedRole: { findMany: vi.fn() }, eventServicePartAllowedRole: { findMany: vi.fn() }, }, })) @@ -20,47 +18,25 @@ beforeEach(() => { describe('getPartAssignmentAllowedRoleIdsForParts', () => { // The event page resolves every slot on the programme at once. Asking per - // part meant three queries per slot and two slots per part, so a twelve-part - // programme spent seventy-two round trips answering one question. - it('answers every part and slot from a fixed number of queries', async () => { + // part meant a query per slot and two slots per part, so a twelve-part + // programme spent two dozen round trips answering one question. + it('answers every part and slot from one query', async () => { vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([ { eventPartId: 1, asKind: 'speaker', roleId: 42 }, { eventPartId: 2, asKind: 'reader', roleId: 43 }, ] as never) - vi.mocked(db.eventPart.findMany).mockResolvedValue([ - { id: 1, presetId: null }, - { id: 2, presetId: 55 }, - ] as never) - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([ - { presetId: 55, asKind: 'speaker', roleId: 700 }, - ] as never) const result = await getPartAssignmentAllowedRoleIdsForParts(db, [1, 2], 1) expect(result.get(1)).toEqual({ speaker: [42], reader: [] }) - // The kind wins for the slot it configures, and the part still answers the - // slot the kind left alone. - expect(result.get(2)).toEqual({ speaker: [700], reader: [43] }) + expect(result.get(2)).toEqual({ speaker: [], reader: [43] }) expect(db.eventPartAllowedRole.findMany).toHaveBeenCalledTimes(1) - expect(db.eventPart.findMany).toHaveBeenCalledTimes(1) - expect(db.partPresetAllowedRole.findMany).toHaveBeenCalledTimes(1) - }) - - it('skips the kind lookup entirely when no part has one', async () => { - vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([] as never) - vi.mocked(db.eventPart.findMany).mockResolvedValue([{ id: 1, presetId: null }] as never) - - const result = await getPartAssignmentAllowedRoleIdsForParts(db, [1], 1) - - expect(result.get(1)).toEqual({ speaker: [], reader: [] }) - expect(db.partPresetAllowedRole.findMany).not.toHaveBeenCalled() }) it('answers for a part that no longer exists rather than omitting it', async () => { // The caller indexes by part id; a missing entry would read as undefined // and crash the picker rather than offer nobody. vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([] as never) - vi.mocked(db.eventPart.findMany).mockResolvedValue([] as never) const result = await getPartAssignmentAllowedRoleIdsForParts(db, [9], 1) @@ -72,7 +48,6 @@ describe('getPartAssignmentAllowedRoleIdsForParts', () => { expect(result.size).toBe(0) expect(db.eventPartAllowedRole.findMany).not.toHaveBeenCalled() - expect(db.eventPart.findMany).not.toHaveBeenCalled() }) }) diff --git a/app/features/events/server/allowed-roles.queries.ts b/app/features/events/server/allowed-roles.queries.ts index fade67e1..99d49d44 100644 --- a/app/features/events/server/allowed-roles.queries.ts +++ b/app/features/events/server/allowed-roles.queries.ts @@ -1,13 +1,12 @@ -import { resolveAllowedRoleIds } from '~/features/events/model/allowed-roles-resolution' import type { TransactionClient } from '~/shared/infra/db.server' import type { PartRoleKind } from './allowed-roles.server' // Read side of assignment eligibility, split from allowed-roles.server.ts so // the write side keeps its own budget. Resolving a whole programme at once is // the only reason these exist — see getPartAssignmentAllowedRoleIds for the -// single-assignment answer and the rule they both apply. +// single-assignment answer. -/** Both slots of one part, as the eligibility rule leaves them. */ +/** Both slots of one part. */ export type PartSlotRoleIds = Record const EMPTY_SLOTS = (): PartSlotRoleIds => ({ speaker: [], reader: [] }) @@ -15,9 +14,9 @@ const EMPTY_SLOTS = (): PartSlotRoleIds => ({ speaker: [], reader: [] }) /** * The same answer as getPartAssignmentAllowedRoleIds, for a whole programme. * - * Asking part by part costs three queries per slot and two slots per part, so - * a twelve-part programme spent seventy-two round trips resolving one page. - * This reads the three tables once each and applies the rule in memory. + * Asking part by part costs a query per slot and two slots per part, so a + * twelve-part programme spent two dozen round trips resolving one page. This + * reads the table once. * * Every requested id gets an entry, including one whose part has since been * deleted: the caller indexes by id, and a missing key would read as undefined @@ -32,48 +31,12 @@ export async function getPartAssignmentAllowedRoleIdsForParts( if (eventPartIds.length === 0) return resolved for (const id of eventPartIds) resolved.set(id, EMPTY_SLOTS()) - const [partRows, parts] = await Promise.all([ - db.eventPartAllowedRole.findMany({ - where: { eventPartId: { in: eventPartIds }, congregationId }, - select: { eventPartId: true, asKind: true, roleId: true }, - }), - db.eventPart.findMany({ - where: { id: { in: eventPartIds }, congregationId }, - select: { id: true, presetId: true }, - }), - ]) - - const ownRoleIds = new Map() - for (const row of partRows) { - const slots = ownRoleIds.get(row.eventPartId) ?? EMPTY_SLOTS() - slots[row.asKind as PartRoleKind].push(row.roleId) - ownRoleIds.set(row.eventPartId, slots) - } - - const presetIds = [...new Set(parts.map(part => part.presetId).filter((id): id is number => id != null))] - const presetRoleIds = new Map() - if (presetIds.length > 0) { - const presetRows = await db.partPresetAllowedRole.findMany({ - where: { presetId: { in: presetIds }, congregationId }, - select: { presetId: true, asKind: true, roleId: true }, - }) - for (const row of presetRows) { - const slots = presetRoleIds.get(row.presetId) ?? EMPTY_SLOTS() - slots[row.asKind as PartRoleKind].push(row.roleId) - presetRoleIds.set(row.presetId, slots) - } - } - - for (const part of parts) { - const own = ownRoleIds.get(part.id) ?? EMPTY_SLOTS() - const fromPreset = part.presetId != null ? presetRoleIds.get(part.presetId) : undefined - resolved.set(part.id, { - speaker: resolveAllowedRoleIds({ - partRoleIds: own.speaker, - presetRoleIds: fromPreset?.speaker ?? [], - }), - reader: resolveAllowedRoleIds({ partRoleIds: own.reader, presetRoleIds: fromPreset?.reader ?? [] }), - }) + const rows = await db.eventPartAllowedRole.findMany({ + where: { eventPartId: { in: eventPartIds }, congregationId }, + select: { eventPartId: true, asKind: true, roleId: true }, + }) + for (const row of rows) { + resolved.get(row.eventPartId)?.[row.asKind as PartRoleKind].push(row.roleId) } return resolved diff --git a/app/features/events/server/allowed-roles.server.test.ts b/app/features/events/server/allowed-roles.server.test.ts index 540c2aec..948d6e4d 100644 --- a/app/features/events/server/allowed-roles.server.test.ts +++ b/app/features/events/server/allowed-roles.server.test.ts @@ -8,7 +8,6 @@ vi.mock('~/shared/infra/db.server', () => ({ templatePartAllowedRole: { findMany: vi.fn(), createMany: vi.fn(), deleteMany: vi.fn() }, eventPartAllowedRole: { findMany: vi.fn(), createMany: vi.fn(), deleteMany: vi.fn() }, eventPart: { findFirst: vi.fn(), findMany: vi.fn() }, - partPresetAllowedRole: { findMany: vi.fn(), createMany: vi.fn(), deleteMany: vi.fn() }, templateServicePartAllowedRole: { findMany: vi.fn(), createMany: vi.fn(), deleteMany: vi.fn() }, eventServicePartAllowedRole: { findMany: vi.fn(), createMany: vi.fn(), deleteMany: vi.fn() }, }, @@ -17,7 +16,6 @@ vi.mock('~/shared/infra/db.server', () => ({ const { getPartAssignmentAllowedRoleIds, resolveEligibleUserIds, - setPartPresetAllowedRoles, setTemplatePartAllowedRoles, setPartAssignmentAllowedRoles, setTemplateServicePartAllowedRoles, @@ -150,109 +148,22 @@ describe('setServicePartAssignmentAllowedRoles', () => { }) }) -describe('getPartAssignmentAllowedRoleIds with a preset', () => { - it("prefers the preset's roles for the slot", async () => { - vi.mocked(db.eventPart.findFirst).mockResolvedValue({ presetId: 55 } as never) - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([{ roleId: 700 }] as never) - vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([{ roleId: 1 }] as never) +describe('getPartAssignmentAllowedRoleIds', () => { + it("reads the part's own rows for the slot", async () => { + vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([{ roleId: 1 }, { roleId: 3 }] as never) - expect(await getPartAssignmentAllowedRoleIds(db, 9, 'speaker', 1)).toEqual([700]) - }) - - it('falls back to the part when the preset has none configured', async () => { - // An empty preset means "not configured", not "everyone" — reading it as - // authoritative would widen this part's audience to the whole congregation. - vi.mocked(db.eventPart.findFirst).mockResolvedValue({ presetId: 55 } as never) - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([] as never) - vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([{ roleId: 1 }] as never) - - expect(await getPartAssignmentAllowedRoleIds(db, 9, 'speaker', 1)).toEqual([1]) - }) - - it('uses the part alone when it has no preset', async () => { - vi.mocked(db.eventPart.findFirst).mockResolvedValue({ presetId: null } as never) - vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([{ roleId: 1 }] as never) - - expect(await getPartAssignmentAllowedRoleIds(db, 9, 'speaker', 1)).toEqual([1]) - expect(db.partPresetAllowedRole.findMany).not.toHaveBeenCalled() + expect(await getPartAssignmentAllowedRoleIds(db, 9, 'speaker', 1)).toEqual([1, 3]) + expect(db.eventPartAllowedRole.findMany).toHaveBeenCalledWith({ + where: { eventPartId: 9, asKind: 'speaker', congregationId: 1 }, + select: { roleId: true }, + }) }) it('keeps the two slots separate', async () => { - vi.mocked(db.eventPart.findFirst).mockResolvedValue({ presetId: 55 } as never) - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([{ roleId: 700 }] as never) vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([] as never) await getPartAssignmentAllowedRoleIds(db, 9, 'reader', 1) - expect(vi.mocked(db.partPresetAllowedRole.findMany).mock.calls[0]?.[0]?.where?.asKind).toBe('reader') - }) -}) - -describe('setPartAssignmentAllowedRoles on a part that carries a kind', () => { - // The read side resolves the kind first, so it answers "who may fill this - // slot". The write side must not reuse that answer as its baseline: the rows - // it adds and deletes belong to the part, and diffing them against the kind's - // list makes every write wrong in a different way. - it("clears the part's own rows when the selection is emptied", async () => { - vi.mocked(db.eventPart.findFirst).mockResolvedValue({ presetId: 55 } as never) - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([{ roleId: 700 }] as never) - vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([{ roleId: 42 }] as never) - - const diff = await setPartAssignmentAllowedRoles(db, 9, 'speaker', [], 1) - - expect(diff.removed).toEqual([42]) - expect(vi.mocked(db.eventPartAllowedRole.deleteMany).mock.calls[0]?.[0]?.where?.roleId).toEqual({ in: [42] }) - }) - - it('reports no change when the selection already matches the part', async () => { - vi.mocked(db.eventPart.findFirst).mockResolvedValue({ presetId: 55 } as never) - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([{ roleId: 700 }] as never) - vi.mocked(db.eventPartAllowedRole.findMany).mockResolvedValue([{ roleId: 42 }] as never) - - const diff = await setPartAssignmentAllowedRoles(db, 9, 'speaker', [42], 1) - - expect(diff).toEqual({ added: [], removed: [] }) - expect(db.eventPartAllowedRole.createMany).not.toHaveBeenCalled() - expect(db.eventPartAllowedRole.deleteMany).not.toHaveBeenCalled() - }) -}) - -describe('setPartPresetAllowedRoles', () => { - it('adds only the roles that are missing', async () => { - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([{ roleId: 1 }] as never) - - const diff = await setPartPresetAllowedRoles(db, 55, 'speaker', [1, 2], 1) - - expect(diff.added).toEqual([2]) - expect(vi.mocked(db.partPresetAllowedRole.createMany).mock.calls[0]?.[0]?.data).toEqual([ - { presetId: 55, roleId: 2, asKind: 'speaker', congregationId: 1 }, - ]) - }) - - it('removes the roles that are no longer wanted', async () => { - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([{ roleId: 1 }, { roleId: 2 }] as never) - - const diff = await setPartPresetAllowedRoles(db, 55, 'speaker', [1], 1) - - expect(diff.removed).toEqual([2]) - }) - - it('writes nothing when the selection is unchanged', async () => { - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([{ roleId: 1 }] as never) - - await setPartPresetAllowedRoles(db, 55, 'speaker', [1], 1) - - expect(db.partPresetAllowedRole.createMany).not.toHaveBeenCalled() - expect(db.partPresetAllowedRole.deleteMany).not.toHaveBeenCalled() - }) - - it('clearing every role leaves the kind unconfigured, not forbidden', async () => { - // Empty means "any member" downstream, so this is how a kind stops - // restricting rather than how it blocks everyone. - vi.mocked(db.partPresetAllowedRole.findMany).mockResolvedValue([{ roleId: 1 }] as never) - - const diff = await setPartPresetAllowedRoles(db, 55, 'speaker', [], 1) - - expect(diff.removed).toEqual([1]) + expect(vi.mocked(db.eventPartAllowedRole.findMany).mock.calls[0]?.[0]?.where?.asKind).toBe('reader') }) }) diff --git a/app/features/events/server/allowed-roles.server.ts b/app/features/events/server/allowed-roles.server.ts index 34177375..e0fd15c3 100644 --- a/app/features/events/server/allowed-roles.server.ts +++ b/app/features/events/server/allowed-roles.server.ts @@ -1,4 +1,3 @@ -import { resolveAllowedRoleIds } from '~/features/events/model/allowed-roles-resolution' import { findMembersWithAnyRole } from '~/shared/auth/permissions.server' import type { TransactionClient } from '~/shared/infra/db.server' @@ -54,32 +53,10 @@ async function getTemplatePartAllowedRoleIds( return rows.map(r => r.roleId) } -/** The part's own rows, before the kind has any say. */ -async function getPartOwnAllowedRoleIds( - db: TransactionClient, - eventPartId: number, - asKind: PartRoleKind, - congregationId: number, -): Promise { - const rows = await db.eventPartAllowedRole.findMany({ - where: { eventPartId, asKind, congregationId }, - select: { roleId: true }, - }) - return rows.map(r => r.roleId) -} - /** - * Which roles may fill a slot on an assignment. - * - * The kind decides when it has roles configured; otherwise the part's own rows - * apply. See resolveAllowedRoleIds for why an empty preset cannot win here — - * empty means "any member", so an unconfigured kind would widen rather than - * restrict. - * - * This is the eligibility answer, not the part's stored state. Anything writing - * `EventPartAllowedRole` must read getPartOwnAllowedRoleIds instead — diffing a - * write against a list that may belong to the kind deletes rows the part never - * had and leaves the ones it does. + * Which roles may fill a slot on an assignment. The part's own rows are the + * single source of truth — the kind carries capability (reader slot, labels, + * share message), never eligibility. Empty means "any member". */ export async function getPartAssignmentAllowedRoleIds( db: TransactionClient, @@ -87,20 +64,11 @@ export async function getPartAssignmentAllowedRoleIds( asKind: PartRoleKind, congregationId: number, ): Promise { - const partRoleIds = await getPartOwnAllowedRoleIds(db, eventPartId, asKind, congregationId) - - const part = await db.eventPart.findFirst({ - where: { id: eventPartId, congregationId }, - select: { presetId: true }, - }) - if (!part?.presetId) return partRoleIds - - const presetRows = await db.partPresetAllowedRole.findMany({ - where: { presetId: part.presetId, asKind, congregationId }, + const rows = await db.eventPartAllowedRole.findMany({ + where: { eventPartId, asKind, congregationId }, select: { roleId: true }, }) - - return resolveAllowedRoleIds({ partRoleIds, presetRoleIds: presetRows.map(r => r.roleId) }) + return rows.map(r => r.roleId) } async function getTemplateServicePartAllowedRoleIds( @@ -143,45 +111,6 @@ function diffRoleIds(previous: number[], desired: number[]): DiffResult { } } -async function getPartPresetAllowedRoleIds( - db: TransactionClient, - presetId: number, - asKind: PartRoleKind, - congregationId: number, -): Promise { - const rows = await db.partPresetAllowedRole.findMany({ - where: { presetId, asKind, congregationId }, - select: { roleId: true }, - }) - return rows.map(r => r.roleId) -} - -/** Mirrors setTemplatePartAllowedRoles, for the kind rather than one part. */ -export async function setPartPresetAllowedRoles( - db: TransactionClient, - presetId: number, - asKind: PartRoleKind, - desiredRoleIds: number[], - congregationId: number, -): Promise { - const previous = await getPartPresetAllowedRoleIds(db, presetId, asKind, congregationId) - const diff = diffRoleIds(previous, desiredRoleIds) - if (diff.added.length === 0 && diff.removed.length === 0) return diff - - if (diff.removed.length > 0) { - await db.partPresetAllowedRole.deleteMany({ - where: { presetId, asKind, congregationId, roleId: { in: diff.removed } }, - }) - } - if (diff.added.length > 0) { - await db.partPresetAllowedRole.createMany({ - data: diff.added.map(roleId => ({ presetId, roleId, asKind, congregationId })), - skipDuplicates: true, - }) - } - return diff -} - export async function setTemplatePartAllowedRoles( db: TransactionClient, partId: number, @@ -214,7 +143,7 @@ export async function setPartAssignmentAllowedRoles( desiredRoleIds: number[], congregationId: number, ): Promise { - const previous = await getPartOwnAllowedRoleIds(db, eventPartId, asKind, congregationId) + const previous = await getPartAssignmentAllowedRoleIds(db, eventPartId, asKind, congregationId) const diff = diffRoleIds(previous, desiredRoleIds) if (diff.added.length === 0 && diff.removed.length === 0) return diff diff --git a/app/features/events/server/event-parts.server.ts b/app/features/events/server/event-parts.server.ts index e65fb58a..1e68352c 100644 --- a/app/features/events/server/event-parts.server.ts +++ b/app/features/events/server/event-parts.server.ts @@ -97,10 +97,8 @@ const NO_ROLE_CHANGE = { added: [] as number[], removed: [] as number[] } * Write the slots the caller actually managed. * * Undefined and [] are different: [] is "the editor offered a selection and it - * is empty", undefined is "the editor never showed one". The part form hides - * its role pickers once a kind is chosen, so it sends undefined — and the - * part's own rows must survive, because that is what the kind falls back to - * while it restricts nobody. + * is empty" and clears the slot's rows, undefined is "this caller does not + * manage eligibility" and leaves them alone. */ async function writePartAllowedRoles( db: TransactionClient, diff --git a/app/features/events/server/event-templates.server.ts b/app/features/events/server/event-templates.server.ts index 7dab12a5..dc885594 100644 --- a/app/features/events/server/event-templates.server.ts +++ b/app/features/events/server/event-templates.server.ts @@ -88,8 +88,8 @@ export async function upsertTemplatePart( // The kind this part defaults to. Null where the template genuinely cannot // know — the ministry parts change kind weekly and are set per event. presetId?: number | null - // Optional on purpose: undefined means the editor did not manage the slot, - // [] means it did and the selection is empty. See partAllowedRolesToWrite. + // Optional on purpose: undefined means the caller does not manage the + // slot, [] means it does and the selection is empty. allowedSpeakerRoleIds?: number[] allowedReaderRoleIds?: number[] }, diff --git a/app/features/events/server/part-presets.queries.ts b/app/features/events/server/part-presets.queries.ts index 72a68703..6a31f594 100644 --- a/app/features/events/server/part-presets.queries.ts +++ b/app/features/events/server/part-presets.queries.ts @@ -66,8 +66,5 @@ export function listPartPresetsForSettings(db: TransactionClient, congregationId /** Returns null when the preset does not exist in this congregation. */ export function getPartPresetById(db: TransactionClient, id: number, congregationId: number) { - return db.partPreset.findFirst({ - where: { id, congregationId }, - include: { allowedRoles: { select: { roleId: true, asKind: true } } }, - }) + return db.partPreset.findFirst({ where: { id, congregationId } }) } diff --git a/app/features/events/server/part-presets.server.test.ts b/app/features/events/server/part-presets.server.test.ts index 00b268c3..9034869b 100644 --- a/app/features/events/server/part-presets.server.test.ts +++ b/app/features/events/server/part-presets.server.test.ts @@ -1,10 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -const setPartPresetAllowedRoles = vi.fn().mockResolvedValue({ added: [], removed: [] }) -vi.mock('~/features/events/server/allowed-roles.server', () => ({ - setPartPresetAllowedRoles: (...args: unknown[]) => setPartPresetAllowedRoles(...args), -})) - vi.mock('~/shared/domain/audit.server', () => ({ audit: vi.fn(), AuditAction: { @@ -36,8 +31,6 @@ function input(overrides: Record = {}) { readerLabel: null, allowExternalSpeaker: true, shareMessage: 'Bonjour {{assigneeFirstname}}, tu as {{partName}} le {{date}}.', - allowedSpeakerRoleIds: [], - allowedReaderRoleIds: [], ...overrides, } } @@ -52,7 +45,6 @@ function updatedData(db: ReturnType) { beforeEach(() => { vi.resetAllMocks() - setPartPresetAllowedRoles.mockResolvedValue({ added: [], removed: [] }) }) describe('createPartPreset', () => { @@ -283,38 +275,3 @@ describe('deletePartPreset', () => { expect(db.partPreset.delete).toHaveBeenCalled() }) }) - -describe('preset allowed roles', () => { - it('stores the roles chosen for each slot', async () => { - const db = makeDb() - - await createPartPreset( - db as never, - input({ hasReaderSlot: true, allowedSpeakerRoleIds: [4], allowedReaderRoleIds: [9] }), - 1, - 7, - ) - - expect(setPartPresetAllowedRoles).toHaveBeenCalledWith(expect.anything(), 1, 'speaker', [4], 1) - expect(setPartPresetAllowedRoles).toHaveBeenCalledWith(expect.anything(), 1, 'reader', [9], 1) - }) - - it('clears reader roles for a kind that has no reader slot', async () => { - // Otherwise the selection applies to a slot nobody is ever offered, and - // silently comes back if the slot is re-enabled later. - const db = makeDb() - - await createPartPreset(db as never, input({ hasReaderSlot: false, allowedReaderRoleIds: [9] }), 1, 7) - - expect(setPartPresetAllowedRoles).toHaveBeenCalledWith(expect.anything(), 1, 'reader', [], 1) - }) - - it('writes them on update too', async () => { - const db = makeDb() - db.partPreset.findFirst.mockResolvedValue({ id: 5, key: 'x', isSystem: false } as never) - - await updatePartPreset(db as never, 5, input({ allowedSpeakerRoleIds: [4] }), 1, 7) - - expect(setPartPresetAllowedRoles).toHaveBeenCalledWith(expect.anything(), 5, 'speaker', [4], 1) - }) -}) diff --git a/app/features/events/server/part-presets.server.ts b/app/features/events/server/part-presets.server.ts index 8fab513b..d8582262 100644 --- a/app/features/events/server/part-presets.server.ts +++ b/app/features/events/server/part-presets.server.ts @@ -1,6 +1,5 @@ import { PartPresetScope } from '~/features/events/model/part-preset.type' import { partPresetName } from '~/features/events/model/part-preset-defaults' -import { setPartPresetAllowedRoles } from '~/features/events/server/allowed-roles.server' import { AuditAction, audit } from '~/shared/domain/audit.server' import type { TransactionClient } from '~/shared/infra/db.server' @@ -15,8 +14,6 @@ export interface PartPresetInput { readerLabel: string | null allowExternalSpeaker: boolean shareMessage: string | null - allowedSpeakerRoleIds: number[] - allowedReaderRoleIds: number[] } /** @@ -67,29 +64,6 @@ function isUniqueViolation(error: unknown): boolean { return typeof error === 'object' && error !== null && (error as { code?: string }).code === 'P2002' } -/** - * Eligibility for the two slots. - * - * A kind with no reader slot cannot have reader roles — the selection would - * apply to a slot that is never offered, and would come back into effect if the - * slot were later re-enabled without anyone revisiting it. - */ -async function writeAllowedRoles( - db: TransactionClient, - presetId: number, - data: PartPresetInput, - congregationId: number, -): Promise { - await setPartPresetAllowedRoles(db, presetId, 'speaker', data.allowedSpeakerRoleIds, congregationId) - await setPartPresetAllowedRoles( - db, - presetId, - 'reader', - data.hasReaderSlot ? data.allowedReaderRoleIds : [], - congregationId, - ) -} - export async function createPartPreset( db: TransactionClient, data: PartPresetInput, @@ -120,8 +94,6 @@ export async function createPartPreset( } if (!preset) throw new Error('createPartPreset: no preset created') - await writeAllowedRoles(db, preset.id, data, congregationId) - audit({ action: AuditAction.PartPresetCreated, congregationId, @@ -156,8 +128,6 @@ export async function updatePartPreset( data: normalize(data), }) - await writeAllowedRoles(db, id, data, congregationId) - audit({ action: AuditAction.PartPresetUpdated, congregationId, diff --git a/app/features/events/server/seed-part-presets.server.test.ts b/app/features/events/server/seed-part-presets.server.test.ts index ca9bcfde..127cd028 100644 --- a/app/features/events/server/seed-part-presets.server.test.ts +++ b/app/features/events/server/seed-part-presets.server.test.ts @@ -90,9 +90,9 @@ describe('seedDefaultPartPresets', () => { expect(rowFor(db, 'watchtower-study')?.hasReaderSlot).toBe(true) }) - it('allows an external speaker on prayer but not on spiritual gems', async () => { - // Both confirmed explicitly — spiritual gems is a local assignment, whereas - // a visiting brother may offer prayer. + it('allows an external speaker on prayer but not on the Bible reading', async () => { + // Both confirmed explicitly — the Bible reading is a local assignment, + // whereas a visiting brother may offer prayer. const db = makeDb() db.partPreset.findFirst.mockResolvedValue(null as never) db.partPreset.create.mockResolvedValue({} as never) @@ -100,7 +100,7 @@ describe('seedDefaultPartPresets', () => { await seedDefaultPartPresets(db, 1, 'fr') expect(rowFor(db, 'prayer')?.allowExternalSpeaker).toBe(true) - expect(rowFor(db, 'spiritual-gems')?.allowExternalSpeaker).toBe(false) + expect(rowFor(db, 'bible-reading')?.allowExternalSpeaker).toBe(false) }) it.each(['fr', 'en'] as const)('uses only known variables in every %s message', async locale => { diff --git a/app/features/events/server/seed-part-presets.server.ts b/app/features/events/server/seed-part-presets.server.ts index ec661e3b..d9f124e5 100644 --- a/app/features/events/server/seed-part-presets.server.ts +++ b/app/features/events/server/seed-part-presets.server.ts @@ -22,14 +22,13 @@ const PRESETS: PresetCapability[] = [ // A visiting brother may offer prayer. { key: PartPresetKey.Prayer, hasReaderSlot: false, allowExternalSpeaker: true }, { key: PartPresetKey.Chairman, hasReaderSlot: false, allowExternalSpeaker: false }, - // Local assignment. - { key: PartPresetKey.SpiritualGems, hasReaderSlot: false, allowExternalSpeaker: false }, - { key: PartPresetKey.SpiritualPearls, hasReaderSlot: false, allowExternalSpeaker: false }, + // One kind for every midweek-meeting talk (Joyaux, Perles, Vie chrétienne). + // External speakers stay possible: a visiting brother may take a talk. + { key: PartPresetKey.MidweekTalk, hasReaderSlot: false, allowExternalSpeaker: true }, { key: PartPresetKey.BibleReading, hasReaderSlot: false, allowExternalSpeaker: false }, // The only school part with two people on stage. { key: PartPresetKey.SchoolDemonstration, hasReaderSlot: true, allowExternalSpeaker: false }, { key: PartPresetKey.SchoolTalk, hasReaderSlot: false, allowExternalSpeaker: false }, - { key: PartPresetKey.ChristianLifeTalk, hasReaderSlot: false, allowExternalSpeaker: true }, { key: PartPresetKey.PublicTalk, hasReaderSlot: false, allowExternalSpeaker: true }, { key: PartPresetKey.WatchtowerStudy, hasReaderSlot: true, allowExternalSpeaker: false }, { key: PartPresetKey.CongregationBibleStudy, hasReaderSlot: true, allowExternalSpeaker: false }, @@ -46,10 +45,6 @@ export const PART_PRESET_COUNT = PRESETS.length * * The locale argument is retained for call-site compatibility and is no longer * used: nothing language-specific is stored any more. - * - * Deliberately does not populate allowedRoles either. Eligibility is set on the - * kind through the preset editor, and a seeded restriction nobody asked for - * would narrow who can be assigned from the first day. */ // biome-ignore lint/suspicious/noExplicitAny: matches seedDefaultTemplates, called with a scoped client export async function seedDefaultPartPresets(db: any, congregationId: number, _locale: Locale): Promise { diff --git a/app/features/events/server/seed-templates.server.test.ts b/app/features/events/server/seed-templates.server.test.ts index 4c163290..1a02404e 100644 --- a/app/features/events/server/seed-templates.server.test.ts +++ b/app/features/events/server/seed-templates.server.test.ts @@ -6,9 +6,8 @@ const { seedDefaultTemplates } = await import('./seed-templates.server') // mistaken for a coincidental match. const SEEDED_PRESETS = [ { id: 901, key: 'prayer' }, - { id: 902, key: 'spiritual-gems' }, + { id: 902, key: 'midweek-talk' }, { id: 903, key: 'bible-reading' }, - { id: 904, key: 'christian-life-talk' }, ] function makeDb() { diff --git a/app/features/events/server/seed-templates.server.ts b/app/features/events/server/seed-templates.server.ts index ddd0a624..af3d14f6 100644 --- a/app/features/events/server/seed-templates.server.ts +++ b/app/features/events/server/seed-templates.server.ts @@ -69,7 +69,7 @@ function getTemplates(locale: Locale): TemplateDefinition[] { }, { name: m.seed_part_discourse({}, { locale }), - preset: PartPresetKey.SpiritualGems, + preset: PartPresetKey.MidweekTalk, section: m.seed_section_spiritual_gems({}, { locale }), order: 2, durationMin: 10, @@ -77,7 +77,7 @@ function getTemplates(locale: Locale): TemplateDefinition[] { }, { name: m.seed_part_search_spiritual_pearls({}, { locale }), - preset: PartPresetKey.SpiritualPearls, + preset: PartPresetKey.MidweekTalk, section: m.seed_section_spiritual_gems({}, { locale }), order: 3, durationMin: 10, @@ -115,7 +115,7 @@ function getTemplates(locale: Locale): TemplateDefinition[] { { name: m.seed_part_song({}, { locale }), section: '', order: 8, durationMin: 5, allowExternalSpeaker: false }, { name: m.seed_part_first_part({}, { locale }), - preset: PartPresetKey.ChristianLifeTalk, + preset: PartPresetKey.MidweekTalk, section: m.seed_section_christian_life({}, { locale }), order: 9, durationMin: null, @@ -123,7 +123,7 @@ function getTemplates(locale: Locale): TemplateDefinition[] { }, { name: m.seed_part_second_part({}, { locale }), - preset: PartPresetKey.ChristianLifeTalk, + preset: PartPresetKey.MidweekTalk, section: m.seed_section_christian_life({}, { locale }), order: 10, durationMin: null, diff --git a/app/features/events/ui/PartEditSheet.tsx b/app/features/events/ui/PartEditSheet.tsx index 4f9b57ef..6e54effa 100644 --- a/app/features/events/ui/PartEditSheet.tsx +++ b/app/features/events/ui/PartEditSheet.tsx @@ -223,10 +223,11 @@ export function PartEditSheet({ {partSpeakerLabel({ speakerLabel: capability.speakerLabel, readerLabel: null })}
- {/* Only offered without a kind. With one, the labels, the - external-speaker rule and the eligible roles all belong to - the preset — editing them here would suggest an override the - model does not have. The summary above shows what applies. */} + {/* Label and external-speaker rule are capability and belong to + the kind when one is chosen — editing them here would suggest + an override the model does not have. The summary above shows + what applies. Eligibility stays per part: two parts of the + same kind can be done by different roles. */} {!selectedPreset && ( <>
@@ -249,20 +250,18 @@ export function PartEditSheet({
)} - {!selectedPreset && ( -
- - -

{m.programs_edit_part_external_speaker_note()}

-
- )} +
+ + +

{m.programs_edit_part_external_speaker_note()}

+
@@ -286,19 +285,17 @@ export function PartEditSheet({ /> )} - {!selectedPreset && ( -
- - -
- )} +
+ + +
)} diff --git a/app/features/events/ui/PartPresetForm.tsx b/app/features/events/ui/PartPresetForm.tsx index 29f2b656..6da84704 100644 --- a/app/features/events/ui/PartPresetForm.tsx +++ b/app/features/events/ui/PartPresetForm.tsx @@ -7,7 +7,6 @@ import { Button } from '~/shared/ui/button' import { Checkbox } from '~/shared/ui/checkbox' import { Input } from '~/shared/ui/input' import { Label } from '~/shared/ui/label' -import { type RoleOption, RolePicker } from '~/shared/ui/RolePicker' import { Textarea } from '~/shared/ui/textarea' type PartPresetFormProps = { @@ -15,7 +14,6 @@ type PartPresetFormProps = { // from what the action will accept. preset: PartPresetFormValues | null isSystem: boolean - roles: RoleOption[] /** Catalogue wording, shown when a field is blank. Leaving a field empty is how a congregation keeps the built-in text and follows its language. */ placeholders?: { name: string; speakerLabel: string; readerLabel: string; shareMessage: string } @@ -42,7 +40,7 @@ const PREVIEW_CONTEXT: ShareMessageContext = { link: 'https://unitae.app/board', } -export function PartPresetForm({ preset, isSystem, roles, placeholders, errors }: PartPresetFormProps) { +export function PartPresetForm({ preset, isSystem, placeholders, errors }: PartPresetFormProps) { const [message, setMessage] = useState(preset?.shareMessage ?? '') const [hasReaderSlot, setHasReaderSlot] = useState(preset?.hasReaderSlot ?? false) @@ -127,33 +125,6 @@ export function PartPresetForm({ preset, isSystem, roles, placeholders, errors } )} - {/* Eligibility belongs to the kind, so it is set once here rather than - repeated on every part that uses it. An empty selection means any - member — that is the widest setting, not the narrowest. */} -
- - -
- - {hasReaderSlot && ( -
- - -
- )} -