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>
POST /api/v1/appointment resolved the selected services, summed their minutes,
used that to compute slot_end — and then dropped the result. It never called
replaceServiceItems() or setServiceDuration(), so an appointment booked from
the public site kept no record of what it was booked for:
- the patient panel showed neither the service nor the duration
- reports counted the appointment as having no services
- a later reschedule had no duration to preserve
The management path did all of this correctly; only the public path did not.
Found by booking through the real endpoint and looking at the panel, which is
the one thing no test did.
The duration was also computed as a naive sum of duration_minutes, ignoring the
solo/additional split. That made a multi-service booking's length disagree with
the slots appointment-service-slots had just offered the patient — the booking
would occupy a different span than the one shown. Both paths now go through
ServiceBookingCalculator, which is what builds those slots.
For data that only sets duration_minutes, the calculator returns the same total
as the old sum, so existing services are unaffected.
assertServicesMatchContext() is gone: the calculator performs the identical
ownership check with the same error code and message, and the tenant-lookup
inventory is updated to match.
Tests: PublicBookingServicePersistenceTest starts at the endpoint rather than
building an appointment in memory — the gap that let this ship. Verified it
fails (4 of 8) with the fix disabled. Full suite 1433 green, slot-mode-frozen
green, phpstan at its 14-error baseline.
Co-Authored-By: Claude Opus 5 <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>
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>
