add upstream service failures to mutli status response (#16465)

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

@@ -87,20 +87,19 @@ def fetch_appointments
       # The mobile app does not distinguish between VA and CC errors so we are only indicating that there are errors
       # If we ever want to distinguish be VA and CC errors, it will require coordination with the front-end team
       def partial_errors(failures)
-        if failures.present?
+        if appointment_errors?(failures)
           {
             errors: [{ source: 'VA Service' }]
           }
         end
       end
 
       def get_response_status(failures)
-        case failures&.size
-        when 0, nil
-          :ok
-        else
-          :multi_status
-        end
+        appointment_errors?(failures) ? :multi_status : :ok
+      end
+
+      def appointment_errors?(failures)
+        failures.any? { |failure| failure[:appointment_errors].present? }
       end
 
       def filter_by_date_range(appointments)

modules/mobile/app/services/mobile/v2/appointments/provider_names.rb

@@ -20,10 +20,17 @@ def form_names_from_appointment_practitioners_list(practitioners_list)
           return nil if practitioners_list.blank?
 
           provider_names = []
+          missing_providers = []
           practitioners_list.each do |practitioner|
-            provider_names << find_provider_name(practitioner)
+            name, id = find_provider_name(practitioner)
+            if name
+              provider_names << name
+            else
+              missing_providers << id
+            end
           end
-          provider_names.compact.join(', ').presence
+          provider_names = provider_names.compact.join(', ').presence
+          [provider_names, missing_providers]
         end
 
         private
@@ -41,7 +48,7 @@ def find_provider_name(practitioner)
           name = provider_data&.name&.strip&.presence
           # cache even if it's nil to avoid duplicate requests
           @providers_cache[id] = name
-          name
+          [name, id]
         end
 
         def find_practitioner_name_in_list(practitioner)

modules/mobile/app/services/mobile/v2/appointments/proxy.rb

@@ -24,13 +24,21 @@ def get_appointments(start_date:, end_date:, include_pending:, pagination_params
           filterer = PresentationFilter.new(include_pending:)
           appointments = appointments.keep_if { |appt| filterer.user_facing?(appt) }
 
-          appointments = merge_clinic_facility_address(appointments)
-          appointments = merge_auxiliary_clinic_info(appointments)
-          appointments = merge_provider_names(appointments)
+          appointments, missing_facilities = merge_clinic_facility_address(appointments)
+          appointments, missing_clinics = merge_auxiliary_clinic_info(appointments)
+          appointments, missing_providers = merge_provider_names(appointments)
 
           appointments = vaos_v2_to_v0_appointment_adapter.parse(appointments)
-
-          [appointments.sort_by(&:start_date_utc), response[:meta][:failures]]
+          failures = [
+            { appointment_errors: Array.wrap(response[:meta][:failures]) },
+            { missing_facilities: },
+            { missing_clinics: },
+            { missing_providers: }
+          ]
+          failures.reject! { |failure| failure.values.first&.empty? }
+          Rails.logger.info('Mobile Appointment Partial Error', errors: failures) if failures.any?
+
+          [appointments.sort_by(&:start_date_utc), failures]
         end
 
         private
@@ -43,12 +51,18 @@ def merge_clinic_facility_address(appointments)
             cached_facilities[facility_id] = appointments_helper.get_facility(facility_id)
           end
 
+          missing_facilities = []
+
           appointments.each do |appt|
             facility_id = appt[:location_id]
             next unless facility_id
 
             appt[:location] = cached_facilities[facility_id]
+
+            missing_facilities << facility_id unless cached_facilities[facility_id]
           end
+
+          [appointments, missing_facilities]
         end
 
         def merge_auxiliary_clinic_info(appointments)
@@ -59,6 +73,8 @@ def merge_auxiliary_clinic_info(appointments)
             cached_clinics[clinic_id] = appointments_helper.get_clinic(location_id, clinic_id)
           end
 
+          missing_clinics = []
+
           appointments.each do |appt|
             clinic_id = appt[:clinic]
             next unless clinic_id
@@ -68,16 +84,26 @@ def merge_auxiliary_clinic_info(appointments)
 
             physical_location = cached_clinics.dig(clinic_id, :physical_location)
             appt[:physical_location] = physical_location
+
+            missing_clinics << clinic_id unless cached_clinics[clinic_id]
           end
+          [appointments, missing_clinics]
         end
 
         def merge_provider_names(appointments)
           provider_names_proxy = ProviderNames.new(@user)
+          missing_providers = []
           appointments.each do |appt|
             practitioners_list = appt[:practitioners]
-            names = provider_names_proxy.form_names_from_appointment_practitioners_list(practitioners_list)
+            next unless practitioners_list
+
+            names, appointment_missing_providers =
+              provider_names_proxy.form_names_from_appointment_practitioners_list(practitioners_list)
             appt[:healthcare_provider] = names
+            missing_providers.concat(appointment_missing_providers) unless names
           end
+
+          [appointments, missing_providers]
         end
 
         def appointments_helper

modules/mobile/spec/request/appointments_vaos_v2_list_request_spec.rb

@@ -13,6 +13,7 @@
     before do
       Flipper.enable('va_online_scheduling')
       allow_any_instance_of(VAOS::UserService).to receive(:session).and_return('stubbed_token')
+      allow(Rails.logger).to receive(:info)
       Timecop.freeze(Time.zone.parse('2022-01-01T19:25:00Z'))
     end
 
@@ -66,6 +67,7 @@
             end
           end
         end
+        expect(response).to have_http_status(:ok)
         location = response.parsed_body.dig('data', 0, 'attributes', 'location')
         physical_location = response.parsed_body.dig('data', 0, 'attributes', 'physicalLocation')
         expect(response.body).to match_json_schema('VAOS_v2_appointments')
@@ -84,6 +86,14 @@
                                  'url' => nil,
                                  'code' => nil })
         expect(physical_location).to eq('MTZ OPC, LAB')
+        expect(response.parsed_body['meta']).to eq({
+                                                     'pagination' => { 'currentPage' => 1,
+                                                                       'perPage' => 10,
+                                                                       'totalPages' => 1,
+                                                                       'totalEntries' => 1 },
+                                                     'upcomingAppointmentsCount' => 0,
+                                                     'upcomingDaysLimit' => 7
+                                                   })
       end
     end
 
@@ -97,6 +107,9 @@
             end
           end
         end
+        expect(response).to have_http_status(:ok)
+        expect(Rails.logger).to have_received(:info).with('Mobile Appointment Partial Error',
+                                                          errors: [{ missing_facilities: ['983'] }])
         expect(response.body).to match_json_schema('VAOS_v2_appointments')
         location = response.parsed_body.dig('data', 0, 'attributes', 'location')
         expect(location).to eq({ 'id' => nil,
@@ -155,6 +168,10 @@
             end
           end
         end
+        expect(response).to have_http_status(:ok)
+        expect(Rails.logger).to have_received(:info).with('Mobile Appointment Partial Error',
+                                                          errors: [{ missing_facilities: ['999AA'] },
+                                                                   { missing_clinics: ['999'] }])
         expect(response.body).to match_json_schema('VAOS_v2_appointments')
         expect(response.parsed_body.dig('data', 0, 'attributes', 'healthcareService')).to be_nil
       end
@@ -187,6 +204,14 @@
         end
 
         expect(response).to have_http_status(:multi_status)
+        appointment_error = { errors: [{ appointment_errors: [{ system: 'the system',
+                                                                id: 'id-string',
+                                                                status: 'status-string',
+                                                                code: 0,
+                                                                trace_id: 'traceId-string',
+                                                                message: 'msg-string',
+                                                                detail: 'detail-string' }] }] }
+        expect(Rails.logger).to have_received(:info).with('Mobile Appointment Partial Error', appointment_error)
         expect(response.parsed_body['data'].count).to eq(1)
         expect(response.parsed_body['meta']).to include(
           {
@@ -287,6 +312,8 @@ def fetch_appointments
           end
           expect(response).to have_http_status(:ok)
           expect(appointment['attributes']['healthcareProvider']).to be_nil
+          expect(Rails.logger).to have_received(:info).with('Mobile Appointment Partial Error',
+                                                            errors: [{ missing_providers: ['1407938061'] }])
         end
 
         it 'falls back to nil when provider service returns 500' do
@@ -304,6 +331,8 @@ def fetch_appointments
           end
           expect(response).to have_http_status(:ok)
           expect(appointment['attributes']['healthcareProvider']).to be_nil
+          expect(Rails.logger).to have_received(:info).with('Mobile Appointment Partial Error',
+                                                            errors: [{ missing_providers: ['1407938061'] }])
         end
       end
 

modules/mobile/spec/services/v2/appointments/provider_names_spec.rb

@@ -69,27 +69,31 @@
     end
 
     it 'returns names as first_name last_name' do
-      name = subject.form_names_from_appointment_practitioners_list(single_practitioner_with_name)
+      name, missing_providers = subject.form_names_from_appointment_practitioners_list(single_practitioner_with_name)
       expect(name).to eq('CAROLYN KNIEFEL')
+      expect(missing_providers).to eq([])
     end
 
     it 'handles partial names predictably' do
       partial_name_data = single_practitioner_with_name.first
       partial_name_data[:name].delete(:given)
-      name = subject.form_names_from_appointment_practitioners_list([partial_name_data])
+      name, missing_providers = subject.form_names_from_appointment_practitioners_list([partial_name_data])
       expect(name).to eq('KNIEFEL')
+      expect(missing_providers).to eq([])
     end
 
     it 'aggregates multiple names as a comma separated list' do
-      name = subject.form_names_from_appointment_practitioners_list(multiple_practioners_with_names)
+      name, missing_providers = subject.form_names_from_appointment_practitioners_list(multiple_practioners_with_names)
       expect(name).to eq('CAROLYN KNIEFEL, MARCY NADEAU')
+      expect(missing_providers).to eq([])
     end
 
     it 'forms names from upstream when an identifier is found without a name' do
       allow_any_instance_of(VAOS::V2::MobilePPMSService).to\
         receive(:get_provider).with('1407938061').and_return(provider_response)
-      name = subject.form_names_from_appointment_practitioners_list(practitioner_without_name)
+      name, missing_providers = subject.form_names_from_appointment_practitioners_list(practitioner_without_name)
       expect(name).to eq('DEHGHAN, AMIR')
+      expect(missing_providers).to eq([])
     end
 
     it 'can request multiple upstream providers' do
@@ -107,8 +111,10 @@
         receive(:get_provider).with('1407938061').and_return(provider_response)
       allow_any_instance_of(VAOS::V2::MobilePPMSService).to\
         receive(:get_provider).with('1407938062').and_return(second_provider_response)
-      name = subject.form_names_from_appointment_practitioners_list(multiple_practitioners_without_names)
+      name, missing_providers =
+        subject.form_names_from_appointment_practitioners_list(multiple_practitioners_without_names)
       expect(name).to eq('DEHGHAN, AMIR, J. Jones')
+      expect(missing_providers).to eq([])
     end
 
     it 'only requests an upstream provider once' do
@@ -121,17 +127,19 @@
     it 'returns nil when the ppms service raises an error' do
       allow_any_instance_of(VAOS::V2::MobilePPMSService).to\
         receive(:get_provider).and_raise(Common::Exceptions::BackendServiceException)
-      name = subject.form_names_from_appointment_practitioners_list(practitioner_without_name)
+      name, missing_providers = subject.form_names_from_appointment_practitioners_list(practitioner_without_name)
       expect(name).to be_nil
+      expect(missing_providers).to eq(['1407938061'])
     end
 
     it 'returns nil if the returned provider does not match the expected structure' do
       nameless_provider = OpenStruct.new
       allow_any_instance_of(VAOS::V2::MobilePPMSService).to\
         receive(:get_provider).with('1407938061').and_return(nameless_provider)
 
-      name = subject.form_names_from_appointment_practitioners_list(practitioner_without_name)
+      name, missing_providers = subject.form_names_from_appointment_practitioners_list(practitioner_without_name)
       expect(name).to be_nil
+      expect(missing_providers).to eq(['1407938061'])
     end
   end
 end

modules/mobile/spec/support/schemas/VAOS_v2_appointments.json

@@ -235,7 +235,7 @@
       ],
       "properties": {
         "errors": {
-          "type": "null"
+          "type": ["array","null"]
         },
         "pagination": {
           "type": "object",