chore(spp_case_base): action_complete and action_approve write per record although every value is uniform

[kneckinator]

Filed at origin/19.0 (f6213293). From the review of #478 (fixes #458) — a cleanup, not a defect; deliberately not folded into that PR because the loop matches the method directly above it and consistency inside the file was worth more than the change.

Summary

action_complete and action_approve (spp_case_base/models/case_intervention_plan.py:171 and :191) each write record-by-record although every value in the dict is identical across the recordset:

def action_approve(self):
    for plan in self:
        plan.write({
            "state": "approved",
            "approved_by_id": self.env.user.id,
            "approved_date": fields.Datetime.now(),
        })

def action_complete(self):
    for plan in self:
        plan.write({
            "state": "completed",
            "actual_end_date": fields.Date.context_today(self),
            "is_current": False,
        })

fields.Date.context_today(self) and fields.Datetime.now() are already evaluated off self, not plan — the code itself confirms the values do not vary per record. self.env.user.id likewise.

Impact

Purely a cost, no wrong behaviour. On an N-record set each method issues N UPDATEs, runs the @api.constrains hooks N times (_check_single_current_plan does its own search each time, :150-157), and drives mail.thread tracking N times, where one batched write would do. Both are button handlers so N is normally 1; the loop only costs anything from a list-view multi-select or a server action, which is exactly where a user would notice.

action_submit_for_approval (:161) and action_activate (:183) legitimately keep their loops — they per-record validate before assigning — and action_reset_to_draft (:236) likewise. Only the two above are uniform-value writes.

Suggested fix

def action_complete(self):
    """Mark plan as completed."""
    self.write({
        "state": "completed",
        "actual_end_date": fields.Date.context_today(self),
        "is_current": False,
    })
    return True

and the equivalent for action_approve. Both keep return True.

Order this after #497 if that lands first: adding the active-only guard to action_complete reintroduces a per-record check, and the natural shape then becomes validate-then-batch —

invalid = self.filtered(lambda p: p.state != "active")
if invalid:
    raise ValidationError(...)
self.write({...})

— rather than a loop. Doing this one first would just be undone.

Tests

Existing coverage is sufficient for the single-record path (test_plan_approval_workflow, the test_complete_* tests). Worth adding one multi-record case per method, since nothing currently calls either on more than one plan:

• (plan_a | plan_b).action_complete() completes both and clears is_current on both.
• Same for action_approve, asserting approved_by_id on both.