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-6878] Fix API filter for versioned guids/preprints #10905

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 ListFilterMixin
from api.base.filters import ReviewActionFilterMixin
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, ListFilterMixin):
class ReviewActionListCreate(JSONAPIBaseView, generics.ListCreateAPIView, ReviewActionFilterMixin):
"""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
104 changes: 63 additions & 41 deletions api/base/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,39 +291,7 @@ def parse_query_params(self, query_params):
query.get(key).update({
field_name: self._parse_date_param(field, source_field_name, op, value),
})
elif not isinstance(value, int) and source_field_name in ['_id', 'guid._id']:
# TODO: this is broken since value can be a multi-value str separated by comma; in addition,
# as for a single-versioned-guid value, the filter is `or` but we need `and`; this will
# be fixed in [ENG-6878](https://openscience.atlassian.net/browse/ENG-6878).
base_guid, version = Guid.split_guid(value)
if base_guid is None and version is None:
raise InvalidFilterValue(
value=value,
field_type='guid',
)
guid_filters = {
field_name: {
'op': 'in',
'value': self.bulk_get_values(value, field),
'source_field_name': source_field_name,
},
}
if version is not None:
guid_filters = {
field_name: {
'op': 'eq',
'value': base_guid,
'source_field_name': 'versioned_guids___id',
},
'versioned_guids__version': {
'op': 'eq',
'value': version,
'source_field_name': 'versioned_guids__version',
},
}

query.get(key).update(guid_filters)
elif not isinstance(value, int) and source_field_name in ['journal_id', 'moderation_state']:
elif not isinstance(value, int) and source_field_name in ['_id', 'guid._id', 'journal_id', 'moderation_state']:
query.get(key).update({
field_name: {
'op': 'in',
Expand Down Expand Up @@ -600,25 +568,28 @@ def get_serializer_method(self, field_name):


class PreprintFilterMixin(ListFilterMixin):
"""View mixin that uses ListFilterMixin, adding postprocessing for preprint querying
"""View mixin for many preprint listing views. It inherits from ListFilterMixin and customize postprocessing for
preprint querying by provider, subjects and versioned `_id`.

Subclasses must define `get_default_queryset()`.
Note: Subclasses must define `get_default_queryset()`.
"""

@staticmethod
def postprocess_versioned_guid_id_query_param(operation):
"""Handle query parameters when filtering on `id` for preprint and versioned guid. With versioned guid,
preprint no longer has guid._id for every version, and thus must switch to filter by pk.
"""Handle query parameters when filtering on `_id` for preprint which are now versioned.

Note: preprint achieves versioning by using versioned guid, and thus no long has guid or guid._id for every
version. Must convert `guid___id__in=` look-up to `pk__in` look-up.
"""
object_ids = []
preprint_pk_list = []
for val in operation['value']:
referent, version = Guid.load_referent(val)
if referent is None:
continue
object_ids.append(referent.id)
# Override the operation to filter `id__in=object_ids`
preprint_pk_list.append(referent.id)
# Override the operation to filter `id__in=preprint_pk_list`
operation['source_field_name'] = 'id__in'
operation['value'] = object_ids
operation['value'] = preprint_pk_list
operation['op'] = 'eq'

def postprocess_query_param(self, key, field_name, operation):
Expand All @@ -641,3 +612,54 @@ def preprints_queryset(self, base_queryset, auth_user, allow_contribs=True, publ
if latest_only:
preprints = preprints.filter(pk__in=[obj.pk for obj in preprints if obj.is_latest_version])
return preprints


class PreprintActionFilterMixin(ListFilterMixin):
"""View mixin for `PreprintActionList`. It inherits from `ListFilterMixin` and customize postprocessing for
versioned 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`.
"""
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`
operation['source_field_name'] = 'target__id'
operation['value'] = referent.id
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.
"""
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:
return
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 @@ -21,7 +21,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
from api.base.filters import ListFilterMixin, PreprintFilterMixin, PreprintActionFilterMixin
from api.base.parsers import (
JSONAPIMultipleRelationshipsParser,
JSONAPIMultipleRelationshipsParserForRegularJSON,
Expand Down Expand Up @@ -587,7 +587,7 @@ def get_object(self):
return obj


class PreprintActionList(JSONAPIBaseView, generics.ListCreateAPIView, ListFilterMixin, PreprintMixin):
class PreprintActionList(JSONAPIBaseView, generics.ListCreateAPIView, PreprintActionFilterMixin, PreprintMixin):
"""Action List *Read-only*

Actions represent state changes and/or comments on a reviewable object (e.g. a preprint)
Expand Down
Loading