Skip to content

Commit

Permalink
5198 handle partial appointments response (#12438)
Browse files Browse the repository at this point in the history
* passed failures error to controller

* updated meta to include errors if avail

* crested cassette for partial errors

* updated cassette

* added test for partial errors in spec

* updated api docs with example for partial appointments

* rerendered docs

* added statsd logging for partials

* fixed controller

* v2 specs fix

* updated partial error cassette

* fixed appointments cache and corresponding specs

* updated appointments v2 specs with count

* added description

* added multi_status

* update partial error method

* changed precaching based on whether failures are present

* fixed pre_cache_appointments job

* re-rendered docs

* fixed comment

* re-rendered docs
  • Loading branch information
cadibemma authored Apr 25, 2023
1 parent f1054bc commit badd7f1
Show file tree
Hide file tree
Showing 11 changed files with 3,170 additions and 1,172 deletions.
69 changes: 49 additions & 20 deletions modules/mobile/app/controllers/mobile/v0/appointments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,15 @@ class AppointmentsController < ApplicationController
after_action :clear_appointments_cache, only: %i[cancel create]

def index
use_cache = params[:useCache] || true
start_date = params[:startDate] || appointments_cache_interface.latest_allowable_cache_start_date
end_date = params[:endDate] || appointments_cache_interface.earliest_allowable_cache_end_date
reverse_sort = !(params[:sort] =~ /-startDateUtc/).nil?

validated_params = Mobile::V0::Contracts::Appointments.new.call(
start_date:,
end_date:,
page_number: params.dig(:page, :number),
page_size: params.dig(:page, :size),
use_cache:,
reverse_sort:,
included: params[:included],
include: params[:include]
)

appointments = fetch_appointments(validated_params)
validated_params = validated_index_params
appointments, failures = fetch_appointments(validated_params)
appointments = filter_by_date_range(appointments, validated_params)
partial_errors = partial_errors(failures)
status = get_response_status(failures)
page_appointments, page_meta_data = paginate(appointments, validated_params)
page_meta_data[:meta] = page_meta_data[:meta].merge(partial_errors) unless partial_errors.nil?

render json: Mobile::V0::AppointmentSerializer.new(page_appointments, page_meta_data)
render json: Mobile::V0::AppointmentSerializer.new(page_appointments, page_meta_data), status:
end

def cancel
Expand All @@ -53,6 +41,24 @@ def create

private

def validated_index_params
use_cache = params[:useCache] || true
start_date = params[:startDate] || appointments_cache_interface.latest_allowable_cache_start_date
end_date = params[:endDate] || appointments_cache_interface.earliest_allowable_cache_end_date
reverse_sort = !(params[:sort] =~ /-startDateUtc/).nil?

Mobile::V0::Contracts::Appointments.new.call(
start_date:,
end_date:,
page_number: params.dig(:page, :number),
page_size: params.dig(:page, :size),
use_cache:,
reverse_sort:,
included: params[:included],
include: params[:include]
)
end

def appointments_service
VAOS::V2::AppointmentsService.new(@current_user)
end
Expand All @@ -66,15 +72,38 @@ def clear_appointments_cache
end

def fetch_appointments(validated_params)
appointments = appointments_cache_interface.fetch_appointments(
appointments, failures = appointments_cache_interface.fetch_appointments(
user: @current_user, start_date: validated_params[:start_date], end_date: validated_params[:end_date],
fetch_cache: validated_params[:use_cache]
)

appointments.filter! { |appt| appt.is_pending == false } unless include_pending?(validated_params)
appointments.reverse! if validated_params[:reverse_sort]

appointments
[appointments, failures]
end

# Data is aggregated from multiple servers and if any of the servers doesnt respond, we receive failures
# The mobile app shows the user a banner when there are partial appointment errors.
# 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?
Rails.logger.info('Appointments has partial errors: ', failures)

{
errors: [{ source: 'VA Service' }]
}
end
end

def get_response_status(failures)
case failures.size
when 0
:ok
else
:multi_status
end
end

def filter_by_date_range(appointments, validated_params)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def initialize
@now = DateTime.now.utc
end

def fetch_appointments(user:, start_date: nil, end_date: nil, fetch_cache: true)
def fetch_appointments(user:, start_date: nil, end_date: nil, fetch_cache: true, cache_on_failures: true)
appointments = nil
search_start_date = [start_date, latest_allowable_cache_start_date].compact.min
search_end_date = [end_date, earliest_allowable_cache_end_date].compact.max
Expand All @@ -15,15 +15,16 @@ def fetch_appointments(user:, start_date: nil, end_date: nil, fetch_cache: true)
appointments = Mobile::V0::Appointment.get_cached(user)
if appointments
Rails.logger.info('mobile appointments cache fetch', user_uuid: user.uuid)
return appointments
return [appointments, nil]
end
end

appointments = fetch_from_external_service(user, search_start_date, search_end_date)
Mobile::V0::Appointment.set_cached(user, appointments)
appointments, failures = fetch_from_external_service(user, search_start_date, search_end_date)

Mobile::V0::Appointment.set_cached(user, appointments) if cache_on_failures == true || failures.blank?

Rails.logger.info('mobile appointments service fetch', user_uuid: user.uuid)
appointments
[appointments, failures]
end

# the mobile client can request appointments for as far back as the beginning of last year.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def get_appointments(start_date:, end_date:, include_pending:, pagination_params

appointments = vaos_v2_to_v0_appointment_adapter.parse(appointments)

appointments.sort_by(&:start_date_utc)
[appointments.sort_by(&:start_date_utc), response[:meta][:failures]]
end

private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,14 @@ def perform(uuid)
user = IAMUser.find(uuid) || User.find(uuid)
raise MissingUserError, uuid unless user

Mobile::AppointmentsCacheInterface.new.fetch_appointments(user:, fetch_cache: false)

Rails.logger.info('mobile appointments pre-cache success', user_uuid: uuid)
_, failures = Mobile::AppointmentsCacheInterface.new.fetch_appointments(user:, fetch_cache: false,
cache_on_failures: false)
message = if failures.present?
'mobile appointments pre-cache fails with partial appointments present'
else
'mobile appointments pre-cache success'
end
Rails.logger.info(message, user_uuid: uuid)
end
end
end
Expand Down
43 changes: 43 additions & 0 deletions modules/mobile/docs/examples/appointments/partial_appointments.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
data:
- type: appointment
id: "23fe358d-6e82-4541-804c-ce7562ba28f4"
attributes:
appointmentType: va
cancelId: MzA4OzIwMjAxMTAzLjA5MDAwMDs0NDI7Q0hZIFBDIEtJTFBBVFJJQ0s=
comment: Please arrive 20 minutes before the start of your appointment
healthcareProvider: null
healthcareService: Blind Rehabilitation Center
location:
name: VA Long Beach Healthcare System
address:
street: 5901 East 7th Street, Building 166
city: Long Beach
state: CA
zipCode: "90822"
lat: 33.7700504
long: -118.1937395
phone:
areaCode: "562"
number: "826-8000"
extension: "5696"
url: null
code: null
minutesDuration: 60
phoneOnly: false
startDateLocal: 2019-04-20T14:15:00.000-04:00
startDateUtc: 2019-04-20T18:15:00.000Z
status: CANCELLED
statusDetail: CANCELLED BY PATIENT
timeZone: America/Los_Angeles
vetextId: 308;20210106.140000
reason: "Follow-up/Routine: Reason 1"
is_covid_vaccine: false
is_pending: false
proposed_times: null
type_of_care: null
patient_phone_number: null
patient_email: null
best_time_to_call: null
friendly_location_name: null
meta:
errors: [{source: "VA Service"}]
4,032 changes: 2,893 additions & 1,139 deletions modules/mobile/docs/index.html

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions modules/mobile/docs/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,10 @@ paths:
VA Appointment Request:
value:
$ref: ./examples/appointments/va_appointment_request.yml
Partial Appointments:
description: For cases when 1 or more errors are returned from upstream, an error message with source `VA Service` is added within the meta
value:
$ref: ./examples/appointments/partial_appointments.yml
schema:
$ref: ./schemas/Appointments.yml
description: OK
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# setting cache to empty array is sufficient for testing the "cache is set" behavior
let(:cached_data) { [] }
let(:mocked_appointments) { %i[appt1 appt2] }
let(:mocked_proxy_response) { [mocked_appointments, nil] }

def set_cache
Mobile::V0::Appointment.set_cached(user, cached_data)
Expand Down Expand Up @@ -62,23 +63,44 @@ def set_cache

it 'sets cache when fetching fresh data from upstream' do
allow_any_instance_of(Mobile::V2::Appointments::Proxy).to \
receive(:get_appointments).and_return(mocked_appointments)
receive(:get_appointments).and_return(mocked_proxy_response)
expect(Mobile::V0::Appointment).to receive(:set_cached).with(user, mocked_appointments)
subject.fetch_appointments(user:, fetch_cache: true)
end

it 'sets cache when failures are present and cache_on_failure is true' do
allow_any_instance_of(Mobile::V2::Appointments::Proxy).to \
receive(:get_appointments).and_return([mocked_appointments, 'error'])
expect(Mobile::V0::Appointment).to receive(:set_cached)
subject.fetch_appointments(user:, cache_on_failures: true)
end

it 'does not set cache when failures are present and cache_on_failure is false' do
allow_any_instance_of(Mobile::V2::Appointments::Proxy).to \
receive(:get_appointments).and_return([mocked_appointments, 'error'])
expect(Mobile::V0::Appointment).not_to receive(:set_cached)
subject.fetch_appointments(user:, cache_on_failures: false)
end

it 'set cache when failures are not present and cache_on_failure is false' do
allow_any_instance_of(Mobile::V2::Appointments::Proxy).to \
receive(:get_appointments).and_return([mocked_appointments, nil])
expect(Mobile::V0::Appointment).to receive(:set_cached)
subject.fetch_appointments(user:, cache_on_failures: false)
end
end

it 'returns data found in the cache when cache is set and fetch_cache is true' do
set_cache
expect(
subject.fetch_appointments(user:, fetch_cache: true)
).to eq(cached_data)
).to eq([cached_data, nil])
end

it 'returns appointments from the V2 server when cache is not set' do
expect_any_instance_of(Mobile::V2::Appointments::Proxy).to \
receive(:get_appointments).and_return(mocked_appointments)
expect(subject.fetch_appointments(user:)).to eq(mocked_appointments)
receive(:get_appointments).and_return(mocked_proxy_response)
expect(subject.fetch_appointments(user:)).to eq(mocked_proxy_response)
end

it 'uses default start and end dates when not provided' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,21 @@
# removed vetextId, typeOfCare, and friendlyLocationName due to change in behavior decided for v2 adaption to v0
expect(response_v2).to eq(response_v0)
end

it 'has access and returned va appointments having partial errors' do
VCR.use_cassette('appointments/VAOS_v2/get_appointment_200_partial_error',
match_requests_on: %i[method uri]) do
get '/mobile/v0/appointments', headers: iam_headers, params:
end

expect(response).to have_http_status(:multi_status)
expect(response.parsed_body['data'].count).to eq(1)
expect(response.parsed_body['meta']).to include(
{
'errors' => [{ 'source' => 'VA Service' }]
}
)
end
end

context 'request all appointments without requests' do
Expand Down
Loading

0 comments on commit badd7f1

Please sign in to comment.