From 7bd73cd60258f907c939a2f543f33c1aef7e38d5 Mon Sep 17 00:00:00 2001 From: David M <207745043+davidmillen50@users.noreply.github.com> Date: Fri, 30 Oct 2026 16:02:45 +0000 Subject: [PATCH] Refactor: Replace the Auditor write path with MemberActivityRecorder to create one funnel for activity rows --- app/controllers/admin/sponsors_controller.rb | 10 ++- .../contact_preferences_controller.rb | 8 +- app/services/member_activity_recorder.rb | 10 ++- .../contact_preferences_controller_spec.rb | 82 +++++++++++++++++++ spec/features/admin/sponsor_spec.rb | 22 ++++- 5 files changed, 122 insertions(+), 10 deletions(-) create mode 100644 spec/controllers/contact_preferences_controller_spec.rb diff --git a/app/controllers/admin/sponsors_controller.rb b/app/controllers/admin/sponsors_controller.rb index fa16493da..52f1a0f30 100644 --- a/app/controllers/admin/sponsors_controller.rb +++ b/app/controllers/admin/sponsors_controller.rb @@ -85,8 +85,14 @@ def audit_contact_subscription(contact) return unless contact.previous_changes.key?(:mailing_list_consent) contact_audit_key = contact.mailing_list_consent ? 'subscribe' : 'unsubscribe' - Auditor::Audit.new(@sponsor, "sponsor.admin_contact_#{contact_audit_key}", current_user, contact) - .log_with_note(contact.email) + + MemberActivityRecorder.record( + actor: current_user, + key: "sponsor.admin_contact_#{contact_audit_key}", + trackable: @sponsor, + recipient: contact, + contact_email: contact.email + ) end def filter_search_params diff --git a/app/controllers/contact_preferences_controller.rb b/app/controllers/contact_preferences_controller.rb index 184fa3e95..1747413d8 100644 --- a/app/controllers/contact_preferences_controller.rb +++ b/app/controllers/contact_preferences_controller.rb @@ -36,7 +36,11 @@ def subscription_preference(contact) end def audit_contact_subscription(contact) - Auditor::Audit.new(contact.sponsor, subscription_preference(contact), contact) - .log_with_note(contact.email) + MemberActivityRecorder.record( + actor: contact, + key: subscription_preference(contact), + trackable: contact.sponsor, + contact_email: contact.email + ) end end diff --git a/app/services/member_activity_recorder.rb b/app/services/member_activity_recorder.rb index b5baf8428..8a25279c3 100644 --- a/app/services/member_activity_recorder.rb +++ b/app/services/member_activity_recorder.rb @@ -42,10 +42,16 @@ class MemberActivityRecorder # historical activity is ever backfilled. TRACKING_STARTED_ON = Date.new(2026, 9, 8).freeze - def self.record(actor:, key:, trackable: nil, recipient: nil) + def self.record(actor:, key:, trackable: nil, recipient: nil, contact_email: nil) Rails.logger.warn("MemberActivityRecorder: unregistered key '#{key}'") unless KEYS.include?(key) trackable ||= actor - PublicActivity::Activity.create!(owner: actor, key:, trackable:, recipient:) + PublicActivity::Activity.create!( + owner: actor, + key:, + trackable:, + recipient:, + parameters: contact_email ? { note: contact_email } : {} + ) rescue StandardError => e Rails.logger.warn("MemberActivityRecorder failed (#{key}): #{e.class}: #{e.message}") end diff --git a/spec/controllers/contact_preferences_controller_spec.rb b/spec/controllers/contact_preferences_controller_spec.rb new file mode 100644 index 000000000..38198fe39 --- /dev/null +++ b/spec/controllers/contact_preferences_controller_spec.rb @@ -0,0 +1,82 @@ +require 'rails_helper' + +RSpec.describe ContactPreferencesController do + let(:contact) { Fabricate(:contact, mailing_list_consent: false) } + + before do + allow(ContactMailingListService).to receive(:sync) + end + + describe 'GET #show' do + it 'sets the contact for a valid token' do + get :show, params: { token: contact.token } + + expect(response).to have_http_status(:success) + expect(controller.instance_variable_get(:@contact)).to eq(contact) + expect(flash[:notice]).to be_nil + end + + it 'shows an invalid-session notice for an unknown token' do + get :show, params: { token: 'unknown-token' } + + expect(response).to have_http_status(:success) + expect(flash[:notice]).to eq(I18n.t('messages.invalid_session')) + end + end + + describe 'PATCH #update' do + it 'passes the contact and email to the activity recorder' do + expect(MemberActivityRecorder).to receive(:record).with( + actor: contact, + key: 'sponsor.contact_subscribe', + trackable: contact.sponsor, + contact_email: contact.email + ) + + patch :update, params: { + contact: { token: contact.token, mailing_list_consent: '1' } + } + end + + it 'subscribes the contact and records the consent activity' do + patch :update, params: { + contact: { token: contact.token, mailing_list_consent: '1' } + } + + expect(contact.reload.mailing_list_consent).to be(true) + expect(ContactMailingListService).to have_received(:sync).with(contact) + expect(response).to redirect_to( + contact_preferences_path(token: contact.token, notice: I18n.t('messages.contact_preferences.updated')) + ) + + activity = PublicActivity::Activity.find_by!(key: 'sponsor.contact_subscribe') + expect(activity.owner).to eq(contact) + expect(activity.trackable).to eq(contact.sponsor) + expect(activity.parameters).to include(note: contact.email) + end + + it 'unsubscribes the contact and records the consent activity' do + contact.update!(mailing_list_consent: true) + + patch :update, params: { + contact: { token: contact.token, mailing_list_consent: '0' } + } + + expect(contact.reload.mailing_list_consent).to be(false) + expect(ContactMailingListService).to have_received(:sync).with(contact) + + activity = PublicActivity::Activity.find_by!(key: 'sponsor.contact_unsubscribe') + expect(activity.owner).to eq(contact) + expect(activity.trackable).to eq(contact.sponsor) + expect(activity.parameters).to include(note: contact.email) + end + + it 'returns not found when the submitted token is unknown' do + patch :update, params: { + contact: { token: 'unknown-token', mailing_list_consent: '1' } + } + + expect(response).to have_http_status(:not_found) + end + end +end diff --git a/spec/features/admin/sponsor_spec.rb b/spec/features/admin/sponsor_spec.rb index f5e49c070..eb3a33d21 100644 --- a/spec/features/admin/sponsor_spec.rb +++ b/spec/features/admin/sponsor_spec.rb @@ -161,8 +161,15 @@ context 'with activities' do scenario 'when there are activities' do contact = sponsor.contacts.first - audit = Auditor::Audit.new(sponsor, 'sponsor.admin_contact_subscribe', manager, contact) - audit.log_with_note(contact.email) + contact_audit_key = contact.mailing_list_consent ? 'subscribe' : 'unsubscribe' + + MemberActivityRecorder.record( + actor: manager, + key: "sponsor.admin_contact_#{contact_audit_key}", + trackable: sponsor, + recipient: contact, + contact_email: contact.email + ) visit admin_sponsor_path(sponsor) @@ -173,8 +180,15 @@ scenario 'when an activity is associated with a deleted contact' do contact = sponsor.contacts.first - audit = Auditor::Audit.new(sponsor, 'sponsor.admin_contact_subscribe', manager, contact) - audit.log_with_note(contact.email) + contact_audit_key = contact.mailing_list_consent ? 'subscribe' : 'unsubscribe' + + MemberActivityRecorder.record( + actor: manager, + key: "sponsor.admin_contact_#{contact_audit_key}", + trackable: sponsor, + recipient: contact, + contact_email: contact.email + ) contact.delete