Skip to content

fix(provider_availability): clear Buffer holds when a buffer is removed from a rule - #434

Open
canvas-investigator[bot] wants to merge 1 commit into
mainfrom
provider-availability-buffer-hold-removal
Open

fix(provider_availability): clear Buffer holds when a buffer is removed from a rule#434
canvas-investigator[bot] wants to merge 1 commit into
mainfrom
provider-availability-buffer-hold-removal

Conversation

@canvas-investigator

@canvas-investigator canvas-investigator Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

🧵 Cecilia Orta cc @ceciliaorta

Generated by the Investigator

Problem

When a provider's appointment buffer is removed (or zeroed) on an availability rule, the "Buffer" calendar holds the plugin previously placed on that provider's Administrative calendar are not deleted. The admin UI shows the buffer as gone, but the stale holds remain and keep blocking slots that should now be bookable.

Reported from a production instance (Rivia) that is standardizing on no buffers — removing them provider-by-provider and finding orphaned holds left behind each time.

Evidence (production)

Future, non-cancelled title = "Buffer" events on Administrative calendars:

  • 122,532 holds across 111 provider Administrative calendars, some scheduled as far out as 2086.
  • 3,318 belong to providers with no availability rule at all (deactivated / rule deleted) — unambiguously orphaned.
  • Providers whose buffer was fully removed had 0 remaining holds — only because they were cleaned up by hand; the plugin never would have.

Why it happens

Two issues, both present in this reference:

  1. Nothing reconciles buffers on a rule change. appointment_buffer.py only reconciles from APPOINTMENT_CREATED / RESCHEDULED / CANCELED. Editing/removing a buffer on a rule (or deleting the rule) emits no appointment event, so the holds are never revisited.
  2. The delete pass sat after the zero-buffer early-return. In _reconcile_buffers, if pre_buffer == 0 and post_buffer == 0: return [] returned before the "delete existing Buffer events" step — so even when an appointment event later fired for a zeroed-buffer provider, the stranded holds were never removed.

Fix

provider_availability/protocols/appointment_buffer.py

  • Extract a provider-scoped reconcile_buffers_for_provider(provider_id, action).
  • The delete pass now runs first and unconditionally — before the buffer-config / no-rule checks — so removing or zeroing a buffer (or deleting the rule) clears the holds it left behind. Holds are recreated only when a buffer is still configured.
  • The appointment handlers resolve the provider and delegate to the same function.

provider_availability/api/availability_api.py

  • Call reconcile_buffers_for_provider(...) from the rule create/update (POST / PUT /rules) and delete (DELETE /rules/<provider>/<rule>, DELETE /rules/<provider>) handlers, alongside the existing sync_provider_availability(...). Removing a buffer now cleans its holds immediately, at save time.

Tests

  • Extended tests/protocols/test_appointment_buffer.py: deletion when a buffer is zeroed, deletion when the provider has no rule, delete-then-recreate when a buffer is present, and the bounded recreation query.
  • Added tests/api/conftest.py (autouse) stubbing the buffer pass for the rule-CRUD API tests, which assert availability-sync effects only.
  • uv run pytest757 passed.

⚠️ Version divergence — must also be ported to the deployed build

This reference is v0.17.2 (cache-backed). The reporting instance runs a diverged v0.20.2 (custom_data + a day-bounded buffer reconcile) that is not in this repo — a production-hardened build that was never ported back to the reference (same situation as gcal_sync #429).

  • Do not deploy this reference to that customer — it predates the v0.18 custom_data migration and would regress them.
  • The same two-part fix must be applied to the deployed v0.20.2 build: (1) move the delete pass ahead of the pre == 0 and post == 0 early-return in its day-bounded _reconcile_buffers, and (2) call a provider-scoped reconcile from its rule create/update/delete API paths. Then port the v0.20.2 line back to this reference (per the gcal_sync v0.6.1 — idempotent push, multi-attendee fix, fleet ops #429 pattern).

Scope notes

  • The existing backlog of orphaned holds is not touched by code; a one-time bulk cleanup on the instance (dev + prod) is tracked separately on the SUPPORT board.
  • Left the CSV bulk-import commit path untouched (it re-syncs availability but does not currently manage buffers) — flag for a follow-up if buffers are ever set via CSV.
  • No manifest version bump, given the reference/deployed divergence above.

@ceciliaorta

Copy link
Copy Markdown
Contributor

cerberus review

@canvas-cerberus canvas-cerberus Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐶 Cerberus review

0 important · 0 nit · 0 pre-existing

No blocking issues found.


Review cost: $0.63.

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.

1 participant