feat(spp_drims): Incident Management review — states, closed-incident limits, dashboard and warehouses - #416
Open
emjay0921 wants to merge 14 commits into
Open
feat(spp_drims): Incident Management review — states, closed-incident limits, dashboard and warehouses#416emjay0921 wants to merge 14 commits into
emjay0921 wants to merge 14 commits into
Conversation
…of allocations) and Distributed net of returns (#1160)
…personnel, field edits (#1158)
…e opens form (#1123)
…ids changes (#1094)
…tion/request pickers (#1164)
QA round 1 on OP#1157 returned three findings and a question. Flag As Alert was inserted before the statusbar, which rendered it last. It is now anchored on the first header button so the order reads Flag As Alert, Start Recovery, Close Incident — the order the workflow runs in. Anchored by button name rather than position, so a reordering of the base view fails loudly at upgrade instead of silently drifting back. New incidents landed straight in Active, skipping the triage step the Alert state exists for. The status default becomes "alert", making the lifecycle Alert -> Active -> Recovery -> Closed. Changed on spp_hazard, where the state machine lives, so every consumer behaves the same rather than the same model behaving differently depending on which modules are installed. Close Incident was already correct in effect — with four states, "not closed" and "alert, active or recovery" are the same set — but it is now stated positively so it stays right if a state is ever added. QA also asked when Recovery can be set: only from Active, since Start Recovery is hidden otherwise. With Alert as the entry state a new incident is confirmed Active before Recovery is offered. A test pins that so the answer does not quietly change. Four spp_hazard tests asserted the old default; they now confirm Active explicitly rather than assuming it. Not changed, and flagged for QA instead: DRIMS low-stock alerting and the request-from-template picker both filter on status = "active", so an incident sitting in Alert reaches neither until it is confirmed. Whether an alert-state incident should drive stock alerting is a product decision. Verified across every spp_hazard dependent — spp_hazard, spp_drims, spp_hazard_programs, spp_api_v2_gis, spp_gis_indicators, spp_drims_sl_demo — and by hand on a fresh database. OP#1157
Closing an incident left Hazard Category clickable, so a closed record still offered a way through to the category form. Making the field readonly does not cover this — a readonly many2one still renders as an internal link — and no_open cannot be made conditional, because options is parsed as a static dict and cannot reference status. Declare the field twice instead, with mutually exclusive invisible: an open incident keeps its link, a closed one is inert text. The same pattern is already used across the codebase wherever a field has to change shape with state. Asserted on the arch rather than through behaviour, since whether a many2one is clickable is decided in the client, not the ORM.
…ent-management # Conflicts: # spp_drims/models/request.py # spp_drims/views/request_views.xml
OP#1079 made the request line's quantity_allocated a stored compute over per-warehouse allocation rows, so the helper's direct write no longer registered and the Units KPI counted stock that had in fact been allocated.
Round 2 made Alert the entry state. QA has since changed the requirement: an incident should be entered as a draft, and 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. Adds a draft state at the head of the selection and makes it the default. Set Active is now offered from Draft as well, so a response already under way does not have to be flagged as an alert first. Start Recovery and Close Incident stay hidden there, which is what leaves a draft showing exactly the two buttons asked for. The list's default filter needed Draft adding too. It pre-selects Alert, Active and Recovery, so with Draft as the entry state a newly created incident would have disappeared from the list it was created in and read as having failed to save. A test asserts both the filter and its default, because nothing at the model level would notice. A draft is deliberately not a live incident: it is excluded from is_ongoing, from the affected-registrant check, and from the Active-only consumers. It starts counting once classified. A draft also cannot be closed — a mistaken one is deleted rather than closed. Six tests asserting the round-2 lifecycle are rewritten, not dropped.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #416 +/- ##
==========================================
+ Coverage 72.24% 72.85% +0.60%
==========================================
Files 419 464 +45
Lines 29813 31568 +1755
==========================================
+ Hits 21539 22998 +1459
- Misses 8274 8570 +296
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
emjay0921
marked this pull request as ready for review
August 14, 2026 03:36
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this change needed?
OP#1100 (DRIMS - Full review - Incident Management) and all of its children have passed QA — this raises the PR for that work, per the flow of opening PRs once QA signs off.
What is in here
Unit tests executed by the author
On the merged branch, across the base module and everything downstream of it:
./spp lint --all-filesclean.How to test manually
Related links
https://openspp.openproject.com/work_packages/1100
Reviewer notes
OP#1157 changed requirement twice, which is why its history on this branch is three commits. Round 1 exposed the existing Alert state via a button; round 2 made Alert the entry state; round 3 replaced that with a new Draft state, because QA wanted the person recording an incident to say what it is rather than have Alert assumed. The final shape is round 3.
Two decisions were made where the ticket was silent, both confirmed by QA on the ticket: a draft cannot be closed — a mistaken one is deleted — and a draft does not count as a live incident anywhere (not "ongoing", not marking registrants affected, and not offered where only Active incidents are, such as low-stock alerting and the request-from-template picker). It starts counting once classified.
spp_hazardis a base module with dependants, so the runs above cover both modules that inherit the incident form, not just the module under change.The branch was brought up to date by merging
19.0, not rebasing, since it was already published.