-
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
Move modify_desired_date logic into appointments service #17134
Conversation
@@ -81,6 +82,11 @@ def post_appointment(request_object_body) | |||
perform(:post, appointments_base_path_vaos, params, headers) | |||
end | |||
|
|||
if request_object_body[:kind] == 'clinic' && |
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.
@cferris32 The only difference between this PR and what we worked on during our pair session is that I moved this date modification block from the top of the method to after the post requests.
I noticed while fixing tests tests that in cases where patientIcn is missing, we would make a redundant get facilities request if we had this logic at the beginning of this method. Instead, if we had this logic here, the post requests above will raise exceptions first and we won't make the get facilities request which I think is slightly better. Let me know if you disagree and I can move the logic back to the top of the method and update the VCR cassettes instead.
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 think it should be good this way, that's a nice catch and saving on a redundant call is preferred. I don't think there should be anything else this impacts
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
Note: Delete the description statements, complete each step. None are optional, but can be justified as to why they cannot be completed as written. Provide known gaps to testing that may raise the risk of merging to production.
Summary
Related issue(s)
Testing done
Acceptance criteria