diff --git a/AGENTS.md b/AGENTS.md index 511f2f62e..0f8020fba 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,7 +43,13 @@ dotnet test Cluckwork.sln # 688 tests as of 2026-07; integratio - **Auth:** asymmetric JWT + rotating refresh tokens. PEM keys come from config with escaped `\n`; normalize via `PemKey.Normalize` before `ImportFromPem`. Integration tests generate an ephemeral RSA pair at test-process startup (`TestJwtKeys` in `CluckworkWebApplicationFactory.cs`) — no real key material is ever committed to source (`deploy/.env.example` carries PEM armor with a `replace-me` body, not a usable key). - **Credential epoch revocation (#364):** every access and refresh token is bound to the user's `CredentialEpoch`; all password-reset paths bump it so already-issued credentials fail on their next use. Epoch **`0` is permanently retired**: users start at `1`, legacy refresh rows start at `0`, and a missing or malformed `credential_epoch` claim is always a mismatch — never default the parsed claim to the current database epoch or otherwise treat it as an exemption. `CredentialEpochMiddleware` deliberately performs a fresh database read for every authenticated request; the round trip is the fail-closed revocation guarantee, so do not cache it. Keep the middleware after `TenantResolutionMiddleware` (the lookup is tenant-scoped) and before `MustChangePasswordMiddleware`; `/api/v1/auth/logout` is deliberately exempt so a superseded credential can still terminate its own session. Login and step-up snapshot the epoch + security stamp that the password actually verified **before** clearing the failed-access count, then refuse issuance if a concurrency-loss reload reveals different credentials — never let an old password adopt a concurrent reset. Their disabled-account branches still pay exactly one real password-hash cost, but must not mutate failed-access/lockout state; `AccountLockout` applies the same boundary after a concurrency reload, stopping rather than charging a stale failure to a newly disabled or reset credential. The SPA serializes refresh and self-password-change through the same per-tab queue + cross-tab Web Lock because both replace the shared HttpOnly cookie — a late old-epoch refresh must never overwrite the fresh pair that keeps the self-changing device signed in. This epoch is distinct from #283's first-login password-change flag, #265's operator-only break-glass temporary password, and #308's single-use browser step-up grant: the mechanisms have different audiences and lifetimes and none substitutes for another. - **Migrations + base provisioning (#283):** EF migrations auto-apply on startup (`Database:MigrateOnStartup`, default true — but **Production sets it `false`**; the `migrate` command runs DDL out-of-band, see the #263 bullet below). The default account, the four assignable roles (`Roles.Assignable`), the default egg grades, and the default packed-unit conversions are **static reference data baked into the migrations themselves** — deterministic and multi-instance-safe by construction, **no runtime seeder, no `Seed:*` config**. They ship as **hand-written `migrationBuilder.Sql` with `WHERE NOT EXISTS` guards, deliberately NOT EF's `HasData`/`InsertData`** — `HasData` keys on the PRIMARY KEY, which assumes a virgin schema, but every install that ever booted pre-#283 already has these rows under *different* ids (the old `DatabaseSeeder` minted roles/grades/conversions with `Guid.NewGuid()`), so `InsertData` either collides on `PK_Accounts` or silently violates the real natural-key unique indexes. The #245 squash retires *that* particular reason (a virgin database is now the only starting state), but **do not "regenerate" these into `HasData`**: `HasData` rows become part of the **model**, and every one of these tables is user-mutable through the app (Settings renames the account, `PUT /api/v1/egg-grades/{id}` renames a grade, `EggUnitConversion.Update` retunes `EggsPerUnit`), so a later model-diff would emit `UpdateData`/`DeleteData` that silently reverts the farm's own edits. Raw SQL seeds once and then leaves the rows alone forever. The guard's scope differs per batch, and the rule is *whether the user can change the natural key or remove a set member*: the **egg grades** batch is gated on the account's grade catalog being **entirely empty** (whole-set), because grades are user-managed — `PUT /api/v1/egg-grades/{id}` renames one, and a per-name guard cannot see a `Small` the farm renamed to `Pullet`, so it would resurrect an active, saleable duplicate (the old seeder skipped the whole set for exactly this reason); **roles, unit conversions, and the default account** stay **per-key**, because their keys are not user-mutable (no role CRUD exists at all; `EggUnitConversion.Update` cannot change `UnitCode` and there is no create/delete; the account's key is a fixed id), and per-key additionally lets a later-added role or unit code back-fill into an older install. `MigrationSecurityReviewTests` pins the SQL shape (12 INSERT statements, 21 rows, every one `WHERE NOT EXISTS`-guarded, the grades guard blind to `"Name"`) and `BaseReferenceDataMigrationTests` pins the result against a real virgin Postgres. `MigrationSecurityReviewTests` also asserts no migration ever inserts a credential-shaped row (`AspNetUsers`, `PasswordHash`, `SecurityStamp`, …) — the schema itself is never a place a credential can hide. The first admin is a **separate, deliberately NOT migration-baked** concern — see the #283 bullet below. -- **One migration, squashed (#245):** `Persistence/Migrations/` holds exactly **one** migration, `InitialCreate`, generated from the current model on 2026-08-01. It replaced the 34 that had accumulated since 2026-06-27 (12 of them carrying raw SQL written for mid-history data states — backfills that touch 0 rows on an empty database but still execute). Done once, before the first production deploy, precisely because no `__EFMigrationsHistory` anywhere needed baselining; **it cannot be repeated after go-live**. Any database still carrying the old history (a dev DB from `deploy/docker-compose.dev.yml`, an old demo/sim instance) can no longer migrate forward — **drop and recreate**: `docker compose -f deploy/docker-compose.dev.yml down -v && docker compose -f deploy/docker-compose.dev.yml up -d`. Two things in that migration are **hand-carried and un-regenerable** — a plain `dotnet ef migrations add` drops both on the floor, so never regenerate `InitialCreate` without re-adding them: (1) the four **`lower("Name")` expression unique indexes** (`IX_EggGrades_AccountId_FarmId_LowerName`, `IX_ExpenseCategories_NameCi`, `UX_InventoryItems_Account_Farm_LowerName`, `IX_Products_AccountId_LowerName`) — EF cannot model a functional index, and losing them loses four uniqueness constraints; (2) the **static base reference data** raw SQL (see the bullet above). `MigrationSecurityReviewTests` and `BaseReferenceDataMigrationTests` fail if either goes missing. +- **Migrations: `InitialCreate`, then one per change — the #407 cutover.** `Persistence/Migrations/` began as exactly **one** migration, `InitialCreate`, generated from the current model on 2026-08-01. It replaced the 34 that had accumulated since 2026-06-27 (12 of them carrying raw SQL written for mid-history data states — backfills that touch 0 rows on an empty database but still execute). Done once, before the first production deploy, precisely because no `__EFMigrationsHistory` anywhere needed baselining. + + **That squash is over, and so is the rule that came with it.** While the app had never been deployed, a schema change was *hand-folded into `InitialCreate`* — legitimate only because no database had applied that file yet. **PR #407 is the last change permitted to do that.** From the commit that merges it, **every schema change gets its own migration** (`dotnet ef migrations add `); `InitialCreate` is frozen. EF never re-runs an already-applied migration, so a hand-folded column is a column that **silently does not exist** on any database that already booted — and it fails as broken behaviour, not as a migration error. That is not hypothetical: it is exactly what #399's amendment did to the dev database, surfacing as a broken login. `MigrationSecurityReviewTests` enforces the new shape instead of the old `Assert.Single` fence, in **two** parts, and the second is the load-bearing one: (1) `InitialCreate` must stay **first** and keep its recorded id `20260801190854_InitialCreate`; (2) its **operations are frozen** by a pinned count + digest over a structural description of every `UpOperation`. Identity alone is not enough and that gap is the whole regression — editing `InitialCreate.cs` to add a column changes neither the class name nor the `MigrationAttribute`, and the other tests in that file only inspect credential-shaped inserts, the reference-data SQL, and the four expression indexes, so an ordinary `AddColumn`/`CreateTable` edit was invisible to all of them (#407 codex review). Note the count alone would not have caught it either: a column folded into an existing `CreateTable` leaves the operation count unchanged and moves only the digest. The digest walks **every public readable property** of each operation, recursing into nested column definitions, constraints, foreign keys and annotations, ordered by property **name** (never reflection order — `GetProperties()` is unspecified, same hazard as `GetTypes()`). It is emphatically **not** a hand-written list of "the properties that matter": that was the first attempt and it missed a column's `defaultValue` and a `CreateTable`'s nested keys, so changing `AspNetUsers.CredentialEpoch` from `defaultValue: 1` to `0` — a value #364 says is **permanently retired** — left the count and the whole digest unchanged (#407 codex review round 2). A property is now included because it *exists*, not because someone remembered it. Sequence order inside a collection is preserved rather than sorted, since a composite key's column order is itself schema. **Annotations are enumerated explicitly via `GetAnnotations()`**, because EF exposes them through a *method*, not a property — a property walk never reaches them, and the version that merely claimed to cover them let `Npgsql:ValueGenerationStrategy` flip from `IdentityByDefaultColumn` to `IdentityAlwaysColumn` with the test still green (#407 codex review round 3). Sorted by name there, since `IReadOnlyAnnotatable` promises no order. A `System.Type` is a **leaf** (described by `FullName` only): recursing into one reaches `Assembly.Location`, so the digest encoded an absolute path to `System.Private.CoreLib.dll` and differed between a dev box and the CI runner — green locally, red in CI, which is the worst shape for a fence because the tempting "fix" is to re-baseline until CI agrees, after which it guards nothing. `InitialCreate_Description_ContainsNoEnvironmentSpecificData` now asserts the described text names no assembly file and contains neither the runtime nor the test-binary directory. Successive rounds of this fence each shipped a comment asserting coverage the code did not have; **the standard for changing it is a mutation that goes red, not an argument that it should** — and for anything digest-shaped, **running it repeatedly on one machine proves nothing about portability**, which is exactly how the path leak reached CI. + + **Never regenerate `InitialCreate`.** Two things in it are **hand-carried and un-regenerable** — a plain `dotnet ef migrations add` drops both on the floor: (1) the four **`lower("Name")` expression unique indexes** (`IX_EggGrades_AccountId_FarmId_LowerName`, `IX_ExpenseCategories_NameCi`, `UX_InventoryItems_Account_Farm_LowerName`, `IX_Products_AccountId_LowerName`) — EF cannot model a functional index, and losing them loses four uniqueness constraints; (2) the **static base reference data** raw SQL (see the bullet above). Regenerating also mints a **new timestamp**, which reorders it behind the migrations that depend on it and desynchronises `__EFMigrationsHistory` on every deployed database. `MigrationSecurityReviewTests` and `BaseReferenceDataMigrationTests` fail if any of that goes missing. + + A database predating the squash (an old dev DB from `deploy/docker-compose.dev.yml`, an old demo/sim instance) still cannot migrate forward — **drop and recreate**: `docker compose -f deploy/docker-compose.dev.yml down -v && docker compose -f deploy/docker-compose.dev.yml up -d`. Same for any dev database created *before* #407 merged, since the columns it folded in will otherwise be absent. - **Seed command (#280, #284):** demo sample data is NOT boot-seeded — there is no `Seed__Demo` flag anymore. Run it explicitly, against a database that has **already been booted once** so the base seed above exists (default account, `Admin` role, default egg grades — the demo seed needs all three and fails fast with a clear message if they're missing): `ASPNETCORE_ENVIRONMENT=Development dotnet Cluckwork.Api.dll seed --profile demo` (an unset `ASPNETCORE_ENVIRONMENT` defaults to Production, which the guard below blocks — always set it for a local/dev/CI run). The command migrates the schema, runs `DemoDataSeeder`, then exits (Kestrel never starts); it is **authoritative** (seeds regardless of any config) and **fail-loud** (a real exit code: `0` only when data was actually seeded or was already present, non-zero with an stderr message otherwise — never a silent no-op). `DemoDataSeeder` is registered only when `!IsProduction()` (defense-in-depth): running the command against a Production-env process fails with a clear message instead of writing fake data. Intended flow: the serving process for a real farm stays Production; a non-Production process (dev box, CI, or a sim/load-test harness) runs `seed --profile demo` against its own already-base-seeded database. The **`simulation` profile** (#279) sits in the same `switch` with the same authoritative + fail-loud + Production-guarded contract (`SimulationDataSeeder`); it additionally records its date anchor + a completion marker in a durable `SimulationSeedState` row (anchor written before the first fixture mutation; completion stamped only after the manifest succeeds). A clean re-run — even one straddling a UTC-midnight rollover — reuses that anchor and converges to `AlreadySeeded` with an unchanged manifest fingerprint instead of writing a shifted duplicate set; `AlreadySeeded` is reported only when the counts are unchanged, so a `SimulationOptions` change that adds fixtures re-runs as `Seeded`. The durable anchor can't be poisoned by a mid-seed crash or by foreign daily entries a load test writes into the account (a data-derived anchor could); but a polluted account still fails the exact count validation **closed** (`Failed`) rather than certifying a fixture that no longer matches the definition. - **Break-glass recovery (#265):** `recover-admin` is a second one-off CLI verb on the same binary, same run-then-exit shape as `seed` — but deliberately **NOT** environment-gated (it must work against a real Production database): `dotnet Cluckwork.Api.dll recover-admin --email [--account ] [--reason ]`. For a locked-out account (the sole-Owner-lost-password case, no email/SMTP reset path) it, in one transaction, resets to a **freshly generated** temporary password (never one passed on the command line), rotates the security stamp, revokes every refresh token, and writes a conspicuous `User.BreakGlassReset` audit row carrying `--reason`. The temp password is printed to **stdout only** (never the logger/OTLP) and exit `0`; failures go to stderr with exit `1` and change nothing. `AdminRecoveryService` orchestrates; `IdentityProvider.BreakGlassResetAsync` shares the reset/revoke core with `SetUserPasswordAsync`. Full procedure + verification drill: `docs/runbooks/break-glass-account-recovery.md`. - **First-run admin provisioning (#283):** `bootstrap-admin` is a fourth one-off CLI verb, same run-then-exit shape as `seed`/`migrate`, always available in Production (same posture as `recover-admin`): `dotnet Cluckwork.Api.dll bootstrap-admin --email `. It migrates the schema (idempotent, like the other verbs), then — **only if the default account has no Owner yet** — creates one with a **freshly generated** password and `ApplicationUser.MustChangePassword = true`, printed to **stdout only** (never the logger/OTLP, identical rule to `recover-admin`). A re-run against an already-provisioned account is a silent no-op: no second Owner, no password reprinted. `FirstRunAdminService` orchestrates (mirrors `AdminRecoveryService`'s CLI-wrapper-calls-a-service shape); `IIdentityProvider.CreateUserAsync` gained an optional `mustChangePassword` parameter (default `false` — an ordinary Users-page-created user is never gated) that this is the only caller to pass `true`. While the flag is set: the JWT carries a `must_change_password` claim (`JwtTokenService`), `MustChangePasswordMiddleware` refuses **every** endpoint except `auth/change-password` and `auth/logout` with 403 (placed before `UseAuthorization`, so it applies uniformly regardless of an endpoint's `AuthPolicies` tier, and before the idempotency middleware, so a blocked write burns no key), and the SPA's `ProtectedRoute` renders a **"Set your password"** screen (`SetPasswordPage`, reusing `/auth/change-password` — the operator already knows the printed password as their "current" one) instead of the app shell, on every route. Any successful password reset (self-service change, an Owner's `SetUserPasswordAsync`, or break-glass) clears the flag — a single invariant in `IdentityProvider`, not re-derived per path. **Deliberately a separate credential type** from `recover-admin`'s temp password and from #308's browser step-up re-confirmation grant — different audience (pre-auth shell access vs. an authenticated Owner), different lifetime, never conflated; do not treat any of #283/#265/#308 as covering another. diff --git a/README.md b/README.md index a01617f55..bca3ca303 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,38 @@ carries the pre-squash migration history: the 34 migrations were replaced by a single `InitialCreate`, which EF then sees as pending and tries to apply over tables that already exist. Such a database cannot migrate forward — recreate it. +### Changing the database schema + +**Add a migration. Never edit `InitialCreate`.** + +```bash +# Against the local dev Postgres. The connection is fail-closed (#318): there is +# no default, and every target is held to the same TLS floor as a Production +# boot — the loopback opt-out below is what permits plaintext, and only for +# localhost/127.0.0.1/::1. +CLUCKWORK_MIGRATIONS_CONNECTION='Host=localhost;Port=5432;Database=cluckwork;Username=…;Password=…' \ +CLUCKWORK_MIGRATIONS_ALLOW_INSECURE_LOOPBACK=true \ + dotnet ef migrations add \ + -p src/Cluckwork.Infrastructure -s src/Cluckwork.Api +``` + +Before PR #407 the repo held exactly one migration and schema changes were +hand-folded into it. That was only safe while the app had never been deployed — +no database had applied the file yet, so rewriting it was free. **#407 was the +cutover.** Now that a database exists which has already applied `InitialCreate`, +EF will never re-run it, so a hand-folded column is a column that silently does +not exist. It surfaces as broken behaviour (a failing login, a missing field), +not as a migration error — which is precisely what makes it worth a rule. + +`InitialCreate` is also **not regenerable**: it carries four `lower("Name")` +expression indexes EF cannot model and 21 rows of guarded reference-data SQL, +and re-adding it would mint a new timestamp that desynchronises +`__EFMigrationsHistory` everywhere. `MigrationSecurityReviewTests` fails if it +stops being the first migration or loses its recorded id. + +A dev database created **before #407 merged** predates those folded-in columns — +wipe and recreate it as above. + ### Backup & restore (self-hosted) Two complementary layers (spec §17.5): diff --git a/docs/superpowers/plans/2026-08-02-daily-entry-reconciliation-and-quality-eggs.md b/docs/superpowers/plans/2026-08-02-daily-entry-reconciliation-and-quality-eggs.md new file mode 100644 index 000000000..bc15bc410 --- /dev/null +++ b/docs/superpowers/plans/2026-08-02-daily-entry-reconciliation-and-quality-eggs.md @@ -0,0 +1,88 @@ +# Daily Entry Reconciliation and Saleable Quality Eggs Implementation Plan + +Tracks #394 and #396. Implementation must follow the non-CI writer rule in +PR #395. + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Require official Daily Entry reconciliation while allowing independently saleable Cracked and Dirty stock with farm-defined product prices. + +**Architecture:** Store a stable Daily Entry kind on egg grades and snapshot the resolved Cracked/Dirty grade ids on the official Daily Entry. Manual grades always reconcile against the non-condition remainder; submission and adjustment add or reconcile all snapshot-backed lots atomically. Product prices remain product-owned and are never derived from another grade. + +**Tech Stack:** .NET 10/C#, EF Core/Postgres migrations, React 19/Vite/TypeScript, FluentValidation, Vitest, xUnit/Testcontainers, Playwright simulation harness. + +## Global Constraints + +- Preserve `DailyEntry.Version++` on every mutation and add parallel-race coverage for new aggregate mutation paths. +- Drafts accept incomplete manual grade lines; submit and adjustment require exact reconciliation. +- `Discarded` stays non-saleable; Cracked and Dirty are saleable only through their snapshotted configured grade. +- **Add no second migration.** `Persistence/Migrations/` holds exactly one, `InitialCreate` (#245), and this application has never been deployed — a virgin database is the only starting state anywhere. Hand-carry the columns, the partial unique index, and the seed changes into `InitialCreate` itself, and hand-edit rather than regenerate: it carries four un-regenerable `lower("Name")` expression indexes and 21 rows of `WHERE NOT EXISTS`-guarded reference data that `dotnet ef migrations add` would silently drop. There is correspondingly **no existing farm whose saleability could be modified**, so the "leave existing farms untouched" half of this constraint describes data that cannot exist — dropping it avoids shipping the dead-backfill pattern the #245 squash was performed to remove. +- Update every non-CI writer and fixture per PR #395, including simulation manifest verification. +- Update `HelpPage.tsx` and `specs/product/GLOSSARY.md` with the behavior change. + +--- + +### Task 1: Persist quality-condition identity and official-entry snapshots + +**Files:** +- Modify: `src/Cluckwork.Domain/Eggs/EggGrade.cs`, `src/Cluckwork.Domain/Eggs/DailyEntry.cs` +- Modify: EF configurations and the hand-maintained `InitialCreate` (hand-edited, never regenerated) — **no second migration**, see Global Constraints +- Modify: base-reference seed SQL and migration security/base-data tests +- Modify: `src/Cluckwork.Infrastructure/Repositories/ExportQueries.cs` — add the two quality snapshot ids to the `daily-entries` projection and `DailyEntryKind` to the `egg-grades` projection, so a full account export can still reconstruct which grade a condition counter represented +- Test: Egg-grade and DailyEntry domain tests; migration tests; `ExportTests.cs` regression coverage for both changed projections + +- [ ] Write failing tests for immutable `DailyEntryKind`, one Cracked/Dirty grade per farm, null/non-null quality snapshots on official entries, and an account export missing both additions. +- [ ] Run the focused tests and verify the model cannot yet distinguish a renamed Cracked/Dirty grade from any other quality grade, and that a full account export cannot yet reconstruct which grade a condition counter represented. +- [ ] Add the enum, partial uniqueness constraint, and official-entry snapshots, carried into `InitialCreate` by hand — columns, index, and the seed defaults together. Confirm afterwards that the four `lower("Name")` expression indexes and all 21 guarded reference-data rows survive, and that `MigrationSecurityReviewTests` and `BaseReferenceDataMigrationTests` still pass; treat a failure there as having destroyed something, not as a test to adjust. +- [ ] Add the quality snapshot ids to the `daily-entries` export projection and `DailyEntryKind` to the `egg-grades` export projection. +- [ ] Verify new-base defaults are saleable, existing official records remain non-reclassified, migration security tests still pass, and both export regression tests are green. + +### Task 2: Enforce exact reconciliation and atomically produce quality lots + +**Files:** +- Modify: `DailyEntry`, `RecordDailyEntryHandler`, `SubmitDailyEntryHandler`, `AdjustDailyEntryHandler`, grade repositories/commands as required +- Test: Domain, record, submit, adjustment, stock, report, and concurrency integration tests + +- [ ] Write failing domain/API tests for ungraded and partially graded official entries, zero remainder, each saleable quality counter, each non-saleable quality counter, an **inactive-but-saleable** condition grade (must snapshot null — `EggGrade.Deactivate()` leaves `IsSaleable` set, so this is the case a saleability-only rule wrongly accepts), a zero-count day for an otherwise-saleable quality counter (snapshot recorded, no lot — `EggLot.Create` rejects zero), and an adjustment that preserves its original snapshot. +- [ ] Write failing API tests submitting a **condition-kind grade id as a manual line** — on both record and adjust — and assert it is refused. Excluding condition grades from the manual pane is a UI affordance, not the enforcement: `RecordDailyEntryHandler` gates manual-line eligibility on `IsSaleable` **alone**, with no `GradeType`/kind restriction, and this feature makes Cracked and Dirty saleable by default. So a direct or stale API client can name a Cracked id as a manual line, the exact-total check passes (it is only a sum), and submission then creates **both** that manual lot and the counter-backed quality lot for the same grade — double-counting the day's stock and breaking the one-lot-per-grade assumption reconciliation depends on. The rejection belongs at the handler/aggregate boundary where the UI cannot be bypassed. +- [ ] Run them red; assert status/version/lots remain unchanged on refusal. +- [ ] Implement `manualGrades == total - cracked - dirty - discarded` at the aggregate boundary, reject any manual line naming a condition-kind grade, resolve snapshots only on first official submission and only for a grade that is both `Active` and `IsSaleable`, and create/reconcile quality lots — skipping lot creation for a zero counter while still recording its snapshot — in the existing transactions. +- [ ] Update report and stock projections to count only snapshot-backed quality stock as marketable; verify no historical row changes its meaning. +- [ ] Run targeted integration suites plus parallel submit/adjust tests. + +### Task 3: Extend the API contract and align Daily Entry and adjustment UI + +**Files:** +- Modify: `src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs` — `DailyEntryResponse` gains the two quality snapshot ids +- Modify: `src/Cluckwork.Api/Endpoints/EggGrades/EggGradeEndpoints.cs` — `EggGradeResponse` gains `DailyEntryKind` +- Modify: `web/src/api/cluckwork.ts` — mirror both DTO changes in the `DailyEntry` and `EggGrade` TypeScript contracts +- Modify: `web/src/routes/DailyEntryPage.tsx`, `web/src/routes/HistoryPage.tsx`, shared dialog/styles if needed +- Modify: `web/src/i18n/en.ts`, `web/src/i18n/es.ts`, `web/src/i18n/tl.ts` +- Test: API integration coverage in `SubmitDailyEntryTests.cs`/`DailyEntryAdjustTests.cs` and `EggGradeManagementTests.cs` for the two new response fields; `DailyEntryPage.test.tsx`, `HistoryPage.test.tsx`, catalog parity/typecheck + +- [ ] Write failing API integration tests asserting `DailyEntryResponse` carries both quality snapshot ids and `EggGradeResponse` carries `DailyEntryKind`, plus failing UI tests that exclude condition grades from manual entry, show quality counts in the marketable summary, disable official saves for an under/over grade total, and render the adjustment dialog as the two-pane Daily Entry layout. +- [ ] Run focused API and Vitest tests red. +- [ ] Add both response fields and their TypeScript contract mirrors, then implement shared calculations and the adjustment layout without changing draft-save behavior, conflict rebind, reason requirement, or accessible live feedback. The History UI must derive its reconciliation feedback from the entry's own snapshots, never the current egg-grade catalog, so a later saleability change cannot retroactively rewrite how a past entry reads. +- [ ] Run API integration tests, page tests, i18n catalog parity, and typecheck. + +### Task 4: Preserve sales configuration and update non-CI writers + +**Files:** +- Modify: product/grade UI guidance and relevant API tests +- Modify: `DemoDataSeeder`, `SimulationDataSeeder`, `tools/simulation/k6/bundles.js`, Manager/Worker Playwright specs, simulation manifest/verification scripts +- Test: seeder/manifest tests and simulation-harness self-check + +- [ ] Write failing coverage proving a Cracked/Dirty product maps only to its saleable quality lot and can retain an independently configured default price. +- [ ] Update all seed and browser writers to satisfy exact reconciliation and include quality lots where intended. +- [ ] Recompute the simulation fixture manifest and add/update self-checks so a stale writer or fixture fails closed. +- [ ] Execute targeted seed, k6 bundle, Playwright, and harness verification commands documented by PR #395. + +### Task 5: Document the operating rule + +**Files:** +- Modify: `web/src/routes/HelpPage.tsx`, i18n help catalogs, `specs/product/GLOSSARY.md`, `specs/product/specs.md` +- Test: Help-page tests and catalog parity + +- [ ] Explain that drafts are flexible but official saves reconcile exactly, and that Cracked/Dirty quality stock is farm-configurable and separately priced through products. +- [ ] Document the no-retroactive-reclassification rule. +- [ ] Run Help, i18n, backend, and SPA verification suites before review. diff --git a/docs/superpowers/specs/2026-08-02-daily-entry-reconciliation-and-quality-eggs-design.md b/docs/superpowers/specs/2026-08-02-daily-entry-reconciliation-and-quality-eggs-design.md new file mode 100644 index 000000000..642d12d15 --- /dev/null +++ b/docs/superpowers/specs/2026-08-02-daily-entry-reconciliation-and-quality-eggs-design.md @@ -0,0 +1,156 @@ +# Daily Entry Reconciliation and Saleable Quality Eggs + +Tracks #394 and #396. Implementation must follow the non-CI writer rule in +PR #395. + +## Goal + +Prevent an official Daily Entry from losing production, while allowing a farm to +sell Cracked and Dirty eggs as separate, optionally discounted stock. + +## Decisions + +- Draft Daily Entries remain editable with incomplete grading. +- Submit and manager adjustment require exact reconciliation; no API client can + bypass it. +- Cracked and Dirty are saleable by default on a new installation and + independently configurable per farm. +- Discarded remains non-saleable. +- A farm prices Cracked and Dirty through normal, separate Products. There is + no global discount percentage or automatic price calculation. +- A configuration change affects future submissions only. Historical entries + keep their recorded disposition and are never backfilled into stock. + +## Data model + +`EggGrade` gains a stable `DailyEntryKind` (`Manual`, `Cracked`, or `Dirty`). +At most one grade of each quality kind exists per farm; a partial unique index +enforces that rule. The kind is immutable, so renaming a grade never changes +its meaning. The default Cracked and Dirty grades receive their matching kinds +and are saleable in fresh base data. + +`DailyEntry` snapshots `CrackedQualityGradeId` and `DirtyQualityGradeId` when +it first becomes official. Resolution requires the condition grade to be +**both `Active` and `IsSaleable`**; anything else snapshots null. Both +conditions are load-bearing, and saleability alone is not sufficient: +`EggGrade.Deactivate()` flips `Active` and deliberately leaves `IsSaleable` +untouched, so an inactive-but-saleable grade is an ordinary reachable state +whenever an owner deactivates a condition grade without first clearing its +saleability. Resolving on saleability alone would then snapshot a grade the +egg-grade endpoints have removed from capture and mint new lots under it. +A null snapshot therefore records "this condition was not stock that day", +whether because it was non-saleable or because it was deactivated. + +Cover the inactive-but-saleable submission explicitly — it is the case a +saleability-only rule passes and this rule refuses. + +The snapshot—not the current grade setting—governs later reporting, adjustment, +and lot reconciliation. Existing official entries initialise both snapshots as +null. + +Both snapshot ids and each grade's `DailyEntryKind` are part of the public API +contract (`DailyEntryResponse`, `EggGradeResponse`) and their TypeScript +mirrors, not domain-internal state: the History UI must derive its +reconciliation feedback from an entry's own snapshot, never from the egg-grade +catalog's current state, or a later saleability change would retroactively +rewrite how a past entry reads. + +## Write path + +The manual Grading pane excludes the two configured quality-condition grades, +and the **record and adjust handlers refuse a manual line naming either of +them**. The pane is an affordance; the handler check is the guarantee, and +both are required. Manual-line eligibility is gated on `IsSaleable` alone with +no kind restriction, and this feature makes Cracked and Dirty saleable by +default — so with UI-only exclusion a direct or stale API client could name a +Cracked id as a manual line, pass the exact-total check (it is only a sum), +and produce **two lots for one grade**: the manual line's and the +counter-backed one. That double-counts the day's stock and breaks the +one-lot-per-grade assumption reconciliation depends on. + +Its target is always `total - cracked - dirty - discarded`. On submit, the +domain verifies that exact total, then creates one lot per manual grade line +plus one lot for each quality snapshot whose counter is positive that day. A +non-null snapshot is always recorded, independent of its counter — later +adjustments and historical reporting need to know which grade a condition +represented even on a zero-count day — but `EggLot.Create` rejects a zero or +negative quantity, so a zero counter must never reach lot creation. A +zero-cracked day and a zero-dirty day are each an explicit, tested case: the +snapshot is recorded and only that condition's lot is skipped. On adjustment, +the same invariant applies and all linked lots, including quality lots, +reconcile in the existing transaction and retain sold-floor protection. + +This preserves one accounting identity: every collected egg is discarded, a +non-saleable condition loss, or represented by exactly one stock lot. + +## UI and pricing + +Daily Entry and the History adjustment dialog share the same counts/grading +hierarchy and live reconciliation feedback. The adjustment dialog has no draft +state, so its save control is disabled until the exact target and reason are +present. The Products screen continues to map one product to one saleable +grade. Products mapped to Cracked and Dirty set their own optional default +price; an operator may price them lower, equal, or leave a price unset. + +## Compatibility and operations + +Both the schema change and the seed change are carried into the hand-maintained +`InitialCreate` migration itself. There is **no follow-on migration** — see the +next paragraph for why one would touch zero rows by construction, and AGENTS.md +(#245) for why adding a dead backfill is the pattern that squash was performed +to eliminate. Cracked and Dirty are therefore saleable in base data on every +database, because a virgin database is the only database. No historical lots +are invented. + +No migration or grandfather path is needed for a pre-existing incomplete +official entry or a pre-existing manual Cracked/Dirty grade line. This +application has never been deployed, and #245 squashed schema history to one +`InitialCreate` migration, so a virgin database is the only starting state — +there is no production data to grandfather. Two narrower checks confirm +nothing else stands in for it: + +- **A pre-existing incomplete official entry, adjusted after this feature + lands.** The exact-reconciliation gate on Submit/`ManagerAdjust` + (`manualGrades == total - cracked - dirty - discarded`) is #394's rule, + shared with PR #400 — whichever of the two lands first establishes it, and + nothing here changes what it requires. What this design adds on top is + quality-lot creation/reconciliation, gated on a non-null quality snapshot. + Every entry that predates this feature, complete or incomplete, snapshots + null for both conditions (see Data model), so that addition is a no-op on + their adjustment path: it neither blocks nor unblocks anything the shared + #394 rule didn't already decide. Whether an entry submitted under the old + lenient rule — including the explicitly supported no-grade case + `SubmitDailyEntryTests.Submit_WithoutGrades_Succeeds_NoLots` — can later be + adjusted is #394's own compatibility question, unaffected by whether this + feature has landed. +- **A pre-existing manual grade line against the Cracked or Dirty grade.** + Cracked and Dirty already exist in `InitialCreate`'s base reference data + today, as ordinary `GradeType = 'Quality'` grades seeded `IsSaleable = + FALSE`. `RecordDailyEntryHandler` gates a manual grade line on `IsSaleable` + alone, with no `GradeType` restriction, so the capability to grade into them + manually has existed since #245 — a farm need only mark one saleable through + the existing egg-grade endpoint. Nothing exercises that capability today: + `DemoDataSeeder` and `SimulationDataSeeder` are the only writers that submit + entries outside a human operator, and both draw manual grade lines only from + `Large`/`Medium`/`Small` by name — Cracked and Dirty are never candidates, + both because neither seeder selects them and because they seed + non-saleable. There is no deployed farm to have done it by hand either. The + one place it could theoretically exist is a developer's own local dev + database, hand-poked through the SPA and never reset — and that is already + covered by the standing remedy AGENTS.md prescribes for this exact class of + dev-database/migration mismatch: drop and recreate (`docker compose -f + deploy/docker-compose.dev.yml down -v && docker compose -f + deploy/docker-compose.dev.yml up -d`), the same fix already required at the + #245 squash boundary. This feature does not need its own migration or + preservation logic for that case. + +Because this changes the Daily Entry write contract, implementation must update +and verify every non-CI writer and fixture named in PR #395: demo/simulation +seeders, k6 bundles, and the Playwright Manager and Worker flows. The +simulation manifest must change when fixture totals or lots change. + +## Out of scope + +- Food-safety eligibility rules and jurisdiction-specific restrictions. +- Automatic discount formulas, price levels, or multi-grade products. +- A later regrade/repack workflow. diff --git a/specs/product/GLOSSARY.md b/specs/product/GLOSSARY.md index 84922e1d4..83c846f31 100644 --- a/specs/product/GLOSSARY.md +++ b/specs/product/GLOSSARY.md @@ -94,6 +94,32 @@ remainder controls; each screen still renders its own form and owns its own state and submission. So the two can drift, and keeping them saying the same thing is a review obligation, not something the code enforces. +**Condition grade (#396)** — the Cracked and Dirty grades, fed by the Daily +Entry's own **counters** rather than by a hand-typed grade line. A farm may sell +them as separate stock (both are saleable on a new installation) or keep them as +losses by switching either off; **Discarded** is always a loss and is never a +condition grade. + +Two things make the binding safe to rename around. The grade carries a fixed +**kind** (`Cracked` / `Dirty`) rather than being matched on its name, so renaming +"Cracked" to "Segunda" does not detach the counter from the grade. And the entry +**snapshots** which grade each counter resolved to at the moment it became +official — so turning a grade off later never rewrites a past day, and turning +one on never invents stock for a day recorded as a loss. Resolution requires the +grade to be **both active and saleable**; anything else records the condition as +a loss. + +A condition grade can never be entered as a manual grade line — the server +refuses it, not just the screen. Its counter already produces a lot, so a manual +line naming it would produce a second lot for the same grade on the same day. + +**Condition (Reports / History column, #396)** — the eggs a day produced that +became stock **without being hand-graded**: the condition counters above, counted +only where that entry resolved them to a grade. Deliberately shown beside +**Sellable** rather than added into it — Sellable is the hand-graded remainder +and the figure grading counts down to, so the two answer different questions and +are added together to get everything the day produced that can be sold. + **Left to grade (#134)** — grading counts **down** to zero rather than reporting *graded n of m*. The figure beside the grades is how many sellable eggs are still unaccounted for; it turns green when the day adds up exactly and red when diff --git a/specs/product/specs.md b/specs/product/specs.md index 17b495895..cf2a0d0cd 100644 --- a/specs/product/specs.md +++ b/specs/product/specs.md @@ -901,6 +901,7 @@ egg_grades - farm_id - name - grade_type: size / quality / custom +- daily_entry_kind: manual / cracked / dirty - sort_order - is_saleable - is_default @@ -922,6 +923,26 @@ Internal Use Seconds ``` +The default Cracked and Dirty quality grades are active and saleable on a new +installation. A farm may independently mark either grade non-saleable. + +`daily_entry_kind` is immutable and makes the one grade fed by a Daily Entry +condition counter explicit. It is not inferred from a mutable grade name: + +```text +manual = entered in the Grading pane +cracked = populated from the Cracked counter +dirty = populated from the Dirty counter +``` + +There is at most one Cracked and one Dirty kind per farm. The manual Grading +pane excludes both condition grades, and the record and adjust handlers reject +a manual line naming either — so a condition egg cannot be entered a second +time as a manual grade, whether through the app or a direct API call. The +server check is what makes that true: manual lines are otherwise accepted on +saleability alone, and a condition grade named manually would produce a second +lot for the same grade alongside its counter-backed one. + ## 9.2 Daily production by grade ```text @@ -939,6 +960,44 @@ daily_egg_grade_entries - updated_at ``` +Daily Entry retains the three observed condition counters (`cracked`, `dirty`, +and `discarded`). Cracked and Dirty are saleable only when the configured +condition grade is **both active and saleable**. Both flags are required and +neither implies the other: deactivating a grade leaves its saleable flag +untouched, so an inactive-but-saleable grade is an ordinary reachable state, +and resolving on saleability alone would mint stock under a grade the farm has +already retired from capture. On the first official save, the entry snapshots +the resolved Cracked and Dirty grade id (or null when that condition is a loss, +whether because the grade is non-saleable or because it is inactive). The +snapshot controls future reporting and lot reconciliation, so a later catalog +change never reclassifies historical production. + +The manual grade lines must reconcile exactly when an entry becomes official: + +```text +manual grade total = total eggs − cracked − dirty − discarded +``` + +Drafts may remain incomplete. Submitting or manager-adjusting must satisfy the +equation. The submitted entry creates one lot per manual grade, plus one lot +for each non-null condition snapshot whose counter is positive that day — a +snapshot is always recorded once its condition grade is both active and +saleable, but a zero-cracked or zero-dirty day creates no lot for that +condition, since a lot cannot hold a zero quantity. Discarded eggs are always a +loss. Total marketable production is the manual grade total plus any Cracked +and Dirty counts represented by a non-null snapshot. + +The snapshot columns are nullable and are never backfilled: the system does not +create lots for, or retroactively convert, production recorded before its +condition grade resolved. Any entry that became official without a resolved +condition keeps a null snapshot permanently, and reads treat that null as the +durable answer "this condition was a loss that day" rather than as missing data. + +(Stated as a property of the columns rather than as a migration step. Under the +single-migration rule a virgin database is the only starting state, so there is +no population of pre-existing entries for a backfill to visit — and none should +ever be added.) + ## 9.3 Egg lots ```text @@ -1055,6 +1114,11 @@ Phase 1 assumes eggs are **graded at collection**: the daily grade entry (§9.2) the final grade of each egg. The lot's grade is fixed at production. This matches farms that candle/sort during collection and keeps the walking skeleton simple. +Cracked and Dirty are final grades at collection too. Their counts are entered +in Egg counts rather than the manual Grading pane, then materialize under the +entry's snapshotted condition grade only when that grade is both active and +saleable (§9.2). + Phase 1 does **not** model a separate post-collection grading or wash/pack shift, nor moving eggs between grades after the lot exists. @@ -1134,6 +1198,12 @@ Example: | Large Eggs | egg | Large | | Medium Eggs | egg | Medium | | Cracked Eggs | egg | Cracked | +| Dirty Eggs | egg | Dirty | + +Each product owns its own optional default price. Cracked and Dirty products +can be priced lower, but Cluckwork does not derive or enforce a discount from +another product: each farm sets the price appropriate to its market. A sale +continues to snapshot the actual selected product price. ## 10.3 Customers diff --git a/src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs b/src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs index 5b27f1cc8..815f7f6c9 100644 --- a/src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs +++ b/src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs @@ -84,6 +84,11 @@ private static async Task ListDailyEntries( private static DailyEntryResponse ToResponse(Cluckwork.Domain.Eggs.DailyEntry e) => new( e.Id, e.FarmId, e.HouseId, e.FlockId, e.Date, e.Status.ToString(), e.TotalEggs, e.CrackedEggs, e.DirtyEggs, e.DiscardedEggs, e.MortalityCount, + // #396 — which grade each condition counter resolved to when this entry + // became official, or null when that condition was a loss. The SPA needs + // it to say whether a day's cracked eggs became stock; it cannot re-derive + // that from the current catalog, which is the whole point of the snapshot. + e.CrackedGradeId, e.DirtyGradeId, e.Grades.Select(g => new GradeLineResponse(g.EggGradeId, g.Quantity)).ToList(), e.Version, e.AdjustReason, e.VoidReason, e.LockedAtUtc, // The audit snapshot is stored as JSON; embed it as an object, not a string. @@ -219,6 +224,10 @@ public sealed record GradeQuantityRequest(Guid EggGradeId, int Quantity); public sealed record DailyEntryResponse( Guid Id, Guid FarmId, Guid HouseId, Guid FlockId, DateOnly Date, string Status, int TotalEggs, int CrackedEggs, int DirtyEggs, int DiscardedEggs, int MortalityCount, + // #396 — null on a draft (nothing has resolved yet) AND on an official entry + // whose condition was a loss. A reader distinguishes the two by Status, not + // by these fields. + Guid? CrackedGradeId, Guid? DirtyGradeId, IReadOnlyList Grades, int Version, string? AdjustReason, string? VoidReason, DateTimeOffset? LockedAtUtc, JsonElement? AdjustedFrom); diff --git a/src/Cluckwork.Api/Endpoints/EggGrades/EggGradeEndpoints.cs b/src/Cluckwork.Api/Endpoints/EggGrades/EggGradeEndpoints.cs index 644c8987d..3165d021b 100644 --- a/src/Cluckwork.Api/Endpoints/EggGrades/EggGradeEndpoints.cs +++ b/src/Cluckwork.Api/Endpoints/EggGrades/EggGradeEndpoints.cs @@ -127,13 +127,21 @@ private static IResult MapFailure(Cluckwork.Domain.Common.Error error) } private static EggGradeResponse ToResponse(EggGrade g) => - new(g.Id, g.FarmId, g.Name, g.GradeType.ToString(), g.SortOrder, g.IsSaleable, g.Active); + new(g.Id, g.FarmId, g.Name, g.GradeType.ToString(), g.SortOrder, g.IsSaleable, + g.DailyEntryKind.ToString(), g.Active); } // FarmId included from day one — grades are farm-scoped (spec §9.1) and a // multi-farm client needs to know which farm each bucket belongs to. public sealed record EggGradeResponse( - Guid Id, Guid FarmId, string Name, string GradeType, int SortOrder, bool IsSaleable, bool Active); + Guid Id, Guid FarmId, string Name, string GradeType, int SortOrder, bool IsSaleable, + // #396 — "Manual", "Cracked" or "Dirty". The client cannot infer this from + // Name (the farm may rename a grade) or from GradeType (a farm can have many + // Quality grades, only one of which is the Cracked counter's). It is what + // lets the Grading pane leave the two counter-fed grades out; the server + // refuses them regardless, so this is the affordance, not the enforcement. + string DailyEntryKind, + bool Active); public sealed record CreateEggGradeRequest( string Name, string GradeType, int SortOrder, bool IsSaleable); diff --git a/src/Cluckwork.Application/Features/DailyEntries/AdjustDailyEntry/AdjustDailyEntryHandler.cs b/src/Cluckwork.Application/Features/DailyEntries/AdjustDailyEntry/AdjustDailyEntryHandler.cs index 577d598ca..dc83b15a9 100644 --- a/src/Cluckwork.Application/Features/DailyEntries/AdjustDailyEntry/AdjustDailyEntryHandler.cs +++ b/src/Cluckwork.Application/Features/DailyEntries/AdjustDailyEntry/AdjustDailyEntryHandler.cs @@ -74,6 +74,15 @@ public async Task> HandleAsync( "DailyEntry.UnknownGrade", "One or more egg grades do not exist, are inactive, or are not saleable.")) .LogFailure(logger, "AdjustDailyEntry"); + + // #396 — deliberately NOT folded into `allowed` above: that set + // unions the lines already on the entry, so a condition grade could + // be talked past it. ConditionGradeGuard asks the catalog directly. + var conditionGrade = await ConditionGradeGuard.CheckAsync( + eggGrades, command.Grades.Select(g => g.EggGradeId), ct); + if (conditionGrade is not null) + return Result.Failure(conditionGrade) + .LogFailure(logger, "AdjustDailyEntry"); } var previousMortality = entry.MortalityCount; @@ -119,6 +128,23 @@ await unitOfWork.ExecuteInTransactionAsync(async transactionCt => // untouchable wherever they sit) and the first lot in canonical // order carries the remainder. var targets = entry.Grades.ToDictionary(l => l.EggGradeId, l => l.Quantity); + + // #396 — the counter-backed lots are targets too. They are NOT in + // entry.Grades (a condition grade is never a manual line — see + // ConditionGradeGuard), so omitting them here does not merely skip + // them: the loop below drives EVERY lot linked to this entry toward + // its target, and a lot with no target reads as target ZERO. An + // adjustment would therefore silently destroy stock the farm holds, + // on an entry the adjustment did not even intend to change that way. + // + // Read from the entry's SNAPSHOT, never re-resolved from the + // catalog: a farm that turned Cracked non-saleable after submitting + // must still be able to correct that day's count, and the day keeps + // the meaning it was recorded with. + if (entry.CrackedGradeId is { } crackedGradeId) + targets[crackedGradeId] = entry.CrackedEggs; + if (entry.DirtyGradeId is { } dirtyGradeId) + targets[dirtyGradeId] = entry.DirtyEggs; foreach (var gradeLots in lockedLots.GroupBy(l => l.EggGradeId)) { var target = targets.GetValueOrDefault(gradeLots.Key, 0); @@ -167,6 +193,13 @@ await eggMovements.AddAsync(EggInventoryMovement.Create( foreach (var (gradeId, quantity) in targets) { if (lockedLots.Any(l => l.EggGradeId == gradeId)) continue; + + // #396 — a condition target can legitimately be ZERO (a day + // with no cracked eggs), and EggLot.Create rejects a zero + // quantity. Nothing to create: no lot, and nothing to + // reconcile away either, since there is no lot to shrink. + if (quantity <= 0) continue; + var newLot = EggLot.Create( Guid.NewGuid(), accountId, entry.FlockId, entry.Date, gradeId, quantity, dailyEntryId: entry.Id); diff --git a/src/Cluckwork.Application/Features/DailyEntries/ConditionGradeGuard.cs b/src/Cluckwork.Application/Features/DailyEntries/ConditionGradeGuard.cs new file mode 100644 index 000000000..4e2acca1f --- /dev/null +++ b/src/Cluckwork.Application/Features/DailyEntries/ConditionGradeGuard.cs @@ -0,0 +1,61 @@ +namespace Cluckwork.Application.Features.DailyEntries; + +using Cluckwork.Application.Features.EggGrades; +using Cluckwork.Domain.Common; +using Cluckwork.Domain.Eggs; + +// #396 — a condition grade is fed by its Daily Entry COUNTER and never by a +// manual grade line. +// +// Hiding those grades from the Grading pane is a UI affordance, not the +// enforcement. The eligibility check a manual line passes is "active and +// saleable" with NO kind restriction, and this feature makes Cracked and Dirty +// saleable — so a direct or stale API client can name a Cracked id as a manual +// line. The exact-total check (#394) does not catch it, because that check is +// only a sum and the sum still reconciles. Submission would then create the +// manual lot AND the counter-backed quality lot for the same grade: two lots for +// one grade on one day, double-counting the day's stock and breaking the +// one-lot-per-grade assumption lot reconciliation depends on. +// +// Lives in the handlers rather than in a validator on purpose (#394): the demo +// and simulation seeders construct commands and call HandleAsync directly, +// skipping validators entirely, so a validator-only rule would let a fixture +// build exactly the state this refuses. +internal static class ConditionGradeGuard +{ + internal const string ErrorCode = "DailyEntry.ConditionGradeNotManual"; + + // Reads the WHOLE catalog (ListAllAsync) rather than the active+saleable + // slice the callers already fetch, because the question here is "is this id + // bound to a counter", which does not depend on the grade's current flags. + // + // Stated at the strength the evidence supports: this is DEFENSIVE, not a + // fix for a reachable bug. Today an inactive grade is already refused + // upstream by the UnknownGrade check, so narrowing this to ListActiveAsync + // would change the error code a caller sees, not whether the write is + // refused — no test distinguishes them and none is claimed to. It is written + // this way so the guard keeps answering the same question if either + // upstream check is later relaxed, which is exactly the kind of coupling + // that turns a refusal into an acceptance without anything going red. + internal static async Task CheckAsync( + IEggGradeRepository eggGrades, + IEnumerable lineGradeIds, + CancellationToken ct) + { + var ids = lineGradeIds as IReadOnlyCollection ?? lineGradeIds.ToList(); + if (ids.Count == 0) return null; + + var conditionGradeIds = (await eggGrades.ListAllAsync(ct)) + .Where(g => g.DailyEntryKind != DailyEntryKind.Manual) + .Select(g => g.Id) + .ToHashSet(); + + return ids.Any(conditionGradeIds.Contains) + ? Error.Validation( + ErrorCode, + "Cracked and Dirty eggs are captured by their own counters, not as graded lines. " + + "Remove the condition grade from the grading breakdown and use the Cracked or " + + "Dirty count instead.") + : null; + } +} diff --git a/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs b/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs index 9effa001e..2914d3843 100644 --- a/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs +++ b/src/Cluckwork.Application/Features/DailyEntries/RecordDailyEntry/RecordDailyEntryHandler.cs @@ -64,6 +64,15 @@ public async Task> HandleAsync( "DailyEntry.UnknownGrade", "One or more egg grades do not exist, are inactive, or are not saleable.")) .LogFailure(logger, "RecordDailyEntry"); + + // #396 — and it must not be a COUNTER-fed grade. This check is + // separate from `known` above because the two ask different + // questions: `known` is "may this grade receive production at all", + // which Cracked and Dirty now pass. See ConditionGradeGuard. + var conditionGrade = await ConditionGradeGuard.CheckAsync( + eggGrades, command.Grades.Select(g => g.EggGradeId), ct); + if (conditionGrade is not null) + return Result.Failure(conditionGrade).LogFailure(logger, "RecordDailyEntry"); } var existing = await repository.FindByNaturalKeyAsync( diff --git a/src/Cluckwork.Application/Features/DailyEntries/SubmitDailyEntry/SubmitDailyEntryHandler.cs b/src/Cluckwork.Application/Features/DailyEntries/SubmitDailyEntry/SubmitDailyEntryHandler.cs index 9269f1146..02ffa3a7a 100644 --- a/src/Cluckwork.Application/Features/DailyEntries/SubmitDailyEntry/SubmitDailyEntryHandler.cs +++ b/src/Cluckwork.Application/Features/DailyEntries/SubmitDailyEntry/SubmitDailyEntryHandler.cs @@ -1,6 +1,7 @@ namespace Cluckwork.Application.Features.DailyEntries.SubmitDailyEntry; using Cluckwork.Application.Common; +using Cluckwork.Application.Features.EggGrades; using Cluckwork.Application.Features.EggLots; using Cluckwork.Application.Features.Eggs; using Cluckwork.Application.Features.Flocks; @@ -18,6 +19,7 @@ namespace Cluckwork.Application.Features.DailyEntries.SubmitDailyEntry; // are never generated twice. public sealed class SubmitDailyEntryHandler( IDailyEntryRepository entries, + IEggGradeRepository eggGrades, IEggLotRepository eggLots, IBirdMovementRepository birdMovements, IEggInventoryMovementRepository eggMovements, @@ -51,7 +53,19 @@ public async Task> HandleAsync( if (scope.IsFailure) return Result.Failure(scope.Error).LogFailure(logger, "SubmitDailyEntry"); - var submit = entry.Submit(); + // #396 — submission is the ONE point at which the Cracked and Dirty + // counters resolve to a grade. Both flags are required and neither + // implies the other: EggGrade.Deactivate() leaves IsSaleable set, so + // "inactive but saleable" is an ordinary reachable state, and resolving + // on saleability alone would mint stock under a grade the farm has + // already retired from capture. Unresolved (null) is a durable record + // that those eggs were a loss, not a gap to be filled in later — see + // DailyEntry.CrackedGradeId. + var farmGrades = await eggGrades.ListActiveAsync(entry.FarmId, ct); + var crackedGradeId = ResolveCondition(farmGrades, DailyEntryKind.Cracked); + var dirtyGradeId = ResolveCondition(farmGrades, DailyEntryKind.Dirty); + + var submit = entry.Submit(crackedGradeId, dirtyGradeId); if (submit.IsFailure) return Result.Failure(submit.Error).LogFailure(logger, "SubmitDailyEntry"); @@ -73,6 +87,35 @@ await eggMovements.AddAsync(EggInventoryMovement.Create( line.Quantity, nameof(DailyEntry), entry.Id, clock.UtcNow), ct); } + // #396 — the counter-backed lots, in the same transaction as the manual + // ones. Read from the entry's own SNAPSHOT rather than from the ids + // resolved above, so this loop and every later reader agree by + // construction and an adjustment (which never re-resolves) takes the + // identical path. + // + // A zero counter is skipped but stays snapshotted: EggLot.Create rejects + // a zero quantity, and "the farm was selling cracked eggs that day" is + // still a fact worth recording. Snapshot and lot are separate decisions. + foreach (var (gradeId, quantity) in new[] + { + (entry.CrackedGradeId, entry.CrackedEggs), + (entry.DirtyGradeId, entry.DirtyEggs), + }) + { + if (gradeId is null || quantity <= 0) continue; + + var lot = EggLot.Create( + Guid.NewGuid(), accountId, entry.FlockId, + entry.Date, gradeId.Value, quantity, + dailyEntryId: entry.Id); + await eggLots.AddAsync(lot, ct); + lotIds.Add(lot.Id); + + await eggMovements.AddAsync(EggInventoryMovement.Create( + Guid.NewGuid(), accountId, lot.Id, EggMovementType.Production, + quantity, nameof(DailyEntry), entry.Id, clock.UtcNow), ct); + } + // The day's mortality becomes a ledger row so the flock's current count // reflects it. Zero-mortality days write nothing. if (entry.MortalityCount > 0) @@ -94,6 +137,13 @@ await birdMovements.AddAsync(BirdMovement.Create( entry.Id, entry.FlockId, entry.Date, lotIds.Count, entry.MortalityCount); return Result.Success(new SubmitDailyEntryResponse(entry.Id, entry.Status.ToString(), lotIds)); } + + // BOTH flags, never one. ListActiveAsync has already excluded inactive + // grades; IsSaleable is checked here, and the pair is what the rule + // requires. Kept as a named method so the requirement reads as one thing + // rather than as an incidental filter clause. + private static Guid? ResolveCondition(IReadOnlyList activeFarmGrades, DailyEntryKind kind) => + activeFarmGrades.SingleOrDefault(g => g.DailyEntryKind == kind && g.IsSaleable)?.Id; } public sealed record SubmitDailyEntryResponse(Guid Id, string Status, IReadOnlyList EggLotIds); diff --git a/src/Cluckwork.Application/Features/Reports/IReportQueries.cs b/src/Cluckwork.Application/Features/Reports/IReportQueries.cs index 9d5e4c082..6400ad342 100644 --- a/src/Cluckwork.Application/Features/Reports/IReportQueries.cs +++ b/src/Cluckwork.Application/Features/Reports/IReportQueries.cs @@ -14,15 +14,26 @@ public interface IReportQueries Task GetProfitAsync(DateOnly from, DateOnly to, CancellationToken ct = default); } +// #396 — `Sellable` is the HAND-GRADED remainder (total − cracked − dirty − +// discarded): the figure Daily Entry's grading counts down to, and what #394 +// requires the grade lines to reconcile against exactly. +// +// `FromCounts` is the eggs that became stock WITHOUT being hand-graded — the +// Cracked and Dirty counters, but only where the entry resolved that condition +// to a grade (see DailyEntry.CrackedGradeId). Deliberately a SEPARATE figure +// rather than folded into Sellable: the two answer different questions ("how +// many did we grade" vs "how many can we sell"), they were only ever equal +// because conditions used to be losses, and merging them would silently move +// the number the capture screen is validated against. public sealed record ProductionDay( DateOnly Date, int TotalEggs, int Cracked, int Dirty, int Discarded, - int Sellable, int Deaths, long HenDays, decimal? HenDayPct); + int Sellable, int FromCounts, int Deaths, long HenDays, decimal? HenDayPct); public sealed record GradeTotal(Guid EggGradeId, string Name, int Quantity); public sealed record ProductionReport( IReadOnlyList Days, - int TotalEggs, int TotalSellable, int TotalDeaths, + int TotalEggs, int TotalSellable, int TotalFromCounts, int TotalDeaths, long TotalHenDays, decimal? PeriodHenDayPct, IReadOnlyList GradeTotals); diff --git a/src/Cluckwork.Domain/Eggs/DailyEntry.cs b/src/Cluckwork.Domain/Eggs/DailyEntry.cs index c722b61d0..ac513e44e 100644 --- a/src/Cluckwork.Domain/Eggs/DailyEntry.cs +++ b/src/Cluckwork.Domain/Eggs/DailyEntry.cs @@ -23,6 +23,21 @@ public sealed class DailyEntry : AggregateRoot // Sellable production by grade (Phase 1: eggs are graded at collection). public IReadOnlyList Grades => _grades.AsReadOnly(); + // #396 — which grade this day's Cracked and Dirty counters resolved to, or + // null when that condition was a loss. Written ONCE, at the first official + // transition, and never re-resolved: the entry is the historical record of + // what the farm's configuration meant ON THAT DAY. Re-resolving on a later + // adjustment would let a saleability or Active change silently restate a + // past day — banking eggs recorded as a loss, or dropping stock the farm + // already holds and may have sold. + // + // The domain does not resolve these itself (it has no grade catalog); the + // handler passes what it resolved, having required both Active and + // IsSaleable. Null therefore means "resolved to nothing", a durable fact, + // not "not yet known". + public Guid? CrackedGradeId { get; private set; } + public Guid? DirtyGradeId { get; private set; } + // Audit trail until the audit-log slice lands (#69): the last adjust's // reason + the values it replaced (JSON snapshot), the void reason, and // when the auto-lock job locked the entry. @@ -81,7 +96,11 @@ public Result RecordProduction( return Result.Success(); } - public Result Submit() + // #396 — crackedGradeId/dirtyGradeId are the grades the CALLER resolved for + // this day's condition counters, each already required to be both Active + // and IsSaleable, or null when that condition is a loss. They are snapshot + // here and never resolved again (see the fields). + public Result Submit(Guid? crackedGradeId = null, Guid? dirtyGradeId = null) { if (Status != DailyEntryStatus.Draft) return Result.Failure(Error.Domain( @@ -98,6 +117,10 @@ public Result Submit() requireExactReconciliation: true); if (gradeResult.IsFailure) return gradeResult; + // The single resolution point for the life of the entry. + CrackedGradeId = crackedGradeId; + DirtyGradeId = dirtyGradeId; + Status = DailyEntryStatus.Submitted; // Version is a concurrency token, not auto-incremented by EF: without this // bump, two racing submits both match WHERE Version = N and both succeed — diff --git a/src/Cluckwork.Domain/Eggs/EggGrade.cs b/src/Cluckwork.Domain/Eggs/EggGrade.cs index e2ec62e8e..f3544bde2 100644 --- a/src/Cluckwork.Domain/Eggs/EggGrade.cs +++ b/src/Cluckwork.Domain/Eggs/EggGrade.cs @@ -15,13 +15,23 @@ public sealed class EggGrade : AggregateRoot public int SortOrder { get; private set; } public bool IsSaleable { get; private set; } public bool Active { get; private set; } + + // #396 — which Daily Entry counter, if any, feeds this grade. The binding + // is an IDENTITY, deliberately not a name match: a farm may rename + // "Cracked" to "Segunda" through the ordinary grade endpoint, and a + // name-matching resolver would then quietly detach the farm's own counter + // from the farm's own grade. At most one Cracked and one Dirty per farm, + // enforced by a partial unique index (see InitialCreate). + public DailyEntryKind DailyEntryKind { get; private set; } + public int Version { get; private set; } private EggGrade() { } public static EggGrade Create( Guid id, Guid accountId, Guid farmId, string name, - EggGradeType gradeType, int sortOrder, bool isSaleable) + EggGradeType gradeType, int sortOrder, bool isSaleable, + DailyEntryKind dailyEntryKind = DailyEntryKind.Manual) { if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Grade name is required.", nameof(name)); @@ -35,13 +45,18 @@ public static EggGrade Create( GradeType = gradeType, SortOrder = sortOrder, IsSaleable = isSaleable, + DailyEntryKind = dailyEntryKind, Active = true }; } // GradeType stays immutable after creation — history recorded under a bucket // keeps the axis it was captured with; relabeling the axis would silently - // reinterpret past entries. + // reinterpret past entries. DailyEntryKind is immutable for a STRONGER + // reason (#396): an official entry snapshots the grade id its Cracked or + // Dirty counter resolved to, so re-pointing a kind afterwards would change + // what a past day's counter is understood to have produced. Neither is a + // parameter here, which is the enforcement — there is no path to change one. public Result Update(string name, int sortOrder, bool isSaleable) { if (string.IsNullOrWhiteSpace(name)) @@ -77,3 +92,15 @@ public Result Activate() } public enum EggGradeType { Size, Quality, Custom } + +// #396 — which Daily Entry input feeds a grade. `Manual` is every ordinary +// grade, entered by hand in the Grading pane. `Cracked` and `Dirty` are the two +// grades fed by the entry's own condition counters instead, and are excluded +// from manual grading precisely so a condition egg cannot also be counted as a +// manual line (which would produce two lots for one grade on one day). +// +// Deliberately separate from EggGradeType: that says what KIND of bucket this +// is (a size, a quality, a custom one) and is the farm's own taxonomy, while +// this says WHERE THE NUMBER COMES FROM and is the app's wiring. A farm can +// have many Quality grades; only one of them can be the Cracked counter's. +public enum DailyEntryKind { Manual, Cracked, Dirty } diff --git a/src/Cluckwork.Infrastructure/Persistence/Configurations/DailyEntryConfiguration.cs b/src/Cluckwork.Infrastructure/Persistence/Configurations/DailyEntryConfiguration.cs index 4da1e6bb4..9c38220fd 100644 --- a/src/Cluckwork.Infrastructure/Persistence/Configurations/DailyEntryConfiguration.cs +++ b/src/Cluckwork.Infrastructure/Persistence/Configurations/DailyEntryConfiguration.cs @@ -86,6 +86,26 @@ public void Configure(EntityTypeBuilder builder) .HasMaxLength(16) .IsRequired(); + // #396 — stored as a string like GradeType, for the same reason: the + // column stays readable in a psql session and an enum reorder cannot + // silently repoint existing rows to a different kind. + builder.Property(g => g.DailyEntryKind) + .HasConversion() + .HasMaxLength(16) + .IsRequired(); + + // At most ONE Cracked and ONE Dirty grade per farm — the Daily Entry has + // exactly one counter for each, so a second claimant makes "which grade + // does this counter feed" ambiguous, and resolution would silently pick + // one. Filtered so the many ordinary Manual grades are unconstrained. + // + // Scoped per FARM rather than per account: grade rows are farm-owned + // (the case-insensitive name index is per farm too), so two farms in one + // account each get their own Cracked. + builder.HasIndex(g => new { g.AccountId, g.FarmId, g.DailyEntryKind }) + .IsUnique() + .HasFilter("\"DailyEntryKind\" <> 'Manual'"); + builder.Property(g => g.Version).IsConcurrencyToken(); // Name uniqueness is case-insensitive per farm — enforced by a raw diff --git a/src/Cluckwork.Infrastructure/Persistence/DemoDataSeeder.cs b/src/Cluckwork.Infrastructure/Persistence/DemoDataSeeder.cs index 4c12ca40a..689c7ca38 100644 --- a/src/Cluckwork.Infrastructure/Persistence/DemoDataSeeder.cs +++ b/src/Cluckwork.Infrastructure/Persistence/DemoDataSeeder.cs @@ -249,10 +249,17 @@ private async Task SeedDemoAsync(Guid accountId, CancellationToken ct) // --- Products (#99): sales sell products, not raw grades. Prices are // per individual egg (unit Egg → factor 1), preserving the old demo math. + // The catalog is deliberately partial — Small has no product either, so + // the Stock screen shows a realistic mix of sellable and unlisted grades. var largeEggs = Require(await createProduct.HandleAsync(new CreateProductCommand( "Large Eggs", "Egg", "Egg", 45, grades["Large"], null), accountId, ct)); var mediumEggs = Require(await createProduct.HandleAsync(new CreateProductCommand( "Medium Eggs", "Egg", "Egg", 38, grades["Medium"], null), accountId, ct)); + // #396: the cracked counter now mints its own lot at submit, so the demo + // carries a discounted product for it — otherwise the feature reads as + // stock appearing that nothing can ever sell. Priced below Small. + Require(await createProduct.HandleAsync(new CreateProductCommand( + "Cracked Eggs", "Egg", "Egg", 18, grades["Cracked"], "Sold at a discount"), accountId, ct)); // --- Orders: one confirmed (exercises FIFO allocation), one open draft. var confirmed = Require(await createOrder.HandleAsync(new CreateSalesOrderCommand( diff --git a/src/Cluckwork.Infrastructure/Persistence/Migrations/20260801190854_InitialCreate.Designer.cs b/src/Cluckwork.Infrastructure/Persistence/Migrations/20260801190854_InitialCreate.Designer.cs index 796be0365..61d75c3a0 100644 --- a/src/Cluckwork.Infrastructure/Persistence/Migrations/20260801190854_InitialCreate.Designer.cs +++ b/src/Cluckwork.Infrastructure/Persistence/Migrations/20260801190854_InitialCreate.Designer.cs @@ -358,12 +358,18 @@ protected override void BuildTargetModel(ModelBuilder modelBuilder) b.Property("CrackedEggs") .HasColumnType("integer"); + b.Property("CrackedGradeId") + .HasColumnType("uuid"); + b.Property("Date") .HasColumnType("date"); b.Property("DirtyEggs") .HasColumnType("integer"); + b.Property("DirtyGradeId") + .HasColumnType("uuid"); + b.Property("DiscardedEggs") .HasColumnType("integer"); @@ -450,6 +456,11 @@ protected override void BuildTargetModel(ModelBuilder modelBuilder) b.Property("Active") .HasColumnType("boolean"); + b.Property("DailyEntryKind") + .IsRequired() + .HasMaxLength(16) + .HasColumnType("character varying(16)"); + b.Property("FarmId") .HasColumnType("uuid"); @@ -475,6 +486,10 @@ protected override void BuildTargetModel(ModelBuilder modelBuilder) b.HasKey("Id"); + b.HasIndex("AccountId", "FarmId", "DailyEntryKind") + .IsUnique() + .HasFilter("\"DailyEntryKind\" <> 'Manual'"); + b.ToTable("EggGrades"); }); diff --git a/src/Cluckwork.Infrastructure/Persistence/Migrations/20260801190854_InitialCreate.cs b/src/Cluckwork.Infrastructure/Persistence/Migrations/20260801190854_InitialCreate.cs index 5c2f5b91c..bba994fad 100644 --- a/src/Cluckwork.Infrastructure/Persistence/Migrations/20260801190854_InitialCreate.cs +++ b/src/Cluckwork.Infrastructure/Persistence/Migrations/20260801190854_InitialCreate.cs @@ -194,6 +194,15 @@ protected override void Up(MigrationBuilder migrationBuilder) DirtyEggs = table.Column(type: "integer", nullable: false), DiscardedEggs = table.Column(type: "integer", nullable: false), MortalityCount = table.Column(type: "integer", nullable: false), + // #396 — the grade each condition counter resolved to when + // this entry became official, or NULL when that condition was + // a loss. NULL is a RECORDED OUTCOME, not "unknown": a draft + // has not resolved yet, and an official entry that reads NULL + // is stating that those eggs were never stock. Deliberately + // no FK to EggGrades — the snapshot must survive its grade + // being deleted, exactly as it must survive a rename. + CrackedGradeId = table.Column(type: "uuid", nullable: true), + DirtyGradeId = table.Column(type: "uuid", nullable: true), AdjustReason = table.Column(type: "character varying(500)", maxLength: 500, nullable: true), AdjustedFromJson = table.Column(type: "text", nullable: true), VoidReason = table.Column(type: "character varying(500)", maxLength: 500, nullable: true), @@ -236,6 +245,16 @@ protected override void Up(MigrationBuilder migrationBuilder) SortOrder = table.Column(type: "integer", nullable: false), IsSaleable = table.Column(type: "boolean", nullable: false), Active = table.Column(type: "boolean", nullable: false), + // #396 — which Daily Entry counter feeds this grade. Defaults + // to 'Manual' so every ordinary grade, and every row an older + // writer inserts without naming a kind, is a hand-graded one: + // a condition binding is only ever created deliberately. The + // partial unique index below is what keeps a farm from having + // two Cracked grades; the default must therefore stay + // 'Manual', since that value is the one the filter excludes. + DailyEntryKind = table.Column( + type: "character varying(16)", maxLength: 16, nullable: false, + defaultValue: "Manual"), Version = table.Column(type: "integer", nullable: false), AccountId = table.Column(type: "uuid", nullable: false) }, @@ -1311,6 +1330,23 @@ protected override void Up(MigrationBuilder migrationBuilder) table: "WaterUsages", columns: new[] { "FlockId", "Date" }); + // #396 — one Cracked and one Dirty grade per farm. The Daily Entry + // has exactly one counter for each, so a second claimant would make + // "which grade does this counter feed" ambiguous and resolution + // would silently pick one. Partial, so the many ordinary 'Manual' + // grades stay unconstrained — which is also why 'Manual' must remain + // the column default (see the EggGrades table above). + // + // EF CAN model this one (unlike the four lower("Name") expression + // indexes below), so it lives above the hand-carried line and is + // reproduced by a regenerate. + migrationBuilder.CreateIndex( + name: "IX_EggGrades_AccountId_FarmId_DailyEntryKind", + table: "EggGrades", + columns: new[] { "AccountId", "FarmId", "DailyEntryKind" }, + unique: true, + filter: "\"DailyEntryKind\" <> 'Manual'"); + // --------------------------------------------------------------- // Carried forward by hand from the squashed history (#245). // Everything above this line is regenerated from the EF model; @@ -1423,21 +1459,28 @@ CREATE UNIQUE INDEX "IX_Products_AccountId_LowerName" // not reference "Name", so either all ten insert or none do). --- migrationBuilder.Sql( """ - INSERT INTO "EggGrades" ("Id", "AccountId", "FarmId", "Name", "GradeType", "SortOrder", "IsSaleable", "Active", "Version") + INSERT INTO "EggGrades" ("Id", "AccountId", "FarmId", "Name", "GradeType", "SortOrder", "IsSaleable", "DailyEntryKind", "Active", "Version") SELECT v.id::uuid, '0000000a-0000-0000-0000-000000000001', '0000000f-0000-0000-0000-000000000001', - v.name, v.grade_type, v.sort_order, v.is_saleable, TRUE, 0 + v.name, v.grade_type, v.sort_order, v.is_saleable, v.daily_entry_kind, TRUE, 0 FROM (VALUES - ('0000000e-0000-0000-0000-000000000001', 'Small', 'Size', 0, TRUE), - ('0000000e-0000-0000-0000-000000000002', 'Medium', 'Size', 1, TRUE), - ('0000000e-0000-0000-0000-000000000003', 'Large', 'Size', 2, TRUE), - ('0000000e-0000-0000-0000-000000000004', 'Jumbo', 'Size', 3, TRUE), - ('0000000e-0000-0000-0000-000000000005', 'Seconds', 'Quality', 4, TRUE), - ('0000000e-0000-0000-0000-000000000006', 'Cracked', 'Quality', 5, FALSE), - ('0000000e-0000-0000-0000-000000000007', 'Dirty', 'Quality', 6, FALSE), - ('0000000e-0000-0000-0000-000000000008', 'Soft Shell', 'Quality', 7, FALSE), - ('0000000e-0000-0000-0000-000000000009', 'Discarded', 'Custom', 8, FALSE), - ('0000000e-0000-0000-0000-000000000010', 'Internal Use', 'Custom', 9, FALSE) - ) AS v(id, name, grade_type, sort_order, is_saleable) + ('0000000e-0000-0000-0000-000000000001', 'Small', 'Size', 0, TRUE, 'Manual'), + ('0000000e-0000-0000-0000-000000000002', 'Medium', 'Size', 1, TRUE, 'Manual'), + ('0000000e-0000-0000-0000-000000000003', 'Large', 'Size', 2, TRUE, 'Manual'), + ('0000000e-0000-0000-0000-000000000004', 'Jumbo', 'Size', 3, TRUE, 'Manual'), + ('0000000e-0000-0000-0000-000000000005', 'Seconds', 'Quality', 4, TRUE, 'Manual'), + -- #396 — Cracked and Dirty are SALEABLE by default and are + -- the two counter-fed grades. Saleable is the product + -- decision (a farm that does not sell them turns either off + -- in Settings); the kind is the wiring, and is what makes + -- the binding survive a rename. Discarded stays a loss and + -- keeps 'Manual' precisely because no counter resolves to a + -- grade for it — discarded eggs never become stock. + ('0000000e-0000-0000-0000-000000000006', 'Cracked', 'Quality', 5, TRUE, 'Cracked'), + ('0000000e-0000-0000-0000-000000000007', 'Dirty', 'Quality', 6, TRUE, 'Dirty'), + ('0000000e-0000-0000-0000-000000000008', 'Soft Shell', 'Quality', 7, FALSE, 'Manual'), + ('0000000e-0000-0000-0000-000000000009', 'Discarded', 'Custom', 8, FALSE, 'Manual'), + ('0000000e-0000-0000-0000-000000000010', 'Internal Use', 'Custom', 9, FALSE, 'Manual') + ) AS v(id, name, grade_type, sort_order, is_saleable, daily_entry_kind) WHERE NOT EXISTS ( SELECT 1 FROM "EggGrades" WHERE "AccountId" = '0000000a-0000-0000-0000-000000000001' diff --git a/src/Cluckwork.Infrastructure/Persistence/Migrations/AppDbContextModelSnapshot.cs b/src/Cluckwork.Infrastructure/Persistence/Migrations/AppDbContextModelSnapshot.cs index 0a860c8d2..1b7049adb 100644 --- a/src/Cluckwork.Infrastructure/Persistence/Migrations/AppDbContextModelSnapshot.cs +++ b/src/Cluckwork.Infrastructure/Persistence/Migrations/AppDbContextModelSnapshot.cs @@ -355,12 +355,18 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("CrackedEggs") .HasColumnType("integer"); + b.Property("CrackedGradeId") + .HasColumnType("uuid"); + b.Property("Date") .HasColumnType("date"); b.Property("DirtyEggs") .HasColumnType("integer"); + b.Property("DirtyGradeId") + .HasColumnType("uuid"); + b.Property("DiscardedEggs") .HasColumnType("integer"); @@ -447,6 +453,11 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("Active") .HasColumnType("boolean"); + b.Property("DailyEntryKind") + .IsRequired() + .HasMaxLength(16) + .HasColumnType("character varying(16)"); + b.Property("FarmId") .HasColumnType("uuid"); @@ -472,6 +483,10 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.HasKey("Id"); + b.HasIndex("AccountId", "FarmId", "DailyEntryKind") + .IsUnique() + .HasFilter("\"DailyEntryKind\" <> 'Manual'"); + b.ToTable("EggGrades"); }); diff --git a/src/Cluckwork.Infrastructure/Persistence/SimulationDataSeeder.cs b/src/Cluckwork.Infrastructure/Persistence/SimulationDataSeeder.cs index 3d68fc788..855599b98 100644 --- a/src/Cluckwork.Infrastructure/Persistence/SimulationDataSeeder.cs +++ b/src/Cluckwork.Infrastructure/Persistence/SimulationDataSeeder.cs @@ -66,8 +66,9 @@ namespace Cluckwork.Infrastructure.Persistence; // paths without any change — it's one entry per (flock, day), so the // production report and the daily-entries/daily-entry-grades/egg-lots export // datasets scale directly with HistoryDays (2 flocks × 90 days = 180 entries, -// ~500+ grade/lot rows once SubmitDailyEntry mints up to 3 egg lots per -// entry). Sales and expenses were the thin spot: Task 3b/3c's lifecycle +// ~500+ grade/lot rows once SubmitDailyEntry mints 3 graded + 2 condition +// (#396) egg lots per entry). Sales and expenses were the thin spot: Task +// 3b/3c's lifecycle // fixtures (draft/confirmed/partially-paid orders, the 4 expenses) all sat // inside the most recent ~week of history, so a sales/expense/profit report // or an export request against an OLDER slice of the window saw nothing. @@ -597,11 +598,21 @@ private async Task SeedFlockHistoryAsync( // One product per saleable grade, sold in individual eggs (factor 1) — // keeps order-item quantities directly comparable to lot sizes, which is // what makes the FIFO-contention shape below easy to reason about. + // + // #396: Cracked/Dirty are here for a different reason than the graded + // three. Submit now mints a lot for each condition counter, so without a + // product mapped to those grades the fixture accumulates stock that can + // never leave — ~180 lots of dead inventory the sales screens can see and + // nothing can sell. Priced below Small, which is the point of the feature + // (a cracked egg is saleable, at a discount). Nothing here orders them; + // the FIFO-contention shape above stays a Large/Medium story. private static readonly (string GradeName, string ProductName, long PriceMinorUnits)[] CatalogWanted = [ ("Large", "Sim Large Eggs", 45), ("Medium", "Sim Medium Eggs", 38), ("Small", "Sim Small Eggs", 30), + ("Cracked", "Sim Cracked Eggs", 18), + ("Dirty", "Sim Dirty Eggs", 22), ]; private static readonly (string Name, string Phone, string Note)[] CustomersWanted = @@ -1126,6 +1137,20 @@ private static void Require(Result result, string what) // not merely "at least one" (#279 Fix 2). private const int GradesPerDailyEntry = 3; + // #396: submit ALSO mints a lot for each condition counter that resolved + // to a condition grade (DailyEntryKind.Cracked/Dirty) and carried a + // positive count. Kept separate from GradesPerDailyEntry rather than + // folded into it: these are NOT grade lines — no GradeQuantityDto names + // them, and ConditionGradeGuard actively refuses one that does — so the + // two constants answer different questions and drift for different + // reasons. Both counters are structurally positive here (`cracked = 4 + + // d % 3` >= 4, `dirty = 2 + d % 2` >= 2, for every day and baseline), and + // the base seed ships Cracked/Dirty active + saleable, so both resolve on + // every submitted entry — exactly 2, never "up to 2". If a future fixture + // lets either counter reach 0, SubmitDailyEntryHandler skips that lot and + // this must become a per-entry sum instead of a constant. + private const int ConditionLotsPerDailyEntry = 2; + // Mirrors SeedSalesAsync's two EnsureDraftOrderAsync calls (customer1/2, // never confirmed) and its three lifecycle EnsureConfirmedOrderAsync // calls (customer1/2 unpaid + customer3 partially paid) — the recurring @@ -1330,7 +1355,7 @@ private static SimulationExpectedCounts ComputeExpectedCounts(SimulationOptions DraftEntries: draftEntries, SubmittedEntries: submittedEntries, LockedEntries: lockedEntries, - EggLots: GradesPerDailyEntry * submittedOrLockedEntries, + EggLots: (GradesPerDailyEntry + ConditionLotsPerDailyEntry) * submittedOrLockedEntries, SalesOrdersTotal: DraftOrdersCount + salesOrdersConfirmed, SalesOrdersDraft: DraftOrdersCount, SalesOrdersConfirmed: salesOrdersConfirmed, diff --git a/src/Cluckwork.Infrastructure/Repositories/ExportQueries.cs b/src/Cluckwork.Infrastructure/Repositories/ExportQueries.cs index 848ef96db..8326541ed 100644 --- a/src/Cluckwork.Infrastructure/Repositories/ExportQueries.cs +++ b/src/Cluckwork.Infrastructure/Repositories/ExportQueries.cs @@ -120,11 +120,19 @@ private sealed class AsyncDisposableAction(Func action) : IAsyncDispo "daily-entries" => Rows(activeDb.DailyEntries.AsNoTracking() .OrderBy(x => x.Date).ThenBy(x => x.Id), + // #396 — the two snapshot ids ride next to the counters they + // explain. Without them an export records that a day had 40 + // cracked eggs but not whether those became saleable stock or a + // loss, and that is no longer derivable from the current + // catalog: the whole point of the snapshot is that the catalog + // may have changed since. ["id", "farmId", "houseId", "flockId", "date", "status", "totalEggs", "crackedEggs", "dirtyEggs", "discardedEggs", "mortalityCount", + "crackedGradeId", "dirtyGradeId", "adjustReason", "adjustedFromJson", "voidReason", "lockedAtUtc", "version"], x => [x.Id, x.FarmId, x.HouseId, x.FlockId, x.Date, x.Status, x.TotalEggs, x.CrackedEggs, x.DirtyEggs, x.DiscardedEggs, x.MortalityCount, + x.CrackedGradeId, x.DirtyGradeId, x.AdjustReason, x.AdjustedFromJson, x.VoidReason, x.LockedAtUtc, x.Version]), "daily-entry-grades" => Rows(activeDb.DailyEntryGrades.AsNoTracking() @@ -134,8 +142,13 @@ private sealed class AsyncDisposableAction(Func action) : IAsyncDispo "egg-grades" => Rows(activeDb.EggGrades.AsNoTracking() .OrderBy(x => x.SortOrder).ThenBy(x => x.Id), - ["id", "farmId", "name", "gradeType", "sortOrder", "isSaleable", "active", "version"], - x => [x.Id, x.FarmId, x.Name, x.GradeType, x.SortOrder, x.IsSaleable, x.Active, x.Version]), + // #396 — dailyEntryKind is what makes the snapshot ids above + // interpretable: it is the only field saying WHICH counter a + // grade serves, and unlike the name it cannot be edited. + ["id", "farmId", "name", "gradeType", "sortOrder", "isSaleable", + "dailyEntryKind", "active", "version"], + x => [x.Id, x.FarmId, x.Name, x.GradeType, x.SortOrder, x.IsSaleable, + x.DailyEntryKind, x.Active, x.Version]), "egg-lots" => Rows(activeDb.EggLots.AsNoTracking() .OrderBy(x => x.ProductionDate).ThenBy(x => x.Id), diff --git a/src/Cluckwork.Infrastructure/Repositories/ReportQueries.cs b/src/Cluckwork.Infrastructure/Repositories/ReportQueries.cs index 20e9b7f8c..20b8033f1 100644 --- a/src/Cluckwork.Infrastructure/Repositories/ReportQueries.cs +++ b/src/Cluckwork.Infrastructure/Repositories/ReportQueries.cs @@ -25,6 +25,14 @@ public async Task GetProductionAsync( Cracked = g.Sum(e => e.CrackedEggs), Dirty = g.Sum(e => e.DirtyEggs), Discarded = g.Sum(e => e.DiscardedEggs), + // #396 — only the conditions this entry actually RESOLVED to a + // grade. A null snapshot means those eggs were a loss on that + // day, and summing the raw counters instead would invent stock + // the farm never had — most visibly for entries recorded before + // this feature, whose conditions were never saleable at all. + FromCounts = g.Sum(e => + (e.CrackedGradeId != null ? e.CrackedEggs : 0) + + (e.DirtyGradeId != null ? e.DirtyEggs : 0)), Deaths = g.Sum(e => e.MortalityCount), }) .ToDictionaryAsync(x => x.Date, ct); @@ -37,8 +45,53 @@ public async Task GetProductionAsync( .GroupBy(g => g.EggGradeId) .Select(g => new { EggGradeId = g.Key, Quantity = g.Sum(x => x.Quantity) }) .ToListAsync(ct); + + // #396 — condition-backed production has NO DailyEntryGrade row and + // never can: ConditionGradeGuard refuses a manual line naming a + // condition grade precisely so one day cannot produce two lots for one + // grade. So a breakdown built from DailyEntryGrades alone omits every + // cracked and dirty egg that this same response counts as produced + // stock in FromCounts/TotalFromCounts — the "By grade" table would sum + // to the hand-graded remainder while the header reported more. Fold the + // snapshot-backed counters in under the grade each entry actually + // resolved to, keyed on the SNAPSHOT (never the live grade catalog), so + // the breakdown reconciles with the stock that exists. + // + // Two queries rather than one: the counters live in different columns + // under different snapshot keys, so a single GROUP BY cannot express it + // without a UNION EF would not translate. + // + // `Counter > 0` mirrors SubmitDailyEntryHandler's own lot condition — + // a snapshot is recorded even on a zero-cracked day, and counting those + // would put a grade in the breakdown the farm holds no stock under, + // which reads as an empty grade rather than as absence. + var crackedTotals = await db.DailyEntries + .Where(e => e.Date >= from && e.Date <= to && OfficialStatuses.Contains(e.Status) + && e.CrackedGradeId != null && e.CrackedEggs > 0) + .GroupBy(e => e.CrackedGradeId!.Value) + .Select(g => new { EggGradeId = g.Key, Quantity = g.Sum(e => e.CrackedEggs) }) + .ToListAsync(ct); + + var dirtyTotals = await db.DailyEntries + .Where(e => e.Date >= from && e.Date <= to && OfficialStatuses.Contains(e.Status) + && e.DirtyGradeId != null && e.DirtyEggs > 0) + .GroupBy(e => e.DirtyGradeId!.Value) + .Select(g => new { EggGradeId = g.Key, Quantity = g.Sum(e => e.DirtyEggs) }) + .ToListAsync(ct); + + // Accumulate rather than assign. A condition grade cannot collide with + // a DailyEntryGrade row today (the guard above), but Cracked and Dirty + // could in principle be bound to the SAME grade id by a farm that + // pointed both kinds at one catalog row — accumulating keeps that + // arithmetic right instead of silently dropping one counter. + var combined = new Dictionary(); + foreach (var t in gradeTotals) + combined[t.EggGradeId] = combined.GetValueOrDefault(t.EggGradeId) + t.Quantity; + foreach (var t in crackedTotals.Concat(dirtyTotals)) + combined[t.EggGradeId] = combined.GetValueOrDefault(t.EggGradeId) + t.Quantity; + var gradeNames = await db.EggGrades - .Where(g => gradeTotals.Select(t => t.EggGradeId).Contains(g.Id)) + .Where(g => combined.Keys.Contains(g.Id)) .ToDictionaryAsync(g => g.Id, g => g.Name, ct); // Hen-days (spec §19.3): the farm's bird count on each day, summed over @@ -119,7 +172,7 @@ public async Task GetProductionAsync( var sellable = total - (row?.Cracked ?? 0) - (row?.Dirty ?? 0) - (row?.Discarded ?? 0); days.Add(new ProductionDay( d, total, row?.Cracked ?? 0, row?.Dirty ?? 0, row?.Discarded ?? 0, - sellable, row?.Deaths ?? 0, henDays, + sellable, row?.FromCounts ?? 0, row?.Deaths ?? 0, henDays, henDays > 0 ? Math.Round(total * 100m / henDays, 1) : null)); if (d == DateOnly.MaxValue) break; // AddDays would overflow } @@ -130,16 +183,18 @@ public async Task GetProductionAsync( days, totalEggs, days.Sum(x => x.Sellable), + days.Sum(x => x.FromCounts), days.Sum(x => x.Deaths), totalHenDays, totalHenDays > 0 ? Math.Round(totalEggs * 100m / totalHenDays, 1) : null, - gradeTotals + combined .Select(t => new GradeTotal( - t.EggGradeId, gradeNames.GetValueOrDefault(t.EggGradeId, "?"), t.Quantity)) + t.Key, gradeNames.GetValueOrDefault(t.Key, "?"), t.Value)) .OrderByDescending(t => t.Quantity) .ToList()); } + public async Task GetSalesAsync( DateOnly from, DateOnly to, CancellationToken ct = default) { diff --git a/tests/Cluckwork.Api.IntegrationTests/BaseReferenceDataMigrationTests.cs b/tests/Cluckwork.Api.IntegrationTests/BaseReferenceDataMigrationTests.cs index a715ae925..1dd956f70 100644 --- a/tests/Cluckwork.Api.IntegrationTests/BaseReferenceDataMigrationTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/BaseReferenceDataMigrationTests.cs @@ -1,6 +1,7 @@ namespace Cluckwork.Api.IntegrationTests; using Cluckwork.Domain.Accounts; +using Cluckwork.Domain.Eggs; using Cluckwork.Infrastructure.Persistence; using Cluckwork.Infrastructure.Persistence.Interceptors; using Cluckwork.Infrastructure.Providers; @@ -101,4 +102,75 @@ public async Task MigratingAVirginDatabase_CreatesTheCaseInsensitiveUniqueIndexe Assert.NotNull(duplicate); Assert.Contains("IX_EggGrades_AccountId_FarmId_LowerName", duplicate.ToString(), StringComparison.Ordinal); } + + // #396 — the product decision ("Cracked and Dirty are saleable on a fresh + // install, and each is bound to its counter") lives entirely in the seed + // SQL, where nothing else would notice it being reverted: the sibling test + // above counts ten grades and would still pass with every flag wrong. + [Fact] + public async Task MigratingAVirginDatabase_MakesTheTwoConditionGradesSaleableAndBound() + { + await using var postgres = new PostgreSqlBuilder(PostgresImage).Build(); + await postgres.StartAsync(); + await using var db = BuildContext(postgres.GetConnectionString()); + + await db.Database.MigrateAsync(); + + var grades = await db.EggGrades.IgnoreQueryFilters() + .Where(g => g.AccountId == SeedDefaults.AccountId && g.FarmId == SeedDefaults.FarmId) + .ToListAsync(); + + var cracked = Assert.Single(grades, g => g.DailyEntryKind == DailyEntryKind.Cracked); + var dirty = Assert.Single(grades, g => g.DailyEntryKind == DailyEntryKind.Dirty); + + Assert.Equal("Cracked", cracked.Name); + Assert.Equal("Dirty", dirty.Name); + Assert.True(cracked.IsSaleable, "Cracked must be saleable on a fresh install (#396)."); + Assert.True(dirty.IsSaleable, "Dirty must be saleable on a fresh install (#396)."); + + // Discarded is always a loss, so it must NOT be bound to a counter — + // binding it would make discarded eggs resolvable to stock. + Assert.Equal( + DailyEntryKind.Manual, + Assert.Single(grades, g => g.Name == "Discarded").DailyEntryKind); + Assert.False(Assert.Single(grades, g => g.Name == "Discarded").IsSaleable); + + // Every other seeded grade is hand-graded. + Assert.Equal(8, grades.Count(g => g.DailyEntryKind == DailyEntryKind.Manual)); + } + + // The partial unique index, proved the same way as the expression indexes + // above — by the behaviour it exists to produce, not by its name. A name + // check passes against an index created without its WHERE clause, which + // would instead forbid a farm from having more than one Manual grade. + [Fact] + public async Task MigratingAVirginDatabase_AllowsManyManualGradesButOnlyOneOfEachCondition() + { + await using var postgres = new PostgreSqlBuilder(PostgresImage).Build(); + await postgres.StartAsync(); + await using var db = BuildContext(postgres.GetConnectionString()); + + await db.Database.MigrateAsync(); + + // A second Cracked for the same farm is refused. + var secondCracked = await Record.ExceptionAsync(() => db.Database.ExecuteSqlAsync($""" + INSERT INTO "EggGrades" ("Id", "AccountId", "FarmId", "Name", "GradeType", "SortOrder", "IsSaleable", "DailyEntryKind", "Active", "Version") + VALUES ({Guid.NewGuid()}, {SeedDefaults.AccountId}, {SeedDefaults.FarmId}, 'Cracked 2', 'Quality', 98, TRUE, 'Cracked', TRUE, 0) + """)); + + Assert.NotNull(secondCracked); + Assert.Contains( + "IX_EggGrades_AccountId_FarmId_DailyEntryKind", + secondCracked.ToString(), + StringComparison.Ordinal); + + // ...but an eleventh Manual grade is fine. This is the half a filterless + // index would break, and it is why the column default must stay 'Manual'. + var anotherManual = await Record.ExceptionAsync(() => db.Database.ExecuteSqlAsync($""" + INSERT INTO "EggGrades" ("Id", "AccountId", "FarmId", "Name", "GradeType", "SortOrder", "IsSaleable", "DailyEntryKind", "Active", "Version") + VALUES ({Guid.NewGuid()}, {SeedDefaults.AccountId}, {SeedDefaults.FarmId}, 'Peewee', 'Size', 97, TRUE, 'Manual', TRUE, 0) + """)); + + Assert.Null(anotherManual); + } } diff --git a/tests/Cluckwork.Api.IntegrationTests/ExportTests.cs b/tests/Cluckwork.Api.IntegrationTests/ExportTests.cs index c36ac594d..ed052d8f2 100644 --- a/tests/Cluckwork.Api.IntegrationTests/ExportTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/ExportTests.cs @@ -34,6 +34,41 @@ private sealed record Created(Guid Id); return (client, accountId, farmId, flockId); } + // #396 — an export is the farm's own copy of its records, so it must stay + // self-describing. Without these two columns it says a day had N cracked + // eggs but not whether they became stock or a loss, and that is NOT + // recoverable from the grade catalog afterwards — the snapshot exists + // precisely because the catalog can change. dailyEntryKind is the other + // half: the only field naming which counter a grade serves, and the only + // one that survives a rename. + [Fact] + public async Task Export_CarriesTheConditionSnapshotsAndTheGradeBinding() + { + var (client, accountId, farmId, _) = await SetupAsync(); + + await factory.WithTenantScopeAsync(accountId, async db => + { + db.EggGrades.Add(Domain.Eggs.EggGrade.Create( + Guid.NewGuid(), accountId, farmId, "Cracked", + Domain.Eggs.EggGradeType.Quality, 50, isSaleable: true, + dailyEntryKind: Domain.Eggs.DailyEntryKind.Cracked)); + await db.SaveChangesAsync(); + }); + + var gradesCsv = await (await client.GetAsync("/api/v1/export/egg-grades")) + .Content.ReadAsStringAsync(); + var entriesHeader = (await (await client.GetAsync("/api/v1/export/daily-entries")) + .Content.ReadAsStringAsync()).Split('\n')[0]; + + // Header, then the value — a header-only check passes against a column + // wired to the wrong property. + Assert.Contains("dailyEntryKind", gradesCsv.Split('\n')[0], StringComparison.Ordinal); + Assert.Contains("Cracked", gradesCsv, StringComparison.Ordinal); + + Assert.Contains("crackedGradeId", entriesHeader, StringComparison.Ordinal); + Assert.Contains("dirtyGradeId", entriesHeader, StringComparison.Ordinal); + } + [Fact] public async Task Export_IsAdminOnly_UnknownDatasetIs404() { diff --git a/tests/Cluckwork.Api.IntegrationTests/MigrationSecurityReviewTests.cs b/tests/Cluckwork.Api.IntegrationTests/MigrationSecurityReviewTests.cs index c6af20487..c128bf5f5 100644 --- a/tests/Cluckwork.Api.IntegrationTests/MigrationSecurityReviewTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/MigrationSecurityReviewTests.cs @@ -1,7 +1,11 @@ namespace Cluckwork.Api.IntegrationTests; +using System.Collections; +using System.Globalization; +using System.Reflection; using System.Text.RegularExpressions; using Cluckwork.Infrastructure.Persistence; +using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Migrations; using Microsoft.EntityFrameworkCore.Migrations.Operations; @@ -70,23 +74,297 @@ private static bool LooksHashShaped(string value) => // shape a migrationBuilder.Sql(...) credential would actually take. private static readonly Regex SqlStringLiteral = new(@"'([^']*)'", RegexOptions.Compiled); - // #245's whole contract is "exactly one migration, InitialCreate" — the - // application has never been deployed, so a virgin database is the only - // starting state that exists anywhere, and there is therefore never a - // reason to add a second migration file instead of hand-folding a change - // into InitialCreate (see that migration's own header comment, most - // recently its #364 addendum). This is the automated fence: a PR that - // adds e.g. "AddFoo.cs" beside InitialCreate.cs now fails here instead of - // only getting flagged in review — which is exactly what happened to the - // migration this test itself replaces (PR #399 shipped a second - // migration; this guard did not yet exist to catch it). + // #407 CUTOVER — this fence used to assert `Assert.Single(AllMigrations())`. + // + // #245's contract was "exactly one migration, InitialCreate", valid only + // while the application had never been deployed: a virgin database was the + // only starting state anywhere, so hand-folding a schema change into + // InitialCreate rewrote a file no deployed database had applied yet. PR + // #407 is the LAST change permitted to do that. From the commit that + // merges it, a real database exists that has already applied + // InitialCreate, and EF will never re-run it — so a hand-folded column is + // a column that silently does not exist in production. (That failure mode + // is not hypothetical: it is exactly what #399's amendment did to the dev + // database, surfacing as a broken login rather than as a migration error.) + // + // What replaces it is NOT "no fence". Two things must still hold, and both + // are cheap to break by accident: + // + // 1. InitialCreate stays FIRST and keeps its recorded id. Regenerating + // it (a fresh `dotnet ef migrations add InitialCreate`) mints a NEW + // timestamp, which reorders it behind migrations that depend on it + // and makes __EFMigrationsHistory disagree with the assembly on every + // deployed database. Pinning the id is what makes that a red test + // rather than a failed deploy. + // 2. Ordering is asserted on the MIGRATION ID, never on list position. + // AllMigrations() is built from Assembly.GetTypes(), whose order is + // explicitly unspecified — a positional check would pass or fail on + // reflection ordering, which is the definition of a flaky guard. + // + // Everything else in this file deliberately scans EVERY migration + // (AllInsertDataOperations/AllSqlOperations already fan out across the + // whole history), so the credential and reference-data guarantees below + // extend to post-cutover migrations for free — they were written that way + // from the start, and that is what makes relaxing this one test safe. + private const string InitialCreateMigrationId = "20260801190854_InitialCreate"; + + [Fact] + public void InitialCreate_IsStillTheFirstMigration_AndKeepsItsRecordedId() + { + var ordered = AllMigrations() + .Select(m => (Migration: m, Id: MigrationIdOf(m))) + .OrderBy(x => x.Id, StringComparer.Ordinal) + .ToList(); + + Assert.NotEmpty(ordered); + Assert.Equal("InitialCreate", ordered[0].Migration.GetType().Name); + Assert.Equal(InitialCreateMigrationId, ordered[0].Id); + } + + private static string MigrationIdOf(Migration migration) => + migration.GetType() + .GetCustomAttributes(typeof(MigrationAttribute), inherit: false) + .Cast() + .Single() + .Id; + + // #407 codex review (P1) — pinning InitialCreate's NAME and ID is not the + // same as freezing it, and the gap is exactly the regression the cutover + // exists to prevent. A later PR that edits InitialCreate.cs to add a column + // (instead of adding a second migration) changes neither the class name nor + // the MigrationAttribute, so the identity test above stays green; the other + // tests in this file only look at credential-shaped inserts, the + // reference-data SQL, and the four expression indexes. An ordinary + // AddColumn/CreateTable edit was invisible to all of them. A database that + // already recorded this ID skips the edit, and the app then runs against a + // schema element that does not exist. + // + // So the operations themselves are fingerprinted. Any added, removed, or + // retyped column/table/index/raw-SQL statement changes the digest below and + // fails here, naming the fix. + // + // Deliberately a STRUCTURAL description, not a serialization of EF's + // operation objects: the latter would break on an EF version bump that + // touched an internal property nobody edited, training people to re-baseline + // the constant on sight — which is how a fence like this dies. Describe() + // reads only the properties that define what the migration DOES. + // + // Its `_` fallback collapses unusual operation types to their type name, so + // two different DropColumns would describe identically. That is covered by + // pinning the COUNT alongside the digest: an added or removed operation + // moves the count even when its description is coarse. Stated plainly + // because it is a real limit, not a claim of totality. + // + // Legitimately changing InitialCreate should now be impossible. If this + // fails, the fix is almost always "add a migration instead" — re-baselining + // the constant is the wrong move unless the change genuinely predates any + // deployed database. + private const int InitialCreateOperationCount = 114; + private const string InitialCreateOperationDigest = + "240b373e763e15b0c73e400bc8ef69b1d80b4548efb9f29439938facd2ae305a"; + + // #407 CI failure — the digest above is only meaningful if it depends on the + // MIGRATION and nothing else. The first version depended on the machine: it + // recursed into ColumnOperation.ClrType (a System.Type) and out through + // Assembly.Location, embedding an absolute path to System.Private.CoreLib.dll. + // It passed here and failed in CI, which is the worst possible shape for a + // fence — the pinned constant becomes a local artefact and the natural + // "fix" is to re-baseline it until CI agrees, at which point it guards + // nothing. + // + // Running it three times on ONE machine is what I checked, and it proved + // exactly nothing about this: same-run determinism is not portability. This + // test is the check that was missing — it asserts the described text carries + // no environment at all, so the failure cannot come back silently the next + // time the walk reaches a new property. + [Fact] + public void InitialCreate_Description_ContainsNoEnvironmentSpecificData() + { + var initialCreate = AllMigrations().Single(m => m.GetType().Name == "InitialCreate"); + var text = string.Join("\n", initialCreate.UpOperations.Select(Describe)); + + // A schema description has no business naming an assembly file, and no + // business containing the runtime's or the test binary's own directory. + Assert.DoesNotContain(".dll", text, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain(AppContext.BaseDirectory, text, StringComparison.Ordinal); + Assert.DoesNotContain( + Path.GetDirectoryName(typeof(object).Assembly.Location) ?? "", + text, + StringComparison.Ordinal); + + // Size is the cheap canary for the same defect: the machine-dependent + // version ballooned to ~168 MB by cycling Type -> Assembly -> Module -> + // Type. A real description of this migration is a few hundred KB at most. + Assert.True(text.Length < 2_000_000, + $"The operation description is {text.Length:N0} chars — the walk is almost certainly " + + "recursing into reflection metadata rather than schema. See the Type leaf case in DescribeValue."); + } + [Fact] - public void ExactlyOneMigrationExists_AndItIsInitialCreate() + public void InitialCreate_OperationsAreFrozen() { - var migration = Assert.Single(AllMigrations()); - Assert.Equal("InitialCreate", migration.GetType().Name); + var initialCreate = AllMigrations() + .Single(m => m.GetType().Name == "InitialCreate"); + + var descriptions = initialCreate.UpOperations.Select(Describe).ToList(); + var digest = Sha256Hex(string.Join("\n", descriptions)); + + Assert.True( + InitialCreateOperationCount == descriptions.Count + && string.Equals(InitialCreateOperationDigest, digest, StringComparison.Ordinal), + $""" + InitialCreate's operations changed (count {descriptions.Count}, digest {digest}). + + Since PR #407 this migration is FROZEN: a database that already applied + it will never re-run it, so a column folded in here silently does not + exist in production and surfaces as broken behaviour, not as an error. + + Add a new migration instead: + dotnet ef migrations add -p src/Cluckwork.Infrastructure -s src/Cluckwork.Api + + Only re-baseline the two constants above if this change genuinely + predates every database that has applied {InitialCreateMigrationId}. + """); + } + + // #407 codex review round 2 (P1) — the hand-written switch this replaces + // listed properties per operation type, and that approach missed something + // on every pass. It captured a column's name/type/nullability but NOT its + // default, nor a CreateTable's nested primary key, foreign keys, unique + // constraints, or a foreign key's OnDelete. So changing + // `AspNetUsers.CredentialEpoch` from `defaultValue: 1` to `0` — a value + // #364 says is PERMANENTLY RETIRED — left the count and every emitted + // description identical, and the freeze test green. + // + // Enumerating harder is the same bet that already lost twice. This walks + // every public readable property instead, recursing into nested operations, + // column definitions, annotations and sequences, so a schema-defining + // property is included because it EXISTS, not because someone remembered + // it. Adding a property to an operation type cannot silently escape the + // digest. + // + // Determinism matters as much as completeness: properties are ordered by + // NAME, never by reflection order, for the same reason the ordering test + // above sorts on the migration id — Type.GetProperties() is explicitly + // unspecified, and a digest built on it would churn between runs. + // + // The EF-version-churn concern that motivated the original structural + // choice still applies and is accepted deliberately: this reads EF's + // PUBLIC operation model (the documented shape of a migration), not + // internals, and a genuine change there is worth a human look. The failure + // message says re-baselining is almost never the right response. + private static string Describe(MigrationOperation operation) => + $"{operation.GetType().Name}({DescribeValue(operation, depth: 0)})"; + + private const int MaxDescribeDepth = 6; + + private static string DescribeValue(object? value, int depth) + { + switch (value) + { + case null: + return "null"; + case string s: + return $"'{s}'"; + case bool or char or decimal or double or float or int or long or short + or uint or ulong or ushort or byte or sbyte or Guid or DateTime or DateTimeOffset: + return Convert.ToString(value, CultureInfo.InvariantCulture) ?? "?"; + case Enum e: + return $"{e.GetType().Name}.{e}"; + + // A System.Type is a LEAF here, described by its full name only. + // + // #407 CI failure — without this the walk recurses into Type's own + // reflection surface (ColumnOperation.ClrType is a Type) and reaches + // Assembly.Location / Module.FullyQualifiedName / CodeBase, which are + // ABSOLUTE PATHS to System.Private.CoreLib.dll. The digest then + // encodes where the .NET runtime happens to live, so it differed + // between this dev box (a /nix/store path) and the CI runner and the + // test failed there while passing locally. It also made the described + // text ~168 MB, since Type -> Assembly -> Module -> Type cycles back + // through the whole reflection graph until MaxDescribeDepth cuts it. + // + // FullName is the schema-relevant part and nothing else about a Type + // is: a column's CLR type matters, the file its metadata lives in + // does not. + // + // NOT AssemblyQualifiedName, and that is deliberate rather than + // lazy: it carries the assembly VERSION, so a routine dependency + // bump would change this digest without any schema changing — the + // same false-red churn the path leak caused, arriving by a tidier + // route. FullName can in principle collide between same-named types + // in different assemblies; every ClrType this migration actually + // uses is a BCL primitive (Guid, string, int, long, decimal, bool, + // DateTime, DateTimeOffset, DateOnly), so there is nothing to + // collide with, and a column typed as two different same-named + // types would be a far larger problem than this digest. + case Type type: + return $"Type({type.FullName})"; + } + + if (depth >= MaxDescribeDepth) return $""; + + if (value is IAnnotation annotation) + return $"@{annotation.Name}={DescribeValue(annotation.Value, depth + 1)}"; + + if (value is IEnumerable enumerable) + { + var items = enumerable.Cast().Select(v => DescribeValue(v, depth + 1)); + // NOT sorted: for columns, key columns and operation lists the ORDER + // is itself schema (a composite key's column order changes the + // index). Sorting here would hide a reordering. + return $"[{string.Join(',', items)}]"; + } + + // #407 codex review round 3 (P1) — annotations are reached through + // GetAnnotations(), a METHOD, so a walk over public non-indexed + // PROPERTIES never sees them. The previous version had an IAnnotation + // branch and a comment claiming annotations were covered; the branch was + // unreachable and the comment was simply false. Proven, not argued: + // with that version in place, flipping InitialCreate's + // Npgsql:ValueGenerationStrategy from IdentityByDefaultColumn to + // IdentityAlwaysColumn — a material identity-generation change — left + // the freeze test GREEN. + // + // Enumerated explicitly and FIRST, so every annotatable in the graph + // (the operation itself, and each AddColumnOperation nested in a + // CreateTable) contributes. Sorted by name: IReadOnlyAnnotatable + // promises no order, and an unsorted digest would churn between runs. + var annotations = value is IReadOnlyAnnotatable annotatable + ? annotatable.GetAnnotations() + .OrderBy(a => a.Name, StringComparer.Ordinal) + .Select(a => $"@{a.Name}={DescribeValue(a.Value, depth + 1)}") + .ToList() + : []; + + var members = value.GetType() + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.CanRead && p.GetIndexParameters().Length == 0) + .OrderBy(p => p.Name, StringComparer.Ordinal) + .Select(p => $"{p.Name}={DescribeValue(SafeGet(p, value), depth + 1)}"); + + return string.Join(';', annotations.Concat(members)); + } + + private static object? SafeGet(PropertyInfo property, object target) + { + try + { + return property.GetValue(target); + } + catch (Exception ex) + { + // A property that throws must not be silently equivalent to null — + // that would be a hole in the digest. Record the failure itself. + return $""; + } } + private static string Sha256Hex(string value) => + Convert.ToHexString( + System.Security.Cryptography.SHA256.HashData(System.Text.Encoding.UTF8.GetBytes(value))) + .ToLowerInvariant(); + [Fact] public void NoMigration_EverInsertsIntoTheUsersTable() { diff --git a/tests/Cluckwork.Api.IntegrationTests/QualityEggTests.cs b/tests/Cluckwork.Api.IntegrationTests/QualityEggTests.cs new file mode 100644 index 000000000..a9e3d6f59 --- /dev/null +++ b/tests/Cluckwork.Api.IntegrationTests/QualityEggTests.cs @@ -0,0 +1,459 @@ +namespace Cluckwork.Api.IntegrationTests; + +using System.Net; +using Cluckwork.Api.IntegrationTests.Infrastructure; +using Cluckwork.Domain.Eggs; +using Microsoft.EntityFrameworkCore; + +// #396 — Cracked and Dirty become saleable stock through their own counters. +// +// The rule these tests exist to hold: a condition grade is fed by its COUNTER +// and never by a manual grade line. Excluding it from the Grading pane is a UI +// affordance; the enforcement has to be server-side, because the eligibility +// check a manual line passes is "active and saleable" with no kind restriction — +// and this feature makes Cracked and Dirty saleable. So a direct or stale API +// client can name a Cracked id as a manual line, the exact-total check passes +// (it is only a sum), and submission then creates BOTH that manual lot and the +// counter-backed one: two lots for one grade on one day, double-counting the +// day's stock and breaking the one-lot-per-grade assumption reconciliation +// depends on. +[Collection(IntegrationCollection.Name)] +public sealed class QualityEggTests(CluckworkWebApplicationFactory factory) +{ + private sealed record RecordedDto(Guid Id); + + private static object Body(Guid farmId, Guid flockId, object[] grades, int total = 618) => new + { + farmId, + houseId = Guid.NewGuid(), + flockId, + date = DateOnly.FromDateTime(DateTime.UtcNow.Date), + totalEggs = total, + crackedEggs = 10, + dirtyEggs = 5, + discardedEggs = 3, + mortalityCount = 0, + grades + }; + + private async Task<(HttpClient Client, Guid AccountId, Guid FarmId, Guid FlockId, + Dictionary Manual, Guid CrackedId, Guid DirtyId)> SetupAsync( + bool crackedSaleable = true, bool crackedActive = true) + { + var email = $"u-{Guid.NewGuid():N}@test.local"; + var accountId = await factory.SeedAccountWithUserAsync(email); + var farmId = Guid.NewGuid(); + var manual = await factory.SeedEggGradesAsync(accountId, farmId, "Large"); + var flockId = await factory.SeedFlockAsync(accountId, farmId); + + var crackedId = Guid.NewGuid(); + var dirtyId = Guid.NewGuid(); + await factory.WithTenantScopeAsync(accountId, async db => + { + var cracked = EggGrade.Create( + crackedId, accountId, farmId, "Cracked", EggGradeType.Quality, + sortOrder: 50, isSaleable: crackedSaleable, + dailyEntryKind: DailyEntryKind.Cracked); + if (!crackedActive) cracked.Deactivate(); + db.EggGrades.Add(cracked); + + db.EggGrades.Add(EggGrade.Create( + dirtyId, accountId, farmId, "Dirty", EggGradeType.Quality, + sortOrder: 51, isSaleable: true, + dailyEntryKind: DailyEntryKind.Dirty)); + await db.SaveChangesAsync(); + }); + + var client = factory.CreateAuthedClient(await factory.LoginForAccessTokenAsync(email)); + return (client, accountId, farmId, flockId, manual, crackedId, dirtyId); + } + + [Fact] + public async Task Recording_a_manual_line_naming_a_condition_grade_is_refused() + { + var (client, _, farmId, flockId, _, crackedId, _) = await SetupAsync(); + + // 600 named against the CRACKED grade as a manual line. The sum still + // reconciles (600 + 10 + 5 + 3 = 618), which is exactly why the total + // check cannot catch this. + var response = await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), + Body(farmId, flockId, [new { eggGradeId = crackedId, quantity = 600 }])); + + Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode); + Assert.Contains("ConditionGradeNotManual", await response.Content.ReadAsStringAsync(), StringComparison.Ordinal); + } + + [Fact] + public async Task A_refused_condition_line_writes_nothing() + { + var (client, accountId, farmId, flockId, _, crackedId, _) = await SetupAsync(); + + await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), + Body(farmId, flockId, [new { eggGradeId = crackedId, quantity = 600 }])); + + // Not "the response was 422" — the durable state. A refusal that still + // created a draft would leave the day half-recorded under a grade the + // farm cannot see in its Grading pane. + await factory.WithTenantScopeAsync(accountId, async db => + Assert.Empty(await db.DailyEntries.IgnoreQueryFilters() + .Where(e => e.FlockId == flockId).ToListAsync())); + } + + [Fact] + public async Task Adjusting_onto_a_condition_grade_is_refused_and_leaves_the_entry_intact() + { + var (client, accountId, farmId, flockId, manual, crackedId, _) = await SetupAsync(); + + var entryId = (await (await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), + Body(farmId, flockId, [new { eggGradeId = manual["Large"], quantity = 600 }]))) + .Content.ReadFromJsonAsync())!.Id; + + await client.PostWithKeyAsync($"/api/v1/daily-entries/{entryId}/submit", Guid.NewGuid().ToString()); + + var before = await StateOf(accountId, entryId); + + var response = await client.PostWithKeyAsync( + $"/api/v1/daily-entries/{entryId}/adjust", Guid.NewGuid().ToString(), + new + { + // Adjust carries the entry's optimistic version. Sent correctly + // so the refusal under test is the condition-grade guard and not + // a 409 the request would have earned anyway. + version = before.Version, + totalEggs = 618, + crackedEggs = 10, + dirtyEggs = 5, + discardedEggs = 3, + mortalityCount = 0, + reason = "recount", + grades = new[] { new { eggGradeId = crackedId, quantity = 600 } } + }); + + Assert.Equal(HttpStatusCode.UnprocessableEntity, response.StatusCode); + + // Adjust is the path that already generated lots, so a partial failure + // here is worse than on record: the entry must keep its status, its + // version, and its lot set exactly. + Assert.Equal(before, await StateOf(accountId, entryId)); + } + + // ---- resolution and lot creation ----------------------------------- + + private async Task SubmitAsync(HttpClient client, Guid farmId, Guid flockId, Guid manualGradeId) + { + var entryId = (await (await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), + Body(farmId, flockId, [new { eggGradeId = manualGradeId, quantity = 600 }]))) + .Content.ReadFromJsonAsync())!.Id; + + var submit = await client.PostWithKeyAsync( + $"/api/v1/daily-entries/{entryId}/submit", Guid.NewGuid().ToString()); + Assert.Equal(HttpStatusCode.OK, submit.StatusCode); + return entryId; + } + + [Fact] + public async Task Submitting_turns_a_saleable_condition_counter_into_its_own_lot() + { + var (client, accountId, farmId, flockId, manual, crackedId, dirtyId) = await SetupAsync(); + + var entryId = await SubmitAsync(client, farmId, flockId, manual["Large"]); + + await factory.WithTenantScopeAsync(accountId, async db => + { + var entry = await db.DailyEntries.IgnoreQueryFilters().SingleAsync(e => e.Id == entryId); + Assert.Equal(crackedId, entry.CrackedGradeId); + Assert.Equal(dirtyId, entry.DirtyGradeId); + + var lots = await db.EggLots.IgnoreQueryFilters() + .Where(l => l.DailyEntryId == entryId).ToListAsync(); + + // 600 manual + 10 cracked + 5 dirty. Discarded is never stock. + Assert.Equal(3, lots.Count); + Assert.Equal(10, Assert.Single(lots, l => l.EggGradeId == crackedId).QuantityProduced); + Assert.Equal(5, Assert.Single(lots, l => l.EggGradeId == dirtyId).QuantityProduced); + }); + } + + // Voiding is the destructive path, and the one where "condition lots are + // just lots" has to actually hold. VoidDailyEntryHandler reverses whatever + // GetByDailyEntryLockedAsync returns — every lot linked to the entry, + // grade-agnostic — so the condition lots are covered by construction rather + // than by a second code path. That is worth pinning precisely BECAUSE it is + // implicit: a future narrowing of that query to the entry's grade LINES + // (the shape AdjustDailyEntryHandler originally had, and the bug this PR + // fixed there) would leave a voided day carrying live, sellable cracked and + // dirty stock with no entry standing behind it — phantom inventory, and + // silent, since nothing else looks at a voided entry again. + [Fact] + public async Task Voiding_empties_the_condition_lots_too() + { + var (client, accountId, farmId, flockId, manual, crackedId, dirtyId) = await SetupAsync(); + var manualGradeId = manual["Large"]; + + var entryId = await SubmitAsync(client, farmId, flockId, manualGradeId); + var version = (await StateOf(accountId, entryId)).Version; + + var response = await client.PostWithKeyAsync( + $"/api/v1/daily-entries/{entryId}/void", Guid.NewGuid().ToString(), + new { version, reason = "recount void" }); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + await factory.WithTenantScopeAsync(accountId, async db => + { + var lots = await db.EggLots.IgnoreQueryFilters() + .Where(l => l.DailyEntryId == entryId).ToListAsync(); + + // All three lots survive as rows (#60 — nothing is deleted, only + // emptied), and every one of them is now at zero. Asserting the + // manual lot alongside the two condition ones is what makes this a + // test of the WHOLE reversal rather than of the condition half: + // a change that emptied only the condition lots would be just as + // wrong, and would pass an assertion that named them alone. + Assert.Equal(3, lots.Count); + Assert.Equal(0, Assert.Single(lots, l => l.EggGradeId == manualGradeId).QuantityProduced); + Assert.Equal(0, Assert.Single(lots, l => l.EggGradeId == crackedId).QuantityProduced); + Assert.Equal(0, Assert.Single(lots, l => l.EggGradeId == dirtyId).QuantityProduced); + + // The eggs left on the ledger, not merely off the lot (#101). One + // Void movement per lot that had stock, so a reversal that zeroed a + // quantity without writing the movement is caught here rather than + // showing up later as stock that cannot be reconciled. + var lotIds = lots.Select(l => l.Id).ToList(); + var voided = await db.EggInventoryMovements.IgnoreQueryFilters() + .Where(m => m.MovementType == EggMovementType.Void && lotIds.Contains(m.EggLotId)) + .ToListAsync(); + Assert.Equal(3, voided.Count); + Assert.Equal(-615, voided.Sum(m => m.QuantityDelta)); // 600 + 10 + 5 + }); + } + + [Fact] + public async Task A_non_saleable_condition_stays_a_loss() + { + var (client, accountId, farmId, flockId, manual, crackedId, _) = + await SetupAsync(crackedSaleable: false); + + var entryId = await SubmitAsync(client, farmId, flockId, manual["Large"]); + + await factory.WithTenantScopeAsync(accountId, async db => + { + var entry = await db.DailyEntries.IgnoreQueryFilters().SingleAsync(e => e.Id == entryId); + Assert.Null(entry.CrackedGradeId); + Assert.Empty(await db.EggLots.IgnoreQueryFilters() + .Where(l => l.DailyEntryId == entryId && l.EggGradeId == crackedId).ToListAsync()); + }); + } + + // The case a saleability-only rule gets wrong. EggGrade.Deactivate() leaves + // IsSaleable set, so "inactive but saleable" is an ordinary reachable state + // — reached by an owner retiring a condition grade without first clearing + // its saleability. Resolving on IsSaleable alone would mint stock under a + // grade the farm has already removed from capture. + [Fact] + public async Task An_inactive_but_saleable_condition_grade_resolves_to_nothing() + { + var (client, accountId, farmId, flockId, manual, crackedId, _) = + await SetupAsync(crackedSaleable: true, crackedActive: false); + + var entryId = await SubmitAsync(client, farmId, flockId, manual["Large"]); + + await factory.WithTenantScopeAsync(accountId, async db => + { + var grade = await db.EggGrades.IgnoreQueryFilters().SingleAsync(g => g.Id == crackedId); + Assert.False(grade.Active); + Assert.True(grade.IsSaleable, "the fixture must keep the trap: inactive yet still saleable"); + + var entry = await db.DailyEntries.IgnoreQueryFilters().SingleAsync(e => e.Id == entryId); + Assert.Null(entry.CrackedGradeId); + Assert.Empty(await db.EggLots.IgnoreQueryFilters() + .Where(l => l.DailyEntryId == entryId && l.EggGradeId == crackedId).ToListAsync()); + }); + } + + // A zero counter still RESOLVES — the snapshot records that the farm was + // selling cracked eggs that day — but mints no lot, because EggLot.Create + // rejects a zero quantity. Snapshot and lot are separate decisions and this + // is the case that proves it. + [Fact] + public async Task A_zero_counter_is_snapshotted_but_creates_no_lot() + { + var (client, accountId, farmId, flockId, manual, crackedId, _) = await SetupAsync(); + + var entryId = (await (await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), + new + { + farmId, + houseId = Guid.NewGuid(), + flockId, + date = DateOnly.FromDateTime(DateTime.UtcNow.Date), + totalEggs = 608, + crackedEggs = 0, + dirtyEggs = 5, + discardedEggs = 3, + mortalityCount = 0, + grades = new[] { new { eggGradeId = manual["Large"], quantity = 600 } } + })).Content.ReadFromJsonAsync())!.Id; + + Assert.Equal(HttpStatusCode.OK, (await client.PostWithKeyAsync( + $"/api/v1/daily-entries/{entryId}/submit", Guid.NewGuid().ToString())).StatusCode); + + await factory.WithTenantScopeAsync(accountId, async db => + { + var entry = await db.DailyEntries.IgnoreQueryFilters().SingleAsync(e => e.Id == entryId); + Assert.Equal(crackedId, entry.CrackedGradeId); + Assert.Empty(await db.EggLots.IgnoreQueryFilters() + .Where(l => l.DailyEntryId == entryId && l.EggGradeId == crackedId).ToListAsync()); + }); + } + + // ---- adjustment reconciles the quality lots too --------------------- + // + // The manual lines and the condition counters produce the same KIND of + // thing — a lot linked to this entry — so an adjustment has to reconcile + // both. Getting this wrong is not a missing feature but active damage: the + // reconciler drives every linked lot toward a target, and a condition lot + // absent from the target set is driven to ZERO, silently destroying stock + // the farm holds. + + private async Task CrackedLotQuantity(Guid accountId, Guid entryId, Guid crackedId) + { + var quantity = -1; + await factory.WithTenantScopeAsync(accountId, async db => + { + var lot = await db.EggLots.IgnoreQueryFilters() + .SingleOrDefaultAsync(l => l.DailyEntryId == entryId && l.EggGradeId == crackedId); + quantity = lot?.QuantityProduced ?? 0; + }); + return quantity; + } + + private Task AdjustAsync( + HttpClient client, Guid entryId, int version, int cracked, int total) => + client.PostWithKeyAsync( + $"/api/v1/daily-entries/{entryId}/adjust", Guid.NewGuid().ToString(), + new + { + version, + totalEggs = total, + crackedEggs = cracked, + dirtyEggs = 5, + discardedEggs = 3, + mortalityCount = 0, + reason = "recount", + grades = new[] { new { eggGradeId = ManualGradeId, quantity = 600 } } + }); + + private Guid ManualGradeId; + + [Fact] + public async Task Adjusting_a_condition_counter_upward_grows_its_lot() + { + var (client, accountId, farmId, flockId, manual, crackedId, _) = await SetupAsync(); + ManualGradeId = manual["Large"]; + + var entryId = await SubmitAsync(client, farmId, flockId, ManualGradeId); + Assert.Equal(10, await CrackedLotQuantity(accountId, entryId, crackedId)); + + var version = (await StateOf(accountId, entryId)).Version; + var response = await AdjustAsync(client, entryId, version, cracked: 20, total: 628); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(20, await CrackedLotQuantity(accountId, entryId, crackedId)); + } + + [Fact] + public async Task Adjusting_a_condition_counter_to_zero_empties_its_lot() + { + var (client, accountId, farmId, flockId, manual, crackedId, _) = await SetupAsync(); + ManualGradeId = manual["Large"]; + + var entryId = await SubmitAsync(client, farmId, flockId, ManualGradeId); + var version = (await StateOf(accountId, entryId)).Version; + + var response = await AdjustAsync(client, entryId, version, cracked: 0, total: 608); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(0, await CrackedLotQuantity(accountId, entryId, crackedId)); + } + + [Fact] + public async Task Adjusting_leaves_the_manual_lot_alone_while_the_counter_changes() + { + // The other half: reconciling the condition lots must not disturb the + // manual ones. A fix that simply overwrote the target set would pass + // the two tests above and empty every manual lot. + var (client, accountId, farmId, flockId, manual, crackedId, _) = await SetupAsync(); + ManualGradeId = manual["Large"]; + + var entryId = await SubmitAsync(client, farmId, flockId, ManualGradeId); + var version = (await StateOf(accountId, entryId)).Version; + + Assert.Equal(HttpStatusCode.OK, + (await AdjustAsync(client, entryId, version, cracked: 20, total: 628)).StatusCode); + + await factory.WithTenantScopeAsync(accountId, async db => + { + var manualLot = await db.EggLots.IgnoreQueryFilters() + .SingleAsync(l => l.DailyEntryId == entryId && l.EggGradeId == ManualGradeId); + Assert.Equal(600, manualLot.QuantityProduced); + }); + Assert.Equal(20, await CrackedLotQuantity(accountId, entryId, crackedId)); + } + + // A day with no cracked eggs is an ordinary good day, and it has a snapshot + // but NO lot. Adjusting such an entry puts a target of zero in front of the + // new-lot loop with no lot to match it — and EggLot.Create rejects a zero + // quantity, so without the skip the adjustment throws instead of applying. + // + // Written because the mutation check caught the guard surviving: every other + // test here submits a positive counter, so the zero-with-no-lot path was + // never exercised and the guard was, at that point, an untested claim. + [Fact] + public async Task Adjusting_a_day_that_never_had_cracked_eggs_still_succeeds() + { + var (client, accountId, farmId, flockId, manual, crackedId, _) = await SetupAsync(); + ManualGradeId = manual["Large"]; + + var entryId = (await (await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), + new + { + farmId, + houseId = Guid.NewGuid(), + flockId, + date = DateOnly.FromDateTime(DateTime.UtcNow.Date), + totalEggs = 608, + crackedEggs = 0, + dirtyEggs = 5, + discardedEggs = 3, + mortalityCount = 0, + grades = new[] { new { eggGradeId = ManualGradeId, quantity = 600 } } + })).Content.ReadFromJsonAsync())!.Id; + + Assert.Equal(HttpStatusCode.OK, (await client.PostWithKeyAsync( + $"/api/v1/daily-entries/{entryId}/submit", Guid.NewGuid().ToString())).StatusCode); + + var version = (await StateOf(accountId, entryId)).Version; + var response = await AdjustAsync(client, entryId, version, cracked: 0, total: 608); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(0, await CrackedLotQuantity(accountId, entryId, crackedId)); + } + + private async Task<(string Status, int Version, int Lots)> StateOf(Guid accountId, Guid entryId) + { + (string, int, int) state = default; + await factory.WithTenantScopeAsync(accountId, async db => + { + var entry = await db.DailyEntries.IgnoreQueryFilters().SingleAsync(e => e.Id == entryId); + var lots = await db.EggLots.IgnoreQueryFilters().CountAsync(l => l.DailyEntryId == entryId); + state = (entry.Status.ToString(), entry.Version, lots); + }); + return state; + } +} diff --git a/tests/Cluckwork.Api.IntegrationTests/ReportsTests.cs b/tests/Cluckwork.Api.IntegrationTests/ReportsTests.cs index 97769a70d..2c2544919 100644 --- a/tests/Cluckwork.Api.IntegrationTests/ReportsTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/ReportsTests.cs @@ -12,11 +12,11 @@ public sealed class ReportsTests(CluckworkWebApplicationFactory factory) private sealed record Created(Guid Id); private sealed record DayRow( DateOnly Date, int TotalEggs, int Cracked, int Dirty, int Discarded, - int Sellable, int Deaths, long HenDays, decimal? HenDayPct); + int Sellable, int FromCounts, int Deaths, long HenDays, decimal? HenDayPct); private sealed record GradeRow(Guid EggGradeId, string Name, int Quantity); private sealed record ProductionDto( - List Days, int TotalEggs, int TotalSellable, int TotalDeaths, - long TotalHenDays, decimal? PeriodHenDayPct, List GradeTotals); + List Days, int TotalEggs, int TotalSellable, int TotalFromCounts, + int TotalDeaths, long TotalHenDays, decimal? PeriodHenDayPct, List GradeTotals); private sealed record SalesDto( int ConfirmedCount, long RevenueMinorUnits, long PaidMinorUnits, long OutstandingMinorUnits, int VoidedCount, string CurrencyCode, int CurrencyMinorUnit); @@ -26,6 +26,87 @@ private sealed record ProfitDto( private static readonly DateOnly Today = DateOnly.FromDateTime(DateTime.UtcNow.Date); + // #396 — Cracked and Dirty can now become stock, so the report has to say + // how many eggs the day produced WITHOUT being hand-graded. Sellable keeps + // its meaning (the hand-graded remainder, and the target Daily Entry counts + // down to); FromCounts is the separate figure, so the two are never + // conflated and the period totals stay addable. + // + // Only SNAPSHOT-BACKED conditions count. This fixture makes Cracked saleable + // and Dirty NOT, so a rule that simply added both counters would report 9 + // where the farm can only sell 6 — inventing stock that was recorded as a + // loss. + [Fact] + public async Task Production_CountsOnlySnapshotBackedConditionsAsFromCounts() + { + var email = $"u-{Guid.NewGuid():N}@test.local"; + var accountId = await factory.SeedAccountWithUserAsync(email); + var farmId = Guid.NewGuid(); + var grades = await factory.SeedEggGradesAsync(accountId, farmId, "Large"); + var flockId = await factory.SeedFlockAsync(accountId, farmId); + + await factory.WithTenantScopeAsync(accountId, async db => + { + db.EggGrades.Add(Domain.Eggs.EggGrade.Create( + Guid.NewGuid(), accountId, farmId, "Cracked", Domain.Eggs.EggGradeType.Quality, + 60, isSaleable: true, dailyEntryKind: Domain.Eggs.DailyEntryKind.Cracked)); + db.EggGrades.Add(Domain.Eggs.EggGrade.Create( + Guid.NewGuid(), accountId, farmId, "Dirty", Domain.Eggs.EggGradeType.Quality, + 61, isSaleable: false, dailyEntryKind: Domain.Eggs.DailyEntryKind.Dirty)); + await db.SaveChangesAsync(); + }); + + var client = factory.CreateAuthedClient(await factory.LoginForAccessTokenAsync(email)); + + // 100 = 90 hand-graded + 6 cracked + 3 dirty + 1 discarded. + var entryId = (await (await client.PostWithKeyAsync( + "/api/v1/daily-entries", Guid.NewGuid().ToString(), new + { + farmId, + houseId = Guid.NewGuid(), + flockId, + date = Today, + totalEggs = 100, + crackedEggs = 6, + dirtyEggs = 3, + discardedEggs = 1, + mortalityCount = 0, + grades = new[] { new { eggGradeId = grades["Large"], quantity = 90 } } + })).Content.ReadFromJsonAsync())!.Id; + await client.PostWithKeyAsync($"/api/v1/daily-entries/{entryId}/submit", Guid.NewGuid().ToString()); + + var report = await client.GetFromJsonAsync( + $"/api/v1/reports/production?from={Today:yyyy-MM-dd}&to={Today:yyyy-MM-dd}"); + + var day = Assert.Single(report!.Days); + Assert.Equal(100, day.TotalEggs); + Assert.Equal(90, day.Sellable); // unchanged: the hand-graded remainder + Assert.Equal(6, day.FromCounts); // cracked only — dirty stayed a loss + Assert.Equal(90, report.TotalSellable); + Assert.Equal(6, report.TotalFromCounts); + + // #396 (codex review of #407): the "By grade" breakdown has to account + // for the condition stock this same response just reported as produced. + // Condition production creates an EggLot but NEVER a DailyEntryGrade row + // — ConditionGradeGuard refuses a manual line naming a condition grade, + // so one can never exist — and a breakdown built from DailyEntryGrades + // alone therefore omitted every cracked egg while the header counted it. + Assert.Equal(90, Assert.Single(report.GradeTotals, g => g.Name == "Large").Quantity); + Assert.Equal(6, Assert.Single(report.GradeTotals, g => g.Name == "Cracked").Quantity); + + // Dirty was non-saleable, so it resolved to nothing and stayed a loss. + // Its absence is the other half of the guarantee: folding the counters + // in must not resurrect the ones the entry recorded as losses. + Assert.DoesNotContain(report.GradeTotals, g => g.Name == "Dirty"); + + // The whole point, stated as the arithmetic a reader of the screen does: + // the breakdown sums to the hand-graded remainder PLUS the condition + // stock, not to the remainder alone. + Assert.Equal( + report.TotalSellable + report.TotalFromCounts, + report.GradeTotals.Sum(g => g.Quantity)); + } + [Fact] public async Task Production_HenDays_OfficialEntriesOnly() { diff --git a/tests/Cluckwork.Api.IntegrationTests/SimulationSeederTests.cs b/tests/Cluckwork.Api.IntegrationTests/SimulationSeederTests.cs index a85143dd9..87b31bac5 100644 --- a/tests/Cluckwork.Api.IntegrationTests/SimulationSeederTests.cs +++ b/tests/Cluckwork.Api.IntegrationTests/SimulationSeederTests.cs @@ -362,7 +362,14 @@ public async Task SimulationSeed_SeedsProductCatalogAndCustomers() var products = await db.Products.IgnoreQueryFilters() .Where(p => p.AccountId == SeedDefaults.AccountId) .ToListAsync(); - foreach (var name in new[] { "Sim Large Eggs", "Sim Medium Eggs", "Sim Small Eggs" }) + // #396: Cracked/Dirty are saleable grades that submit now mints lots + // for, so the catalog covers them too — without a product those lots + // are stock nothing can sell. + foreach (var name in new[] + { + "Sim Large Eggs", "Sim Medium Eggs", "Sim Small Eggs", + "Sim Cracked Eggs", "Sim Dirty Eggs", + }) Assert.Contains(products, p => p.Name == name); var customers = await db.Customers.IgnoreQueryFilters() @@ -471,12 +478,13 @@ public async Task SimulationSeed_Sales_IsIdempotent_NoDuplicatesAfterTwoHostStar .ToListAsync(); // A second full seed pass converges rather than doubling — same - // three products, three customers, six orders (2 draft + 2 + // five products (#396 added the two condition grades), three + // customers, six orders (2 draft + 2 // confirmed-unpaid + 1 confirmed-partially-paid + 1 recurring // confirmed, #243 Task 3d's RecurringStartDay/RecurringCadenceDays // drip — exactly one point lands inside a 12-day HistoryDays window) // as a single pass. - Assert.Equal(3, products.Count); + Assert.Equal(5, products.Count); Assert.Equal(3, customers.Count); Assert.Equal(6, orders.Count); Assert.DoesNotContain(products.GroupBy(p => p.Name), g => g.Count() > 1); @@ -813,6 +821,13 @@ private static async Task ReadManifestAsync(string path) private const int ExpectedDraftWindowDays = 2; private const int ExpectedGradesPerDailyEntry = 3; + // #396: submit also mints a lot for each condition counter that resolved to + // a condition grade. Kept as its own constant, mirroring the seeder's + // ConditionLotsPerDailyEntry — the two answer different questions (grade + // LINES vs counter-backed lots) and a single merged "5" would hide which + // half moved if either changes. + private const int ExpectedConditionLotsPerDailyEntry = 2; + [Fact] public async Task SimulationSeed_EmitsACompleteManifestWithValidatedCountsAndLifecycleStates() { @@ -846,9 +861,13 @@ public async Task SimulationSeed_EmitsACompleteManifestWithValidatedCountsAndLif // #279 review Fix 2: one lot per grade (Large/Medium/Small) for every // entry that reached Submitted — Draft entries (the most recent // ExpectedDraftWindowDays per flock) never call SubmitDailyEntryHandler, - // so they mint no lots. + // so they mint no lots. #396 adds the two condition lots (Cracked and + // Dirty), which the seeder's own deterministic counter math keeps + // strictly positive on every entry, so they are minted on exactly the + // same population. Assert.Equal( - ExpectedGradesPerDailyEntry * 2 * (SimulationSeedFactory.HistoryDays - ExpectedDraftWindowDays), + (ExpectedGradesPerDailyEntry + ExpectedConditionLotsPerDailyEntry) + * 2 * (SimulationSeedFactory.HistoryDays - ExpectedDraftWindowDays), counts.EggLots); Assert.Equal(2 + ExpectedConfirmedOrders, counts.SalesOrdersTotal); Assert.Equal(1, counts.Payments); diff --git a/tests/Cluckwork.Domain.Tests/Eggs/DailyEntryTests.cs b/tests/Cluckwork.Domain.Tests/Eggs/DailyEntryTests.cs index 311857e55..857873321 100644 --- a/tests/Cluckwork.Domain.Tests/Eggs/DailyEntryTests.cs +++ b/tests/Cluckwork.Domain.Tests/Eggs/DailyEntryTests.cs @@ -11,6 +11,72 @@ private static DailyEntry MakeDraft() => private static readonly Guid GradeLarge = Guid.NewGuid(); private static readonly Guid GradeMedium = Guid.NewGuid(); + // ---- #396 quality-condition snapshots ------------------------------- + // + // An official entry records WHICH grade its Cracked and Dirty counters + // resolved to, so a later catalog change cannot reinterpret a past day. + // The domain does not resolve them — it has no catalog — so the caller + // passes the ids it resolved (both Active and IsSaleable, or null). + + private static readonly Guid GradeCracked = Guid.NewGuid(); + private static readonly Guid GradeDirty = Guid.NewGuid(); + + [Fact] + public void Draft_has_no_quality_snapshots() + { + var entry = MakeDraft(); + entry.RecordProduction(100, 10, 5, 0, 0, [new GradeQuantity(GradeLarge, 85)]); + + Assert.Null(entry.CrackedGradeId); + Assert.Null(entry.DirtyGradeId); + } + + [Fact] + public void Submit_records_the_resolved_quality_grades() + { + var entry = MakeDraft(); + entry.RecordProduction(100, 10, 5, 0, 0, [new GradeQuantity(GradeLarge, 85)]); + + Assert.True(entry.Submit(GradeCracked, GradeDirty).IsSuccess); + + Assert.Equal(GradeCracked, entry.CrackedGradeId); + Assert.Equal(GradeDirty, entry.DirtyGradeId); + } + + [Fact] + public void Submit_records_null_for_a_condition_that_is_a_loss() + { + // Non-saleable, or inactive: either way the caller resolved nothing, + // and null is the durable record that this day's cracked eggs were a + // loss — not an absence of information to be re-derived later. + var entry = MakeDraft(); + entry.RecordProduction(100, 10, 5, 0, 0, [new GradeQuantity(GradeLarge, 85)]); + + entry.Submit(crackedGradeId: null, dirtyGradeId: GradeDirty); + + Assert.Null(entry.CrackedGradeId); + Assert.Equal(GradeDirty, entry.DirtyGradeId); + } + + [Fact] + public void Adjust_keeps_the_original_snapshot_and_never_re_resolves() + { + // The load-bearing one. Submission is the ONLY resolution point: if an + // adjustment re-resolved, a farm that turned Cracked non-saleable (or + // deactivated it) between submit and correction would see a corrected + // day silently stop counting eggs it had already banked as stock — or + // the reverse, minting lots for a day that was recorded as a loss. + var entry = MakeDraft(); + entry.RecordProduction(100, 10, 5, 0, 0, [new GradeQuantity(GradeLarge, 85)]); + entry.Submit(GradeCracked, GradeDirty); + + var result = entry.ManagerAdjust(120, 10, 5, 0, 0, "recount", [new GradeQuantity(GradeLarge, 105)]); + + Assert.True(result.IsSuccess); + Assert.Equal(GradeCracked, entry.CrackedGradeId); + Assert.Equal(GradeDirty, entry.DirtyGradeId); + } + [Fact] public void RecordProduction_OnDraft_Succeeds() { diff --git a/tests/Cluckwork.Domain.Tests/Eggs/EggGradeTests.cs b/tests/Cluckwork.Domain.Tests/Eggs/EggGradeTests.cs new file mode 100644 index 000000000..1802720de --- /dev/null +++ b/tests/Cluckwork.Domain.Tests/Eggs/EggGradeTests.cs @@ -0,0 +1,67 @@ +namespace Cluckwork.Domain.Tests.Eggs; + +using Cluckwork.Domain.Eggs; + +// #396 — a Daily Entry's Cracked and Dirty counters feed a specific grade, and +// which grade that is must survive the farm renaming it. `DailyEntryKind` is +// that stable binding: an identity the code can resolve, as opposed to matching +// on the mutable `Name` (a farm renaming "Cracked" to "Segunda" would otherwise +// silently detach its own counter from its own grade). +public sealed class EggGradeTests +{ + private static EggGrade Make( + string name = "Large", + EggGradeType type = EggGradeType.Size, + bool saleable = true, + DailyEntryKind kind = DailyEntryKind.Manual) => + EggGrade.Create( + Guid.NewGuid(), Guid.NewGuid(), Guid.NewGuid(), name, + type, sortOrder: 0, isSaleable: saleable, dailyEntryKind: kind); + + [Fact] + public void Create_defaults_to_the_manual_kind() + { + // Every ordinary grade is graded by hand in the Grading pane. Only the + // two condition grades are fed by a counter, so Manual is the default + // and a condition kind is always a deliberate act. + Assert.Equal(DailyEntryKind.Manual, Make().DailyEntryKind); + } + + [Theory] + [InlineData(DailyEntryKind.Cracked)] + [InlineData(DailyEntryKind.Dirty)] + public void Create_carries_a_condition_kind(DailyEntryKind kind) => + Assert.Equal(kind, Make("Cracked", EggGradeType.Quality, kind: kind).DailyEntryKind); + + [Fact] + public void Update_cannot_change_the_kind() + { + // Same reasoning as GradeType's immutability, and stronger: an entry + // that already snapshotted this grade as its Cracked identity would be + // reinterpreted by a later kind change. Update takes no kind parameter + // at all — this pins that the signature cannot quietly grow one. + var grade = Make("Cracked", EggGradeType.Quality, kind: DailyEntryKind.Cracked); + + var result = grade.Update("Segunda", sortOrder: 3, isSaleable: false); + + Assert.True(result.IsSuccess); + Assert.Equal("Segunda", grade.Name); + Assert.Equal(DailyEntryKind.Cracked, grade.DailyEntryKind); + } + + [Fact] + public void Deactivate_leaves_saleability_alone() + { + // The trap #397's review round 3 caught, pinned at the source: the two + // flags move independently, so "inactive but still saleable" is an + // ordinary reachable state. Every consumer resolving a condition grade + // must therefore require BOTH, and this test is what makes that + // requirement visible rather than folklore. + var grade = Make("Cracked", EggGradeType.Quality, kind: DailyEntryKind.Cracked); + + Assert.True(grade.Deactivate().IsSuccess); + + Assert.False(grade.Active); + Assert.True(grade.IsSaleable); + } +} diff --git a/web/src/api/cluckwork.ts b/web/src/api/cluckwork.ts index 0ad5770dd..30316389a 100644 --- a/web/src/api/cluckwork.ts +++ b/web/src/api/cluckwork.ts @@ -8,6 +8,15 @@ export interface EggGrade { gradeType: string; sortOrder: number; isSaleable: boolean; + /** + * #396 — "Manual" | "Cracked" | "Dirty". Which Daily Entry input feeds this + * grade. Not inferable from `name` (renameable) or `gradeType` (a farm can + * have many Quality grades, only one of which is the Cracked counter's). + * + * Used to keep the two counter-fed grades out of the Grading pane. That is an + * affordance only — the server refuses a manual line naming one regardless. + */ + dailyEntryKind: string; active: boolean; } @@ -51,6 +60,17 @@ export interface DailyEntry { dirtyEggs: number; discardedEggs: number; mortalityCount: number; + /** + * #396 — which grade each condition counter resolved to when this entry + * became official; null when that condition was a loss (and on any draft, + * which has not resolved yet — tell the two apart by `status`). + * + * The only way to know whether a past day's cracked eggs became stock: the + * current grade catalog cannot answer it, because the farm may have changed + * a grade's saleability since. + */ + crackedGradeId: string | null; + dirtyGradeId: string | null; grades: GradeLine[]; version: number; adjustReason: string | null; @@ -729,7 +749,15 @@ export interface ProductionDay { cracked: number; dirty: number; discarded: number; + /** Hand-graded remainder: total − cracked − dirty − discarded (#394). */ sellable: number; + /** + * #396 — eggs that became stock WITHOUT being hand-graded: the cracked and + * dirty counters, but only where that entry resolved the condition to a + * grade. Separate from `sellable` on purpose; the two answer different + * questions and were only ever equal while conditions were always losses. + */ + fromCounts: number; deaths: number; henDays: number; henDayPct: number | null; @@ -739,6 +767,7 @@ export interface ProductionReport { days: ProductionDay[]; totalEggs: number; totalSellable: number; + totalFromCounts: number; totalDeaths: number; totalHenDays: number; periodHenDayPct: number | null; diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index 2c9250613..b73030a90 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -1498,6 +1498,10 @@ export const en = { statusHeader: "Status", totalHeader: "Total", lossesHeader: "Losses (cr/di/ds)", + // #396 — Losses counts cracked/dirty/discarded whatever became of them; + // this counts only the ones that became stock, per the entry's snapshot. + // Em dash on a draft: nothing has resolved yet, and 0 would claim "loss". + conditionHeader: "Condition", mortalityHeader: "Mortality", gradedHeader: "Graded", // Lowercase link-styled row actions, same treatment as @@ -1544,6 +1548,12 @@ export const en = { eggsHeader: "Eggs", lossesHeader: "Losses (cr/di/ds)", sellableHeader: "Sellable", + // #396 — cracked/dirty eggs that became stock instead of being written off. + // "Condition" is the term the specs and glossary already use for those two + // grades; deliberately NOT "ungraded" (they do carry a grade, shown in the + // By-grade breakdown on this same screen) and NOT "seconds" (a real seeded + // grade name, which would make the column and the breakdown disagree). + conditionHeader: "Condition", deathsHeader: "Deaths", henDaysHeader: "Hen-days", henDayPctHeader: "Hen-day %", @@ -2169,9 +2179,19 @@ export const en = { // Reports reportsHeading: "Reports", reportsProduction: - "Production (everyone): pick a date range — per-day eggs, losses, sellable, deaths, " - + "and hen-day % (eggs collected ÷ birds alive that day × 100), with period totals and " - + "a by-grade breakdown. Draft and voided entries don't count.", + "Production (everyone): pick a date range — per-day eggs, losses, sellable, " + + "condition, deaths, and hen-day % (eggs collected ÷ birds alive that day × 100), " + + "with period totals and a by-grade breakdown. Draft and voided entries don't count.", + // #396 — the new column needs explaining because two of its neighbours look + // like they should already cover it: Losses shows the same cracked/dirty + // numbers (as counts, whether sold or not), and Sellable is the graded + // figure that deliberately excludes them. + reportsCondition: + "Condition: cracked and dirty eggs that became stock instead of being written off. " + + "They are counted in Egg counts, never graded by hand, so they are not part of " + + "Sellable — add the two together for everything the day produced that you can " + + "sell. A day shows 0 here if you have those grades switched off in Settings, and days recorded " + + "before you switched them on keep showing 0 — turning a grade on never rewrites a past day.", reportsMoney: "Money (admin): sales summary for the range's orders (revenue / paid / outstanding), " + "expenses by category, and basic profit — confirmed revenue minus recorded expenses, " diff --git a/web/src/i18n/es.ts b/web/src/i18n/es.ts index f97ebdedb..0e6c076fe 100644 --- a/web/src/i18n/es.ts +++ b/web/src/i18n/es.ts @@ -1223,6 +1223,8 @@ export const es = { // Spanish words (ag/su/de) — confirm this reads clearly as a // table-header abbreviation. lossesHeader: "Pérdidas (ag/su/de)", + // #396 — machine-drafted, pending native review (#182). + conditionHeader: "Con defecto", mortalityHeader: "Mortalidad", gradedHeader: "Clasificado", editButton: "editar", @@ -1254,6 +1256,9 @@ export const es = { eggsHeader: "Huevos", lossesHeader: "Pérdidas (ag/su/de)", sellableHeader: "Vendible", + // #396 — huevos rajados/sucios que pasaron a inventario en vez de perderse. + // Machine-drafted, pending native review (#182). + conditionHeader: "Con defecto", deathsHeader: "Muertes", henDaysHeader: "Días-gallina", henDayPctHeader: "% días-gallina", @@ -1839,9 +1844,17 @@ export const es = { reportsHeading: "Informes", reportsProduction: "Producción (todos): elija un rango de fechas — huevos por día, pérdidas, " - + "vendibles, muertes, y % de puesta diaria (huevos recolectados ÷ aves vivas ese " - + "día × 100), con totales del período y un desglose por grado. Las entradas en borrador y anuladas " - + "no cuentan.", + + "vendibles, con defecto, muertes, y % de puesta diaria (huevos recolectados ÷ " + + "aves vivas ese día × 100), con totales del período y un desglose por grado. Las entradas en " + + "borrador y anuladas no cuentan.", + // #396 — machine-drafted, pending native review (#182). + reportsCondition: + "Con defecto: huevos rajados y sucios que pasaron a inventario en vez de " + + "registrarse como pérdida. Se cuentan en Conteo de huevos, nunca se clasifican a " + + "mano, así que no forman parte de Vendible — sume ambos para obtener todo lo que " + + "el día produjo y usted puede vender. El día muestra 0 si esos grados están desactivados en " + + "Ajustes, y los días registrados antes de activarlos siguen mostrando 0: activar un grado nunca " + + "reescribe un día pasado.", reportsMoney: "Dinero (admin): resumen de ventas de los pedidos del rango (ingresos / pagado / " + "pendiente), gastos por categoría, y ganancia básica — ingresos confirmados menos " diff --git a/web/src/i18n/tl.ts b/web/src/i18n/tl.ts index 629a0f348..b5977ba1a 100644 --- a/web/src/i18n/tl.ts +++ b/web/src/i18n/tl.ts @@ -1260,6 +1260,8 @@ export const tl = { // Filipino words (basag/marumi/tapon → b/m/t) — confirm this reads // clearly as a table-header abbreviation. lossesHeader: "Nasira (b/m/t)", + // #396 — machine-drafted, pending native review (#182). + conditionHeader: "May depekto", mortalityHeader: "Mortalidad", gradedHeader: "Na-grade", editButton: "i-edit", @@ -1296,6 +1298,9 @@ export const tl = { eggsHeader: "Itlog", lossesHeader: "Nasira (b/m/t)", sellableHeader: "Maibebenta", + // #396 — basag/marumi na itlog na naging stock sa halip na malugi. + // Machine-drafted, pending native review (#182). + conditionHeader: "May depekto", deathsHeader: "Namatay", henDaysHeader: "Hen-days", henDayPctHeader: "Hen-day %", @@ -1910,8 +1915,17 @@ export const tl = { reportsHeading: "Mga Report", reportsProduction: "Production (lahat): pumili ng date range — itlog kada araw, nawala, naibibenta, " - + "namatay, at hen-day % (itlog na nakolekta ÷ ibong buhay sa araw na iyon × 100), " - + "may kasamang period total at breakdown kada grado. Hindi binibilang ang draft at na-void na entry.", + + "may depekto, namatay, at hen-day % (itlog na nakolekta ÷ ibong buhay sa araw na " + + "iyon × 100), may kasamang period total at breakdown kada grado. Hindi binibilang ang draft at " + + "na-void na entry.", + // #396 — machine-drafted, pending native review (#182). + reportsCondition: + "May depekto: basag at maruming itlog na naging stock sa halip na ituring na " + + "lugi. Binibilang ang mga ito sa Bilang ng itlog, hindi kailanman gine-grade sa " + + "kamay, kaya wala ang mga ito sa Maibebenta — pagsamahin ang dalawa para makuha " + + "ang lahat ng naibebenta sa araw na iyon. Magpapakita ng 0 kung naka-off ang mga gradong iyon sa " + + "Settings, at ang mga araw na naitala bago mo ito i-on ay mananatiling 0 — ang pag-on ng grado ay " + + "hindi kailanman binabago ang nakaraang araw.", reportsMoney: "Pera (admin): buod ng benta para sa mga order ng range (kita / nabayaran / " + "natitira), gastos kada kategorya, at basic profit — nakumpirmang kita bawas ang " diff --git a/web/src/routes/DailyEntryPage.test.tsx b/web/src/routes/DailyEntryPage.test.tsx index 478f3be56..140fc8e9f 100644 --- a/web/src/routes/DailyEntryPage.test.tsx +++ b/web/src/routes/DailyEntryPage.test.tsx @@ -32,9 +32,15 @@ const FLOCK: Flock = { placementDate: "2026-01-01", initialCount: 100, currentBirds: 98, status: "Active", }; const GRADES: EggGrade[] = [ - { id: "gr1", farmId: "farm1", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, active: true }, - { id: "gr2", farmId: "farm1", name: "Grade B", gradeType: "Size", sortOrder: 2, isSaleable: true, active: true }, + { id: "gr1", farmId: "farm1", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, dailyEntryKind: "Manual", active: true }, + { id: "gr2", farmId: "farm1", name: "Grade B", gradeType: "Size", sortOrder: 2, isSaleable: true, dailyEntryKind: "Manual", active: true }, ]; +// #396 — saleable AND active, so it passes every pre-existing filter. The only +// thing that keeps it out of the Grading pane is its kind. +const CRACKED: EggGrade = { + id: "gr-cracked", farmId: "farm1", name: "Cracked", gradeType: "Quality", + sortOrder: 3, isSaleable: true, dailyEntryKind: "Cracked", active: true, +}; beforeEach(() => { vi.clearAllMocks(); @@ -73,6 +79,35 @@ async function renderReady() { expect(mockListDailyEntries).toHaveBeenCalled(); // prefill really ran } +// #396 — Cracked and Dirty are fed by their own counters in Egg counts, so they +// must never appear as grade fields in the Grading pane. Their counter already +// produces a lot; a manual line naming one would produce a SECOND lot for the +// same grade on the same day. The server refuses it outright, so this is the +// affordance — the screen not offering a control whose only outcome is a +// rejection. +describe("DailyEntryPage grading pane excludes counter-fed grades", () => { + it("offers no grade field for a saleable, active condition grade", async () => { + mockListEggGrades.mockResolvedValue([...GRADES, CRACKED]); + await renderReady(); + + // Scoped to the GRADING pane. "Cracked" is a deliberately ambiguous label + // on this screen: Egg counts has a Cracked *counter*, which must stay — an + // unscoped query matches that and passes whatever the Grading pane does. + // (The first version of this test did exactly that and had to be tightened.) + const grading = screen.getByRole("heading", { name: /Grading/ }) + .closest("section") as HTMLElement; + + expect(within(grading).getByLabelText("Grade A")).toBeInTheDocument(); + expect(within(grading).getByLabelText("Grade B")).toBeInTheDocument(); + expect(within(grading).queryByLabelText("Cracked")).not.toBeInTheDocument(); + + // ...while the counter it IS fed by stays where it belongs. + const counts = screen.getByRole("heading", { name: /Egg counts/ }) + .closest("section") as HTMLElement; + expect(within(counts).getByLabelText("Cracked")).toBeInTheDocument(); + }); +}); + describe("DailyEntryPage accuracy gating", () => { // #394: grading must reconcile EXACTLY to sellable before submit — "within // sellable" (the old gate) is no longer enough. This used to be the case @@ -194,7 +229,7 @@ describe("DailyEntryPage prefill gating", () => { it("locks the form when the day already has a submitted entry", async () => { const existing: DailyEntry = { id: "de1", farmId: "farm1", houseId: "h1", flockId: "f1", date: todayIso(), status: "Submitted", - totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, + totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, crackedGradeId: null, dirtyGradeId: null, grades: [{ eggGradeId: "gr1", quantity: 60 }, { eggGradeId: "gr2", quantity: 25 }], version: 1, adjustReason: null, voidReason: null, lockedAtUtc: "2026-07-20T10:00:00Z", adjustedFrom: null, }; @@ -217,7 +252,7 @@ describe("DailyEntryPage prefill gating", () => { it("shows the harmonized 'adjusted' label, not the raw status, for a manager-adjusted day", async () => { const existing: DailyEntry = { id: "de1", farmId: "farm1", houseId: "h1", flockId: "f1", date: todayIso(), status: "ManagerAdjusted", - totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, + totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, crackedGradeId: null, dirtyGradeId: null, grades: [{ eggGradeId: "gr1", quantity: 60 }, { eggGradeId: "gr2", quantity: 25 }], version: 1, adjustReason: null, voidReason: null, lockedAtUtc: "2026-07-20T10:00:00Z", adjustedFrom: null, }; @@ -398,7 +433,7 @@ describe("DailyEntryPage structure", () => { describe("DailyEntryPage draft badge", () => { const draftFor = (date: string) => ({ id: "de1", farmId: "farm1", houseId: "h1", flockId: "f1", date, status: "Draft", - totalEggs: 40, crackedEggs: 1, dirtyEggs: 0, discardedEggs: 0, mortalityCount: 0, + totalEggs: 40, crackedEggs: 1, dirtyEggs: 0, discardedEggs: 0, mortalityCount: 0, crackedGradeId: null, dirtyGradeId: null, grades: [], version: 1, adjustReason: null, voidReason: null, lockedAtUtc: null, adjustedFrom: null, } as DailyEntry); @@ -832,7 +867,7 @@ describe("DailyEntryPage i18n wiring (#182, Task 11)", () => { it("interpolates the enum-labelled status into the entry-locked banner from the catalog", async () => { const existing: DailyEntry = { id: "de1", farmId: "farm1", houseId: "h1", flockId: "f1", date: todayIso(), status: "Submitted", - totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, + totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, crackedGradeId: null, dirtyGradeId: null, grades: [], version: 1, adjustReason: null, voidReason: null, lockedAtUtc: "2026-07-20T10:00:00Z", adjustedFrom: null, }; mockListDailyEntries.mockResolvedValue([existing]); diff --git a/web/src/routes/DailyEntryPage.tsx b/web/src/routes/DailyEntryPage.tsx index f616c2e39..e1ae450c3 100644 --- a/web/src/routes/DailyEntryPage.tsx +++ b/web/src/routes/DailyEntryPage.tsx @@ -94,7 +94,13 @@ export function DailyEntryPage() { .then(([all, g]) => { const f = capturable(all); setFlocks(f); - setGrades(g.filter((x) => x.isSaleable)); + // #396 — saleable AND hand-graded. Cracked and Dirty are saleable now, + // so filtering on isSaleable alone would put them in the Grading pane; + // their counters already produce a lot, and a manual line naming one + // would produce a second lot for the same grade on the same day. The + // server refuses that outright (ConditionGradeGuard) — this keeps the + // screen from offering a control whose only outcome is a rejection. + setGrades(g.filter((x) => x.isSaleable && x.dailyEntryKind === "Manual")); // Deep link from History's Draft "edit" (#85): ?flockId=…&date=…. // The pair is applied atomically — applying only the date against a // fallback flock would open a DIFFERENT flock's day under the linked diff --git a/web/src/routes/Dashboard.test.tsx b/web/src/routes/Dashboard.test.tsx index afdafbea2..ab87a5df9 100644 --- a/web/src/routes/Dashboard.test.tsx +++ b/web/src/routes/Dashboard.test.tsx @@ -33,7 +33,8 @@ const flock = (id: string, status: string): Flock => ({ }); const entry = (flockId: string, status: string, totalEggs: number): DailyEntry => ({ id: `de-${flockId}`, farmId: "f", houseId: "h", flockId, date: "2026-07-21", status, - totalEggs, crackedEggs: 0, dirtyEggs: 0, discardedEggs: 0, mortalityCount: 0, grades: [], + totalEggs, crackedEggs: 0, dirtyEggs: 0, discardedEggs: 0, mortalityCount: 0, + crackedGradeId: null, dirtyGradeId: null, grades: [], version: 1, adjustReason: null, voidReason: null, lockedAtUtc: null, adjustedFrom: null, }); const STOCK: StockRow[] = [ diff --git a/web/src/routes/GradesPage.test.tsx b/web/src/routes/GradesPage.test.tsx index 2f2cc74b0..bd85578c9 100644 --- a/web/src/routes/GradesPage.test.tsx +++ b/web/src/routes/GradesPage.test.tsx @@ -24,8 +24,8 @@ const mockUpdate = vi.mocked(updateEggGrade); const mockDeactivate = vi.mocked(deactivateEggGrade); const mockActivate = vi.mocked(activateEggGrade); -const GRADE_A: EggGrade = { id: "g1", farmId: "f", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, active: true }; -const GRADE_OLD: EggGrade = { id: "g2", farmId: "f", name: "Legacy", gradeType: "Quality", sortOrder: 2, isSaleable: false, active: false }; +const GRADE_A: EggGrade = { id: "g1", farmId: "f", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, dailyEntryKind: "Manual", active: true }; +const GRADE_OLD: EggGrade = { id: "g2", farmId: "f", name: "Legacy", gradeType: "Quality", sortOrder: 2, isSaleable: false, dailyEntryKind: "Manual", active: false }; const ADMIN = { sub: "u1", role: "Admin" }; const WORKER = { sub: "u1" }; diff --git a/web/src/routes/HelpPage.tsx b/web/src/routes/HelpPage.tsx index 08cef7e07..b3f3798f1 100644 --- a/web/src/routes/HelpPage.tsx +++ b/web/src/routes/HelpPage.tsx @@ -318,6 +318,9 @@ export function HelpPage() {
  • }} />
    • +
  • + }} /> +
  • }} />
    • diff --git a/web/src/routes/HistoryPage.test.tsx b/web/src/routes/HistoryPage.test.tsx index 81975e608..ee48e3b84 100644 --- a/web/src/routes/HistoryPage.test.tsx +++ b/web/src/routes/HistoryPage.test.tsx @@ -32,13 +32,13 @@ const FLOCK: Flock = { placementDate: "2026-01-01", initialCount: 100, currentBirds: 98, status: "Active", }; const ARCHIVED_FLOCK: Flock = { ...FLOCK, id: "f2", name: "Old Coop", status: "Archived" }; -const GRADE_A: EggGrade = { id: "gr1", farmId: "farm1", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, active: true }; -const GRADE_B: EggGrade = { id: "gr2", farmId: "farm1", name: "Grade B", gradeType: "Size", sortOrder: 2, isSaleable: true, active: true }; +const GRADE_A: EggGrade = { id: "gr1", farmId: "farm1", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, dailyEntryKind: "Manual", active: true }; +const GRADE_B: EggGrade = { id: "gr2", farmId: "farm1", name: "Grade B", gradeType: "Size", sortOrder: 2, isSaleable: true, dailyEntryKind: "Manual", active: true }; // sellable = 100 − 2 − 3 − 5 = 90; two graded lines summing to 60 (within). const SUBMITTED: DailyEntry = { id: "de1", farmId: "farm1", houseId: "h1", flockId: "f1", date: "2026-07-19", status: "Submitted", - totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, + totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, crackedGradeId: null, dirtyGradeId: null, grades: [{ eggGradeId: "gr1", quantity: 40 }, { eggGradeId: "gr2", quantity: 20 }], version: 1, adjustReason: null, voidReason: null, lockedAtUtc: null, adjustedFrom: null, }; @@ -68,6 +68,77 @@ async function openAdjustPanel() { fireEvent.click(await screen.findByRole("button", { name: "adjust" })); } +// #396 — the Condition column answers "how many of this day's cracked/dirty +// eggs became stock", read from the ENTRY's own snapshot. It must never be +// re-derived from the current grade catalog: a farm that switches Cracked off +// today would otherwise see past days lose stock they already sold. +// Columns: date, flock, status, total, losses, CONDITION, mortality, graded, +// actions. Indexed rather than matched by text because "0" and the em dash both +// occur in sibling cells, so a text query can pass against the wrong column. +const conditionCell = (row: HTMLElement) => within(row).getAllByRole("cell")[5]; + +describe("HistoryPage condition column", () => { + it("counts only the conditions this entry resolved to a grade", async () => { + // cracked 2 resolved (a grade id), dirty 3 did NOT (null) — so 2, not 5. + // Every number here is distinct, so a column wired to the wrong field, or + // one that sums the raw counters, produces a different value. + mockListDailyEntries.mockResolvedValue([ + { ...SUBMITTED, crackedGradeId: "gr1", dirtyGradeId: null }, + ]); + renderWithProviders(, { token: ADMIN }); + + const row = await screen.findByRole("row", { name: /2026-07-19/ }); + within(row).getByText("2/3/5"); // Losses still shows all three counters + // By CELL, not by text: "2" also appears inside the losses cell, so a text + // match would pass against a column that renders the wrong number. + expect(conditionCell(row)).toHaveTextContent("2"); + }); + + it("shows an em dash for a draft rather than 0", async () => { + // A draft has resolved nothing yet. 0 would state "these were a loss", + // which is a different fact and not yet true. + mockListDailyEntries.mockResolvedValue([DRAFT]); + renderWithProviders(, { token: ADMIN }); + + const row = await screen.findByRole("row", { name: /2026-07-18/ }); + expect(conditionCell(row)).toHaveTextContent("—"); + }); + + it("shows 0 for an official entry whose conditions were losses", async () => { + mockListDailyEntries.mockResolvedValue([SUBMITTED]); // both snapshots null + renderWithProviders(, { token: ADMIN }); + + const row = await screen.findByRole("row", { name: /2026-07-19/ }); + expect(conditionCell(row)).toHaveTextContent("0"); + }); +}); + +// #396 — the adjust dialog is the Daily entry form, so the same rule holds: +// a counter-fed grade is never offered for adding. Excluded from the CATALOG +// half only — an existing line stays correctable whatever it names. +describe("HistoryPage adjust panel excludes counter-fed grades", () => { + const CRACKED: EggGrade = { + id: "gr-cracked", farmId: "farm1", name: "Cracked", gradeType: "Quality", + sortOrder: 3, isSaleable: true, dailyEntryKind: "Cracked", active: true, + }; + + it("offers no grade field for a saleable, active condition grade", async () => { + mockListEggGrades.mockResolvedValue([GRADE_A, GRADE_B, CRACKED]); + mockListDailyEntries.mockResolvedValue([SUBMITTED]); + await openAdjustPanel(); + + const dialog = screen.getByRole("dialog"); + expect(within(dialog).getByLabelText("Grade A")).toBeInTheDocument(); + // Scoped to the dialog: the row behind it also renders the word, and the + // dialog's own Egg-counts half has a Cracked COUNTER that must stay. + expect(within(dialog).queryByLabelText("Cracked")).toBeInTheDocument(); + // ...so assert on the grade field specifically: the counter is the only + // "Cracked" control the dialog may show, and it is a count input, not a + // grade line. A grade field would make TWO. + expect(within(dialog).getAllByLabelText("Cracked")).toHaveLength(1); + }); +}); + describe("HistoryPage dialog dismissal", () => { it("closes the adjust dialog on Cancel without writing", async () => { mockListDailyEntries.mockResolvedValue([SUBMITTED]); @@ -172,6 +243,9 @@ describe("HistoryPage adjust — reconciliation guard", () => { const [id, body] = mockAdjustDailyEntry.mock.calls[0]; expect(id).toBe("de1"); expect(body).toMatchObject({ + // No crackedGradeId/dirtyGradeId here on purpose: an adjustment never + // re-resolves the condition grades, so the request does not carry them + // (#396). They live on the RESPONSE only. version: 1, totalEggs: 100, crackedEggs: 2, dirtyEggs: 3, discardedEggs: 5, mortalityCount: 1, reason: "recount", grades: [{ eggGradeId: "gr1", quantity: 45 }, { eggGradeId: "gr2", quantity: 45 }], }); diff --git a/web/src/routes/HistoryPage.tsx b/web/src/routes/HistoryPage.tsx index 3bc8bde80..d57e51d49 100644 --- a/web/src/routes/HistoryPage.tsx +++ b/web/src/routes/HistoryPage.tsx @@ -127,6 +127,20 @@ export function HistoryPage() { const correctable = (e: DailyEntry) => e.status === "Submitted" || e.status === "Locked" || e.status === "ManagerAdjusted"; + // #396 — how many of this day's cracked/dirty eggs became stock. Read from + // the ENTRY's snapshot, never from the current grade catalog: a farm that + // switched Cracked off after recording a day must still see that day as it + // was recorded, and one that switched it on must not see past losses + // retroactively turn into stock. + // + // A draft has resolved nothing yet, so it shows an em dash rather than 0 — + // "not decided" and "decided to be a loss" are different facts, and 0 would + // state the second while the first is true. + const conditionStock = (e: DailyEntry) => { + if (e.status === "Draft") return "—"; + return (e.crackedGradeId ? e.crackedEggs : 0) + (e.dirtyGradeId ? e.dirtyEggs : 0); + }; + function startAdjust(e: DailyEntry) { setAdjusting(e); setTotal(e.totalEggs); @@ -143,9 +157,17 @@ export function HistoryPage() { // The entry's own lines (possibly deactivated grades — still correctable) // plus the active saleable catalog for adding a missed grade. + // + // #396 — a counter-fed grade is never offered for adding. It is excluded from + // the CATALOG half only, not from the entry's own lines: an existing line must + // stay correctable whatever it names, and a condition grade cannot legitimately + // be on an entry anyway (ConditionGradeGuard refuses it server-side). Adding + // one here would ask the server for a second lot on a grade its counter + // already produced, and be rejected. function panelGrades(e: DailyEntry): EggGrade[] { const onEntry = new Set(e.grades.map((g) => g.eggGradeId)); - return grades.filter((g) => onEntry.has(g.id) || (g.active && g.isSaleable)); + return grades.filter((g) => + onEntry.has(g.id) || (g.active && g.isSaleable && g.dailyEntryKind === "Manual")); } // #394: an adjustment has no draft state of its own — it replaces the @@ -513,7 +535,12 @@ export function HistoryPage() { {t("dateHeader")}{t("flockHeader")}{t("statusHeader")}{t("totalHeader")} - {t("lossesHeader")}{t("mortalityHeader")}{t("gradedHeader")} + {t("lossesHeader")} + {/* #396 — Losses shows the cracked/dirty/discarded COUNTS + whatever became of them; this shows how many of those + actually became stock, per the entry's own snapshot. */} + {t("conditionHeader")} + {t("mortalityHeader")}{t("gradedHeader")} @@ -525,6 +552,7 @@ export function HistoryPage() { {statusCell(e)} {e.totalEggs} {e.crackedEggs}/{e.dirtyEggs}/{e.discardedEggs} + {conditionStock(e)} {e.mortalityCount} {e.grades.length === 0 diff --git a/web/src/routes/ProductsPage.test.tsx b/web/src/routes/ProductsPage.test.tsx index 44c1e9658..76b063636 100644 --- a/web/src/routes/ProductsPage.test.tsx +++ b/web/src/routes/ProductsPage.test.tsx @@ -43,8 +43,8 @@ const mockDeactivate = vi.mocked(deactivateProduct); const mockActivate = vi.mocked(activateProduct); const mockUpdateConversion = vi.mocked(updateEggUnitConversion); -const GRADE_A: EggGrade = { id: "g1", farmId: "f", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, active: true }; -const GRADE_B: EggGrade = { id: "g2", farmId: "f", name: "Grade B", gradeType: "Size", sortOrder: 2, isSaleable: true, active: true }; +const GRADE_A: EggGrade = { id: "g1", farmId: "f", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, dailyEntryKind: "Manual", active: true }; +const GRADE_B: EggGrade = { id: "g2", farmId: "f", name: "Grade B", gradeType: "Size", sortOrder: 2, isSaleable: true, dailyEntryKind: "Manual", active: true }; // KWD (3 decimals) throughout so a hard-coded 2-decimal money path fails: 500 // minor units renders "0.500 KWD" and typed "0.5" parses to 500 (not 50). diff --git a/web/src/routes/ReportsPage.test.tsx b/web/src/routes/ReportsPage.test.tsx index c54664804..9fbe741b8 100644 --- a/web/src/routes/ReportsPage.test.tsx +++ b/web/src/routes/ReportsPage.test.tsx @@ -36,10 +36,10 @@ const mockGetProfitReport = vi.mocked(getProfitReport); // of the `?? "—"` fallback (DATA, left raw per the namespace header comment). const PRODUCTION: ProductionReport = { days: [ - { date: "2026-07-19", totalEggs: 100, cracked: 2, dirty: 3, discarded: 5, sellable: 90, deaths: 1, henDays: 98, henDayPct: 91.8 }, - { date: "2026-07-18", totalEggs: 95, cracked: 1, dirty: 1, discarded: 2, sellable: 91, deaths: 0, henDays: 98, henDayPct: null }, + { date: "2026-07-19", totalEggs: 100, cracked: 2, dirty: 3, discarded: 5, sellable: 90, fromCounts: 6, deaths: 1, henDays: 98, henDayPct: 91.8 }, + { date: "2026-07-18", totalEggs: 95, cracked: 1, dirty: 1, discarded: 2, sellable: 91, fromCounts: 0, deaths: 0, henDays: 98, henDayPct: null }, ], - totalEggs: 195, totalSellable: 181, totalDeaths: 1, totalHenDays: 196, periodHenDayPct: 92.3, + totalEggs: 195, totalSellable: 181, totalFromCounts: 6, totalDeaths: 1, totalHenDays: 196, periodHenDayPct: 92.3, gradeTotals: [ { eggGradeId: "gr1", name: "Grade A", quantity: 60 }, { eggGradeId: "gr2", name: "Grade B", quantity: 30 }, @@ -104,6 +104,36 @@ describe("ReportsPage production section (renders for every role)", () => { expect(screen.getByText("By grade: Grade A 60, Grade B 30")).toBeInTheDocument(); }); + // #396 — Condition sits BESIDE Sellable, never folded into it. The fixture + // gives day 1 sellable 90 and fromCounts 6, two values that cannot be + // confused for one another, so a column wired to the wrong field fails. + it("renders the Condition column beside Sellable, and keeps Sellable unchanged", async () => { + renderWithProviders(, { token: NON_ADMIN }); + + const row1 = await screen.findByRole("row", { name: /2026-07-19/ }); + within(row1).getByText("90"); // sellable — still the hand-graded remainder + within(row1).getByText("6"); // condition — what the counters contributed + + const periodRow = screen.getByRole("row", { name: /Period/ }); + within(periodRow).getByText("181"); // totalSellable + within(periodRow).getByText("6"); // totalFromCounts + + // The header must resolve through the catalog, not be hardcoded English. + expect(screen.getByRole("columnheader", { name: "Condition" })).toBeInTheDocument(); + }); + + it("shows 0 in the Condition column for a day whose conditions were losses", async () => { + renderWithProviders(, { token: NON_ADMIN }); + + // Day 2 has cracked 1 / dirty 1 but fromCounts 0 — recorded as losses. The + // column must NOT fall back to the raw counters, which is the mistake that + // would invent stock the farm never had. + // Columns: date, eggs, losses, sellable, CONDITION, deaths, henDays, pct. + // Indexed: day 2 has a 0 in deaths too, so a text match proves nothing. + const row2 = await screen.findByRole("row", { name: /2026-07-18/ }); + expect(within(row2).getAllByRole("cell")[4]).toHaveTextContent("0"); + }); + it("falls back to the em dash for a null periodHenDayPct in the Period row too, not just per-day", async () => { mockGetProductionReport.mockResolvedValue({ ...PRODUCTION, periodHenDayPct: null }); renderWithProviders(, { token: NON_ADMIN }); diff --git a/web/src/routes/ReportsPage.tsx b/web/src/routes/ReportsPage.tsx index bc953e6b6..5bfb6225f 100644 --- a/web/src/routes/ReportsPage.tsx +++ b/web/src/routes/ReportsPage.tsx @@ -110,6 +110,10 @@ export function ReportsPage() { {t("dateHeader")}{t("eggsHeader")}{t("lossesHeader")}{t("sellableHeader")} + {/* #396 — beside Sellable, not folded into it: Sellable is the + hand-graded remainder, Condition is what the cracked/dirty + counters contributed as stock. */} + {t("conditionHeader")} {t("deathsHeader")}{t("henDaysHeader")}{t("henDayPctHeader")} @@ -120,6 +124,7 @@ export function ReportsPage() { {d.totalEggs} {d.cracked}/{d.dirty}/{d.discarded} {d.sellable} + {d.fromCounts} {d.deaths} {d.henDays} {d.henDayPct ?? "—"} @@ -132,6 +137,7 @@ export function ReportsPage() { {production.totalEggs} {production.totalSellable} + {production.totalFromCounts} {production.totalDeaths} {production.totalHenDays} {production.periodHenDayPct ?? "—"} diff --git a/web/src/routes/SalesPage.test.tsx b/web/src/routes/SalesPage.test.tsx index 10ae2c01f..957ee4541 100644 --- a/web/src/routes/SalesPage.test.tsx +++ b/web/src/routes/SalesPage.test.tsx @@ -52,7 +52,7 @@ const mockRecordPayment = vi.mocked(recordPayment); const CUSTOMER: Customer = { id: "c1", name: "Acme Eggs", phone: "555", email: null, address: null, note: null }; // Only gr1 is saleable → the picker offers PRODUCT_A only; gr2/PRODUCT_B exists // solely to resolve the second line's display name (allProducts). -const GRADE: EggGrade = { id: "gr1", farmId: "farm1", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, active: true }; +const GRADE: EggGrade = { id: "gr1", farmId: "farm1", name: "Grade A", gradeType: "Size", sortOrder: 1, isSaleable: true, dailyEntryKind: "Manual", active: true }; const PRODUCT_A: Product = { id: "p1", name: "Grade A Dozen", productType: "Egg", defaultUnit: "Dozen", defaultPriceMinorUnits: 300, currencyCode: "USD", currencyMinorUnit: 2,