-
Notifications
You must be signed in to change notification settings - Fork 28
Adds the pathways for v3 handling around fhir_ids (BB-4166) #1419
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
Merged
Merged
Changes from all commits
Commits
Show all changes
55 commits
Select commit
Hold shift + click to select a range
df80391
initial commit
bwang-icf cfeb712
adding version specific permission checking so v3 resources are recog…
bwang-icf ef64ba6
TODO in authorization/views.py. Need to write unit tests for new util…
JamesDemeryNava cf33395
Address TODOs in bb2_tools/admin.py
JamesDemeryNava ab29772
found request.path to contain version. Different from other places bu…
bwang-icf 433d5c5
Ensure fhir_id v2 or v3 are updated if none when re-authorizing. v3 t…
JamesDemeryNava 157d988
adding extra validation for incorrect versions in utils and using it …
bwang-icf 2707686
V2 calls all working, v3 patient returning correct data but built in …
JamesDemeryNava 6cae063
adding more generalized version grabbing for crosswalk permissions
bwang-icf a58570a
Revert log to reduce test failures (will revisit this log once all fu…
JamesDemeryNava 6bf69b0
Ensure v3 coverage and EOD endpoints work. Currently 7 test failures …
JamesDemeryNava cb698da
changes to get_patient_id, test_read_and_search and other tests to ex…
bwang-icf 4838058
Modify some tests, pass a version to generate_info_headers. Tests are…
JamesDemeryNava 8614644
Merge branch 'master' into bw/BB2-4166
JamesDemeryNava c65b63c
added notes and some changes to mymedicare views
bwang-icf 90c7921
Interim collab commit
jadudm 672c806
Remove completed TODOs
JamesDemeryNava 87c39e7
Remove more completed TODOs
JamesDemeryNava 717462b
Remove more TODOs that had been completed
JamesDemeryNava ebf5125
Fix audit_logger tests, WIP on authentication
JamesDemeryNava 8525ea3
Fixed mocked tests
jadudm 8ea6bd7
Removing print statements
jadudm 6aec825
Merge branch 'master' into bw/BB2-4166
jadudm e6bc40b
Add version param to create_fhir_mock to help tests succeed
JamesDemeryNava aa7dc60
Revert unneeded test changes, ensure still passes
JamesDemeryNava f4c043d
Fix last failing test in test_authentication.py
JamesDemeryNava 69e9d8d
Add unit test, remove completed TODOs
JamesDemeryNava dd315f8
WIP On fixing tests
JamesDemeryNava 98c523b
Interim
jadudm b873afb
Fixed callback tests
jadudm c180fd1
fix for test_callback_allow_slsx_changes_to_hicn_and_mbi
JamesDemeryNava f280493
Revert responses.py changes
JamesDemeryNava 2da17de
Fixed import
jadudm d97fd9b
Renaming.
jadudm 5b43959
Rename function, modify return type per PR feedback
JamesDemeryNava 173ec43
Modify imports to reflect new file name
JamesDemeryNava c411169
Modify default param and update to absolute paths
JamesDemeryNava 56928af
Remove unneeded TODO
JamesDemeryNava 1c1732c
Modify a function name and call to more accurately reflect what it does
JamesDemeryNava 3982daa
revert to have v2 as default to fhir_id, as it broke tests, and we on…
JamesDemeryNava 0bdd43d
Check on blank string in get_and_update_user to accommodate changes f…
JamesDemeryNava b95785b
Single quotes and comment context
JamesDemeryNava f6d4d7d
Refactor get_and_update_user to reduce BFD calls, and to ensure fhir_…
JamesDemeryNava 1ea136e
Updates based on PR self-review with Brandon and Matt
JamesDemeryNava 85fd19c
adding latest versions and changes to the offset math test
bwang-icf 675f69d
added defaults if there is no fhir_id found for a version
bwang-icf 72f0819
Merge branch 'master' into bw/BB2-4166
bwang-icf a31aeba
Merge branch 'master' into bw/BB2-4166
bwang-icf f19d8a4
addressing comments
bwang-icf 4cf4355
Merge branch 'master' into bw/BB2-4166
JamesDemeryNava 4e41897
Merge branch 'master' into bw/BB2-4166
clewellyn-nava 63e5245
Resolve conflict, still work to do to resolve TODO
JamesDemeryNava 80307e3
changing check to empty string over None but will change back to None…
bwang-icf 6021985
Address TODO that arose from a change made in BB2-4233
JamesDemeryNava 91c3a37
use self.version instead
JamesDemeryNava File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| from django.test import TestCase | ||
|
|
||
| from apps.versions import VersionNotMatched | ||
| from ..utils import get_api_version_number_from_url | ||
|
|
||
| SUPPORTED_VERSION_TEST_CASES = [ | ||
| {'url_path': '/v2/fhir/Patient/', 'expected': 2}, | ||
| # return 0 because v2 does not have a leading / | ||
| {'url_path': 'v2/fhir/Patient/', 'expected': 0}, | ||
| {'url_path': '/v3/fhir/Coverage/', 'expected': 3}, | ||
| {'url_path': '/v3/fhir/Coverage/v2/', 'expected': 3}, | ||
| ] | ||
|
|
||
|
|
||
| class TestDOTUtils(TestCase): | ||
| def test_get_api_version_number(self): | ||
| for test in SUPPORTED_VERSION_TEST_CASES: | ||
| result = get_api_version_number_from_url(test['url_path']) | ||
| assert result == test['expected'] | ||
|
|
||
| def test_get_api_version_number_unsupported_version(self): | ||
| # unsupported version will raise an exception | ||
| with self.assertRaises(VersionNotMatched, msg='4 extracted from /v4/fhir/Coverage/'): | ||
| get_api_version_number_from_url('/v4/fhir/Coverage/') |
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,29 +42,23 @@ def test_not_require_user_mbi(self): | |
| cw = Crosswalk.objects.get(user=user) | ||
| self.assertEqual(cw.user_mbi, None) | ||
|
|
||
| # TODO BB2-4166: This was commented out as part of | ||
| # BB2-4181, but after the work of 4166 is done, this test should | ||
| # pass. However, in 4181, we are storing _v3 values in _v2 (at the model level), and | ||
| # therefore the logic of this test does not work in 4181's branch. | ||
| # Once 4166 is fully implemented, uncomment this test, and it should pass and | ||
| # navigating the test client using the v3 pathway should work. | ||
| # def test_mutable_fhir_id(self): | ||
| # user = self._create_user( | ||
| # "john", | ||
| # "password", | ||
| # first_name="John", | ||
| # last_name="Smith", | ||
| # email="[email protected]", | ||
| # fhir_id_v2="-20000000000001", | ||
| # fhir_id_v3="-30000000000001", | ||
| # ) | ||
| # cw = Crosswalk.objects.get(user=user) | ||
| # self.assertEqual(cw.fhir_id(2), "-20000000000001") | ||
| # self.assertEqual(cw.fhir_id(3), "-30000000000001") | ||
| # cw.set_fhir_id("-20000000000002", 2) | ||
| # cw.set_fhir_id("-30000000000002", 3) | ||
| # self.assertEqual(cw.fhir_id(2), "-20000000000002") | ||
| # self.assertEqual(cw.fhir_id(3), "-30000000000002") | ||
| def test_mutable_fhir_id(self): | ||
| user = self._create_user( | ||
| "john", | ||
| "password", | ||
| first_name="John", | ||
| last_name="Smith", | ||
| email="[email protected]", | ||
| fhir_id_v2="-20000000000001", | ||
| fhir_id_v3="-30000000000001", | ||
| ) | ||
| cw = Crosswalk.objects.get(user=user) | ||
| self.assertEqual(cw.fhir_id(2), "-20000000000001") | ||
| self.assertEqual(cw.fhir_id(3), "-30000000000001") | ||
| cw.set_fhir_id("-20000000000002", 2) | ||
| cw.set_fhir_id("-30000000000002", 3) | ||
| self.assertEqual(cw.fhir_id(2), "-20000000000002") | ||
| self.assertEqual(cw.fhir_id(3), "-30000000000002") | ||
|
|
||
| def test_mutable_user_hicn_hash(self): | ||
| user = self._create_user( | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Do we have
fhir_id_v1or both v1 and v2 sharefhir_id_v2, if so could you add comment in here to avoid confusion?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.
Sure thing. I can add a comment here, but for most areas we are definitely treating v1/v2 as the same thing since their pathways are fairly similar.
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.
@stiwarisemanticbits , for reference (and so we have it here):
There used to be only
fhir_id. We have now introduced_v2and_v3, and the references tofhir_idbecame a function on the model that now takes aversionparameter. So, everywhere we used to say:(which was, at that time, implicitly already v1 and v2)
we now say:
So, commenting that V1 now shares V2 is something we would have to comment throughout the entire codebase. @bwang-icf , before we start commenting this in the code, we may want to decide if/where we need to document this kind of design work. I personally think it would be the TRD.
Uh oh!
There was an error while loading. Please reload this page.
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 commenting on the model is sufficient, otherwise we run into issues you described.
I believe we can also use
|in case statements in Python 3.11case Version.V1 | Version.V2This has the code read that they are treated the same, and if someone is curious as to why, they can reference the model comment. Just my two cents.