Skip to content

Commit

Permalink
Use mhv_user_account cached response or nil on User
Browse files Browse the repository at this point in the history
  • Loading branch information
rileyanderson committed Jan 10, 2025
1 parent 4893f1c commit b04b6d6
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 55 deletions.
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def mhv_correlation_id
end

def mhv_user_account
@mhv_user_account ||= MHV::UserAccount::Creator.new(user_verification:).perform
@mhv_user_account ||= MHV::UserAccount::Creator.new(user_verification:, from_cache_only: true).perform
rescue => e
log_mhv_user_account_error(e.message)
nil
Expand Down
16 changes: 11 additions & 5 deletions app/services/mhv/user_account/creator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
module MHV
module UserAccount
class Creator
attr_reader :user_verification, :break_cache
attr_reader :user_verification, :break_cache, :from_cache_only

def initialize(user_verification:, break_cache: false)
def initialize(user_verification:, break_cache: false, from_cache_only: false)
@user_verification = user_verification
@break_cache = break_cache
@from_cache_only = from_cache_only
end

def perform
Expand All @@ -30,16 +31,17 @@ def perform
private

def create_mhv_user_account!
return nil if mhv_account_creation_response.nil? && from_cache_only

account = MHVUserAccount.new(mhv_account_creation_response)
account.validate!
MPIData.find(icn)&.destroy
account
end

def mhv_account_creation_response
tou_occurred_at = current_tou_agreement.created_at

mhv_client.create_account(icn:, email:, tou_occurred_at:, break_cache:)
@mhv_account_creation_response ||= mhv_client.create_account(icn:, email:, tou_occurred_at:, break_cache:,
from_cache_only:)
end

def icn
Expand All @@ -58,6 +60,10 @@ def user_account
@user_account ||= user_verification.user_account
end

def tou_occurred_at
current_tou_agreement.created_at
end

def mhv_client
MHV::AccountCreation::Service.new
end
Expand Down
15 changes: 14 additions & 1 deletion lib/mhv/account_creation/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ module AccountCreation
class Service < Common::Client::Base
configuration Configuration

def create_account(icn:, email:, tou_occurred_at:, break_cache: false)
def create_account(icn:, email:, tou_occurred_at:, break_cache: false, from_cache_only: false)
return find_cached_response(icn) if from_cache_only

params = build_create_account_params(icn:, email:, tou_occurred_at:)

create_account_with_cache(icn:, force: break_cache, expires_in: 1.day) do
Expand All @@ -23,6 +25,17 @@ def create_account(icn:, email:, tou_occurred_at:, break_cache: false)

private

def find_cached_response(icn)
account = Rails.cache.read("#{config.service_name}_#{icn}")

if account.present?
log_payload = { icn:, account:, from_cache: true, from_cache_only: true }
Rails.logger.info("#{config.logging_prefix} create_account success", log_payload)
end

account
end

def create_account_with_cache(icn:, force:, expires_in:, &request)
cache_hit = true
start = nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@
expect(mhv_client).to have_received(:create_account).with(icn:,
email: user_credential_email.credential_email,
tou_occurred_at: terms_of_use_agreement.created_at,
break_cache: true)
break_cache: true,
from_cache_only: false)
end
end

Expand Down
62 changes: 52 additions & 10 deletions spec/lib/mhv/account_creation/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

describe MHV::AccountCreation::Service do
describe '#create_account' do
subject { described_class.new.create_account(icn:, email:, tou_occurred_at:, break_cache:) }
subject { described_class.new.create_account(icn:, email:, tou_occurred_at:, break_cache:, from_cache_only:) }

let(:icn) { '10101V964144' }
let(:email) { '[email protected]' }
Expand All @@ -16,9 +16,20 @@
let(:account_creation_base_url) { 'https://apigw-intb.aws.myhealth.va.gov' }
let(:account_creation_path) { 'v1/usermgmt/account-service/account' }
let(:break_cache) { false }
let(:from_cache_only) { false }
let(:start_time) { Time.zone.now }
let(:end_time) { start_time + 10.seconds }

let(:user_profile_id) { '12345678' }
let(:premium) { true }
let(:champ_va) { true }
let(:patient) { true }
let(:sm_account_created) { true }
let(:message) { 'Existing MHV Account Found for ICN' }
let(:expected_response_body) do
{ user_profile_id:, premium:, champ_va:, patient:, sm_account_created:, message: }
end

before do
allow(Rails.logger).to receive(:info)
allow(Rails.logger).to receive(:error)
Expand Down Expand Up @@ -47,22 +58,53 @@
end
end

context 'when from_cache_only is true' do
let(:from_cache_only) { true }
let(:expected_cache_key) { "mhv_account_creation_#{icn}" }

context 'when the account is in the cache' do
before do
allow(Rails.cache).to receive(:read).with(expected_cache_key).and_return(expected_response_body)
end

it 'returns the cached response' do
expect(subject).to eq(expected_response_body)
end

it 'does not make a request to the account creation service' do
subject
expect(a_request(:post, "#{account_creation_base_url}/#{account_creation_path}")).not_to have_been_made
expect(Rails.logger).not_to have_received(:info).with("#{log_prefix} create_account request",
anything)
end
end

context 'when the account is not in the cache' do
before do
allow(Rails.cache).to receive(:read).with(expected_cache_key).and_return(nil)
end

it 'returns nil' do
expect(subject).to be_nil
end

it 'does not make a request to the account creation service' do
subject
expect(a_request(:post, "#{account_creation_base_url}/#{account_creation_path}")).not_to have_been_made
expect(Rails.logger).not_to have_received(:info).with("#{log_prefix} create_account request",
anything)
end
end
end

context 'when the response is successful' do
let(:successful_response_cassette) { 'mhv/account_creation/account_creation_service_200_found' }
let(:expected_log_message) { "#{log_prefix} create_account success" }
let(:expected_log_payload) do
{ icn:, account: expected_response_body, from_cache: expected_from_cache_log,
duration_ms: expected_duration }.compact
end
let(:user_profile_id) { '12345678' }
let(:premium) { true }
let(:champ_va) { true }
let(:patient) { true }
let(:sm_account_created) { true }
let(:message) { 'Existing MHV Account Found for ICN' }
let(:expected_response_body) do
{ user_profile_id:, premium:, champ_va:, patient:, sm_account_created:, message: }
end

let(:expected_duration) { 10_000.0 }

shared_examples 'a successful external request' do
Expand Down
77 changes: 44 additions & 33 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1399,6 +1399,7 @@
describe '#mhv_user_account' do
let(:user) { build(:user, :loa3) }
let(:icn) { user.icn }
let(:expected_cache_key) { "mhv_account_creation_#{icn}" }

let!(:user_verification) do
create(:idme_user_verification, idme_uuid: user.idme_uuid, user_credential_email:, user_account:)
Expand All @@ -1423,55 +1424,65 @@
before do
allow(Rails.logger).to receive(:info)
allow(MHV::AccountCreation::Service).to receive(:new).and_return(mhv_client)
allow(mhv_client).to receive(:create_account).and_return(mhv_response)
allow(Rails.cache).to receive(:read).with(expected_cache_key).and_return(mhv_response)
end

context 'when the user has all required attributes' do
it 'returns a MHVUserAccount with the expected attributes' do
mhv_user_account = user.mhv_user_account
context 'when the mhv response is cached' do
context 'when the user has all required attributes' do
it 'returns a MHVUserAccount with the expected attributes' do
mhv_user_account = user.mhv_user_account

expect(mhv_user_account).to be_a(MHVUserAccount)
expect(mhv_user_account.attributes).to eq(mhv_response.with_indifferent_access)
expect(mhv_user_account).to be_a(MHVUserAccount)
expect(mhv_user_account.attributes).to eq(mhv_response.with_indifferent_access)
end
end
end

context 'when there is an error creating the account' do
shared_examples 'mhv_user_account error' do
let(:expected_log_message) { '[User] mhv_user_account error' }
let(:expected_log_payload) { { error_message: /#{expected_error_message}/, icn: user.icn } }
context 'when there is an error creating the account' do
shared_examples 'mhv_user_account error' do
let(:expected_log_message) { '[User] mhv_user_account error' }
let(:expected_log_payload) { { error_message: /#{expected_error_message}/, icn: user.icn } }

it 'logs and returns nil' do
expect(user.mhv_user_account).to be_nil
expect(Rails.logger).to have_received(:info).with(expected_log_message, expected_log_payload)
it 'logs and returns nil' do
expect(user.mhv_user_account).to be_nil
expect(Rails.logger).to have_received(:info).with(expected_log_message, expected_log_payload)
end
end
end

context 'when the user does not have a terms_of_use_agreement' do
let(:terms_of_use_agreement) { nil }
let(:expected_error_message) { 'Current terms of use agreement must be present' }
context 'when the user does not have a terms_of_use_agreement' do
let(:terms_of_use_agreement) { nil }
let(:expected_error_message) { 'Current terms of use agreement must be present' }

it_behaves_like 'mhv_user_account error'
end
it_behaves_like 'mhv_user_account error'
end

context 'when the user has not accepted the terms of use' do
let(:terms_of_use_response) { 'declined' }
let(:expected_error_message) { "Current terms of use agreement must be 'accepted'" }
context 'when the user has not accepted the terms of use' do
let(:terms_of_use_response) { 'declined' }
let(:expected_error_message) { "Current terms of use agreement must be 'accepted'" }

it_behaves_like 'mhv_user_account error'
end
it_behaves_like 'mhv_user_account error'
end

context 'when the user does not have a user_credential_email' do
let(:user_credential_email) { nil }
let(:expected_error_message) { 'Email must be present' }

it_behaves_like 'mhv_user_account error'
end

context 'when the user does not have a user_credential_email' do
let(:user_credential_email) { nil }
let(:expected_error_message) { 'Email must be present' }
context 'when the user does not have an icn' do
let(:icn) { nil }
let(:expected_error_message) { 'ICN must be present' }

it_behaves_like 'mhv_user_account error'
it_behaves_like 'mhv_user_account error'
end
end
end

context 'when the user does not have an icn' do
let(:icn) { nil }
let(:expected_error_message) { 'ICN must be present' }
context 'when the mhv response is not cached' do
let(:mhv_response) { nil }

it_behaves_like 'mhv_user_account error'
it 'returns nil' do
expect(user.mhv_user_account).to be_nil
end
end
end
Expand Down
42 changes: 38 additions & 4 deletions spec/services/mhv/user_account/creator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
require 'mhv/account_creation/service'

RSpec.describe MHV::UserAccount::Creator do
subject { described_class.new(user_verification:, break_cache:) }
subject { described_class.new(user_verification:, break_cache:, from_cache_only:) }

let(:user_account) { create(:user_account, icn:) }
let(:user_verification) { create(:user_verification, user_account:, user_credential_email:) }
Expand All @@ -14,6 +14,7 @@
let(:email) { '[email protected]' }
let(:tou_occurred_at) { terms_of_use_agreement&.created_at }
let(:break_cache) { false }
let(:from_cache_only) { false }
let(:mhv_client) { instance_double(MHV::AccountCreation::Service) }
let(:mhv_response_body) do
{
Expand All @@ -30,7 +31,7 @@

allow(MHV::AccountCreation::Service).to receive(:new).and_return(mhv_client)
allow(mhv_client).to receive(:create_account)
.with(icn:, email:, tou_occurred_at:, break_cache:)
.with(icn:, email:, tou_occurred_at:, break_cache:, from_cache_only:)
.and_return(mhv_response_body)
end

Expand Down Expand Up @@ -80,7 +81,8 @@
context 'when break_cache is false' do
it 'calls MHV::AccountCreation::Service#create_account with break_cache: false' do
subject.perform
expect(mhv_client).to have_received(:create_account).with(icn:, email:, tou_occurred_at:, break_cache: false)
expect(mhv_client).to have_received(:create_account).with(icn:, email:, tou_occurred_at:, break_cache: false,
from_cache_only:)
end
end

Expand All @@ -89,7 +91,39 @@

it 'calls MHV::AccountCreation::Service#create_account with break_cache: true' do
subject.perform
expect(mhv_client).to have_received(:create_account).with(icn:, email:, tou_occurred_at:, break_cache: true)
expect(mhv_client).to have_received(:create_account).with(icn:, email:, tou_occurred_at:, break_cache: true,
from_cache_only:)
end
end

context 'when from_cache_only is true' do
let(:from_cache_only) { true }
let(:expected_cache_key) { "mhv_account_creation_#{icn}" }

before do
allow(Rails.cache).to receive(:read).with(expected_cache_key).and_return(mhv_response_body)
end

it 'calls MHV::AccountCreation::Service#create_account with from_cache_only: true' do
subject.perform
expect(mhv_client).to have_received(:create_account).with(icn:, email:, tou_occurred_at:, break_cache:,
from_cache_only: true)
end

context 'when the response is cached' do
it 'returns the cached response' do
mhv_user_account = subject.perform
expect(mhv_user_account).to be_a(MHVUserAccount)
expect(mhv_user_account).to be_valid
end
end

context 'when the response is not cached' do
let(:mhv_response_body) { nil }

it 'returns nil' do
expect(subject.perform).to be_nil
end
end
end
end
Expand Down

0 comments on commit b04b6d6

Please sign in to comment.