mirror of
https://github.com/Dvorinka/Bookra.git
synced 2026-10-28 15:51:15 +00:00
fix(reminders): lifecycle-aware jobs and dispatch skip guard
The reminder pipeline fired regardless of what happened to the booking after creation: cancelled customers got reminded about bookings that weren't happening, reschedules kept the old reminder time, and waitlisted bookings armed jobs too. Pending jobs are now cancelled on both cancel paths, deleted and re-armed on reschedule, and dispatch joins bookings.status to skip anything not confirmed. Reschedule also gained the same availability-window and blocked-day validation as booking creation, closing a 3am-reschedule hole via the manage URL. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
0dc26e0c16
commit
cdad7875e1
12 files changed
+272
-9
No files matched your search
@@ -168,10 +168,35 @@ func (s *CustomerService) RescheduleBooking(ctx context.Context, reference strin
|
||||
}
|
||||
}
|
||||
|
||||
// Appointments must land inside bookable hours and off blocked days;
|
||||
// class bookings keep their session-defined window.
|
||||
if booking.ClassSessionID == nil {
|
||||
tenant, err := s.repo.GetTenantByID(ctx, booking.TenantID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := validateAppointmentTarget(ctx, s.repo, tenant, booking.StaffID, booking.LocationID, newStartsAt, newEndsAt); err != nil {
|
||||
return ErrInvalidReschedule
|
||||
}
|
||||
}
|
||||
|
||||
if err := s.repo.RescheduleBooking(ctx, booking.ID, newStartsAt, newEndsAt); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
// Re-arm the reminder for the new time; a booking moved inside the
|
||||
// no-reminder horizon simply gets none.
|
||||
if err := s.repo.DeletePendingReminderJobsForBooking(ctx, booking.ID); err == nil {
|
||||
if scheduledFor, ok := reminderSchedule(newStartsAt); ok {
|
||||
_ = s.repo.CreateReminderJob(ctx, db.ReminderJobParams{
|
||||
TenantID: booking.TenantID,
|
||||
BookingID: booking.ID,
|
||||
Channel: "email",
|
||||
ScheduledFor: scheduledFor,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Send reschedule email (async)
|
||||
go s.sendRescheduleEmail(booking, newStartsAt, newEndsAt)
|
||||
|
||||
@@ -195,6 +220,7 @@ func (s *CustomerService) CancelBooking(ctx context.Context, reference string, t
|
||||
if err := s.repo.UpdateBookingStatus(ctx, booking.ID, "cancelled"); err != nil {
|
||||
return err
|
||||
}
|
||||
_ = s.repo.CancelPendingReminderJobsForBooking(ctx, booking.ID)
|
||||
|
||||
// Send cancellation email (async)
|
||||
go s.sendCancellationEmail(booking)
|
||||
|
||||
@@ -290,7 +290,7 @@ func (s *Service) Create(ctx context.Context, request domain.CreateBookingReques
|
||||
if !startsAt.Add(expectedDuration).Equal(endsAt) {
|
||||
return domain.CreateBookingResponse{}, fmt.Errorf("%w: appointment duration must match service duration", ErrInvalidBooking)
|
||||
}
|
||||
if err := s.validateAppointmentTarget(ctx, tenant, request.StaffID, request.LocationID, startsAt, endsAt); err != nil {
|
||||
if err := validateAppointmentTarget(ctx, s.repo, tenant, request.StaffID, request.LocationID, startsAt, endsAt); err != nil {
|
||||
return domain.CreateBookingResponse{}, err
|
||||
}
|
||||
for _, booking := range existing {
|
||||
@@ -383,9 +383,9 @@ func (s *Service) Create(ctx context.Context, request domain.CreateBookingReques
|
||||
// resources owned by the tenant and lands inside bookable time: the staff and
|
||||
// location must belong to the tenant, the window must fit an availability rule
|
||||
// the booking could claim, and it must not overlap a blocked day.
|
||||
func (s *Service) validateAppointmentTarget(ctx context.Context, tenant db.TenantRecord, staffID, locationID *string, startsAt, endsAt time.Time) error {
|
||||
func validateAppointmentTarget(ctx context.Context, repo db.Repository, tenant db.TenantRecord, staffID, locationID *string, startsAt, endsAt time.Time) error {
|
||||
if staffID != nil {
|
||||
staff, err := s.repo.GetStaffByID(ctx, *staffID)
|
||||
staff, err := repo.GetStaffByID(ctx, *staffID)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return fmt.Errorf("%w: staffId is not available for tenant", ErrInvalidBooking)
|
||||
@@ -397,7 +397,7 @@ func (s *Service) validateAppointmentTarget(ctx context.Context, tenant db.Tenan
|
||||
}
|
||||
}
|
||||
if locationID != nil {
|
||||
location, err := s.repo.GetLocationByID(ctx, *locationID)
|
||||
location, err := repo.GetLocationByID(ctx, *locationID)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return fmt.Errorf("%w: locationId is not available for tenant", ErrInvalidBooking)
|
||||
@@ -409,11 +409,11 @@ func (s *Service) validateAppointmentTarget(ctx context.Context, tenant db.Tenan
|
||||
}
|
||||
}
|
||||
|
||||
rules, err := s.repo.ListAvailabilityRulesByTenant(ctx, tenant.ID)
|
||||
rules, err := repo.ListAvailabilityRulesByTenant(ctx, tenant.ID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
blockedDays, err := s.repo.ListBlockedDaysByTenant(ctx, tenant.ID, startsAt.UTC(), endsAt.UTC())
|
||||
blockedDays, err := repo.ListBlockedDaysByTenant(ctx, tenant.ID, startsAt.UTC(), endsAt.UTC())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -518,3 +518,125 @@ func TestDashboardSummaryComputesKPIsWithTrends(t *testing.T) {
|
||||
t.Fatalf("utilization kpi = %+v", utilization)
|
||||
}
|
||||
}
|
||||
|
||||
func bookAppointmentSlot(t *testing.T, repo db.Repository, service *Service) domain.CreateBookingResponse {
|
||||
t.Helper()
|
||||
// Target a day ~72h out so the booking lands in the -24h reminder horizon.
|
||||
target := time.Now().UTC().AddDate(0, 0, 3).Format("2006-01-02")
|
||||
availability, err := service.Availability(context.Background(), "studio-atelier", AvailabilityFilter{Date: target})
|
||||
if err != nil {
|
||||
t.Fatalf("availability: %v", err)
|
||||
}
|
||||
var slot domain.TimeSlot
|
||||
for _, s := range availability.Slots {
|
||||
if s.Mode == "appointment" {
|
||||
slot = s
|
||||
break
|
||||
}
|
||||
}
|
||||
if slot.StartsAt == "" {
|
||||
t.Fatal("expected appointment slot")
|
||||
}
|
||||
resp, err := service.Create(context.Background(), domain.CreateBookingRequest{
|
||||
TenantSlug: "studio-atelier",
|
||||
BookingMode: "appointment",
|
||||
ServiceID: slot.ServiceID,
|
||||
StaffID: slot.StaffID,
|
||||
LocationID: slot.LocationID,
|
||||
CustomerName: "Reminder Test",
|
||||
CustomerEmail: "[email protected]",
|
||||
StartsAt: slot.StartsAt,
|
||||
EndsAt: slot.EndsAt,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create booking: %v", err)
|
||||
}
|
||||
return resp
|
||||
}
|
||||
|
||||
func pendingJobsFor(t *testing.T, repo db.Repository, bookingID string) []db.ReminderJobRecord {
|
||||
t.Helper()
|
||||
jobs, err := repo.ListDueReminderJobs(context.Background(), time.Now().UTC().AddDate(1, 0, 0), 100)
|
||||
if err != nil {
|
||||
t.Fatalf("list due jobs: %v", err)
|
||||
}
|
||||
var out []db.ReminderJobRecord
|
||||
for _, job := range jobs {
|
||||
if job.BookingID == bookingID {
|
||||
out = append(out, job)
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func TestCancelBookingClearsPendingReminder(t *testing.T) {
|
||||
repo := db.NewMemoryRepository()
|
||||
service := NewService(repo, nil, "http://localhost:3000")
|
||||
customerService := NewCustomerService(repo, nil, "http://localhost:3000")
|
||||
ctx := context.Background()
|
||||
|
||||
resp := bookAppointmentSlot(t, repo, service)
|
||||
if len(pendingJobsFor(t, repo, resp.BookingID)) != 1 {
|
||||
t.Fatal("expected one pending reminder job after booking")
|
||||
}
|
||||
|
||||
manageURL, _ := url.Parse(resp.ManageURL)
|
||||
token := manageURL.Query().Get("token")
|
||||
if err := customerService.CancelBooking(ctx, resp.Reference, token); err != nil {
|
||||
t.Fatalf("cancel: %v", err)
|
||||
}
|
||||
if got := pendingJobsFor(t, repo, resp.BookingID); len(got) != 0 {
|
||||
t.Fatalf("expected no pending jobs after cancel, got %d", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
func TestRescheduleRearmsReminder(t *testing.T) {
|
||||
repo := db.NewMemoryRepository()
|
||||
service := NewService(repo, nil, "http://localhost:3000")
|
||||
customerService := NewCustomerService(repo, nil, "http://localhost:3000")
|
||||
ctx := context.Background()
|
||||
|
||||
resp := bookAppointmentSlot(t, repo, service)
|
||||
booking, _ := repo.GetBookingByID(ctx, resp.BookingID)
|
||||
|
||||
newStartsAt := booking.StartsAt.Add(48 * time.Hour)
|
||||
newEndsAt := booking.EndsAt.Add(48 * time.Hour)
|
||||
manageURL, _ := url.Parse(resp.ManageURL)
|
||||
token := manageURL.Query().Get("token")
|
||||
if err := customerService.RescheduleBooking(ctx, resp.Reference, domain.RescheduleBookingRequest{
|
||||
NewStartsAt: newStartsAt.UTC().Format(time.RFC3339),
|
||||
NewEndsAt: newEndsAt.UTC().Format(time.RFC3339),
|
||||
}, token); err != nil {
|
||||
t.Fatalf("reschedule: %v", err)
|
||||
}
|
||||
|
||||
jobs := pendingJobsFor(t, repo, resp.BookingID)
|
||||
if len(jobs) != 1 {
|
||||
t.Fatalf("expected one pending job after reschedule, got %d", len(jobs))
|
||||
}
|
||||
expected := newStartsAt.Add(-24 * time.Hour)
|
||||
if !jobs[0].ScheduledFor.Equal(expected) {
|
||||
t.Fatalf("expected reminder at %v, got %v", expected, jobs[0].ScheduledFor)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRescheduleRejectsTimeOutsideAvailabilityWindow(t *testing.T) {
|
||||
repo := db.NewMemoryRepository()
|
||||
service := NewService(repo, nil, "http://localhost:3000")
|
||||
customerService := NewCustomerService(repo, nil, "http://localhost:3000")
|
||||
ctx := context.Background()
|
||||
|
||||
resp := bookAppointmentSlot(t, repo, service)
|
||||
manageURL, _ := url.Parse(resp.ManageURL)
|
||||
token := manageURL.Query().Get("token")
|
||||
|
||||
// 03:00 UTC next week is outside every availability rule.
|
||||
night := time.Now().UTC().AddDate(0, 0, 7).Truncate(24 * time.Hour).Add(3 * time.Hour)
|
||||
err := customerService.RescheduleBooking(ctx, resp.Reference, domain.RescheduleBookingRequest{
|
||||
NewStartsAt: night.UTC().Format(time.RFC3339),
|
||||
NewEndsAt: night.Add(time.Hour).UTC().Format(time.RFC3339),
|
||||
}, token)
|
||||
if !errors.Is(err, ErrInvalidReschedule) {
|
||||
t.Fatalf("expected ErrInvalidReschedule, got %v", err)
|
||||
}
|
||||
}
|
||||
@@ -1298,6 +1298,7 @@ func (s *Service) CancelBooking(ctx context.Context, principal domain.Principal,
|
||||
if err := s.repo.UpdateBookingStatus(ctx, booking.ID, "cancelled"); err != nil {
|
||||
return err
|
||||
}
|
||||
_ = s.repo.CancelPendingReminderJobsForBooking(ctx, booking.ID)
|
||||
|
||||
go s.sendBookingEmail(booking, false)
|
||||
|
||||
|
||||
@@ -108,7 +108,7 @@ func (r *PGRepository) ListDueReminderJobs(ctx context.Context, dueBefore time.T
|
||||
rows, err := r.pool.Query(ctx, `
|
||||
SELECT rj.id, rj.tenant_id, t.name, t.locale, t.timezone,
|
||||
rj.booking_id, rj.channel, rj.scheduled_for,
|
||||
b.customer_name, b.customer_email, b.reference, b.starts_at, rj.status
|
||||
b.customer_name, b.customer_email, b.reference, b.starts_at, rj.status, b.status
|
||||
FROM reminder_jobs rj
|
||||
INNER JOIN bookings b ON b.id = rj.booking_id
|
||||
INNER JOIN tenants t ON t.id = rj.tenant_id
|
||||
@@ -138,6 +138,7 @@ func (r *PGRepository) ListDueReminderJobs(ctx context.Context, dueBefore time.T
|
||||
&record.Reference,
|
||||
&record.StartsAt,
|
||||
&record.Status,
|
||||
&record.BookingStatus,
|
||||
); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -191,3 +192,20 @@ func (r *PGRepository) GetDashboardMetrics(ctx context.Context, tenantID string,
|
||||
)
|
||||
return metrics, err
|
||||
}
|
||||
|
||||
func (r *PGRepository) CancelPendingReminderJobsForBooking(ctx context.Context, bookingID string) error {
|
||||
_, err := r.pool.Exec(ctx, `
|
||||
UPDATE reminder_jobs
|
||||
SET status = 'cancelled'
|
||||
WHERE booking_id = $1 AND status = 'pending'
|
||||
`, bookingID)
|
||||
return err
|
||||
}
|
||||
|
||||
func (r *PGRepository) DeletePendingReminderJobsForBooking(ctx context.Context, bookingID string) error {
|
||||
_, err := r.pool.Exec(ctx, `
|
||||
DELETE FROM reminder_jobs
|
||||
WHERE booking_id = $1 AND status = 'pending'
|
||||
`, bookingID)
|
||||
return err
|
||||
}
|
||||
@@ -40,6 +40,8 @@ type Repository interface {
|
||||
CreateReminderJob(ctx context.Context, params ReminderJobParams) error
|
||||
ListDueReminderJobs(ctx context.Context, dueBefore time.Time, limit int) ([]ReminderJobRecord, error)
|
||||
MarkReminderJobDispatched(ctx context.Context, reminderJobID string, status string, dispatchedAt time.Time) error
|
||||
CancelPendingReminderJobsForBooking(ctx context.Context, bookingID string) error
|
||||
DeletePendingReminderJobsForBooking(ctx context.Context, bookingID string) error
|
||||
CreateNotificationDeliveryLog(ctx context.Context, params NotificationDeliveryLogParams) error
|
||||
GetDashboardMetrics(ctx context.Context, tenantID string, startsAt time.Time, endsAt time.Time) (DashboardMetrics, error)
|
||||
GetSubscriptionSnapshot(ctx context.Context, tenantID string) (BillingSnapshotRecord, error)
|
||||
@@ -418,6 +420,7 @@ type ReminderJobRecord struct {
|
||||
Reference string
|
||||
StartsAt time.Time
|
||||
Status string
|
||||
BookingStatus string
|
||||
}
|
||||
|
||||
type NotificationDeliveryLogParams struct {
|
||||
@@ -1413,6 +1416,12 @@ func (r *MemoryRepository) ListDueReminderJobs(_ context.Context, dueBefore time
|
||||
var out []ReminderJobRecord
|
||||
for _, job := range r.reminderJobs {
|
||||
if job.Status == "pending" && !job.ScheduledFor.After(dueBefore) {
|
||||
for _, booking := range r.bookings {
|
||||
if booking.ID == job.BookingID {
|
||||
job.BookingStatus = booking.Status
|
||||
break
|
||||
}
|
||||
}
|
||||
out = append(out, job)
|
||||
}
|
||||
}
|
||||
@@ -1433,6 +1442,27 @@ func (r *MemoryRepository) MarkReminderJobDispatched(_ context.Context, reminder
|
||||
return pgx.ErrNoRows
|
||||
}
|
||||
|
||||
func (r *MemoryRepository) CancelPendingReminderJobsForBooking(_ context.Context, bookingID string) error {
|
||||
for index := range r.reminderJobs {
|
||||
if r.reminderJobs[index].BookingID == bookingID && r.reminderJobs[index].Status == "pending" {
|
||||
r.reminderJobs[index].Status = "cancelled"
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *MemoryRepository) DeletePendingReminderJobsForBooking(_ context.Context, bookingID string) error {
|
||||
kept := r.reminderJobs[:0]
|
||||
for _, job := range r.reminderJobs {
|
||||
if job.BookingID == bookingID && job.Status == "pending" {
|
||||
continue
|
||||
}
|
||||
kept = append(kept, job)
|
||||
}
|
||||
r.reminderJobs = kept
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *MemoryRepository) CreateNotificationDeliveryLog(_ context.Context, params NotificationDeliveryLogParams) error {
|
||||
r.deliveryLogs = append(r.deliveryLogs, params)
|
||||
return nil
|
||||
|
||||
@@ -251,6 +251,7 @@ type DispatchReminderJobsResponse struct {
|
||||
ProcessedCount int `json:"processedCount"`
|
||||
SentCount int `json:"sentCount"`
|
||||
FailedCount int `json:"failedCount"`
|
||||
SkippedCount int `json:"skippedCount"`
|
||||
}
|
||||
|
||||
// ============================================
|
||||
|
||||
@@ -68,6 +68,14 @@ func (s *Service) DispatchDue(ctx context.Context, limit int) (domain.DispatchRe
|
||||
for _, job := range jobs {
|
||||
response.ProcessedCount++
|
||||
|
||||
if job.BookingStatus != "confirmed" {
|
||||
if err := s.repo.MarkReminderJobDispatched(ctx, job.ID, "skipped", s.now()); err != nil {
|
||||
return domain.DispatchReminderJobsResponse{}, err
|
||||
}
|
||||
response.SkippedCount++
|
||||
continue
|
||||
}
|
||||
|
||||
status := "sent"
|
||||
provider := "unknown"
|
||||
externalID := ""
|
||||
|
||||
@@ -65,9 +65,21 @@ func TestDispatchDueProcessesPendingEmailReminders(t *testing.T) {
|
||||
|
||||
func TestDispatchDueFailsUnknownChannel(t *testing.T) {
|
||||
repo := db.NewMemoryRepository()
|
||||
startsAt := time.Now().UTC().Add(26 * time.Hour)
|
||||
created, err := repo.CreateBooking(context.Background(), db.CreateBookingParams{
|
||||
TenantID: "5d6b3551-0a3e-4b86-bdf0-e9df20a47148",
|
||||
BookingMode: "appointment",
|
||||
StartsAt: startsAt,
|
||||
EndsAt: startsAt.Add(time.Hour),
|
||||
Status: "confirmed",
|
||||
Reference: "BK-TEST-PUSH",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create booking: %v", err)
|
||||
}
|
||||
if err := repo.CreateReminderJob(context.Background(), db.ReminderJobParams{
|
||||
TenantID: "5d6b3551-0a3e-4b86-bdf0-e9df20a47148",
|
||||
BookingID: "booking-unknown-channel",
|
||||
BookingID: created.ID,
|
||||
Channel: "push",
|
||||
ScheduledFor: time.Now().UTC().Add(-time.Minute),
|
||||
}); err != nil {
|
||||
@@ -90,3 +102,39 @@ func TestDispatchRequestContractShape(t *testing.T) {
|
||||
t.Fatalf("expected limit 20, got %d", request.Limit)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDispatchDueSkipsNonConfirmedBooking(t *testing.T) {
|
||||
repo := db.NewMemoryRepository()
|
||||
startsAt := time.Now().UTC().Add(26 * time.Hour)
|
||||
created, err := repo.CreateBooking(context.Background(), db.CreateBookingParams{
|
||||
TenantID: "5d6b3551-0a3e-4b86-bdf0-e9df20a47148",
|
||||
BookingMode: "appointment",
|
||||
StartsAt: startsAt,
|
||||
EndsAt: startsAt.Add(time.Hour),
|
||||
Status: "confirmed",
|
||||
Reference: "BK-TEST-SKIP",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("create booking: %v", err)
|
||||
}
|
||||
if err := repo.CreateReminderJob(context.Background(), db.ReminderJobParams{
|
||||
TenantID: "5d6b3551-0a3e-4b86-bdf0-e9df20a47148",
|
||||
BookingID: created.ID,
|
||||
Channel: "email",
|
||||
ScheduledFor: time.Now().UTC().Add(-time.Minute),
|
||||
}); err != nil {
|
||||
t.Fatalf("create reminder job: %v", err)
|
||||
}
|
||||
if err := repo.UpdateBookingStatus(context.Background(), created.ID, "cancelled"); err != nil {
|
||||
t.Fatalf("cancel booking: %v", err)
|
||||
}
|
||||
|
||||
service := NewService(config.Config{Environment: "development"}, repo)
|
||||
response, err := service.DispatchDue(context.Background(), 10)
|
||||
if err != nil {
|
||||
t.Fatalf("dispatch due: %v", err)
|
||||
}
|
||||
if response.ProcessedCount != 1 || response.SkippedCount != 1 || response.SentCount != 0 {
|
||||
t.Fatalf("expected 1 skipped, got %+v", response)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user