-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
VAOS: Migrate MFS Clinics Search Call Over to VAOS Service #12863
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks fine. Per my comment below, would it make sense to add a get_clinics method? Maybe even make a "get_clinics_with_cache" method so that we can get cache optimizations.
Speaking of which, I wasn't aware of the get_clinic_with_cache
method. We aren't using this. So thanks for pointing this out. I'll talk to Jayson about changing our code to use it.
@@ -14,12 +14,12 @@ class MobileFacilityService < VAOS::SessionService | |||
# @return [OpenStruct] An OpenStruct object containing information about the clinic. | |||
# | |||
def get_clinic(station_id:, clinic_id:) | |||
params = {} | |||
params = { clinicIds: clinic_id } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So is it possible to send multiple ids? That could actually be useful to the mobile team.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in VAOS Service it is possible to send multiple clinic ids for a facility. I'll go ahead and submit a GitHub ticket to add a get_clinics method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, AJ! Just let us know. I didn't realize your team only recently added those "with cache" methods and that Corey updated our proxy to use the cache version of the get facility method. Thanks for doing that. I'll try to update us to use the clinic cache as well. That may largely make the "get clinics in bulk" method irrelevant, but it seems like a reasonable thing to have.
* refactor(vaos): update MobileFacilityService to use VAOS Service instead of MFS to get clinic data refactor(vaos): update friendly_name field to use service_name test(vaos): updated VAOS rspec tests work with the changes * test(mobile): update get clinic vcr cassettes due to change from mfs to vaos service endpoints
Summary
We have updated the system for obtaining clinic information from the Mobile Facility Service (MFS) to the VAOS Service. Update the VAOS::V2::MobileFacilityService#get_clinic method to use the "/vaos/v1/locations/<site_id>/clinics" endpoint, which points to the VAOS Service.
Additionally, the assignments for the friendly name in VAOS::V2::AppointmentsController were modified to retrieve
its value from the clinics service name. The VAOS service provides the friendly name in the service name, if
available, otherwise the internal clinic will be returned.
Related issue(s)
department-of-veterans-affairs/va.gov-team#58434
Testing done
I have made adjustments to the RSpec tests and VCR Cassettes to accommodate the recent change.