From 82e22f25c3000f7c888ea1eaa1b6e81fa7a374ce Mon Sep 17 00:00:00 2001
From: Matt Van Horn <mvanhorn@gmail.com>
Date: Tue, 12 May 2026 02:22:44 -0700
Subject: [PATCH] fix(user): restrict_with_error on case_assignments to prevent
 history loss
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The has_many :case_assignments association on User had `dependent:
:destroy` with a `# TODO destroy is wrong` comment. Cascading destroy
on a volunteer would hard-delete every case_assignment, losing the
audit trail (active flag, dates, the volunteer↔case mapping).

The supported retirement flow is Volunteer#deactivate, which marks the
user and each case_assignment inactive while preserving the rows.
`dependent: :restrict_with_error` makes that invariant explicit:
anything that tries to hard-delete a volunteer with live assignments
now fails fast and surfaces an error instead of silently losing
history.

Adds a shoulda-matchers assertion for the dependent option and two
behavioral specs that verify the destroy is refused and the
case_assignment rows are preserved.

Resolves #6911
---
 app/models/user.rb       |  7 ++++++-
 spec/models/user_spec.rb | 26 +++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 99c3f24e98..0e2766f15d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -18,7 +18,12 @@ class User < ApplicationRecord
 
   belongs_to :casa_org
 
-  has_many :case_assignments, foreign_key: "volunteer_id", dependent: :destroy # TODO destroy is wrong
+  # Volunteers are deactivated, not destroyed: Volunteer#deactivate sets the
+  # user and each case_assignment inactive while preserving the rows for
+  # audit/history. restrict_with_error catches anything that tries to
+  # hard-delete a volunteer that still has assignments, rather than silently
+  # cascading and losing the history. Issue #6911.
+  has_many :case_assignments, foreign_key: "volunteer_id", dependent: :restrict_with_error
   has_many :casa_cases, -> { where(case_assignments: {active: true}) }, through: :case_assignments
 
   has_many :case_contacts, foreign_key: "creator_id"
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index def203c0dc..b13ae3fb8b 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -3,8 +3,32 @@
 RSpec.describe User, type: :model do
   it { is_expected.to belong_to(:casa_org) }
 
-  it { is_expected.to have_many(:case_assignments) }
+  it { is_expected.to have_many(:case_assignments).with_foreign_key(:volunteer_id).dependent(:restrict_with_error) }
   it { is_expected.to have_many(:casa_cases).through(:case_assignments) }
+
+  describe "destroying a volunteer with case_assignments" do
+    # The deactivate flow (Volunteer#deactivate) is the supported way to retire
+    # a volunteer: it sets `active: false` on the user and on every
+    # case_assignment, preserving rows for audit. Hard-deleting a volunteer
+    # that still has assignments would silently discard that history, so the
+    # association is :restrict_with_error. Issue #6911.
+    let(:volunteer) { create(:volunteer) }
+
+    before do
+      create(:case_assignment, casa_case: create(:casa_case, casa_org: volunteer.casa_org), volunteer: volunteer)
+    end
+
+    it "refuses to destroy the volunteer" do
+      expect { volunteer.destroy }.not_to change(CaseAssignment, :count)
+      expect(volunteer.persisted?).to eq(true)
+    end
+
+    it "leaves the volunteer's case_assignments intact" do
+      assignment = volunteer.case_assignments.first
+      volunteer.destroy
+      expect(CaseAssignment.exists?(assignment.id)).to eq(true)
+    end
+  end
   it { is_expected.to have_many(:case_contacts) }
   it { is_expected.to have_many(:sent_emails) }
   it { is_expected.to have_many(:user_languages) }