Skip to content
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

[ENG-6884] Fix filter for PreprintProviderWithdrawRequestList view and refactored versioned target list filter #10910

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/actions/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
53 changes: 29 additions & 24 deletions api/base/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__'):
Expand Down Expand Up @@ -614,52 +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`.
"""Handles a special case when filtering on `target` when `target` is a Preprint.
"""
if field_name == 'target':
PreprintActionFilterMixin.postprocess_versioned_guid_target_query_param(operation)
referent, version = Guid.load_referent(operation['value'])
if referent:
if version:
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 ReviewActionFilterMixin(ListFilterMixin):
"""View mixin for `ReviewActionListCreate`. 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)
4 changes: 2 additions & 2 deletions api/preprints/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions api/providers/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
InvalidFilterOperator,
InvalidFilterValue,
)
from api.base.filters import PreprintFilterMixin, ListFilterMixin
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
Expand Down Expand Up @@ -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, ListFilterMixin, ProviderMixin):
class PreprintProviderWithdrawRequestList(JSONAPIBaseView, generics.ListAPIView, PreprintAsTargetFilterMixin, ProviderMixin):
provider_class = PreprintProvider
permission_classes = (
drf_permissions.IsAuthenticated,
Expand Down
Loading