feat(spp_drims): Requests review — UI/UX overhaul, per-warehouse allocation, reviewer notes - #396
Merged
Merged
Conversation
- Overview list: hide the 'Code' column. It is the state field (related to state_id.code, label 'Code'); in Odoo 19 invisible="1" no longer hides a list column, so use column_invisible — keeps the field for row decorations. - Form: reference fields (Incident, Cluster, Destination Area, Service Point, Destination Warehouse, Priority, Requested by) are display-only (no_open). - Restructure the form into Request Details / Priority and Dates / Destination Information / Products; remove the Fulfillment section (the allocation wizard sets the source warehouse). - Add a Destination Type selector (Warehouse | Service Point); the matching destination field is shown conditionally and the service point is filtered to the destination area. - Flag allocation shortfall: a line where, post-approval, allocated < requested shows Quantity Allocated in red (is_allocation_short); the Allocate Stock button is hidden once the request is fully allocated (is_fully_allocated), and the 'Ready to Allocate' banner is replaced by a 'Fully Allocated' one. Per-warehouse allocation split display is deferred to #1079.
The Allocate Stock wizard now opens without a source warehouse so the user can first see which warehouses hold stock for the requested items, pick one, and have it written back to the request. Adds a Source Warehouse field to the request form (optional, editable while approved) so it can also be set before opening the wizard. - Warehouse suggestions now cover both the no-warehouse case (where stock is available) and the shortfall case (where else to look), using net availability. - Confirm requires a warehouse with a clear message instead of failing on save; clearing the warehouse now clears stale lines. - Fix line save failure when changing warehouse: the allocation line's product is now derived from the request line (stored related) so it is no longer a required readonly input omitted from the save payload.
Request Details now lists Incident, Humanitarian Cluster, Affected Population, Justification and Requested by, in that order — Justification moves into the section (the standalone Justification group is removed) and the Contact Name / Phone fields (not part of the spec) are dropped from the form.
…ent %, drop Still Needed column, lock rows (#1079)
…location tabs (#1075)
… sections (#1075)
…ed for approval (#1162)
Brings the branch up to date with 19.0 (13 commits) ahead of raising the PR for OP#1075 and its children, which have passed QA. Merged rather than rebased because the branch is already published.
OP#1075 locked a dispatch's Operations list by making the move_ids field readonly, to stop products being added. A readonly x2many also blocks editing Quantity, and entering less than Demand is how a partial dispatch and its backorder are produced (OP#1087) — so the lock took the short-shipment flow out with it. Removed here in favour of the lock added by OP#1057, which sets create/delete domains in the field's options: lines cannot be added or removed, Quantity stays editable. That change also adds stock.picking._check_drims_dispatch_matches_request, refusing at validation any item not on the request and any quantity beyond what was allocated, which covers RPC and imports as a view attribute never could. Both branches edited the same field, so leaving this in place would have left readonly and options fighting on the merged result, with readonly winning and quantities frozen. OP#1075 OP#1057
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #396 +/- ##
==========================================
+ Coverage 70.17% 71.42% +1.25%
==========================================
Files 205 243 +38
Lines 17751 20790 +3039
==========================================
+ Hits 12457 14850 +2393
- Misses 5294 5940 +646
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
emjay0921
marked this pull request as ready for review
August 7, 2026 02:29
emjay0921
marked this pull request as draft
August 7, 2026 03:42
emjay0921
marked this pull request as ready for review
August 7, 2026 03:43
gonzalesedwin1123
approved these changes
Aug 10, 2026
emjay0921
added a commit
that referenced
this pull request
Aug 13, 2026
Cancelling or declining a backorder stopped releasing the quantity after PR #396 merged. OP#1079 turned the request line's quantity_dispatched into a stored compute over per-warehouse allocation rows, and _reconcile_quantity_dispatched still assigned to the line. A second dispatch then refused with "Nothing left to dispatch on this request". Rebuild the allocation rows instead. Every dispatch move carries the allocation it draws from - action_create_dispatch is the only thing that creates them and always stamps drims_allocation_id, and a backorder copies the link - so splitting the reconciled total per allocation is unambiguous. Nothing was wrong when this ticket passed QA; the ground moved underneath it after merge. The tests were weaker than they looked. Writing a stored compute does persist, until something retriggers it, so only one of the three release tests failed - the other two asserted the line and passed on ordering luck while the allocation underneath was left at the full dispatched quantity. That is the figure _drims_available_quantity subtracts, so the released stock was never actually freed for re-allocation. All three now assert the allocation rows, and all three fail without the fix. Also records why each sudo() in this ticket's code is deliberate. Those findings are what has been failing pre-commit on PR #390 since it opened.
emjay0921
added a commit
that referenced
this pull request
Aug 13, 2026
…ating test_waiting_state_is_reachable_for_a_dispatch built its Waiting dispatch by allocating the same stock to two requests. Since PR #396 (OP#1079) that is refused - _drims_available_quantity subtracts pending allocations, so stock cannot be promised twice - and the test errored on its own setup. The ticket's reasoning still holds. Allocation records per-warehouse rows and reserves nothing, so it cannot guarantee the stock is still there when the dispatch is created: an inventory adjustment or transfer in between leaves the dispatch with nothing to reserve and it lands in confirmed. That is why Waiting is hidden only as a future step, never as the current value. Rebuilt around that instead. No production code changes.
emjay0921
added a commit
that referenced
this pull request
Aug 13, 2026
PR #396 landed on 19.0 after this branch last merged up. OP#1079 removed spp.drims.request.source_warehouse_id and made the request line's quantity_allocated a stored compute over allocation rows, so the fixture no longer built a request at all and took the whole delivery-confirmation class with it.
emjay0921
added a commit
that referenced
this pull request
Aug 20, 2026
…#390) * fix(spp_drims): stop backorder dispatches bypassing the DRIMS request Validating a dispatch short of its demand and choosing "Create Backorder" left the request reading as fully dispatched, told no one, and attributed the parent shipment's per-shipment facts to goods still in the warehouse. Odoo builds a backorder with picking.copy(), so every field left at the default copy=True was inherited. Mark the per-shipment facts copy=False: beneficiary count, departure/arrival, the pod_* block, transport and driver details, discrepancy notes and drims_return_id. This also stops the Duplicate action producing a dispatch that claims a delivery, and it makes the beneficiary guard in button_validate() fire on the backorder instead of being pre-satisfied by the inherited value — which had let one 100-unit distribution to 500 people report 1000 beneficiaries served on spp.hazard.incident.drims_beneficiaries_served. Announce the backorder on the request: an internal note plus a to-do activity for the coordinators of the destination area, resolved by mirroring rule_request_coordinator_scope so it reaches exactly those permitted to see the request. Reopen the request at Ready for Dispatch while a backorder is pending, and re-advance to Dispatched once nothing is outstanding. Because action_create_dispatch counts a quantity as dispatched when it is committed to a picking rather than when it ships, that running total is now rebuilt from the moves that still stand whenever moves are validated or cancelled. A single reconciliation covers a cancelled backorder, a cancelled dispatch, and declining Create Backorder — the last of which cancels no move at all, it just drops the excess demand, so a cancellation hook alone would have missed it. Odoo already carries drims_request_id and drims_request_line_id onto the backorder and its split move, so the request link and per-line attribution were correct and are covered by a regression test. OP#1087 * test(spp_drims): follow the per-warehouse allocation model OP#1079 removed spp.drims.request.source_warehouse_id and made the request line's quantity_allocated a stored compute over allocation rows, so these fixtures no longer built a request at all. * fix(drims): release the backorder quantity onto the allocation rows Cancelling or declining a backorder stopped releasing the quantity after PR #396 merged. OP#1079 turned the request line's quantity_dispatched into a stored compute over per-warehouse allocation rows, and _reconcile_quantity_dispatched still assigned to the line. A second dispatch then refused with "Nothing left to dispatch on this request". Rebuild the allocation rows instead. Every dispatch move carries the allocation it draws from - action_create_dispatch is the only thing that creates them and always stamps drims_allocation_id, and a backorder copies the link - so splitting the reconciled total per allocation is unambiguous. Nothing was wrong when this ticket passed QA; the ground moved underneath it after merge. The tests were weaker than they looked. Writing a stored compute does persist, until something retriggers it, so only one of the three release tests failed - the other two asserted the line and passed on ordering luck while the allocation underneath was left at the full dispatched quantity. That is the figure _drims_available_quantity subtracts, so the released stock was never actually freed for re-allocation. All three now assert the allocation rows, and all three fail without the fix. Also records why each sudo() in this ticket's code is deliberate. Those findings are what has been failing pre-commit on PR #390 since it opened. * fix(spp_drims): address the backorder review — version bump, wider hook, named audit trail Version bump to 19.0.3.0.1 with its changelog entry. Nothing malfunctions without an upgrade here — the change is Python plus registry-level field attributes — but the module ships in a release, and a same-version change gives upgraders no signal. The re-advance moves from button_validate to picking._action_done. It used to fire only for the web client's Validate button, so a backorder released through the API, the barcode flow or a direct _action_done reconciled its quantities through the move hook and then left the request sitting at "allocated" with everything already shipped. _action_done is the point every path goes through, and running after super() means any backorder has already been split off, which is what the sync inspects before advancing. The backorder note now names who validated short. It is posted through sudo, so OdooBot authors it; in a humanitarian-accountability trail "who shipped short" is part of the record. _set_state_by_code reuses _get_state_by_code instead of repeating its search. The Semgrep pragmas move onto the line each finding anchors to, matching the form used elsewhere in the repo. The rationale comments above each sudo already explained why it is needed; the placement is what the scanner reads. Two tests added: a backorder validated outside the web client advances the request, and the note names the acting user. * fix(spp_drims): keep both semgrep rule ids on the res.users sudo Moving the pragma onto the matched line dropped odoo-sudo-without-context, which the local semgrep hook then flagged. Both rules match that expression, so both ids belong on it.
emjay0921
added a commit
that referenced
this pull request
Aug 20, 2026
#393) * feat(spp_drims): collect proof of delivery in a popup off the dispatch The Confirm Departure and Confirm Delivery buttons sat in their own column beside Distribution Details, far enough from Departure & Arrival to read as unrelated. Each now sits on the row of the field it fills. Confirm Delivery stays hidden until departure is recorded, with a hint in its place. The ticket asked for it greyed out, which an Odoo form cannot express: ViewButton's `disabled` is a component prop and is never wired to the arch, so `invisible` is the only declarative option. The ordering is enforced on the model as well, in action_open_delivery_confirmation and action_confirm_pod, so no path can log an arrival for goods that never left. Confirming delivery now opens spp.drims.delivery.confirmation.wizard instead of expecting the officer to have typed the receiver's details into the form and then pressing a button that refused if they had not. The wizard collects receiver, delivery status, signature, photos, GPS and notes, writes them back to the POD block, and locks that block once confirmed so the delivery record is not casually edited afterwards. It also records what actually arrived per line. Nothing in the module wrote spp.drims.request.line.quantity_delivered before this, so total_delivered and fulfillment_pct sat at 0 however much had been delivered, and spp.drims.alert kept reporting the full requested quantity as still needed. Quantities default to everything dispatched, are capped at it, and accumulate so a request filled by several dispatches totals correctly rather than overwriting. Lines are populated in both default_get and create. default_get alone only covers callers that pass the picking through the context; passing picking_id in the values — the obvious way from a script or over RPC — produced a wizard with no lines that silently recorded nothing. A test pins that path. test_pod_confirmation asserted the old behaviour of confirming delivery with no departure recorded, and is updated for the new ordering. OP#1088 * fix(spp_drims): keep the delivery popup's line context off the move Confirming a delivery from the popup failed with "Expected singleton: uom.uom()". The wizard line held its move, request line, product, unit and dispatched quantity as plain readonly fields, and the web client does not send readonly fields back when it saves, so a confirmation arrived carrying nothing but quantity_delivered. The empty unit turned uom_id.compare into a bare singleton error. The crash was the fortunate part. The request line came back empty too, so had the check been more forgiving the confirmation would have recorded no delivered quantities at all and left fulfillment reading 0 - the very number this wizard exists to write. Derive the line from its move instead: move_id is required and writable, and the request line, product, unit and dispatched quantity are related off it, so the move is the only value that has to survive the round trip. The quantity check also falls back to float_compare, so a missing unit can never produce a raw singleton error again. Move the departure and delivery buttons left as well, by hiding each date while it is still empty. An empty datetime widget was pushing the button across the row. The field and its button are mutually exclusive: an unset date means there is an action to take, a set one means it is done. Round 1 passed every server-side test and still shipped broken, because the tests built the wizard themselves with all values supplied. The new test sends the payload the browser actually sends, and a second one guards move_id against going readonly again. * test(drims): follow the per-warehouse allocation model PR #396 landed on 19.0 after this branch last merged up. OP#1079 removed spp.drims.request.source_warehouse_id and made the request line's quantity_allocated a stored compute over allocation rows, so the fixture no longer built a request at all and took the whole delivery-confirmation class with it. * fix(spp_drims): refuse delivery confirmation before the transfer is validated Version bump with its changelog entry, per the settled convention. The wizard's lines come from done moves, so on a departed-but-unvalidated dispatch it opened empty — and confirming an empty wizard wrote the proof of delivery, set is_pod_confirmed and recorded no delivered quantities, after which the already-confirmed guard prevented ever recording them. Fulfilment stayed at zero with no way back short of editing fields by hand. The trap is reachable by following OP#1087's documented order, which confirms departure before validating. Two guards rather than one: the opener now requires state 'done', and action_confirm refuses when there are no lines, so the direct API cannot burn the one-shot flag either. Delivery photos upload through the wizard form, so they were filed against the transient model. They are re-pointed to the picking on confirm, which is where the evidence belongs and whose access rules should govern it.
emjay0921
added a commit
that referenced
this pull request
Aug 20, 2026
…#392) * fix(spp_drims): show only reachable states on the dispatch status bar action_create_dispatch confirms a request dispatch the moment it creates it, so a dispatch never sits in Draft. Both Draft and Waiting were still drawn as greyed-out steps on the form, inherited from stock.picking. Core renders two status bars for stock.picking, split on picking_type_code. A dispatch is outgoing, so it picks up the non-incoming one with draft,confirmed,assigned,done. Narrow that bar to exclude dispatches and add a dispatch-only bar showing assigned,done, rather than editing the shared statusbar_visible — which would have dropped Draft from every non-incoming transfer in the database. The three bars' conditions are mutually exclusive, so exactly one renders per picking. Waiting is hidden as a *future* step only, and deliberately so: contrary to the ticket's rationale it is genuinely reachable. DRIMS allocation writes quantity_allocated on the request line and creates no Odoo reservation, so two requests can allocate the same units and whichever dispatches second has nothing to reserve and lands in confirmed. That is safe here because the statusbar widget always renders the current value even when it is excluded from statusbar_visible (getAllItems filters on `value === currentValue || visibleSelection.includes(value)`), so a dispatch short of stock still shows Waiting to warehouse staff. Both facts are covered by tests so neither gets "simplified" later on a false assumption, and the mutual exclusivity test evaluates the real invisible expressions per picking kind rather than matching strings. OP#1086 * test(spp_drims): follow the per-warehouse allocation model OP#1079 removed spp.drims.request.source_warehouse_id and made the request line's quantity_allocated a stored compute over allocation rows, so these fixtures no longer built a request at all. * test(drims): produce Waiting by losing the stock, not by double-allocating test_waiting_state_is_reachable_for_a_dispatch built its Waiting dispatch by allocating the same stock to two requests. Since PR #396 (OP#1079) that is refused - _drims_available_quantity subtracts pending allocations, so stock cannot be promised twice - and the test errored on its own setup. The ticket's reasoning still holds. Allocation records per-warehouse rows and reserves nothing, so it cannot guarantee the stock is still there when the dispatch is created: an inventory adjustment or transfer in between leaves the dispatch with nothing to reserve and it lands in confirmed. That is why Waiting is hidden only as a future step, never as the current value. Rebuilt around that instead. No production code changes. * fix(spp_drims): bump version for the dispatch status bar change The review's one blocker: 19.0.4.0.3, above the 19.0.4.0.2 #393 landed, with its changelog entry and the regenerated README. * docs(spp_drims): regenerate README for the 19.0.4.0.3 entry
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this change needed?
OP#1075 (DRIMS - Full review - Requests) and all three of its children have passed QA — this raises the PR for that work, per the flow of opening PRs once QA signs off.
What is in here
spp.drims.request.allocationmodel recording allocation per source warehouse, withstock.move.drims_allocation_idlinking a move to the allocation it dispatches. The allocation wizard gained in-screen warehouse selection, clearer warnings, fulfillment %, and locked rows; source warehouse became optional.spp.drims.request.revision.wizardso "Request Changes" captures a required note instead of silently bouncing the request back.action_resubmit, so a request sent back for revision can actually be resubmitted for approval.Two things a reviewer should know
1. This branch was brought up to date by merging
19.0, not rebasing. It was already published, and rebasing would have rewritten shared history and required a force-push. Hence the merge commit.2. One QA-passed behaviour was deliberately removed — see
7cdd47a2.OP#1075 locked a dispatch's Operations list by making
move_idsreadonly, to stop products being added. But a readonly x2many also blocks editing Quantity, and entering less than Demand is exactly how a partial dispatch and its backorder are produced (OP#1087). The lock took the short-shipment flow out with it.The same lock is implemented differently in OP#1057 (PR #391), which sets
create/deletedomains in the field'soptions: lines cannot be added or removed, while Quantity stays editable. It also addsstock.picking._check_drims_dispatch_matches_request, which refuses at validation any item not on the request and any quantity beyond what was allocated — covering RPC and imports, which a view attribute cannot.Both branches edit the same
//field[@name='move_ids']. Left as it was,readonlyandoptionswould both land andreadonlywould win, freezing quantities. So the readonly attribute is removed here and the OP#1057 lock governs.This means QA signed off on a dispatch whose quantities could not be edited. If that was actually the desired behaviour rather than an untested side effect, say so and it can be reinstated — but OP#1087's partial-dispatch flow would then need rethinking.
Unit tests executed by the author
Full module suite on the merged branch:
./spp lintclean. No test asserted the removedreadonlyattribute.Unrelated and pre-existing, noted in passing:
spp_drims/models/stock_warehouse.py:104still callsread_group, deprecated since 19.0, which logs aDeprecationWarningtraceback during the suite. Not a failure.Related links
Notes for the reviewer
This branch carries
readme/HISTORY.mdand a regeneratedstatic/description/index.html, which differs from how the other in-flight DRIMS branches are prepared — those leave the changelog for19.0after merge, to avoid conflicts between concurrent PRs on the same module. Left as the branch already had it; worth a consistency decision if several of these land together.Overlap with the other open DRIMS PRs. This shares eight files with them —
__manifest__.py,models/request.py,models/request_line.py,models/stock_move.py,security/ir.model.access.csv,tests/test_stock.py,views/stock_picking_views.xml,wizard/__init__.py— so whichever lands second will need a merge pass. #391 (OP#1057) is the one to watch, for the reason above.Also worth knowing for OP#1151 (branch
feat/1151-1150-drims-waybill-and-dispatch-page, pushed, PR not raised yet as it is still in QA): it exposesstock.move's DRIMS linkage fields in a view, but only the two that exist on19.0. This branch adds a third,drims_allocation_id, which should be exposed alongside them once both are in.🤖 Generated with Claude Code