diff --git a/spp_drims/README.rst b/spp_drims/README.rst
index 6096faff..4a860357 100644
--- a/spp_drims/README.rst
+++ b/spp_drims/README.rst
@@ -179,6 +179,15 @@ Dependencies
Changelog
=========
+19.0.4.0.1
+~~~~~~~~~~
+
+- fix(drims): only let a dispatch ship what its request approved.
+ Products cannot be added to a request dispatch and quantities cannot
+ be raised past what was allocated — enforced on the model, so imports
+ and API callers are covered too, with the Operations tab's Add a line
+ and delete affordances hidden to match (#1057)
+
19.0.4.0.0
~~~~~~~~~~
diff --git a/spp_drims/__manifest__.py b/spp_drims/__manifest__.py
index 3c83a5cd..c79520b8 100644
--- a/spp_drims/__manifest__.py
+++ b/spp_drims/__manifest__.py
@@ -5,7 +5,7 @@
"and distribution tracking. Links to hazard incidents with multi-tier "
"approval workflows and warehouse operations.",
"category": "OpenSPP/Inventory",
- "version": "19.0.4.0.0",
+ "version": "19.0.4.0.1",
"sequence": 1,
"author": "OpenSPP.org",
"website": "https://github.com/OpenSPP/OpenSPP2",
diff --git a/spp_drims/models/stock_picking.py b/spp_drims/models/stock_picking.py
index 9ea48b01..32ca19a8 100644
--- a/spp_drims/models/stock_picking.py
+++ b/spp_drims/models/stock_picking.py
@@ -321,13 +321,98 @@ def action_view_drims_return(self):
"res_id": self.drims_return_id.id,
}
+ def _check_drims_dispatch_matches_request(self):
+ """Refuse to dispatch anything the request did not approve (OP#1057).
+
+ A dispatch is generated from an approved request, but the Operations tab
+ stays editable in Ready state, so two things could still be smuggled past
+ the approval workflow:
+
+ 1. **Extra products.** "Add a Product" attaches a move that no request
+ line asked for. Note this is keyed on ``drims_request_line_id`` rather
+ than Odoo's ``additional`` flag: ``additional`` is only set when a line
+ is added through the form, so a move created over RPC or by an import
+ has ``additional = False`` and would slip past a check based on it.
+
+ 2. **Inflated quantities.** Unlocking the picking makes Demand editable
+ again, so an approved line can be raised above what was allocated.
+ Allocation is itself capped at the requested quantity by
+ ``_allocate_stock_fifo``, so comparing against ``quantity_allocated``
+ transitively enforces the approved amount.
+
+ Raises:
+ UserError: naming the offending products, if either check fails.
+ """
+ for picking in self:
+ if picking.drims_type != "request_dispatch" or not picking.drims_request_id:
+ continue
+
+ live_moves = picking.move_ids.filtered(lambda m: m.state != "cancel")
+ live_move_ids = set(live_moves.ids)
+ approved_line_ids = set(picking.drims_request_id.line_ids.ids)
+
+ # 1. Every move has to trace back to a line of *this* request.
+ unapproved_products = sorted(
+ {m.product_id.display_name for m in live_moves if m.drims_request_line_id.id not in approved_line_ids}
+ )
+ if unapproved_products:
+ raise UserError(
+ _(
+ "Dispatch %(picking)s contains items that are not part of "
+ "request %(request)s: %(products)s.\n\n"
+ "A dispatch may only ship what the request had approved and "
+ "allocated. Remove these lines, or raise a new request for "
+ "them and have it approved.",
+ picking=picking.name,
+ request=picking.drims_request_id.reference,
+ products=", ".join(unapproved_products),
+ )
+ )
+
+ # 2. Nothing may ship beyond what the request line had allocated,
+ # counting what earlier dispatches already shipped for that line.
+ over_dispatched = []
+ for line in live_moves.drims_request_line_id:
+ line_moves = self.env["stock.move"].search(
+ [
+ ("drims_request_line_id", "=", line.id),
+ ("state", "!=", "cancel"),
+ ]
+ )
+ already_shipped = sum(m.quantity for m in line_moves if m.state == "done")
+ about_to_ship = sum(m.quantity for m in line_moves if m.id in live_move_ids)
+ if line.uom_id.compare(already_shipped + about_to_ship, line.quantity_allocated) > 0:
+ over_dispatched.append(
+ _(
+ "%(product)s: dispatching %(total)s but only %(allocated)s is allocated",
+ product=line.product_id.display_name,
+ total=already_shipped + about_to_ship,
+ allocated=line.quantity_allocated,
+ )
+ )
+ if over_dispatched:
+ raise UserError(
+ _(
+ "Dispatch %(picking)s would ship more than request "
+ "%(request)s allocated:\n\n%(details)s\n\n"
+ "Reduce the quantities, or allocate more stock to the "
+ "request first.",
+ picking=picking.name,
+ request=picking.drims_request_id.reference,
+ details="\n".join(over_dispatched),
+ )
+ )
+
def button_validate(self):
"""Override button_validate to enforce beneficiary validation and invalidate cache.
When a request_dispatch picking is validated, this:
- 1. Validates that beneficiary tracking fields are filled (beneficiary_count, beneficiary_area_id)
- 2. Invalidates the cached KPI values to ensure dashboard shows current data
+ 1. Refuses items or quantities the request never approved (OP#1057)
+ 2. Validates that beneficiary tracking fields are filled (beneficiary_count, beneficiary_area_id)
+ 3. Invalidates the cached KPI values to ensure dashboard shows current data
"""
+ self._check_drims_dispatch_matches_request()
+
# Validate beneficiary tracking for DRIMS dispatches
for picking in self:
if picking.drims_type == "request_dispatch":
diff --git a/spp_drims/readme/HISTORY.md b/spp_drims/readme/HISTORY.md
index 7d77d616..8313a6a6 100644
--- a/spp_drims/readme/HISTORY.md
+++ b/spp_drims/readme/HISTORY.md
@@ -1,3 +1,7 @@
+### 19.0.4.0.1
+
+- fix(drims): only let a dispatch ship what its request approved. Products cannot be added to a request dispatch and quantities cannot be raised past what was allocated — enforced on the model, so imports and API callers are covered too, with the Operations tab's Add a line and delete affordances hidden to match (#1057)
+
### 19.0.4.0.0
- feat(drims): Donations review — creation, receipt, inspection and follow-up. Donations start in a new **Draft** state; the donor list is limited to organisations whose role is Donor and a donation cannot be recorded against a closed incident; at least one item is required to save, Pledged must be entered and be greater than zero, and Received is entered manually rather than copied from Pledged. Line columns appear progressively through the lifecycle (Received and Variance from Announced; Condition and Action from Inspected), non-accepted items gain a follow-up/disposal trail, and adding an item is blocked once the donation has moved past its editable states (#1055, #1058, #1108, #1163)
diff --git a/spp_drims/static/description/index.html b/spp_drims/static/description/index.html
index e908b20d..09f5cd08 100644
--- a/spp_drims/static/description/index.html
+++ b/spp_drims/static/description/index.html
@@ -565,6 +565,16 @@
+
19.0.4.0.1
+
+- fix(drims): only let a dispatch ship what its request approved.
+Products cannot be added to a request dispatch and quantities cannot
+be raised past what was allocated — enforced on the model, so imports
+and API callers are covered too, with the Operations tab’s Add a line
+and delete affordances hidden to match (#1057)
+
+
19.0.4.0.0
- feat(drims): Donations review — creation, receipt, inspection and
@@ -584,7 +594,7 @@
19.0.4.0.0
but no longer readable through the ORM or shown in any view (#1076)
-
+
19.0.3.1.0
- feat(drims): Incident Management review — incidents are entered as a
@@ -601,7 +611,7 @@
19.0.3.1.0
refresh cron skips (#1100)
-
+
19.0.3.0.4
- feat(drims): rework the dispatch page and correct the waybill.
@@ -621,7 +631,7 @@
19.0.3.0.4
barcode (#1151)
-
+
19.0.3.0.1
- fix(drims): a dispatch validated short no longer leaves the request
@@ -633,7 +643,7 @@
19.0.3.0.1
API (#1087)
-
+
19.0.3.0.0
- feat(drims): allocate stock per source warehouse. The Allocate Stock
@@ -653,7 +663,7 @@
19.0.3.0.0
destination-type selector (#1075)
-
+
19.0.2.0.0
- Initial migration to OpenSPP2
diff --git a/spp_drims/tests/__init__.py b/spp_drims/tests/__init__.py
index d3ae52a3..3deaf2c1 100644
--- a/spp_drims/tests/__init__.py
+++ b/spp_drims/tests/__init__.py
@@ -5,6 +5,7 @@
from . import test_allocation_preview_wizard
from . import test_approval
from . import test_coordination
+from . import test_dispatch_line_lock
from . import test_dispatch_page
from . import test_dispatch_backorder
from . import test_donation
diff --git a/spp_drims/tests/test_dispatch_line_lock.py b/spp_drims/tests/test_dispatch_line_lock.py
new file mode 100644
index 00000000..efbcee3a
--- /dev/null
+++ b/spp_drims/tests/test_dispatch_line_lock.py
@@ -0,0 +1,304 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+from datetime import date, timedelta
+
+from lxml import etree
+
+from odoo.exceptions import UserError
+from odoo.tests import tagged
+
+from .common import DrimsTestCommon
+
+
+@tagged("post_install", "-at_install")
+class TestDrimsDispatchLineLock(DrimsTestCommon):
+ """OP#1057: a dispatch may only ship what its request approved.
+
+ The Operations tab stays editable while a dispatch is Ready, so products
+ could be added and quantities inflated past the approval workflow. These
+ tests cover the model-side guard, which is what protects RPC and imports —
+ the view attributes only stop the UI inviting the mistake.
+ """
+
+ def setUp(self):
+ super().setUp()
+ self.future_date = date.today() + timedelta(days=30)
+ self.rogue_product = self.env["product.product"].create(
+ {
+ "name": "Unapproved Item",
+ "type": "consu",
+ "is_storable": True,
+ "standard_price": 999.0,
+ }
+ )
+
+ # ------------------------------------------------------------------
+ # helpers
+ # ------------------------------------------------------------------
+
+ def _stock_up(self, product, quantity):
+ self.env["stock.quant"].create(
+ {
+ "product_id": product.id,
+ "location_id": self.warehouse.lot_stock_id.id,
+ "quantity": quantity,
+ }
+ )
+
+ def _allocate(self, line, quantity, warehouse=None):
+ """Record a per-warehouse allocation for ``line``.
+
+ OP#1079 replaced the writable ``quantity_allocated`` on the request line
+ with a stored compute over ``allocation_ids``, so allocation has to be
+ expressed as a row against a warehouse.
+ """
+ return self.env["spp.drims.request.allocation"].create(
+ {
+ "request_line_id": line.id,
+ "warehouse_id": (warehouse or self.warehouse).id,
+ "quantity_allocated": quantity,
+ }
+ )
+
+ def _dispatch_for(self, requested=100, allocated=100):
+ """An allocated request plus its confirmed dispatch, ready to validate."""
+ request = self.env["spp.drims.request"].create(
+ {
+ "incident_id": self.incident.id,
+ "destination_area_id": self.area.id,
+ "date_needed": self.future_date,
+ "line_ids": [
+ (
+ 0,
+ 0,
+ {
+ "product_id": self.product.id,
+ "quantity_requested": requested,
+ "uom_id": self.product.uom_id.id,
+ },
+ )
+ ],
+ }
+ )
+ request.action_submit()
+ request.action_approve()
+ self._allocate(request.line_ids[0], allocated)
+ request.state_id = self.env["spp.vocabulary.code"].search(
+ [
+ ("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:drims:request-states"),
+ ("code", "=", "allocated"),
+ ],
+ limit=1,
+ )
+ request.action_create_dispatch()
+ picking = request.picking_ids
+ picking.write({"beneficiary_count": 500, "beneficiary_area_id": self.area.id})
+ return request, picking
+
+ def _pick_everything(self, picking):
+ for move in picking.move_ids:
+ move.quantity = move.product_uom_qty
+ picking.move_ids.picked = True
+
+ # ------------------------------------------------------------------
+ # extra products
+ # ------------------------------------------------------------------
+
+ def test_added_product_blocks_validation(self):
+ """A product the request never asked for must stop the dispatch."""
+ self._stock_up(self.product, 100)
+ self._stock_up(self.rogue_product, 100)
+ _request, picking = self._dispatch_for()
+
+ self.env["stock.move"].create(
+ {
+ "picking_id": picking.id,
+ "product_id": self.rogue_product.id,
+ "product_uom_qty": 50.0,
+ "location_id": picking.location_id.id,
+ "location_dest_id": picking.location_dest_id.id,
+ }
+ )
+ self._pick_everything(picking)
+
+ with self.assertRaises(UserError) as cm:
+ picking.button_validate()
+ self.assertIn("Unapproved Item", str(cm.exception))
+ self.assertNotEqual(picking.state, "done")
+
+ def test_guard_is_not_keyed_on_the_additional_flag(self):
+ """The check must catch moves Odoo never flagged as ``additional``.
+
+ ``additional`` is only set when a line is added through the form, so a
+ move created over RPC or by an import leaves it False. Keying the guard on
+ it would leave exactly that hole.
+ """
+ self._stock_up(self.product, 100)
+ self._stock_up(self.rogue_product, 100)
+ _request, picking = self._dispatch_for()
+
+ rogue_move = self.env["stock.move"].create(
+ {
+ "picking_id": picking.id,
+ "product_id": self.rogue_product.id,
+ "product_uom_qty": 50.0,
+ "location_id": picking.location_id.id,
+ "location_dest_id": picking.location_dest_id.id,
+ }
+ )
+ self.assertFalse(rogue_move.additional, "precondition: RPC-created moves are not 'additional'")
+ self._pick_everything(picking)
+
+ with self.assertRaises(UserError):
+ picking.button_validate()
+
+ def test_move_from_another_request_blocks_validation(self):
+ """A move linked to a different request's line is still not approved here.
+
+ The message names the dispatch's own request — the one whose approval was
+ bypassed — rather than the request the line was borrowed from.
+ """
+ self._stock_up(self.product, 300)
+ request_a, picking_a = self._dispatch_for()
+ _request_b, picking_b = self._dispatch_for()
+
+ # Smuggle B's move onto A's dispatch.
+ picking_b.move_ids[0].picking_id = picking_a.id
+ self._pick_everything(picking_a)
+
+ with self.assertRaises(UserError) as cm:
+ picking_a.button_validate()
+ message = str(cm.exception)
+ self.assertIn(request_a.reference, message)
+ self.assertIn(self.product.display_name, message)
+ self.assertNotEqual(picking_a.state, "done")
+
+ # ------------------------------------------------------------------
+ # inflated quantities
+ # ------------------------------------------------------------------
+
+ def test_demand_raised_above_allocation_blocks_validation(self):
+ """Unlocking and raising Demand must not ship more than was allocated."""
+ self._stock_up(self.product, 300)
+ _request, picking = self._dispatch_for(requested=100, allocated=100)
+
+ picking.is_locked = False
+ move = picking.move_ids[0]
+ self.assertTrue(move.is_initial_demand_editable, "precondition: unlocking frees Demand")
+ move.product_uom_qty = 150.0
+ self._pick_everything(picking)
+
+ with self.assertRaises(UserError) as cm:
+ picking.button_validate()
+ self.assertIn("more than", str(cm.exception))
+ self.assertNotEqual(picking.state, "done")
+
+ def test_picked_quantity_above_allocation_blocks_validation(self):
+ """Over-picking beyond the allocation is refused even with Demand intact."""
+ self._stock_up(self.product, 300)
+ _request, picking = self._dispatch_for(requested=100, allocated=100)
+
+ move = picking.move_ids[0]
+ move.quantity = 130.0
+ move.picked = True
+
+ with self.assertRaises(UserError):
+ picking.button_validate()
+
+ # ------------------------------------------------------------------
+ # legitimate flows must be untouched
+ # ------------------------------------------------------------------
+
+ def test_full_dispatch_validates(self):
+ """The ordinary case still works."""
+ self._stock_up(self.product, 100)
+ _request, picking = self._dispatch_for()
+ self._pick_everything(picking)
+
+ picking.button_validate()
+
+ self.assertEqual(picking.state, "done")
+
+ def test_partial_dispatch_validates(self):
+ """Shipping less than Demand must stay allowed.
+
+ This is how a partial dispatch and its backorder are produced (OP#1087),
+ so the guard must not treat under-picking as a violation.
+ """
+ self._stock_up(self.product, 100)
+ _request, picking = self._dispatch_for()
+ move = picking.move_ids[0]
+ move.quantity = 90.0
+ move.picked = True
+
+ # No UserError from the DRIMS guard; Odoo asks about the backorder.
+ action = picking.button_validate()
+ self.assertIsInstance(action, dict)
+ self.assertEqual(action["res_model"], "stock.backorder.confirmation")
+
+ def test_second_dispatch_after_top_up_validates(self):
+ """The cumulative check must not false-positive across dispatches."""
+ self._stock_up(self.product, 5000)
+ request, first = self._dispatch_for(requested=5000, allocated=2000)
+ self._pick_everything(first)
+ first.button_validate()
+ self.assertEqual(first.state, "done")
+
+ # Allocate the rest and dispatch again.
+ request.line_ids[0].allocation_ids[0].quantity_allocated = 5000
+ request.action_create_dispatch()
+ second = request.picking_ids - first
+ second.write({"beneficiary_count": 200, "beneficiary_area_id": self.area.id})
+ self._pick_everything(second)
+
+ second.button_validate()
+
+ self.assertEqual(second.state, "done")
+ self.assertEqual(request.line_ids[0].quantity_dispatched, 5000)
+
+ def test_non_drims_picking_is_unaffected(self):
+ """The guard must only apply to DRIMS request dispatches."""
+ self._stock_up(self.rogue_product, 50)
+ picking = self.env["stock.picking"].create(
+ {
+ "picking_type_id": self.warehouse.out_type_id.id,
+ "location_id": self.warehouse.lot_stock_id.id,
+ "location_dest_id": self.env.ref("stock.stock_location_customers").id,
+ "move_ids": [
+ (
+ 0,
+ 0,
+ {
+ "product_id": self.rogue_product.id,
+ "product_uom_qty": 10.0,
+ "location_id": self.warehouse.lot_stock_id.id,
+ "location_dest_id": self.env.ref("stock.stock_location_customers").id,
+ },
+ )
+ ],
+ }
+ )
+ picking.action_confirm()
+ self._pick_everything(picking)
+
+ picking.button_validate()
+
+ self.assertEqual(picking.state, "done")
+
+ def test_the_operations_field_denies_create_and_delete_on_a_dispatch(self):
+ """Pin the options dict, because the first attempt at this was silent.
+
+ Bare create/delete attributes on the field tag are ignored — QA caught
+ that — so the guard rides on `options`, whose entries are domains
+ evaluated against the picking. A regression here would restore the Add
+ a line and trash affordances with nothing failing to say so
+ (OP#1057 review).
+ """
+ view = self.env.ref("stock.view_picking_form")
+ arch = etree.fromstring(self.env["stock.picking"].get_view(view.id, "form")["arch"])
+ moves = arch.xpath("//page[@name='operations']/field[@name='move_ids']")
+
+ self.assertTrue(moves, "the Operations page should still carry move_ids")
+ options = moves[0].get("options") or ""
+ for action in ("create", "delete"):
+ self.assertIn(f"'{action}'", options, f"{action} must be denied through options")
+ self.assertIn("request_dispatch", options, "the denial is scoped to DRIMS dispatches")
diff --git a/spp_drims/views/stock_picking_views.xml b/spp_drims/views/stock_picking_views.xml
index 13d539c9..f4b50f38 100644
--- a/spp_drims/views/stock_picking_views.xml
+++ b/spp_drims/views/stock_picking_views.xml
@@ -218,13 +218,43 @@
partial dispatch and its backorder are produced (OP#1087) — so it
took the short-shipment flow out with it.
- The lock now lives in OP#1057, which sets create/delete domains
- in the field's ``options`` so lines cannot be added or removed
- while Quantity stays editable, and adds
- stock.picking._check_drims_dispatch_matches_request to refuse at
- validation anything the request never approved. That guard also
- covers RPC and imports, which a view attribute cannot.
+ OP#1057: a request dispatch may only ship what its request
+ approved, so its line-up is locked — no "Add a Product" row and
+ no deleting lines.
+
+ Quantity stays editable on purpose. OP#1075 first locked this
+ list by making the whole field readonly, but 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 that attribute took the short-shipment flow out with it and
+ has since been removed. Hence create/delete domains rather than
+ a wholesale readonly.
+
+ These have to go in ``options`` rather than being bare
+ ``create``/``delete`` attributes on the field. The x2many field
+ passes its ``options`` dict through as ``crudOptions``
+ (``x2ManyField.extractProps``), and useActiveActions evaluates
+ each entry as a *domain* against the parent record
+ (``new Domain(action).contains(evalContext)`` in the web client's
+ relational_utils.js) — which is what lets them depend on
+ drims_type. Bare create/delete attributes on the field tag are
+ ignored; only the inner honours those, and only as static
+ booleans. ``drims_type`` is in the DRIMS page below, so it is
+ loaded and present in the eval context.
+
+ The model-side guard in _check_drims_dispatch_matches_request is
+ what actually enforces this — it refuses at validation anything
+ the request never approved, covering RPC and imports too. These
+ options only keep the UI from inviting the mistake.
-->
+
+ {'create': [('drims_type', '!=', 'request_dispatch')], 'delete': [('drims_type', '!=', 'request_dispatch')]}
+