BB2-4266 - C4DIC Endpoint #1428
Merged
Conversation
This file contains hidden or 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
…ub.com/CMSgov/bluebutton-web-server into clewellyn-nava/BB2-4266/C4DIC-endpoint
…ub.com/CMSgov/bluebutton-web-server into clewellyn-nava/BB2-4266/C4DIC-endpoint
…ub.com/CMSgov/bluebutton-web-server into clewellyn-nava/BB2-4266/C4DIC-endpoint
We're discovering what is needed for "integration" or "e2e" tests, and perhaps Selenium should just do the lifting. Cleaning up a bit, and diving into the token permissions.
After much sleuthing and conversation, we decided that trying to make the ProtectedCapabilities model work for C4DIC does not make sense. Further, HasDigitalInsuranceCardScope does *almost* the same checking; the only thing it does not check is the *method*, which is not a showstopper (or even a concern, really, because we already gate that elsewhere).
This does not have tests for `DigitalInsuranceCard`. That would imply that we have a user we can plug in for upstream tests against BFD. It is unclear if we have that user.
jadudm
reviewed
Dec 8, 2025
jadudm
reviewed
Dec 8, 2025
jadudm
reviewed
Dec 8, 2025
jadudm
reviewed
Dec 8, 2025
jadudm
reviewed
Dec 8, 2025
jadudm
reviewed
Dec 8, 2025
Contributor
|
Looks great! Testclient looks good (did v2 as well just to be sure), Postman works as expected, and when I only have read scopes for an access token, I get a 403. All the functionality looks good, going through the diff now. Just left a few questions/comments. |
| """ | ||
| def has_permission(self, request, view): | ||
|
|
||
| def has_permission(self, request, view) -> bool: # type: ignore |
Contributor
There was a problem hiding this comment.
Why do we need the type: ignore here?
| class TokenHasProtectedCapability(permissions.BasePermission): | ||
|
|
||
| def has_permission(self, request, view): | ||
| def has_permission(self, request, view) -> bool: # type: ignore |
JamesDemeryNava
approved these changes
Dec 9, 2025
Contributor
JamesDemeryNava
left a comment
JamesDemeryNava
left a comment
There was a problem hiding this comment.
This looks good to me! All the functionality works from what I tested, and comments have been addressed. I'd say this could use another set of eyes before merging, but there was a ton of collaboration on this already.
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.
JIRA Ticket:
What Does This PR Do?
This PR:
/v3/DigitalInsuranceCardHasDigitalInsuranceCardScope) and unit tests for covering the scopes requiredjsonorapplication/jsontoapplication/fhir+json, which is the correct/appropriate MIME type for those calls.What Should Reviewers Watch For?
ViewSetimplementations in the codebase for discussion. After discussion, we might either move them into the primary way we handle API calls, or we might remove them. They are not executed in production, and therefore are left for discussion at this time. (If this is deemed to be a bad idea, we can pull them from this PR.)Validation
Unit tests pass, integration tests pass, local testclient works as expected. This shouldn't be a functional change of any existing endpoints or tests.
For testing the C4DIC endpoint, you will need to authenticate with BBUser09995 (PW09995!@), as it has the C4DIC data populated. (BBUser09003 also seems to work.)
What Security Implications Does This PR Have?
N/A
Any Migrations?
N/A