Four pages on the existing design system: a resources list whose branch/type/skill/
status filters live in the URL and go straight to the server, and three supporting
pages for types, skills and pools. Filtering client-side over a list the server had
already filtered would have been a second source of truth, so the page does neither.
The pool members dialog only offers resources from the pool's own branch and type —
the same rule the server enforces with 422, applied early so the user never reaches
the error. Skill assignment and pool membership are both full replacements, and both
say so in the dialog, because a partial-looking save that silently drops rows is
worse than an explicit one.
Wiring that was missing: deactivating a staff member through
PATCH /api/v1/staff/{uuid}/toggle now closes their resource too. Without it an
inactive operator would still have shown up in availability search. It is an explicit
call rather than a Doctrine lifecycle callback, since callbacks do not fire for
getArrayResult() — which is how every admin list is built — and that asymmetry is
its own bug. The reverse does not hold: closing a resource does not deactivate the
person, who may be purely administrative.
docs/api/resource.md documents all sixteen endpoints with responses captured from
real curl runs against ddev, including the 422 bodies for person-capacity and
non-scalar attributes. staff.md gains a "relationship to resources" section stating
that job_title is not a skill. tenancy.md contrasts these aggregate children —
whose roots do carry a tenant pair — with the branch_working_hours case from task 01,
where the root was global and the classification was wrong.
Also fixed a pre-existing flaky test: NumericFieldNormalizerTest guarded its random
mobile against collision on the never-reset db_test but not its random national code,
so a full-suite run could fail with 422 and close the EntityManager, taking an
unrelated test down with it. Both are now guarded, and the assertion prints the
server's response instead of a bare "422 is not 201".
Verified: phpunit 1119 tests / 3113 assertions green; slot-mode frozen contract green;
phpstan 14 errors before and after, none in touched files; tsc clean; vitest 88 files
/ 617 tests green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The document's first golden rule is "the calendar belongs to the resource, not to
the doctor". Today the only thing that can be occupied is a doctor, and ClinicStaff
is a label on services and appointments with no calendar, capacity or skills. This
adds the layer underneath: anything that can be busy — doctor, operator, assistant,
device, room, bed, chair.
Two corrections to the planned schema:
- `address_id` → doctor_addresses, not `branch_id` → a new branches table. The
branch already exists and is the address (task 01).
- UNIQUE is (doctor_id, address_id), not (doctor_id). A WeeklySchedule is per
(doctor, clinic) but every session inside it carries its own location_id, so one
doctor already works at several addresses within one environment. Keying on the
doctor alone would have made that unrepresentable — and task 03 gives each
resource its own calendar, which is exactly per-location.
Design points worth keeping:
- Resources bridge to Doctor/ClinicStaff/Room rather than absorbing them; those
three have live consumers (appointments.doctor_id, service_item_staff, the public
site) and subclassing would mean migrating all of them at once. At most one bridge
column is non-null, enforced in the entity because MariaDB will not reliably
enforce a multi-column CHECK.
- Capacity is concurrency: a three-bed injection room is one resource with capacity
3, not three resources, so occupancy in task 06 stays a COUNT against a limit
instead of a merge of three calendars. A person resource is refused capacity > 1.
- Skills are a table, not rules. With 50 operators and 200 services, expressing
"who may operate what" as policy would mean 10,000 rules.
- findEligible() uses HAVING COUNT(DISTINCT …) because "skills A and B" means both;
a plain IN would have matched a resource holding only one.
- setup/cleanup minutes occupy the resource without being part of the patient's
appointment, and are per-resource — distinct from the existing per-doctor
WeeklySchedule.meta.buffer_minutes, which stays untouched.
Two real bugs found by running the backfill against real data rather than fixtures:
ResourceLinker::systemType() persisted a type without flushing, so the next lookup
missed it and created a second — the run died on "Duplicate entry 'doctor-1-staff'
for key uniq_rt_tenant_code". It now keeps an identity map for the unit of work.
The command looped over every WeeklySchedule once per environment, which is
quadratic and never finished on real data. Doctors are now a single pass keyed by
the schedule's own environment. It also flushes per environment and accepts
--pair=clinic:12, so one bad row cannot close the EntityManager and abort a
fleet-wide run, and operators can re-run for a single clinic.
Staff are the one case that cannot be derived: nothing records which branch they
work at. Rather than guessing the first one and seating them in the wrong building,
multi-branch environments are skipped and reported.
88 tests, 230 assertions across tests/Resource and tests/Branch. phpstan clean on
src/Resource.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three pages, all on the existing design system: BranchesPage lists the current
environment's booking locations with their working-hours and active-room counts,
and two subpages edit the week and the rooms. The list page deliberately does not
create or rename a branch — clinic and doctor detail pages already do that, and
duplicating it would give one physical place two edit surfaces. Route permission
reuses `appointment_settings` rather than inventing a new one.
Two real bugs fell out of exercising this end to end:
`days` was serialising as a JSON *array*, not an object keyed "0".."6" — keys 0..6
are sequential so json_encode collapses them to a list. The client reads days["0"]
either way, so nothing looked broken, but the response shape was unstable: one
missing day would flip the same field to an object. The controller now casts to
stdClass and WorkingHoursTest::testDaysIsAJsonObjectNotAnArray pins it. Found by
curling the endpoint for the docs, not by any test.
`<input type="time">` caps at 23:59, so it can neither display nor produce the
legal end value 1440. An all-day range would have vanished from the form and been
corrupted by the first save. Ranges now carry an explicit end-of-day flag, with a
round-trip test proving 1440 survives.
docs/api/branch.md documents all eight endpoints with responses captured from real
curl runs against ddev, including the 422 and 404 bodies. doctor.md records that
active/timezone now appear on all nine existing address endpoints (additive), and
tenancy.md gains the two lessons this task taught: an aggregate child whose root is
itself declared global inherits no environment and needs a real pair, and
TenantFilter is not a substitute for an explicit ownership check because hard
isolation only applies to a *chosen* context.
Verified: phpunit 1067 tests / 2974 assertions green; slot-mode frozen contract
green; phpstan 14 errors before and after, none in touched files; tsc clean;
vitest 87 files / 612 tests green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 01 planned a new `branches` table with `doctor_addresses.branch_id` bridging
to it. That plan was wrong: the branch already exists and is called
`DoctorAddress`. It carries name, address, telephone, coordinates, city/province
FKs and an owner (`forDoctor` / `forClinic` + `type`), and the whole system
already consumes it with exactly that meaning — `WeeklySchedule.sessions[].location_id`
points at `doctor_addresses.id`, `appointment-booking-locations` calls each row a
booking location, and nine CRUD endpoints plus four admin pages manage them.
A parallel table would mean two sources of truth for one physical place and a
branch that `location_id` never references.
So no `branches` table and no duplicate branch CRUD. Only the three genuinely
missing pieces:
- `doctor_addresses.active` / `.timezone`, both NOT NULL with a default so
existing rows need no backfill and no current behaviour changes. `active` is
stored only — applying it to slot calculation is task 03, since touching
`SlotCalculatorService` is off limits in this phase.
- `branch_working_hours`, keyed to `doctor_addresses.id`. Minutes from midnight
rather than "09:00" strings so range intersection stays arithmetic. PUT
replaces all seven days; validation of the whole week runs before any DELETE,
so an invalid sixth day cannot wipe the five valid ones and then answer 422.
- `rooms`, with `capacity` as concurrency (a three-bed injection room is one
resource with capacity 3, not three resources) and a deletion-guard iterator
so tasks 02 and 07 can add reasons without editing RoomService.
`BranchWorkingHours` first registered as an aggregate child of `DoctorAddress`;
TenantSchemaCoverageTest rejected it correctly, because that root is itself
declared global. It now carries a real tenant pair instead, derived in the
constructor from the address's `type` — a total mapping, and the address is only
ever listed in its own context, so nothing is hidden wrongly.
RoomController checks ownership explicitly rather than trusting TenantFilter:
hard isolation only applies to a *chosen* context, so a doctor who had not
selected one could PATCH another clinic's room. Caught by
RoomCrudTest::testForeignRoomIsNotFound, which failed with 200 before the fix.
35 tests, 97 assertions. Slot-mode frozen contract still green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds PublicSiteAppointmentContractTest over GET /api/v1/appointments/user — the
endpoint the public site's user panel actually calls. Task 00's note claimed this
prerequisite was met by extending my/appointments, which is the admin panel's
endpoint; appointments/user returns Appointment::toArray(), which the same task
extended separately. The outcome was right, the reasoning in the note was not.
This test pins it so neither can drift silently: breaking these fields produces no
build error in either repo.
Documents why appointment-service-slots cannot be grouped into shifts by the
client, and records task 00b's checklist including the two items deliberately not
done (colour rewrite, reschedule button) with the evidence for each.
Task: docs/new_feture/taskes/task-00b-nobat724-service-mode/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/api/appointment.md gains the service-reschedule endpoint, the service-mode
section under PATCH, exclude_appointment_uuid and clinic_uuid on
appointment-service-slots, and the my/appointments additions. All JSON bodies are
real output captured from the running endpoints, not hand-written.
New docs/architecture/booking-modes.md holds the endpoint/mode matrix, the
duration contract with a worked example (35 + 10 buffer means a 45-minute step,
so 11:00 is not offered even though it looks free), the reserve-entry rules, and
a placeholder for the resource mode task 06 will add.
Also fixes a pre-existing flaky test that blocked a green suite:
NumericFieldNormalizerTest used a fixed national_code against db_test, which is
never reset, so depending on execution order the endpoint rejected it as a
duplicate. The test already looped for a unique mobile but not for the national
code. Out of this task's scope, fixed and declared so the definition of done is
actually green rather than apparently green.
phpstan was measured against the pre-task commit rather than asserted: 14 errors
in 9 files before, the same 14 in the same 9 files now.
Task 00 complete: 1026 tests green across three consecutive runs, 604 frontend
tests green, slot-mode contract frozen and verified.
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fills service_total_minutes/service_buffer_minutes on future service-mode
appointments booked before the columns existed.
The value comes from the appointment itself (slot_end - slot_start), not from
recomputing the services: an existing appointment may have been booked with a
manual duration and recomputing would rewrite the past. Slot-mode, past,
reserve and cancelled appointments are skipped.
Dry-run by default. Idempotency comes from the query filtering on
serviceTotalMinutes IS NULL rather than from a flag, so a second run has nothing
to do.
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Marks two task-text assumptions as unfounded with the evidence (DataTable
migration, i18n file), defers the URL-state row to a task that owns it, and logs
the three pieces of work discovered mid-run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TransferReserveModal built the live appointment from appointment_time/end_time,
which on a reserve entry are both 00:00 because slot_start == slot_end. Moving a
reserve back to the appointment list silently created a zero-length appointment
at midnight. With the new duration validation it would now fail loudly instead.
Converting back now asks for a real time: the service picker in service mode,
two required time inputs in slot mode. The appointment -> reserve direction is
untouched.
GET /my/appointments has its own array-hydration serializer rather than
Appointment::toArray(), so it exposed none of the service fields the panel needs.
Added service_items (separate query, no row multiplication and no N+1),
clinic_uuid and the duration pair. This was also a hidden prerequisite of the
public-site task, whose checklist listed it as "verify first".
The reserve table now lists every service instead of only the first.
Not done, deliberately: the DataTable migration the task asked for. Its stated
reason — inline tokens breaking dark mode — does not hold; this table's th/td
already use CSS variables and dark mode works. Rewriting a working table for no
real gain is unjustified risk.
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
In service mode the page now mounts the existing ServiceSlotPicker and hides the
three free-form time inputs plus the single-service select: a 45-minute service
could previously be shortened to 20 and the next patient would sit on top of it.
Hidden rather than disabled — a disabled field reads as "you must do something
here".
Saving splits in two: the service-aware endpoint takes the time and services
(the client sends no duration), then the usual PATCH carries deposit, insurance,
status and note without slot_start/slot_end/version, since the reschedule already
advanced the optimistic-lock version.
Booking mode is read from the appointment's own schedule via an explicit
clinic_uuid, not from the panel's current environment: a doctor can be slot-based
in their office and service-based in a clinic. That required exposing clinic_uuid
in Appointment::toArray(), which was missing.
appointment-service-slots accepts exclude_appointment_uuid, gated on canManage of
that appointment — an ungated parameter would let anyone fabricate availability.
ServiceSlotPicker gained two optional props; its existing callers pass neither and
are unaffected. Its reset-on-doctor-change effect now skips the first run, which
would otherwise wipe the initial selection.
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
POST /api/v1/appointment/{uuid}/service-reschedule takes only a start time and
derives the length from the appointment's services. PATCH also validates the
duration, but the client must already know the correct slot_end; not needing that
knowledge is what lets the edit form drop its manual time inputs.
The start must be a member of getServiceStartTimes(), not merely free:
isSlotTaken() reports collisions with other appointments, while the offered list
also applies shift bounds, holidays, date overrides, the booking window and the
buffer. Without it a secretary could park an appointment at 3am.
forManagement comes from canManageContext(), not canManage(): a patient moving
their own appointment must still respect the public booking window.
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Also logs three side findings as debt rather than silently passing: an
unreproduced flaky failure in the combined suite, a pre-existing PHPUnit notice,
and db_test having a migration history separate from dev (later tasks will need
the same manual ALTER).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
In service mode PATCH accepted any duration and only updated the single
service_item column while the service_items collection stayed untouched, so an
edit could leave an appointment with old services and a new length. A 45-minute
service could be shortened to 20 and the next patient would sit on top of it.
Services are now resolved before the time block (duration depends on them) and
the stored end must equal start + total minutes. Reserve entries are exempt:
they carry slot_start == slot_end and occupy no interval, but they do store the
computed duration so a later conversion does not lose it.
No convert-reserve endpoint was added: PATCH already converts a reserve to a
timed appointment via rescheduleTo($start, $end, $isReserve), which refreshes
active_slot_key itself. Project rule 8 — a new endpoint needs an existing one to
be insufficient even after extension.
Slot mode is untouched: with booking_mode = slot the duration stays null and not
one of the new branches runs. Covered by an explicit test.
New error codes are ERR_APPOINTMENT_003/004 (the file only had 001/002).
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
findBusyIntervals() and getServiceStartTimes() gain an optional
excludeAppointmentId, mirroring isSlotTaken($doctor, $start, $end, $excludeId)
which already had it. Without it an appointment being rescheduled sees itself as
busy, so its current time never appears among the candidates and "same hour,
different service" is impossible.
The parameter is optional with a null default and only affects the service-mode
path; no existing call site changes behaviour. SlotModeFrozenTest caught the
signature change immediately while both response contracts stayed green, so the
signature fixture was updated once with a written rationale, as its own header
permits.
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Appointment gains:
- replaceServiceItems(): full replacement that unconditionally syncs the
legacy single serviceItem column. addServiceItem() only fills it when null,
which would leave stale service names in the four consumers that read
service_item (admin lists, public site, desktop app).
- currentServiceUuids(): input-order uuids, falling back to the single column
for appointments created before multi-service support.
- service_total_minutes / service_buffer_minutes (both nullable, NULL in slot
mode). slot_end - slot_start carries the number but cannot say whether it
was intentional, and a reserve entry has slot_start == slot_end so its
duration had nowhere to live.
Existing columns untouched: slot_start, slot_end, active_slot_key, is_reserve
verified unchanged via SHOW COLUMNS.
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Allowed duration of a service combination" lived inside
AppointmentController::serviceSlots(). Three upcoming callers need the same
computation (PATCH duration validation, service-aware reschedule, reserve
conversion); copying it would mean four variants with four different edge-case
behaviours.
The extraction is behaviour-preserving: BaseController::error() and
ExceptionSubscriber emit an identical envelope, so returning $this->error() was
replaced by throwing AppException with the same code/message/field.
Tenant ownership now goes through TenantOwnershipChecker::belongsToPair() (the
documented single point) instead of an inline section pair comparison. The repo
property is named itemRepo on purpose: TenantLookupInventoryTest only counts
recognised property names, so any other name would slip past the safety net.
The naive duration sum is kept deliberately — switching to solo/additional
minutes is task 04 and changes one line here.
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds SlotModeFrozenTest (#[Group('slot-mode-frozen')]) locking three things
against the multi-resource booking phase:
- GET /api/v1/appointment-slots response shape
- GET /api/v1/appointment-settings/month-availability/{uuid} response shape
- public method signatures of SlotCalculatorService
Fixtures are structural, not raw snapshots: a fixed past date is rejected by
isWithinBookingWindow so an empty snapshot would prove nothing. Instead a
deterministic schedule on a computed near-future date, with epoch/uuid values
normalized to placeholders. What stays locked is the contract itself: keys,
ordering, types and local times.
No production code touched.
Task: docs/new_feture/taskes/task-00-service-mode-completion/
Slot-mode contract: unchanged (--group=slot-mode-frozen green, 3 tests)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Created new JSON file for database.md with nodes and edges representing the structure and relationships within the document.
- Created new JSON file for architecture.md with nodes and edges detailing the architecture of the task 14 events utilization.
- Created checklist for task 11: Package and Credit Ledger
- Created checklist for task 12: Treatment Course
- Created checklist for task 13: Cancellation Policy, No-Show, and Waitlist
- Created checklist for task 14: Domain Events and Utilization Reports
- Add task for completing service mode in clinicpro with detailed objectives and acceptance criteria.
- Create architecture documentation for task 00b, outlining involved components and necessary changes.
- Develop checklist for task 00b to ensure all requirements are met.
- Document implementation notes for task 00b, emphasizing API contract checks and design system adherence.
- Update task documentation for task 00b, specifying goals and current issues with service mode.
- Add implementation notes for cancellation and waitlist features.
- Create task documentation outlining goals, current status, and acceptance criteria for cancellation policy and resource utilization reporting.
- Establish architecture for domain events and outbox pattern to ensure reliable event publishing.
- Define database schema for domain events and necessary queries for resource utilization and plan accuracy reports.
- Implement detailed implementation notes covering edge cases, testing strategies, and documentation requirements.
- Implemented SidebarStaff component tests to ensure staff users see only their dashboard and services.
- Created StaffMyServicesPage to display assigned services for staff users.
- Added migration to link clinic staff rows to user accounts for ROLE_STAFF access.
- Defined StaffPermissions class for static permissions related to staff role.
- Introduced StaffRouteGuardSubscriber to restrict API access for staff users.
- Developed StaffAccountService for managing staff user accounts and linking them to clinic staff.
- Added comprehensive tests for StaffAccountService to validate user creation, mobile number handling, and account attachment.
- Implemented tests for staff dashboard access to ensure proper permissions and access control.
- Created tests for staff login context to verify correct environment visibility based on user roles.
- Refactor multiple admin pages (BlogsPage, ClinicsPage, DoctorsPage, etc.) to utilize the new useUrlState hook for managing pagination, search, and filter states via URL.
- Ensure that the state persists in the URL, allowing users to return to the same state when navigating back from detail pages.
- Update relevant components to handle state changes appropriately and maintain clean URLs by removing default values.
- Add SlotPicker component for selecting appointment slots based on availability.
- Create tests for useUrlState to validate its functionality and ensure correct behavior when interacting with the URL.
- Update API documentation to reflect changes in appointment creation and slot selection processes.
- Added BackButton component to standardize back navigation across pages.
- Integrated BackButton into various pages, replacing custom back buttons for consistency.
- Updated PageHeader to accept backTo prop for displaying BackButton when navigating from subpages.
- Created useGoBack hook to handle navigation logic, determining whether to go back in history or redirect to a fallback page.
- Added tests for BackButton and its integration with PageHeader to ensure expected behavior.
- Updated CoverageRule and related entities to include franchise_percent instead of franchise_rials.
- Modified Appointment entity to carry supplementary insurance ID alongside base insurance.
- Implemented SessionBillingService to ensure finalized invoices for insured patient sessions.
- Created InvoiceFinalized event to trigger claims creation upon invoice finalization.
- Added BackfillMissingClaimsCommand to generate claims for finalized invoices without existing claims.
- Developed tests to validate the new functionality for supplementary insurance handling in appointments and claims.
- Fix tag filtering to correctly match Persian tags by adjusting JSON encoding in the applyTagFilter method.
- Add new endpoint GET /api/v1/blogs/tags to retrieve distinct tag names and their counts for published posts, respecting city scope.
- Update API documentation to reflect changes in tag filtering and the new tags endpoint.
- Create BlogTagFilterTest to ensure correct functionality of tag filtering and facets, including edge cases for Persian tags and city filtering.
- Changed franchise_rials to franchise_percent in tenant_insurances and tenant_service_coverage tables.
- Reset old rial values to 0/NULL as they are not convertible to percentage.
feat(command): add SeedInsuranceScenarioCommand for seeding insurance data
- Implemented a command to seed supplementary insurance contracts, patients, and claims for a specified doctor.
- Includes functionality for purging existing scenario data and generating new entries with predefined contracts and patient scenarios.
Phase 6 created idx_wallet_user_recorded_entity in the migration but never
declared it on WalletTransaction, so doctrine:schema:validate reported the
production database as out of sync with the mapping.
The index itself is right — WalletTransactionRepository::findByUserForEnvironment
filters on exactly (user_id, recorded_entity_type, recorded_entity_id). Only the
attribute was missing. No migration is needed; the index already exists.
This matters beyond tidiness: while validate is red it cannot be used to detect
real drift.
schema:validate is now green on both mapping and database.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 7 concluded that aggregate children needed no column of their own,
because every repository query anchors to its root. That was true of the
repositories, and it missed the case where the anchor never happens:
$item = $this->serviceItemRepo->findByUuid($data['service_item_uuid']);
A lookup by uuid is itself an unanchored query, and TenantFilter cannot help
when the table has no column to filter on. All three leaks phase 7 found had
exactly this shape, including the one that put another environment's service
price on a patient's invoice.
Measuring which children are actually loaded that way gives eight of the
twenty-five — service_items (15 call sites), patient_sessions (7),
session_payments, patient_notes, patient_calls, patient_messages,
patient_attachments, patient_medical_records. They now carry their own pair
and leave AGGREGATE_CHILDREN; the other seventeen are only ever traversed
from their root and stay as they were.
The pair is derived from the root inside the constructor rather than passed
in, so no creation site can forget it and the value has one source. A root
never changes environment, so the copy is written once and cannot drift.
This is defence at the data layer rather than at the entry point: a forgotten
guard now returns nothing instead of another environment's row. The existing
TenantOwnershipChecker guards stay as the outer layer.
Verified against an imported production database: 8 tables backfilled, zero
rows unmatched, zero rows inconsistent with their root. Dropping the column
again turns the leak test red.
Tests: 911 backend (+5). PHPStan unchanged at 17.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
buildAvailableContexts used ClinicRepository::findByUser(), which is a
findOneBy — so a user who owns two clinics only ever saw the first one.
switchContext validates its input against that same list, so the second
clinic could not be selected at all.
Before tenant isolation this was merely annoying. Since phase 4 it is a
blocker: an environment that cannot be selected is an environment
TenantFilter hides from its own owner. Found by running the suite against
an imported production database, where one account owns two clinics and
its second clinic had become unreachable.
findByUser() stays for the fallbacks that only need "some clinic"; the
context list now uses findAllByUser(). The other 20 findByUser() call sites
are single-clinic fallbacks used when no context is chosen, and keep their
current behaviour — once the owner can switch, UserActiveContext decides.
Removing the fix turns 3 of the 4 new tests red.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ownsRecord guards the patient record, not the rows underneath it, so
GET /api/v1/patient/{uuid}/wallet/transactions — and the recent_transactions
in the balance summary — returned the patient's entire history. Clinic A
could read what the patient paid at clinic B, down to the name of the staff
member who entered it.
The wallet stays the person's: the balance is still the sum of that user's
credits minus debits across every environment. Scoping it would show a
patient part of their own money and would make the running balance_after
meaningless. So this is attribution per row, not ownership per wallet.
The columns are deliberately named recorded_entity_type / recorded_entity_id
rather than entity_type / entity_id. TenantFilter keys on the latter and
would then scope the balance query too — the exact bug this avoids. The
naming is load-bearing, and both the entity and the architecture doc say so.
Rows that cannot be attributed — entered before this split, or outside any
environment such as a representation's commission — stay NULL and remain
visible everywhere; hiding them would make an existing patient's history
look deleted. The migration reports how many there are (0 in dev, all
attributable from payments and session references).
Consequence, documented in both docs/api/patient.md and the wallet tab: the
listed rows no longer sum to the displayed balance.
Removing the fix turns 3 of the 6 new tests red.
Tests: 902 backend (+6), 570 frontend. PHPStan unchanged at 17.
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 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 6 covers the eight entities parked in GlobalTables::DEFERRED. The reason
recorded there — dual ownership needing separate analysis — turned out to be
wrong: payments carry only two types, both with a derivable environment, and a
patient never has a chosen context so the filter is off for them anyway.
Bank accounts and POS devices move from the user to the environment, per the
product decision. Existing rows whose owner has more than one environment stay
NULL rather than being guessed, since nothing in the data says which clinic an
account belongs to.
Phase 7 addresses the blind spot flagged in phase 4: aggregate children are not
covered by the filter, and the coverage test only proves the declared chain
reaches a tenant-owning root, not that queries actually start there. It may well
conclude no work is needed — the phase 5 audit found no rootless query — in
which case the guard plus the report is the deliverable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
doctrine:schema:update kept proposing the same two ALTERs on every run, because
three columns carry a default in MariaDB that the mapping never declared. The
drift was noise, but it hid real diffs: both tenant migrations in this series
picked these ALTERs up as unrelated changes that had to be stripped by hand.
All three fields already default in PHP (0, 0, and STATUS_CONFIRMED), and the
database columns already match, so the fix is to state the default in the
mapping rather than alter live tables. schema:update now reports the schema in
sync and migrations:diff finds nothing.
Tests: 856 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 5, the last of the tenant-marking series. The Doctrine filter added in
phase 4 does not see raw DBAL, so every place that writes SQL by hand was read
and classified rather than assumed safe.
The audit found no code to fix. ClaimRepository was the only tenant-owning table
reached by raw SQL, and all three of its queries already close on
c.entity_type/:entity_id. That protection had no test, so it now has one: the
claims dashboard is the only tenant surface whose isolation depends entirely on
a hand-written WHERE, and nothing would have reported its removal.
Everything else falls outside the question. AdminApiController is cross-tenant
on purpose behind a class-level ROLE_ADMIN. RepresentationActionController only
counts doctors, scoped by representation_id. CategoryImporter interpolates a
table name, but it comes from a hardcoded const map behind isValidBundle() and
ROLE_ADMIN, so it cannot be steered by input. The purge and seed commands are
console-only, dry-run by default, and blocked from prod at the kernel. The
health check is SELECT 1 and the logger writes to a global table. getReference()
appears once in src, on User, which is global.
app:tenant:dump gives one environment's rows as SQL — the practical benefit of
database-per-tenant without its cost. It reads the table list from metadata using
the same test the filter applies, so a table that gains a tenant pair later is
included automatically instead of being silently missed. The --tenant value ends
up inside a --where clause and an argv entry, so it is validated by a closed
regex rather than escaped; seven malformed inputs are covered, including SQL and
shell injection attempts.
Verified by running it against the dev database: a real clinic produced 20 tables
with only that clinic's rows and no doctor-owned row, an unknown id exited
non-zero with a Persian message, "clinic:1 OR 1=1" was refused, and a tenant with
no data still produced a valid file.
Not verified: browser-level checks of the admin panel and the public site. The
OTP login is behind an Altcha proof-of-work, so no interactive token was
obtained. What was checked instead: the admin SPA type-checks clean, the public
doctor and specialty endpoints answer 200 with cross-tenant results, and neither
nobat724_front nor clinic-pro-tauri references owner_type, owner_id, clinic_key
or db_type anywhere. The functional suite already exercises the same HTTP path
with real JWTs and the subscriber active.
Tests: 856 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
