spp_case_base: nothing constrains is_current against state — a completed plan can be re-marked current, undoing #458

[kneckinator]

Filed at origin/19.0 (f6213293). From the review of #478 (fixes #458) — deliberately not folded into that PR; see "Why this is not a release-week change" below.

Summary

#478 makes action_complete release is_current, and ships a data migration that repairs the rows the old behaviour left behind. Neither stops the state coming straight back, because nothing in the model expresses the invariant a finished plan is not the case's current plan.

_check_single_current_plan (spp_case_base/models/case_intervention_plan.py:146-159) is the only constraint on is_current, and it only rejects a second current plan:

@api.constrains("is_current")
def _check_single_current_plan(self):
    for plan in self:
        if plan.is_current:
            other_current = self.search([... ("is_current", "=", True), ("id", "!=", plan.id)])
            if other_current:
                raise ValidationError(_("Only one plan can be marked as current for a case."))

is_current is a plain editable Boolean, rendered on the plan form with no readonly (views/case_intervention_views.xml:84). So:

• A case worker opens a completed plan, ticks Is Current Plan, saves. The constraint finds no competing current plan and the write succeeds. The case is back to current_plan_id pointing at completed work while has_active_plan reads False — spp_case_base: action_complete never clears is_current, leaving a finished plan as the case's current plan #458 verbatim.
• plan.write({"is_current": True}) over RPC does the same. Write is granted to group_case_worker / group_case_supervisor / group_case_manager (security/ir.model.access.csv:18-20).
• The same hole admits state = 'revised' with is_current = True, which action_create_revision never produces but a bare write does.

Impact

Low likelihood, but it makes the #478 migration a one-time cleanup of a state the model still permits, rather than the enforcement of an invariant. Anything keyed on "the case's current plan" — current_plan_id, has_active_plan, and downstream domains — can silently go back to disagreeing with each other. It also means the invariant is enforced in exactly one place (action_complete), so every future terminal transition has to remember to clear the flag; #478 was the third hand-written is_current mutation site added one at a time.

Per AGENTS.md's own state-machine rule ("records must have consistent state"), this is the layer the rule belongs in.

Two candidate fixes

(a) Add the missing constraint

@api.constrains("is_current", "state")
def _check_current_plan_not_finished(self):
    for plan in self:
        if plan.is_current and plan.state in ("completed", "revised"):
            raise ValidationError(_("A completed or revised plan cannot be the case's current plan."))

Plus readonly on the form field, or a readonly="state in ['completed', 'revised']" gate, so the UI matches.

(b) Stop storing it

Make is_current derived — state not in ('completed', 'revised') combined with the highest version per case — and replace the Python constraint with a partial unique index (UNIQUE (case_id) WHERE is_current). That removes the bug class rather than its third instance, and the DB-level index closes a second gap: _check_single_current_plan uses an un-sudo'd self.search, so a user who cannot see the competing plan does not trip the constraint at all, and two current plans can be created legitimately by two differently-scoped users.

(b) is the better end state and the larger change: is_current is written by default=True on create, by action_create_revision at :209 and :219, and by action_complete at :198; it is consumed by two non-stored computes (models/case.py:328-339), three view sites, and downstream modules that write it directly. It needs a maintainer decision, not a drive-by.

Why this is not a release-week change

Raised during #478 and left out on purpose. Constraint (a) as written would reject create({"state": "completed", ...}), and that shape is relied on by existing tests — test_cannot_reset_completed_plan (tests/test_case_intervention_plan.py) creates a plan with state="completed" and the default is_current=True — and by at least one downstream implementation's test fixtures. So (a) needs either a create-time carve-out, or those fixtures updated in the same change. Either is fine on a normal cycle; neither belongs in a fix going into next week's release.

Tests

• Ticking is_current on a completed plan is refused (and on a revised plan).
• create({"state": "completed", "is_current": True}) — decide and pin the intended behaviour explicitly, since it is currently the shape existing fixtures use.
• Two users with disjoint record-rule scopes cannot each mark a plan on the same case as current (only closed by (b)).