Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-- Drops `Member.type`. It cached a member's standing pioneer status; PioneerEnrolment is the source
-- of truth for the same fact and has been since the plan/actual split. The preceding release moved
-- every reader onto the stints and backfilled the members who had a type but no stint, so this file
-- only removes what nothing reads any more.
--
-- Contract: no change in who is a pioneer. Anyone whose status came from this column already has an
-- ongoing stint carrying it.
--
-- The CHECK constraint has to go first — it references the column, so the DROP COLUMN would fail on
-- a dependency error rather than cascading.

ALTER TABLE "Member" DROP CONSTRAINT IF EXISTS "member_pioneer_requires_baptism";

ALTER TABLE "Member" DROP COLUMN "type";
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,26 @@ function migrationStatements(): string[] {
.filter(statement => statement.length > 0)
}

// A later migration drops `Member.type`, so by the time this test runs against a migrated database
// the column this backfill reads is gone. Recreate a minimal stand-in — `IF NOT EXISTS` makes it a
// no-op on a database where the drop has not been applied, so the test behaves identically in both.
// The PublisherType enum itself survives — PioneerEnrolment and PublisherActivity still use it —
// and the stand-in must BE that enum: the migration selects this column straight into
// PioneerEnrolment.type, which a text column cannot satisfy.
// Prisma's client speaks the enum NAMES; the database stores the `@map`-ed values. Raw SQL needs
// the latter, and the mapping is only in schema.prisma, so it is spelled out here.
const DB_ENUM_VALUE: Record<PublisherType, string> = {
[PublisherType.Normal]: 'normal',
[PublisherType.PionnierAuxiliaires]: 'pionnier-auxiliaires',
[PublisherType.PionnierPermanant]: 'pionnier-permanant',
[PublisherType.PionnierSpecial]: 'pionnier-special',
[PublisherType.Missionnaire]: 'missionnaire',
}

const RECREATE_DROPPED_COLUMN = `
ALTER TABLE "Member" ADD COLUMN IF NOT EXISTS "type" "PublisherType" NOT NULL DEFAULT 'normal'
`

/** Thrown to roll the fixture back; every assertion runs on captured values. */
class Rollback extends Error {}

Expand Down Expand Up @@ -71,23 +91,32 @@ async function execute(): Promise<Captured> {
await testDb.$transaction(
async tx => {
const stamp = `mig-${process.pid}-${globalThis.performance.now().toString().replace('.', '')}`
await tx.$executeRawUnsafe(RECREATE_DROPPED_COLUMN)
const congregation = await tx.congregation.create({ data: { name: stamp, slug: stamp, active: true } })
const congregationId = congregation.id

// baptismDate is not optional here: a `member_pioneer_requires_baptism` CHECK constraint
// means a pioneer-typed row cannot exist without one, which also bounds the population this
// migration can ever see.
const makeMember = (firstname: string, type: PublisherType) =>
tx.member.create({
// Prisma no longer knows the column a later migration dropped, so the fixture writes it
// in raw SQL against the stand-in recreated above.
const makeMember = async (firstname: string, type: PublisherType) => {
const member = await tx.member.create({
data: {
firstname,
lastname: stamp,
isPublisher: true,
type,
baptismDate: new Date('2015-01-01'),
congregationId,
},
})
await tx.$executeRawUnsafe(
'UPDATE "Member" SET "type" = $1::"PublisherType" WHERE "id" = $2',
DB_ENUM_VALUE[type],
member.id,
)
return member
}

// Has reported activity — the stint should start at their EARLIEST reported month.
const withActivity = await makeMember('WithActivity', PublisherType.PionnierPermanant)
Expand Down
60 changes: 60 additions & 0 deletions app/database/migrations/drop-member-type.integration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { readFileSync } from 'node:fs'
import { resolve } from 'node:path'
import { PrismaPg } from '@prisma/adapter-pg'
import { afterAll, describe, expect, it } from 'vitest'
import { PrismaClient } from '~/database/generated/client'

// Runs against DB_URL rather than DB_RUNTIME_URL: a migration executes as the schema owner.
const adapter = new PrismaPg({ connectionString: process.env.DB_URL, max: 3, connectionTimeoutMillis: 5000 })
const testDb = new PrismaClient({ adapter })

// Hoisted: a regex literal inside a function body is a lint error in this repo.
const DROP_COLUMN_RE = /DROP COLUMN "type"/
const DROP_CONSTRAINT_RE = /DROP CONSTRAINT IF EXISTS "member_pioneer_requires_baptism"/
const ALTER_TABLE_RE = /ALTER TABLE "(\w+)"/g

const MIGRATION_SQL = resolve(import.meta.dirname, '20260831230000_drop_member_type', 'migration.sql')

function migrationSql(): string {
return readFileSync(MIGRATION_SQL, 'utf8')
}

function migrationStatements(): string[] {
return migrationSql()
.split('\n')
.filter(line => !line.trimStart().startsWith('--'))
.join('\n')
.split(';')
.map(statement => statement.trim())
.filter(statement => statement.length > 0)
}

afterAll(async () => {
await testDb.$disconnect()
})

describe('drop Member.type (migration)', () => {
// Not executed here. DROP COLUMN takes ACCESS EXCLUSIVE on Member and every table whose foreign
// keys reference it, held until rollback — running it in a fixture deadlocks unrelated suites at
// random. The shape of the file is what this asserts; that it applies cleanly is proved by CI
// running `prisma migrate deploy` against a fresh database before any test executes.
it('drops the dependent CHECK constraint before the column', () => {
const statements = migrationStatements()
const constraintIndex = statements.findIndex(s => s.includes('member_pioneer_requires_baptism'))
const columnIndex = statements.findIndex(s => DROP_COLUMN_RE.test(s))

expect(constraintIndex).toBeGreaterThanOrEqual(0)
expect(columnIndex).toBeGreaterThanOrEqual(0)
// Postgres refuses to drop a column a constraint still references, so the order is load-bearing.
expect(constraintIndex).toBeLessThan(columnIndex)
})

it('drops the constraint idempotently', () => {
expect(migrationSql()).toMatch(DROP_CONSTRAINT_RE)
})

it('touches only Member', () => {
const tables = [...migrationSql().matchAll(ALTER_TABLE_RE)].map(match => match[1])
expect([...new Set(tables)]).toEqual(['Member'])
})
})
6 changes: 3 additions & 3 deletions app/database/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,10 @@ model Member {
dpaCardUpToDate Boolean @default(false)
survivalBackpackReady Boolean @default(false)

// Domain status. `isPublisher = false` means ministry-school student.
// `type` is only meaningful when `isPublisher = true`.
// Domain status. `isPublisher = false` means ministry-school student. Whether a publisher is a
// pioneer is NOT stored here — it is their PioneerEnrolment stints, so that a standing status and
// the period it covers cannot disagree.
isPublisher Boolean @default(false)
type PublisherType @default(Normal)
baptismDate DateTime?
isAnointed Boolean @default(false)
isHelder Boolean @default(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ beforeAll(async () => {
firstname: 'Active',
lastname: 'Pioneer',
isPublisher: true,
type: PublisherType.PionnierPermanant,
baptismDate: new Date('2010-01-01'),
congregationId,
},
Expand All @@ -66,7 +65,6 @@ beforeAll(async () => {
firstname: 'Inactive',
lastname: 'Pioneer',
isPublisher: true,
type: PublisherType.PionnierPermanant,
baptismDate: new Date('2010-01-01'),
inactiveAt: new Date('2026-01-01'),
congregationId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ beforeAll(async () => {
lastname: 'Martin',
baptismDate: new Date('2010-01-01'),
isPublisher: true,
type: PublisherType.PionnierPermanant,
congregationId,
},
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ function fetchPublisherGroups(db: TransactionClient, congregationId: number) {
deputy: { select: userSelect },
members: {
where: { leftAt: null, isPublisher: true, inactiveAt: null },
select: { ...userSelect, type: true },
select: userSelect,
orderBy: [{ lastname: 'asc' }, { firstname: 'asc' }],
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ interface PublisherGroupsViewData {
adress: string
responsible: Person
deputy: (Person & { id: number }) | null
members: (Person & { type: string })[]
members: Person[]
}[]
}

Expand Down
5 changes: 3 additions & 2 deletions app/features/events/routes/programs/events/publisher-info.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { EventTemplateKey } from '~/features/events/model/event-template.type'
import type { PartSlot } from '~/features/events/server/cadence-shared.server'
import { listUserSameEventAssignments } from '~/features/events/server/list-user-same-event-assignments.server'
import { resolvePublisherCadence } from '~/features/events/server/resolve-publisher-cadence.server'
import { standingTypeFromEnrolments } from '~/features/publishers'
import { currentAccountContext, permissionsContext, withScopeFromContext } from '~/shared/auth/route-context.server'
import { Permission } from '~/shared/types/permission'
import { formatGroupName } from '~/shared/utils/format-group-name'
Expand Down Expand Up @@ -42,7 +43,7 @@ export function loader({ request, params, context }: Route.LoaderArgs) {
// Publisher profile
const user = await db.member.findFirst({
where: { id: userId, congregationId },
include: { publisherGroup: true, account: { select: { id: true } } },
include: { publisherGroup: true, account: { select: { id: true } }, pioneerEnrolments: true },
})
if (!user) return Response.json(null)

Expand Down Expand Up @@ -85,7 +86,7 @@ export function loader({ request, params, context }: Route.LoaderArgs) {
lastname: user.lastname,
isHelder: user.isHelder,
isServant: user.isServant,
type: user.type,
type: standingTypeFromEnrolments(user.pioneerEnrolments),
group: user.publisherGroup ? formatGroupName(user.publisherGroup.name) : null,
},
daysOff: daysOff.map(d => ({
Expand Down
8 changes: 7 additions & 1 deletion app/features/publishers/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ export {
getEnrolmentsForServiceYear,
resolveEnrolmentMonthlyGoal,
} from './server/pioneer-enrolment.queries'
export { endPioneerEnrolment, enrolPioneer, removePioneerEnrolment } from './server/pioneer-enrolment.workflow'
export {
endOngoingEnrolmentsOfType,
endPioneerEnrolment,
enrolPioneer,
removePioneerEnrolment,
updatePioneerEnrolment,
} from './server/pioneer-enrolment.workflow'
export { backfillCongregationEnrolments } from './server/pioneer-enrolment-backfill.server'
export { listPioneerGoalsForYear, type PioneerGoalRow, resolvePioneerGoal } from './server/pioneer-goals.queries'
export { getPublishers } from './server/publishers.server'
Expand Down
11 changes: 11 additions & 0 deletions app/features/publishers/model/pioneer-enrolment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,17 @@
import { PublisherType } from '~/shared/types/publisher-type'
import { type MonthRef, serviceYearMonths } from './pioneer-pace'

// The Prisma `select` that produces an EnrolmentPeriod. Kept beside the type so a schema change
// cannot leave a caller selecting a shape the helpers no longer accept.
export const ENROLMENT_PERIOD_SELECT = {
type: true,
startMonth: true,
startYear: true,
endMonth: true,
endYear: true,
monthlyGoal: true,
} as const

// A pioneer stint, DB-free and serialisable — mirrors the PioneerEnrolment row's period fields.
export interface EnrolmentPeriod {
type: PublisherType
Expand Down
17 changes: 11 additions & 6 deletions app/features/publishers/routes/activity/new.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { useState } from 'react'
import { data, Form, redirect, useSearchParams } from 'react-router'
import { commitSession, getSession } from '~/features/authentication/index.server'
import { getEnrolmentsForMember, resolveEnrolmentMonthlyGoal } from '~/features/publishers/index.server'
import { enrolmentForMonth } from '~/features/publishers/model/pioneer-enrolment'
import { enrolmentForMonth, standingTypeFromEnrolments } from '~/features/publishers/model/pioneer-enrolment'
import { toServiceYear } from '~/features/publishers/model/pioneer-pace'
import { createActivitySchema } from '~/features/publishers/schemas/activity.schema'
import { createPublisherActivity } from '~/features/publishers/server/publisher-activity-mutations.server'
Expand Down Expand Up @@ -84,16 +84,20 @@ export function loader({ request, context }: Route.LoaderArgs) {
}

// Show the enrolled goal as read-only context so the report can be entered against the plan.
// `standingType` is the member's current status, derived from their stints — the form uses it to
// decide whether to offer the auxiliary-pioneer picker at all.
let enrolmentGoal: number | null = null
let standingType: PublisherType = PublisherType.Normal
if (publisher != null) {
const enrolments = await getEnrolmentsForMember(db, publisher.id, currentUser.congregationId)
standingType = standingTypeFromEnrolments(enrolments)
const active = enrolmentForMonth(enrolments, month, year)
if (active != null) enrolmentGoal = await resolveEnrolmentMonthlyGoal(db, active, toServiceYear(month, year))
}

return {
publishers,
publisher,
publisher: publisher == null ? null : { ...publisher, standingType },
selectedMonth: { month, year },
enrolmentGoal,
previousPage: request.headers.get('referer'),
Expand All @@ -105,7 +109,7 @@ export default function NewActivity({ loaderData, actionData }: Route.ComponentP
const { publishers, publisher, selectedMonth, enrolmentGoal, previousPage } = loaderData
const [searchParams, setSearchParams] = useSearchParams()
const [pioneer, setPioneer] = useState<PublisherType | null>(
publisher?.type === PublisherType.PionnierAuxiliaires ? PublisherType.PionnierAuxiliaires : null,
publisher?.standingType === PublisherType.PionnierAuxiliaires ? PublisherType.PionnierAuxiliaires : null,
)
const { blocker, markDirty } = useUnsavedChanges()

Expand Down Expand Up @@ -240,7 +244,7 @@ export default function NewActivity({ loaderData, actionData }: Route.ComponentP
</p>
)}

{publisher?.type != null && publisher.type === PublisherType.Normal && (
{publisher != null && publisher.standingType === PublisherType.Normal && (
<div className="space-y-2">
<Label htmlFor="type">{m.activity_new_pioneer_label()}</Label>
<Select
Expand Down Expand Up @@ -269,7 +273,7 @@ export default function NewActivity({ loaderData, actionData }: Route.ComponentP
PublisherType.PionnierSpecial,
PublisherType.Missionnaire,
] as PublisherType[]
).includes(publisher?.type ?? PublisherType.Normal) ||
).includes(publisher?.standingType ?? PublisherType.Normal) ||
(pioneer != null && ([PublisherType.PionnierAuxiliaires] as PublisherType[]).includes(pioneer)) ? (
<div className="space-y-2">
<Label htmlFor={fields.hours.id}>{m.activity_new_hours_label()}</Label>
Expand Down Expand Up @@ -360,9 +364,10 @@ export async function action({ request, context }: Route.ActionArgs) {
// (a Normal member reporting an auxiliary month without a formal enrolment).
const enrolments = await getEnrolmentsForMember(db, publisher.id, currentUser.congregationId)
const enrolledType = enrolmentForMonth(enrolments, month, year)?.type
const standingType = standingTypeFromEnrolments(enrolments)
const type =
enrolledType ??
(publisher.type === PublisherType.Normal ? (submission.value.type ?? PublisherType.Normal) : publisher.type)
(standingType === PublisherType.Normal ? (submission.value.type ?? PublisherType.Normal) : standingType)
const activity = await createPublisherActivity(db, {
publisherId: publisher.id,
month,
Expand Down
9 changes: 1 addition & 8 deletions app/features/publishers/routes/publishers/edit-publisher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,7 @@ export default function EditPublisher({ loaderData }: Route.ComponentProps) {
<Form id="edit-publisher-form" method="post" className="flex flex-col gap-6" onChange={markDirty}>
<PublisherPersonalInformationForm user={user} onGenderChange={setGender} />
<PublisherNominationForm user={user} gender={gender} />
<PublisherFieldServiceForm
user={user}
groups={groups}
hideAuxiliaryPioneer={hideAuxiliaryPioneer}
hideTypeSelect
/>
<PublisherFieldServiceForm user={user} groups={groups} />
</Form>

{/* Pioneer appointments — separate forms, each posting its own enrolment intent (saved on their
Expand Down Expand Up @@ -182,7 +177,6 @@ export async function action({ request, params, context }: Route.ActionArgs) {
isServant,
isAnointed,
group,
type,
phone,
address,
} = submission.value
Expand All @@ -205,7 +199,6 @@ export async function action({ request, params, context }: Route.ActionArgs) {
isAnointed,
groupId: group ?? null,
email: email ?? '',
type,
address,
phone,
},
Expand Down
Loading