-
Notifications
You must be signed in to change notification settings - Fork 7
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
AP-5307: Add firm offices to new PDA result #7209
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsugarman
force-pushed
the
ap-5307/query-pda-provider-firm-offices
branch
5 times, most recently
from
September 20, 2024 12:40
2dddad8
to
121038b
Compare
The existing CCMS PDA returns all offices in the providers firm as part of the "provider offices". The new PDA API only returns the offices of the specific individual provider (a person/user). To understand the impact and how to handle it we want to compare all the offices in the firm of the specific provider from the new PDA with those coming from the old CCSM PDA. The new PDA requires a second call to achieve this, so the relevant part of the response body of the second call have been merged with the response body of the first call and returned as a result object.
jsugarman
force-pushed
the
ap-5307/query-pda-provider-firm-offices
branch
from
September 20, 2024 13:52
121038b
to
796345a
Compare
This should be ingress.hosts but has been changed in the values files for some reason.
Makes it more of an integration test and inline with changes to the comparision tool class.
jsugarman
force-pushed
the
ap-5307/query-pda-provider-firm-offices
branch
from
September 23, 2024 07:09
2589a2f
to
872af11
Compare
Quality Gate passedIssues Measures |
colinbruce
approved these changes
Sep 23, 2024
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.
It looks okay to me, but could probably do with a tech UAT run to check the results
jsugarman
added
active-record-encryption
approved
Approved by code reviewers
and removed
active-record-encryption
labels
Sep 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Add and log firm offices from new PDA API
Link to story
The existing PDA returns all offices in the providers firm as part of the "provider offices". The new PDA
only returns the offices of the specific individual provider (a person/user). To understand the impact and how
to handle it we want to compare all the offices in the firm of the specific provider from the new PDA with those coming from the old CCSM PDA.
The new PDA requires a second call to achieve this, so the relevant part of the response body of the second call has been merged with the response body of the first call and returned as a result object.
Checklist
Before you ask people to review this PR:
bundle exec rake
git rebase main
.