diff --git a/app/mailers/devise_mailer.rb b/app/mailers/devise_mailer.rb index c648168fd..cf1777dd4 100644 --- a/app/mailers/devise_mailer.rb +++ b/app/mailers/devise_mailer.rb @@ -61,12 +61,20 @@ def create_notification_record return unless @mail && @record kind = notification_kind_for_devise_action[action_name] + + # For email changes (reconfirmation), use account_email_change_requested instead + if action_name == "confirmation_instructions" && @record.try(:pending_reconfirmation?) + kind = "account_email_change_requested" + end + return unless kind # don’t create fyi emails for Devise mailers you don’t care about + recipient_email = (kind == "account_email_change_requested") ? @record.unconfirmed_email : @record.email + notification = NotificationServices::CreateNotification.call( noticeable: @record, recipient_role: :person, - recipient_email: @record.email, + recipient_email: recipient_email, kind: kind, notification_type: 1, deliver: false # Devise already sent the email, so no need to deliver via the job @@ -104,6 +112,12 @@ def track_devise_email_event "confirmation_instructions" => "auth.confirmation_email_sent", "unlock_instructions" => "auth.unlock_email_sent" }[action_name] + + # For email changes (reconfirmation), use a specific event name + if action_name == "confirmation_instructions" && @record.try(:pending_reconfirmation?) + event_name = "auth.email_change_requested_email_sent" + end + return unless event_name Analytics::AhoyTracker.track_auth_event( diff --git a/app/models/notification.rb b/app/models/notification.rb index 7159164bb..355a4e63c 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -13,6 +13,8 @@ class Notification < ApplicationRecord welcome_instructions account_confirmation account_confirmation_fyi + account_email_change_requested + account_email_changed account_confirmed account_confirmed_fyi account_unlock_fyi diff --git a/app/models/user.rb b/app/models/user.rb index e8f9bde52..0c3f66eb9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -17,6 +17,9 @@ class User < ApplicationRecord after_update :track_admin_change after_update :track_inactive_change after_update :track_password_reset_sent + after_update :track_password_changed + + after_commit :create_email_changed_notification, on: :update before_destroy :track_account_deleted @@ -341,12 +344,31 @@ def track_password_reset_sent track_auth_event("auth.password_reset_sent", { sent_at: reset_password_sent_at }) end + def track_password_changed + return unless saved_change_to_encrypted_password? + track_auth_event("auth.password_changed") + end + def sync_email_to_person return unless saved_change_to_email? && person.present? person.update(email: email) end + def create_email_changed_notification + return unless previous_changes.key?("email") + + _from, to = previous_changes["email"] + NotificationServices::CreateNotification.call( + noticeable: self, + recipient_role: :person, + recipient_email: to, + kind: :account_email_changed, + notification_type: 1, + deliver: false + ) + end + def sync_locked_at_from_locked return unless @locked_will_change diff --git a/spec/mailers/devise_mailer_spec.rb b/spec/mailers/devise_mailer_spec.rb new file mode 100644 index 000000000..744258819 --- /dev/null +++ b/spec/mailers/devise_mailer_spec.rb @@ -0,0 +1,115 @@ +require "rails_helper" + +RSpec.describe DeviseMailer, type: :mailer do + let(:user) { create(:user, email: "user@example.com") } + let(:token) { "fake-token-123" } + + describe "#track_devise_email_event" do + context "when sending confirmation for initial signup" do + it "tracks auth.confirmation_email_sent" do + expect(Analytics::AhoyTracker).to receive(:track_auth_event).with( + "auth.confirmation_email_sent", + hash_including(record_id: user.id, record_type: "User"), + user: anything + ) + + described_class.confirmation_instructions(user, token).deliver_now + end + end + + context "when sending confirmation for email change (reconfirmation)" do + before do + user.skip_confirmation_notification! + user.update!(email: "new@example.com") + # Devise sets unconfirmed_email and keeps original email + end + + it "tracks auth.email_change_requested_email_sent instead of confirmation_email_sent" do + expect(Analytics::AhoyTracker).to receive(:track_auth_event).with( + "auth.email_change_requested_email_sent", + hash_including(record_id: user.id, record_type: "User"), + user: anything + ) + + described_class.confirmation_instructions(user, token).deliver_now + end + end + + context "when sending reset password instructions" do + it "tracks auth.reset_password_email_sent" do + expect(Analytics::AhoyTracker).to receive(:track_auth_event).with( + "auth.reset_password_email_sent", + hash_including(record_id: user.id, record_type: "User"), + user: anything + ) + + described_class.reset_password_instructions(user, token).deliver_now + end + end + + # unlock_instructions not tested here — app uses unlock_strategy: :none + # so user_unlock_url route doesn't exist and the view can't render + end + + describe "#create_notification_record" do + let(:notification) { build(:notification) } + + before do + allow(Rails.env).to receive(:test?).and_return(false) + allow(NotificationServices::PersistDeliveredEmail).to receive(:call) + allow(NotificationServices::CreateNotification).to receive(:call).and_return(notification) + end + + context "when sending confirmation for initial signup" do + it "creates an account_confirmation notification" do + expect(NotificationServices::CreateNotification).to receive(:call).with( + hash_including( + kind: "account_confirmation", + recipient_role: :person, + recipient_email: user.email, + deliver: false + ) + ).and_return(notification) + + described_class.confirmation_instructions(user, token).deliver_now + end + end + + context "when sending confirmation for email change (reconfirmation)" do + before do + user.skip_confirmation_notification! + user.update!(email: "new@example.com") + end + + it "creates an account_email_change_requested notification" do + expect(NotificationServices::CreateNotification).to receive(:call).with( + hash_including( + kind: "account_email_change_requested", + recipient_role: :person, + recipient_email: user.unconfirmed_email, + deliver: false + ) + ).and_return(notification) + + described_class.confirmation_instructions(user, token).deliver_now + end + end + + context "when sending reset password instructions" do + it "creates a reset_password notification and a reset_password_fyi" do + expect(NotificationServices::CreateNotification).to receive(:call).with( + hash_including(kind: "reset_password", recipient_role: :person, deliver: false) + ).ordered.and_return(notification) + + expect(NotificationServices::CreateNotification).to receive(:call).with( + hash_including(kind: "reset_password_fyi", recipient_role: :admin, deliver: true) + ).ordered + + described_class.reset_password_instructions(user, token).deliver_now + end + end + + # unlock_instructions not tested here — app uses unlock_strategy: :none + # so user_unlock_url route doesn't exist and the view can't render + end +end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index c593548b7..720f78579 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -8,6 +8,26 @@ it { should have_many(:child_notifications).class_name('Notification').with_foreign_key(:parent_notification_id) } end + describe "KINDS" do + it "includes account_email_change_requested" do + expect(Notification::KINDS).to include("account_email_change_requested") + end + + it "includes account_email_changed" do + expect(Notification::KINDS).to include("account_email_changed") + end + + it "validates account_email_change_requested as a valid kind" do + notification = build(:notification, kind: "account_email_change_requested", recipient_role: "person") + expect(notification).to be_valid + end + + it "validates account_email_changed as a valid kind" do + notification = build(:notification, kind: "account_email_changed", recipient_role: "person") + expect(notification).to be_valid + end + end + describe "#resendable?" do it "returns true for notification kinds handled by the mailer job" do notification = build(:notification, kind: "reset_password_fyi") diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b57c8ce3b..658ca7c54 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -336,4 +336,79 @@ expect(results).not_to include(regular_user, inactive_user) end end + + describe "#track_password_changed" do + let(:user) { create(:user, password: "original_password") } + + it "tracks auth.password_changed when encrypted_password changes" do + expect(Analytics::LifecycleBuffer).to receive(:push).with( + hash_including(name: "auth.password_changed") + ) + + user.update!(password: "new_secure_password", password_confirmation: "new_secure_password") + end + + it "does not track when other fields change" do + expect(Analytics::LifecycleBuffer).not_to receive(:push).with( + hash_including(name: "auth.password_changed") + ) + + user.update!(first_name: "Updated") + end + end + + describe "#create_email_changed_notification" do + let(:user) { create(:user, email: "old@example.com") } + + it "creates an account_email_changed notification when email changes" do + user.skip_reconfirmation! + + expect { + user.update!(email: "changed@example.com") + }.to change(Notification, :count).by(1) + + notification = Notification.last + expect(notification.kind).to eq("account_email_changed") + expect(notification.recipient_role).to eq("person") + expect(notification.recipient_email).to eq("changed@example.com") + expect(notification.noticeable).to eq(user) + expect(notification.delivered_at).to be_nil + end + + it "does not create notification when email does not change" do + expect { + user.update!(first_name: "Updated") + }.not_to change(Notification, :count) + end + end + + describe "#track_email_change" do + let(:user) { create(:user, email: "old@example.com") } + + it "tracks auth.email_changed when email is confirmed" do + user.skip_reconfirmation! + + expect(Analytics::LifecycleBuffer).to receive(:push).with( + hash_including( + name: "auth.email_changed", + properties: hash_including(changes: { email: { before: "old@example.com", after: "new@example.com" } }) + ) + ) + + user.update!(email: "new@example.com") + end + + it "tracks auth.email_update_requested when unconfirmed_email is set" do + expect(Analytics::LifecycleBuffer).to receive(:push).with( + hash_including( + name: "auth.email_update_requested", + properties: hash_including(changes: { email: { before: "old@example.com", after: "pending@example.com" } }) + ) + ) + + user.email = "pending@example.com" + user.skip_confirmation_notification! + user.save! + end + end end