fix(seed): never let the demo seed cleanup delete the Owner's data - #1083
Conversation
|
Review by Codex (gpt-6-astra) at 61eb69e Verdict: no P1/P2 or qualifying P3 findings. The refusal protects pre-existing Owner data reachable through ordinary operations. I reproduced the mutations, retaining the new regression test:
I traced all four cleanup calls. The nine explicit deletes are unchanged:
There is also a tenth affected table, Payments are not purged and restrict deletion of both their customer and order. Customers have no archive/delete operation. Product prices and mappings, general inventory and expenses are outside the delete/cascade set. Discount details live on orders and lines. Five temporary probes verified:
The customer query uses the tenant filter. The precondition runs after tenant resolution and product compatibility, before writes and outside the cleanup catch. Its message is actionable; the two documentation edits match that behavior. The existing fixture contract/implementation and seeder reach declarations suffice; no new ledger, seam or bypass entry is needed.
Both requested quality skills found no issue in touched lines. Ponytail review: Lean already. Ship. Every test invocation used |
Closes #1081
What and why
On a farm with no flocks, a demo seed that fails late (for example because the Owner deactivated the Small grade) ran its cleanup. That cleanup deleted every customer, order and order line of the farm, including the Owner's own.
The seed now refuses up front, with
Failedand an actionable message, when the farm already has a customer. This is option (a) from the issue. I chose it over (b), which tracks the ids this run inserted, because:if. Option (b) needs id tracking through every Commerce write, and the purge predicates would have to change.Every purged table is covered:
CustomersSalesOrdersCustomerIdis required, with an FK toCustomers(RESTRICT)SalesOrderItemsSalesOrdersBirdMovementsFlocks; existing flock checkDailyEntries,EggLotsFlockIdis required. These have no DB FK toFlocks(858 already records this), but only handlers write them, every handler needs an existing flock, and nothing deletes a flockDailyEntryGradesDailyEntriesEggInventoryMovementsEggLotsThe check runs after #1076's product check, so the product-mismatch tests keep their customer canary unchanged. It reads through the tenant filter, because the tenant is already resolved at that point. That follows
CommerceFixture's convention, so it needs no tenant-bypass registry entry.A retry after a failed run still works: the cleanup removes the demo's customers, so the next run passes the check (
LateFailure_LeavesTheFarmReseedablestays green).Change map
src/Cluckwork.Infrastructure/Persistence/DemoDataSeeder.cssrc/Cluckwork.Application/Features/Sales/ICommerceFixture.cs,src/Cluckwork.Infrastructure/Repositories/CommerceFixture.csAnyCustomerAsync, a tenant-filtered read.tests/Cluckwork.Api.IntegrationTests/DemoSeedCleanupTests.csOwnersSales_SurviveAFailedSeed, Astra's repro with no fault injection.OwnerScopeAsyncis extracted so it can be shared, andCreateOwnerCatalogAsyncalso returns the customer id.docs/decisions/858-platform-composition.mdsrc/AGENTS.mdMutations
All runs use
DemoSeedCleanupTests, undersg docker.OwnersSales_SurviveAFailedSeed: Customers, SalesOrders and SalesOrderItems eachExpected: 1, Actual: 0if (false && …))OwnersSales_SurviveAFailedSeed);Cleanup_LeavesAnotherFarmsDemoRowsgreenAnyCustomerAsyncignores the tenant filter, so it counts customers on every farmCleanup_LeavesAnotherFarmsDemoRows,LateFailure_RemovesEveryFlockRootedAndCommerceRow,FailedDelete_RollsBackEveryEarlierDelete. Only the first fails on every run; the other two depend on test orderTest counts (measured locally, one class at a time)
DemoSeedCleanupTestsDemoSeedTestsSeedCommandTestsDemoSeedNoOwnerTests/DemoSeedAttributionTests/DemoSeedDisabledOwnerTests/DemoSeedOnlyDisabledOwnerTestsAccessSeederActorTests/AccessSeedOwnerFailureTestsFixturePortRegistrationTestsSchemaDocsTests(image pins)~Architecture~TenancyDocsFreshnessTests|~TenantBypassBase counts were measured only for the class this PR changes. I did not run the full suite; CI is the authority.
Not covered
provision-account.DailyEntriesandEggLotsstill have no DB FK toFlocks. The flock check covers them only as long as the application invariant above holds. This PR does not change that.