Commit Graph
5 Commits
Author SHA1 Message Date
hamedandClaude Opus 5 1fdfdf9e48 feat(resource): resource calendars, exceptions and national holidays
Section 9 of the design document builds free time by subtracting seven layers.
Four existed and all of them hung off the doctor. This adds the missing ones and
puts them on the resource:

  branch hours ∩ resource shifts − national holidays − resource exceptions

Booked appointments and holds are deliberately NOT subtracted here — those are
tasks 06/07, as is intersecting several resources. The method is called
rawAvailability() so nobody mistakes the output for bookable time. Nothing in this
change calls SlotCalculatorService; the existing slot path stays frozen.

Four types of exception (leave, absence, maintenance, ad-hoc closure) share one
table because all four are "an interval subtracted from a resource's calendar";
splitting them would mean four queries per availability lookup instead of one.
Holiday overrides work in both directions: a clinic that opens on a public holiday,
and a clinic that closes on an ordinary day.

Every empty day carries a reason (national_holiday, no_shift, branch_closed,
outside_branch_hours, exception, …). Without it an empty response is
indistinguishable from a bug and the first person debugging has to read four tables
by hand.

Three real defects found on the way:

JalaliDateService.gregorianToJalali() was wrong — it returned [3006, 7, 3] for
2026-07-30 instead of [1405, 5, 8], roughly 1601 years off. jalaliYear(),
jalaliMonth(), jalaliMonthRange() and jalaliYearRange() all inherit that, so the
representation reports built on them have been filtering by nonsense ranges. The
class's own formatDateTime() was already correct because it used IntlDateFormatter,
so both conversions now go through the same mechanism, and JalaliDateServiceTest
pins Nowruz and the 6/31→7/1 boundary. There were no tests before, which is why
nobody noticed.

TimeInterval added a seconds-based midnight to a minutes-based interval, turning an
eight-hour shift into eight seconds. The conversion is now an explicitly named
minutesToAbsolute() so the unit change cannot happen silently again.

HolidayService.upsertNational() persisted but left flushing to the caller. Every
HTTP request reboots the kernel, so the caller often held a different
EntityManager: persist landed on one, flush on the other, and nothing was written
with no error at all. The write is now self-contained.

119 tests across tests/Resource, tests/Branch and tests/Representation. phpstan
clean on both touched domains.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-30 18:14:23 +03:30
hamedandClaude Opus 5 964c09cc00 feat(resource): resource types, resources, skills and pools
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>
2026-07-30 17:34:36 +03:30
hamedandClaude Opus 5 a95ee9a618 feat(tenant): give a tenant pair to the children reachable by a request uuid
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>
2026-07-28 16:50:51 +03:30
hamedandClaude Opus 5 c9d4348c46 feat(tenant): mark the financial tables with their owning environment
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>
2026-07-28 15:06:28 +03:30
hamedandClaude Opus 5 75d5052f72 feat(tenant): enforce environment isolation in the ORM layer
Phase 4 of the tenant-marking series. Until now isolation depended on every
query remembering its own WHERE clause. With 82 entities and 844 tests, that is
not a guarantee — it is a hope. MariaDB has no row-level security, so the
backstop has to live in Doctrine.

TenantFilter appends (entity_type, entity_id) to every DQL query on a
tenant-owning entity. It ships disabled and TenantFilterSubscriber turns it on
per request.

The filter engages only for a **chosen** environment — an explicit clinic_uuid
on the request, or a stored UserActiveContext. EntityContext now records which
of the two produced it. Locking a user to the role fallback instead would hide
data they are entitled to: a clinic-member doctor who never switched context
lost every appointment belonging to that clinic. Five tests caught exactly that
before the gate was added. Admins and unauthenticated marketplace traffic stay
outside the filter by design.

Two findings from running it rather than reasoning about it:

- Dereferencing a lazy proxy whose target the filter excluded raises
  EntityNotFoundException, which surfaced as 500 on four patient endpoints.
  ExceptionSubscriber now maps it to 404: outside your environment means it does
  not exist for you. It is logged at info level so a genuinely broken FK is still
  visible.
- EntityManager::find() by primary key IS filtered in Doctrine ORM 3, contrary
  to the limitation carried over from older versions. The stronger guarantee is
  pinned by a test so a future regression is noticed, and the documented table
  was corrected.

The filter also caught a real leak: a clinic secretary's appointment list
filtered by doctor id alone, so a doctor's personal-practice booking appeared in
the clinic list. The test had been asserting that behaviour.

GlobalTables classifies all 82 entities into four states — carries a tenant,
deliberately global, aggregate child, or recorded debt — and
TenantSchemaCoverageTest fails on anything unclassified. Aggregate children
declare their root explicitly, because several attach through a scalar FK rather
than a Doctrine association and cannot be inferred from metadata; the test walks
each chain to a tenant-owning root. Financial tables stay in DEFERRED with a
ceiling assertion so the list cannot grow quietly.

Deliberately not built: the prePersist assignment listener from the plan. The
tenant columns are NOT NULL without a default, so a missing assignTenant()
already fails loudly at flush — phase 2 surfaced 123 such failures. A listener
would add silent auto-assignment where the current behaviour is an explicit
crash.

EXPLAIN with the filter's conditions still picks idx_appointments_tenant_slot
and uniq_patient_record.

Tests: 844 passing. PHPStan unchanged at its 17 pre-existing errors, none in
files touched here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-28 12:27:27 +03:30