diff --git a/spp_drims/README.rst b/spp_drims/README.rst index 35393c23..2c37a3d2 100644 --- a/spp_drims/README.rst +++ b/spp_drims/README.rst @@ -179,6 +179,17 @@ Dependencies Changelog ========= +19.0.3.0.1 +~~~~~~~~~~ + +- fix(drims): a dispatch validated short no longer leaves the request + looking fully dispatched. The backorder is announced on the request + with a to-do for the coordinators, the request reopens as Ready for + Dispatch so the remaining balance can be dispatched again, and the + dispatched totals are rebuilt on the allocation rows. Applies however + the transfer is validated — the web client, the barcode flow or the + API (#1087) + 19.0.3.0.0 ~~~~~~~~~~ diff --git a/spp_drims/__manifest__.py b/spp_drims/__manifest__.py index e775182a..5f36b0e9 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.3.0.0", + "version": "19.0.3.0.1", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_drims/models/request.py b/spp_drims/models/request.py index 5f455f3e..b20c3ee1 100644 --- a/spp_drims/models/request.py +++ b/spp_drims/models/request.py @@ -2,6 +2,8 @@ import logging from datetime import timedelta +from markupsafe import Markup + from odoo import _, api, fields, models from odoo.exceptions import UserError, ValidationError @@ -609,13 +611,7 @@ def action_resubmit(self): def _set_state_by_code(self, code): """Set the request state to the vocab code, if it exists.""" self.ensure_one() - state = self.env["spp.vocabulary.code"].search( - [ - ("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:drims:request-states"), - ("code", "=", code), - ], - limit=1, - ) + state = self._get_state_by_code(code) if state: self.state_id = state return state @@ -878,3 +874,161 @@ def action_view_pickings(self): "view_mode": "list,form", "domain": [("drims_request_id", "=", self.id)], } + + # ------------------------------------------------------------------ + # Dispatch backorders (OP#1087) + # ------------------------------------------------------------------ + + def _get_state_by_code(self, code): + """Return the request-state vocabulary code record matching ``code``.""" + return self.env["spp.vocabulary.code"].search( + [ + ( + "vocabulary_id.namespace_uri", + "=", + "urn:openspp:vocab:drims:request-states", + ), + ("code", "=", code), + ], + limit=1, + ) + + def _get_drims_coordinators(self): + """Coordinators accountable for this request's destination area. + + Mirrors ``rule_request_coordinator_scope`` (security/rules.xml), which + grants a coordinator access when ``destination_area_id`` is a descendant + of one of their ``drims_area_ids``. The reverse lookup therefore matches + coordinators assigned to the destination area itself or to any of its + ancestors. Runs sudo because the warehouse officer triggering this has no + read access to other users' area assignments. + """ + self.ensure_one() + group = self.env.ref( + "spp_drims.group_drims_coordinator_supervisor", + raise_if_not_found=False, + ) + area = self.destination_area_id + if not group or not area: + return self.env["res.users"] + # ``parent_path`` is a "1/4/9/" style chain that already includes self. + area_ids = {int(part) for part in (area.parent_path or "").split("/") if part} + area_ids.add(area.id) + # Resolving who to notify about a backorder. Reads res.users only to + # collect recipient ids for a message; the acting warehouse officer has + # no reason to be able to search users, and nothing about them is + # exposed beyond being messaged. + users = self.env["res.users"].sudo() # nosemgrep: odoo-sudo-on-sensitive-models,odoo-sudo-without-context + return users.search( + [ + ("all_group_ids", "in", group.id), + ("drims_area_ids", "in", list(area_ids)), + ] + ) + + def _notify_dispatch_backorder(self, backorder): + """Announce a dispatch backorder on the request and assign a follow-up. + + Odoo creates backorders silently, so without this the coordinator gets no + signal that part of an approved request never left the warehouse. Posted + as an internal note addressed to the coordinators (rather than a customer + message) so they are notified without mailing external partners, plus a + to-do activity so the outstanding balance is owned by somebody. + """ + self.ensure_one() + items = Markup("").join( + Markup("
  • %s: %s %s
    • ") + % ( + move.product_id.display_name, + move.product_uom_qty, + move.product_uom.name, + ) + for move in backorder.move_ids + ) + # Named explicitly because the note is posted through sudo, which makes + # OdooBot its author. In a humanitarian-accountability trail, "who + # shipped short" is part of the record (OP#1087 review). + body = Markup("

    %s

    ") % ( + _( + "Dispatch %(parent)s was validated short of its demand by %(user)s. Backorder " + "%(backorder)s holds the remaining balance and has not been dispatched yet.", + parent=backorder.backorder_id.name or _("(unknown)"), + user=self.env.user.display_name, + backorder=backorder.name, + ), + items, + ) + coordinators = self._get_drims_coordinators() + self.message_post( + body=body, + partner_ids=coordinators.partner_id.ids, + subtype_xmlid="mail.mt_note", + ) + for coordinator in coordinators: + self.activity_schedule( + "mail.mail_activity_data_todo", + summary=_("Release dispatch backorder %s", backorder.name), + user_id=coordinator.id, + ) + + def _on_dispatch_backorder_created(self, backorder): + """React to Odoo splitting off a backorder from one of this request's dispatches. + + The backordered quantity never left the warehouse, so a request that had + already advanced to ``dispatched`` reopens at ``allocated`` (Ready for + Dispatch) until the backorder is validated too. Runs sudo because the + warehouse officer validating the short dispatch may sit outside the + request's area scope, and this is system bookkeeping rather than a user + edit. + """ + self.ensure_one() + request = self.sudo() # nosemgrep: odoo-sudo-without-context + request._notify_dispatch_backorder(backorder) + if request.state == "dispatched": + allocated_state = request._get_state_by_code("allocated") + if allocated_state: + request.state_id = allocated_state + + def _reopen_if_not_fully_dispatched(self): + """Drop back to ``allocated`` when the dispatched balance no longer covers + the request. + + Called after a dispatch quantity is released (see + ``stock.move._action_cancel``): a request that reads ``dispatched`` while + part of it was cancelled rather than shipped has to become actionable + again. + + Runs sudo for the same reason as the caller: the officer releasing the + quantity may sit outside the request's area scope, and moving the + request back to actionable is bookkeeping rather than a user edit. + """ + for rec in self.sudo(): # nosemgrep: odoo-sudo-without-context + if rec.state != "dispatched" or not rec.line_ids: + continue + if all(line.quantity_dispatched >= line.quantity_requested for line in rec.line_ids): + continue + allocated_state = rec._get_state_by_code("allocated") + if allocated_state: + rec.state_id = allocated_state + + def _sync_state_after_dispatch_done(self): + """Re-advance to ``dispatched`` once no dispatch of this request is pending. + + Counterpart to ``_on_dispatch_backorder_created``: when the outstanding + backorder is finally validated, the request returns to ``dispatched``. + + Runs sudo for the same reason as its counterpart: the validating officer + need not have write access to the request under the area record rules. + """ + for rec in self.sudo(): # nosemgrep: odoo-sudo-without-context + if rec.state != "allocated": + continue + pending = rec.picking_ids.filtered( + lambda p: p.drims_type == "request_dispatch" and p.state not in ("done", "cancel") + ) + if pending: + continue + if rec.line_ids and all(line.quantity_dispatched >= line.quantity_requested for line in rec.line_ids): + dispatched_state = rec._get_state_by_code("dispatched") + if dispatched_state: + rec.state_id = dispatched_state diff --git a/spp_drims/models/request_line.py b/spp_drims/models/request_line.py index b465d3d6..b03873b6 100644 --- a/spp_drims/models/request_line.py +++ b/spp_drims/models/request_line.py @@ -153,6 +153,55 @@ def _compute_fulfillment(self): else: line.fulfillment_pct = 0.0 + def _reconcile_quantity_dispatched(self): + """Recompute ``quantity_dispatched`` from the dispatch moves that still stand. + + ``quantity_dispatched`` counts quantity committed to a dispatch picking, + which ``spp.drims.request.action_create_dispatch`` increments when the + picking is created rather than when it ships. Once moves are validated or + cancelled that running total can drift from reality, so it is rebuilt + here (OP#1087): + + - a cancelled move never shipped and no longer counts at all; + - a done move counts what actually moved, not what was demanded, which is + what makes declining "Create Backorder" release the balance; + - a move still in progress keeps counting its demand, so a pending + backorder stays committed to the request. + + Rebuilt onto the **per-warehouse allocation rows** rather than onto the + line (OP#1079). The line's ``quantity_dispatched`` is a stored compute + summing ``allocation_ids.quantity_dispatched``, so assigning to it + directly no longer registers at all — the write is silently discarded + and the released quantity never comes back, which is what broke the + backorder flow after the per-warehouse allocation model landed. + + Splitting by allocation is unambiguous because every dispatch move + carries the allocation it draws from: ``action_create_dispatch`` creates + one move per allocation and stamps ``drims_allocation_id`` on it, and a + backorder copies that link along with the rest of the move. + + ``quantity`` and ``product_uom_qty`` are both expressed in the move's + ``product_uom``, which the dispatch sets from the allocation's + ``uom_id``, so the two are directly comparable. + + Runs sudo: warehouse staff validating or cancelling a dispatch need not + have write access to the request under the area record rules, and this is + system bookkeeping rather than a user edit. + """ + Move = self.env["stock.move"].sudo() # nosemgrep: odoo-sudo-without-context + for allocation in self.sudo().mapped("allocation_ids"): # nosemgrep: odoo-sudo-without-context + dispatched = 0.0 + for move in Move.search( + [ + ("drims_allocation_id", "=", allocation.id), + ("state", "!=", "cancel"), + ] + ): + dispatched += move.quantity if move.state == "done" else move.product_uom_qty + allocation.quantity_dispatched = dispatched + # nosemgrep: odoo-sudo-without-context + self.sudo().request_id._reopen_if_not_fully_dispatched() + @api.onchange("product_id") def _onchange_product_id(self): if self.product_id: diff --git a/spp_drims/models/stock_move.py b/spp_drims/models/stock_move.py index e0c21ec8..a3ac75ac 100644 --- a/spp_drims/models/stock_move.py +++ b/spp_drims/models/stock_move.py @@ -22,6 +22,30 @@ class StockMove(models.Model): help="Link to the donation line this move receives", ) + def _action_done(self, cancel_backorder=False): + """Reconcile the request's dispatch counter once moves are validated. + + Needed because Odoo has more than one way to drop undelivered demand: + declining "Create Backorder" leaves the move done at the picked quantity + without cancelling anything, so the shortfall is invisible to a + cancellation hook (OP#1087). + """ + lines = self.drims_request_line_id + result = super()._action_done(cancel_backorder=cancel_backorder) + lines.exists()._reconcile_quantity_dispatched() + return result + + def _action_cancel(self): + """Reconcile the request's dispatch counter when moves are cancelled. + + Covers a cancelled backorder and a cancelled dispatch: neither quantity + ever shipped, so neither may keep counting as dispatched (OP#1087). + """ + lines = self.drims_request_line_id + result = super()._action_cancel() + lines.exists()._reconcile_quantity_dispatched() + return result + @api.model def _prepare_merge_moves_distinct_fields(self): """Keep DRIMS donation/request lines distinct when Odoo merges moves. diff --git a/spp_drims/models/stock_picking.py b/spp_drims/models/stock_picking.py index 703c4403..4bd30b0f 100644 --- a/spp_drims/models/stock_picking.py +++ b/spp_drims/models/stock_picking.py @@ -38,6 +38,7 @@ class StockPicking(models.Model): "spp.drims.return", string="DRIMS Return", index=True, + copy=False, ) incident_id = fields.Many2one( "spp.hazard.incident", @@ -53,6 +54,7 @@ class StockPicking(models.Model): ) beneficiary_count = fields.Integer( string="Estimated Beneficiaries Reached", + copy=False, help="Estimated number of beneficiaries who received items (exact counts often unknown in emergencies)", ) distribution_type_id = fields.Many2one( @@ -84,49 +86,61 @@ class StockPicking(models.Model): ) # Transport + # Every field below records what happened on one physical shipment, so none + # of them may be carried onto a copy of the picking. Odoo builds a backorder + # with ``picking.copy()`` (``stock.picking._create_backorder_picking``), so + # without ``copy=False`` a backorder inherits the parent's departure + # timestamp, driver, POD and beneficiary count — claiming a delivery for + # goods still sitting in the warehouse, and double-counting the parent's + # beneficiaries in ``spp.hazard.incident.drims_beneficiaries_served`` + # (OP#1087). The same applies to the Duplicate action. transport_mode_id = fields.Many2one( "spp.vocabulary.code", string="Transport Mode", domain="[('vocabulary_id.namespace_uri', '=', 'urn:openspp:vocab:drims:transport-modes')]", + copy=False, ) - vehicle_registration = fields.Char(string="Vehicle Registration") - driver_name = fields.Char(string="Driver Name") - driver_phone = fields.Char(string="Driver Phone") + vehicle_registration = fields.Char(string="Vehicle Registration", copy=False) + driver_name = fields.Char(string="Driver Name", copy=False) + driver_phone = fields.Char(string="Driver Phone", copy=False) # Proof of Delivery (POD) pod_status_id = fields.Many2one( "spp.vocabulary.code", string="POD Status", domain="[('vocabulary_id.namespace_uri', '=', 'urn:openspp:vocab:drims:pod-statuses')]", + copy=False, ) is_pod_confirmed = fields.Boolean( string="POD Confirmed", default=False, + copy=False, ) - pod_received_by = fields.Char(string="Received By") - pod_receiver_title = fields.Char(string="Receiver Title") - pod_receiver_id_number = fields.Char(string="Receiver ID Number") - pod_signature = fields.Binary(string="Signature") + pod_received_by = fields.Char(string="Received By", copy=False) + pod_receiver_title = fields.Char(string="Receiver Title", copy=False) + pod_receiver_id_number = fields.Char(string="Receiver ID Number", copy=False) + pod_signature = fields.Binary(string="Signature", copy=False) pod_photo_ids = fields.Many2many( "ir.attachment", string="Delivery Photos", + copy=False, ) - pod_gps_latitude = fields.Float(string="GPS Latitude", digits=(10, 6)) - pod_gps_longitude = fields.Float(string="GPS Longitude", digits=(10, 6)) + pod_gps_latitude = fields.Float(string="GPS Latitude", digits=(10, 6), copy=False) + pod_gps_longitude = fields.Float(string="GPS Longitude", digits=(10, 6), copy=False) pod_gps_point = fields.GeoPointField( string="POD GPS Point", compute="_compute_pod_gps_point", store=True, help="Computed geographic point from POD GPS coordinates for GIS mapping", ) - pod_notes = fields.Text(string="POD Notes") + pod_notes = fields.Text(string="POD Notes", copy=False) # Dates - date_departed = fields.Datetime(string="Departed At") - date_arrived = fields.Datetime(string="Arrived At") + date_departed = fields.Datetime(string="Departed At", copy=False) + date_arrived = fields.Datetime(string="Arrived At", copy=False) # Discrepancy - discrepancy_notes = fields.Text(string="Discrepancy Notes") + discrepancy_notes = fields.Text(string="Discrepancy Notes", copy=False) @api.model_create_multi def create(self, vals_list): @@ -306,6 +320,40 @@ def button_validate(self): return result + def _action_done(self): + """Settle the request's state once a dispatch is really done. + + This used to hang off button_validate, which only covers the web + client's Validate button: a backorder released through the API, the + barcode flow or a direct _action_done reconciled its quantities through + the move hook but never re-advanced the request, leaving it at + "allocated" with everything already shipped (OP#1087 review). + + _action_done is the point every path goes through, and calling it after + super() means Odoo has already split off any backorder — which is what + _sync_state_after_dispatch_done inspects before advancing. + """ + result = super()._action_done() + + requests = self.filtered(lambda p: p.drims_type == "request_dispatch").drims_request_id + if requests: + requests._sync_state_after_dispatch_done() + + return result + + def _create_backorder(self, backorder_moves=None): + """Surface DRIMS dispatch backorders on their request (OP#1087). + + Odoo creates the backorder picking silently, so on its own a partially + validated dispatch leaves the coordinator with no notification and the + request still reading as fully dispatched. + """ + backorders = super()._create_backorder(backorder_moves=backorder_moves) + for backorder in backorders: + if backorder.drims_type == "request_dispatch" and backorder.drims_request_id: + backorder.drims_request_id._on_dispatch_backorder_created(backorder) + return backorders + def _invalidate_drims_kpi_cache(self, incident_ids): """Invalidate DRIMS KPI cache for distributed and stock values. diff --git a/spp_drims/readme/HISTORY.md b/spp_drims/readme/HISTORY.md index 41a4b4ea..8b23f40e 100644 --- a/spp_drims/readme/HISTORY.md +++ b/spp_drims/readme/HISTORY.md @@ -1,3 +1,7 @@ +### 19.0.3.0.1 + +- fix(drims): a dispatch validated short no longer leaves the request looking fully dispatched. The backorder is announced on the request with a to-do for the coordinators, the request reopens as Ready for Dispatch so the remaining balance can be dispatched again, and the dispatched totals are rebuilt on the allocation rows. Applies however the transfer is validated — the web client, the barcode flow or the API (#1087) + ### 19.0.3.0.0 - feat(drims): allocate stock per source warehouse. The Allocate Stock wizard now auto-splits each requested line across the DRIMS warehouses that hold stock (e.g. 70 → 50 @ WH1 + 20 @ WH2) with editable rows; the split is captured on a new per-warehouse allocation record, shown on the request's Allocations tab and summarised in a "Source Warehouse(s)" column on the Requests list; dispatch creates one picking per source warehouse. The single "Source Warehouse" field on the request has been removed — the warehouse(s) are chosen in the wizard. The allocation wizard distinguishes no-stock, stock-shortfall and deliberate partial-allocation cases with clear messages, and the request line's Fulfillment % tracks allocated ÷ requested so the bar reflects allocation progress (#1079) diff --git a/spp_drims/static/description/index.html b/spp_drims/static/description/index.html index f64fb3c6..26845b56 100644 --- a/spp_drims/static/description/index.html +++ b/spp_drims/static/description/index.html @@ -565,6 +565,18 @@

    Changelog

    +

    19.0.3.0.1

    + +
    +

    19.0.3.0.0

    -
    +

    19.0.2.0.0