adding merge functionality for eps to regular appointments #19754

lee-delarm6 · 2024-12-05T21:44:04Z

Summary

This work is behind a param
This change introduces EPS (Electronic Prescribing System) appointment integration into the VAOS (VA Online Scheduling) system.
Implementing EPS appointment normalization and merging functionality
Modifying appointment handling logic to incorporate EPS appointments
Cleaning up some code formatting and rubocop issues
The solution integrates EPS appointments with existing VAOS appointments by normalizing the EPS appointment data structure to match VAOS format and merging them while avoiding duplicates. This unified approach allows for consistent appointment handling across both systems.
Based on the code patterns and feature toggles present, this appears to be a gradual integration approach allowing for controlled rollout and testing.

Testing done

New code is covered by unit tests
New behavior: Both VAOS and EPS appointments are merged and displayed when the EPS include flag is present
Run this function with an without EPS appointments

What areas of the site does it impact?

VAOS (VA Online Scheduling) system, specifically:

Appointment retrieval and display
Configuration settings
Appointment service layer

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

adding merge functionality for eps to regular appointments

rubocop fixes

randomsync

I understand that this is a draft so just providing general feedback. There are other changes that would be needed to make sure the merged appointments are used in the rest of the controller method. Please add tests as well so it's easier to understand the changes and the desired effect.

modules/vaos/app/services/vaos/v2/appointments_service.rb

modules/vaos/app/controllers/vaos/v2/appointments_controller.rb

randomsync · 2024-12-09T23:00:45Z

modules/vaos/app/controllers/vaos/v2/appointments_controller.rb

@@ -21,14 +21,16 @@ class AppointmentsController < VAOS::BaseController
      COMMENT = 'comment'

      def index
-        appointments[:data].each do |appt|
+        merged_appointments = appointments_service.merge_appointments(eps_appointments, appointments)


I'd suggest putting these changes behind a feature flag

@lee-delarm6 did we end up putting these behind a feature flag? do we need to? I'm thinking that when FE adds the code to include eps param, we might still want to control when we merge or not merge the appointments

modules/vaos/app/services/vaos/v2/appointments_service.rb

randomsync · 2024-12-09T23:09:16Z

modules/vaos/app/services/vaos/v2/appointments_service.rb

+        normalized_new = new_appointments[:appointments].map { |appt| normalize_eps_appointment(appt) }
+        basic_data = basic_appointments[:data].is_a?(Array) ? basic_appointments[:data] : [basic_appointments[:data]]
+        existing_ids = basic_data.to_set { |a| a[:referralId] }
+        merged_data = basic_data + normalized_new.reject { |a| existing_ids.include?(a[:referralId]) }


are we deduping by referralId?

Yes, just marking this as a confirmation

Could you please see if the vaos appointment data have referralId & is available under referralId key?

This has been updated, we're using referral_number, a few data points have been changed. We are also using start because referral_number is not purely unique, but it is when combined with a time

modules/vaos/app/controllers/vaos/v2/appointments_controller.rb

revert controller, move logic to service

update test

moved controller logic to service

dropping and fixing a few items with test and removing patient_id

removing unused function

update to master

fixes

more fixes

last fix

maybe last fix

Moved status to serialize step, altered status method slightly

modules/vaos/app/serializers/vaos/v2/eps_appointment.rb

Co-authored-by: Gaurav Gupta <randomsync@users.noreply.github.com>

randomsync

lgtm with some minor feedback

randomsync · 2025-01-28T17:45:11Z

modules/vaos/app/serializers/vaos/v2/eps_appointment.rb

+        referral_details = params[:referral]
+
+        @id = params[:id]&.to_s
+        @status = appointment_details[:status],


suggest removing the comma from here

stevenjcumming · 2025-01-29T15:23:35Z

@lee-delarm6 can you put this PR in a draft state until the lint/test warnings/failures are resolved?

lee-delarm6 · 2025-01-29T15:29:20Z

@lee-delarm6 can you put this PR in a draft state until the lint/test warnings/failures are resolved?

I'm updating to master branch, my tests should still be passing, I believe it's most likely due to not being updated to master

rubocop fix

…artment-of-veterans-affairs/vets-api into 96412_merge_eps_appointments

lee-delarm6 · 2025-01-29T15:34:31Z

@lee-delarm6 can you put this PR in a draft state until the lint/test warnings/failures are resolved?

Fixed! It was mine :D

stevenjcumming · 2025-01-29T16:25:10Z

@lee-delarm6 there is still a failing test

Fixed testing and rubocop errors

lee-delarm6 · 2025-01-29T21:13:10Z

@lee-delarm6 there is still a failing test

It was my test, I fixed it. I totally forgot I updated another file

stevenjcumming

@lee-delarm6 the EpsAppointment class and corresponding spec should be moved under the model/vaos/v2 folder

Sorry I didn't notice that sooner

adding merge functionality for eps to regular appointments

a7c7ab2

adding merge functionality for eps to regular appointments

lee-delarm6 requested review from randomsync and kanchanasuriya December 5, 2024 21:44

github-actions bot added the lint-failure label Dec 5, 2024

va-vfs-bot temporarily deployed to 96412_merge_eps_appointments/main/main December 5, 2024 21:49 Inactive

github-actions bot added the test-failure label Dec 5, 2024

rubocop fixes

7b9caa6

rubocop fixes

github-actions bot removed the lint-failure label Dec 5, 2024

va-vfs-bot temporarily deployed to 96412_merge_eps_appointments/main/main December 5, 2024 22:07 Inactive

randomsync requested changes Dec 9, 2024

View reviewed changes

lee-delarm6 added 5 commits December 12, 2024 11:24

revert controller, move logic to service

f9d0a36

revert controller, move logic to service

update test

0577b6e

update test

Merge branch 'master' into 96412_merge_eps_appointments

6401a1f

moved controller logic to service

a749327

moved controller logic to service

dropping and fixing a few items

36cbafd

dropping and fixing a few items with test and removing patient_id

github-actions bot added the lint-failure label Dec 12, 2024

removing unused function

170d535

removing unused function

github-actions bot added the audit-service-failure label Dec 12, 2024

va-vfs-bot temporarily deployed to 96412_merge_eps_appointments/main/main December 12, 2024 19:32 Inactive

lee-delarm6 added 2 commits December 12, 2024 15:14

update to master

8a362ce

update to master

fixes

c565abd

fixes

github-actions bot removed the audit-service-failure label Dec 12, 2024

lee-delarm6 added 3 commits December 12, 2024 15:20

more fixes

26804f8

more fixes

last fix

7631136

last fix

maybe last fix

fbde9be

maybe last fix

github-actions bot added test-passing test-failure and removed test-failure labels Dec 12, 2024

Moved status to serialize step

e3fc186

Moved status to serialize step, altered status method slightly

github-actions bot added the lint-failure label Jan 27, 2025

va-vfs-bot had a problem deploying to 96412_merge_eps_appointments/main/main January 27, 2025 17:25 Error

github-actions bot added test-failure and removed test-passing labels Jan 27, 2025

randomsync reviewed Jan 27, 2025

View reviewed changes

modules/vaos/app/serializers/vaos/v2/eps_appointment.rb Outdated Show resolved Hide resolved

Update modules/vaos/app/serializers/vaos/v2/eps_appointment.rb

246ed56

Co-authored-by: Gaurav Gupta <randomsync@users.noreply.github.com>

va-vfs-bot temporarily deployed to 96412_merge_eps_appointments/main/main January 28, 2025 15:17 Inactive

randomsync previously approved these changes Jan 28, 2025

View reviewed changes

platform-support-slack-integration bot mentioned this pull request Jan 28, 2025

we have some PRs pending platform review that have been appr... department-of-veterans-affairs/va.gov-team#101945

Closed

randomsync requested a review from a team January 28, 2025 17:57

Merge branch 'master' into 96412_merge_eps_appointments

5f05d5b

lee-delarm6 added 2 commits January 29, 2025 10:34

rubocop fix

6fea526

rubocop fix

Merge branch '96412_merge_eps_appointments' of https://github.com/dep…

0b4093c

…artment-of-veterans-affairs/vets-api into 96412_merge_eps_appointments

lee-delarm6 dismissed randomsync’s stale review via 0b4093c January 29, 2025 15:34

github-actions bot removed the lint-failure label Jan 29, 2025

va-vfs-bot temporarily deployed to 96412_merge_eps_appointments/main/main January 29, 2025 15:57 Inactive

lee-delarm6 and others added 3 commits January 29, 2025 13:36

Merge branch 'master' into 96412_merge_eps_appointments

5e9be5f

Merge branch 'master' into 96412_merge_eps_appointments

303da0c

Fixed testing and rubocop errors

f817cac

Fixed testing and rubocop errors

github-actions bot added test-passing ready-for-backend-review and removed test-failure labels Jan 29, 2025

va-vfs-bot deployed to 96412_merge_eps_appointments/main/main January 29, 2025 19:48 View deployment

stevenjcumming requested changes Jan 29, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding merge functionality for eps to regular appointments #19754

adding merge functionality for eps to regular appointments #19754

lee-delarm6 commented Dec 5, 2024 •

edited

Loading

randomsync left a comment

randomsync Dec 9, 2024

randomsync Jan 22, 2025

randomsync Dec 9, 2024

lee-delarm6 Dec 12, 2024

kanchanasuriya Dec 18, 2024

lee-delarm6 Jan 17, 2025

randomsync left a comment

randomsync Jan 28, 2025

stevenjcumming commented Jan 29, 2025

lee-delarm6 commented Jan 29, 2025

lee-delarm6 commented Jan 29, 2025

stevenjcumming commented Jan 29, 2025

lee-delarm6 commented Jan 29, 2025

stevenjcumming left a comment •

edited

Loading

adding merge functionality for eps to regular appointments #19754

Are you sure you want to change the base?

adding merge functionality for eps to regular appointments #19754

Conversation

lee-delarm6 commented Dec 5, 2024 • edited Loading

Summary

Testing done

What areas of the site does it impact?

Acceptance criteria

randomsync left a comment

Choose a reason for hiding this comment

randomsync Dec 9, 2024

Choose a reason for hiding this comment

randomsync Jan 22, 2025

Choose a reason for hiding this comment

randomsync Dec 9, 2024

Choose a reason for hiding this comment

lee-delarm6 Dec 12, 2024

Choose a reason for hiding this comment

kanchanasuriya Dec 18, 2024

Choose a reason for hiding this comment

lee-delarm6 Jan 17, 2025

Choose a reason for hiding this comment

randomsync left a comment

Choose a reason for hiding this comment

randomsync Jan 28, 2025

Choose a reason for hiding this comment

stevenjcumming commented Jan 29, 2025

lee-delarm6 commented Jan 29, 2025

lee-delarm6 commented Jan 29, 2025

stevenjcumming commented Jan 29, 2025

lee-delarm6 commented Jan 29, 2025

stevenjcumming left a comment • edited Loading

Choose a reason for hiding this comment

lee-delarm6 commented Dec 5, 2024 •

edited

Loading

stevenjcumming left a comment •

edited

Loading