fix(spp_case_base): clear is_current when an intervention plan is completed

spp_case_base/README.rst

@@ -151,6 +151,15 @@ Dependencies
 Changelog
 =========
 
+19.0.2.0.1
+~~~~~~~~~~
+
+- fix(case): completing an intervention plan now clears ``is_current``,
+  so a finished plan stops being the case's current plan. Previously
+  only the revision path released the flag, leaving ``current_plan_id``
+  pointing at completed work while ``has_active_plan`` read False, and
+  blocking any new plan from being marked current.
+
 19.0.2.0.0
 ~~~~~~~~~~
 

spp_case_base/__manifest__.py

@@ -1,7 +1,7 @@
 # pylint: disable=pointless-statement
 {
     "name": "OpenSPP Case Management Base",
-    "version": "19.0.2.0.0",
+    "version": "19.0.2.0.1",
     "category": "OpenSPP/Monitoring",
     "summary": "Core case management functionality for OpenSPP",
     "author": "OpenSPP.org",

spp_case_base/migrations/19.0.2.0.1/post-migration.py

@@ -0,0 +1,56 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""Release ``is_current`` on intervention plans that already finished.
+
+``action_complete`` never cleared ``is_current`` (#458), and the only other
+writer of ``is_current = False`` is the revision path, so a plan that finished
+NORMALLY stayed its case's current plan forever. The code fix covers future
+completions only; the rows already in a released database keep the incoherent
+pair — ``case.current_plan_id`` pointing at completed work while
+``has_active_plan`` reads False — and keep tripping the one-current-plan-per-case
+constraint, so they are cleared here.
+
+Deliberately narrow: only ``completed``. ``action_create_revision`` writes
+``is_current = False`` together with ``revised``, so that state has no
+equivalent stale population, and widening the predicate would demote plans this
+fix makes no claim about.
+
+Affected cases are left with NO current plan, which is the intended end state —
+the plan is finished, and marking a fresh plan current is now possible again
+(that constraint failure is the user-facing half of #458). The plan's own record
+is untouched: ``state``, ``actual_end_date`` and its interventions all stay.
+
+Raw SQL with no ORM invalidation, matching
+``spp_grm_cel/migrations/19.0.2.0.2/post-migration.py``: nothing reads
+``is_current`` through the ORM later in this transaction, and a flush-and-
+invalidate would risk writing a cached value back over the UPDATE. Callers that
+DO read back in the same transaction — the test for this script, for one — must
+invalidate on their side.
+"""
+
+import logging
+
+_logger = logging.getLogger(__name__)
+
+
+def migrate(cr, version):
+    if not version:
+        return
+
+    cr.execute(
+        "UPDATE spp_case_intervention_plan SET is_current = false "
+        "WHERE state = 'completed' AND is_current = true "
+        "RETURNING case_id"
+    )
+    # One row per demoted plan; the one-current-plan constraint means a case
+    # cannot appear twice, but sort and de-duplicate so the log is stable.
+    case_ids = sorted({row[0] for row in cr.fetchall()})
+
+    if case_ids:
+        _logger.warning(
+            "Released is_current on %s completed intervention plan(s) that were still "
+            "flagged as their case's current plan. Cases affected (spp.case ids): %s. "
+            "Those cases now report no current plan; where case work is continuing, "
+            "mark the successor plan current on the case's Intervention Plans tab.",
+            len(case_ids),
+            ", ".join(str(cid) for cid in case_ids),
+        )

spp_case_base/models/case_intervention_plan.py

@@ -195,6 +195,7 @@ def action_complete(self):
                 {
                     "state": "completed",
                     "actual_end_date": fields.Date.context_today(self),
+                    "is_current": False,
                 }
             )
         return True

spp_case_base/readme/HISTORY.md

@@ -1,3 +1,10 @@
+### 19.0.2.0.1
+
+- fix(case): completing an intervention plan now clears `is_current`, so a finished plan
+  stops being the case's current plan. Previously only the revision path released the
+  flag, leaving `current_plan_id` pointing at completed work while `has_active_plan` read
+  False, and blocking any new plan from being marked current.
+
 ### 19.0.2.0.0
 
 - Initial migration to OpenSPP2

spp_case_base/static/description/index.html

@@ -540,6 +540,16 @@ <h2><a class="toc-backref" href="#toc-entry-1">Changelog</a></h2>
 </div>
 </div>
 <div class="section" id="section-1">
+<h1>19.0.2.0.1</h1>
+<ul class="simple">
+<li>fix(case): completing an intervention plan now clears <tt class="docutils literal">is_current</tt>,
+so a finished plan stops being the case’s current plan. Previously
+only the revision path released the flag, leaving <tt class="docutils literal">current_plan_id</tt>
+pointing at completed work while <tt class="docutils literal">has_active_plan</tt> read False, and
+blocking any new plan from being marked current.</li>
+</ul>
+</div>
+<div class="section" id="section-2">
 <h1>19.0.2.0.0</h1>
 <ul class="simple">
 <li>Initial migration to OpenSPP2</li>

spp_case_base/tests/__init__.py

@@ -6,3 +6,4 @@
 from . import test_case_security
 from . import test_compliance_generated
 from . import test_case_models
+from . import test_migration_complete_clears_is_current

spp_case_base/tests/test_case_intervention_plan.py

@@ -177,6 +177,99 @@ def test_plan_approval_workflow(self):
         plan.action_complete()
         self.assertEqual(plan.state, "completed", "Plan should be completed after completion action")
         self.assertTrue(plan.actual_end_date, "Completion date should be recorded")
+        self.assertFalse(plan.is_current, "Completed plan should no longer be marked current")
+        self.assertFalse(self.case.current_plan_id, "Case should have no current plan once the plan is completed")
+
+    def _active_plan(self, name):
+        """Return a plan taken through the approval cycle to ``active``.
+
+        The Complete button is only offered on an active plan
+        (``views/case_intervention_views.xml``), so the completion tests below
+        drive the same path rather than completing a draft.
+        """
+        plan = self.env["spp.case.intervention.plan"].create(
+            {
+                "name": name,
+                "case_id": self.case.id,
+                "goals": "<p>Reach self-sufficiency</p>",
+            }
+        )
+        self.env["spp.case.intervention"].create(
+            {
+                "name": f"{name} Intervention",
+                "plan_id": plan.id,
+            }
+        )
+        plan.action_submit_for_approval()
+        plan.with_user(self.supervisor).action_approve()
+        plan.action_activate()
+        self.assertEqual(plan.state, "active", "Plan should be active before completion")
+        return plan
+
+    def test_complete_clears_is_current(self):
+        """Test that completing a plan ends its tenure as the case's current plan."""
+        plan = self._active_plan("Plan To Complete")
+
+        self.assertTrue(plan.is_current, "Active plan should still be current")
+        self.assertEqual(
+            self.case.current_plan_id,
+            plan,
+            "Active plan should be the case's current plan",
+        )
+
+        plan.action_complete()
+
+        self.assertEqual(plan.state, "completed", "Plan should be completed")
+        self.assertFalse(
+            plan.is_current,
+            "Completed plan should no longer be marked current",
+        )
+        self.assertFalse(
+            self.case.current_plan_id,
+            "Case should have no current plan once the plan is completed",
+        )
+        self.assertFalse(
+            self.case.has_active_plan,
+            "Case should not report an active plan once the plan is completed",
+        )
+
+    def test_complete_frees_the_current_plan_slot(self):
+        """Test that a fresh plan can be made current once the previous one completes.
+
+        This is the user-facing half of the fix: while a finished plan kept
+        ``is_current``, the one-current-plan-per-case constraint refused every
+        attempt to start the next plan.
+        """
+        self._active_plan("Finished Plan").action_complete()
+
+        # Defaults to is_current=True, so this create is what used to raise.
+        successor = self.env["spp.case.intervention.plan"].create(
+            {
+                "name": "Successor Plan",
+                "case_id": self.case.id,
+                "goals": "<p>Second cycle goals</p>",
+            }
+        )
+
+        self.assertTrue(successor.is_current, "Successor plan should be current")
+        self.assertEqual(
+            self.case.current_plan_id,
+            successor,
+            "Case should point at the successor plan",
+        )
+
+    def test_complete_by_case_worker(self):
+        """Test that the assigned case worker may complete a plan and release the flag.
+
+        ``action_complete`` writes ``is_current`` through ``write()``, so the
+        worker record rule (own cases only) has to permit it.
+        """
+        plan = self._active_plan("Worker Completed Plan")
+
+        plan.with_user(self.case_worker).action_complete()
+
+        self.assertEqual(plan.state, "completed", "Plan should be completed")
+        self.assertFalse(plan.is_current, "Completed plan should no longer be marked current")
 
     def test_submit_without_interventions(self):
         """Test that plan cannot be submitted without interventions."""

spp_case_base/tests/test_migration_complete_clears_is_current.py

@@ -0,0 +1,159 @@
+# Part of OpenSPP. See LICENSE file for full copyright and licensing details.
+"""Exercise the 19.0.2.0.1 post-migration that demotes finished plans.
+
+The code fix in ``action_complete`` only covers future completions. Databases
+released before it keep rows with ``state = 'completed'`` and
+``is_current = true``, which is what #458 reports: ``current_plan_id`` points at
+finished work and the one-current-plan constraint refuses a successor plan. This
+pins that the script clears exactly those rows and nothing else.
+
+``migrations/`` is not a package, so the script is loaded through ``importlib``
+— same pattern as ``spp_gis/tests/test_migration_geofence_tags.py`` and
+``spp_hide_menus_base/tests/test_migration_dedup_hide_menu.py``.
+"""
+
+import importlib.util
+from pathlib import Path
+
+from odoo import Command
+from odoo.tests import TransactionCase, tagged
+
+MIGRATION_PATH = Path(__file__).parent.parent / "migrations" / "19.0.2.0.1" / "post-migration.py"
+
+
+def _load_migrate():
+    spec = importlib.util.spec_from_file_location("spp_case_base_post_migration_19_0_2_0_1", MIGRATION_PATH)
+    module = importlib.util.module_from_spec(spec)
+    spec.loader.exec_module(module)
+    return module.migrate
+
+
+migrate = _load_migrate()
+
+
+@tagged("post_install", "-at_install")
+class TestCompleteClearsIsCurrentMigration(TransactionCase):
+    """A released database's stale 'completed but still current' rows."""
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+
+        cls.case_worker = cls.env["res.users"].create(
+            {
+                "name": "Migration Case Worker",
+                "login": "test_worker_plan_migration",
+                "email": "worker_plan_migration@test.com",
+                "group_ids": [
+                    Command.link(cls.env.ref("base.group_user").id),
+                    Command.link(cls.env.ref("spp_case_base.group_case_worker").id),
+                ],
+            }
+        )
+        cls.client = cls.env["res.partner"].create({"name": "Migration Client"})
+        cls.case_type = cls.env["spp.case.type"].create(
+            {
+                "name": "Migration Case Type",
+                "code": "MIGR01",
+                "domain": "social_protection",
+            }
+        )
+        cls.Plan = cls.env["spp.case.intervention.plan"]
+
+    def _case(self, issue):
+        return self.env["spp.case"].create(
+            {
+                "case_type_id": self.case_type.id,
+                "partner_id": self.client.id,
+                "case_worker_id": self.case_worker.id,
+                "presenting_issue": f"<p>{issue}</p>",
+            }
+        )
+
+    def _stale_plan(self, name, issue):
+        """A plan in the shape pre-fix ``action_complete`` left behind.
+
+        The state is forced with SQL on purpose: writing ``state`` through the
+        ORM is fine, but going through ``action_complete`` would apply the fix
+        and there would be nothing left to migrate.
+        """
+        plan = self.Plan.create(
+            {
+                "name": name,
+                "case_id": self._case(issue).id,
+                "goals": "<p>Goals</p>",
+            }
+        )
+        self.env.cr.execute(
+            "UPDATE spp_case_intervention_plan SET state = 'completed', actual_end_date = CURRENT_DATE WHERE id = %s",
+            (plan.id,),
+        )
+        plan.invalidate_recordset(["state", "actual_end_date"])
+        self.assertEqual(plan.state, "completed")
+        self.assertTrue(plan.is_current, "Test premise: the finished plan is still flagged current")
+        return plan
+
+    def test_migration_demotes_completed_plans(self):
+        """Test that the script releases is_current on completed plans."""
+        stale = self._stale_plan("Stale Completed Plan", "Stale case")
+
+        migrate(self.env.cr, "19.0.2.0.0")
+
+        # The script writes with raw SQL and deliberately does not invalidate,
+        # so the read-back has to.
+        stale.invalidate_recordset(["is_current"])
+        self.assertFalse(stale.is_current, "Migration should release is_current")
+        self.assertEqual(stale.state, "completed", "Migration should not touch state")
+        self.assertTrue(stale.actual_end_date, "Migration should not touch actual_end_date")
+        self.assertFalse(
+            stale.case_id.current_plan_id,
+            "Case should report no current plan after the migration",
+        )
+
+    def test_migration_frees_the_current_plan_slot(self):
+        """Test that a successor plan can be created once the migration has run."""
+        stale = self._stale_plan("Stale Completed Plan", "Blocked case")
+        case = stale.case_id
+
+        migrate(self.env.cr, "19.0.2.0.0")
+        stale.invalidate_recordset(["is_current"])
+
+        successor = self.Plan.create(
+            {
+                "name": "Successor Plan",
+                "case_id": case.id,
+                "goals": "<p>Next cycle</p>",
+            }
+        )
+
+        self.assertTrue(successor.is_current, "Successor plan should be current")
+        self.assertEqual(case.current_plan_id, successor, "Case should point at the successor")
+
+    def test_migration_leaves_unfinished_plans_alone(self):
+        """Test that plans that have not completed keep is_current."""
+        keep = self.Plan.create(
+            {
+                "name": "Active Current Plan",
+                "case_id": self._case("Live case").id,
+                "goals": "<p>Goals</p>",
+                "state": "active",
+            }
+        )
+        self.assertTrue(keep.is_current, "Test premise: the active plan is current")
+
+        migrate(self.env.cr, "19.0.2.0.0")
+
+        keep.invalidate_recordset(["is_current"])
+        self.assertTrue(keep.is_current, "An active plan must stay its case's current plan")
+
+    def test_migration_skips_fresh_install(self):
+        """Test that the script is a no-op when there is no installed version."""
+        stale = self._stale_plan("Stale Completed Plan", "Fresh install case")
+
+        migrate(self.env.cr, None)
+
+        stale.invalidate_recordset(["is_current"])
+        self.assertTrue(
+            stale.is_current,
+            "A fresh install has no legacy rows to repair, so the script must return early",
+        )

spp_case_demo/README.rst

@@ -135,6 +135,15 @@ Dependencies
 Changelog
 =========
 
+19.0.2.0.1
+~~~~~~~~~~
+
+- fix(case): the generator no longer seeds intervention plans that are
+  both ``completed`` and their case's current plan. The ``close_case``
+  journey step and the random-plan helper now complete plans through
+  ``action_complete()``, so a finished demo plan gets an
+  ``actual_end_date`` and releases ``is_current``.
+
 19.0.2.0.0
 ~~~~~~~~~~