Commit Graph
3 Commits
Author SHA1 Message Date
hamedandClaude Opus 5 bab7b57a9d refactor: take the three risky rows back to the plan, without the bugs they invited
All three were deviations I had argued for. Reversing them as asked, each in
the shape the plan wanted and with the failure it would otherwise cause closed.

consume now catches the unique-constraint violation, as specified, instead of
relying only on a read-before-insert. The read stays for the ordinary path, but
it never closed the race — only the unique key does. What made the catch
dangerous is that Doctrine closes the EntityManager on a constraint violation
and the rest of the request dies with it, so the catch resets the registry.
Without that, "already consumed" would surface as an unrelated 500. A test
inserts the ledger row from a second connection and then asks the service to
consume: it returns true, the manager is still open, and exactly one session is
taken.

Cancellation is one transaction now: status, capacity release, credit refund,
penalty and the timeline row commit together. An appointment marked cancelled
whose capacity was never released is the worst of both — the patient has no
appointment and nobody can take the slot. Notification stays outside the
commit, because an SMS cannot be rolled back and must not sit inside something
that can. A test with an SMS provider that always throws proves the
cancellation still commits.

The ledger's running balance is computed in the UI from the rows on screen. The
server still sends its own and remains the reference; the point of computing it
here is that the column now reflects the rows the user is actually looking at,
so a truncated list shows up as a mismatch rather than as a number nobody can
check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 16:28:30 +03:30
hamedandClaude Opus 5 b3c331f0cb perf(reports): read every resource's calendar in one batch, and close the owed tests
Writing the query-count test that task 14 owed showed the growth was real: one
resource cost 10 queries, six cost 33 — about five per resource, because the
available-minutes figure walked each resource's calendar on its own.

Holidays, tenant overrides and branch hours are identical for every resource in
a report, so they now load once outside the loop; shifts and exceptions load for
all resources in one query each. The batched path is a new method rather than a
change to rawAvailability, which the booking engine also calls. The test pins
the shape of the growth, not an exact count.

Also landed:

- app:segment:seed-templates with beauty, dental and physio presets. Building
  four segments and their requirements by hand is the first thing a new clinic
  must do and the most tedious; this gives them something to edit instead of an
  empty page. It refuses to touch a service that already has segments unless
  --force, and it will not invent resource types the tenant never defined.
- book-all is all-or-nothing, proven rather than asserted: with a calendar open
  one day a week and a 1-2 day protocol gap, session one finds a slot and
  session two cannot, and every session must come back planned.
- credit_refundable: false takes the credit back with a negative adjustment and
  deletes nothing — the ledger stays append-only.
- the segments editor has frontend tests, including that it sends back what the
  user sees and renders read-only without the permission.

useBranches now returns [] for a non-array payload instead of throwing
"branches.map is not a function" and taking the page down with it.

BookingLocationsScanTest built a Clinic around a Doctor loaded from a different
manager, which Doctrine treats as a new entity; it flushed fine most runs and
failed on cascade in others. It now loads the doctor from the same manager.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 14:22:22 +03:30
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>
2026-07-31 11:11:03 +03:30