Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CPDNPQ-2128] Keep the user TRN unchanged when TRA returns an empty TRN #1782

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down
89 changes: 89 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: "[email protected]",
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 "[email protected]"
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 "[email protected]"
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: "[email protected]", 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 "[email protected]"
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
Loading