Remove unused distribution_mode field from Scenario #61

New issue

Closed

opened 2026-06-27 01:10:37 +02:00 by myrmidex · 1 comment

myrmidex commented

2026-06-27 01:10:37 +02:00

Owner

Context

Scenario.distributionMode (enum App\Enum\DistributionMode = EVEN | PRIORITY) is stored but unused. It has a DB column, a typed property, getter, setter — but nothing reads it:

getDistributionMode() is called only by its own persistence test, never by any service.
The #32 allocation engine (EvenSplitter, AllocateIncome, PreviewProcessor) hardcodes even-splitting and does not branch on the scenario's mode. The test comment at tests/Service/Allocation/AllocateIncomeTest.php:17 confirms it: "preview() no longer accepts a DistributionMode parameter at all."

It was a placeholder for a PRIORITY distribution strategy that was never wired in. Discovered while scoping #58 (scenario create-from-UI). Removing it now — before the SPA is built against the schema — is the cheapest moment, and avoids the create form exposing a meaningless picker.

Scope

Entity: remove the distributionMode property, getter, setter, and the use App\Enum\DistributionMode; import from src/Entity/Scenario.php.
Enum: delete src/Enum/DistributionMode.php (no remaining references after this ticket).
Migration: add a forward migration to DROP COLUMN distribution_mode from the scenario table. Leave the historical Version20260618213556.php (which created the column) untouched. CS-fix the generated migration (php-cs-fixer strips declare(strict_types=1)).
Vestigial test references (these set the mode as incidental fixture data, assert nothing about it — delete the setDistributionMode(...) calls + now-unused imports):
- tests/Service/Allocation/AllocateIncomeTest.php:480
- tests/Service/Allocation/ResultTest.php:82
Field-characterization tests (delete entirely — they exist only to test the removed field):
- tests/Entity/ScenarioPersistenceTest.php::testDistributionModeDefaultsToEvenWhenNotExplicitlySet
- tests/Entity/ScenarioPersistenceTest.php::testItRoundTripsAnExplicitlySetPriorityDistributionMode
- DistributionMode assertions in tests/Enum/RemainingEnumsTest.php (lines 26–27 + import)

Acceptance

No reference to distributionMode / DistributionMode / distribution_mode anywhere in src/, tests/, or config/.
New migration drops the column; full suite green; PHPStan + Pint clean.
POST /api/scenarios create contract unaffected (name required, description optional) — verified by #58's characterization tests.

Notes

Migration follows the #50 empty-table assumption — no data to preserve (dev DB scenarios are disposable; tests build their own fixtures with DAMA rollback).
If a PRIORITY (or other) distribution strategy is ever actually built, reintroduce a field then, wired to a real consumer.

## Context `Scenario.distributionMode` (enum `App\Enum\DistributionMode` = EVEN | PRIORITY) is **stored but unused**. It has a DB column, a typed property, getter, setter — but **nothing reads it**: - `getDistributionMode()` is called only by its own persistence test, never by any service. - The #32 allocation engine (`EvenSplitter`, `AllocateIncome`, `PreviewProcessor`) hardcodes even-splitting and does **not** branch on the scenario's mode. The test comment at `tests/Service/Allocation/AllocateIncomeTest.php:17` confirms it: *"`preview()` no longer accepts a `DistributionMode` parameter at all."* It was a placeholder for a PRIORITY distribution strategy that was never wired in. Discovered while scoping #58 (scenario create-from-UI). Removing it now — before the SPA is built against the schema — is the cheapest moment, and avoids the create form exposing a meaningless picker. ## Scope - **Entity:** remove the `distributionMode` property, getter, setter, and the `use App\Enum\DistributionMode;` import from `src/Entity/Scenario.php`. - **Enum:** delete `src/Enum/DistributionMode.php` (no remaining references after this ticket). - **Migration:** add a forward migration to `DROP COLUMN distribution_mode` from the `scenario` table. Leave the historical `Version20260618213556.php` (which created the column) untouched. CS-fix the generated migration (php-cs-fixer strips `declare(strict_types=1)`). - **Vestigial test references** (these set the mode as incidental fixture data, assert nothing about it — delete the `setDistributionMode(...)` calls + now-unused imports): - `tests/Service/Allocation/AllocateIncomeTest.php:480` - `tests/Service/Allocation/ResultTest.php:82` - **Field-characterization tests** (delete entirely — they exist only to test the removed field): - `tests/Entity/ScenarioPersistenceTest.php::testDistributionModeDefaultsToEvenWhenNotExplicitlySet` - `tests/Entity/ScenarioPersistenceTest.php::testItRoundTripsAnExplicitlySetPriorityDistributionMode` - `DistributionMode` assertions in `tests/Enum/RemainingEnumsTest.php` (lines 26–27 + import) ## Acceptance - No reference to `distributionMode` / `DistributionMode` / `distribution_mode` anywhere in `src/`, `tests/`, or `config/`. - New migration drops the column; full suite green; PHPStan + Pint clean. - `POST /api/scenarios` create contract unaffected (name required, description optional) — verified by #58's characterization tests. ## Notes - Migration follows the #50 empty-table assumption — no data to preserve (dev DB scenarios are disposable; tests build their own fixtures with DAMA rollback). - If a PRIORITY (or other) distribution strategy is ever actually built, reintroduce a field then, wired to a real consumer.

myrmidex added this to the v0.3.0 milestone 2026-06-27 01:10:37 +02:00

myrmidex added the

enhancement

label 2026-06-27 01:10:37 +02:00

myrmidex self-assigned this 2026-06-27 01:10:37 +02:00

myrmidex referenced this issue

2026-06-27 13:48:34 +02:00

Backend: Scenario create-from-UI support (validation + defaults) #58

myrmidex commented

2026-06-27 13:48:35 +02:00

Author

Owner

Resolved in e563816 (on release/v0.3.0).

Removed the dead Scenario.distributionMode field — stored but never read (the allocation engine hardcodes even-splitting; getDistributionMode() was only called by its own tests). Done as one atomic commit (entity + enum + migration + tests must move together to stay bisect-safe — the entity/schema mismatch breaks inserts otherwise):

Removed property/getter/setter/import from src/Entity/Scenario.php; deleted src/Enum/DistributionMode.php.
Migration Version20260627065959 — DROP distribution_mode (down() re-adds it, with an empty-table-assumption comment).
Stripped the 2 field-characterization tests from ScenarioPersistenceTest, 2 enum assertions from RemainingEnumsTest, and vestigial setDistributionMode() fixture calls from AllocateIncomeTest/ResultTest.

Gotcha captured in PLATFORM.md: removing an #[ApiResource] property needs a cache:clear (stale API Platform metadata → NoSuchPropertyException → 500 on GET) and the test DB needs the migration run manually (it persists between runs).

195 tests green, php-cs-fixer + PHPStan level 6 clean. pr-reviewer: Approve, 0 must-fix.

Resolved in `e563816` (on `release/v0.3.0`). Removed the dead `Scenario.distributionMode` field — stored but never read (the allocation engine hardcodes even-splitting; `getDistributionMode()` was only called by its own tests). Done as one atomic commit (entity + enum + migration + tests must move together to stay bisect-safe — the entity/schema mismatch breaks inserts otherwise): - Removed property/getter/setter/import from `src/Entity/Scenario.php`; deleted `src/Enum/DistributionMode.php`. - Migration `Version20260627065959` — `DROP distribution_mode` (down() re-adds it, with an empty-table-assumption comment). - Stripped the 2 field-characterization tests from `ScenarioPersistenceTest`, 2 enum assertions from `RemainingEnumsTest`, and vestigial `setDistributionMode()` fixture calls from `AllocateIncomeTest`/`ResultTest`. Gotcha captured in PLATFORM.md: removing an `#[ApiResource]` property needs a `cache:clear` (stale API Platform metadata → `NoSuchPropertyException` → 500 on GET) and the test DB needs the migration run manually (it persists between runs). 195 tests green, php-cs-fixer + PHPStan level 6 clean. pr-reviewer: Approve, 0 must-fix.

myrmidex closed this issue

2026-06-27 14:13:58 +02:00