Skip to content

feat:Define and enforce cancellation deadlines - #822

Merged
Qoder-Voidd merged 1 commit into
Akazajan:mainfrom
bibi-fay:feat/Define-and-enforce-cancellation-deadlines
Oct 20, 2026
Merged

Qoder-Voidd merged 1 commit into
Akazajan:mainfrom
bibi-fay:feat/Define-and-enforce-cancellation-deadlines

Conversation

@bibi-fay

Copy link
Copy Markdown
Contributor

fix(bookings): enforce cancellation window/fee policy and make confirmation atomic

Summary

This PR makes two booking-lifecycle changes:

  1. Cancellation window and fee policy. Member and staff cancellations now follow one clear, timezone-safe policy. The outcome (fee applied or not, and why) is returned in the API response.
  2. Race-safe confirmation. Confirming a booking now validates and updates its status in a single atomic operation, so two concurrent staff requests can't both confirm a booking, and a cancelled or expired booking can't be confirmed.

Motivation

  • Cancellations: Without a defined cutoff, fees were applied inconsistently, and comparing times without a fixed timezone could shift the cutoff by hours depending on server or user locale. Members and staff need a predictable rule, and clients need the result in the response.
  • Confirmations: Confirm was a read-then-write sequence. Two staff requests arriving together could both read the booking as PENDING and both succeed. A request could also confirm a booking that was cancelled or expired between the read and the write, leaving it in an invalid state.

What changed

1. Cancellation window and fee policy

  • backend/src/bookings/providers/cancel-booking.provider.ts
    • Adds a cancellation cutoff: a booking cancelled at least [N hours] before its start time is free; one cancelled inside the window incurs [fee rule, e.g. a flat fee or X% of the booking price].
    • Boundary rule: a cancellation exactly at the cutoff is treated as [within the free window / inside the fee window]. This is explicit in code and covered by tests.
    • Timezone safety: all comparisons are done in UTC using absolute instants (the booking start stored as timestamptz or an ISO-8601 string with offset). The cutoff is computed as startTime - N hours on that instant, so server locale, member locale, and DST transitions can't shift it. [If the policy is defined in the venue's local time, describe how that is resolved.]
    • Member vs staff: [describe differences, e.g. staff cancellations waive the fee / follow the same policy but record the actor].
    • Already-cancelled or completed bookings are rejected with [409/400].
    • The "now" value is injected (or read from a clock helper) so tests can control time.
  • backend/src/bookings/dto/create-booking.dto.ts
    • [Describe the DTO change, e.g. validates that the start time includes a timezone offset / accepts an optional timezone field / exposes the response fields for the cancellation outcome.]
  • API response now includes the policy result, for example:
  {
    "id": "…",
    "status": "CANCELLED",
    "cancelledAt": "2026-10-19T10:30:00.000Z",
    "cancelledBy": "MEMBER",
    "cancellationFee": 0,
    "feeApplied": false,
    "cancellationCutoff": "2026-10-20T09:00:00.000Z"
  }

[Adjust field names to match the actual response.]

2. Atomic booking confirmation

  • backend/src/bookings/providers/confirm-booking.provider.ts
    • Replaces the read-check-save flow with a single conditional update: set status to CONFIRMED only WHERE id = :id AND status = 'PENDING' (and the booking has not expired). [Or describe the alternative used: pessimistic lock inside a transaction / optimistic version check.]
    • The number of affected rows decides the outcome: 1 means success; 0 means another request won or the booking is no longer confirmable, and the provider returns [409 Conflict] with a message based on the current status (cancelled, expired, or already confirmed).
  • backend/src/bookings/entities/booking.entity.ts
    • [Describe the entity change, e.g. adds a @VersionColumn()for optimistic locking / adds an index onstatus/ addsexpiresAt used by the expiry check.]
    • [Include a migration note if the schema changed.]

3. Tests

  • backend/src/bookings/bookings.service.spec.ts covers the cancellation policy:
    • Immediately before the cutoff (cutoff minus 1 second/ms): [free / fee] as specified.
    • At the cutoff (exact boundary): [expected result].
    • Immediately after the cutoff (cutoff plus 1 second/ms): [expected result].
    • Member vs staff behavior, the fee amount, and the response fields.
    • Timezone cases: the same instant expressed in different UTC offsets, and a booking across a DST change, produce the same result.
  • backend/src/bookings/providers/confirm-booking.provider.spec.ts covers confirmation:
    • Concurrent test: two confirm requests fired in parallel against the same PENDING booking, where exactly one succeeds and the other fails with a conflict, and the final status is CONFIRMED.
    • Confirming a CANCELLED booking is rejected and its status is unchanged.
    • Confirming an EXPIRED booking is rejected and its status is unchanged.
    • Confirming an already CONFIRMED booking is rejected.

Acceptance criteria

Cancellation policy

  • Policy is timezone-safe (UTC-instant comparisons)
  • Policy outcome is reflected in the API response
  • Tested immediately before, at, and after the cutoff

Atomic confirmation

  • Status is validated and updated atomically
  • In a concurrent test, only one transition succeeds

How to test

  1. Run the booking tests: npm run test --workspace backend -- bookings (or [the project's command]).
  2. Confirm that the three cutoff tests (before, at, after) pass and that the response includes the fee fields.
  3. Run the concurrent confirm test several times to check it is not flaky; it needs a real PostgreSQL test database for the atomicity guarantee to be meaningful, not a mocked repository. [Note how the spec sets this up.]
  4. Manual check:
    • Create a booking starting in more than [N] hours and cancel it: feedApplied: false.
    • Create one starting in less than [N] hours and cancel it: the fee is applied.
    • Send two simultaneous confirm requests for one pending booking and verify that one returns success and the other 409.
    • Try to confirm a cancelled booking and verify it is rejected.

Notes for reviewers

  • Boundary decision: "at the cutoff" is the one policy call worth confirming with product, so please check it against the intended behavior.
  • Atomicity comes from the database statement itself, not from an in-process check, so it holds across multiple API instances.
  • [Mention any migration, new config values such as the window length and fee, and whether existing bookings are affected.]
  • [Mention follow-ups, e.g. refunding or charging the fee through the payment provider, notifications, and an expiry job that moves stale PENDING bookings to EXPIRED.]

Checklist

  • Tests added/updated for all three cutoff cases and the concurrent confirm
  • Migration included and tested (if the entity changed)
  • Swagger/API docs updated with the new response fields and 409 case
  • Config for window and fee documented
  • Change stays within the three scoped files per issue

Closes #599
Closes #601
Closes #603
Closes #602

@drips-wave

drips-wave Bot commented Oct 19, 2026

Copy link
Copy Markdown

@bibi-fay Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Qoder-Voidd
Qoder-Voidd merged commit 989bfbb into Akazajan:main Oct 20, 2026
0 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants