Modify viewsets_base to look for read/search. Correct format=json in several places

Author: JamesDemeryNava

apps/fhir/bluebutton/permissions.py

@@ -106,3 +106,8 @@ def has_permission(self, request, view):
             )
 
         return True
+
+
+class AlwaysDeny(permissions.BasePermission):
+    def has_permission(self, request, view):
+        return False

apps/fhir/bluebutton/tests/test_fhir_resources_read_search_w_validation.py

@@ -224,7 +224,7 @@ def _search_eob_by_parameter_tag(self, version=1):
         def catchall_w_tag_qparam(url, req):
             # this is called in case EOB search with good tag
             self.assertIn(f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/ExplanationOfBenefit/', req.url)
-            self.assertIn('_format=application%2Fjson%2Bfhir', req.url)
+            self.assertIn('_format=application%2Ffhir%2Bjson', req.url)
             # parameters encoded in prepared request's body
             self.assertTrue(('_tag=Adjudicated' in req.url) or ('_tag=PartiallyAdjudicated' in req.url))
 
@@ -236,7 +236,7 @@ def catchall_w_tag_qparam(url, req):
         @all_requests
         def catchall(url, req):
             self.assertIn(f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/ExplanationOfBenefit/', req.url)
-            self.assertIn('_format=application%2Fjson%2Bfhir', req.url)
+            self.assertIn('_format=application%2Ffhir%2Bjson', req.url)
 
             return {
                 'status_code': 200,
@@ -280,7 +280,7 @@ def _search_eob_by_parameters_request(self, version=1):
         @all_requests
         def catchall(url, req):
             self.assertIn(f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/ExplanationOfBenefit/', req.url)
-            self.assertIn('_format=application%2Fjson%2Bfhir', req.url)
+            self.assertIn('_format=application%2Ffhir%2Bjson', req.url)
 
             return {
                 'status_code': 200,

apps/fhir/bluebutton/tests/test_read_and_search.py

@@ -26,7 +26,7 @@
 def get_expected_read_request(version: int):
     return {
         'method': 'GET',
-        'url': f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/Patient/{FHIR_ID_V2}/?_format=application/json+fhir&_id={FHIR_ID_V2}',
+        'url': f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/Patient/{FHIR_ID_V2}/?_format=application/fhir+json&_id={FHIR_ID_V2}',
         'headers': {
             # 'User-Agent': 'python-requests/2.20.0',
             'Accept-Encoding': 'gzip, deflate',
@@ -284,7 +284,7 @@ def _search_request(self, version: int = 1):
         @all_requests
         def catchall(url, req):
             self.assertIn(f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/Patient/', req.url)
-            self.assertIn('_format=application%2Fjson%2Bfhir', req.url)
+            self.assertIn('_format=application%2Ffhir%2Bjson', req.url)
             self.assertIn(f'_id={FHIR_ID_V2}', req.url)
             self.assertIn('_count=5', req.url)
             self.assertNotIn('hello', req.url)
@@ -362,7 +362,7 @@ def _search_request_not_found(self, version: int = 1):
         @all_requests
         def catchall(url, req):
             self.assertIn(f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/Patient/', req.url)
-            self.assertIn('_format=application%2Fjson%2Bfhir', req.url)
+            self.assertIn('_format=application%2Ffhir%2Bjson', req.url)
             self.assertIn(f'_id={FHIR_ID_V2}', req.url)
             self.assertEqual(expected_request['method'], req.method)
             self.assertDictContainsSubset(expected_request['headers'], req.headers)
@@ -444,7 +444,7 @@ def _search_request_failed(self, version: int = 1, bfd_status_code=500):
         @all_requests
         def catchall(url, req):
             self.assertIn(f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/Patient/', req.url)
-            self.assertIn('_format=application%2Fjson%2Bfhir', req.url)
+            self.assertIn('_format=application%2Ffhir%2Bjson', req.url)
             self.assertIn(f'_id={FHIR_ID_V2}', req.url)
             self.assertEqual(expected_request['method'], req.method)
             self.assertDictContainsSubset(expected_request['headers'], req.headers)
@@ -499,7 +499,7 @@ def fhir_request(url, req):
         @all_requests
         def catchall(url, req):
             self.assertIn(f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/Patient/', req.url)
-            self.assertIn('_format=application%2Fjson%2Bfhir', req.url)
+            self.assertIn('_format=application%2Ffhir%2Bjson', req.url)
             self.assertIn(f'_id={FHIR_ID_V2}', req.url)
             self.assertEqual(expected_request['method'], req.method)
             self.assertDictContainsSubset(expected_request['headers'], req.headers)
@@ -532,7 +532,7 @@ def _search_parameters_request(self, version: int = 1):
         @all_requests
         def catchall(url, req):
             self.assertIn(f'{FHIR_SERVER["FHIR_URL"]}/v{version}/fhir/ExplanationOfBenefit/', req.url)
-            self.assertIn('_format=application%2Fjson%2Bfhir', req.url)
+            self.assertIn('_format=application%2Ffhir%2Bjson', req.url)
 
             return {
                 'status_code': 200,

apps/fhir/bluebutton/v2/urls.py

@@ -4,27 +4,28 @@
 from apps.fhir.bluebutton.views.read import (
     ReadViewCoverage,
     ReadViewExplanationOfBenefit,
+    ReadViewPatient,
 )
 from apps.fhir.bluebutton.views.search import (
     SearchViewCoverage,
     SearchViewExplanationOfBenefit,
+    SearchViewPatient,
 )
-from apps.fhir.bluebutton.views.patient_viewset import PatientViewSet
 
 admin.autodiscover()
 
 urlpatterns = [
     # Patient ReadView
     re_path(
-        r'Patient/(?P<resource_id>[^/]+)',
-        PatientViewSet.as_view({'get': 'read'}, version=2),
-        name='bb_oauth_fhir_patient_read_or_update_or_delete_v2',
+        r"Patient/(?P<resource_id>[^/]+)",
+        ReadViewPatient.as_view(version=2),
+        name="bb_oauth_fhir_patient_read_or_update_or_delete_v2",
     ),
     # Patient SearchView
     re_path(
-        r'Patient[/]?',
-        PatientViewSet.as_view({'get': 'search'}, version=2),
-        name='bb_oauth_fhir_patient_search_v2',
+        r"Patient[/]?",
+        SearchViewPatient.as_view(version=2),
+        name="bb_oauth_fhir_patient_search_v2",
     ),
     # Coverage ReadView
     re_path(

apps/fhir/bluebutton/v3/urls.py

@@ -5,52 +5,53 @@
 from apps.fhir.bluebutton.views.read import (
     ReadViewCoverage,
     ReadViewExplanationOfBenefit,
+    ReadViewPatient,
 )
 from apps.fhir.bluebutton.views.search import (
     SearchViewCoverage,
     SearchViewExplanationOfBenefit,
+    SearchViewPatient,
 )
-from apps.fhir.bluebutton.views.patient_viewset import PatientViewSet
 from apps.fhir.bluebutton.views.insurancecard_viewset import DigitalInsuranceCardViewSet
 
 admin.autodiscover()
 
 urlpatterns = [
     # Patient ReadView
     re_path(
-        r'Patient/(?P<resource_id>[^/]+)',
-        waffle_switch('v3_endpoints')(PatientViewSet.as_view({'get': 'read'}, version=3)),
-        name='bb_oauth_fhir_patient_read_or_update_or_delete_v3',
+        r"Patient/(?P<resource_id>[^/]+)",
+        waffle_switch("v3_endpoints")(ReadViewPatient.as_view(version=3)),
+        name="bb_oauth_fhir_patient_read_or_update_or_delete_v3",
     ),
     # Patient SearchView
     re_path(
-        r'Patient[/]?',
-        waffle_switch('v3_endpoints')(PatientViewSet.as_view({'get': 'search'}, version=3)),
-        name='bb_oauth_fhir_patient_search_v3',
+        r"Patient[/]?",
+        waffle_switch("v3_endpoints")(SearchViewPatient.as_view(version=3)),
+        name="bb_oauth_fhir_patient_search_v3",
     ),
     # Coverage ReadView
     re_path(
-        r'Coverage/(?P<resource_id>[^/]+)',
-        waffle_switch('v3_endpoints')(ReadViewCoverage.as_view(version=3)),
-        name='bb_oauth_fhir_coverage_read_or_update_or_delete_v3',
+        r"Coverage/(?P<resource_id>[^/]+)",
+        waffle_switch("v3_endpoints")(ReadViewCoverage.as_view(version=3)),
+        name="bb_oauth_fhir_coverage_read_or_update_or_delete_v3",
     ),
     # Coverage SearchView
     re_path(
-        r'Coverage[/]?',
-        waffle_switch('v3_endpoints')(SearchViewCoverage.as_view(version=3)),
-        name='bb_oauth_fhir_coverage_search_v3',
+        r"Coverage[/]?",
+        waffle_switch("v3_endpoints")(SearchViewCoverage.as_view(version=3)),
+        name="bb_oauth_fhir_coverage_search_v3",
     ),
     # EOB ReadView
     re_path(
-        r'ExplanationOfBenefit/(?P<resource_id>[^/]+)',
-        waffle_switch('v3_endpoints')(ReadViewExplanationOfBenefit.as_view(version=3)),
-        name='bb_oauth_fhir_eob_read_or_update_or_delete_v3',
+        r"ExplanationOfBenefit/(?P<resource_id>[^/]+)",
+        waffle_switch("v3_endpoints")(ReadViewExplanationOfBenefit.as_view(version=3)),
+        name="bb_oauth_fhir_eob_read_or_update_or_delete_v3",
     ),
     # EOB SearchView
     re_path(
-        r'ExplanationOfBenefit[/]?',
-        waffle_switch('v3_endpoints')(SearchViewExplanationOfBenefit.as_view(version=3)),
-        name='bb_oauth_fhir_eob_search_v3',
+        r"ExplanationOfBenefit[/]?",
+        waffle_switch("v3_endpoints")(SearchViewExplanationOfBenefit.as_view(version=3)),
+        name="bb_oauth_fhir_eob_search_v3",
     ),
     # C4DIC
     # Digital Insurance Card ViewSet
@@ -59,7 +60,7 @@
     # application, which means we're kinda breaking REST principles here.
     re_path(
         r'DigitalInsuranceCard[/]?',
-        waffle_switch('v3_endpoints')(DigitalInsuranceCardViewSet.as_view({'get': 'list'}, version=3)),
+        waffle_switch('v3_endpoints')(DigitalInsuranceCardViewSet.as_view({'get': 'search'}, version=3)),
         name='bb_oauth_fhir_dic_read',
     ),
 ]

apps/fhir/bluebutton/views/read.py

@@ -35,7 +35,8 @@ def get(self, request, *args, **kwargs):
 
     def build_parameters(self, *args, **kwargs):
         return {
-            '_format': 'json'
+            '_format': 'application/fhir+json'
+            # '_format': 'json'
         }
 
     def build_url(self, fhir_settings, resource_type, resource_id, **kwargs):  # type: ignore

apps/fhir/bluebutton/views/search.py

@@ -95,7 +95,7 @@ def __init__(self, version=1):
 
     def build_parameters(self, request, *args, **kwargs):
         return {
-            '_format': 'application/json+fhir',
+            '_format': 'application/fhir+json',
         }
 
 
@@ -109,7 +109,7 @@ def __init__(self, version=1):
 
     def build_parameters(self, request, *args, **kwargs):
         return {
-            '_format': 'application/json+fhir',
+            '_format': 'application/fhir+json',
             'beneficiary': 'Patient/' + request.crosswalk.fhir_id(self.version)
         }
 

apps/fhir/bluebutton/views/viewsets_base.py

@@ -2,6 +2,7 @@
 from rest_framework.response import Response
 from voluptuous import Schema, REMOVE_EXTRA
 
+from apps.fhir.bluebutton.permissions import AlwaysDeny
 from apps.fhir.bluebutton.views.generic import FhirDataView
 
 
@@ -25,12 +26,14 @@ def initial(self, request, *args, **kwargs):
 
     def get_permissions(self):
         action = getattr(self, 'action', None)
-        if action == 'list':
+        if action == 'search':
             permission_classes = getattr(self, 'SEARCH_PERMISSION_CLASSES', self.SEARCH_PERMISSION_CLASSES)
-        elif action == 'retrieve':
+        elif action == 'read':
             permission_classes = getattr(self, 'READ_PERMISSION_CLASSES', self.READ_PERMISSION_CLASSES)
         else:
-            permission_classes = (permissions.IsAuthenticated,)
+            # If it's not a read or search call, make the permissions_classes list contain a single class,
+            # permissions.AlwaysDeny, as if it is not a search or read call, we should not proceed.
+            permission_classes = (AlwaysDeny,)
         return [permission_class() for permission_class in permission_classes]
 
     def search(self, request, *args, **kwargs):
@@ -71,8 +74,9 @@ def filter_parameters(self, request) -> dict:
         return schema(params)
 
     # TODO - investigate a better way to structure this
+    # TODO - seems like we could be adding parameters that don't need to be added
     def build_parameters(self, request):
-        if getattr(self, 'action', None) != 'list':
+        if getattr(self, 'action', None) != 'search':
             return {
                 '_format': 'application/json+fhir',
             }

apps/testclient/constants.py

@@ -31,7 +31,7 @@ class EndpointUrl:
     coverage = "coverage"
     nav = "nav"
 
-    # TODO - theses are all format=json, not format=application/json+fhir, is this good?
+    # TODO - theses are all format=json, not format=application/fhir+json, is this good?
     @staticmethod
     def fmt(name: str, uri: str, version: int, patient: str = BAD_PATIENT_ID):
         version_as_string = Versions.as_str(version)
@@ -42,11 +42,11 @@ def fmt(name: str, uri: str, version: int, patient: str = BAD_PATIENT_ID):
                 if patient is None or patient == BAD_PATIENT_ID:
                     logger.error('EndpointUrl format called with invalid patient id')
                     raise EndpointFormatException('EndpointUrl format called with invalid patient id')
-                return f'{uri}/{version_as_string}/fhir/Patient/{patient}?_format=json'
+                return f'{uri}/{version_as_string}/fhir/Patient/{patient}?_format=application/fhir+json'
             case EndpointUrl.explanation_of_benefit:
-                return f'{uri}/{version_as_string}/fhir/ExplanationOfBenefit/?_format=json'
+                return f'{uri}/{version_as_string}/fhir/ExplanationOfBenefit/?_format=application/fhir+json'
             case EndpointUrl.coverage:
-                return f'{uri}/{version_as_string}/fhir/Coverage/?_format=json'
+                return f'{uri}/{version_as_string}/fhir/Coverage/?_format=application/fhir+json'
             case _:
                 logger.error(f'Could not match name in EndpointUrl: {name}')