diff --git a/docs/notifications.md b/docs/notifications.md index f6bca95..f37ed26 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -37,62 +37,155 @@ every send just logs to the container's console instead. (`recipientUserId` set) — a guest customer never gets one, since they have nothing to sign into. -## Preferences — who can turn what off +## Preferences — who can turn what off, and what's on by default See `src/services/notifications/types.ts`'s `PREFERENCE_CONTROLLED_TYPES` -— this is the single source of truth; everything not listed there is -**required** (always sent on every channel it has a valid target for, no -toggle anywhere). Currently: +— this is the single source of truth for which types have a toggle at +all; everything not listed there is **required** (always sent on every +channel it has a valid target for, no toggle anywhere — see "Fixed +no-email types" below for a separate, smaller mechanism that trims one +channel off a handful of these without making them toggleable). -- **Customer-facing**: only `BOOKING_REMINDER` is optional. Everything - else (confirmations, changes, cancellations, loyalty) always notifies - the customer — not finding out your appointment was cancelled is a - real risk, not just noise. +- **Customer-facing**: only `BOOKING_REMINDER` is optional (and only + controls the 24h reminder — see "Reminder timing"). Everything else + (confirmations, changes, cancellations, loyalty) always notifies the + customer on every resolvable channel — not finding out your + appointment was cancelled is a real risk, not just noise. - **Staff- and owner-facing**: every type is optional. The owner of a business is commonly also its only barber — one `StaffProfile`, one account — so without this, a single new booking would unconditionally notify that one person twice (`STAFF_NEW_APPOINTMENT` + `OWNER_NEW_BOOKING`) on every channel, including in-app, with no way to - quiet either copy. + quiet either copy. (Cross-role dedup, below, now also prevents the + literal duplicate even when both happen to be enabled.) Toggles live at **Settings → Notifications** — customers see one "Appointment reminders" group; staff see their own six groups, plus five more owner-only groups if they hold `VIEW_REPORTING` (i.e. are actually an owner — see `src/services/notifications/recipients.ts`'s -`getOwnerUserIds`, which checks the same permission). A preference with -no saved row yet defaults to **on** — turning something off is always an -explicit, deliberate action. +`getOwnerUserIds`, which checks the same permission). + +**Defaults**: `types.ts`'s `defaultPreferenceIsEnabled(type)` is the +single source of truth for what a toggle shows/does before anyone has +ever saved a preference row for it — used both by the actual send +decision (`notification.service.ts`'s `isChannelEnabled`) and by the +Settings pages themselves, so a toggle's initial on/off state always +matches what will actually happen. Once a user explicitly saves a +preference, that stored value always wins, in either direction — this +function only governs the "nobody has touched this yet" fallback. + +Most staff/owner types default to **on**. These default to **off** +instead — a solo owner-barber otherwise gets emailed/pushed/in-app'd for +everything the moment they sign in, with no gentle on-ramp: + +| Type | Why | +|---|---| +| `STAFF_NEW_APPOINTMENT` | Noisy for a busy barber; `STAFF_APPOINTMENT_CANCELLED` (which stays on) is the one that actually needs acting on. | +| `STAFF_APPOINTMENT_RESCHEDULED` | Same reasoning. | +| `STAFF_APPOINTMENT_REMINDER` | A same-day heads-up is nice-to-have, not essential, for the person actually doing the cutting. | +| `OWNER_NEW_BOOKING` | Same as STAFF_NEW_APPOINTMENT, from the owner's side. | +| `OWNER_NO_SHOW` | The barber who marked it already knows — there's no staff-facing equivalent by design. | + +## Fixed no-email types + +A few types are `EMAIL` never, always, and unconditionally — not a +default that can be toggled back on, because the type either has no +preference toggle at all or (for `BOOKING_BARBER_CHANGED`) the decision +is per-booking context a stored preference can't express. This is +`NotifyParams.skipEmail` in `notification.service.ts` — a single boolean +the caller passes, checked in `resolveChannels()`'s EMAIL branch +independently of the preference system. PUSH/IN_APP are unaffected in +every case below. + +| Type | Condition | +|---|---| +| `BOOKING_COMPLETED` | Always — a "thanks for visiting" email after every single cut was more noise than value. | +| `LOYALTY_STAMP_ADDED` | Always — a stamp is added on nearly every visit. | +| `LOYALTY_REWARD_REDEEMED` | Always — the customer is present when a reward is redeemed; an email receipt afterward added little. | +| `BOOKING_BARBER_CHANGED` | Only when the customer originally booked without picking a specific staff member ("any available barber" — see `Appointment.staffSelectedByCustomer`) and hasn't been reassigned before. Someone who booked "any available" is less likely to care who they end up with; someone who picked this barber specifically still gets emailed. | + +## Cross-role deduplication + +A person who holds two roles for the same booking (most commonly a solo +owner-barber: one `User`, one `StaffProfile`, and a role with +`VIEW_REPORTING`) can be the target of two *equivalent* notifications for +one underlying event — e.g. a new booking fires both +`STAFF_NEW_APPOINTMENT` (they're the assigned staff) and +`OWNER_NEW_BOOKING` (they're also an owner). Since both resolve to the +identical `recipientUserId`, `notification.service.ts`'s `notify()` +detects this and suppresses the less-specific one — on every channel, +not just email — rather than relying on the person to manually mute one +side. + +`EQUIVALENCE_GROUPS` in that file lists which types are considered the +same underlying event, and their relative priority: + +| Group | Types (higher priority first) | +|---|---| +| `new_appointment` | `STAFF_NEW_APPOINTMENT` > `OWNER_NEW_BOOKING` | +| `appointment_cancelled` | `STAFF_APPOINTMENT_CANCELLED` > `OWNER_BOOKING_CANCELLED` | + +When an equivalent, higher-or-equal-priority notification already exists +for the same `(businessId, appointmentId, recipientUserId)`, the new one +is still created as a `Notification` row (audit trail, keeps +`idempotencyKey` semantics intact) but with **zero** deliveries — nothing +sends on any channel. This relies on call order: every real call site +(`booking-notifications.ts`) already notifies the STAFF_* type before +looping the OWNER_* type for the same event, so the more specific one is +always created first. Only types explicitly listed in +`EQUIVALENCE_GROUPS` ever participate — e.g. `OWNER_NO_SHOW` has no staff +equivalent and is never touched by this, and any future owner-only +*actionable* type (e.g. "approve this booking") would likewise stay +untouched simply by not being added to the map. Unrelated events for the +same appointment (a confirmation, then later a cancellation) are never +conflated — they're different types in different groups, or no group at +all. + +## Reminder timing + +`src/services/scheduling/reminder-scheduling.ts` schedules reminders at +fixed hours-before-appointment offsets, independently for the customer +and the assigned staff member: + +- **Customer** (`BOOKING_REMINDER`): 24 hours before only. +- **Staff** (`STAFF_APPOINTMENT_REMINDER`): 24 hours and 2 hours before + (each its own send) — but see the defaults table above, this type + defaults to off, so nothing actually sends unless the staff member + opts in. + +An offset that's already in the past by the time the appointment is +booked/approved is silently skipped, never scheduled retroactively. --- ## Customer-facing -| Type | Fires when | Default subject | -|---|---|---| -| `BOOKING_CONFIRMED` | A new booking is created and doesn't need approval | "Booking confirmed" | -| `BOOKING_PENDING_APPROVAL` | A new booking is created but the business requires approval first | "Booking request received" | -| `BOOKING_APPROVED` | An owner approves a pending booking | "Booking approved" | -| `BOOKING_RESCHEDULED` | The appointment's time changes (including an owner reassigning staff, which also triggers a reschedule) | "Booking rescheduled" | -| `BOOKING_CANCELLED` | The appointment is cancelled, by anyone | "Booking cancelled" (carries a "This was a late cancellation." note when applicable — not a separate type) | -| `BOOKING_BARBER_CHANGED` | An owner reassigns the appointment to a different staff member | "Your barber has changed" | -| `BOOKING_COMPLETED` | The appointment is marked completed | "Thanks for visiting" | -| `BOOKING_REMINDER` *(optional)* | 24 hours and 2 hours before a confirmed appointment (each is its own send) | "Upcoming appointment reminder" | -| `LOYALTY_STAMP_ADDED` | A loyalty stamp is added to their card | "You've earned a loyalty stamp" | -| `LOYALTY_REWARD_EARNED` | Their stamp count reaches the reward threshold | "You've earned a reward" | -| `LOYALTY_REWARD_EXPIRING` | A daily sweep finds their reward is nearing its expiry date (fires once per calendar day it's still found, not once total) | "Your reward is expiring soon" | -| `LOYALTY_REWARD_REDEEMED` | Their reward is applied to an appointment | "Reward applied" | +| Type | Fires when | Default subject | Email | +|---|---|---|---| +| `BOOKING_CONFIRMED` | A new booking is created and doesn't need approval | "Booking confirmed" | Always | +| `BOOKING_PENDING_APPROVAL` | A new booking is created but the business requires approval first | "Booking request received" | Always | +| `BOOKING_APPROVED` | An owner approves a pending booking | "Booking approved" | Always | +| `BOOKING_RESCHEDULED` | The appointment's time changes (including an owner reassigning staff, which also triggers a reschedule) | "Booking rescheduled" | Always | +| `BOOKING_CANCELLED` | The appointment is cancelled, by anyone | "Booking cancelled" (carries a "This was a late cancellation." note when applicable — not a separate type) | Always | +| `BOOKING_BARBER_CHANGED` | An owner reassigns the appointment to a different staff member | "Your barber has changed" | Only if the customer originally picked a specific staff member — see "Fixed no-email types" | +| `BOOKING_COMPLETED` | The appointment is marked completed | "Thanks for visiting" | Never | +| `BOOKING_REMINDER` *(optional)* | 24 hours before a confirmed appointment | "Upcoming appointment reminder" | Default on | +| `LOYALTY_STAMP_ADDED` | A loyalty stamp is added to their card | "You've earned a loyalty stamp" | Never | +| `LOYALTY_REWARD_EARNED` | Their stamp count reaches the reward threshold | "You've earned a reward" | Always | +| `LOYALTY_REWARD_EXPIRING` | A daily sweep finds their reward is nearing its expiry date (fires once per calendar day it's still found, not once total) | "Your reward is expiring soon" | Always | +| `LOYALTY_REWARD_REDEEMED` | Their reward is applied to an appointment | "Reward applied" | Never | ## Staff-facing -| Type | Fires when | Default subject | -|---|---|---| -| `STAFF_NEW_APPOINTMENT` *(optional)* | A new appointment is booked with them | "New appointment" | -| `STAFF_APPOINTMENT_RESCHEDULED` *(optional)* | One of their appointments is rescheduled | "Appointment rescheduled" | -| `STAFF_APPOINTMENT_CANCELLED` *(optional)* | One of their appointments is cancelled | "Appointment cancelled" | -| `STAFF_APPOINTMENT_REMINDER` *(optional)* | 24 hours and 2 hours before their appointment | "Upcoming appointment reminder" | -| `STAFF_SCHEDULE_CHANGED` *(optional)* | An **owner** edits their working hours/breaks/time-off on their behalf — never fires from the self-service editors, since notifying someone about their own edit would just be noise | "Your schedule was updated" | -| `STAFF_INVITATION_RECEIVED` *(optional)* | They're added as a new team member | "You've been invited to join `{business}`" | -| `STAFF_APPOINTMENT_CHECKED_IN` | Defined in the schema, has a recipient assigned — **but nothing in the codebase currently triggers it.** Preference-controlled for forward-compatibility, but has no toggle in the UI since there's nothing to preview | — | +| Type | Fires when | Default subject | Default | +|---|---|---|---| +| `STAFF_NEW_APPOINTMENT` *(optional)* | A new appointment is booked with them | "New appointment" | Off | +| `STAFF_APPOINTMENT_RESCHEDULED` *(optional)* | One of their appointments is rescheduled | "Appointment rescheduled" | Off | +| `STAFF_APPOINTMENT_CANCELLED` *(optional)* | One of their appointments is cancelled | "Appointment cancelled" | On | +| `STAFF_APPOINTMENT_REMINDER` *(optional)* | 24 hours and 2 hours before their appointment | "Upcoming appointment reminder" | Off | +| `STAFF_SCHEDULE_CHANGED` *(optional)* | An **owner** edits their working hours/breaks/time-off on their behalf — never fires from the self-service editors, since notifying someone about their own edit would just be noise | "Your schedule was updated" | On | +| `STAFF_INVITATION_RECEIVED` *(optional)* | They're added as a new team member | "You've been invited to join `{business}`" | On | +| `STAFF_APPOINTMENT_CHECKED_IN` | Defined in the schema, has a recipient assigned — **but nothing in the codebase currently triggers it.** Preference-controlled for forward-compatibility, but has no toggle in the UI since there's nothing to preview. Left dormant on purpose | — | — | ## Owner-facing @@ -100,13 +193,13 @@ Sent to *every* active staff member whose role holds `VIEW_REPORTING` — that can be more than one person; there's no single "the owner" column anywhere in the schema. -| Type | Fires when | Default subject | -|---|---|---| -| `OWNER_NEW_BOOKING` *(optional)* | A new booking is made anywhere in the business | "New booking" | -| `OWNER_BOOKING_CANCELLED` *(optional)* | A booking is cancelled anywhere in the business | "Booking cancelled" | -| `OWNER_NO_SHOW` *(optional)* | A customer is marked as a no-show (staff-facing equivalent doesn't exist — the barber who marked it already knows) | "No-show recorded" | -| `OWNER_STAFF_INVITATION_ACCEPTED` *(optional)* | An invited staff member verifies their email and joins | "Invitation accepted" | -| `OWNER_REPEATED_DELIVERY_FAILURE` *(optional)* | The background processor itself detects repeated delivery failures that day — a self-monitoring alert, not triggered by a domain event. At most once per business per day | "Some notifications failed to send" | +| Type | Fires when | Default subject | Default | +|---|---|---|---| +| `OWNER_NEW_BOOKING` *(optional)* | A new booking is made anywhere in the business | "New booking" | Off | +| `OWNER_BOOKING_CANCELLED` *(optional)* | A booking is cancelled anywhere in the business | "Booking cancelled" | On | +| `OWNER_NO_SHOW` *(optional)* | A customer is marked as a no-show (staff-facing equivalent doesn't exist — the barber who marked it already knows) | "No-show recorded" | Off | +| `OWNER_STAFF_INVITATION_ACCEPTED` *(optional)* | An invited staff member verifies their email and joins | "Invitation accepted" | On | +| `OWNER_REPEATED_DELIVERY_FAILURE` *(optional)* | The background processor itself detects repeated delivery failures that day — a self-monitoring alert, not triggered by a domain event. At most once per business per day | "Some notifications failed to send" | On | --- @@ -136,7 +229,16 @@ Not queued, not preference-controlled, no in-app record. - **What exists / who it's for**: `prisma/schema.prisma`'s `NotificationType` enum, and `RECIPIENT_KIND` in `src/services/notifications/types.ts`. -- **Whether it's optional**: `PREFERENCE_CONTROLLED_TYPES`, same file. +- **Whether it's optional, and its default**: `PREFERENCE_CONTROLLED_TYPES` + and `defaultPreferenceIsEnabled()`, same file. +- **Fixed no-email types**: `NotifyParams.skipEmail` in + `notification.service.ts`, set at the relevant call site in + `booking-notifications.ts`/`loyalty-notifications.ts`. +- **Cross-role dedup**: `EQUIVALENCE_GROUPS` in `notification.service.ts`. +- **Whether the customer picked a specific barber originally**: + `Appointment.staffSelectedByCustomer`, set once at booking creation in + `booking.service.ts`'s `createBooking()`, read at reassignment time in + `src/features/owner/actions/appointments.ts`. - **When it actually fires**: `src/services/scheduling/booking-notifications.ts`, `reminder-scheduling.ts`, `src/services/loyalty/loyalty-notifications.ts`, `src/services/staff/staff-notifications.ts`, diff --git a/prisma/migrations/20260912165903_add_staff_selected_by_customer/migration.sql b/prisma/migrations/20260912165903_add_staff_selected_by_customer/migration.sql new file mode 100644 index 0000000..01bea00 --- /dev/null +++ b/prisma/migrations/20260912165903_add_staff_selected_by_customer/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE `Appointment` ADD COLUMN `staffSelectedByCustomer` BOOLEAN NOT NULL DEFAULT true; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 6ecf615..d30522b 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -1124,6 +1124,18 @@ model Appointment { staffId String staff StaffProfile @relation(fields: [staffId], references: [id], onDelete: Restrict) + /// Whether the customer picked THIS specific staff member when they + /// originally booked, as opposed to "any available barber" (the system + /// then auto-assigns one — see BookingService.createBooking). Set once + /// at creation and never rewritten by a later reassignment — it + /// describes the ORIGINAL booking, used to decide whether a later + /// staff reassignment is worth emailing the customer about (see + /// notifyBookingBarberChanged): someone who never expressed a + /// preference is less likely to care who they end up with. Defaults to + /// true so every appointment created before this field existed keeps + /// notifying on reassignment exactly as it did before. + staffSelectedByCustomer Boolean @default(true) + status AppointmentStatus @default(CONFIRMED) /// Stored as UTC instants; the business's `timezone` (see Business model) diff --git a/src/app/(customer)/settings/notifications/page.tsx b/src/app/(customer)/settings/notifications/page.tsx index 08a845e..5f04cfb 100644 --- a/src/app/(customer)/settings/notifications/page.tsx +++ b/src/app/(customer)/settings/notifications/page.tsx @@ -5,6 +5,7 @@ import { TopNav } from "@/components/navigation/top-nav"; import { NotificationChannel, NotificationType } from "@/generated/prisma/enums"; import { prisma } from "@/lib/db/prisma"; import { getNotificationPreferences } from "@/services/notifications/notification.service"; +import { defaultPreferenceIsEnabled } from "@/services/notifications/types"; import { NotificationPreferencesForm } from "@/features/staff/components/notification-preferences-form"; import type { NotificationPreferenceGroup, PreferenceCellState } from "@/features/staff/components/notification-preferences-form"; import { PushSubscribeToggle } from "@/features/notifications/components/push-subscribe-toggle"; @@ -25,7 +26,7 @@ export default async function CustomerNotificationSettingsPage() { CHANNELS.map((channel) => ({ type: group.type, channel, - isEnabled: rows.find((r) => r.type === group.type && r.channel === channel)?.isEnabled ?? true, + isEnabled: rows.find((r) => r.type === group.type && r.channel === channel)?.isEnabled ?? defaultPreferenceIsEnabled(group.type), })), ); diff --git a/src/app/(staff)/staff/settings/notifications/page.tsx b/src/app/(staff)/staff/settings/notifications/page.tsx index caaca66..deb38a6 100644 --- a/src/app/(staff)/staff/settings/notifications/page.tsx +++ b/src/app/(staff)/staff/settings/notifications/page.tsx @@ -6,6 +6,7 @@ import { NotificationChannel, NotificationType, Permission } from "@/generated/p import { prisma } from "@/lib/db/prisma"; import { hasPermission } from "@/lib/auth/permissions"; import { getNotificationPreferences } from "@/services/notifications/notification.service"; +import { defaultPreferenceIsEnabled } from "@/services/notifications/types"; import { NotificationPreferencesForm } from "@/features/staff/components/notification-preferences-form"; import type { NotificationPreferenceGroup, PreferenceCellState } from "@/features/staff/components/notification-preferences-form"; import { PushSubscribeToggle } from "@/features/notifications/components/push-subscribe-toggle"; @@ -52,7 +53,7 @@ export default async function StaffNotificationsPage() { CHANNELS.map((channel) => ({ type: group.type, channel, - isEnabled: rows.find((r) => r.type === group.type && r.channel === channel)?.isEnabled ?? true, + isEnabled: rows.find((r) => r.type === group.type && r.channel === channel)?.isEnabled ?? defaultPreferenceIsEnabled(group.type), })), ); diff --git a/src/features/owner/actions/appointments.ts b/src/features/owner/actions/appointments.ts index 8740b0f..c51b79e 100644 --- a/src/features/owner/actions/appointments.ts +++ b/src/features/owner/actions/appointments.ts @@ -183,7 +183,9 @@ export async function ownerRescheduleAppointmentAction(params: { entityId: params.appointmentId, metadata: { fromStaffId: previousStaffId, toStaffId: params.staffId }, }); - await notifyBookingBarberChanged(prisma, result.appointment); + await notifyBookingBarberChanged(prisma, result.appointment, { + customerSelectedStaff: appointment.staffSelectedByCustomer, + }); } await syncAppointmentSafely(prisma, params.appointmentId); diff --git a/src/services/loyalty/__tests__/loyalty-notifications.integration.test.ts b/src/services/loyalty/__tests__/loyalty-notifications.integration.test.ts index 3602819..a61e6a4 100644 --- a/src/services/loyalty/__tests__/loyalty-notifications.integration.test.ts +++ b/src/services/loyalty/__tests__/loyalty-notifications.integration.test.ts @@ -141,8 +141,31 @@ describe("loyalty notifications (integration)", () => { const redeemedNotifications = await prisma.notification.findMany({ where: { recipientUserId: userId, type: "LOYALTY_REWARD_REDEEMED" }, + include: { deliveries: true }, }); expect(redeemedNotifications).toHaveLength(1); expect(redeemedNotifications[0]!.loyaltyRewardId).toBe(reward.id); + // No longer emailed by default — the customer is present when a reward + // is redeemed, so an email receipt afterward was more noise than value. + const channels = redeemedNotifications[0]!.deliveries.map((d) => d.channel); + expect(channels).not.toContain("EMAIL"); + expect(channels).toContain("IN_APP"); + }); + + it("a stamp-added notification no longer emails by default (still in-app)", async () => { + await enableLoyalty(5); + const { staffId } = await createTestStaff(business); + const { customerId, userId } = await createTestCustomer(business); + const appointment = await completedAppointment(staffId, customerId); + + await awardStampForCompletedAppointment(prisma, { appointmentId: appointment.id, staffId }); + + const notification = await prisma.notification.findFirstOrThrow({ + where: { recipientUserId: userId, type: "LOYALTY_STAMP_ADDED" }, + include: { deliveries: true }, + }); + const channels = notification.deliveries.map((d) => d.channel); + expect(channels).not.toContain("EMAIL"); + expect(channels).toContain("IN_APP"); }); }); diff --git a/src/services/loyalty/loyalty-notifications.ts b/src/services/loyalty/loyalty-notifications.ts index bafd41c..797f06e 100644 --- a/src/services/loyalty/loyalty-notifications.ts +++ b/src/services/loyalty/loyalty-notifications.ts @@ -17,6 +17,7 @@ async function resolveCustomerRecipient(prisma: PrismaClient, customerId: string }); } +/** No longer emailed by default (still push/in-app) — a stamp is added on nearly every visit, so an email each time was more noise than value. */ export async function notifyLoyaltyStampAdded( prisma: PrismaClient, params: { businessId: string; customerId: string; transactionId: string; appointmentId?: string }, @@ -32,6 +33,7 @@ export async function notifyLoyaltyStampAdded( bodyText: "You've earned a loyalty stamp. Keep it up!", deepLinkPath: "/loyalty", idempotencyKey: `notify:loyalty-stamp-added:${params.transactionId}`, + skipEmail: true, }); } @@ -74,6 +76,7 @@ export async function notifyLoyaltyRewardExpiring( }); } +/** No longer emailed by default (still push/in-app) — the customer is present when a reward is redeemed (it's applied to their current visit), so an email receipt afterward was more noise than value. */ export async function notifyLoyaltyRewardRedeemed( prisma: PrismaClient, params: { businessId: string; customerId: string; rewardId: string; rewardLabel: string; appointmentId: string }, @@ -90,5 +93,6 @@ export async function notifyLoyaltyRewardRedeemed( bodyText: `Your reward has been applied: ${params.rewardLabel}.`, deepLinkPath: "/loyalty", idempotencyKey: `notify:loyalty-reward-redeemed:${params.rewardId}`, + skipEmail: true, }); } diff --git a/src/services/notifications/__tests__/notification.service.integration.test.ts b/src/services/notifications/__tests__/notification.service.integration.test.ts index 219ab55..3bbab00 100644 --- a/src/services/notifications/__tests__/notification.service.integration.test.ts +++ b/src/services/notifications/__tests__/notification.service.integration.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { NotificationChannel, NotificationType } from "@/generated/prisma/enums"; import { cleanupTestBusiness, + createTestAppointment, createTestBusiness, createTestCustomer, createTestGuestCustomer, @@ -197,6 +198,15 @@ describe("NotificationService (integration)", () => { channel: NotificationChannel.IN_APP, isEnabled: false, }); + // STAFF_NEW_APPOINTMENT now defaults to off (see types.ts) — explicitly + // enable it here so this test isolates "one type's preference doesn't + // leak into another's", independent of what either defaults to. + await setNotificationPreference(prisma, { + userId, + type: NotificationType.STAFF_NEW_APPOINTMENT, + channel: NotificationChannel.IN_APP, + isEnabled: true, + }); const { notification: ownerNotification } = await notify(prisma, { businessId: business.businessId, @@ -217,6 +227,101 @@ describe("NotificationService (integration)", () => { expect(staffNotification.deliveries.map((d) => d.channel)).toContain(NotificationChannel.IN_APP); }); + it("STAFF_NEW_APPOINTMENT, STAFF_APPOINTMENT_RESCHEDULED, STAFF_APPOINTMENT_REMINDER, OWNER_NEW_BOOKING, and OWNER_NO_SHOW default to off with no preference row", async () => { + const { userId } = await createTestCustomer(business); + const disabledByDefault = [ + NotificationType.STAFF_NEW_APPOINTMENT, + NotificationType.STAFF_APPOINTMENT_RESCHEDULED, + NotificationType.STAFF_APPOINTMENT_REMINDER, + NotificationType.OWNER_NEW_BOOKING, + NotificationType.OWNER_NO_SHOW, + ]; + for (const type of disabledByDefault) { + const { notification } = await notify(prisma, { + businessId: business.businessId, + type, + recipientUserId: userId, + bodyText: "x", + idempotencyKey: `test:${userId}:default-off:${type}`, + }); + expect(notification.deliveries).toHaveLength(0); + } + }); + + it("STAFF_APPOINTMENT_CANCELLED, STAFF_SCHEDULE_CHANGED, STAFF_INVITATION_RECEIVED, OWNER_BOOKING_CANCELLED, OWNER_STAFF_INVITATION_ACCEPTED, and OWNER_REPEATED_DELIVERY_FAILURE still default to on with no preference row", async () => { + const { userId } = await createTestCustomer(business); + const stillEnabledByDefault = [ + NotificationType.STAFF_APPOINTMENT_CANCELLED, + NotificationType.STAFF_SCHEDULE_CHANGED, + NotificationType.STAFF_INVITATION_RECEIVED, + NotificationType.OWNER_BOOKING_CANCELLED, + NotificationType.OWNER_STAFF_INVITATION_ACCEPTED, + NotificationType.OWNER_REPEATED_DELIVERY_FAILURE, + ]; + for (const type of stillEnabledByDefault) { + const { notification } = await notify(prisma, { + businessId: business.businessId, + type, + recipientUserId: userId, + bodyText: "x", + idempotencyKey: `test:${userId}:default-on:${type}`, + }); + expect(notification.deliveries.map((d) => d.channel)).toContain(NotificationChannel.IN_APP); + } + }); + + it("an explicit preference always wins over the new default, in both directions", async () => { + const { userId } = await createTestCustomer(business); + + // A type that now defaults OFF, explicitly turned ON. + await setNotificationPreference(prisma, { + userId, + type: NotificationType.STAFF_NEW_APPOINTMENT, + channel: NotificationChannel.EMAIL, + isEnabled: true, + }); + // A type that still defaults ON, explicitly turned OFF. + await setNotificationPreference(prisma, { + userId, + type: NotificationType.STAFF_APPOINTMENT_CANCELLED, + channel: NotificationChannel.EMAIL, + isEnabled: false, + }); + + const { notification: onOverride } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.STAFF_NEW_APPOINTMENT, + recipientUserId: userId, + bodyText: "x", + idempotencyKey: `test:${userId}:on-override`, + }); + expect(onOverride.deliveries.map((d) => d.channel)).toContain(NotificationChannel.EMAIL); + + const { notification: offOverride } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.STAFF_APPOINTMENT_CANCELLED, + recipientUserId: userId, + bodyText: "x", + idempotencyKey: `test:${userId}:off-override`, + }); + expect(offOverride.deliveries.map((d) => d.channel)).not.toContain(NotificationChannel.EMAIL); + }); + + it("skipEmail suppresses only EMAIL, never PUSH or IN_APP", async () => { + const { userId } = await createTestCustomer(business); + const { notification } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.BOOKING_COMPLETED, + recipientUserId: userId, + bodyText: "Thanks for visiting!", + idempotencyKey: `test:${userId}:booking-completed-skip-email`, + skipEmail: true, + }); + const channels = notification.deliveries.map((d) => d.channel); + expect(channels).not.toContain(NotificationChannel.EMAIL); + expect(channels).toContain(NotificationChannel.IN_APP); + }); + it("setNotificationPreference now accepts every STAFF/OWNER-facing type (previously refused everything but the two reminders)", async () => { const { userId } = await createTestCustomer(business); await expect( @@ -245,14 +350,14 @@ describe("NotificationService (integration)", () => { const { notification: n1 } = await notify(prisma, { businessId: business.businessId, - type: NotificationType.STAFF_NEW_APPOINTMENT, + type: NotificationType.STAFF_APPOINTMENT_CANCELLED, recipientUserId: staffUserId, bodyText: "New appointment booked.", idempotencyKey: `test:${staffId}:appt-1`, }); await notify(prisma, { businessId: business.businessId, - type: NotificationType.STAFF_NEW_APPOINTMENT, + type: NotificationType.STAFF_APPOINTMENT_CANCELLED, recipientUserId: staffUserId, bodyText: "Another appointment booked.", idempotencyKey: `test:${staffId}:appt-2`, @@ -281,4 +386,187 @@ describe("NotificationService (integration)", () => { expect(unreadOnly).toHaveLength(0); }); }); + + /** + * Cross-role deduplication: a solo owner-barber (one recipientUserId + * holding both roles) should get only one equivalent notification per + * booking event — see notification.service.ts's EQUIVALENCE_GROUPS. + * Call order matters (staff before owner, matching every real call site + * in booking-notifications.ts) — see that map's own doc comment. + */ + describe("notify — cross-role deduplication", () => { + // STAFF_NEW_APPOINTMENT and OWNER_NEW_BOOKING both default to off (see + // types.ts) — every test below explicitly enables both so it's + // actually exercising dedup, not just the unrelated default-value + // change (which is covered separately above). + async function enableNewBookingTypes(...userIds: string[]) { + for (const userId of userIds) { + await setNotificationPreference(prisma, { userId, type: NotificationType.STAFF_NEW_APPOINTMENT, channel: NotificationChannel.IN_APP, isEnabled: true }); + await setNotificationPreference(prisma, { userId, type: NotificationType.OWNER_NEW_BOOKING, channel: NotificationChannel.IN_APP, isEnabled: true }); + } + } + + it("a lower-priority OWNER_NEW_BOOKING called after an equivalent STAFF_NEW_APPOINTMENT for the same recipient+appointment is suppressed (zero deliveries, row still created)", async () => { + const staff = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + const { appointmentId } = await createTestAppointment(business, { customerId, staffId: staff.staffId }); + await enableNewBookingTypes(staff.userId); + + const { notification: staffNotification } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.STAFF_NEW_APPOINTMENT, + recipientUserId: staff.userId, + appointmentId, + bodyText: "New appointment.", + idempotencyKey: `test:dedup:staff-new-appointment:${appointmentId}`, + }); + expect(staffNotification.deliveries.length).toBeGreaterThan(0); + + // Same person (staff.userId) is also "the owner" here — this is the + // solo owner-barber case the dedup exists for. + const { notification: ownerNotification } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.OWNER_NEW_BOOKING, + recipientUserId: staff.userId, + appointmentId, + bodyText: "New booking.", + idempotencyKey: `test:dedup:owner-new-booking:${appointmentId}`, + }); + expect(ownerNotification.deliveries).toHaveLength(0); + + // The row itself still exists (audit trail) — just with no deliveries. + const stillExists = await prisma.notification.findUnique({ where: { id: ownerNotification.id } }); + expect(stillExists).not.toBeNull(); + }); + + it("cancellation dedup: STAFF_APPOINTMENT_CANCELLED then OWNER_BOOKING_CANCELLED for the same recipient+appointment — only the staff copy has deliveries", async () => { + const staff = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + const { appointmentId } = await createTestAppointment(business, { customerId, staffId: staff.staffId }); + + await notify(prisma, { + businessId: business.businessId, + type: NotificationType.STAFF_APPOINTMENT_CANCELLED, + recipientUserId: staff.userId, + appointmentId, + bodyText: "Cancelled.", + idempotencyKey: `test:dedup:staff-cancelled:${appointmentId}`, + }); + const { notification: ownerNotification } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.OWNER_BOOKING_CANCELLED, + recipientUserId: staff.userId, + appointmentId, + bodyText: "Cancelled.", + idempotencyKey: `test:dedup:owner-cancelled:${appointmentId}`, + }); + expect(ownerNotification.deliveries).toHaveLength(0); + }); + + it("a booking confirmation and a later cancellation for the same appointment are NOT deduped against each other — different events, not equivalent", async () => { + const staff = await createTestStaff(business); + const { customerId, userId } = await createTestCustomer(business); + const { appointmentId } = await createTestAppointment(business, { customerId, staffId: staff.staffId }); + + await notify(prisma, { + businessId: business.businessId, + type: NotificationType.BOOKING_CONFIRMED, + recipientUserId: userId, + recipientCustomerId: customerId, + appointmentId, + bodyText: "Confirmed.", + idempotencyKey: `test:dedup:booking-confirmed:${appointmentId}`, + }); + const { notification: cancelled } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.BOOKING_CANCELLED, + recipientUserId: userId, + recipientCustomerId: customerId, + appointmentId, + bodyText: "Cancelled.", + idempotencyKey: `test:dedup:booking-cancelled:${appointmentId}`, + }); + // BOOKING_CONFIRMED/BOOKING_CANCELLED aren't in any equivalence group + // at all, so this is unaffected regardless of order. + expect(cancelled.deliveries.length).toBeGreaterThan(0); + }); + + it("does not dedup across different appointments, even for the same recipient and equivalent types", async () => { + const staff = await createTestStaff(business); + const { customerId: customerA } = await createTestCustomer(business); + const { customerId: customerB } = await createTestCustomer(business); + const { appointmentId: appointmentA } = await createTestAppointment(business, { customerId: customerA, staffId: staff.staffId }); + const { appointmentId: appointmentB } = await createTestAppointment(business, { customerId: customerB, staffId: staff.staffId }); + await enableNewBookingTypes(staff.userId); + + await notify(prisma, { + businessId: business.businessId, + type: NotificationType.STAFF_NEW_APPOINTMENT, + recipientUserId: staff.userId, + appointmentId: appointmentA, + bodyText: "New appointment.", + idempotencyKey: `test:dedup:staff-new-appointment:${appointmentA}`, + }); + const { notification: ownerNotification } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.OWNER_NEW_BOOKING, + recipientUserId: staff.userId, + appointmentId: appointmentB, + bodyText: "New booking.", + idempotencyKey: `test:dedup:owner-new-booking:${appointmentB}`, + }); + expect(ownerNotification.deliveries.length).toBeGreaterThan(0); + }); + + it("does not dedup across different recipients, even for the same appointment and equivalent types", async () => { + const staffPerson = await createTestStaff(business); + const ownerPerson = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + const { appointmentId } = await createTestAppointment(business, { customerId, staffId: staffPerson.staffId }); + await enableNewBookingTypes(staffPerson.userId, ownerPerson.userId); + + await notify(prisma, { + businessId: business.businessId, + type: NotificationType.STAFF_NEW_APPOINTMENT, + recipientUserId: staffPerson.userId, + appointmentId, + bodyText: "New appointment.", + idempotencyKey: `test:dedup:staff-new-appointment:${appointmentId}:${staffPerson.userId}`, + }); + const { notification: ownerNotification } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.OWNER_NEW_BOOKING, + recipientUserId: ownerPerson.userId, + appointmentId, + bodyText: "New booking.", + idempotencyKey: `test:dedup:owner-new-booking:${appointmentId}:${ownerPerson.userId}`, + }); + expect(ownerNotification.deliveries.length).toBeGreaterThan(0); + }); + + it("if the lower-priority OWNER_* is somehow created first, a later higher-priority STAFF_* still sends in full (order-inverted case)", async () => { + const staff = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + const { appointmentId } = await createTestAppointment(business, { customerId, staffId: staff.staffId }); + await enableNewBookingTypes(staff.userId); + + await notify(prisma, { + businessId: business.businessId, + type: NotificationType.OWNER_NEW_BOOKING, + recipientUserId: staff.userId, + appointmentId, + bodyText: "New booking.", + idempotencyKey: `test:dedup:owner-new-booking:${appointmentId}`, + }); + const { notification: staffNotification } = await notify(prisma, { + businessId: business.businessId, + type: NotificationType.STAFF_NEW_APPOINTMENT, + recipientUserId: staff.userId, + appointmentId, + bodyText: "New appointment.", + idempotencyKey: `test:dedup:staff-new-appointment:${appointmentId}`, + }); + expect(staffNotification.deliveries.length).toBeGreaterThan(0); + }); + }); }); diff --git a/src/services/notifications/notification.service.ts b/src/services/notifications/notification.service.ts index af70746..44b2d06 100644 --- a/src/services/notifications/notification.service.ts +++ b/src/services/notifications/notification.service.ts @@ -1,7 +1,7 @@ import { Prisma } from "@/generated/prisma/client"; import type { PrismaClient } from "@/generated/prisma/client"; import { NotificationChannel, NotificationDeliveryStatus, NotificationType } from "@/generated/prisma/enums"; -import { isPreferenceControlled } from "./types"; +import { defaultPreferenceIsEnabled, isPreferenceControlled } from "./types"; /** * The notification domain layer: creates Notification + NotificationDelivery @@ -32,6 +32,17 @@ export interface NotifyParams { idempotencyKey: string; /** Omit for "send as soon as the worker picks it up" (every non-reminder type). When set, EMAIL/PUSH deliveries are created directly as SCHEDULED for this time rather than PENDING — see reminder-scheduling.ts, which is the only caller that sets this. */ scheduledFor?: Date; + /** + * When true, EMAIL is never resolved for this call, regardless of any + * stored preference — for a fixed "this no longer emails" policy + * (e.g. BOOKING_COMPLETED, LOYALTY_REWARD_REDEEMED) rather than a + * user-toggleable default. PUSH/IN_APP are unaffected. Deliberately + * separate from the preference system: these types either aren't + * preference-controlled at all, or (BOOKING_BARBER_CHANGED) the + * decision depends on per-booking context a stored preference can't + * express. + */ + skipEmail?: boolean; } type NotificationWithDeliveries = Prisma.NotificationGetPayload<{ include: { deliveries: true } }>; @@ -113,7 +124,10 @@ async function isChannelEnabled( const preference = await prisma.notificationPreference.findUnique({ where: { userId_type_channel: { userId: params.userId, type: params.type, channel: params.channel } }, }); - return preference?.isEnabled ?? true; // sensible default: on unless the user explicitly turned it off + // A stored row always wins, whichever way it's set — only the fallback + // for "nobody has ever touched this" varies by type (see types.ts's + // defaultPreferenceIsEnabled). + return preference?.isEnabled ?? defaultPreferenceIsEnabled(params.type); } async function resolveChannels( @@ -137,7 +151,11 @@ async function resolveChannels( } const emailAddress = await resolveEmailAddress(prisma, params); - if (emailAddress && (await isChannelEnabled(prisma, { userId: params.recipientUserId, type: params.type, channel: NotificationChannel.EMAIL }))) { + if ( + emailAddress && + !params.skipEmail && + (await isChannelEnabled(prisma, { userId: params.recipientUserId, type: params.type, channel: NotificationChannel.EMAIL })) + ) { channels.push(NotificationChannel.EMAIL); } @@ -151,6 +169,81 @@ async function resolveChannels( return channels; } +/** + * Notification types that are two views of the SAME underlying booking + * event, from two different roles — a "new appointment" booked for a + * staff member is the exact same event as "a new booking" for the + * owner. When one person holds both roles for the same appointment + * (a solo owner-barber), they resolve to the identical recipientUserId + * (see recipients.ts's getOwnerUserIds vs appointment.staff.userId), and + * should only ever get ONE of the two — the one more directly tied to + * their actual involvement (higher `priority`), per the product rule + * "deduplicate equivalent information, don't suppress a genuinely + * different actionable purpose". Only types listed here ever participate + * in dedup — e.g. OWNER_NO_SHOW has no staff equivalent by design (the + * barber who marked it already knows) and is deliberately absent, as + * would any future owner-only actionable type (e.g. "approve this + * booking") — nothing about those is touched by this map. + */ +const EQUIVALENCE_GROUPS: Partial> = { + [NotificationType.STAFF_NEW_APPOINTMENT]: { group: "new_appointment", priority: 2 }, + [NotificationType.OWNER_NEW_BOOKING]: { group: "new_appointment", priority: 1 }, + [NotificationType.STAFF_APPOINTMENT_CANCELLED]: { group: "appointment_cancelled", priority: 2 }, + [NotificationType.OWNER_BOOKING_CANCELLED]: { group: "appointment_cancelled", priority: 1 }, +}; + +type EquivalenceOutcome = + | { action: "suppress"; existingType: NotificationType } + | { action: "order-inverted"; existingType: NotificationType }; + +/** + * Checks for an already-created, equivalent (same group) notification for + * this exact recipient+appointment. Relies on call order: every current + * call site (booking-notifications.ts) already notifies STAFF_* before + * looping OWNER_* for the same event, so by the time an OWNER_* call + * runs, an equivalent STAFF_* row (if any) already exists in the + * database. + * + * - The existing row's priority >= this call's → "suppress": the caller + * should create zero deliveries (still create the Notification row + * itself — keeps idempotencyKey semantics intact and leaves an audit + * trail of "this would have fired"). + * - The existing row's priority < this call's → "order-inverted": this + * shouldn't happen given today's call order; proceed normally, but + * this is surfaced so a future call-order regression is at least + * observable rather than silently wrong. Retroactively un-sending an + * already-SENT IN_APP row for a case that can't currently occur would + * be disproportionate — see this file's own design notes. + */ +async function checkEquivalenceGroup( + prisma: PrismaClient, + params: Pick, +): Promise { + const info = EQUIVALENCE_GROUPS[params.type]; + if (!info || !params.recipientUserId || !params.appointmentId) return null; + + const otherTypesInGroup = (Object.keys(EQUIVALENCE_GROUPS) as NotificationType[]).filter( + (t) => t !== params.type && EQUIVALENCE_GROUPS[t]!.group === info.group, + ); + if (otherTypesInGroup.length === 0) return null; + + const existing = await prisma.notification.findFirst({ + where: { + businessId: params.businessId, + appointmentId: params.appointmentId, + recipientUserId: params.recipientUserId, + type: { in: otherTypesInGroup }, + }, + select: { type: true }, + }); + if (!existing) return null; + + const existingPriority = EQUIVALENCE_GROUPS[existing.type]!.priority; + return existingPriority >= info.priority + ? { action: "suppress", existingType: existing.type } + : { action: "order-inverted", existingType: existing.type }; +} + /** * Creates a Notification and one NotificationDelivery per applicable * channel — idempotent on `idempotencyKey`: a second call with the same @@ -170,7 +263,21 @@ export async function notify(prisma: PrismaClient, params: NotifyParams): Promis }); if (existing) return { created: false, notification: existing }; - const channels = await resolveChannels(prisma, params); + let channels = await resolveChannels(prisma, params); + + const equivalence = await checkEquivalenceGroup(prisma, params); + if (equivalence?.action === "suppress") { + // Someone already has an equivalent, at-least-as-relevant notification + // for this exact booking (e.g. they're both the owner and the + // assigned staff) — don't duplicate it on any channel. The + // Notification row below is still created (audit trail, consistent + // idempotencyKey behaviour), just with zero deliveries. + channels = []; + } else if (equivalence?.action === "order-inverted") { + console.warn( + `[NotificationService] ${params.type} has higher priority than an existing equivalent ${equivalence.existingType} for the same recipient/appointment, but the earlier notification can't be retroactively suppressed — check call order for this event.`, + ); + } try { const notification = await prisma.notification.create({ diff --git a/src/services/notifications/types.ts b/src/services/notifications/types.ts index b610783..392817e 100644 --- a/src/services/notifications/types.ts +++ b/src/services/notifications/types.ts @@ -83,3 +83,37 @@ const PREFERENCE_CONTROLLED_TYPES = new Set([ export function isPreferenceControlled(type: NotificationType): boolean { return PREFERENCE_CONTROLLED_TYPES.has(type); } + +/** + * Which preference-controlled types default to OFF when no + * NotificationPreference row exists yet — every other preference- + * controlled type keeps defaulting to on (today's original blanket + * behaviour). A stored row always wins regardless of what's here (see + * NotificationService.isChannelEnabled's `preference?.isEnabled ?? ...`), + * so changing this set never touches anyone's explicit choice — only + * what a brand-new user/business sees before they've ever visited + * Settings. The owner of a business is commonly also its only barber, so + * without softer defaults here a solo owner's very first booking would + * arrive as two emails (STAFF_NEW_APPOINTMENT + OWNER_NEW_BOOKING) they + * never asked for — see also the dedup logic in notification.service.ts, + * which independently prevents a *duplicate* even when both default on. + */ +const DEFAULT_DISABLED_TYPES = new Set([ + NotificationType.STAFF_NEW_APPOINTMENT, + NotificationType.STAFF_APPOINTMENT_RESCHEDULED, + NotificationType.STAFF_APPOINTMENT_REMINDER, + NotificationType.OWNER_NEW_BOOKING, + NotificationType.OWNER_NO_SHOW, +]); + +/** + * The single source of truth for "what does this preference-controlled + * type do when nobody has ever saved a row for it" — used both by the + * actual send decision (isChannelEnabled) and by the Settings pages that + * render each toggle's initial on/off state, so the UI can never show a + * toggle as on for something that actually defaults to off (or vice + * versa). Meaningless for a required (non-preference-controlled) type. + */ +export function defaultPreferenceIsEnabled(type: NotificationType): boolean { + return !DEFAULT_DISABLED_TYPES.has(type); +} diff --git a/src/services/scheduling/__tests__/booking-notifications.integration.test.ts b/src/services/scheduling/__tests__/booking-notifications.integration.test.ts index f72d0f2..0462020 100644 --- a/src/services/scheduling/__tests__/booking-notifications.integration.test.ts +++ b/src/services/scheduling/__tests__/booking-notifications.integration.test.ts @@ -1,9 +1,12 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { Weekday } from "@/generated/prisma/enums"; +import { NotificationChannel, NotificationType, Weekday } from "@/generated/prisma/enums"; import { addDays, todayIn, wallClockToInstant, weekdayOf } from "@/lib/time/business-time"; import type { CalendarDate } from "@/lib/time/business-time"; +import { setNotificationPreference } from "@/services/notifications/notification.service"; +import { getAppointmentById } from "../appointment.service"; +import { notifyBookingBarberChanged, notifyBookingCompleted } from "../booking-notifications"; import { cancelBooking, createBooking, rescheduleBooking } from "../booking.service"; -import { cleanupTestBusiness, createTestBusiness, createTestCustomer, createTestGuestCustomer, createTestStaff, prisma } from "./fixtures"; +import { cleanupTestBusiness, createTestAppointment, createTestBusiness, createTestCustomer, createTestGuestCustomer, createTestStaff, prisma } from "./fixtures"; import type { TestBusiness } from "./fixtures"; function nextWeekday(timezone: string, target: Weekday, minDaysAhead = 7): CalendarDate { @@ -66,15 +69,19 @@ describe("booking notifications (integration)", () => { expect(result.success).toBe(true); if (!result.success) return; - const notifications = await prisma.notification.findMany({ where: { appointmentId: result.appointment.id } }); + const notifications = await prisma.notification.findMany({ + where: { appointmentId: result.appointment.id }, + include: { deliveries: true }, + }); const types = notifications.map((n) => n.type).sort(); - // A booking far enough out (monday is >= 7 days ahead — see nextWeekday) also schedules both - // 24h/2h reminders for the customer and the staff member — see reminder-scheduling.ts. + // A booking far enough out (monday is >= 7 days ahead — see + // nextWeekday) also schedules the customer's 24h reminder (only one — + // the 2h leg was removed, see reminder-scheduling.ts) and the staff + // member's 24h+2h reminders. expect(types).toEqual( [ "BOOKING_CONFIRMED", "BOOKING_REMINDER", - "BOOKING_REMINDER", "OWNER_NEW_BOOKING", "STAFF_APPOINTMENT_REMINDER", "STAFF_APPOINTMENT_REMINDER", @@ -89,9 +96,49 @@ describe("booking notifications (integration)", () => { const staffNotification = notifications.find((n) => n.type === "STAFF_NEW_APPOINTMENT"); expect(staffNotification?.recipientUserId).toBe(staffUserId); - // Same staff member holds both roles in this test, so OWNER_NEW_BOOKING also targets them. + // Same staff member holds both roles in this test, so OWNER_NEW_BOOKING + // also targets them — but the row has zero deliveries: dedup suppresses + // it in favour of the STAFF_NEW_APPOINTMENT they already got (see + // notification.service.ts's EQUIVALENCE_GROUPS). const ownerNotification = notifications.find((n) => n.type === "OWNER_NEW_BOOKING"); expect(ownerNotification?.recipientUserId).toBe(staffUserId); + expect(ownerNotification?.deliveries).toHaveLength(0); + }); + + it("an owner who is a DIFFERENT person from the assigned staff gets their own full, undeduped OWNER_NEW_BOOKING", async () => { + const { staffId, userId: staffUserId } = await createTestStaff(business); + const otherOwner = await createTestStaff(business); + await grantOwnerPermission(otherOwner.userId); + const { customerId } = await createTestCustomer(business); + // STAFF_NEW_APPOINTMENT/OWNER_NEW_BOOKING both default to off (see + // types.ts) — enable both explicitly so this test isolates "different + // people don't dedup against each other" from the unrelated default. + await setNotificationPreference(prisma, { userId: staffUserId, type: NotificationType.STAFF_NEW_APPOINTMENT, channel: NotificationChannel.IN_APP, isEnabled: true }); + await setNotificationPreference(prisma, { userId: otherOwner.userId, type: NotificationType.OWNER_NEW_BOOKING, channel: NotificationChannel.IN_APP, isEnabled: true }); + + const result = await createBooking(prisma, { + businessId: business.businessId, + customerId, + staffId, + serviceIds: [business.haircutServiceId], + startAt: wallClockToInstant(monday, 10 * 60, business.timezone), + }); + expect(result.success).toBe(true); + if (!result.success) return; + + const staffNotification = await prisma.notification.findFirst({ + where: { appointmentId: result.appointment.id, type: "STAFF_NEW_APPOINTMENT" }, + include: { deliveries: true }, + }); + expect(staffNotification?.recipientUserId).toBe(staffUserId); + expect(staffNotification?.deliveries.length).toBeGreaterThan(0); + + const ownerNotification = await prisma.notification.findFirst({ + where: { appointmentId: result.appointment.id, type: "OWNER_NEW_BOOKING" }, + include: { deliveries: true }, + }); + expect(ownerNotification?.recipientUserId).toBe(otherOwner.userId); + expect(ownerNotification?.deliveries.length).toBeGreaterThan(0); }); it("a guest booking's customer notification has no recipientUserId but does have a deep link with the guest token", async () => { @@ -194,9 +241,16 @@ describe("booking notifications (integration)", () => { const cancelNotifications = await prisma.notification.findMany({ where: { appointmentId: created.appointment.id, type: { in: ["BOOKING_CANCELLED", "STAFF_APPOINTMENT_CANCELLED", "OWNER_BOOKING_CANCELLED"] } }, + include: { deliveries: true }, }); expect(cancelNotifications).toHaveLength(3); + // Same staff member holds both roles — OWNER_BOOKING_CANCELLED is + // deduped against STAFF_APPOINTMENT_CANCELLED (zero deliveries), same + // as the new-booking case above. + const ownerCancelNotification = cancelNotifications.find((n) => n.type === "OWNER_BOOKING_CANCELLED"); + expect(ownerCancelNotification?.deliveries).toHaveLength(0); + // Cancelling again is rejected by cancelBooking itself (already CANCELLED) — never reaches notify() a second time. const secondAttempt = await cancelBooking(prisma, { appointmentId: created.appointment.id }); expect(secondAttempt.success).toBe(false); @@ -205,4 +259,56 @@ describe("booking notifications (integration)", () => { }); expect(stillOnlyOnce).toBe(1); }); + + it("notifyBookingCompleted no longer emails by default (still in-app)", async () => { + const { staffId } = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + const { appointmentId } = await createTestAppointment(business, { customerId, staffId }); + const appointment = await getAppointmentById(prisma, appointmentId); + + await notifyBookingCompleted(prisma, appointment!); + + const notification = await prisma.notification.findFirst({ + where: { appointmentId, type: "BOOKING_COMPLETED" }, + include: { deliveries: true }, + }); + const channels = notification?.deliveries.map((d) => d.channel) ?? []; + expect(channels).not.toContain("EMAIL"); + expect(channels).toContain("IN_APP"); + }); + + describe("notifyBookingBarberChanged — email depends on the ORIGINAL booking's staff selection", () => { + it("emails the customer when they originally picked this staff member specifically", async () => { + const { staffId } = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + const { appointmentId } = await createTestAppointment(business, { customerId, staffId, staffSelectedByCustomer: true }); + const appointment = await getAppointmentById(prisma, appointmentId); + + await notifyBookingBarberChanged(prisma, appointment!, { customerSelectedStaff: true }); + + const notification = await prisma.notification.findFirst({ + where: { appointmentId, type: "BOOKING_BARBER_CHANGED" }, + include: { deliveries: true }, + }); + expect(notification?.deliveries.map((d) => d.channel)).toContain("EMAIL"); + }); + + it("does not email the customer when they originally booked 'any available barber'", async () => { + const { staffId } = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + const { appointmentId } = await createTestAppointment(business, { customerId, staffId, staffSelectedByCustomer: false }); + const appointment = await getAppointmentById(prisma, appointmentId); + + await notifyBookingBarberChanged(prisma, appointment!, { customerSelectedStaff: false }); + + const notification = await prisma.notification.findFirst({ + where: { appointmentId, type: "BOOKING_BARBER_CHANGED" }, + include: { deliveries: true }, + }); + const channels = notification?.deliveries.map((d) => d.channel) ?? []; + expect(channels).not.toContain("EMAIL"); + // Still notified on other channels — this is "don't email", not "don't notify". + expect(channels).toContain("IN_APP"); + }); + }); }); diff --git a/src/services/scheduling/__tests__/fixtures.ts b/src/services/scheduling/__tests__/fixtures.ts index 1134d38..9140305 100644 --- a/src/services/scheduling/__tests__/fixtures.ts +++ b/src/services/scheduling/__tests__/fixtures.ts @@ -170,6 +170,31 @@ export async function createTestGuestCustomer( return { customerId: created.id, email, phone }; } +/** + * A minimal, directly-inserted Appointment row — for tests that only need + * a real appointmentId to satisfy the Notification.appointmentId foreign + * key (e.g. notification-service dedup tests), not the full + * booking.service.ts availability/validation workflow. Deliberately + * skips AppointmentService line items — nothing here reads them. + */ +export async function createTestAppointment( + business: TestBusiness, + params: { customerId: string; staffId: string; startAt?: Date; staffSelectedByCustomer?: boolean }, +): Promise<{ appointmentId: string }> { + const startAt = params.startAt ?? new Date(Date.now() + 24 * 60 * 60_000); + const appointment = await prisma.appointment.create({ + data: { + businessId: business.businessId, + customerId: params.customerId, + staffId: params.staffId, + startAt, + endAt: new Date(startAt.getTime() + 30 * 60_000), + ...(params.staffSelectedByCustomer !== undefined ? { staffSelectedByCustomer: params.staffSelectedByCustomer } : {}), + }, + }); + return { appointmentId: appointment.id }; +} + export async function cleanupTestBusiness(business: TestBusiness): Promise { // Appointment.staff is onDelete: Restrict (see prisma/schema.prisma) — a // deliberate production safeguard against deleting a staff member with diff --git a/src/services/scheduling/__tests__/reminder-scheduling.integration.test.ts b/src/services/scheduling/__tests__/reminder-scheduling.integration.test.ts index d0f4da4..e6a7db4 100644 --- a/src/services/scheduling/__tests__/reminder-scheduling.integration.test.ts +++ b/src/services/scheduling/__tests__/reminder-scheduling.integration.test.ts @@ -1,7 +1,8 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { Weekday } from "@/generated/prisma/enums"; +import { NotificationChannel, NotificationType, Weekday } from "@/generated/prisma/enums"; import { addDays, todayIn, wallClockToInstant, weekdayOf } from "@/lib/time/business-time"; import type { CalendarDate } from "@/lib/time/business-time"; +import { setNotificationPreference } from "@/services/notifications/notification.service"; import { cancelBooking, createBooking, rescheduleBooking } from "../booking.service"; import { cleanupTestBusiness, createTestBusiness, createTestCustomer, createTestStaff, prisma } from "./fixtures"; import type { TestBusiness } from "./fixtures"; @@ -25,6 +26,17 @@ async function reminderDeliveries(appointmentId: string) { }); } +/** + * Customer and staff reminders now have independently-controllable + * defaults (see reminder-scheduling.ts's split CUSTOMER_/STAFF_ + * REMINDER_HOURS_BEFORE, and types.ts's DEFAULT_DISABLED_TYPES): a + * customer's reminder is 24h-before only and defaults on; a staff + * member's is still 24h+2h-before but defaults OFF (an explicit + * NotificationPreference row is needed to see it). Most tests below + * enable the staff preference up front specifically to exercise that + * still-working mechanism end to end — see "with the staff reminder + * preference enabled" for why. + */ describe("reminder scheduling (integration)", () => { let business: TestBusiness; let monday: CalendarDate; @@ -38,7 +50,7 @@ describe("reminder scheduling (integration)", () => { await cleanupTestBusiness(business); }); - it("schedules 24h and 2h EMAIL reminders for both customer and staff, as SCHEDULED (not PENDING)", async () => { + it("by default, only schedules the customer's 24h EMAIL reminder — the staff member gets none (disabled by default)", async () => { const { staffId } = await createTestStaff(business); const { customerId } = await createTestCustomer(business); @@ -52,12 +64,155 @@ describe("reminder scheduling (integration)", () => { if (!result.success) throw new Error("setup failed"); const deliveries = await reminderDeliveries(result.appointment.id); - expect(deliveries).toHaveLength(4); // 2 offsets × (customer + staff) - for (const d of deliveries) { - expect(d.status).toBe("SCHEDULED"); - expect(d.scheduledFor).not.toBeNull(); - expect(d.scheduledFor!.getTime()).toBeLessThan(result.appointment.startAt.getTime()); + expect(deliveries).toHaveLength(1); + expect(deliveries[0]!.notification.type).toBe("BOOKING_REMINDER"); + expect(deliveries[0]!.status).toBe("SCHEDULED"); + expect(deliveries[0]!.scheduledFor!.getTime()).toBe( + result.appointment.startAt.getTime() - 24 * 60 * 60 * 1000, + ); + }); + + it("never schedules a 2-hour reminder for the customer, even with the staff preference enabled", async () => { + const { staffId, userId: staffUserId } = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + await setNotificationPreference(prisma, { + userId: staffUserId, + type: NotificationType.STAFF_APPOINTMENT_REMINDER, + channel: NotificationChannel.EMAIL, + isEnabled: true, + }); + + const result = await createBooking(prisma, { + businessId: business.businessId, + customerId, + staffId, + serviceIds: [business.haircutServiceId], + startAt: wallClockToInstant(monday, 10 * 60, business.timezone), + }); + if (!result.success) throw new Error("setup failed"); + + const deliveries = await reminderDeliveries(result.appointment.id); + const customerReminders = deliveries.filter((d) => d.notification.type === "BOOKING_REMINDER"); + expect(customerReminders).toHaveLength(1); // 24h only, never a second one at 2h + }); + + describe("with the staff reminder preference enabled", () => { + async function enableStaffReminderEmail(staffUserId: string) { + await setNotificationPreference(prisma, { + userId: staffUserId, + type: NotificationType.STAFF_APPOINTMENT_REMINDER, + channel: NotificationChannel.EMAIL, + isEnabled: true, + }); } + + it("schedules 24h and 2h EMAIL reminders for the staff member, alongside the customer's 24h one, as SCHEDULED (not PENDING)", async () => { + const { staffId, userId: staffUserId } = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + await enableStaffReminderEmail(staffUserId); + + const result = await createBooking(prisma, { + businessId: business.businessId, + customerId, + staffId, + serviceIds: [business.haircutServiceId], + startAt: wallClockToInstant(monday, 10 * 60, business.timezone), + }); + if (!result.success) throw new Error("setup failed"); + + const deliveries = await reminderDeliveries(result.appointment.id); + expect(deliveries).toHaveLength(3); // customer 24h + staff 24h + staff 2h + for (const d of deliveries) { + expect(d.status).toBe("SCHEDULED"); + expect(d.scheduledFor).not.toBeNull(); + expect(d.scheduledFor!.getTime()).toBeLessThan(result.appointment.startAt.getTime()); + } + const staffReminders = deliveries.filter((d) => d.notification.type === "STAFF_APPOINTMENT_REMINDER"); + expect(staffReminders).toHaveLength(2); + }); + + it("does not schedule a reminder offset that would already be in the past", async () => { + const { staffId, userId: staffUserId } = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + await enableStaffReminderEmail(staffUserId); + + // Book for 3 hours from now — the customer's only offset (24h) and + // the staff's 24h offset are both already past; the staff's 2h + // offset is still in the future. + const result = await createBooking(prisma, { + businessId: business.businessId, + customerId, + staffId, + serviceIds: [business.haircutServiceId], + startAt: new Date(Date.now() + 3 * 60 * 60_000), + }).catch(() => null); + + // This may fail validation depending on min-notice/working-hours fixtures; only assert the + // reminder-skipping behaviour when the booking itself actually succeeded. + if (!result || !result.success) return; + const deliveries = await reminderDeliveries(result.appointment.id); + expect(deliveries).toHaveLength(1); + expect(deliveries[0]!.notification.type).toBe("STAFF_APPOINTMENT_REMINDER"); + expect(deliveries[0]!.notification.idempotencyKey).toContain("reminder-2h"); + }); + + it("rescheduling cancels the old reminders and schedules fresh ones for the new time", async () => { + const { staffId, userId: staffUserId } = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + await enableStaffReminderEmail(staffUserId); + + const created = await createBooking(prisma, { + businessId: business.businessId, + customerId, + staffId, + serviceIds: [business.haircutServiceId], + startAt: wallClockToInstant(monday, 10 * 60, business.timezone), + }); + if (!created.success) throw new Error("setup failed"); + + const originalDeliveries = await reminderDeliveries(created.appointment.id); + expect(originalDeliveries).toHaveLength(3); + + const rescheduled = await rescheduleBooking(prisma, { + appointmentId: created.appointment.id, + startAt: wallClockToInstant(monday, 14 * 60, business.timezone), + }); + expect(rescheduled.success).toBe(true); + if (!rescheduled.success) return; + + const allDeliveries = await reminderDeliveries(created.appointment.id); + const cancelledCount = allDeliveries.filter((d) => d.status === "CANCELLED").length; + const scheduledCount = allDeliveries.filter((d) => d.status === "SCHEDULED").length; + expect(cancelledCount).toBe(3); // the original 3, now superseded + expect(scheduledCount).toBe(3); // 3 fresh ones for the new time + + const freshOnes = allDeliveries.filter((d) => d.status === "SCHEDULED"); + for (const d of freshOnes) { + expect(d.scheduledFor!.getTime()).toBeLessThan(rescheduled.appointment.startAt.getTime()); + } + }); + + it("cancelling the appointment cancels its scheduled reminders", async () => { + const { staffId, userId: staffUserId } = await createTestStaff(business); + const { customerId } = await createTestCustomer(business); + await enableStaffReminderEmail(staffUserId); + + const created = await createBooking(prisma, { + businessId: business.businessId, + customerId, + staffId, + serviceIds: [business.haircutServiceId], + startAt: wallClockToInstant(monday, 10 * 60, business.timezone), + }); + if (!created.success) throw new Error("setup failed"); + + const cancelled = await cancelBooking(prisma, { appointmentId: created.appointment.id }); + expect(cancelled.success).toBe(true); + + const deliveries = await reminderDeliveries(created.appointment.id); + expect(deliveries.every((d) => d.status === "CANCELLED")).toBe(true); + expect(deliveries).toHaveLength(3); + }); }); it("does not schedule reminders for a PENDING (requireApproval) booking", async () => { @@ -77,82 +232,4 @@ describe("reminder scheduling (integration)", () => { const deliveries = await reminderDeliveries(result.appointment.id); expect(deliveries).toHaveLength(0); }); - - it("does not schedule a reminder offset that would already be in the past", async () => { - const { staffId } = await createTestStaff(business); - const { customerId } = await createTestCustomer(business); - - // Book for 1 hour from now — the 24h and 2h-before offsets are both already past. - const result = await createBooking(prisma, { - businessId: business.businessId, - customerId, - staffId, - serviceIds: [business.haircutServiceId], - startAt: new Date(Date.now() + 3 * 60 * 60_000), // 3h out, staff working hours permitting — see note below - }).catch(() => null); - - // This may fail validation depending on min-notice/working-hours fixtures; only assert the - // reminder-skipping behaviour when the booking itself actually succeeded. - if (!result || !result.success) return; - const deliveries = await reminderDeliveries(result.appointment.id); - // 3h out: the 2h-before offset is still in the future, the 24h-before offset is in the past. - expect(deliveries.length).toBeLessThanOrEqual(2); - expect(deliveries.every((d) => d.notification.idempotencyKey.includes("reminder-2h"))).toBe(true); - }); - - it("rescheduling cancels the old reminders and schedules fresh ones for the new time", async () => { - const { staffId } = await createTestStaff(business); - const { customerId } = await createTestCustomer(business); - - const created = await createBooking(prisma, { - businessId: business.businessId, - customerId, - staffId, - serviceIds: [business.haircutServiceId], - startAt: wallClockToInstant(monday, 10 * 60, business.timezone), - }); - if (!created.success) throw new Error("setup failed"); - - const originalDeliveries = await reminderDeliveries(created.appointment.id); - expect(originalDeliveries).toHaveLength(4); - - const rescheduled = await rescheduleBooking(prisma, { - appointmentId: created.appointment.id, - startAt: wallClockToInstant(monday, 14 * 60, business.timezone), - }); - expect(rescheduled.success).toBe(true); - if (!rescheduled.success) return; - - const allDeliveries = await reminderDeliveries(created.appointment.id); - const cancelledCount = allDeliveries.filter((d) => d.status === "CANCELLED").length; - const scheduledCount = allDeliveries.filter((d) => d.status === "SCHEDULED").length; - expect(cancelledCount).toBe(4); // the original 4, now superseded - expect(scheduledCount).toBe(4); // 4 fresh ones for the new time - - const freshOnes = allDeliveries.filter((d) => d.status === "SCHEDULED"); - for (const d of freshOnes) { - expect(d.scheduledFor!.getTime()).toBeLessThan(rescheduled.appointment.startAt.getTime()); - } - }); - - it("cancelling the appointment cancels its scheduled reminders", async () => { - const { staffId } = await createTestStaff(business); - const { customerId } = await createTestCustomer(business); - - const created = await createBooking(prisma, { - businessId: business.businessId, - customerId, - staffId, - serviceIds: [business.haircutServiceId], - startAt: wallClockToInstant(monday, 10 * 60, business.timezone), - }); - if (!created.success) throw new Error("setup failed"); - - const cancelled = await cancelBooking(prisma, { appointmentId: created.appointment.id }); - expect(cancelled.success).toBe(true); - - const deliveries = await reminderDeliveries(created.appointment.id); - expect(deliveries.every((d) => d.status === "CANCELLED")).toBe(true); - expect(deliveries).toHaveLength(4); - }); }); diff --git a/src/services/scheduling/booking-notifications.ts b/src/services/scheduling/booking-notifications.ts index 27001ef..7b15e7d 100644 --- a/src/services/scheduling/booking-notifications.ts +++ b/src/services/scheduling/booking-notifications.ts @@ -186,8 +186,28 @@ export async function notifyBookingApproved(prisma: PrismaClient, appointment: A await scheduleAppointmentReminders(prisma, appointment); } -/** The owner reassigns an appointment to a different staff member — distinct from a plain time reschedule (see notifyBookingRescheduled, which still also fires for a reassignment since the underlying rescheduleBooking() call can't distinguish "why", see ownerRescheduleAppointmentAction). This tells the customer specifically who they're now booked with. */ -export async function notifyBookingBarberChanged(prisma: PrismaClient, appointment: Appointment): Promise { +/** + * The owner reassigns an appointment to a different staff member — + * distinct from a plain time reschedule (see notifyBookingRescheduled, + * which still also fires for a reassignment since the underlying + * rescheduleBooking() call can't distinguish "why", see + * ownerRescheduleAppointmentAction). This tells the customer specifically + * who they're now booked with. + * + * `customerSelectedStaff` should reflect the ORIGINAL booking (see + * Appointment.staffSelectedByCustomer), not the current staffId — a + * customer who booked "any available barber" and never expressed a + * preference doesn't need an email every time the business reassigns + * who's cutting their hair, though push/in-app still fire either way + * (the deep link is still useful, this is specifically about inbox + * noise). Defaults to true (email) so every existing call site keeps + * behaving exactly as before unless it explicitly opts out. + */ +export async function notifyBookingBarberChanged( + prisma: PrismaClient, + appointment: Appointment, + options: { customerSelectedStaff?: boolean } = {}, +): Promise { await notifySafely(prisma, { businessId: appointment.businessId, type: NotificationType.BOOKING_BARBER_CHANGED, @@ -198,10 +218,11 @@ export async function notifyBookingBarberChanged(prisma: PrismaClient, appointme bodyText: `Your appointment for ${serviceNames(appointment)} is now with ${appointment.staff.user.name}.`, deepLinkPath: `/appointments/${appointment.id}`, idempotencyKey: `notify:booking-barber-changed:${appointment.id}:${appointment.staffId}`, + skipEmail: options.customerSelectedStaff === false, }); } -/** The appointment reaches COMPLETED — a distinct, non-loyalty "thanks for visiting" notice (see LoyaltyService for the separate stamp/reward notifications, which fire independently). */ +/** The appointment reaches COMPLETED — a distinct, non-loyalty "thanks for visiting" notice (see LoyaltyService for the separate stamp/reward notifications, which fire independently). No longer emailed by default (still push/in-app) — a receipt-style email after every single visit was more noise than value. */ export async function notifyBookingCompleted(prisma: PrismaClient, appointment: Appointment): Promise { await notifySafely(prisma, { businessId: appointment.businessId, @@ -213,6 +234,7 @@ export async function notifyBookingCompleted(prisma: PrismaClient, appointment: bodyText: `Thanks for visiting ${appointment.staff.user.name ? `and seeing ${appointment.staff.user.name}` : ""} — hope you loved your ${serviceNames(appointment)}!`, deepLinkPath: `/appointments/${appointment.id}`, idempotencyKey: `notify:booking-completed:${appointment.id}`, + skipEmail: true, }); } diff --git a/src/services/scheduling/booking.service.ts b/src/services/scheduling/booking.service.ts index 6a41562..84d037e 100644 --- a/src/services/scheduling/booking.service.ts +++ b/src/services/scheduling/booking.service.ts @@ -143,6 +143,11 @@ export async function createBooking( status, notes: input.notes, createdByUserId: input.createdByUserId, + // input.staffId (not the resolved local `staffId` above, which is + // always concrete by this point) is undefined exactly when the + // customer asked for "any available barber" — see + // Appointment.staffSelectedByCustomer in schema.prisma. + staffSelectedByCustomer: !!input.staffId, effectiveBuffer: context.effectiveBuffer, services: context.services.map((s) => ({ serviceId: s.serviceId, diff --git a/src/services/scheduling/reminder-scheduling.ts b/src/services/scheduling/reminder-scheduling.ts index 0f6ec4a..f34ee97 100644 --- a/src/services/scheduling/reminder-scheduling.ts +++ b/src/services/scheduling/reminder-scheduling.ts @@ -10,8 +10,17 @@ import type { Appointment } from "./booking.service"; * docs/phase-7-summary.md, "Known limitations"). A natural home for this * would be two new BookingRule fields feeding the existing Booking Rules * editor — not built this phase. + * + * Customer and staff reminders deliberately have separate offset lists, + * not one shared one — a 2-hour-before reminder on top of the 24-hour one + * was more noise than value for a customer, but a barber still benefits + * from both (a same-day heads-up on top of the previous day's one). */ -const REMINDER_HOURS_BEFORE = [24, 2] as const; +const CUSTOMER_REMINDER_HOURS_BEFORE = [24] as const; +const STAFF_REMINDER_HOURS_BEFORE = [24, 2] as const; +const ALL_REMINDER_HOURS_BEFORE = Array.from( + new Set([...CUSTOMER_REMINDER_HOURS_BEFORE, ...STAFF_REMINDER_HOURS_BEFORE]), +).sort((a, b) => b - a); function formatDateLabel(date: Date, timezone: string): string { return date.toLocaleDateString("en-GB", { weekday: "short", day: "numeric", month: "short", timeZone: timezone }); @@ -66,33 +75,37 @@ export async function scheduleAppointmentReminders(prisma: PrismaClient, appoint const now = new Date(); const versionSuffix = appointment.startAt.getTime(); - for (const hoursBefore of REMINDER_HOURS_BEFORE) { + for (const hoursBefore of ALL_REMINDER_HOURS_BEFORE) { const scheduledFor = new Date(appointment.startAt.getTime() - hoursBefore * 60 * 60 * 1000); if (scheduledFor <= now) continue; - await notifySafely(prisma, { - businessId: appointment.businessId, - type: NotificationType.BOOKING_REMINDER, - recipientUserId: appointment.customer.userId ?? undefined, - recipientCustomerId: appointment.customer.id, - appointmentId: appointment.id, - subject: "Upcoming appointment reminder", - bodyText: `Reminder: your appointment for ${services} with ${appointment.staff.user.name} is on ${dateLabel} at ${timeLabel}.`, - deepLinkPath: `/appointments/${appointment.id}`, - idempotencyKey: `notify:booking-reminder-${hoursBefore}h:${appointment.id}:${versionSuffix}`, - scheduledFor, - }); + if ((CUSTOMER_REMINDER_HOURS_BEFORE as readonly number[]).includes(hoursBefore)) { + await notifySafely(prisma, { + businessId: appointment.businessId, + type: NotificationType.BOOKING_REMINDER, + recipientUserId: appointment.customer.userId ?? undefined, + recipientCustomerId: appointment.customer.id, + appointmentId: appointment.id, + subject: "Upcoming appointment reminder", + bodyText: `Reminder: your appointment for ${services} with ${appointment.staff.user.name} is on ${dateLabel} at ${timeLabel}.`, + deepLinkPath: `/appointments/${appointment.id}`, + idempotencyKey: `notify:booking-reminder-${hoursBefore}h:${appointment.id}:${versionSuffix}`, + scheduledFor, + }); + } - await notifySafely(prisma, { - businessId: appointment.businessId, - type: NotificationType.STAFF_APPOINTMENT_REMINDER, - recipientUserId: appointment.staff.userId, - appointmentId: appointment.id, - subject: "Upcoming appointment reminder", - bodyText: `Reminder: ${appointment.customer.firstName} ${appointment.customer.lastName} (${services}) is booked for ${dateLabel} at ${timeLabel}.`, - deepLinkPath: `/staff/appointments/${appointment.id}`, - idempotencyKey: `notify:staff-appointment-reminder-${hoursBefore}h:${appointment.id}:${versionSuffix}`, - scheduledFor, - }); + if ((STAFF_REMINDER_HOURS_BEFORE as readonly number[]).includes(hoursBefore)) { + await notifySafely(prisma, { + businessId: appointment.businessId, + type: NotificationType.STAFF_APPOINTMENT_REMINDER, + recipientUserId: appointment.staff.userId, + appointmentId: appointment.id, + subject: "Upcoming appointment reminder", + bodyText: `Reminder: ${appointment.customer.firstName} ${appointment.customer.lastName} (${services}) is booked for ${dateLabel} at ${timeLabel}.`, + deepLinkPath: `/staff/appointments/${appointment.id}`, + idempotencyKey: `notify:staff-appointment-reminder-${hoursBefore}h:${appointment.id}:${versionSuffix}`, + scheduledFor, + }); + } } } diff --git a/src/services/scheduling/scheduling.service.ts b/src/services/scheduling/scheduling.service.ts index 0042078..9108802 100644 --- a/src/services/scheduling/scheduling.service.ts +++ b/src/services/scheduling/scheduling.service.ts @@ -65,6 +65,8 @@ export interface CreateAppointmentParams { createdByUserId?: string; effectiveBuffer: { beforeMinutes: number; afterMinutes: number }; services: AppointmentServiceLine[]; + /** See Appointment.staffSelectedByCustomer in schema.prisma. */ + staffSelectedByCustomer: boolean; } export interface RescheduleAppointmentParams { @@ -188,6 +190,7 @@ export async function createAppointment(prisma: PrismaClient, params: CreateAppo status: params.status, notes: params.notes, createdByUserId: params.createdByUserId, + staffSelectedByCustomer: params.staffSelectedByCustomer, services: { create: params.services.map((s) => ({ serviceId: s.serviceId,