5708 vet360 linking (#12866)

* 5708: reduce performance impact of vet360 linking

config/redis.yml

@@ -123,9 +123,6 @@ development: &defaults
   iam_user_identity:
     namespace: iam-user-identity
     each_ttl: 1800
-  mobile_vets360_account_link_lock:
-    namespace: mobile-vets360-account-link
-    each_ttl: 180
   mobile_app_appointments_store:
     namespace: mobile-app-appointments-store
     each_ttl: 1800

modules/mobile/app/controllers/mobile/application_controller.rb

@@ -19,7 +19,6 @@ def authenticate
 
       session_manager = IAMSSOeOAuth::SessionManager.new(access_token)
       @current_user = session_manager.find_or_create_user
-      link_user_with_vets360 if @current_user.vet360_id.blank?
       StatsD.increment('iam_ssoe_oauth.auth.success')
       @current_user
     end
@@ -36,29 +35,6 @@ def raise_unauthorized(detail)
       raise Common::Exceptions::Unauthorized.new(detail:)
     end
 
-    def link_user_with_vets360
-      uuid = @current_user.uuid
-
-      unless vet360_linking_locked?(uuid)
-        lock_vets360_linking(uuid)
-        jid = Mobile::V0::Vet360LinkingJob.perform_async(uuid)
-        Rails.logger.info('Mobile Vet360 account link job id', { job_id: jid })
-      end
-    end
-
-    def vets360_link_redis_lock
-      @redis ||= Redis::Namespace.new(REDIS_CONFIG[:mobile_vets360_account_link_lock][:namespace], redis: $redis)
-    end
-
-    def lock_vets360_linking(account_uuid)
-      vets360_link_redis_lock.set(account_uuid, 1)
-      vets360_link_redis_lock.expire(account_uuid, REDIS_CONFIG[:mobile_vets360_account_link_lock][:each_ttl])
-    end
-
-    def vet360_linking_locked?(account_uuid)
-      !vets360_link_redis_lock.get(account_uuid).nil?
-    end
-
     def session
       return super if sis_authentication?
 

modules/mobile/app/controllers/mobile/v0/users_controller.rb

@@ -4,6 +4,7 @@ module Mobile
   module V0
     class UsersController < ApplicationController
       after_action :pre_cache_resources, only: :show
+      after_action :link_user_with_vet360, only: :show, if: -> { @current_user.vet360_id.blank? }
 
       def show
         map_logingov_to_idme
@@ -40,6 +41,10 @@ def map_logingov_to_idme
           @current_user.identity.sign_in[:service_name] = 'oauth_IDME'
         end
       end
+
+      def link_user_with_vet360
+        Mobile::V0::Vet360LinkingJob.perform_async(@current_user.uuid)
+      end
     end
   end
 end

modules/mobile/app/controllers/mobile/v1/users_controller.rb

@@ -4,6 +4,7 @@ module Mobile
   module V1
     class UsersController < ApplicationController
       after_action :pre_cache_resources, only: :show
+      after_action :link_user_with_vet360, only: :show, if: -> { @current_user.vet360_id.blank? }
 
       def show
         render json: Mobile::V1::UserSerializer.new(@current_user, options)
@@ -25,6 +26,10 @@ def pre_cache_resources
         end
         Mobile::V0::PreCacheClaimsAndAppealsJob.perform_async(@current_user.uuid)
       end
+
+      def link_user_with_vet360
+        Mobile::V0::Vet360LinkingJob.perform_async(@current_user.uuid)
+      end
     end
   end
 end

modules/mobile/app/services/mobile/v0/profile/sync_update_service.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require 'va_profile/person/service'
-
 module Mobile
   module V0
     module Profile
@@ -49,18 +47,6 @@ def save_and_await_response(resource_type:, params:, update: false)
           )
         end
 
-        def await_vet360_account_link
-          response = person_service.init_vet360_id
-          initial_transaction = AsyncTransaction::Vet360::InitializePersonTransaction.start(@user, response)
-
-          # return non-received status transactions (errors)
-          return initial_transaction unless initial_transaction.transaction_status == TRANSACTION_RECEIVED
-
-          poll_with_backoff do
-            check_transaction_status!(initial_transaction.transaction_id)
-          end
-        end
-
         private
 
         def save!(http_method, resource_type, params)
@@ -145,10 +131,6 @@ def contact_information_service
           VAProfile::ContactInformation::Service.new @user
         end
 
-        def person_service
-          VAProfile::Person::Service.new @user
-        end
-
         def raise_timeout_error(elapsed, try)
           Rails.logger.error(
             'mobile syncronous profile update timeout',

modules/mobile/app/workers/mobile/v0/vet360_linking_job.rb

@@ -1,6 +1,19 @@
 # frozen_string_literal: true
 
 require 'sidekiq'
+require 'va_profile/person/service'
+
+# This job is run when a user does not have a vet360_id, which indicates that the user does not have an account on the
+# VAProfile service. The mobile app depends on the VAProfile service for user contact data, and mobile features
+# involving contact data will not work if the user does not have a VAProfile account. This job sends the VAProfile
+# service a request to create a profile for the user so that they will have access to more mobile app features. Those
+# features will probably not be available until the next time the mobile app requests the user data.
+#
+# The log outputs from this job seem to mostly come in batches, which may indicate that a lack of vet360_id can also
+# occur as a result of some other issue, such as an upstream service being down. We should keep an eye on this.
+#
+# Success for this job indicates that a request to create an account for the user was successfully received. It does not
+# indicate that the account was successfully created.
 
 module Mobile
   module V0
@@ -15,16 +28,13 @@ def perform(uuid)
         user = IAMUser.find(uuid) || User.find(uuid)
         raise MissingUserError, uuid unless user
 
-        result = Mobile::V0::Profile::SyncUpdateService.new(user).await_vet360_account_link
-        Rails.logger.info('Mobile Vet360 account linking succeeded for user with uuid',
-                          { user_uuid: uuid, transaction_id: result.transaction_id })
+        result = VAProfile::Person::Service.new(user).init_vet360_id
+        Rails.logger.info('Mobile Vet360 account linking request succeeded for user with uuid',
+                          { user_uuid: uuid, transaction_id: result.transaction.id })
       rescue => e
-        Rails.logger.error('Mobile Vet360 account linking failed for user with uuid',
-                           { user_uuid: uuid })
+        Rails.logger.error('Mobile Vet360 account linking request failed for user with uuid',
+                           { user_uuid: uuid, message: e.message })
         raise e
-      ensure
-        redis = Redis::Namespace.new(REDIS_CONFIG[:mobile_vets360_account_link_lock][:namespace], redis: $redis)
-        redis.del(uuid)
       end
     end
   end

modules/mobile/spec/controllers/application_controller_spec.rb

@@ -101,22 +101,6 @@ def append_info_to_payload(payload)
         end
       end
 
-      context 'with a user without vet360 id' do
-        before { iam_sign_in(FactoryBot.build(:iam_user, :no_vet360_id)) }
-
-        it 'returns returns ok' do
-          get :index
-          expect(response).to have_http_status(:ok)
-        end
-
-        it 'calls async linking job on first call and does not on second after redis lock is in place' do
-          expect(Mobile::V0::Vet360LinkingJob).to receive(:perform_async)
-          get :index
-          expect(Mobile::V0::Vet360LinkingJob).not_to receive(:perform_async)
-          get :index
-        end
-      end
-
       context 'with a user with id theft flag set' do
         before { FactoryBot.create(:iam_user, :id_theft_flag) }
 

modules/mobile/spec/request/user_request_spec.rb

@@ -754,6 +754,42 @@
         )
       end
     end
+
+    describe 'vet360 linking' do
+      context 'when user has a vet360_id' do
+        before { iam_sign_in(FactoryBot.build(:iam_user)) }
+
+        it 'does not enqueue vet360 linking job' do
+          expect(Mobile::V0::Vet360LinkingJob).not_to receive(:perform_async)
+
+          VCR.use_cassette('mobile/payment_information/payment_information') do
+            VCR.use_cassette('mobile/user/get_facilities') do
+              VCR.use_cassette('mobile/va_profile/demographics/demographics') do
+                get '/mobile/v0/user', headers: iam_headers
+              end
+            end
+          end
+          expect(response).to have_http_status(:ok)
+        end
+      end
+
+      context 'when user does not have a vet360_id' do
+        before { iam_sign_in(FactoryBot.build(:iam_user, :no_vet360_id)) }
+
+        it 'enqueues vet360 linking job' do
+          expect(Mobile::V0::Vet360LinkingJob).to receive(:perform_async)
+
+          VCR.use_cassette('mobile/payment_information/payment_information') do
+            VCR.use_cassette('mobile/user/get_facilities_no_ids') do
+              VCR.use_cassette('mobile/va_profile/demographics/demographics') do
+                get '/mobile/v0/user', headers: iam_headers
+              end
+            end
+          end
+          expect(response).to have_http_status(:ok)
+        end
+      end
+    end
   end
 
   describe 'GET /mobile/v0/user/logout' do

modules/mobile/spec/request/v1/user_request_spec.rb

@@ -299,7 +299,7 @@
           VCR.use_cassette('mobile/payment_information/payment_information') do
             VCR.use_cassette('mobile/user/get_facilities') do
               VCR.use_cassette('mobile/va_profile/demographics/demographics') do
-                get '/mobile/v0/user', headers: iam_headers
+                get '/mobile/v1/user', headers: iam_headers
               end
             end
           end
@@ -357,7 +357,7 @@
             VCR.use_cassette('mobile/payment_information/payment_information') do
               VCR.use_cassette('mobile/user/get_facilities_no_ids', match_requests_on: %i[method uri]) do
                 VCR.use_cassette('mobile/va_profile/demographics/demographics') do
-                  get '/mobile/v0/user', headers: iam_headers
+                  get '/mobile/v1/user', headers: iam_headers
                 end
               end
             end
@@ -381,7 +381,7 @@
             VCR.use_cassette('mobile/payment_information/payment_information') do
               VCR.use_cassette('mobile/user/get_facilities_no_ids', match_requests_on: %i[method uri]) do
                 VCR.use_cassette('mobile/va_profile/demographics/demographics') do
-                  get '/mobile/v0/user', headers: iam_headers
+                  get '/mobile/v1/user', headers: iam_headers
                 end
               end
             end
@@ -405,7 +405,7 @@
           VCR.use_cassette('mobile/payment_information/payment_information') do
             VCR.use_cassette('mobile/user/get_facilities', match_requests_on: %i[method uri]) do
               VCR.use_cassette('mobile/va_profile/demographics/demographics') do
-                get '/mobile/v0/user', headers: iam_headers
+                get '/mobile/v1/user', headers: iam_headers
               end
             end
           end
@@ -705,5 +705,41 @@
         )
       end
     end
+
+    describe 'vet360 linking' do
+      context 'when user has a vet360_id' do
+        before { iam_sign_in(FactoryBot.build(:iam_user)) }
+
+        it 'does not enqueue vet360 linking job' do
+          expect(Mobile::V0::Vet360LinkingJob).not_to receive(:perform_async)
+
+          VCR.use_cassette('mobile/payment_information/payment_information') do
+            VCR.use_cassette('mobile/user/get_facilities') do
+              VCR.use_cassette('mobile/va_profile/demographics/demographics') do
+                get '/mobile/v1/user', headers: iam_headers
+              end
+            end
+          end
+          expect(response).to have_http_status(:ok)
+        end
+      end
+
+      context 'when user does not have a vet360_id' do
+        before { iam_sign_in(FactoryBot.build(:iam_user, :no_vet360_id)) }
+
+        it 'enqueues vet360 linking job' do
+          expect(Mobile::V0::Vet360LinkingJob).to receive(:perform_async)
+
+          VCR.use_cassette('mobile/payment_information/payment_information') do
+            VCR.use_cassette('mobile/user/get_facilities_no_ids') do
+              VCR.use_cassette('mobile/va_profile/demographics/demographics') do
+                get '/mobile/v1/user', headers: iam_headers
+              end
+            end
+          end
+          expect(response).to have_http_status(:ok)
+        end
+      end
+    end
   end
 end

modules/mobile/spec/workers/vet360_linking_spec.rb

@@ -6,53 +6,31 @@
 RSpec.describe Mobile::V0::Vet360LinkingJob, type: :job do
   let(:user) { create(:user, :loa3) }
 
-  context 'when linking succeeds' do
-    it 'logs the completed transaction id that linked an account with vet360' do
-      VCR.use_cassette('mobile/profile/init_vet360_id_status_complete') do
-        VCR.use_cassette('mobile/profile/init_vet360_id_success') do
-          allow(Rails.logger).to receive(:info).with(
-            'mobile syncronous profile update complete',
-            { transaction_id: 'd8951c96-5b8c-42ea-9fbe-e656941b7236' }
-          )
-          expect(Rails.logger).to receive(:info).with(
-            'Mobile Vet360 account linking succeeded for user with uuid',
-            { user_uuid: user.uuid, transaction_id: 'd8951c96-5b8c-42ea-9fbe-e656941b7236' }
-          )
-          subject.perform(user.uuid)
-        end
+  context 'when linking request is successfully made' do
+    it 'logs the user id and transaction id that linked an account with vet360' do
+      VCR.use_cassette('mobile/profile/init_vet360_id_success') do
+        expect(Rails.logger).to receive(:info).with(
+          'Mobile Vet360 account linking request succeeded for user with uuid',
+          { user_uuid: user.uuid, transaction_id: 'd8951c96-5b8c-42ea-9fbe-e656941b7236' }
+        )
+        subject.perform(user.uuid)
       end
     end
   end
 
-  context 'when linking fails' do
-    it 'logs the failure with the user uuid' do
+  context 'when linking request fails' do
+    it 'logs the user uuid and error message and raises an error' do
       VCR.use_cassette('mobile/profile/init_vet360_id_status_400') do
         expect(Rails.logger).to receive(:error).with(
-          'Mobile Vet360 account linking failed for user with uuid', { user_uuid: user.uuid }
+          'Mobile Vet360 account linking request failed for user with uuid',
+          {
+            user_uuid: user.uuid,
+            message: 'BackendServiceException: {:source=>"VAProfile::Person::Service", :code=>"VET360_PERS101"}'
+          }
         )
-        expect { subject.perform(user.uuid) }.to raise_error(Common::Exceptions::BackendServiceException)
-      end
-    end
-  end
-
-  context 'with IAM user' do
-    let(:user) { FactoryBot.build(:iam_user, :no_vet360_id) }
-
-    before { iam_sign_in(FactoryBot.build(:iam_user, :no_vet360_id)) }
-
-    it 'works as expected' do
-      VCR.use_cassette('mobile/profile/init_vet360_id_status_complete') do
-        VCR.use_cassette('mobile/profile/init_vet360_id_success') do
-          allow(Rails.logger).to receive(:info).with(
-            'mobile syncronous profile update complete',
-            { transaction_id: 'd8951c96-5b8c-42ea-9fbe-e656941b7236' }
-          )
-          expect(Rails.logger).to receive(:info).with(
-            'Mobile Vet360 account linking succeeded for user with uuid',
-            { user_uuid: user.uuid, transaction_id: 'd8951c96-5b8c-42ea-9fbe-e656941b7236' }
-          )
+        expect do
           subject.perform(user.uuid)
-        end
+        end.to raise_error(Common::Exceptions::BackendServiceException)
       end
     end
   end