Use mhv_user_account cached response or nil on User (#20216)

Author: rileyanderson

app/models/user.rb

@@ -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

app/services/mhv/user_account/creator.rb

@@ -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
@@ -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
@@ -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

lib/mhv/account_creation/service.rb

@@ -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
@@ -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

spec/controllers/v0/user/mhv_user_accounts_controller_spec.rb

@@ -50,7 +50,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
 

spec/lib/mhv/account_creation/service_spec.rb

@@ -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]' }
@@ -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)
@@ -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

spec/models/user_spec.rb

@@ -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:)
@@ -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

spec/services/mhv/user_account/creator_spec.rb

@@ -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:) }
@@ -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
     {
@@ -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
 
@@ -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
 
@@ -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