From bd9e30fab42a26a51c75e1cc80ea18ddd3e0ed29 Mon Sep 17 00:00:00 2001
From: Morgan Roderick <morgan@roderick.dk>
Date: Tue, 4 Aug 2026 21:57:16 +0200
Subject: [PATCH] fix(member): persist newsletter checkbox state on form
 re-render

Coerce the newsletter param to a real boolean so the checkbox reflects the
user's choice after a failed validation, and so unchecking actually
unsubscribes ("false" was previously truthy).
---
 app/controllers/concerns/member_concerns.rb   |  4 ++
 .../member/details_controller_spec.rb         | 48 +++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/app/controllers/concerns/member_concerns.rb b/app/controllers/concerns/member_concerns.rb
index 873f4ae21..6d4776eab 100644
--- a/app/controllers/concerns/member_concerns.rb
+++ b/app/controllers/concerns/member_concerns.rb
@@ -22,6 +22,10 @@ def member_params
         if permitted_params[:dietary_restrictions]
           permitted_params[:dietary_restrictions] = permitted_params[:dietary_restrictions].reject(&:blank?)
         end
+        # The checkbox submits the raw string "true"/"false". Coerce to a real
+        # boolean so the re-rendered checkbox reflects the user's choice and the
+        # subscribe/unsubscribe decision evaluates correctly ("false" is truthy).
+        permitted_params[:newsletter] = ActiveModel::Type::Boolean.new.cast(permitted_params[:newsletter])
       end
     end
 
diff --git a/spec/controllers/member/details_controller_spec.rb b/spec/controllers/member/details_controller_spec.rb
index bb06347f6..d0db8a953 100644
--- a/spec/controllers/member/details_controller_spec.rb
+++ b/spec/controllers/member/details_controller_spec.rb
@@ -73,6 +73,54 @@
         expect(member.how_you_found_us_other_reason).to eq('From a colleague')
         expect(response).to redirect_to(step2_member_path)
       end
+
+      it 'subscribes to the newsletter when checked' do
+        patch :update, params: {
+          id: member.id,
+          member: {
+            how_you_found_us: 'social_media',
+            newsletter: 'true'
+          }
+        }
+
+        expect(mailing_list).to have_received(:subscribe)
+        expect(mailing_list).not_to have_received(:unsubscribe)
+      end
+
+      it 'unsubscribes from the newsletter when unchecked' do
+        patch :update, params: {
+          id: member.id,
+          member: {
+            how_you_found_us: 'social_media',
+            newsletter: 'false'
+          }
+        }
+
+        expect(mailing_list).to have_received(:unsubscribe)
+        expect(mailing_list).not_to have_received(:subscribe)
+      end
+    end
+
+    context 'with a validation failure' do
+      it 'keeps the newsletter checkbox checked when it was checked' do
+        patch :update, params: {
+          id: member.id,
+          member: { newsletter: 'true' }
+        }
+
+        expect(response.body).to include('You must select one option')
+        expect(response.body).to have_css('input#member_newsletter[checked]')
+      end
+
+      it 'keeps the newsletter checkbox unchecked when it was unchecked' do
+        patch :update, params: {
+          id: member.id,
+          member: { newsletter: 'false' }
+        }
+
+        expect(response.body).to include('You must select one option')
+        expect(response.body).to have_no_css('input#member_newsletter[checked]')
+      end
     end
 
     context 'when update fails (invalid data)' do