Refactor get_and_update_user to reduce BFD calls, and to ensure fhir_ids are always updated if DB values differ from what BFD returns

Author: JamesDemeryNava

apps/fhir/bluebutton/utils.py

@@ -410,7 +410,6 @@ def prepend_q(pass_params):
     return pass_params
 
 
-# BB2-4166-TODO - if tests are failing this could be related
 def dt_patient_reference(user, version):
     """Get Patient Reference from Crosswalk for user"""
 
@@ -428,7 +427,6 @@ def crosswalk_patient_id(user, version):
     logger.debug("\ncrosswalk_patient_id User:%s" % user)
     try:
         patient = Crosswalk.objects.get(user=user)
-        # BB2-4166-TODO - if tests are failing this could be related
         if patient.fhir_id(version):
             return patient.fhir_id(version)
 

apps/fhir/server/tests/test_authentication.py

@@ -6,9 +6,9 @@
 from rest_framework import exceptions
 from apps.fhir.bluebutton.exceptions import UpstreamServerException
 from apps.test import BaseApiTest
-from ..authentication import match_fhir_id
+from apps.fhir.server.authentication import match_fhir_id
 from apps.versions import Versions
-from .responses import responses
+from apps.fhir.server.tests.responses import responses
 
 from hhs_oauth_server.settings.base import MOCK_FHIR_ENDPOINT_HOSTNAME, MOCK_FHIR_V3_ENDPOINT_HOSTNAME
 
@@ -17,9 +17,8 @@ def mock_fhir_url(version):
     return MOCK_FHIR_ENDPOINT_HOSTNAME if version in [1, 2] else MOCK_FHIR_V3_ENDPOINT_HOSTNAME
 
 
-#     MOCK_FHIR_PATH = "/v1/fhir/Patient/"
 def mock_fhir_path(version):
-    return f"/v{version}/fhir/Patient"
+    return f'/v{version}/fhir/Patient'
 
 
 class TestAuthentication(BaseApiTest):
@@ -41,10 +40,7 @@ def setUp(self):
         self.request = self.factory.get('http://localhost:8000/mymedicare/sls-callback')
         self.request.session = self.client.session
 
-    # The mock uses data from responses.py. We need to have mock data for a given MBI/FHIR ID
-    # for each version of the API we want to test this way. We can use the v2 test data for
-    # v3 because we're just asking "do you have a valid us-mbi query string?", and usnig that to then
-    # look up the mock response.
+    # The mock uses data from responses.py
     @classmethod
     def create_fhir_mock(cls, hicn_response_key, mbi_response_key, version=Versions.NOT_AN_API_VERSION):
         @urlmatch(netloc=mock_fhir_url(version), path=mock_fhir_path(version), method='POST')

apps/mymedicare_cb/models.py

@@ -17,6 +17,7 @@
 
 MAX_HICN_HASH_LENGTH = 64
 MAX_MBI_LENGTH = 11
+FHIR_ID_VERSIONS_TO_CHECK = [2, 3]
 
 
 class BBMyMedicareCallbackCrosswalkCreateException(APIException):
@@ -68,25 +69,27 @@ def get_and_update_user(slsx_client: OAuth2ConfigSLSx, request):
     # If the lookup for the requested version fails, raise the exception
     # This is wrapped in the case that if the requested version fails, match_fhir_id
     # will still bubble up UpstreamServerException
-    for supported_version in Versions.supported_versions():
+    for supported_version in FHIR_ID_VERSIONS_TO_CHECK:
         try:
             fhir_id, hash_lookup_type = match_fhir_id(
                 mbi=slsx_client.mbi,
                 hicn_hash=hicn_hash,
                 request=request,
                 version=supported_version,
-
             )
             versioned_fhir_ids[supported_version] = fhir_id
         except UpstreamServerException as e:
             if supported_version == version:
                 raise e
 
+    bfd_fhir_id_v2 = versioned_fhir_ids[Versions.V2]
+    bfd_fhir_id_v3 = versioned_fhir_ids[Versions.V3]
+
     log_dict = {
         'type': 'mymedicare_cb:get_and_update_user',
         'subject': slsx_client.user_id,
-        'fhir_id_v2': versioned_fhir_ids.get(Versions.V2, None),
-        'fhir_id_v3': versioned_fhir_ids.get(Versions.V3, None),
+        'fhir_id_v2': bfd_fhir_id_v2,
+        'fhir_id_v3': bfd_fhir_id_v3,
         'hicn_hash': slsx_client.hicn_hash,
         'hash_lookup_type': hash_lookup_type,
         'crosswalk': {},
@@ -104,7 +107,12 @@ def get_and_update_user(slsx_client: OAuth2ConfigSLSx, request):
             hicn_updated = True
 
         update_fhir_id = False
-        if user.crosswalk.fhir_id(Versions.V2) == '' or user.crosswalk.fhir_id(Versions.V3) == '':
+        if (
+            user.crosswalk.fhir_id(Versions.V2) == ''
+            or user.crosswalk.fhir_id(Versions.V3) == ''
+            or user.crosswalk.fhir_id(Versions.V2) != bfd_fhir_id_v2
+            or user.crosswalk.fhir_id(Versions.V3) != bfd_fhir_id_v3
+        ):
             update_fhir_id = True
         # Update Crosswalk if the user_mbi is null, but we have an mbi value from SLSx or
         # if the saved user_mbi value is different than what SLSx has
@@ -119,8 +127,8 @@ def get_and_update_user(slsx_client: OAuth2ConfigSLSx, request):
                 'crosswalk_before': {
                     'id': user.crosswalk.id,
                     'user_hicn_hash': user.crosswalk.user_hicn_hash,
-                    'fhir_id_v2': versioned_fhir_ids.get(Versions.V2, None),
-                    'fhir_id_v3': versioned_fhir_ids.get(Versions.V3, None),
+                    'fhir_id_v2': bfd_fhir_id_v2,
+                    'fhir_id_v3': bfd_fhir_id_v3,
                     'user_id_type': user.crosswalk.user_id_type,
                 },
             })
@@ -129,8 +137,8 @@ def get_and_update_user(slsx_client: OAuth2ConfigSLSx, request):
                 # Archive to audit crosswalk changes
                 ArchivedCrosswalk.create(user.crosswalk)
                 if update_fhir_id:
-                    user.crosswalk.fhir_id_v2 = versioned_fhir_ids.get(Versions.V2)
-                    user.crosswalk.fhir_id_v3 = versioned_fhir_ids.get(Versions.V3)
+                    user.crosswalk.fhir_id_v2 = bfd_fhir_id_v2
+                    user.crosswalk.fhir_id_v3 = bfd_fhir_id_v3
                 # Update crosswalk per changes
                 user.crosswalk.user_id_type = hash_lookup_type
                 user.crosswalk.user_hicn_hash = slsx_client.hicn_hash
@@ -147,8 +155,8 @@ def get_and_update_user(slsx_client: OAuth2ConfigSLSx, request):
             'crosswalk': {
                 'id': user.crosswalk.id,
                 'user_hicn_hash': user.crosswalk.user_hicn_hash,
-                'fhir_id_v2': versioned_fhir_ids.get(Versions.V2, None),
-                'fhir_id_v3': versioned_fhir_ids.get(Versions.V3, None),
+                'fhir_id_v2': bfd_fhir_id_v2,
+                'fhir_id_v3': bfd_fhir_id_v3,
                 'user_id_type': user.crosswalk.user_id_type,
             },
         })
@@ -160,8 +168,8 @@ def get_and_update_user(slsx_client: OAuth2ConfigSLSx, request):
 
     user = create_beneficiary_record(
         slsx_client,
-        fhir_id_v2=versioned_fhir_ids.get(Versions.V2, None),
-        fhir_id_v3=versioned_fhir_ids.get(Versions.V3, None),
+        fhir_id_v2=bfd_fhir_id_v2,
+        fhir_id_v3=bfd_fhir_id_v3,
         user_id_type=hash_lookup_type,
         request=request
     )
@@ -175,8 +183,8 @@ def get_and_update_user(slsx_client: OAuth2ConfigSLSx, request):
         'crosswalk': {
             'id': user.crosswalk.id,
             'user_hicn_hash': user.crosswalk.user_hicn_hash,
-            'fhir_id_v2': versioned_fhir_ids.get(Versions.V2, None),
-            'fhir_id_v3': versioned_fhir_ids.get(Versions.V3, None),
+            'fhir_id_v2': bfd_fhir_id_v2,
+            'fhir_id_v3': bfd_fhir_id_v3,
             'user_id_type': user.crosswalk.user_id_type,
         },
     })