Refactor versioned target filter mixin

api/actions/views.py

@@ -6,7 +6,7 @@
 from api.actions.permissions import ReviewActionPermission
 from api.actions.serializers import NodeRequestActionSerializer, ReviewActionSerializer, PreprintRequestActionSerializer
 from api.base.exceptions import Conflict
-from api.base.filters import ReviewActionFilterMixin
+from api.base.filters import TargetFilterMixin
 from api.base.views import JSONAPIBaseView
 from api.base.parsers import (
     JSONAPIMultipleRelationshipsParser,
@@ -110,7 +110,7 @@ def get_object(self):
         return action
 
 
-class ReviewActionListCreate(JSONAPIBaseView, generics.ListCreateAPIView, ReviewActionFilterMixin):
+class ReviewActionListCreate(JSONAPIBaseView, generics.ListCreateAPIView, TargetFilterMixin):
     """List of review actions viewable by this user
 
     Actions represent state changes and/or comments on a reviewable object (e.g. a preprint)

api/base/filters.py

@@ -21,6 +21,7 @@
 from osf.models import Subject, Preprint
 from osf.models.base import GuidMixin, Guid
 from functools import cmp_to_key
+from framework import sentry
 
 def lowercase(lower):
     if hasattr(lower, '__call__'):
@@ -614,74 +615,56 @@ def preprints_queryset(self, base_queryset, auth_user, allow_contribs=True, publ
         return preprints
 
 
-class PreprintActionFilterMixin(ListFilterMixin):
-    """View mixin for `PreprintActionList`. It inherits from `ListFilterMixin` and customize postprocessing for
-    versioned preprint.
+class TargetFilterMixin(ListFilterMixin):
+    """View mixin for multi-content-type list views (e.g. `ReviewActionListCreate`). It inherits from `ListFilterMixin`
+     and customizes the postprocessing of the `target` field when target is a preprint.
 
     Note: Subclasses must define `get_default_queryset()`.
     """
 
     @staticmethod
-    def postprocess_versioned_guid_target_query_param(operation):
-        """When target is a preprint, which must be versioned, the traditional non-versioned `guid___id==target`
-        look-up no longer works. Must convert to PK look-up `referent__id==pk`.
+    def postprocess_preprint_as_target_query_param(operation, target_pk):
+        """When target is a preprint, which must be versioned, the traditional non-versioned `guid___id==_id`
+        look-up no longer works. Must convert it to PK look-up `target__id==pk`.
         """
-        referent, version = Guid.load_referent(operation['value'])
-        # A valid preprint must have referent and version
-        if not referent or not version:
-            return
-        # Override the operation to filter  `target__id=target.pk`
+        # Override the operation to filter `target__id==pk`
         operation['source_field_name'] = 'target__id'
-        operation['value'] = referent.id
+        operation['value'] = target_pk
         operation['op'] = 'eq'
 
     def postprocess_query_param(self, key, field_name, operation):
-        """Handles a special case when filtering on `target`.
-        """
-        if field_name == 'target':
-            PreprintActionFilterMixin.postprocess_versioned_guid_target_query_param(operation)
-        else:
-            super().postprocess_query_param(key, field_name, operation)
-
-class ReviewActionFilterMixin(ListFilterMixin):
-    """View mixin for `ReviewActionListCreate`. It inherits from `ListFilterMixin` and uses `PreprintActionFilterMixin`
-    to customized postprocessing for handling versioned preprint.
-
-    Note: Subclasses must define `get_default_queryset()`.
-    """
-    def postprocess_query_param(self, key, field_name, operation):
-        """Handles a special case when filtering on `target` and when `target` is a versioned Preprint.
+        """Handles a special case when filtering on `target` when `target` is a Preprint.
         """
         if field_name == 'target':
             referent, version = Guid.load_referent(operation['value'])
             if referent:
                 if version:
-                    PreprintActionFilterMixin.postprocess_versioned_guid_target_query_param(operation)
+                    TargetFilterMixin.postprocess_preprint_as_target_query_param(operation, referent.id)
                 else:
                     super().postprocess_query_param(key, field_name, operation)
             else:
+                sentry.log_message(f'Target object invalid or not found: [target={operation['value']}]')
                 return
         else:
             super().postprocess_query_param(key, field_name, operation)
 
 
-class PreprintProviderWithdrawRequestFilterMixin(ListFilterMixin):
-    """View mixin for `PreprintProviderWithdrawRequestList`. It inherits from `ListFilterMixin` and uses
-    `PreprintActionFilterMixin` to customized postprocessing for handling versioned preprint.
+class PreprintAsTargetFilterMixin(TargetFilterMixin):
+    """View mixin for preprint related list views (e.g. `PreprintProviderWithdrawRequestList` and `PreprintActionList`).
+    It inherits from `TargetFilterMixin` and customizes postprocessing the `target` field for preprint.
 
     Note: Subclasses must define `get_default_queryset()`.
     """
+
     def postprocess_query_param(self, key, field_name, operation):
-        """Handles a special case when filtering on `target` and when `target` is a versioned Preprint.
+        """Handles a special case when filtering on `target`.
         """
         if field_name == 'target':
             referent, version = Guid.load_referent(operation['value'])
-            if referent:
-                if version:
-                    PreprintActionFilterMixin.postprocess_versioned_guid_target_query_param(operation)
-                else:
-                    super().postprocess_query_param(key, field_name, operation)
-            else:
+            # A valid preprint must have referent and version
+            if not referent or not version:
+                sentry.log_message(f'Preprint invalid or note found: [target={operation['value']}]')
                 return
+            TargetFilterMixin.postprocess_preprint_as_target_query_param(operation, referent.id)
         else:
             super().postprocess_query_param(key, field_name, operation)

api/preprints/views.py

@@ -22,7 +22,7 @@
 from api.base.pagination import PreprintContributorPagination
 from api.base.exceptions import Conflict
 from api.base.views import JSONAPIBaseView, WaterButlerMixin
-from api.base.filters import ListFilterMixin, PreprintFilterMixin, PreprintActionFilterMixin
+from api.base.filters import ListFilterMixin, PreprintAsTargetFilterMixin, PreprintFilterMixin
 from api.base.parsers import (
     JSONAPIMultipleRelationshipsParser,
     JSONAPIMultipleRelationshipsParserForRegularJSON,
@@ -590,7 +590,7 @@ def get_object(self):
         return obj
 
 
-class PreprintActionList(JSONAPIBaseView, generics.ListCreateAPIView, PreprintActionFilterMixin, PreprintMixin):
+class PreprintActionList(JSONAPIBaseView, generics.ListCreateAPIView, PreprintAsTargetFilterMixin, PreprintMixin):
     """Action List *Read-only*
 
     Actions represent state changes and/or comments on a reviewable object (e.g. a preprint)

api/providers/views.py

@@ -15,7 +15,7 @@
     InvalidFilterOperator,
     InvalidFilterValue,
 )
-from api.base.filters import ListFilterMixin, PreprintFilterMixin, PreprintProviderWithdrawRequestFilterMixin
+from api.base.filters import ListFilterMixin, PreprintAsTargetFilterMixin, PreprintFilterMixin
 from api.base.metrics import PreprintMetricsViewMixin
 from api.base.pagination import MaxSizePagination, IncreasedPageSizePagination
 from api.base.settings import BULK_SETTINGS
@@ -571,7 +571,7 @@ def perform_create(self, serializer):
         raise ValidationError(f'Provider {provider.name} has no primary collection to submit to.')
 
 
-class PreprintProviderWithdrawRequestList(JSONAPIBaseView, generics.ListAPIView, PreprintProviderWithdrawRequestFilterMixin, ProviderMixin):
+class PreprintProviderWithdrawRequestList(JSONAPIBaseView, generics.ListAPIView, PreprintAsTargetFilterMixin, ProviderMixin):
     provider_class = PreprintProvider
     permission_classes = (
         drf_permissions.IsAuthenticated,