diff --git a/app/models/user.rb b/app/models/user.rb index 7187daf7d7..89afe939c0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -65,19 +65,21 @@ def self.find_or_create_from_tra_data_on_uid(provider_data, feature_flag_id:) user_from_provider_data = find_or_initialize_by(provider: provider_data.provider, uid: provider_data.uid) user_from_provider_data.feature_flag_id = feature_flag_id - trn = provider_data.info.trn + provider_trn = provider_data.info.trn user_from_provider_data.assign_attributes( email: provider_data.info.email, date_of_birth: provider_data.info.date_of_birth, - trn:, trn_lookup_status: provider_data.info.trn_lookup_status, - trn_verified: trn.present?, + trn_verified: provider_trn.present?, full_name: provider_data.info.preferred_name || provider_data.info.name, raw_tra_provider_data: provider_data, updated_from_tra_at: Time.zone.now, ) + # The user's TRN should remain unchanged if the TRA returns an empty TRN + user_from_provider_data.trn = provider_trn if provider_trn.present? + user_from_provider_data.tap(&:save) end @@ -86,20 +88,22 @@ def self.find_or_create_from_tra_data_on_unclaimed_email(provider_data, feature_ user_from_provider_data.feature_flag_id = feature_flag_id - trn = provider_data.info.trn + provider_trn = provider_data.info.trn user_from_provider_data.assign_attributes( provider: provider_data.provider, uid: provider_data.uid, date_of_birth: provider_data.info.date_of_birth, - trn:, trn_lookup_status: provider_data.info.trn_lookup_status, - trn_verified: trn.present?, + trn_verified: provider_trn.present?, full_name: provider_data.info.preferred_name || provider_data.info.name, raw_tra_provider_data: provider_data, updated_from_tra_at: Time.zone.now, ) + # The user's TRN should remain unchanged if the TRA returns an empty TRN + user_from_provider_data.trn = provider_trn if provider_trn.present? + unless user_from_provider_data.save Rails.logger.info("[GAI] User not persisted, #{user_from_provider_data.errors.full_messages.join(';')}, ID=#{user_from_provider_data.id}, UID=#{provider_data.uid}, trying to reclaim email failed") end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4d00bf2412..38755bdfb9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -157,4 +157,93 @@ end end end + + context "when updating the user with the TRA data" do + let(:provider_data) do + OpenStruct.new( + provider: "example_provider", + uid: "example_uid", + info: OpenStruct.new( + email: "user@example.com", + email_verified: true, + name: "Example User", + trn:, + ), + ) + end + + let(:feature_flag_id) { 1 } + let(:trn) { "1234567" } + + shared_examples "a TRN updater" do |method_name| + context "when TRA provides a TRN" do + it "update the user" do + described_class.public_send(method_name, provider_data, feature_flag_id:) + + expect(user.trn).to eq "1234567" + expect(user.email).to eq "user@example.com" + expect(user.full_name).to eq "Example User" + end + end + + context "when TRA provides a nil TRN" do + let(:trn) { nil } + + it "update the user, but keep the TRN unchanged" do + original_trn = user.trn + + described_class.public_send(method_name, provider_data, feature_flag_id:) + + expect(user.trn).to eq original_trn + expect(user.email).to eq "user@example.com" + expect(user.full_name).to eq "Example User" + end + end + end + + describe ".find_or_create_from_tra_data_on_uid" do + let(:user) { create(:user, provider: "example_provider", trn: "1020304") } + + before do + allow(User).to receive(:find_or_initialize_by).and_return(user) + end + + it_behaves_like "a TRN updater", :find_or_create_from_tra_data_on_uid + end + + describe ".find_or_create_from_tra_data_on_unclaimed_email" do + let(:user) { create(:user, provider: nil, uid: nil, email: "user@example.com", trn: "1020304") } + + before do + allow(User).to receive(:find_or_initialize_by).and_return(user) + end + + it_behaves_like "a TRN updater", :find_or_create_from_tra_data_on_unclaimed_email + + context "when TRA provides a TRN and user has unclaimed email" do + it "updates provider and UID along with TRN" do + described_class.find_or_create_from_tra_data_on_unclaimed_email(provider_data, feature_flag_id:) + + expect(user.trn).to eq "1234567" + expect(user.email).to eq "user@example.com" + expect(user.full_name).to eq "Example User" + expect(user.provider).to eq "example_provider" + expect(user.uid).to eq "example_uid" + end + end + + context "when user cannot be saved" do + before do + allow(user).to receive(:save).and_return(false) + allow(Rails.logger).to receive(:info) + end + + it "logs the error" do + described_class.find_or_create_from_tra_data_on_unclaimed_email(provider_data, feature_flag_id:) + + expect(Rails.logger).to have_received(:info).with(/\[GAI\] User not persisted, .+ trying to reclaim email failed/) + end + end + end + end end