update va_profile, return null for mvi down, 'not found' for no record (

#438)

* added mvi status to user va_profile

app/models/decorators/mvi_user_decorator.rb

@@ -3,6 +3,12 @@
 require 'mvi/service_factory'
 
 class Decorators::MviUserDecorator
+  MVI_RESPONSE_STATUS = {
+    ok: 'OK',
+    not_found: 'NOT_FOUND',
+    server_error: 'SERVER_ERROR'
+  }.freeze
+
   def initialize(user)
     @user = user
     @mvi_service = MVI::ServiceFactory.get_service(mock_service: ENV['MOCK_MVI_SERVICE'])
@@ -12,28 +18,19 @@ def create
     raise Common::Exceptions::ValidationErrors, @user unless @user.valid?
     message = create_message
     response = @mvi_service.find_candidate(message)
-    # in most cases (other than ids) the user attributes from the identity provider are more up-to-date
-    # but stashing the MVI data if it's needed for confirmation
-    @user.attributes = {
-      edipi: select_source_id(response[:edipi]),
-      icn: select_source_id(response[:icn]),
-      participant_id: select_source_id(response[:vba_corp_id]),
-      mhv_id: select_source_id(response[:mhv_id]),
-      mvi: response
-    }
-    @user
+    user_with_mvi_attributes(response)
   rescue MVI::RecordNotFound
     # TODO(AJD): add metric
     Rails.logger.error "MVI record not found for user: #{@user.uuid}"
-    @user
+    user_status_not_found
   rescue MVI::HTTPError => e
     # TODO(AJD): add metric
-    Rails.logger.error "MVI returned HTTP error code: #{e.code} for user: #{@user.uuid}"
-    raise Common::Exceptions::InternalServerError, e
+    Rails.logger.error "MVI HTTP error code: #{e.code} for user: #{@user.uuid}"
+    user_status_mvi_error
   rescue MVI::ServiceError => e
     # TODO(AJD): add metric
-    Rails.logger.error "Error retrieving MVI data for user: #{@user.uuid}"
-    raise Common::Exceptions::InternalServerError, e
+    Rails.logger.error "MVI service error: #{e.message} for user: #{@user.uuid}"
+    user_status_mvi_error
   end
 
   private
@@ -54,4 +51,28 @@ def select_source_id(correlation_id)
     return nil unless correlation_id
     correlation_id.split('^').first
   end
+
+  def user_with_mvi_attributes(response)
+    # in most cases (other than ids) the user attributes from the identity provider are more up-to-date
+    # but stashing the MVI data if it's needed for confirmation
+    response[:status] = MVI_RESPONSE_STATUS[:ok]
+    @user.attributes = {
+      edipi: select_source_id(response[:edipi]),
+      icn: select_source_id(response[:icn]),
+      participant_id: select_source_id(response[:vba_corp_id]),
+      mhv_id: select_source_id(response[:mhv_id]),
+      mvi: response
+    }
+    @user
+  end
+
+  def user_status_not_found
+    @user.mvi = { status: MVI_RESPONSE_STATUS[:not_found] }
+    @user
+  end
+
+  def user_status_mvi_error
+    @user.mvi = { status: MVI_RESPONSE_STATUS[:server_error] }
+    @user
+  end
 end

app/serializers/user_serializer.rb

@@ -10,26 +10,28 @@ def id
 
   def profile
     {
-      email:          object.email,
-      first_name:     object.first_name,
-      middle_name:    object.middle_name,
-      last_name:      object.last_name,
-      birth_date:     object.birth_date,
-      gender:         object.gender,
-      zip:            object.zip,
+      email: object.email,
+      first_name: object.first_name,
+      middle_name: object.middle_name,
+      last_name: object.last_name,
+      birth_date: object.birth_date,
+      gender: object.gender,
+      zip: object.zip,
       last_signed_in: object.last_signed_in,
-      loa:            object.loa
+      loa: object.loa
     }
   end
 
   def va_profile
-    return nil if object.mvi.nil?
+    return { status: 'NOT_AUTHORIZED' } if object.mvi.nil?
+    return { status: object.mvi[:status] } unless object.mvi[:status] == 'OK'
     {
-      birth_date:   object.mvi['birth_date'],
-      family_name:  object.mvi['family_name'],
-      gender:       object.mvi['gender'],
-      given_names:  object.mvi['given_names'],
-      status:       object.mvi['status']
+      status: object.mvi[:status],
+      birth_date: object.mvi[:birth_date],
+      family_name: object.mvi[:family_name],
+      gender: object.mvi[:gender],
+      given_names: object.mvi[:given_names],
+      active_status: object.mvi[:active_status]
     }
   end
 

lib/mvi/responses/find_candidate.rb

@@ -38,7 +38,7 @@ def body
         return nil unless patient
         name = parse_name(get_patient_name(patient))
         {
-          status: locate_element(patient, STATUS_XPATH),
+          active_status: locate_element(patient, STATUS_XPATH),
           given_names: name[:given],
           family_name: name[:family],
           gender: locate_element(patient, GENDER_XPATH),

spec/factories/users.rb

@@ -25,6 +25,7 @@
       participant_id '12345678'
       mvi do
         {
+          status: 'OK',
           birth_date: '18090212',
           edipi: '1234^NI^200DOD^USDOD^A',
           vba_corp_id: '12345678^PI^200CORP^USVBA^A',
@@ -34,7 +35,7 @@
           icn: '1000123456V123456^NI^200M^USVHA^P',
           mhv_id: '123456^PI^200MH^USVHA^A',
           ssn: '272111863',
-          status: 'active'
+          active_status: 'active'
         }
       end
     end
@@ -53,6 +54,7 @@
     end
     mvi do
       {
+        status: 'OK',
         birth_date: '18090212',
         edipi: '1234^NI^200DOD^USDOD^A',
         vba_corp_id: '12345678^PI^200CORP^USVBA^A',
@@ -62,7 +64,7 @@
         icn: '1000123456V123456^NI^200M^USVHA^P',
         mhv_id: "#{ENV['MHV_USER_ID']}^PI^200MH^USVHA^A",
         ssn: '272111863',
-        status: 'active'
+        active_status: 'active'
       }
     end
   end
@@ -94,10 +96,11 @@
       end
       mvi do
         {
+          status: 'OK',
           edipi: '1234^NI^200DOD^USDOD^A',
           icn: '1000123456V123456^NI^200M^USVHA^P',
           mhv: '123456^PI^200MHV^USVHA^A',
-          status: 'active',
+          active_status: 'active',
           given_names: %w(george),
           family_name: 'washington',
           gender: 'M',

spec/lib/mvi/responses/find_candidate_spec.rb

@@ -37,7 +37,7 @@
             icn: '1000123456V123456^NI^200M^USVHA^P',
             mhv_id: '123456^PI^200MHV^USVHA^A',
             ssn: '555443333',
-            status: 'active'
+            active_status: 'active'
           )
         end
       end
@@ -55,7 +55,7 @@
             icn: '1000123456V123456^NI^200M^USVHA^P',
             mhv_id: '123456^PI^200MHV^USVHA^A',
             ssn: '555443333',
-            status: 'active'
+            active_status: 'active'
           )
         end
       end
@@ -80,7 +80,7 @@
           given_names: %w(Mitchell),
           icn: '1008714701V416111^NI^200M^USVHA^P',
           ssn: '796122306',
-          status: 'active'
+          active_status: 'active'
         )
       end
     end
@@ -116,7 +116,7 @@
           icn: '1008711094V567547^NI^200M^USVHA^P',
           mhv_id: nil,
           ssn: '796127160',
-          status: 'active',
+          active_status: 'active',
           vba_corp_id: '32318174^PI^200CORP^USVBA^A'
         )
       end

spec/lib/mvi/service_spec.rb

@@ -58,7 +58,7 @@
             icn: '1008714701V416111^NI^200M^USVHA^P',
             mhv_id: nil,
             vba_corp_id: '9100792239^PI^200CORP^USVBA^A',
-            status: 'active',
+            active_status: 'active',
             given_names: %w(Mitchell G),
             family_name: 'Jenkins',
             gender: 'M',

spec/models/decorators/mvi_user_decorator_spec.rb

@@ -17,7 +17,7 @@
         icn: mvi_user.mvi[:icn],
         mhv_id: mvi_user.mvi[:mhv_id],
         ssn: mvi_user.mvi[:ssn],
-        status: mvi_user.mvi[:status]
+        active_status: mvi_user.mvi[:active_status]
       }
     end
 
@@ -38,19 +38,21 @@
           allow_any_instance_of(MVI::Service).to receive(:find_candidate).and_raise(
             MVI::HTTPError.new('MVI HTTP call failed', 500)
           )
-          expect(Rails.logger).to receive(:error).once.with(/MVI returned HTTP error code: 500 for user:/)
-          expect { Decorators::MviUserDecorator.new(user).create }.to raise_error(
-            Common::Exceptions::InternalServerError
-          )
+          expect(Rails.logger).to receive(:error).once.with(/MVI HTTP error code: 500 for user:/)
+          user_result = Decorators::MviUserDecorator.new(user).create
+          expect(user_result).to_not be_nil
+          expect(user_result.attributes).to eq(user.attributes)
+          expect(user_result.mvi).to eq(status: Decorators::MviUserDecorator::MVI_RESPONSE_STATUS[:server_error])
         end
       end
       context 'when a MVI::ServiceError is raised' do
         it 'should log an error message' do
-          allow_any_instance_of(MVI::Service).to receive(:find_candidate).and_raise(MVI::ServiceError)
-          expect(Rails.logger).to receive(:error).once.with(/Error retrieving MVI data for user:/)
-          expect { Decorators::MviUserDecorator.new(user).create }.to raise_error(
-            Common::Exceptions::InternalServerError
-          )
+          allow_any_instance_of(MVI::Service).to receive(:find_candidate).and_raise(MVI::InvalidRequestError)
+          expect(Rails.logger).to receive(:error).once.with(/MVI service error: MVI::InvalidRequestError for user:/)
+          user_result = Decorators::MviUserDecorator.new(user).create
+          expect(user_result).to_not be_nil
+          expect(user_result.attributes).to eq(user.attributes)
+          expect(user_result.mvi).to eq(status: Decorators::MviUserDecorator::MVI_RESPONSE_STATUS[:server_error])
         end
       end
       context 'when MVI::RecordNotFound is raised' do
@@ -64,7 +66,7 @@
           expect(Rails.logger).to receive(:error).once.with(/MVI record not found for user:/)
           user_result = Decorators::MviUserDecorator.new(user).create
           expect(user_result).to_not be_nil
-          expect(user_result.mvi).to be_nil
+          expect(user_result.mvi).to eq(status: Decorators::MviUserDecorator::MVI_RESPONSE_STATUS[:not_found])
         end
       end
     end

spec/serializers/user_serializer_spec.rb

@@ -58,19 +58,19 @@
   context 'inside "va_profile"' do
     context 'when user.mvi is not nil' do
       it 'should include birth_date' do
-        expect(va_profile['birth_date']).to eq(user.mvi['birth_date'])
+        expect(va_profile['birth_date']).to eq(user.mvi[:birth_date])
       end
       it 'should include family_name' do
-        expect(va_profile['family_name']).to eq(user.mvi['family_name'])
+        expect(va_profile['family_name']).to eq(user.mvi[:family_name])
       end
       it 'should include gender' do
-        expect(va_profile['gender']).to eq(user.mvi['gender'])
+        expect(va_profile['gender']).to eq(user.mvi[:gender])
       end
       it 'should include given_names' do
-        expect(va_profile['given_names']).to eq(user.mvi['given_names'])
+        expect(va_profile['given_names']).to eq(user.mvi[:given_names])
       end
       it 'should include status' do
-        expect(va_profile['status']).to eq(user.mvi['status'])
+        expect(va_profile['status']).to eq(user.mvi[:status])
       end
     end
 
@@ -81,7 +81,26 @@
       let(:va_profile) { attributes['va_profile'] }
 
       it 'returns va_profile as null' do
-        expect(va_profile).to be_nil
+        expect(va_profile).to eq(
+          'status' => 'NOT_AUTHORIZED'
+        )
+      end
+    end
+
+    context 'when user.mvi is not found' do
+      let(:user) do
+        build :user, mvi: {
+          status: 'NOT_FOUND'
+        }
+      end
+      let(:data) { JSON.parse(subject)['data'] }
+      let(:attributes) { data['attributes'] }
+      let(:va_profile) { attributes['va_profile'] }
+
+      it 'returns va_profile as null' do
+        expect(va_profile).to eq(
+          'status' => 'NOT_FOUND'
+        )
       end
     end
   end

spec/support/schemas/profile.json

@@ -48,19 +48,25 @@
             "va_profile": {
               "type": ["object", "null"],
               "required": [
-                "birth_date",
-                "family_name",
-                "gender",
-                "given_names",
                 "status"
               ],
-              "properties": {
-                "birth_date": { "type": [ "string", null ] },
-                "family_name": { "type": [ "string", null ] },
-                "gender": { "type": [ "string", null ] },
-                "given_names": { "type": [ "array", null ] },
-                "status": { "type": [ "string", null ]  }
-              }
+              "oneOf": [
+                {
+                  "properties": {
+                    "status": { "type": [ "string" ]  }
+                  }
+                },
+                {
+                  "properties": {
+                    "birth_date": { "type": "string" },
+                    "family_name": { "type": "string" },
+                    "gender": { "type": "string" },
+                    "given_names": { "type": "array" },
+                    "active_status": { "type": "string"  },
+                    "status": { "type": "string"  }
+                  }
+                }
+              ]
             }
           }
         }