my/clinic-doctors now reports has_schedule per doctor, and the appointments page
builds tabs from it. A doctor with no working days had a tab that could only ever
show an empty timeline.
The flag is resolved with one query for the whole list rather than one per
doctor. Clinic owners now read this authenticated endpoint too instead of the
public clinic doctor-list, which is where the flag lives; admin keeps the public
list and, with no flag present, hides nobody.
Also repairs fallout from making the resource supervisor mandatory: four test
classes build resources through their own helpers and were failing with 422. The
supervisorFor helper moved to ApiTestCase so all domains share one, rather than
copying it per suite. Full backend suite is green again (1306 tests) — the
previous commit only ran tests/Resource and missed this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ad-hoc resource blocking
- "The laser is being serviced this afternoon" is a specific range, not a change
to the resource's working pattern. It stays separate from calendar exceptions
and the modal says which is which — merging them means either an afternoon's
closure lives in the calendar forever, or a change to working hours vanishes
with one click
- Blocking a range that already holds an appointment is refused with 409 rather
than silently taking capacity back; the appointment is still there and someone
has to decide about it first
- Deleting an occupancy that belongs to an appointment is refused too, otherwise
a patient's booking would quietly lose its resource with no record
409 on hold now recovers
Saying "someone just took it" is not enough — the operator would have to search
again by hand. The page drops the stale selection and refetches, so alternatives
are on screen immediately.
Flake, second half
The earlier fix only covered createUser's retry path. Any test that trips a
unique constraint closes the EntityManager, and the next test inherits the same
closed instance from the container. setUp now resets the registry when it finds
a closed manager, so a test's starting state no longer depends on how the
previous one failed.
Three consecutive full runs green: 1340 tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Strategies (task 06 debt, task 12 dependency)
- ResourcePicker orders candidates; it deliberately does not choose. Only the
engine knows which resource actually fits this slot and which was already
taken by another role, and a strategy that picked would have to duplicate
both checks
- Four implementations behind a tagged iterator: first_available (name order,
the previous behaviour and still the default because it is predictable),
least_gap, least_loaded, same_as_previous
- least_gap and least_loaded are deliberate opposites and both are correct;
choosing between them is a business decision, so it lives in settings
- same_as_previous lifts a course's preferred resource to the front and keeps
everyone else behind it. A preference, not a filter: forcing the same
operator would make the patient wait two weeks, which is worse than a
different operator
- Availability accepts course_uuid to supply that preference, closing the
dependency task 12 recorded against task 06
- An unknown strategy falls back at search time but is rejected at save time.
Stale settings must not stop bookings; a user typing a wrong value must not
believe it took effect
Test suite flake
createUser() retries on a mobile-number collision — db_test is never reset and
holds tens of thousands of users, so the random draw does collide. The failed
INSERT closes the EntityManager, and the retry asked the container for it
again, which hands back the *same closed instance*. So the retry threw, and
every later test in that process inherited a dead manager.
That is the intermittent "EntityManager is closed" on an unrelated,
always-different test that made roughly half of full runs red and never
reproduced in a subset. Resetting the registry gives a live manager back.
UserCollisionRetryTest pins it by closing the manager on purpose.
Two consecutive full runs are green: 1334 tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 6 of the tenant series. GlobalTables::DEFERRED is now empty and the
coverage test asserts it stays that way.
payments carries the (entity_type, entity_id) pair and belongs to the
receiving side, never the payer: an appointment payment takes the
appointment's environment, a subscription takes the environment its buyer
owns, and an SMS wallet top-up takes the wallet's. The patient never chose
an environment, so TenantFilter stays off for them and they still see their
own payment.
Three corrections to the analysis the phase was planned on, each backed by
the code or the data rather than the plan:
- A third payment type exists. Payment::TYPE_SMS_WALLET is created in
SmsWalletController and already carries its environment in the metadata;
without assigning it the write would fail at flush.
- clinic_subscriptions has no user_id, and its trial rows carry no payment,
so it cannot drive the subscription backfill. The environment is derived
the way handleSubscriptionActivation derives it — and that method now
reads the pair off the payment instead of re-deriving it, so a payment and
the subscription it buys can no longer land on different environments.
- WalletTransaction is not a child of Payment. payment_id is nullable and
none of the four creation sites set it; the wallet is a person's, with a
running balance per user. It and Settlement, which withdraws from that same
wallet, are global with a recorded reason instead.
bank_accounts and pos_devices move from the registering user to the
environment. Their pair is deliberately nullable: nothing in the existing
data says which of a multi-environment owner's cards belongs where, and
guessing would point real money at the wrong account. Ambiguous rows stay
unassigned and the migration reports how many. The cost is that such a row
is invisible in every environment, so the owner reaches it through a
user-scoped lookup that runs outside the filter, and assigns it with
PATCH .../{uuid}/environment. The admin panel marks those rows and offers
the assignment.
Tests: 896 backend (+11), 570 frontend (+4). PHPStan unchanged at its 17
pre-existing errors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 3 of the tenant-marking series. The same concept was written four ways,
and the Doctrine filter arriving in phase 4 keys on the field name — so the
tables using a different spelling would have been skipped silently, which is
exactly the leak this work exists to prevent.
- discount_rules: owner_type/owner_id renamed to entity_type/entity_id. Pure
rename, no data moves.
- doctor_secretaries: owner_type plus a nullable clinic_id replaced by the
shared pair. The environment now comes from the clinic argument alone, so the
inconsistent combination (owner_type='clinic', clinic_id=NULL) can no longer
be constructed, and the redundant constructor parameter is gone.
- user_active_context: added db_type, so resolving an environment is one lookup
instead of "try clinics, then try doctors". Filled from the type already
present in available_contexts.
- entity_type is VARCHAR(10) in all twenty tenant tables; four of them were 20.
Behaviour change, the only one in this series: the doctor_secretaries unique key
went from (doctor_id, secretary_id, owner_type) to (doctor_id, secretary_id,
entity_type, entity_id). With clinic_id outside the key, one secretary could not
be assigned to the same doctor in two clinics — the second row collided on
owner_type='clinic'. The duplicate check in SecretaryController had the same
blind spot and would have rejected the request before the database saw it; both
are fixed together.
Correcting an assumption from the phase-3 plan: mobile_verification_otp.entity_type
really is a tenant pair. NotificationMobileController validates the target against
['doctor','clinic'] and stores that entity's id, so the column was normalised with
the rest rather than treated as unrelated.
TenantOwnedTrait gained assignTenantPair() for callers that resolved the pair as
scalars and hold no entity — building an EntityContext from scalars would produce
one where isClinic() is true but ->clinic is null, breaking consumers silently.
tests/ApiTestCase::createUser now retries on a duplicate mobile. db_test is never
reset and already holds ~38k users, so the 9-digit random draw collided often
enough to fail unrelated tests a few percent of runs.
Tests: 830 passing. PHPStan reports no new errors on the changed files.
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>
Phase 1 of the tenant-marking series. The "which environment is this user
working in?" decision was reimplemented in six places, each reading
UserActiveContext.db_uuid and then guessing whether the uuid belongs to a
clinic or a doctor. Every copy was a place the roles could silently diverge.
EntityContextResolver already encoded the right precedence (explicit
clinic_uuid > stored active context > role) but only five files used it, and
it did not recognise secretaries at all: canActInClinic accepted admins,
clinic owners and member doctors, so a secretary's active clinic context
always collapsed to unknown. That gap is why SecretaryAccessChecker carried
its own copy of the logic.
- canActInClinic now also accepts an active DoctorSecretary relation, and a
matching canActForDoctor covers the personal-practice branch.
- AppointmentAccessChecker, ClinicDoctorAccessChecker, SecretaryAccessChecker,
PatientRecordScopeResolver, MyAppointmentsController and the secretary
dashboard all resolve through it now.
- PatientRecordScopeResolver keeps only its real responsibility: which
doctors' patients are visible inside the resolved environment.
- The resolver answers "where"; ClinicDoctorPermissionChecker and
SecretaryPermissionChecker still answer "what may you do".
Left deliberately untouched, with the reason recorded at each site:
SubscriptionController, InventoryController and TenantTagController check
ROLE_DOCTOR unconditionally and ignore the active context, so a member doctor
sees personal inventory/tags/subscription even inside a clinic. Switching them
changes what users see, which is a product decision, not a refactor.
AuthController keeps its repository because it writes the active context.
tests/ApiTestCase now seeds the "free" subscription plan. db_test had no such
row, so getEffectivePlan returned null, every hasFeature() was false and 83
tests across Patient, ClinicService, Insurance and Appointment failed with 403.
No schema, route, request, response or error code changed.
Tests: 813 passing (was 730 passing / 83 failing). PHPStan clean on all
changed files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
findByClinicWithFilters left-joined specialties only for filtering, so
toListArray() lazy-loaded them per doctor (N+1). addSelect them and switch the
result fetch to Paginator(fetchJoinCollection: true) so LIMIT still paginates by
doctor.
Test infra: ApiTestCase::countQueries() (via doctrine.debug_data_holder).
Regression: tests/Doctor/ClinicDoctorListNPlusOneTest asserts the query count
does not grow with doctor count (4→10 without the fix).
Also relaxed AppointmentExpiryServiceTest's exact-count assertion (it counts all
stale pendings in the shared db_test, which accumulates) — logged test-isolation
debt as E6.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hamed