fix(spp_drims): only let a dispatch ship what its request approved

[emjay0921]

Why is this change needed?

A DRIMS dispatch is generated from an approved request, but its Operations tab stayed editable in Ready state. Reproduced on a dev instance before changing anything — a request for 100 units of one approved item yielded two ways past the approval workflow, both validating successfully with no warning:

Vector	Result
Add a Product	50 units of a never-requested item shipped done, drims_request_line_id empty
Unlock (padlock) → raise Demand	Demand 100 → 150; 150 units shipped against a request for 100 with 100 allocated

The ticket describes the first. The second has the same impact and is covered by the ticket's own Impact section ("inventory records will reflect quantities that don't match the approved request"), so it is fixed here too.

Fallout in both cases: the request line was left reading dispatched=150 against requested=100, and the unapproved item's unit cost was booked into the incident's drims_distributed_value as distributed relief value. In a humanitarian context that is unauthorised aid movement plus a corrupted distribution figure.

How was the change implemented?

_check_drims_dispatch_matches_request() (stock_picking.py), called at the top of button_validate:

1. Every live move must trace back to a line of this request.
2. Nothing may ship beyond that line's quantity_allocated, counting what earlier dispatches already shipped for it.

Two deliberate choices:

• Keyed on drims_request_line_id, not Odoo's additional flag. additional looks like the natural discriminator, but it is only set when a line is added through the form — a move created over RPC or by an import leaves it False and would slip straight past a check based on it. There is a test asserting that precondition so the reasoning is not lost later.
• Capping at quantity_allocated transitively enforces approval, because _allocate_stock_fifo never allocates beyond quantity_requested. That avoids introducing a second, parallel notion of "the approved quantity" that could drift.

Form lock — the Operations field's options now carry create and delete domains on drims_type, removing the "Add a Product" row and the row-delete for a request dispatch.

Worth flagging for reviewers, since it is easy to get wrong: these must be options entries, not bare create=/delete= attributes on the <field> tag. The x2many passes its options dict through as crudOptions (x2ManyField.extractProps) and useActiveActions evaluates each entry as a domain against the parent record; bare attributes on a field tag are silently ignored, and only the inner <list> honours those, as static booleans. Both rendered controls were traced to their gates — displayRowCreates → canCreate → activeActions.create, and hasX2ManyAction → activeActions.delete (list_renderer.xml:346).

Quantity is left editable on purpose. Entering less than Demand is how a partial dispatch and its backorder are produced, so a blanket readonly on the moves list would have broken that flow. Core already makes product_id readonly unless additional is set, and Demand readonly until the padlock is used, so the view change stays narrow.

New unit tests

spp_drims/tests/test_dispatch_line_lock.py — 9 tests:

• An added, unapproved product blocks validation and is named in the error.
• The guard catches moves Odoo never flagged as additional (the RPC/import hole).
• A move borrowed from another request's line is refused.
• Demand raised above the allocation is refused; so is over-picking with Demand intact.
• Full dispatch still validates; partial dispatch still reaches the Create Backorder prompt; a second dispatch after a top-up validates, so the cumulative check does not false-positive.
• A plain non-DRIMS delivery is unaffected.

Unit tests executed by the author

Full module suite:

0 failed, 0 error(s) of 256 tests

All 9 new tests confirmed executing by name in the log rather than inferred from the total. ./spp lint (ruff, ruff-format, prettier) passes on all four files. Ruff flagged B023 on the first draft — lambdas capturing loop variables, harmless as written since filtered() runs immediately, but rewritten with comprehensions rather than suppressed.

Verified in the running app on a fresh database, not only in tests:

• both vectors refused, picking stays assigned, nothing ships;
• full dispatch validates; partial dispatch still returns the backorder wizard;
• "Add a Product" and the row-delete are gone from a DRIMS dispatch (confirmed visually);
• Quantity remains editable (is_quantity_done_editable=True, and a write of 90 persists) while Demand stays read-only;
• a plain delivery and a DRIMS donation receipt both still show "Add a Product" (confirmed visually) — so the lock is scoped to request_dispatch and has not leaked onto ordinary Inventory operations or other DRIMS picking types.

How to test manually

Requires a DRIMS warehouse with stock and an approved, allocated request. demo-scripts/ds-openspp2-op1057-spp_drims-20260803-02.py builds the whole fixture set on a fresh DB; …-01.py is a 4/4 regression check of the server-side guard.

1. Open a Ready DRIMS dispatch → Operations tab. "Add a Product" and the row-delete must be absent. Quantity must still be editable; Demand read-only.
2. Set Quantity below Demand and Validate → you should get Create Backorder?, not an error.
3. Add a move for an unrequested product over RPC (or XML-RPC) and Validate → refused, naming the product.
4. Click the padlock to unlock, raise Demand above the allocated quantity, Validate → refused, reporting dispatched vs allocated.
5. Open a plain delivery under Inventory → Operations, and a DRIMS donation receipt. Both must still show "Add a Product".

Related links

• OP#1057 — DRIMS: Should not be able to add products during validation of a dispatch

Notes for the reviewer

readme/HISTORY.md and the manifest version are deliberately untouched — per our convention those land on 19.0 after merge, to avoid conflicts between concurrent PRs on the same module. The entry for this change would be fix(spp_drims): only let a dispatch ship what its request approved.

This branch is independent of #390 (OP#1087). Both edit button_validate, but in different hunks — this one adds a call at the top, #390 adds state settling after super(). They should merge cleanly in either order; whichever lands second may want a quick rebase check.

Unrelated defect noticed while building fixtures, left alone: spp_drims/models/constants.py declares PRIORITY_LOW/MEDIUM/HIGH, but the shipped vocabulary data is routine/urgent/critical, and transport modes are road/air/sea/foot rather than truck. Those constants appear to be dead and will mislead whoever trusts them next — worth its own ticket.

🤖 Generated with Claude Code

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.35%. Comparing base (c33d3cb) to head (1491685).
⚠️ Report is 3 commits behind head on 19.0.

Additional details and impacted files

@@            Coverage Diff             @@
##             19.0     #391      +/-   ##
==========================================
+ Coverage   71.49%   72.35%   +0.85%     
==========================================
  Files         243     1010     +767     
  Lines       20785    60854   +40069     
==========================================
+ Hits        14860    44030   +29170     
- Misses       5925    16824   +10899     

Flag	Coverage Δ
spp_base_common	91.07% <ø> (ø)
spp_drims	82.39% <100.00%> (+0.15%)	⬆️
spp_drims_sl_demo	68.72% <ø> (-0.29%)	⬇️
spp_programs	65.27% <ø> (ø)
spp_registry	87.22% <ø> (+0.07%)	⬆️
spp_security	69.56% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
spp_drims/models/stock_picking.py	80.30% <100.00%> (+3.51%)	⬆️

... and 787 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gonzalesedwin1123]

This closes a hole that is live on 19.0 right now (OP#1075's readonly went out with PR #396), and it closes it well. What I verified:

• Both design forks are right and both are tested. Keying on drims_request_line_id rather than additional is correct — and the precondition test proving RPC-created moves aren't flagged additional means the reasoning can't silently rot. Capping at quantity_allocated (which _allocate_stock_fifo already caps at the requested quantity) enforces approval without inventing a second source of truth.
• The cumulative check handles the pending-backorder case correctly: done moves count across pickings, this picking's live moves count at their picked quantity, and an unlocked backorder with raised demand is refused against what already shipped. uom_id.compare for the float comparison is right.
• The options-domains mechanism is the correct channel — traced as described through crudOptions/useActiveActions, and the history (QA catching the bare-attribute attempt) is exactly why the long comment earns its place.
• Partial dispatch still reaches the Create Backorder wizard (tested), so this doesn't reintroduce the OP#1075 regression.
• The #390 coexistence claim holds: different hunks of button_validate; whichever lands second should rebase-check, as you noted.

1. Blocker — version bump (policy now settled)

Edwin has ruled on the convention question your reviewer note raises: version bumps + HISTORY entries land in the PR, with the concrete number assigned at merge time in the queue order he sets — not as post-merge commits on 19.0. (Same answer as on #412: keep bumps in PRs.) The reasoning: the upgrade signal stays atomic with the change it describes — the #414 case, where deploying code without its data would have silently killed the donation flow, is the class of problem this prevents — and it keeps the proposed #441 CI check enforceable. So: please add the bump + your drafted HISTORY line ("only let a dispatch ship what its request approved") when this rebases for merge. With that, this is an approve.

2. Suggestions (non-blocking)

• An arch test pinning the options dict on the Operations move_ids field. The failure mode QA caught on the first attempt — bare attributes silently ignored — is precisely a view regression, and your own arch-test style from #414 (column_invisible test) fits perfectly here.
• The dead-constants find (PRIORITY_*, truck vs the shipped routine/urgent/critical and road/air/sea/foot) is worth its own ticket as you say — happy to file it if you haven't.

[emjay0921]

Thanks — both done. Pushed as 14916850.

1. Version bump — done. spp_drims → 19.0.3.0.2 with the HISTORY line, and thanks for settling the convention question explicitly; I have followed the same pattern on #390, #414 and #416, so all four now carry their bump and entry, with the concrete numbers to be reconciled at merge time in whatever order you set. #414 is the case that proves the rule — there, shipping code without its data record would have created every new donation with no state at all.

2. Arch test — added. It reads the options off the combined arch of stock.view_picking_form and asserts create and delete are both denied and both scoped to request_dispatch. This is exactly the regression that would otherwise be silent: the first attempt used bare create/delete attributes on the field tag, Odoo ignored them, and nothing failed — QA caught it by clicking. A revert to that form now fails here instead.

3. The dead constants are already filed and fixed: OP#1165, PR #438 — PRIORITY_LOW/MEDIUM/HIGH and DRIMS_TYPE_TRANSFER named codes the vocabulary never shipped, and the consumers that hardcoded the same strings (list decorations, the High Priority search filter, and get_approval_sla_hours, where an urgent request silently fell back to the 24-hour routine default instead of 8). That PR also adds a test walking every code constant against the vocabulary, so a constant naming a code the data lacks fails there rather than quietly matching nothing.

spp_drims suite: 273 tests, 0 failed. pre-commit clean.