Phase 7 was scoped to guard aggregate children, which the Doctrine filter cannot
reach. Measuring first — as the plan required — moved the target: all 22 children
and their 20 repositories were already sound. Every list query anchors on its
root, and ServiceItemRepository even joins service_sections and filters on the
pair by hand. A repository-level guard would have found nothing.
The real exposure was one layer up. Where a uuid arrives from a request body or
query string, the entity it names is loaded by uuid alone, and the filter is no
help: aggregate children have no tenant column, and a panel user who never chose
an environment is not filtered at all. Three leaks, each proven by removing the
fix and watching the new tests go red:
- GET /api/v1/appointment-service-slots accepted service_item_uuids from any
environment. Existence, bookable state and duration leaked through the error
messages and the returned slots. The booking path in the same controller had
guarded this since it was written; the slot path never did.
- POST /api/v1/my/appointment attached service_section_uuid, service_item_uuid,
staff_uuid and the service list without any check, and persisted them onto the
appointment. A write, not just a read.
- PatientService did the same in all three of its loops — pricing, session
create, session update — so another environment's service price entered the
invoice and its SessionService row was stored, staff included.
TenantOwnershipChecker is the single place that answers "does this belong to the
current environment?". It reads getEntityType()/getEntityId(), so ServiceItem now
delegates that pair to its section: an aggregate child exposing the tenant it
inherits. An entity that exposes no pair throws rather than returning false —
silence here builds an always-closed guard, which is its own bug.
TenantLookupInventoryTest keeps a per-file count of these lookups. It earned its
place immediately: the first run found more sites than the manual grep had, and
reviewing them turned up the third PatientService loop. StaffController looked
unguarded until read properly — ownsStaff sits two lines below the null check.
One assertion was wrong before it was right: the create-path test read
`$session['services'] ?? []`, which passes vacuously. It now counts the stored
rows through the repository, and fails without the fix.
Tests: 879 passing. PHPStan unchanged at its 17 pre-existing errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 2 of the tenant-marking series. appointments, weekly_schedules and
date_overrides kept their environment implicit in a nullable clinic_id, so
every query that wanted "this environment's rows" had to rebuild
clinic_id IS NULL ? doctor : clinic itself. The two calendar tables also
depended on a MariaDB-only generated column, clinic_key = IFNULL(clinic_id, 0),
purely to make a unique key work across NULLs.
All three now carry the (entity_type, entity_id) pair that service_sections,
patient_records and clinic_staff already use, via a shared TenantOwnedTrait.
The pair is a deliberate denormalisation of clinic_id/doctor_id: the automatic
tenant filter and the tenant-leading indexes both need a real column, and
neither can be built on an IF() expression.
- Unique keys keep doctor_id alongside the pair. A clinic has several doctors
and each has their own schedule, so (entity_type, entity_id) alone would
reject the second doctor.
- clinic_key is gone from both calendar tables.
- appointments gained tenant-leading indexes; EXPLAIN on the panel's list query
now picks idx_appointments_tenant_slot.
Deliberately unchanged, both with the reason already recorded in the code:
active_slot_key stays keyed on doctor + slot, since adding the environment
would let one doctor be booked in their own practice and a clinic at the same
moment. holidays keeps its nullable clinic_id, where NULL means "every
environment" rather than "personal practice" — a meaning the pair cannot carry.
The migration adds the columns nullable, backfills, aborts if any row is left
without an owner, and only then tightens to NOT NULL. It creates each
replacement unique index before dropping the old one, so the tables are never
left unprotected — MariaDB commits implicitly on DDL, so ordering is the only
safety net. It runs its statements through the connection rather than addSql()
because the guard has to sit between the backfill and the NOT NULL change.
Columns are NOT NULL with no default on purpose: a construction site that
forgets assignTenant() fails at flush instead of silently writing entity_id 0,
which the phase 4 filter would then hide from everyone.
Verified on the dev database: 0 rows without a tenant, 0 personal bookings
mismatched against their doctor, 0 clinic bookings mismatched against their
clinic.
Tests: 819 passing (813 + 6 new in BookingTenantTest). PHPStan clean on every
changed file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Service-booking mode now selects services by section like slot mode:
appointment-booking-services returns service_section per item; ServiceSlotPicker
groups by section (SearchableSelect), accumulates picks across sections into a
removable 'section -> service' chip list.
Secretaries can override a service's duration for a single appointment without
changing the service default: appointment-service-slots accepts durations[uuid]
and both create endpoints accept service_durations; the override drives total
duration and slot_end. Online (patient) booking is unaffected — it never sends
overrides. Backend + frontend tests and docs updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
