diff --git a/spp_drims/README.rst b/spp_drims/README.rst index 42d0b62c8..4450a015c 100644 --- a/spp_drims/README.rst +++ b/spp_drims/README.rst @@ -179,6 +179,22 @@ Dependencies Changelog ========= +19.0.3.1.0 +~~~~~~~~~~ + +- feat(drims): Incident Management review — incidents are entered as a + **Draft** and then flagged Alert or set Active, a closed incident + refuses DRIMS operations and no longer accepts lifecycle changes, + dashboard KPI cards no longer open the record when a box is clicked, + warehouses can be linked to an incident and drive the warehouse + choices on donations and requests, and the Impact tab is hidden where + it does not apply (#1094, #1123, #1157, #1158, #1159, #1160, #1164) +- fix(drims): incident stock KPIs now count incident-related stock net + of allocations, and distributed value is net of confirmed returns. + Both are stored computes whose meaning changed, so upgrading + recomputes them for every incident — including closed ones, which the + refresh cron skips (#1100) + 19.0.3.0.4 ~~~~~~~~~~ diff --git a/spp_drims/__manifest__.py b/spp_drims/__manifest__.py index cea612fb4..17335a07f 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.4", + "version": "19.0.3.1.0", "sequence": 1, "author": "OpenSPP.org", "website": "https://github.com/OpenSPP/OpenSPP2", diff --git a/spp_drims/data/config_defaults.xml b/spp_drims/data/config_defaults.xml index f355d013e..e190706cd 100644 --- a/spp_drims/data/config_defaults.xml +++ b/spp_drims/data/config_defaults.xml @@ -90,4 +90,15 @@ drims.performance.kpi_cache_ttl_minutes 30 + + + + + + + drims.warehouse.filter_by_incident + True + diff --git a/spp_drims/migrations/19.0.3.1.0/post-migration.py b/spp_drims/migrations/19.0.3.1.0/post-migration.py new file mode 100644 index 000000000..32428bf33 --- /dev/null +++ b/spp_drims/migrations/19.0.3.1.0/post-migration.py @@ -0,0 +1,61 @@ +# Part of OpenSPP. See LICENSE file for full copyright and licensing details. +"""Recompute the incident KPIs whose meaning changed in this version (OP#1100). + +Three stored computes were redefined rather than added: + +* ``drims_total_stock_units`` and ``drims_stock_item_count`` went from + "everything physically in the warehouse" to "incident-related stock net of + what has been allocated out". +* ``drims_distributed_value`` is now net of confirmed returns. + +Odoo only computes a stored field for existing rows when its column is newly +created, so an upgraded database keeps the old numbers until something happens +to touch a dependency. For a quiet incident that may be never — and closed +incidents are excluded from ``_cron_refresh_drims_kpis``, so nothing would ever +correct them. + +The cached ``spp.data.value`` rows behind the value KPIs hold pre-change +numbers for the same reason, and the compute prefers a live cache entry over +recomputing, so they are dropped first. +""" + +import logging + +from odoo import SUPERUSER_ID, api + +_logger = logging.getLogger(__name__) + +STALE_CACHE_VARIABLES = ("drims_distributed_value", "drims_stock_value") + +RECOMPUTED_FIELDS = ( + "drims_total_stock_units", + "drims_stock_item_count", + "drims_distributed_value", +) + + +def migrate(cr, version): + if not version: + return + + env = api.Environment(cr, SUPERUSER_ID, {}) + + data_value = env.get("spp.data.value") + if data_value is not None: + for variable in STALE_CACHE_VARIABLES: + data_value.invalidate(variable_name=variable) + _logger.info("Dropped cached values for %s", ", ".join(STALE_CACHE_VARIABLES)) + + # Every incident, not just the open ones: the cron that would otherwise + # heal these skips closed incidents, which is exactly where a stale number + # would sit unnoticed. + incidents = env["spp.hazard.incident"].search([]) + if not incidents: + return + + for field_name in RECOMPUTED_FIELDS: + field = incidents._fields.get(field_name) + if field is not None: + env.add_to_compute(field, incidents) + env.flush_all() + _logger.info("Recomputed %s for %d incident(s)", ", ".join(RECOMPUTED_FIELDS), len(incidents)) diff --git a/spp_drims/models/donation.py b/spp_drims/models/donation.py index 5938145f3..8e2c9cdd9 100644 --- a/spp_drims/models/donation.py +++ b/spp_drims/models/donation.py @@ -96,6 +96,15 @@ class DrimsDonation(models.Model): string="Receiving Warehouse", required=True, tracking=True, + domain="[('id', 'in', allowed_warehouse_ids)]", + ) + # OP#1164: warehouses selectable for this donation — the incident's linked + # warehouses when incident-filtering is enabled, otherwise all DRIMS + # warehouses. Used to domain the Receiving Warehouse field. + allowed_warehouse_ids = fields.Many2many( + "stock.warehouse", + compute="_compute_allowed_warehouse_ids", + string="Allowed Warehouses", ) # Dates @@ -245,6 +254,15 @@ def _compute_has_acceptable_items(self): for line in rec.line_ids ) + @api.depends("incident_id", "incident_id.drims_warehouse_ids") + def _compute_allowed_warehouse_ids(self): + """OP#1164: selectable warehouses = the incident's warehouses when + incident-filtering is on (and the incident has any), else all DRIMS + warehouses. The fallback avoids locking out donation creation when an + incident has no warehouses linked yet.""" + for rec in self: + rec.allowed_warehouse_ids = rec.incident_id._drims_allowed_warehouses() + @api.depends("picking_ids") def _compute_picking_count(self): for rec in self: @@ -253,6 +271,9 @@ def _compute_picking_count(self): @api.model_create_multi def create(self, vals_list): for vals in vals_list: + # OP#1158: no new donations may be accepted for a closed incident. + if vals.get("incident_id"): + self.env["spp.hazard.incident"].browse(vals["incident_id"])._drims_ensure_open(_("accept a donation")) if vals.get("reference", _("New")) == _("New"): vals["reference"] = self.env["ir.sequence"].next_by_code("spp.drims.donation") or _("New") records = super().create(vals_list) diff --git a/spp_drims/models/hazard_incident.py b/spp_drims/models/hazard_incident.py index 0f4b4a51a..7e868194a 100644 --- a/spp_drims/models/hazard_incident.py +++ b/spp_drims/models/hazard_incident.py @@ -1,8 +1,10 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. import logging +from collections import defaultdict from datetime import timedelta -from odoo import api, fields, models +from odoo import _, api, fields, models +from odoo.exceptions import UserError _logger = logging.getLogger(__name__) @@ -112,6 +114,9 @@ class HazardIncident(models.Model): "stock.warehouse", string="DRIMS Warehouses", compute="_compute_drims_warehouses", + inverse="_inverse_drims_warehouses", + domain="[('is_drims_warehouse', '=', True)]", + help="Warehouses responding to this incident. Editable here or from a warehouse's Active Incidents.", ) drims_picking_ids = fields.One2many( "stock.picking", @@ -170,6 +175,21 @@ def _compute_drims_warehouses(self): ] ) + def _inverse_drims_warehouses(self): + """OP#1164: allow defining an incident's warehouses from the incident. + + Writes back to the ``stock.warehouse.incident_ids`` many2many (the same + link editable from the warehouse's "Active Incidents"), so both sides + stay in sync. Writing through ``stock.warehouse.write`` also lets the + OP#1094 stock-KPI cache invalidation fire for the affected incidents. + """ + Warehouse = self.env["stock.warehouse"] + for rec in self: + current = Warehouse.search([("incident_ids", "in", rec.id), ("is_drims_warehouse", "=", True)]) + target = rec.drims_warehouse_ids + (target - current).write({"incident_ids": [(4, rec.id)]}) + (current - target).write({"incident_ids": [(3, rec.id)]}) + @api.depends( "drims_donation_ids", "drims_donation_ids.total_value", @@ -214,74 +234,115 @@ def _compute_drims_kpis(self): rec.drims_donation_value = sum(donations.mapped("total_value")) _logger.debug("Cache miss for drims_donation_value on incident %s", rec.id) + def _drims_incident_stock_units(self): + """OP#1160: units + distinct-product count of stock related to THIS + incident, across any warehouse. + + "Related to the incident" = items stocked in from this incident's + donations, net of what its requests have committed out (allocated, + which already covers dispatched). This is deliberately NOT the physical + contents of the incident's warehouses (which mixes in unrelated stock). + + Returns: + tuple(float, int): (total net units, number of distinct products + with a positive net). + """ + self.ensure_one() + Picking = self.env["stock.picking"] + + # Stocked in: done moves on this incident's donation-receipt pickings. + stocked = defaultdict(float) + receipts = Picking.search( + [ + ("incident_id", "=", self.id), + ("drims_type", "=", "donation_receipt"), + ("state", "=", "done"), + ] + ) + for move in receipts.mapped("move_ids").filtered(lambda m: m.state == "done"): + stocked[move.product_id.id] += move.quantity + + # Committed out: quantity allocated by this incident's request lines + # (allocation covers what is subsequently dispatched). + allocated = defaultdict(float) + for line in self.drims_request_ids.mapped("line_ids"): + if line.quantity_allocated: + allocated[line.product_id.id] += line.quantity_allocated + + total_units = 0.0 + product_count = 0 + for product_id, qty_in in stocked.items(): + net = qty_in - allocated.get(product_id, 0.0) + if net > 0: + total_units += net + product_count += 1 + return total_units, product_count + + def _drims_distributed_net(self): + """OP#1160: distributed value net of returns. + + Value dispatched via completed request dispatches, minus the value of + returned items (returns that are past draft and not cancelled) — those + items came back and are therefore no longer distributed. Floored at 0. + """ + self.ensure_one() + Picking = self.env["stock.picking"] + pickings = Picking.search( + [ + ("incident_id", "=", self.id), + ("state", "=", "done"), + ("drims_type", "=", "request_dispatch"), + ] + ) + dispatched_value = 0.0 + for picking in pickings: + for move in picking.move_ids.filtered(lambda m: m.state == "done"): + dispatched_value += move.quantity * (move.product_id.standard_price or 0.0) + + returned_value = sum( + self.drims_return_ids.filtered(lambda r: r.state not in ("draft", "cancelled")).mapped("total_value") + ) + return max(0.0, dispatched_value - returned_value) + @api.depends( "drims_picking_ids", "drims_picking_ids.state", "drims_picking_ids.drims_type", + "drims_request_ids.line_ids.quantity_allocated", + "drims_request_ids.line_ids.product_id", + "drims_return_ids.total_value", + "drims_return_ids.state", ) def _compute_drims_stock_kpis(self): - """Compute stock and distributed values from warehouse data. + """Compute stock and distributed KPIs. - Uses hybrid approach for expensive aggregations: - 1. Try to read from spp.data.value cache - 2. Fall back to direct computation if cache miss - - Uses ORM-based aggregation for proper handling of Odoo 19 Properties fields. + - Units / product count (OP#1160): computed live from incident-related + stock (donations stocked in, net of request allocations). + - Stock value (monetary): warehouse contents, cached with fallback. + - Distributed value (OP#1160): dispatched net of returns, cached with + fallback. """ if not self: return DataValue = self.env["spp.data.value"] - # Try to read cached values for all records - # Note: variable_name implicitly identifies subject_model (no explicit filter needed) - cached_stock_values = DataValue.read_values( - "drims_stock_value", - self.ids, - period_key="current", - ) - cached_distributed_values = DataValue.read_values( - "drims_distributed_value", - self.ids, - period_key="current", - ) + # Cached monetary aggregations (variable_name implies subject_model). + cached_stock_values = DataValue.read_values("drims_stock_value", self.ids, period_key="current") + cached_distributed_values = DataValue.read_values("drims_distributed_value", self.ids, period_key="current") Warehouse = self.env["stock.warehouse"] Quant = self.env["stock.quant"] Picking = self.env["stock.picking"] for rec in self: - # Stock KPIs - use cache with fallback + # OP#1160: Units / Products — stock related to THIS incident. + rec.drims_total_stock_units, rec.drims_stock_item_count = rec._drims_incident_stock_units() + + # Stock value (monetary) still reflects warehouse contents — cached. if rec.id in cached_stock_values: rec.drims_stock_value = cached_stock_values[rec.id] - _logger.debug("Cache hit for drims_stock_value on incident %s", rec.id) - # Still need to compute units and item count (not cached) - warehouses = Warehouse.search( - [ - ("incident_ids", "in", rec.id), - ("is_drims_warehouse", "=", True), - ] - ) - if warehouses: - location_ids = warehouses.mapped("lot_stock_id").ids - if location_ids: - quants = Quant.search( - [ - ("location_id", "child_of", location_ids), - ("quantity", ">", 0), - ] - ) - rec.drims_total_stock_units = sum(quants.mapped("quantity")) - rec.drims_stock_item_count = len(quants.mapped("product_id")) - else: - rec.drims_total_stock_units = 0.0 - rec.drims_stock_item_count = 0 - else: - rec.drims_total_stock_units = 0.0 - rec.drims_stock_item_count = 0 else: - # Cache miss - compute directly using ORM - _logger.debug("Cache miss for drims_stock_value on incident %s", rec.id) warehouses = Warehouse.search( [ ("incident_ids", "in", rec.id), @@ -289,12 +350,9 @@ def _compute_drims_stock_kpis(self): ] ) stock_value = 0.0 - total_units = 0.0 - item_count = 0 if warehouses: location_ids = warehouses.mapped("lot_stock_id").ids if location_ids: - # Use ORM to properly handle standard_price Properties field quants = Quant.search( [ ("location_id", "child_of", location_ids), @@ -302,33 +360,15 @@ def _compute_drims_stock_kpis(self): ] ) stock_value = sum(q.quantity * (q.product_id.standard_price or 0.0) for q in quants) - total_units = sum(quants.mapped("quantity")) - item_count = len(quants.mapped("product_id")) rec.drims_stock_value = stock_value - rec.drims_total_stock_units = total_units - rec.drims_stock_item_count = item_count - # Distributed value - use cache with fallback + # OP#1160: Distributed = dispatched net of returns — cached with fallback. if rec.id in cached_distributed_values: rec.drims_distributed_value = cached_distributed_values[rec.id] - _logger.debug("Cache hit for drims_distributed_value on incident %s", rec.id) else: - # Cache miss - compute directly from completed dispatch pickings using ORM - _logger.debug("Cache miss for drims_distributed_value on incident %s", rec.id) - pickings = Picking.search( - [ - ("incident_id", "=", rec.id), - ("state", "=", "done"), - ("drims_type", "=", "request_dispatch"), - ] - ) - distributed_value = 0.0 - for picking in pickings: - for move in picking.move_ids.filtered(lambda m: m.state == "done"): - distributed_value += move.quantity * (move.product_id.standard_price or 0.0) - rec.drims_distributed_value = distributed_value + rec.drims_distributed_value = rec._drims_distributed_net() - # Beneficiaries served - count from completed dispatches in last 30 days + # Beneficiaries served - count from completed dispatches in last 30 days. thirty_days_ago = fields.Date.today() - timedelta(days=30) recent_pickings = Picking.search( [ @@ -340,6 +380,55 @@ def _compute_drims_stock_kpis(self): ) rec.drims_beneficiaries_served = sum(recent_pickings.mapped("beneficiary_count")) + def action_set_alert(self): + """OP#1157: flag the incident into the 'alert' state. + + The 'alert' status exists in the base state machine but had no setter; + this exposes it via the "Flag As Alert" header button. + + Goes through the base gate so a closed incident cannot be flagged back + into alert over RPC, which would sidestep every OP#1158 guard that keys + off `closed` (OP#1100 review). + """ + self._ensure_status_change_allowed("alert") + self.write({"status": "alert"}) + + def _drims_allowed_warehouses(self): + """OP#1164: the warehouses selectable for work on this incident. + + The incident's own warehouses when incident-filtering is on and it has + any, otherwise every DRIMS warehouse. The fallback is what stops an + incident with nothing linked yet from locking out donation and request + creation. + + Lives here rather than in donation.py and request.py, which each held a + verbatim copy, so the policy has one home (OP#1100 review). + + Callable on an empty incident — a donation or request that has not been + pointed at one yet — which then gets the full DRIMS list. The warehouse + search only runs on the fallback path, so the common case (filtering on, + incident has warehouses) costs no query per row when a list computes + this field. + """ + filter_on = self.env["res.config.settings"].is_warehouse_filter_by_incident_enabled() + if filter_on and self.drims_warehouse_ids: + return self.drims_warehouse_ids + return self.env["stock.warehouse"].search([("is_drims_warehouse", "=", True)]) + + def _drims_ensure_open(self, action): + """OP#1158: block DRIMS operations once an incident is closed. + + Raises a UserError if any incident in the recordset is closed. Callers + pass a short label (e.g. "approve a request") for the message. An empty + recordset is a no-op. + """ + for rec in self: + if rec.status == "closed": + raise UserError( + _("Cannot %(action)s: incident '%(name)s' is closed.") + % {"action": action, "name": rec.display_name} + ) + def action_view_drims_donations(self): """Open list view of donations for this incident. @@ -524,7 +613,7 @@ def _refresh_incident_kpi_cache(self): } ) - # 3. Compute distributed value + # 3. Compute distributed value (OP#1160: net of returns). pickings = Picking.search( [ ("incident_id", "=", incident.id), @@ -532,11 +621,8 @@ def _refresh_incident_kpi_cache(self): ("drims_type", "=", "request_dispatch"), ] ) - distributed_value = 0.0 picking_count = len(pickings) - for picking in pickings: - for move in picking.move_ids.filtered(lambda m: m.state == "done"): - distributed_value += move.quantity * (move.product_id.standard_price or 0.0) + distributed_value = incident._drims_distributed_net() values_to_upsert.append( { "variable_name": "drims_distributed_value", @@ -590,7 +676,7 @@ def _cron_refresh_drims_kpis(self): Called by scheduled action (recommended frequency: every 30 minutes). """ # Find active incidents (not closed) - # status field values: alert, active, recovery, closed + # status field values: draft, alert, active, recovery, closed active_incidents = self.search( [ ("status", "!=", "closed"), diff --git a/spp_drims/models/personnel.py b/spp_drims/models/personnel.py index 0fe1da9b7..47e0168fa 100644 --- a/spp_drims/models/personnel.py +++ b/spp_drims/models/personnel.py @@ -1,5 +1,5 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. -from odoo import api, fields, models +from odoo import _, api, fields, models class DrimsPersonnel(models.Model): @@ -151,6 +151,14 @@ def _onchange_partner_id(self): self.phone = self.phone or self.partner_id.phone self.email = self.email or self.partner_id.email + @api.model_create_multi + def create(self, vals_list): + # OP#1158: personnel cannot be deployed to a closed incident. + for vals in vals_list: + if vals.get("incident_id"): + self.env["spp.hazard.incident"].browse(vals["incident_id"])._drims_ensure_open(_("deploy personnel")) + return super().create(vals_list) + def action_mark_returned(self): """Mark personnel as returned.""" self.write( diff --git a/spp_drims/models/request.py b/spp_drims/models/request.py index b20c3ee17..fe5dd37ab 100644 --- a/spp_drims/models/request.py +++ b/spp_drims/models/request.py @@ -155,10 +155,18 @@ def _compute_source_warehouse_names(self): destination_warehouse_id = fields.Many2one( "stock.warehouse", string="Destination Warehouse", - domain="[('is_drims_warehouse', '=', True), ('area_id', '=', destination_area_id)]", + domain="[('id', 'in', allowed_warehouse_ids), ('area_id', '=', destination_area_id)]", tracking=True, help="Warehouse in the destination area to receive the dispatch", ) + # OP#1164: warehouses selectable for this request — the incident's linked + # warehouses when incident-filtering is enabled, otherwise all DRIMS + # warehouses. Used to domain the source/destination warehouse fields. + allowed_warehouse_ids = fields.Many2many( + "stock.warehouse", + compute="_compute_allowed_warehouse_ids", + string="Allowed Warehouses", + ) # Contact contact_name = fields.Char(string="Contact Name") @@ -354,6 +362,15 @@ def _compute_fulfillment_progress(self): rec.allocation_pct = 0 rec.fulfillment_pct = 0 + @api.depends("incident_id", "incident_id.drims_warehouse_ids") + def _compute_allowed_warehouse_ids(self): + """OP#1164: selectable warehouses = the incident's warehouses when + incident-filtering is on (and the incident has any), else all DRIMS + warehouses. The fallback avoids locking out allocation/dispatch when an + incident has no warehouses linked yet.""" + for rec in self: + rec.allowed_warehouse_ids = rec.incident_id._drims_allowed_warehouses() + @api.depends("picking_ids") def _compute_picking_count(self): for rec in self: @@ -435,6 +452,7 @@ def action_submit(self): limit=1, ) for rec in self: + rec.incident_id._drims_ensure_open(_("submit a request")) # OP#1158 if not rec.line_ids: raise UserError(_("Cannot submit request without items.")) rec.state_id = submitted_state @@ -481,6 +499,7 @@ def _on_reject(self): def action_approve(self): """Approve the request (for direct approval without workflow).""" for rec in self: + rec.incident_id._drims_ensure_open(_("approve a request")) # OP#1158 if rec.approval_state not in ("pending", "submitted"): raise UserError(_("Only pending requests can be approved.")) rec.approval_state = "approved" @@ -630,6 +649,7 @@ def action_allocate(self): state stays at Ready for Allocation. """ for rec in self: + rec.incident_id._drims_ensure_open(_("allocate a request")) # OP#1158 if rec.approval_state != "approved": raise UserError(_("Only approved requests can be allocated.")) rec._auto_allocate() diff --git a/spp_drims/models/res_config_settings.py b/spp_drims/models/res_config_settings.py index ba1233e47..5957b550d 100644 --- a/spp_drims/models/res_config_settings.py +++ b/spp_drims/models/res_config_settings.py @@ -181,3 +181,21 @@ def get_kpi_cache_ttl_minutes(self): # nosemgrep: odoo-sudo-without-context — standard Odoo pattern for system parameter access ICP = self.env["ir.config_parameter"].sudo() return int(ICP.get_param("drims.performance.kpi_cache_ttl_minutes", 30)) + + # ========================================================================= + # WAREHOUSE HELPERS + # ========================================================================= + + @api.model + def is_warehouse_filter_by_incident_enabled(self): + """Whether donation/request warehouse pickers are restricted to the + incident's linked warehouses (OP#1164). + + Returns: + bool: True if filtering is enabled (default True). Set the + ``drims.warehouse.filter_by_incident`` parameter to False to allow + selecting from any DRIMS warehouse. + """ + # nosemgrep: odoo-sudo-without-context — standard Odoo pattern for system parameter access + ICP = self.env["ir.config_parameter"].sudo() + return ICP.get_param("drims.warehouse.filter_by_incident", "True") == "True" diff --git a/spp_drims/models/returns.py b/spp_drims/models/returns.py index 4631d97c0..8603dbe0d 100644 --- a/spp_drims/models/returns.py +++ b/spp_drims/models/returns.py @@ -325,10 +325,11 @@ def _invalidate_incident_kpi_cache(self, records): incident_ids = list(set(rec.incident_id.id for rec in records if rec.incident_id)) if incident_ids: DataValue = self.env["spp.data.value"] - # Delete stale cache entries for return KPI + # Delete stale cache entries for the return KPI and, since a return + # reduces the distributed total (OP#1160), the distributed KPI too. DataValue.search( [ - ("variable_name", "=", "drims_return_value"), + ("variable_name", "in", ["drims_return_value", "drims_distributed_value"]), ("subject_model", "=", "spp.hazard.incident"), ("subject_id", "in", incident_ids), ] diff --git a/spp_drims/models/stock_warehouse.py b/spp_drims/models/stock_warehouse.py index 0553f1651..36a5e02ff 100644 --- a/spp_drims/models/stock_warehouse.py +++ b/spp_drims/models/stock_warehouse.py @@ -78,6 +78,48 @@ class StockWarehouse(models.Model): compute="_compute_drims_stock_health", ) + def write(self, vals): + """OP#1094: refresh incident stock KPIs when warehouse ↔ incident links + change. + + The incident stock KPI (``drims_stock_value``) is a cached, stored + compute keyed off the warehouses linked to the incident, but its + dependencies don't include ``stock.warehouse.incident_ids`` — so + linking/unlinking a warehouse (Active Incidents) after stock already + exists left the KPI stale (e.g. showing 0). Invalidate the cache and + recompute for every incident affected before and after the change. + """ + affected = set() + if "incident_ids" in vals: + for wh in self: + affected.update(wh.incident_ids.ids) # incidents linked before + result = super().write(vals) + if "incident_ids" in vals: + for wh in self: + affected.update(wh.incident_ids.ids) # incidents linked after + if affected: + self._drims_refresh_incident_stock_kpis(list(affected)) + return result + + def _drims_refresh_incident_stock_kpis(self, incident_ids): + """Drop the cached ``drims_stock_value`` for the given incidents and + recompute their stock KPIs (OP#1094). + + Recompute is explicit because the KPI's ``@api.depends`` does not cover + ``stock.warehouse.incident_ids``, so a stored value would otherwise stay + stale even after the cache is cleared. + """ + self.env["spp.data.value"].search( + [ + ("variable_name", "=", "drims_stock_value"), + ("subject_model", "=", "spp.hazard.incident"), + ("subject_id", "in", incident_ids), + ] + ).unlink() + incidents = self.env["spp.hazard.incident"].browse(incident_ids).exists() + if incidents: + incidents._compute_drims_stock_kpis() + def _compute_drims_counts(self): Donation = self.env["spp.drims.donation"] Picking = self.env["stock.picking"] diff --git a/spp_drims/readme/HISTORY.md b/spp_drims/readme/HISTORY.md index 033ce02e7..d5697e67f 100644 --- a/spp_drims/readme/HISTORY.md +++ b/spp_drims/readme/HISTORY.md @@ -1,3 +1,8 @@ +### 19.0.3.1.0 + +- feat(drims): Incident Management review — incidents are entered as a **Draft** and then flagged Alert or set Active, a closed incident refuses DRIMS operations and no longer accepts lifecycle changes, dashboard KPI cards no longer open the record when a box is clicked, warehouses can be linked to an incident and drive the warehouse choices on donations and requests, and the Impact tab is hidden where it does not apply (#1094, #1123, #1157, #1158, #1159, #1160, #1164) +- fix(drims): incident stock KPIs now count incident-related stock net of allocations, and distributed value is net of confirmed returns. Both are stored computes whose meaning changed, so upgrading recomputes them for every incident — including closed ones, which the refresh cron skips (#1100) + ### 19.0.3.0.4 - feat(drims): rework the dispatch page and correct the waybill. **Dispatch & Delivery** leads the form instead of sitting behind Additional Info, a dispatch shows its destination location rather than an empty Delivery Address, and everything the request already decided — operation type, source document, source location and the DRIMS fields — is locked, with Quantity left editable so a partial dispatch and its backorder can still be produced. The waybill prints on one page with the signature block intact and the TO box filled in (#1150, #1151) diff --git a/spp_drims/static/description/index.html b/spp_drims/static/description/index.html index b742de145..bd601373c 100644 --- a/spp_drims/static/description/index.html +++ b/spp_drims/static/description/index.html @@ -565,6 +565,23 @@

Changelog

+

19.0.3.1.0

+ +
+

19.0.3.0.4

-
+

19.0.3.0.1

-
+

19.0.3.0.0

  • feat(drims): allocate stock per source warehouse. The Allocate Stock @@ -616,7 +633,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/test_incident.py b/spp_drims/tests/test_incident.py index b41ab019f..9ba7581c2 100644 --- a/spp_drims/tests/test_incident.py +++ b/spp_drims/tests/test_incident.py @@ -1,7 +1,11 @@ # 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 odoo.tools.safe_eval import safe_eval from .common import DrimsTestCommon @@ -162,6 +166,107 @@ def test_incident_drims_warehouses_computed(self): self.incident.invalidate_recordset() self.assertIn(self.warehouse, self.incident.drims_warehouse_ids) + def test_1164_link_warehouses_from_incident_side(self): + """OP#1164: editing drims_warehouse_ids on the incident links/unlinks the + warehouse both ways (via stock.warehouse.incident_ids).""" + self.warehouse.is_drims_warehouse = True + # Link from the incident side. + self.incident.drims_warehouse_ids = [(4, self.warehouse.id)] + self.assertIn(self.incident, self.warehouse.incident_ids) + self.incident.invalidate_recordset() + self.assertIn(self.warehouse, self.incident.drims_warehouse_ids) + # Unlink from the incident side. + self.incident.drims_warehouse_ids = [(3, self.warehouse.id)] + self.assertNotIn(self.incident, self.warehouse.incident_ids) + + def test_1164_donation_allowed_warehouses_respects_filter(self): + """OP#1164: donation warehouse choices = the incident's warehouses when + filtering is on; all DRIMS warehouses when off.""" + self.warehouse.is_drims_warehouse = True + other = self.env["stock.warehouse"].create( + {"name": "Other WH 1164", "code": "O1164", "is_drims_warehouse": True} + ) + self.incident.drims_warehouse_ids = [(6, 0, [self.warehouse.id])] + donation = self.env["spp.drims.donation"].create( + { + "incident_id": self.incident.id, + "warehouse_id": self.warehouse.id, + "donor_name": "D", + # A donation needs at least one item (_check_has_lines, added by + # the donations review on fix/1076-drims-donations-review). This + # test is about warehouse choices, so the line is only there to + # make the donation valid — without it, whichever of the two + # branches merges second turns 19.0 red with no textual conflict + # to warn either author. + "line_ids": [ + (0, 0, {"product_id": self.product.id, "quantity_pledged": 100, "uom_id": self.product.uom_id.id}) + ], + } + ) + # Filter ON (default): only the incident's warehouse. + self.assertIn(self.warehouse, donation.allowed_warehouse_ids) + self.assertNotIn(other, donation.allowed_warehouse_ids) + # Filter OFF: all DRIMS warehouses. + self.env["ir.config_parameter"].sudo().set_param("drims.warehouse.filter_by_incident", "False") + donation.invalidate_recordset(["allowed_warehouse_ids"]) + self.assertIn(other, donation.allowed_warehouse_ids) + + def test_1164_allowed_warehouses_fallback_when_incident_has_none(self): + """OP#1164: with filtering on but no warehouses linked, fall back to all + DRIMS warehouses (so donation/request creation isn't locked out).""" + self.warehouse.is_drims_warehouse = True + self.incident.drims_warehouse_ids = [(5, 0, 0)] # ensure none linked + donation = self.env["spp.drims.donation"].create( + { + "incident_id": self.incident.id, + "warehouse_id": self.warehouse.id, + "donor_name": "D", + # See the note above: a donation must carry at least one item. + "line_ids": [ + (0, 0, {"product_id": self.product.id, "quantity_pledged": 100, "uom_id": self.product.uom_id.id}) + ], + } + ) + self.assertIn(self.warehouse, donation.allowed_warehouse_ids) + + def test_1157_flag_as_alert(self): + """OP#1157: action_set_alert moves the incident into the Alert state.""" + self.assertNotEqual(self.incident.status, "alert") + self.incident.action_set_alert() + self.assertEqual(self.incident.status, "alert") + + def test_1094_stock_kpi_refreshes_on_warehouse_link(self): + """OP#1094: linking a warehouse to an incident refreshes the stock KPI, + even when the stock existed before the link (cache was populated with 0).""" + wh = self.env["stock.warehouse"].create({"name": "KPI WH 1094", "code": "K1094", "is_drims_warehouse": True}) + self.product.standard_price = 25.0 + self.env["stock.quant"].create( + {"product_id": self.product.id, "location_id": wh.lot_stock_id.id, "quantity": 100} + ) + incident = self.env["spp.hazard.incident"].create( + { + "name": "KPI Incident 1094", + "code": "KPI-1094", + "category_id": self.hazard_category.id, + "start_date": "2024-01-01", + "status": "active", + } + ) + # Not linked yet -> stock KPI is 0 (and this stores 0). + self.assertEqual(incident.drims_stock_value, 0.0) + + # Link the warehouse AFTER stock already exists. + wh.write({"incident_ids": [(4, incident.id)]}) + self.env.flush_all() + incident.invalidate_recordset() + self.assertEqual(incident.drims_stock_value, 2500.0) # 100 * 25 + + # Unlinking refreshes back to 0. + wh.write({"incident_ids": [(3, incident.id)]}) + self.env.flush_all() + incident.invalidate_recordset() + self.assertEqual(incident.drims_stock_value, 0.0) + def test_incident_stock_value_initially_zero(self): """Test stock value is zero when no warehouse linked.""" self.incident.invalidate_recordset() @@ -223,6 +328,143 @@ def test_incident_distributed_value_from_dispatch(self): # 10 * 25 = 250 self.assertEqual(self.incident.drims_distributed_value, 250.0) + # ── OP#1160: Units/Products = incident-related stock (stocked − allocated) ── + def _drims_type(self, code): + return self.env["spp.vocabulary.code"].search( + [ + ("vocabulary_id.namespace_uri", "=", "urn:openspp:vocab:drims:drims-types"), + ("code", "=", code), + ], + limit=1, + ) + + def _stock_in_receipt(self, qty): + """Create + validate a done donation-receipt picking into the warehouse.""" + drims_type = self._drims_type("donation_receipt") + if not drims_type: + self.skipTest("donation_receipt vocabulary code not found") + picking = self.env["stock.picking"].create( + { + "picking_type_id": self.warehouse.in_type_id.id, + "location_id": self.env.ref("stock.stock_location_suppliers").id, + "location_dest_id": self.warehouse.lot_stock_id.id, + "incident_id": self.incident.id, + "drims_type_id": drims_type.id, + } + ) + move = self.env["stock.move"].create( + { + "product_id": self.product.id, + "product_uom_qty": qty, + "product_uom": self.product.uom_id.id, + "picking_id": picking.id, + "location_id": picking.location_id.id, + "location_dest_id": picking.location_dest_id.id, + } + ) + picking.action_confirm() + move.quantity = qty + picking.button_validate() + return picking + + def _request_with_allocation(self, requested, allocated): + 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, + }, + ) + ], + } + ) + # OP#1079 made the line's quantity_allocated a stored compute over + # per-warehouse allocation rows, so allocating means recording a row — + # writing the total directly no longer registers at all. + self.env["spp.drims.request.allocation"].create( + { + "request_line_id": request.line_ids[0].id, + "warehouse_id": self.warehouse.id, + "quantity_allocated": allocated, + } + ) + return request + + def test_incident_units_net_of_allocation(self): + """Units/Products = stocked-in from this incident's donations minus what + its requests have allocated (OP#1160).""" + self._stock_in_receipt(100) + self._request_with_allocation(requested=100, allocated=30) + self.incident.invalidate_recordset() + self.assertEqual(self.incident.drims_total_stock_units, 70.0) + self.assertEqual(self.incident.drims_stock_item_count, 1) + + def test_incident_units_zero_when_fully_allocated(self): + """A product fully allocated away drops out of the incident stock count.""" + self._stock_in_receipt(50) + self._request_with_allocation(requested=50, allocated=50) + self.incident.invalidate_recordset() + self.assertEqual(self.incident.drims_total_stock_units, 0.0) + self.assertEqual(self.incident.drims_stock_item_count, 0) + + def test_incident_distributed_net_of_returns(self): + """Distributed value is reduced by returned items (OP#1160).""" + # Dispatch 10 @ 25 = 250 distributed (mirrors the dispatch test). + drims_type = self._drims_type("request_dispatch") + if not drims_type: + self.skipTest("request_dispatch vocabulary code not found") + self.product.standard_price = 25.0 + dispatch = 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, + "incident_id": self.incident.id, + "drims_type_id": drims_type.id, + } + ) + move = self.env["stock.move"].create( + { + "product_id": self.product.id, + "product_uom_qty": 10, + "product_uom": self.product.uom_id.id, + "picking_id": dispatch.id, + "location_id": dispatch.location_id.id, + "location_dest_id": dispatch.location_dest_id.id, + } + ) + dispatch.action_confirm() + move.quantity = 10 + dispatch.beneficiary_count = 50 + dispatch.beneficiary_area_id = self.area.id + dispatch.button_validate() + + # A draft return does not yet reduce distributed. + return_rec = self.env["spp.drims.return"].create( + { + "incident_id": self.incident.id, + "original_picking_id": dispatch.id, + "warehouse_id": self.warehouse.id, + "line_ids": [(0, 0, {"product_id": self.product.id, "quantity_returned": 4})], + } + ) + self.assertEqual(return_rec.total_value, 100.0) # 4 * 25 + self.incident.invalidate_recordset() + self.assertEqual(self.incident.drims_distributed_value, 250.0) + + # Once the return is active, 100 of the 250 is no longer distributed. + return_rec.state = "confirmed" + self.incident.invalidate_recordset() + self.assertEqual(self.incident.drims_distributed_value, 150.0) + def test_incident_picking_ids_relation(self): """Test incident has access to related pickings.""" picking = self.env["stock.picking"].create( @@ -234,3 +476,235 @@ def test_incident_picking_ids_relation(self): } ) self.assertIn(picking, self.incident.drims_picking_ids) + + # ── OP#1157 QA round 1: header button order and Alert entry state ── + + def _incident_form_buttons(self): + """Header action buttons of the incident form, in rendered order.""" + view = self.env["spp.hazard.incident"].get_view(self.env.ref("spp_hazard.view_hazard_incident_form").id, "form") + tree = etree.fromstring(view["arch"]) + return [b.get("name") for b in tree.xpath("//header/button[@name]")] + + def test_flag_as_alert_is_the_leftmost_header_button(self): + """QA asked for Flag As Alert · Start Recovery · Close Incident. + + It was inserted before the statusbar, which put it last. Anchoring it + on the first base button puts it where the workflow starts. + """ + buttons = self._incident_form_buttons() + + self.assertIn("action_set_alert", buttons, "Flag As Alert is missing from the header") + self.assertEqual( + buttons[0], + "action_set_alert", + f"Flag As Alert should lead the header, got {buttons}", + ) + # The rest keep their base order behind it. + self.assertLess(buttons.index("action_set_recovery"), buttons.index("action_close")) + + def test_close_incident_hidden_only_outside_the_open_states(self): + """QA asked to confirm Close shows only for alert / active / recovery.""" + view = self.env["spp.hazard.incident"].get_view(self.env.ref("spp_hazard.view_hazard_incident_form").id, "form") + tree = etree.fromstring(view["arch"]) + close = tree.xpath("//header/button[@name='action_close']")[0] + condition = close.get("invisible") + + for status in ("alert", "active", "recovery"): + self.assertFalse( + safe_eval(condition, {"status": status}), + f"Close Incident should be offered while {status}", + ) + self.assertTrue(safe_eval(condition, {"status": "closed"})) + + def test_new_drims_incident_starts_in_draft(self): + """The entry state, seen from DRIMS rather than the base module.""" + incident = self.env["spp.hazard.incident"].create( + { + "name": "DRIMS Fresh Incident", + "code": "DRIMS-ALERT-1157", + "category_id": self.hazard_category.id, + "start_date": "2026-08-01", + } + ) + self.assertEqual(incident.status, "draft") + + def test_draft_offers_exactly_flag_as_alert_and_set_active(self): + """OP#1157 round 3: a draft presents those two choices and no others. + + Evaluated off the form arch because button visibility is a view + concern; the buttons themselves are contributed by two modules, which + is precisely why the combined header is worth asserting. + """ + arch = etree.fromstring( + self.env["spp.hazard.incident"].get_view(self.env.ref("spp_hazard.view_hazard_incident_form").id, "form")[ + "arch" + ] + ) + shown = [ + b.get("name") + for b in arch.xpath("//header/button") + if not safe_eval(b.get("invisible", "False"), {"status": "draft"}) + ] + self.assertEqual( + shown, + ["action_set_alert", "action_set_active"], + f"a draft should offer Flag As Alert then Set Active, got {shown}", + ) + + def test_flag_as_alert_returns_an_incident_to_alert(self): + """The button itself still does its job from active and recovery.""" + self.incident.action_set_active() + self.incident.action_set_alert() + self.assertEqual(self.incident.status, "alert") + + self.incident.action_set_active() + self.incident.action_set_recovery() + self.incident.action_set_alert() + self.assertEqual(self.incident.status, "alert") + + +@tagged("post_install", "-at_install") +class TestDrimsIncidentClosedGuards(DrimsTestCommon): + """OP#1158: limit DRIMS operations on incidents in the 'closed' state.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.future_date = date.today() + timedelta(days=30) + cls.closed_incident = cls.env["spp.hazard.incident"].create( + { + "name": "Closed Incident 1158", + "code": "CLOSED-1158", + "category_id": cls.hazard_category.id, + "start_date": "2024-01-01", + "status": "closed", + } + ) + + def _new_request(self, incident): + return self.env["spp.drims.request"].create( + { + "incident_id": incident.id, + "destination_area_id": self.area.id, + "date_needed": self.future_date, + "line_ids": [ + (0, 0, {"product_id": self.product.id, "quantity_requested": 10, "uom_id": self.product.uom_id.id}) + ], + } + ) + + # ── requests: submit / approve / allocate blocked on a closed incident ── + def test_1158_submit_blocked_when_closed(self): + req = self._new_request(self.closed_incident) + with self.assertRaises(UserError): + req.action_submit() + + def test_1158_approve_blocked_when_closed(self): + req = self._new_request(self.closed_incident) + with self.assertRaises(UserError): + req.action_approve() + + def test_1158_allocate_blocked_when_closed(self): + req = self._new_request(self.closed_incident) + with self.assertRaises(UserError): + req.action_allocate() + + def test_1158_submit_allowed_when_open(self): + """Sanity: the guard does not block an open incident.""" + req = self._new_request(self.incident) # common incident is active + req.action_submit() # should not raise + self.assertIn(req.approval_state, ("pending", "submitted")) + + # ── donations: no new donations accepted on a closed incident ── + def test_1158_donation_blocked_when_closed(self): + with self.assertRaises(UserError): + self.env["spp.drims.donation"].create( + { + "incident_id": self.closed_incident.id, + "warehouse_id": self.warehouse.id, + "donor_name": "Closed Donor", + "line_ids": [ + (0, 0, {"product_id": self.product.id, "quantity_pledged": 5, "uom_id": self.product.uom_id.id}) + ], + } + ) + + def test_1158_donation_allowed_when_open(self): + donation = self.env["spp.drims.donation"].create( + { + "incident_id": self.incident.id, + "warehouse_id": self.warehouse.id, + "donor_name": "Open Donor", + "line_ids": [ + (0, 0, {"product_id": self.product.id, "quantity_pledged": 5, "uom_id": self.product.uom_id.id}) + ], + } + ) + self.assertTrue(donation.exists()) + + # ── lifecycle: the state machine is enforced on the server, not just hidden ── + def test_1100_closed_incident_cannot_be_reopened(self): + """The header hides these buttons; hiding is not enforcement. + + Over RPC, an import or the shell, flipping a closed incident back to + active or alert would sidestep every guard above, all of which key off + `closed` (OP#1100 review). + """ + for action in ("action_set_active", "action_set_alert", "action_set_recovery"): + with self.subTest(action=action): + with self.assertRaises(UserError): + getattr(self.closed_incident, action)() + + self.assertEqual(self.closed_incident.status, "closed") + + def test_1100_closed_incident_cannot_be_closed_again(self): + with self.assertRaises(UserError): + self.closed_incident.action_close() + + def test_1100_a_draft_is_not_closed_but_deleted(self): + """QA's rule: a mistakenly entered incident is deleted, not closed.""" + draft = self.env["spp.hazard.incident"].create( + { + "name": "Draft Incident 1100", + "code": "DRAFT-1100", + "category_id": self.hazard_category.id, + "start_date": "2024-01-01", + } + ) + self.assertEqual(draft.status, "draft") + + with self.assertRaises(UserError): + draft.action_close() + + draft.unlink() + + def test_1100_an_open_incident_still_moves_through_its_lifecycle(self): + """The guard must not block the transitions that are the point of it.""" + incident = self.env["spp.hazard.incident"].create( + { + "name": "Lifecycle Incident 1100", + "code": "LIFE-1100", + "category_id": self.hazard_category.id, + "start_date": "2024-01-01", + } + ) + + incident.action_set_alert() + self.assertEqual(incident.status, "alert") + incident.action_set_active() + self.assertEqual(incident.status, "active") + incident.action_set_recovery() + self.assertEqual(incident.status, "recovery") + incident.action_close() + self.assertEqual(incident.status, "closed") + + # ── personnel: cannot deploy to a closed incident ── + def test_1158_personnel_blocked_when_closed(self): + with self.assertRaises(UserError): + self.env["spp.drims.personnel"].create( + {"name": "Closed Deployment", "incident_id": self.closed_incident.id} + ) + + def test_1158_personnel_allowed_when_open(self): + person = self.env["spp.drims.personnel"].create({"name": "Open Deployment", "incident_id": self.incident.id}) + self.assertTrue(person.exists()) diff --git a/spp_drims/views/dashboard_views.xml b/spp_drims/views/dashboard_views.xml index a151a372e..1459c94e6 100644 --- a/spp_drims/views/dashboard_views.xml +++ b/spp_drims/views/dashboard_views.xml @@ -24,7 +24,11 @@ spp.hazard.incident.kanban.drims spp.hazard.incident - + + @@ -42,11 +46,17 @@ -
    + +
    - + + +
    @@ -359,11 +369,23 @@ spp.hazard.incident - + + + diff --git a/spp_drims/views/donation_views.xml b/spp_drims/views/donation_views.xml index b419fe085..380c21b7c 100644 --- a/spp_drims/views/donation_views.xml +++ b/spp_drims/views/donation_views.xml @@ -132,6 +132,8 @@ /> + + diff --git a/spp_drims/views/hazard_incident_views.xml b/spp_drims/views/hazard_incident_views.xml index 089ee4b69..db6512a0e 100644 --- a/spp_drims/views/hazard_incident_views.xml +++ b/spp_drims/views/hazard_incident_views.xml @@ -6,6 +6,36 @@ spp.hazard.incident + + + 1 + + + + +
    +

    19.0.2.1.0

    +
      +
    • feat(hazard): incidents start as a Draft and reach Alert or Active +deliberately, rather than being assumed active on entry. Lifecycle +moves are now refused server-side as well as hidden in the form: a +draft cannot be closed (delete it instead) and a closed incident +cannot be reopened (#1157, #1158)
    • +
    +
    +

    19.0.2.0.2

    • fix(security): grant group_hazard_viewer to spp_user_roles roles @@ -2469,7 +2479,7 @@

      19.0.2.0.2

      Support).
    -
    +

    19.0.2.0.1

    • fix(views): apply spp_registry.x2many_no_padding widget to the @@ -2477,7 +2487,7 @@

      19.0.2.0.1

      (showing a muted info line instead) (#943).
    -
    +

    19.0.2.0.0

    • Initial migration to OpenSPP2
    • diff --git a/spp_hazard/tests/test_hazard_incident.py b/spp_hazard/tests/test_hazard_incident.py index c92f4e43b..b16f1e2c9 100644 --- a/spp_hazard/tests/test_hazard_incident.py +++ b/spp_hazard/tests/test_hazard_incident.py @@ -1,10 +1,12 @@ # Part of OpenSPP. See LICENSE file for full copyright and licensing details. +from lxml import etree from psycopg2 import IntegrityError from odoo import Command from odoo.exceptions import ValidationError from odoo.tests import mute_logger +from odoo.tools.safe_eval import safe_eval from .common import HazardTestCase @@ -30,8 +32,83 @@ def test_01_incident_creation(self): """Test basic incident creation.""" self.assertTrue(self.incident) self.assertEqual(self.incident.name, "Test Typhoon Incident") - self.assertEqual(self.incident.status, "active") # default - self.assertTrue(self.incident.is_ongoing) + self.assertEqual(self.incident.status, "draft") # OP#1157: incidents are entered as drafts + # A draft is not under way yet, so it does not count as ongoing. + self.assertFalse(self.incident.is_ongoing) + + def test_new_incident_starts_in_draft(self): + """OP#1157: an incident is entered as a draft, then classified. + + The person recording it says what it is — Flag As Alert for something + being watched, Set Active for a response already under way. Neither is + assumed on their behalf, so the entry state is Draft. + """ + incident = self.env["spp.hazard.incident"].create( + { + "name": "Freshly Reported", + "code": "TEST-INC-ALERT", + "category_id": self.category_typhoon.id, + "start_date": "2024-02-01", + } + ) + self.assertEqual(incident.status, "draft") + + def test_draft_is_not_filtered_out_of_the_incident_list(self): + """Draft is the entry state, so the default filter has to include it. + + The action pre-selects status filters. Without Draft among them a newly + recorded incident would disappear from the very list it was created in, + which would read as the record not having been saved. + """ + action = self.env.ref("spp_hazard.action_hazard_incident") + context = safe_eval(action.context or "{}") + self.assertTrue( + context.get("search_default_draft"), + f"Draft is missing from the incident list's default filter: {context}", + ) + + search = etree.fromstring(self.env.ref("spp_hazard.view_hazard_incident_search").arch) + self.assertTrue( + search.xpath("//filter[@name='draft']"), + "the search view offers no Draft filter to switch back on", + ) + + def test_draft_can_go_straight_to_active(self): + """A response already under way should not have to be flagged first.""" + incident = self.env["spp.hazard.incident"].create( + { + "name": "Already Responding", + "code": "TEST-INC-DIRECT", + "category_id": self.category_typhoon.id, + "start_date": "2024-02-01", + } + ) + self.assertEqual(incident.status, "draft") + + incident.action_set_active() + self.assertEqual(incident.status, "active") + + def test_alert_is_reachable_and_reversible(self): + """Alert is a state you can return to, not only start in.""" + self.incident.action_set_active() + self.assertEqual(self.incident.status, "active") + + self.incident.write({"status": "alert"}) + self.assertEqual(self.incident.status, "alert") + + self.incident.action_set_active() + self.assertEqual(self.incident.status, "active") + + def test_recovery_is_reached_from_active_only(self): + """Answers QA's question about when Recovery is available. + + Start Recovery is offered only from Active, so a newly entered incident + goes Draft -> Active -> Recovery rather than jumping straight in. + """ + self.assertEqual(self.incident.status, "draft") + self.incident.action_set_active() + self.incident.action_set_recovery() + self.assertEqual(self.incident.status, "recovery") def test_02_incident_code_unique(self): """Test that incident codes must be unique.""" @@ -60,7 +137,9 @@ def test_03_date_validation(self): def test_04_is_ongoing_computation(self): """Test is_ongoing computed field.""" - # Active incident with no end date should be ongoing + # A draft is not under way; confirming it into Active makes it ongoing. + self.assertFalse(self.incident.is_ongoing) + self.incident.action_set_active() self.assertTrue(self.incident.is_ongoing) # Set end date - should no longer be ongoing @@ -73,7 +152,9 @@ def test_04_is_ongoing_computation(self): def test_05_status_transitions(self): """Test status transition actions.""" - # Start in active status + # OP#1157: a new incident starts in draft and is classified from there. + self.assertEqual(self.incident.status, "draft") + self.incident.action_set_active() self.assertEqual(self.incident.status, "active") # Transition to recovery @@ -205,6 +286,8 @@ def test_15_close_sets_end_date(self): } ) self.assertFalse(incident.end_date) + # OP#1100: a draft is deleted, not closed — take it through the lifecycle first. + incident.action_set_active() incident.action_close() self.assertTrue(incident.end_date) self.assertEqual(incident.status, "closed") @@ -220,6 +303,8 @@ def test_16_close_preserves_existing_end_date(self): "end_date": "2024-02-01", } ) + # OP#1100: a draft is deleted, not closed — take it through the lifecycle first. + incident.action_set_active() incident.action_close() self.assertEqual(str(incident.end_date), "2024-02-01") self.assertEqual(incident.status, "closed") @@ -334,9 +419,34 @@ def test_21_multi_record_close(self): "end_date": "2024-04-01", } ) + # OP#1100: a draft is deleted, not closed — take it through the lifecycle first. + (inc1 | inc2).action_set_active() (inc1 | inc2).action_close() self.assertEqual(inc1.status, "closed") self.assertEqual(inc2.status, "closed") # inc1 gets auto end_date, inc2 preserves its own self.assertTrue(inc1.end_date) self.assertEqual(str(inc2.end_date), "2024-04-01") + + def test_closed_incident_does_not_link_out_to_its_category(self): + """A closed incident's Hazard Category must be inert text, not a link. + + Readonly alone does not do it — a readonly many2one still renders as an + internal link — and ``no_open`` cannot be made conditional, because + ``options`` is a static dict that cannot reference ``status``. Hence two + declarations of the field with mutually exclusive ``invisible`` + (OP#1158). Asserted on the arch because "is it clickable" is decided in + the client, not the ORM. + """ + arch = etree.fromstring(self.env.ref("spp_hazard.view_hazard_incident_form").arch) + nodes = arch.xpath("//group[@name='main_info']/field[@name='category_id']") + self.assertEqual(len(nodes), 2, "expected an open-incident and a closed-incident variant") + + closed = [n for n in nodes if n.get("invisible") == "status != 'closed'"] + opened = [n for n in nodes if n.get("invisible") == "status == 'closed'"] + self.assertEqual(len(closed), 1, "no closed-incident variant of category_id") + self.assertEqual(len(opened), 1, "no open-incident variant of category_id") + + self.assertIn("'no_open': True", closed[0].get("options") or "") + # The open incident keeps its link — the ticket only restricts closed ones. + self.assertNotIn("no_open", opened[0].get("options") or "") diff --git a/spp_hazard/tests/test_registrant.py b/spp_hazard/tests/test_registrant.py index e343295b7..1bf7c003b 100644 --- a/spp_hazard/tests/test_registrant.py +++ b/spp_hazard/tests/test_registrant.py @@ -52,6 +52,8 @@ def test_hazard_impact_count(self): def test_has_active_impact_with_active_incident(self): """Test flag is True when incident is active.""" + # OP#1157: incidents now start in draft, so set it active first. + self.incident.action_set_active() self.assertEqual(self.incident.status, "active") self.assertTrue(self.registrant.has_active_impact) diff --git a/spp_hazard/views/hazard_incident_views.xml b/spp_hazard/views/hazard_incident_views.xml index 50a7f6c28..90b0ede2e 100644 --- a/spp_hazard/views/hazard_incident_views.xml +++ b/spp_hazard/views/hazard_incident_views.xml @@ -44,11 +44,15 @@
      +
      @@ -140,10 +147,28 @@ placeholder="e.g., 2013-YOLANDA" readonly="status == 'closed'" /> + + @@ -246,6 +271,11 @@ + hazard-incidents {'search_default_alert': 1, 'search_default_active': 1, 'search_default_recovery': 1} + >{'search_default_draft': 1, 'search_default_alert': 1, 'search_default_active': 1, 'search_default_recovery': 1}

      Record your first hazard incident