feat(branch): branch working hours and rooms on the existing address entity
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>
This commit is contained in:
hamed
@@ -1,36 +1,59 @@
|
||||
# نکات پیادهسازی — تسک ۰۱
|
||||
|
||||
## ۱. چرا شعبه بالای آدرس مینشیند، نه جای آن
|
||||
## ۱. شعبه ساخته نمیشود — پیدا میشود
|
||||
|
||||
سه مصرفکنندهٔ زنده به `doctor_addresses.id` وابستهاند:
|
||||
نسخهٔ اول این فایل استدلال میکرد «چرا شعبه بالای آدرس مینشیند». استدلال درست بود ولی
|
||||
نتیجهاش غلط: اگر آدرس همان مکان فیزیکی است و همهجا هم به همان معنا مصرف میشود، لایهٔ
|
||||
بالایی چیزی جز یک جدول دوم برای همان نام و تلفن نیست. سه مصرفکنندهٔ زندهای که
|
||||
`doctor_addresses.id` را میخوانند —
|
||||
|
||||
1. `WeeklySchedule.setting[day].sessions[].location_id` (JSON)
|
||||
2. `SlotCalculatorService::buildSessionSlots()` که آن را در هر اسلات کپی میکند
|
||||
3. `AppointmentController::bookingLocations()` که به سایت عمومی `location_uuid` میدهد
|
||||
|
||||
عوض کردن این قرارداد در build هیچکدام از سه ریپو خطا نمیدهد — فقط در runtime آدرس گم میشود.
|
||||
پس `branch_id` روی آدرس اضافه میشود و آدرس همانجا میماند.
|
||||
— دلیل اصلیاند که «شعبه» همان آدرس است، نه دلیلِ ساختن لایهٔ دوم.
|
||||
جزئیات کامل: [`_shared/branch-is-doctor-address.md`](../_shared/branch-is-doctor-address.md).
|
||||
|
||||
## ۲. پزشک مستقل هم شعبه دارد
|
||||
|
||||
وسوسه میشود که شعبه را فقط برای `entity_type=clinic` بسازیم. نکن. اگر پزشک مستقل شعبه
|
||||
نداشته باشد، تسک ۰۲ باید دو مسیر کد برای «منبع مال شعبه» و «منبع مال پزشک» داشته باشد و
|
||||
تسک ۰۶ هر دو را جدا حساب کند. مطب شخصی = شعبهای با `entity_type=doctor`.
|
||||
خوشبختانه از قبل درست است: `DoctorAddress::forDoctor()` با `type = 'personal'` وجود دارد و
|
||||
`findForContext()` هم آدرسهای شخصی و هم کلینیکی را برمیگرداند. پس تسک ۰۲ لازم نیست دو
|
||||
مسیر کد برای «منبع مال شعبه» و «منبع مال پزشک» داشته باشد. مطب شخصی = آدرسی با
|
||||
`type='personal'`.
|
||||
|
||||
## ۳. حذف شعبه
|
||||
## ۳. `TenantFilter` روی `doctor_addresses` کار نمیکند
|
||||
|
||||
هرگز `CASCADE` روی حذف شعبه به منابع و نوبتها نده. `DELETE` فقط وقتی مجاز است که:
|
||||
مهمترین تلهٔ این تسک. `doctor_addresses` ستون `entity_type`/`entity_id` ندارد، پس:
|
||||
|
||||
- هیچ `Room` فعالی نداشته باشد، **و**
|
||||
- هیچ `Resource` فعالی (تسک ۰۲) نداشته باشد، **و**
|
||||
- هیچ نوبت آیندهٔ فعالی روی منابعش نباشد (تسک ۰۷)
|
||||
```php
|
||||
// ❌ آدرس محیط دیگر را هم برمیگرداند — filter اینجا تور ایمنی نیست
|
||||
$address = $this->addresses->findOneBy(['uuid' => $uuid]);
|
||||
|
||||
تا آن تسکها نیامدهاند، فقط شرط اول را چک کن ولی سرویس را طوری بنویس که افزودن دو شرط
|
||||
بعدی یک خط باشد (لیست `DeletionGuardInterface` و تزریق آرایهای از گاردها).
|
||||
// ✅ از BranchResolver رد شو
|
||||
$address = $this->branches->resolve($uuid); // 404 اگر محیط جاری نباشد
|
||||
```
|
||||
|
||||
`active=false` مسیر اصلی است، نه `DELETE`.
|
||||
`Room` خودش `TenantOwnedTrait` دارد (uuidش از request میآید) پس روی آن filter کار میکند؛
|
||||
ولی `BranchWorkingHours` فرزند aggregate است و **هیچ** فیلتری ندارد — تنها محافظش این است
|
||||
که فقط از راه `BranchResolver` قابل دسترسی باشد. `RequestReachableChildTenantTest` همین را
|
||||
اجبار میکند: هیچ endpointی نباید uuid فرزند را مستقیم بگیرد.
|
||||
|
||||
## ۴. ساعت کاری — دقیقه، نه رشته
|
||||
## ۴. حذف
|
||||
|
||||
روی حذف آدرس هیچ دست نمیبریم (endpointهایش موجودند). فقط:
|
||||
|
||||
- `branch_working_hours.address_id` و `rooms.address_id` هر دو `ON DELETE CASCADE` — حذف
|
||||
آدرس ساعت و اتاقش را هم میبرد. این درست است: ساعت کاری بدون مکان معنا ندارد.
|
||||
- حذف **اتاق**: در این تسک بیقید (منبعی هنوز وجود ندارد). سرویس را طوری بنویس که افزودن
|
||||
گاردهای تسک ۰۲ (منبع فعال) و ۰۷ (نوبت آینده) یک خط باشد — آرایهٔ تزریقی از
|
||||
`RoomDeletionGuardInterface`، نه زنجیرهٔ `if`.
|
||||
- `active=false` مسیر اصلی است، نه `DELETE`.
|
||||
|
||||
⚠️ CASCADE روی حذف آدرس + وجود نوبت روی اتاقهای آن = دادهٔ گمشده. تا تسک ۰۷ که نوبت به
|
||||
اتاق وصل میشود، این ریسک وجود ندارد؛ آنجا باید گاردِ حذف آدرس اضافه شود. در checklist با
|
||||
مقصد صریح ثبت شده.
|
||||
|
||||
## ۵. ساعت کاری — دقیقه، نه رشته
|
||||
|
||||
```php
|
||||
// ❌ اشتباه: مقایسهٔ رشتهای در تسک ۰۶ میشکند ("9:00" < "10:00" غلط است)
|
||||
@@ -44,52 +67,71 @@ private int $startMinute = 540;
|
||||
- `0 <= start < end <= 1440`
|
||||
- بازههای یک روز نباید همپوشانی داشته باشند (مرتب کن، بعد `prev.end <= next.start`)
|
||||
- `sequence` را خود سرویس بعد از مرتبسازی تخصیص میدهد، نه کلاینت
|
||||
- **اعتبارسنجی کاملِ هر هفت روز قبل از هر `DELETE`** — وگرنه یک بازهٔ نامعتبر در روز ششم،
|
||||
شش روز درست را هم پاک میکند و ۴۲۲ برمیگرداند
|
||||
|
||||
## ۵. تفسیر «شعبه بدون ساعت کاری»
|
||||
## ۶. تفسیر «شعبه بدون ساعت کاری»
|
||||
|
||||
تصمیم صریح: **تعریفنشده، نه همیشهباز.** تسک ۰۳ وقتی برای شعبهای ساعتی پیدا نکرد، به
|
||||
رفتار فعلی برمیگردد (برنامهٔ پزشک تنها مرجع است). این باعث میشود همهٔ دادههای موجود
|
||||
بدون ساعت کاری شعبه دقیقاً مثل امروز کار کنند.
|
||||
تصمیم صریح: **تعریفنشده، نه همیشهباز.** تسک ۰۳ وقتی برای آدرسی ساعتی پیدا نکرد، به
|
||||
رفتار فعلی برمیگردد (برنامهٔ پزشک تنها مرجع است). پس همهٔ دادهٔ موجود — که هیچ ساعت کاری
|
||||
شعبه ندارد — دقیقاً مثل امروز کار میکند. این خطِ دفاعیِ «منطق اسلاتی دست نمیخورد» است.
|
||||
|
||||
این نکته را در `docs/api/branch.md` بنویس، وگرنه اولین کسی که کش را دیباگ میکند فکر میکند
|
||||
باگ است.
|
||||
همینطور `active=false` روی آدرس در این تسک **هیچ اثری بر اسلات ندارد**؛ فقط ذخیره میشود.
|
||||
اعمالش در تسک ۰۳ است. اگر همینجا اعمال شود، `SlotCalculatorService` عوض میشود که در
|
||||
`_shared/red-lines.md` ممنوع است.
|
||||
|
||||
## ۶. edge case ها
|
||||
هر دو نکته در `docs/api/branch.md` نوشته شود، وگرنه اولین کسی که دیباگ میکند فکر میکند باگ است.
|
||||
|
||||
## ۷. edge case ها
|
||||
|
||||
| حالت | رفتار درست |
|
||||
|---|---|
|
||||
| شعبه در محیط A، اتاق ساختهشده با uuid شعبهٔ محیط B | `404` — `TenantOwnershipChecker::belongsTo` قبل از هر کاری |
|
||||
| دو شعبه همنام در یک محیط | مجاز (نام یکتا نیست؛ آدرس فرق دارد) |
|
||||
| `capacity = 0` | `422` — حداقل ۱ |
|
||||
| آدرس محیط A، اتاق ساختهشده با uuid آدرس محیط B | `404` — `BranchResolver` قبل از هر کاری |
|
||||
| دو آدرس همنام در یک محیط | مجاز (نام یکتا نیست) |
|
||||
| `capacity = 0` یا منفی | `422` — حداقل ۱ |
|
||||
| ساعت کاری روز جمعه خالی | معتبر — یعنی شعبه جمعه بسته است |
|
||||
| شعبهای که تنها شعبهٔ محیط است و غیرفعال میشود | مجاز، ولی هشدار در UI: «هیچ شعبهٔ فعالی باقی نمیماند» |
|
||||
| ساعت شبانهروزی | `start=0, end=1440` — نه دو ردیف |
|
||||
| `PUT` با آرایهٔ خالی | همهٔ ساعتها پاک میشوند — شعبه کامل بسته |
|
||||
| ساعت شبانهروزی | `start=0, end=1440` — یک ردیف، نه دو |
|
||||
| آدرسی که تنها آدرس فعال محیط است و غیرفعال میشود | مجاز، ولی هشدار در UI |
|
||||
| `timezone` نامعتبر مثل `"Tehran"` | `422` — با `DateTimeZone::listIdentifiers()` چک کن، نه regex |
|
||||
|
||||
## ۷. تست
|
||||
## ۸. تست
|
||||
|
||||
```
|
||||
tests/Branch/BranchCrudTest.php
|
||||
- ساخت شعبه با نقش مالک کلینیک → 201 و tenant درست
|
||||
- ساخت با نقش منشیِ بدون محیط انتخابشده → 403
|
||||
- دیدن شعبهٔ محیط دیگر → 404 (نه 403)
|
||||
tests/Branch/WorkingHoursTest.php
|
||||
- هفت روز معتبر → 200 و بازخوانی یکسان
|
||||
- هفت روز معتبر → 200 و بازخوانی یکسان (کلیدهای 0..6)
|
||||
- end <= start → 422
|
||||
- دو بازهٔ همپوشان در یک روز → 422
|
||||
- بازهٔ شبانهروزی 0..1440 → 200
|
||||
tests/Branch/BranchDeletionTest.php
|
||||
- حذف شعبهٔ دارای اتاق فعال → 422
|
||||
- حذف شعبهٔ خالی → 204
|
||||
tests/Shared/TenantSchemaCoverageTest.php ← باید سبز بماند
|
||||
- آرایهٔ خالی → 200 و صفر ردیف
|
||||
- بازهٔ نامعتبر در روز ششم → 422 و شش روز قبلی دستنخورده (اتمی بودن)
|
||||
- uuid آدرس محیط دیگر → 404
|
||||
tests/Branch/RoomCrudTest.php
|
||||
- ساخت با نقش مالک کلینیک → 201 و جفت tenant مشتق از آدرس
|
||||
- capacity=0 → 422
|
||||
- آدرس محیط دیگر → 404
|
||||
- ویرایش/حذف اتاق محیط دیگر → 404
|
||||
tests/Branch/BranchAddressFieldsTest.php
|
||||
- آدرس موجود بدون مقدار → active=true و timezone='Asia/Tehran'
|
||||
- timezone نامعتبر → 422
|
||||
tests/Appointment/SlotModeFrozenTest.php ← باید سبز بماند (اسلات دستنخورده)
|
||||
tests/Shared/TenantSchemaCoverageTest.php ← باید سبز بماند
|
||||
tests/Shared/TenantLookupInventoryTest.php ← repository جدید باید ثبت شود
|
||||
```
|
||||
|
||||
اجرا:
|
||||
```bash
|
||||
ddev exec php bin/phpunit tests/Branch
|
||||
ddev exec php bin/phpunit --group=slot-mode-frozen
|
||||
ddev exec php vendor/bin/phpstan analyse src/Branch
|
||||
```
|
||||
|
||||
## ۸. مستندات
|
||||
⚠️ قبل از اجرای تست، ستونها و جدولها را دستی روی `db_test` بساز — رجوع به بخش
|
||||
Migration در [database.md](database.md). `db_test` تاریخچهٔ migration جدا دارد.
|
||||
|
||||
`docs/api/branch.md` بساز (الگو: `docs/api/staff.md`). در `docs/api/README.md` هم اضافه کن.
|
||||
در `docs/architecture/tenancy.md` جدول طبقهبندی را با سه جدول جدید بهروز کن.
|
||||
## ۹. مستندات
|
||||
|
||||
`docs/api/branch.md` بساز (الگو: `docs/api/staff.md`). در `docs/api/README.md` اضافه کن.
|
||||
`docs/api/doctor.md` را برای دو فیلد جدید `DoctorAddress::toArray()` بهروز کن — این فیلدها
|
||||
در پاسخ ۹ endpoint موجود آدرس ظاهر میشوند، پس تغییر قرارداد است.
|
||||
در `docs/architecture/tenancy.md` جدول طبقهبندی را با دو جدول جدید بهروز کن.
|
||||
|
||||
Reference in New Issue
Block a user