6847a473d4028140f7e8347a52ebdb340e43dce5
19
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 hamedandClaude Opus 5
|
6847a473d4 |
feat(treatment): open a treatment case with snapshotted areas and its sessions
Opening a case copies what must not move afterwards — the session count and the list of body areas, each with its category name — because a treatment record is a medical document and editing settings tomorrow must not rewrite what was done yesterday. The areas are the leaf categories under the service's own category: "توتال" contains bikini, leg and hand, and treatment happens on those three, not on the grouping node above them. A category with no children is its own single area, so "لیزر دست" gets one area rather than none. Every session in the course is created up front so that "session 5 of 8" has somewhere to live, but none of them is booked: creating eight real appointments would lock eight months of slots for a patient who may not attend session three. CategoryClosureResolver gains leaves(); the graph walk it already does is what tells a leaf from a grouping node, so this belongs next to descendants() rather than in a second traversal elsewhere. TreatmentCase and TreatmentSession carry no money field, and must not: billing lives on PatientSession, which is created when an appointment is confirmed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
e2e3e6b43b |
feat(treatment): add treatment protocols, the multi-session course of a service
A protocol says a course of a service runs over several sessions, when each
falls due, which doctor supervises it and which staff may perform it. The row
existing IS the "طول درمان" switch, so there is no separate boolean that could
disagree with the step list.
Each step's offset is measured from the previous session rather than from the
start of the course: laser spacing is a clinical requirement — hair regrows
relative to the last treatment — so a late patient shifts the rest of their
course instead of getting the next session early. That also lets one course use
uneven gaps, which a single min/ideal/max triple cannot express: a botox course
is session 1, then +15 days, then monthly.
Steps and staff are cleared and rewritten in two flushes inside a transaction.
A single flush sends inserts before deletes and the replacement row collides
with the unique (protocol, step_number) index — caught by the replace test.
Removes docs/api/course.md and the task-12 folder. They documented src/Course/,
a module deleted in
|
||
|
hamedandClaude Opus 5
|
85985b04a0 |
feat(practice-domain): add practice domains and let a clinic select one
A practice domain is the field a clinic operates in — beauty, dentistry —
and unlike Specialty it is configuration, not a label: treatment workflows
will bind to its code, so the code is immutable once created and only a
platform admin can mint one. A clinic that has not chosen a domain keeps
behaving exactly as it does today.
Assignment reuses PATCH /api/v1/clinic/{uuid} rather than adding a second
endpoint. An unknown domain uuid is rejected instead of silently dropped,
because a lost selection would only surface at the first protocol-driven
booking.
Also corrects ADR-0003: resource occupancy does not in fact guard the panel
booking path, which writes appointments.resource_id and no occupancy row at
all, so the doctor slot key cannot simply be dropped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
hamedandClaude Opus 5
|
4fe0c4f9bf |
refactor(pricing): make the service the only price source
Price lists, annual tariffs and per-branch price overrides each answered
"what does this service cost?" differently, so a single date could carry
several answers and nobody could say which one was right. Price now lives
only on ServiceItem.price_rials, edited from the services page.
- drop PriceList/PriceListItem, their repositories and the seven
/api/v1/price-list(s) endpoints; PricingController keeps only quote and
the appointment price snapshot
- drop Tariff, TariffRepository, TariffService and the two
/service-items/{uuid}/tariffs endpoints; creating or repricing a service
no longer upserts a current-year tariff
- drop price_rials from ServiceBranchOverride; the entity stays for its
duration columns, which DurationCalculator and ServiceSelectionValidator
still read
- InvoiceService reads the item price directly
- PricingEngine collapses to a single source; breakdown.sources always
reports service_item, keeping the response contract intact
- remove the price-lists admin page, its route and settings-menu entry, the
tariff modal and the service detail tariffs tab; useAppointmentInvoice
moves to its own hook file
Migration drops price_lists, price_list_items, service_tariffs and the
override price column.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
hamedandClaude Opus 5
|
6d7c54508c |
Let categories contain other categories, and share them with resources
Two gaps against the spec. Resources could not be categorised at all — only services carried a catalog category — so "this device is for hands and feet" was unsayable. And CatalogCategory::$parent is a tree built for menu ordering: one parent per category. Laser areas overlap, so "hand" belongs under both "whole body" and "upper limb" at once, which a tree cannot express. Containment is therefore a separate directed acyclic graph (catalog_category_includes) sitting beside the display hierarchy, and resources join the existing clinic-wide categories through a many-to-many rather than growing a parallel list of their own. CategoryClosureResolver walks it transitively: whole body includes lower body includes foot, so whole body includes foot without anyone writing that pair down. The walk reads every edge of the environment in one query and traverses in memory — a query per level would tie round-trips to graph depth. The visited set doubles as the cycle guard, so even data that already contains a loop cannot hang the traversal, and assertNoCycle refuses to create one. Selection now rejects picking an area together with a category that contains it: "whole body laser" and "hand laser" in one appointment is a 422 with a Persian message naming both. This replaces hand-written incompatible_with pairs for the area case — defined once on the category instead of per item pair — while that relation stays for incompatibilities that have nothing to do with areas. Nine tests, including the two-parents case a tree could not hold, the cycle refusal, the self-edge, and the empty-graph boundary. TenantSchemaCoverageTest caught the new edge entity as unclassified; it is registered as an aggregate child of the parent category, which is what the constructor already enforces. Suite 1286 green, phpstan at its 14-error baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
826b940c00 |
Add the resource↔service link that decides who offers what, and for how much
Until now a resource was picked by type and skill alone, so two devices of the same type were indistinguishable even when only one of them performed the service — and there was nowhere to say that this doctor takes 30 minutes for a filler while that one takes 45. ResourceServiceOffering is that link: resource ↔ service item, with an optional duration, an optional price and an active flag. Because a "service option" here is itself a ServiceItem inside an ItemGroup, one table covers both levels the spec asks for — a row against the parent item is "resource + service", a row against a member item is "resource + option". A third table would have meant two sources of truth for one concept and a rewrite of every path that already speaks ServiceItem. It is an aggregate child of ClinicResource, like ResourceSkill: no tenant columns of its own, since the resource already carries the pair and a copy is just something that can drift. The constructor refuses a resource and a service from different environments — TenantFilter does not cover that case, as both uuids arrive from the request body and the filter does not apply to aggregate children. null means inherit, not zero: an explicit zero is a duration that does not exist, while null means this resource has nothing to say and the resolver should look one level up. Zero and negative values are rejected outright. Tests cover the pair being stored, the duplicate pair hitting the unique constraint, the cross-environment guard, null-means-inherit, one service across two devices with different numbers, and deactivating without losing them. Suite 1264 green, phpstan at its 14-error baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
hamed
|
c4f1f25c80 |
Refactor booking system: Remove unused policies, packages, and related entities
- Removed package consumption flags and related properties from PriceQuote. - Eliminated unused domain event publishing for policies and waitlist in Schedule. - Cleaned up BookingEngineSeeder by removing package and policy related logic. - Updated SeedScenariosCommand to reflect removal of policies from output. - Dropped policy, package, treatment course, cancellation, waitlist, and domain event tables in migration. - Removed domain event assertions from tests related to resource blocking. |
||
|
hamedandClaude Opus 5
|
fc504f4415 |
feat(course): treatment courses with protocol-driven session planning
Laser is six to eight sessions; the previous design only knew single appointments, which is the exception rather than the rule. - CourseProtocol per service: session count and three distinct spacings — min is the earliest that is clinically allowed, ideal is best, max is where the course starts losing its effect - Starting a course creates every session up front as `planned` and copies the protocol's numbers and per-session params, so changing the protocol tomorrow leaves a running course alone - Suggestions anchor on the last *completed* session, not the course start: when session 2 slips, session 3 moves with it - Slots are ranked by distance from ideal, not by earliest available — day 21 is worse than day 27 when 28 is the target - book-all is all-or-nothing inside one transaction, with a moving anchor and a 90-day horizon; sessions past the horizon stay planned and are reported, not treated as failures - The effective minimum is the stricter of the protocol and the task-09 spacing policy, so a clinic rule never fights the protocol - Cancelling one session returns only that session to planned; abandoning a course does not cancel its appointments, which stays an explicit decision One active course per (patient, service) via active_course_key, the same partial-uniqueness trick as Appointment::activeSlotKey. Admin: CourseProtocolsPage, TreatmentCoursePage and a courses tab on the patient record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
ca9648732d |
feat(package): session packages backed by a credit ledger
"Six laser sessions" is the common case in an aesthetics clinic: the patient pays once and books the sessions later. Credit is a ledger, not a counter. No table has a remaining/used_count column and a schema test enforces that — the balance is always SUM(delta) over append-only rows, so every number a patient sees has a full history behind it. Corrections are new rows, never edits. - purchase / consume / refund / adjustment / expiry, each with a reason, an author and the appointment it belongs to - consume happens in confirm(), never in quote(): if the preview consumed, a page refresh would cost the patient a session - cancelling adds a refund row; the consume row stays - FIFO across a patient's packages — the oldest is closest to expiring - an empty package is not an error, it just does not apply and the patient pays - adjust/expire need a doctor or clinic role, and adjust always needs a reason - app:package:expire writes the closing row so "where did my 3 sessions go?" always has an answer Consume takes a pessimistic lock on the one package row. That is the opposite of task 07's slot buckets, and docs/api/package.md carries the table explaining why, so nobody unifies them later. Idempotency checks for an existing consume row before inserting rather than catching the unique violation: in Doctrine that exception closes the EntityManager and burns the rest of the request. The unique key stays as the last line of defence. Admin: PackagesPage, a packages tab on the patient record, and a ledger page whose running-balance column shows where the final number came from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
584ea4067f |
feat(policy): six-category policy engine wired into the booking flow
Rules become data instead of code: a clinic can say "laser under 18 requires parental consent" without a deploy. Engine - Policy / PolicyVersionLog entities, closed field/operator/effect lists per category (PolicySchema), condition validation at write time - PolicyResolver: priority -> specificity -> age, combining effects by veto / max / sum / union - A missing fact fails its clause instead of silently passing it - Policies are drafts until activated, and are versioned rather than edited Wiring - selection -> ServiceSelectionValidator - eligibility + spacing -> BookingPolicyGuard, at hold time not confirm time - resource + timing -> AppointmentPlanBuilder, including template-less services - pricing -> PricingEngine, alongside (not replacing) the manual discount The condition column is named condition_json: `condition` is a MariaDB keyword and broke every INSERT. Tests: 17 in tests/Policy including NoPolicyRegressionTest, which pins that a clinic with no policies sees byte-identical output to task 08. Docs: docs/api/policy.md (real captured JSON) + docs/architecture/policy-engine.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
34b07421bd |
feat(pricing): date-ranged price lists and immutable appointment invoices
Section 12 and the fifth closing rule: changing a price never changes an already-booked appointment. The pricing chain already existed and worked. Two things were missing. Tariff only carries a year, so a rate change starting in Mehr could not be expressed — PriceList now takes an explicit date range and Tariff remains the layer beneath it. And an appointment stored a single number, so after a price change or a discount nobody could say what those 2,400,000 rials were made of. Price resolution walks four layers per service and takes the first hit: branch override, then the covering price list, then the yearly tariff, then the service's own price. The last one is the guarantee that a date no list covers still returns a price rather than zero or an exception. breakdown.sources reports which layer answered, so a surprising number can be traced instead of guessed at. Two calculation decisions worth stating. Tax is computed on the patient's share, not the gross — a patient does not pay tax on the portion the insurer covers. And a discount larger than the amount floors the total at zero rather than going negative, because a negative balance would mean the clinic owes the patient money, which nothing downstream is built to mean. A branch-specific list deliberately does not count as overlapping a general one; it takes precedence instead. Treating them as a conflict would have made per-branch exceptions impossible to express. Lists have no effect until activated, so drafting next quarter's prices cannot disturb today's. PriceSnapshot has no setters and a unique key on appointment_id: a snapshot that can be edited is not a snapshot, and two invoices for one appointment would be two truths. Corrections are a new row plus voiding the old one. Invoices are written during confirm with the prices of that moment — computing later would let a rate change between booking and invoicing produce a different number, which is exactly what rule five forbids. 12 tests. The one that matters is testBookedAppointmentKeepsItsOriginalInvoiceAfterAPriceChange: book, double the service price, watch quote return the new number while the appointment's invoice returns the old one. Without it rule five is only a claim. 1220 tests / 3551 assertions. phpstan back at its 14-error baseline. Frozen slot contract green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
4395eea56e |
feat(booking): multi-resource holds and confirmation with a database-level guarantee
Section 11 and the third closing rule of the design document: preventing a double booking is the database's job, not the code's. Any "is it free?" check in PHP has a race window between the read and the write — two concurrent requests both see free and both write. MariaDB has no range EXCLUDE constraint, so every occupied interval is broken into fixed five-minute buckets under UNIQUE(resource_id, bucket_at, seat). The code only INSERTs; a rejection from the database *is* the answer. `seat` carries capacity: a three-bed room has seats 0..2, allocation walks upward on each collision, and the fourth concurrent hold finds nowhere to sit. Counting capacity in PHP would have rebuilt the very race this removes. Buckets are written through DBAL rather than the ORM on purpose: a unique violation raised inside flush() closes the EntityManager, and the next seat attempt would then fail with "EntityManager is closed", hiding the real outcome. Occupancy is one row per (segment × resource). The reference test asserts the payoff directly: for a 55-minute appointment of numbing / waiting / laser, the room gets three rows and the operator only two — the operator holds nothing during the wait and stays bookable for someone else. A partial hold never survives. If the second resource has no room, the first is released and the hold itself removed; otherwise a resource stays locked for an appointment that will never exist. Confirming does not re-reserve anything — the seats were taken at hold time and only the label changes. Re-reserving on confirm would reopen the race the hold closed. Cancelling marks rows `released` instead of deleting them, because the history of which resource was busy when is the input to the utilisation reports; the uniqueness buckets *are* deleted, or that interval would stay locked forever. Expired holds are released by the existing scheduler rather than a new one. That exposed a bug in my own change: the flush guard used $count, which now includes released holds, so reset([]) could pass false to save(). It is guarded on $expired. The appointment itself is still built with the existing constructor, so active_slot_key, events and the payment path behave exactly as before — the multi-resource occupancy sits beside them, not instead of them. 12 tests. Two matter most: the second hold on the same resource and interval getting 409, and a test that writes a duplicate bucket row over a *separate connection* and expects the unique-key violation — if that one ever passes silently, the guarantee had moved back into the code. 1208 tests / 3495 assertions. phpstan at its 14-error baseline. Frozen slot contract green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
22c89fbae4 |
feat(plan): multi-segment appointments with per-segment resource requirements
Section 7 of the design document, and the reason the whole resource layer exists. A laser session is not one block: numbing cream (5 min, room + operator), waiting for it to work (30 min, room only), the laser itself (20 min, room + operator + device), aftercare (5 min, room + operator). Under the single-interval model the operator is locked for all 60 minutes while actually working 30 — half the capacity thrown away. AppointmentPlanBuilder turns (service, selected items, branch, patient) into a plan: segments with offsets, durations and resource requirements. It deliberately assigns no absolute time and no specific resource — that is the next task. This only produces the *shape* of the appointment. Segment duration comes from one of two sources. A fixed segment carries its own number; an item-driven one gets its duration from task 04's DurationCalculator, so "the laser itself" grows with two treated areas while "waiting for the cream" does not. One number could not have expressed that. Three contracts worth stating: - A service with no segment templates falls back to a single continuous segment requiring the doctor resource — exactly today's behaviour. Without it every existing service would have become unplannable overnight. - A segment with no requirements is valid: "waiting at home" consumes time but occupies nothing. - same_gender_as_patient with an unknown patient gender is a 422, not a silently dropped requirement. Dropping it quietly would route the patient to a resource the clinic said must not serve them. When no resource qualifies, the error names the role, the skill and the branch — "no female operator with the skill «Alexandrite laser» is available at «Central»" — rather than an empty result the caller has to interpret (section 10). occupancy_offset carries each requirement's setup/cleanup minutes for the availability engine. It is taken as the maximum across candidates, because the builder does not yet know which resource will be picked and under-reserving means the next appointment lands on top of the cleanup. 11 tests covering the document's reference example (offsets 0/5/35/55, total 60), item-driven scaling, the no-template fallback, all three gender-constraint outcomes, merging and both caps. 1186 tests overall. phpstan back at its 14-error baseline; slot-mode frozen contract green. The admin segments page is not built; the checklist records it with a target. The backend and preview endpoint are complete and consumable without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
hamedandClaude Opus 5
|
b1b06c1b36 |
feat(catalog): dual durations, item groups, relations and branch overrides
Section 5 of the design document rejects summing service durations. "Face + bikini" is not 15+12=27 minutes but 15+8=23 — preparation and settling the patient do not happen twice. Seven wasted minutes times twenty appointments a day is an hour of capacity lost daily, and AppointmentController was doing exactly that plain sum. Each item now carries a solo duration and an additional duration. One item counts at its solo duration and the rest at their additional; the anchor is the item with the *largest* solo duration rather than the first one selected. Anchoring on selection order would have let the same basket cost different amounts depending on click order, so a patient could buy a shorter appointment by reordering. Largest-first is also conservative: no combination is ever under-estimated, and under-estimating pushes the next appointment on top of this one. additional_duration_minutes stays NULL by default and the entity reads NULL as "same as solo", so every existing service keeps behaving exactly as before — the 236 appointment-domain tests pass unchanged. The old duration_minutes column is kept and written in step rather than renamed, because other consumers still read it. ServiceBookingCalculator now delegates to DurationCalculator, which is the one-line change task 00 predicted when it deliberately preserved the naive sum. Selection rules are data, not policy: min/max per group is a number, and "bikini does not combine with full body" is a relation. Putting either in a rules engine means several rules per service and nobody able to explain a rejection. Validation returns *all* errors at once rather than the first, since a user with three problems should not make three round trips. Prerequisite cycles are rejected at write time — storing both "A requires B" and "B requires A" would make every selection permanently invalid. Named CatalogCategory, not ServiceCategory: that name is already an insurance enum (outpatient/inpatient) living on ServiceItem itself, so the two would have collided in the same file's imports. Also fixed a defect the tests caught: breakdown() used $overrides[$id]?->… on a key that may not exist, which warns instead of yielding null. 1175 tests / 3289 assertions. phpstan measured at 14 errors both with and without this change (verified by stashing). Slot-mode frozen contract green. The admin UI tab for groups and relations is not built; the checklist records it as outstanding with a target. The backend is complete and POST /service-selection/validate is consumable without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
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> |
||
|
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> |
||
|
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>
|
||
|
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>
|
||
|
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> |