Skip to content
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

CC Referral Appointments - Draft Appointment Submit #20323

Merged
merged 7 commits into from
Jan 29, 2025

Conversation

devin-mccurdy
Copy link
Contributor

@devin-mccurdy devin-mccurdy commented Jan 16, 2025

Summary

  • This work is behind a feature toggle (flipper): NO - this is a new endpoint and not behind a backend feature flag.

  • Add a new route to the VAOS module that submits CC referral draft appointments to EPS. The submission doesn't return any useful data other than the 201 response.

Related issue(s)

Testing done

  • New code is covered by unit tests

Screenshots

N/A

What areas of the site does it impact?

VAOS module

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)

@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 16, 2025 20:01 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 16, 2025 20:33 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 16, 2025 20:59 Inactive
@devin-mccurdy devin-mccurdy force-pushed the 98251_draft_appointment_submit branch from 97208ba to eeb109a Compare January 16, 2025 21:30
@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 16, 2025 21:31 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 16, 2025 22:10 Inactive
@devin-mccurdy devin-mccurdy marked this pull request as ready for review January 17, 2025 15:37
@devin-mccurdy devin-mccurdy requested review from a team as code owners January 17, 2025 15:37
@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 17, 2025 15:39 Inactive
@stevenjcumming
Copy link
Contributor

@devin-mccurdy this PR has a failing test

@devin-mccurdy
Copy link
Contributor Author

@devin-mccurdy this PR has a failing test

Whoops, last minute update to match the draft serializer PR and missed that I'd also need to update that test

@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 17, 2025 16:29 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 17, 2025 17:08 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 21, 2025 22:03 Inactive
randomsync
randomsync previously approved these changes Jan 23, 2025
Copy link
Member

@randomsync randomsync left a comment

Choose a reason for hiding this comment

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

lgtm

stevenjcumming
stevenjcumming previously approved these changes Jan 28, 2025
@devin-mccurdy devin-mccurdy force-pushed the 98251_draft_appointment_submit branch from e2ddc57 to f6a2309 Compare January 28, 2025 20:09
randomsync
randomsync previously approved these changes Jan 28, 2025
Copy link
Member

@randomsync randomsync left a comment

Choose a reason for hiding this comment

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

lgtm

@devin-mccurdy devin-mccurdy enabled auto-merge (squash) January 28, 2025 20:21
Comment on lines -448 to -451

vaos:
eps:
key_path: modules/health_quest/config/rsa/sandbox_rsa
Copy link
Contributor

Choose a reason for hiding this comment

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

I just approved a PR that added this. Please confirm this deletion

Copy link
Member

@randomsync randomsync Jan 28, 2025

Choose a reason for hiding this comment

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

there were 2 PRs that were merged that both included this change in different places of the test.yml, which caused this to be duped in final merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is correct - it also got added and merged in another PR and this is removing one of them

@@ -26,4 +26,52 @@
end
end
end

describe '#build_patient_attributes' do
Copy link
Contributor

Choose a reason for hiding this comment

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

2 small things:

  • the describe method should match the name exactly#patient_attributes
  • you don't need to test private methods. I would encourage you to consider removing this because it make refactoring difficult.

@va-vfs-bot va-vfs-bot temporarily deployed to 98251_draft_appointment_submit/main/main January 28, 2025 22:45 Inactive
@devin-mccurdy devin-mccurdy merged commit 23ba7e6 into master Jan 29, 2025
22 checks passed
@devin-mccurdy devin-mccurdy deleted the 98251_draft_appointment_submit branch January 29, 2025 16:28
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.

4 participants