fixes from convo with Matt

Author: clewellyn-nava

apps/fhir/bluebutton/v2/urls.py

@@ -17,13 +17,13 @@
     # Patient ReadView
     re_path(
         r'Patient/(?P<resource_id>[^/]+)',
-        PatientViewSet.as_view({'get': 'retrieve'}, version=2),
+        PatientViewSet.as_view({'get': 'read'}, version=2),
         name='bb_oauth_fhir_patient_read_or_update_or_delete_v2',
     ),
     # Patient SearchView
     re_path(
         r'Patient[/]?',
-        PatientViewSet.as_view({'get': 'list'}, version=2),
+        PatientViewSet.as_view({'get': 'search'}, version=2),
         name='bb_oauth_fhir_patient_search_v2',
     ),
     # Coverage ReadView

apps/fhir/bluebutton/v3/urls.py

@@ -19,13 +19,13 @@
     # Patient ReadView
     re_path(
         r'Patient/(?P<resource_id>[^/]+)',
-        waffle_switch('v3_endpoints')(PatientViewSet.as_view({'get': 'retrieve'}, version=3)),
+        waffle_switch('v3_endpoints')(PatientViewSet.as_view({'get': 'read'}, 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': 'list'}, version=3)),
+        waffle_switch('v3_endpoints')(PatientViewSet.as_view({'get': 'search'}, version=3)),
         name='bb_oauth_fhir_patient_search_v3',
     ),
     # Coverage ReadView

apps/fhir/bluebutton/views/insurancecard_viewset.py

@@ -24,7 +24,6 @@ class DigitalInsuranceCardViewSet(ResourceViewSet):
         viewsets: django-rest-framework ViewSet base class
     """
 
-    version = 1
     resource_type = 'Bundle'
 
     required_coverage_search_scopes = ['patient/Coverage.rs', 'patient/Coverage.s', 'patient/Coverage.read']
@@ -40,7 +39,7 @@ class DigitalInsuranceCardViewSet(ResourceViewSet):
         HasDigitalInsuranceCardScope,
     ]
 
-    def __init__(self, version=1, **kwargs):
+    def __init__(self, version, **kwargs):
         super().__init__(version)
 
     def initial(self, request, *args, **kwargs):

apps/fhir/bluebutton/views/patient_viewset.py

@@ -1,5 +1,12 @@
 from apps.fhir.bluebutton.views.viewsets_base import ResourceViewSet
-from apps.fhir.bluebutton.views.search import HasSearchScope, SearchView
+from voluptuous import (
+    Required,
+    All,
+    Match,
+    Range,
+    Coerce,
+)
+from apps.fhir.bluebutton.constants import DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE
 from apps.authorization.permissions import DataAccessGrantPermission
 from apps.capabilities.permissions import TokenHasProtectedCapability
 from apps.fhir.bluebutton.permissions import (
@@ -11,6 +18,18 @@
 from rest_framework import permissions
 
 
+class HasSearchScope(permissions.BasePermission):
+    def has_permission(self, request, view) -> bool:  # type: ignore
+        required_scopes = getattr(view, 'required_scopes', None)
+        if required_scopes is None:
+            return True
+
+        if hasattr(request, 'auth') and request.auth is not None:
+            token_scopes = request.auth.scope
+            return any(scope in token_scopes for scope in required_scopes)
+        return False
+
+
 class PatientViewSet(ResourceViewSet):
     """Patient django-rest-framework ViewSet experiment
 
@@ -19,13 +38,19 @@ class PatientViewSet(ResourceViewSet):
         viewsets: django-rest-framework ViewSet base class
     """
 
-    version = 1
     resource_type = 'Patient'
 
     # TODO - I don't love the separation here, could be indicative that we don't want to move to resource based ViewSets, or that
     # we need a better base class, or these differences should be defined in PatientViewSet.
-    SEARCH_QUERY_TRANSFORMS = getattr(SearchView, 'QUERY_TRANSFORMS', {})
-    SEARCH_QUERY_SCHEMA = {**getattr(SearchView, 'QUERY_SCHEMA', {}), '_id': str, 'identifier': str}
+    REGEX_LASTUPDATED_VALUE = r'^((lt)|(le)|(gt)|(ge)).+'
+    SEARCH_QUERY_TRANSFORMS = {
+        'count': '_count',
+    }
+    SEARCH_QUERY_SCHEMA = {
+        'startIndex': Coerce(int),
+        Required('_count', default=DEFAULT_PAGE_SIZE): All(Coerce(int), Range(min=0, max=MAX_PAGE_SIZE)),  # type: ignore
+        '_lastUpdated': [Match(REGEX_LASTUPDATED_VALUE, msg='the _lastUpdated operator is not valid')]
+    }
 
     SEARCH_PERMISSION_CLASSES = (
         permissions.IsAuthenticated,
@@ -48,6 +73,9 @@ class PatientViewSet(ResourceViewSet):
 
     required_scopes = ['patient/Patient.read', 'patient/Patient.rs', 'patient/Patient.s']
 
+    def __init__(self, version, **kwargs):
+        super().__init__(version)
+
     def build_url(self, fhir_settings, resource_type, resource_id=None, *args, **kwargs):
         if fhir_settings.fhir_url.endswith('v1/fhir/'):
             # only if called by tests

apps/fhir/bluebutton/views/viewsets_base.py

@@ -13,12 +13,11 @@ class ResourceViewSet(FhirDataView, viewsets.ViewSet):
         viewsets: django-rest-framework ViewSet base class
     """
 
-    resource_type = None
-
     SEARCH_PERMISSION_CLASSES = (permissions.IsAuthenticated,)
     READ_PERMISSION_CLASSES = (permissions.IsAuthenticated,)
 
     def initial(self, request, *args, **kwargs):
+        self.resource_type = None
         return super().initial(request, self.resource_type, *args, **kwargs)
 
     def get_permissions(self):
@@ -31,35 +30,35 @@ def get_permissions(self):
             permission_classes = (permissions.IsAuthenticated,)
         return [permission_class() for permission_class in permission_classes]
 
-    def list(self, request, *args, **kwargs):
+    def search(self, request, *args, **kwargs):
         out = self.fetch_data(request, self.resource_type, *args, **kwargs)
         return Response(out)
 
-    def retrieve(self, request, resource_id, *args, **kwargs):
+    def read(self, request, resource_id, *args, **kwargs):
         out = self.fetch_data(request, self.resource_type, resource_id=resource_id, *args, **kwargs)
         return Response(out)
 
     # A lot of this is copied (haphazardly) from generic, and the names and handling of the schema could be cleaned up
-    def get_query_schema(self):
-        if getattr(self, 'action', None) == 'list':
-            return getattr(self, 'SEARCH_QUERY_SCHEMA', getattr(self, 'QUERY_SCHEMA', {}))
+    def get_query_schema(self) -> dict:
+        if getattr(self, 'action', None) == 'search':
+            return getattr(self, 'SEARCH_QUERY_SCHEMA', getattr(self, 'READ_QUERY_TRANSFORMS', {}))
         return {}
 
-    def get_query_transforms(self):
-        if getattr(self, 'action', None) == 'list':
-            return getattr(self, 'SEARCH_QUERY_TRANSFORMS', getattr(self, 'QUERY_TRANSFORMS', {}))
-        return getattr(self, 'QUERY_TRANSFORMS', {})
+    def get_query_transforms(self) -> dict:
+        if getattr(self, 'action', None) == 'search':
+            return getattr(self, 'SEARCH_QUERY_TRANSFORMS', getattr(self, 'READ_QUERY_TRANSFORMS', {}))
+        return {}
 
-    def map_parameters(self, params):
+    def map_parameters(self, params) -> dict:
         transforms = self.get_query_transforms()
         for key, correct in transforms.items():
             val = params.pop(key, None)
             if val is not None:
                 params[correct] = val
         return params
 
-    def filter_parameters(self, request):
-        if getattr(self, 'action', None) != 'list':
+    def filter_parameters(self, request) -> dict:
+        if getattr(self, 'action', None) != 'search':
             return {}
 
         params = self.map_parameters(request.query_params.dict())