spp_case_base: action_create_revision ignores who holds is_current — collides with a successor plan, and erases a completed plan's completion record

[kneckinator]

Filed at origin/19.0 (f6213293). From the review of #478 (fixes #458) — did not block that PR; @gonzalesedwin1123 raised the first half in his review there ("the #458 dead-end partially relocates").

Summary

action_create_revision (spp_case_base/models/case_intervention_plan.py:202) demotes the plan and copies it, unconditionally claiming current status for the copy:

def action_create_revision(self):
    self.ensure_one()
    self.write({"is_current": False, "state": "revised"})      # :207-211
    new_version = self.copy({
        ...
        "is_current": True,                                     # :219
        "state": "draft",
        "actual_end_date": False,
        ...
    })

It never checks whether some other plan on the case is already current, and it never checks whether self is current at all. The button is offered for state in ['active', 'completed'] (views/case_intervention_views.xml:40-45).

Two distinct defects

1. The revision now collides with a successor plan (newly reachable via #478)

1. Complete plan A. Post-fix(spp_case_base): clear is_current when an intervention plan is completed #478 it is state = 'completed', is_current = False.
2. Create plan B on the same case. It defaults is_current = True and becomes the case's current plan — this is the whole point of fix(spp_case_base): clear is_current when an intervention plan is completed #478, and it is what the migration's own log line tells admins to do.
3. Open plan A. The Create Revision button is visible (completed is in its gate). Click it.
4. copy({... "is_current": True ...}) trips _check_single_current_plan (:146-159) against B and the action aborts with "Only one plan can be marked as current for a case." — a message that names neither conflicting plan.

Pre-#478 this pairing was unreachable: B could never have been created while A still held the flag, so the dead-end sat one step earlier. #478 removes the original dead-end and this one becomes reachable in the normal flow.

Not a data-integrity problem — the ValidationError propagates out of the button handler and the whole transaction rolls back, so A's demote-to-revised is undone with it. It is a usability dead-end with an unhelpful message.

2. Revising a completed plan erases the completion record

Independent of #478 and reachable today. Because completed is in the button's gate, revising a finished plan writes state = 'revised' over 'completed' (:210) while leaving actual_end_date set on the same record. The plan then reads as superseded rather than finished, and the only trace that it ever completed is a date field whose state no longer corresponds to it. The reverse asymmetry is telling: the copy explicitly resets actual_end_date (:222) precisely because a revision has not ended, but the original's own is never reconsidered.

Suggested fix

Both follow from the same question — what does "revise" mean for a plan that is not the case's current plan?

• Guard the entry, e.g. refuse when another plan on the case holds is_current, naming it:

  other = self.search([("case_id", "=", self.case_id.id), ("is_current", "=", True), ("id", "!=", self.id)])
  if other:
      raise ValidationError(_(
          "Cannot revise %(plan)s: %(other)s is the case's current plan. "
          "Revise that plan instead, or clear its current flag first.",
          plan=self.name, other=other[0].name,
      ))

  and consider narrowing the button's gate to state == 'active', or to is_current, so the case is unreachable rather than merely refused. Note _check_single_current_plan uses an un-sudo'd self.search, so it is subject to record rules — a guard here inherits that limitation and a caller who cannot see the conflicting plan still gets the raw constraint error.

• For defect 2, decide whether revising a completed plan should be offered at all. If it should, it needs to preserve the completion: either keep state = 'completed' on the original (and stop treating revised as the only supersede marker — previous_version_id on the successor already records the chain), or clear actual_end_date when overwriting to revised. Leaving both set is the one option that cannot be right.

The narrower reading — a finished plan is not revised, it is followed by a new plan — argues for dropping completed from the button's gate entirely, which fixes both defects at once. That is a product call.

Tests

• Complete A, create B, then A.action_create_revision() → either a message naming B, or a hidden/refused button; not the bare one-current-plan constraint.
• Revising a completed plan does not leave state = 'revised' together with a populated actual_end_date.
• test_plan_versioning and test_plan_copy_behavior (tests/test_case_intervention_plan.py) both revise from approved, so neither covers the completed origin.