Skip to content

Fix availability exception overlap check for existing slots - #3772

Open
SajalDevX wants to merge 5 commits into
ohcnetwork:developfrom
SajalDevX:issues/3771/exception-booking-overlap
Open

SajalDevX wants to merge 5 commits into
ohcnetwork:developfrom
SajalDevX:issues/3771/exception-booking-overlap

Conversation

@SajalDevX

@SajalDevX SajalDevX commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Proposed Changes

  • When an availability exception is created, find the slots it conflicts with by overlap: start_datetime__time__lt=end_time and end_datetime__time__gt=start_time. Before, the query was start_datetime__time between start_time and end_time.

The old query only looked at where a slot starts, so:

  • a booked 09:00–10:00 slot didn't block a 09:30–10:30 exception, and an empty 09:00–10:00 slot wasn't removed, so it stayed bookable during the exception;
  • a booked 10:30–11:30 slot did block a 09:30–10:30 exception, although the two only touch.

The new condition is the same half-open overlap convert_availability_and_exceptions_to_slots already uses when it leaves slots out for an exception. Exception creation and slot generation now agree on what counts as a conflict. Slots are single-day (the slot listing filters start_datetime__date and end_datetime__date to the same day), so comparing times within the date range is enough.

Associated Issue

Merge Checklist

  • Tests added/fixed
    • test_create_exception_rejects_booked_slot_that_starts_before_it: booked 09:00–10:00, exception 09:30–10:30 → 400 (was 200)
    • test_create_exception_allows_booked_slot_starting_at_its_end: booked 10:30–11:30, exception 09:30–10:30 → 200 and the slot is kept (was 400)
    • test_create_exception_removes_unbooked_slot_that_starts_before_it: empty 09:00–10:00 slot is removed (was kept)
    • All three fail on develop and pass with this change. care.emr.tests.test_schedule_api, test_booking_api and test_token_api pass locally.
  • Update docs in /docs
  • Linting Complete (ruff check / ruff format --check on the changed files)
  • Any other necessary step

AI-assisted: I used Claude to help trace the bug and write the tests, then reviewed the change and ran the test suites locally.

@ohcnetwork/care-backend-maintainers @ohcnetwork/care-backend-admins

Summary by CodeRabbit

  • Bug Fixes
    • Availability exceptions now correctly account for scheduled slots that overlap the exception period, including slots that begin before it or extend across midnight.
    • Creating an exception is blocked when an overlapping slot is allocated. Unallocated overlapping slots are removed, while slots beginning exactly when the exception ends remain unaffected. This helps prevent exceptions from conflicting with existing bookings while preserving slots outside the exception period.

Update: a booked slot that ends at midnight (e.g. 23:00–00:00) is stored with an end time of 00:00, so the time-of-day overlap check missed it. The filter now also treats a slot whose end time isn't after its start time as running to the end of the day, and test_create_exception_rejects_booked_slot_ending_at_midnight covers it (it fails without the change). The schedule and booking suites pass (110 tests).

The check only looked at whether a slot starts inside the exception
window. Use the same overlap rule as slot generation: the slot starts
before the exception ends and ends after it starts.
@SajalDevX
SajalDevX requested a review from a team as a code owner September 26, 2026 06:20
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: cea2a965-d0b8-432e-8e3d-da02811cbff0

📥 Commits

Reviewing files that changed from the base of the PR and between adf206f and 2e38731.

📒 Files selected for processing (2)
  • care/emr/api/viewsets/scheduling/availability_exceptions.py
  • care/emr/tests/test_schedule_api.py

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

Availability exception creation now selects slots that overlap the exception window, including slots that start earlier or end at midnight. Allocated matching slots cause a validation error. Unallocated matching slots are marked deleted.

Changes

Availability exception overlap handling

Layer / File(s) Summary
Overlap check and regression tests
care/emr/api/viewsets/scheduling/availability_exceptions.py, care/emr/tests/test_schedule_api.py
The viewset selects slots that start before the exception ends and whose end is after the exception starts or no later than the slot’s start. Tests cover allocated overlaps, midnight-ending slots, a slot starting at the exception end, and removal of an unallocated overlapping slot.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: vigneshhari

Merge Risk: ⚪ Minimal · up to 2e387

The midnight test covers the query behavior it targets, and no actionable overlap defect is established. No merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2e387

The change improves overlap handling without showing a new permission bypass or cross-resource access. A concurrent booking can still race with exception creation, and the effect on any legacy overnight slots is not established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed mutation is bounded to slots selected for the authorized resource and exception start-date range. The evidence does not establish a new cross-resource or cross-facility write path.

Trust Boundaries and Controls

  • observed — The create lifecycle invokes authorization before the changed persistence hook, and the viewset checks the requesting user's schedule-write authority over the resolved resource.

Resilience and Maintainability Implications

  • inferred — Booking locks by resource, whereas exception creation does not acquire that lock. A booking using a slot loaded before its deletion could race with exception creation because the booking operation does not recheck deleted state. This coordination gap predates the PR for slots already selected by the base predicate; its runtime incidence is unverified.

Hardening Proposals

  • proposed — Consider coordinating booking and exception creation under the same resource-level lock or revalidating slot state at booking commit, with a concurrency test for the transition.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR meets the coding requirements in issue #3771. perform_create filters slots with start_datetime__time__lt=instance.end_time and an end-time condition that detects normal overlaps and slots c…
Out of Scope Changes check ✅ Passed The PR changes the availability-exception conflict query and adds focused schedule API tests for issue #3771. The midnight handling and test helper support the same conflict-detection objective. No un…
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files.
Title check ✅ Passed The title clearly describes the main change: correcting availability-exception overlap checks for existing slots.
Description check ✅ Passed The description explains the behavior change, linked issue, overlap cases, added tests, lint status, and local test results. The optional architecture section is correctly omitted, and the unchecked d…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Availability exception booking check misses slots that start before the exception

1 participant