Skip to content

Conversation

PhilipDeFraties
Copy link
Contributor

@PhilipDeFraties PhilipDeFraties commented Oct 15, 2025

Summary

  • *This work is behind a feature toggle (flipper): NO

This pull request enhances the VAOS referral details API to include appointment data directly within the referral response. It introduces a new service method to fetch active appointments for a referral, prioritizing EPS data and falling back to VAOS if necessary. The changes also refactor the data model and serializer to support this new appointments structure, and update tests accordingly.

IMPORTANT NOTE
It was decided to check for active appointments with the given referral number (not in draft or cancelled state) in EPS first, if no appointment is found then VAOS is checked for non-cancelled appointments. If a cancelled appointment is found in EPS and an active appointment is found in VAOS then the discrepancy is logged to rails logger but no appointment id will be returned. Below is a list of potential returns for the appointments attribute being added to the details response payload object:

# Possible return values for VAOS::V2::AppointmentsService#get_active_appointments_for_referral(referral_number)

# Success with EPS active appointments
{ system: 'EPS', data: [{ id: 'eps-123', ... }] }

# Success with VAOS active appointments
{ system: 'VAOS', data: [{ id: 'vaos-456', ... }] }

# Success but EPS has only cancelled appointments (no active in either system)
{ system: 'EPS', data: [] }

# Success but neither EPS nor VAOS have any appointments
{ system: nil, data: [] }

# Discrepancy: EPS has cancelled but VAOS has active appointment (still returns empty)
{ system: 'EPS', data: [], errors: { 'Appointment status discrepancy' => 'EPS has cancelled but VAOS has active appointment' } }

# EPS service failure
{ system: 'EPS', data: [], errors: { 'Failure to fetch EPS appointments' => 'Eps::ServiceException' } }

# VAOS service failure
{ system: 'VAOS', data: [], errors: { 'Failure to fetch VAOS appointments' => 'Common::Exceptions::BackendServiceException' } }

Note the order of the appointment id objects in the data array is based on appointment start date-time with most recent at the end of the list, those with no start date-time will be ordered first (so index 0)

Related issue(s)

department-of-veterans-affairs/va.gov-team#118645

See this comment for explanation of dedup methodology:
department-of-veterans-affairs/va.gov-team#118645 (comment)

Testing done

  • New code is covered by unit tests

Screenshots

Note: Optional

What areas of the site does it impact?

(Describe what parts of the site are impacted andifcode touched other areas)

Acceptance criteria

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No error nor warning in the console.
  • Events are being sent to the appropriate logging solution
  • Documentation has been updated (link to documentation)
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Feature/bug has a monitor built into Datadog (if applicable)
  • If app impacted requires authentication, did you login to a local build and verify all authenticated routes work as expected
  • I added a screenshot of the developed feature

Requested Feedback

@PhilipDeFraties PhilipDeFraties force-pushed the 118645-community-care-appts-return-existing-appt-id branch from 930523f to 5d88096 Compare October 16, 2025 00:30
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds referral-aware appointment lookups and exposes active (non-cancelled/non-draft) appointment IDs for a referral. Replaces the previous has_appointments boolean with a richer appointments payload and introduces EPS filtering by referralNumber.

  • Added get_active_appointments_for_referral plus controller integration to include appointments summary in referral responses
  • Replaced has_appointments boolean with appointments attribute across model, serializer, tests, and factories
  • Enhanced EPS AppointmentService to support referralNumber query param; updated/added VCR cassettes and specs

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
appointments_service_spec.rb Updated specs to use new fixtures and added tests for active appointment filtering/sorting.
appointments_spec.rb Adjusted request specs to new appointments behavior and fixtures.
referrals_spec.rb Stubs new active appointments lookup.
referral_detail_spec.rb Updated model tests for new appointments attribute replacing has_appointments.
referrals_controller_spec.rb Added tests ensuring appointments payload included in referral responses.
appointments_service.rb Added referral_appointment_already_exists? rewrite and new get_active_appointments_for_referral logic.
eps/appointment_service.rb Added referralNumber filtering in EPS requests.
referral_detail_serializer.rb Swapped has_appointments for appointments attribute in serialization.
referral_detail.rb Model updated to track appointments instead of has_appointments.
referrals_controller.rb Injects active appointment data into referral response.
Various VCR cassettes New/modified cassettes for mixed statuses, referral filtering, empty results, and updated EPS behavior.
Factories & other specs Removed has_appointments usage and aligned with new appointments structure.

@PhilipDeFraties PhilipDeFraties force-pushed the 118645-community-care-appts-return-existing-appt-id branch from 8f32142 to 875ddb6 Compare October 16, 2025 18:40
@PhilipDeFraties PhilipDeFraties force-pushed the 118645-community-care-appts-return-existing-appt-id branch from e1a1f03 to 38bd26d Compare October 17, 2025 17:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.

@PhilipDeFraties PhilipDeFraties force-pushed the 118645-community-care-appts-return-existing-appt-id branch from 32237b7 to ccbf1e3 Compare October 17, 2025 19:09
end

eps_appointments = eps_appointments_service.get_appointments
eps_appointments = eps_appointments_service.get_appointments(referral_number: referral_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a "might as well while I'm at it" change, added the optional referral number to the service method request params and it is a quick win to implement for this method, which is used in the draft appointment creation step, to prevent over-fetching.

appointments.any? do |appt|
appt[:referral] && appt[:referral][:referral_number] == referral_id
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method no longer needed now that the fetch scopes by referral number

@PhilipDeFraties PhilipDeFraties force-pushed the 118645-community-care-appts-return-existing-appt-id branch from 09fcc9c to 2ef1212 Compare October 17, 2025 19:15
def get_appointments(referral_number: nil)
params = { patientId: patient_id }
params[:referralNumber] = referral_number if referral_number.present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now scopes by referral number if present in method call

Screenshot 2025-10-17 at 1 47 40 PM

@PhilipDeFraties PhilipDeFraties force-pushed the 118645-community-care-appts-return-existing-appt-id branch from e3d25ec to 7904951 Compare October 21, 2025 22:39
@PhilipDeFraties PhilipDeFraties changed the title 118645 return ids of existing appointments for referral 118645 append ids of active appts to referral details Oct 21, 2025
@PhilipDeFraties PhilipDeFraties force-pushed the 118645-community-care-appts-return-existing-appt-id branch from bd7d7de to 58045e1 Compare October 22, 2025 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant