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.
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
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