-
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
CC Referral Appointments - Draft Appointment Submit #20323
Conversation
97208ba
to
eeb109a
Compare
@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 |
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
f6a2309
e2ddc57
to
f6a2309
Compare
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
|
||
vaos: | ||
eps: | ||
key_path: modules/health_quest/config/rsa/sandbox_rsa |
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.
I just approved a PR that added this. Please confirm this deletion
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.
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
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, 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 |
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.
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.
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
Screenshots
N/A
What areas of the site does it impact?
VAOS module
Acceptance criteria