Skip to content

Commit

Permalink
search: added 'verified' field sort option.
Browse files Browse the repository at this point in the history
  • Loading branch information
alejandromumo committed Aug 9, 2023
1 parent 9d5de7f commit e9383c1
Show file tree
Hide file tree
Showing 22 changed files with 156 additions and 71 deletions.
3 changes: 3 additions & 0 deletions invenio_rdm_records/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ def always_valid(identifier):
},
}

RDM_SEARCH_SORT_BY_VERIFIED = False
"""Sort records by 'verified' first."""

RDM_SORT_OPTIONS = {
"bestmatch": dict(
title=_("Best match"),
Expand Down
2 changes: 2 additions & 0 deletions invenio_rdm_records/records/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
PIDStatusCheckField,
)
from invenio_requests.records.api import Request
from invenio_requests.records.dumpers import CalculatedFieldDumperExt
from invenio_requests.records.systemfields.relatedrecord import RelatedRecord
from invenio_vocabularies.contrib.affiliations.api import Affiliation
from invenio_vocabularies.contrib.awards.api import Award
Expand Down Expand Up @@ -67,6 +68,7 @@ class RDMParent(ParentRecordBase):
dumper = SearchDumper(
extensions=[
GrantTokensDumperExt("access.grant_tokens"),
CalculatedFieldDumperExt("is_verified"),
]
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
"id" : {
"type" : "keyword"
},
"is_verified": {
"type": "boolean"
},
"access": {
"properties" : {
"owned_by" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
"id" : {
"type" : "keyword"
},
"is_verified": {
"type": "boolean"
},
"access": {
"properties" : {
"owned_by" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@
}
}
},
"is_verified": {
"type": "boolean"
},
"access": {
"properties" : {
"owned_by" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@
}
}
},
"is_verified": {
"type": "boolean"
},
"created": {
"type": "date"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
"id" : {
"type" : "keyword"
},
"is_verified": {
"type": "boolean"
},
"access": {
"properties" : {
"owned_by" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
"id" : {
"type" : "keyword"
},
"is_verified": {
"type": "boolean"
},
"access": {
"properties" : {
"owned_by" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@
}
}
},
"is_verified": {
"type": "boolean"
},
"access": {
"properties" : {
"owned_by" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@
}
}
},
"is_verified": {
"type": "boolean"
},
"created": {
"type": "date"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
"id" : {
"type" : "keyword"
},
"is_verified": {
"type": "boolean"
},
"access": {
"properties" : {
"owned_by" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
"id" : {
"type" : "keyword"
},
"is_verified": {
"type": "boolean"
},
"access": {
"properties" : {
"owned_by" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@
}
}
},
"is_verified": {
"type": "boolean"
},
"access": {
"properties" : {
"owned_by" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@
}
}
},
"is_verified": {
"type": "boolean"
},
"created": {
"type": "date"
},
Expand Down
9 changes: 3 additions & 6 deletions invenio_rdm_records/records/systemfields/is_verified.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,8 @@ def __init__(self, key=None):
super().__init__(key=key, use_cache=False)

def calculate(self, record):
"""Calculate the ``is_expired`` property of the request."""
owner = record.access.owner
"""Calculate the ``is_verified`` property of the record."""
owner = record.access.owner.resolve()
if not owner:
return False
is_verified = (
owner.resolve().verified_at is not None
) # TODO property is_verified in user
return is_verified
return owner.verified_at is not None
9 changes: 6 additions & 3 deletions invenio_rdm_records/requests/user_moderation/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@ def on_restore(user_id, uow=None, **kwargs):
pass


def on_approve(user_id, **kwargs):
def on_approve(user_id, uow=None, **kwargs):
"""Execute on user approve.
Re-index user records and dump verified field into records.
"""
current_rdm_records_service.reindex_user_records(
identity=system_identity, id_=user_id
from invenio_search.engine import dsl

user_records_q = dsl.Q("term", **{"parent.access.owned_by.user": user_id})
current_rdm_records_service.reindex_latest_first(
identity=system_identity, extra_filter=user_records_q, uow=uow
)
17 changes: 17 additions & 0 deletions invenio_rdm_records/services/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
is_draft,
is_record,
)
from invenio_drafts_resources.services.records.search_params import AllVersionsParam
from invenio_indexer.api import RecordIndexer
from invenio_records_resources.services import ConditionalLink, FileServiceConfig
from invenio_records_resources.services.base.config import (
Expand All @@ -47,6 +48,11 @@
RecordLink,
pagination_links,
)
from invenio_records_resources.services.records.params import (
FacetsParam,
PaginationParam,
QueryStrParam,
)
from invenio_requests.services.requests import RequestItem, RequestList
from invenio_requests.services.requests.config import RequestSearchOptions
from requests import Request
Expand Down Expand Up @@ -75,6 +81,7 @@
from .schemas.parent.access import Grant as GrantSchema
from .schemas.parent.access import SecretLink as SecretLinkSchema
from .schemas.record_communities import RecordCommunitiesSchema
from .sort import VerifiedRecordsSortParam


def is_draft_and_has_review(record, ctx):
Expand Down Expand Up @@ -126,6 +133,16 @@ class RDMSearchOptions(SearchOptions, SearchOptionsMixin):
"access_status": facets.access_status,
}

# TODO we could do SearchOptions.params_interpreters_cls
# TODO but SortParams would be duplicated (e.g. search.sort() would be set twice)
params_interpreters_cls = [
AllVersionsParam.factory("versions.is_latest"),
QueryStrParam,
PaginationParam,
FacetsParam,
VerifiedRecordsSortParam,
]


class RDMSearchDraftsOptions(SearchDraftsOptions, SearchOptionsMixin):
"""Search options for drafts search."""
Expand Down
16 changes: 0 additions & 16 deletions invenio_rdm_records/services/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,3 @@ def oai_result_item(self, identity, oai_record_source):
record,
links_tpl=self.links_item_tpl,
)

@unit_of_work()
def reindex_user_records(self, identity, id_, uow=None):
"""Reindexes all the records from a given user."""
self.require_permission(identity, "manage")

user_records_q = f"parent.access.owned_by.user:{id_}"
params = {"allversions": True, "q": user_records_q}

records = self.scan(identity, params=params)

for record in records:
record, parent = self._get_record_and_parent_by_id(record["id"])
self._index_related_records(record, parent, uow=uow)

return True
26 changes: 26 additions & 0 deletions invenio_rdm_records/services/sort.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# -*- coding: utf-8 -*-
#
# Copyright (C) 2023 CERN.
#
# Invenio-RDM-Records is free software; you can redistribute it and/or modify
# it under the terms of the MIT License; see LICENSE file for more details.
"""Sort parameter interpreter API."""

from flask import current_app
from invenio_records_resources.services.records.params.sort import SortParam


class VerifiedRecordsSortParam(SortParam):
"""Evaluate the 'sort' parameter for RDM records."""

def apply(self, identity, search, params):
"""Evaluate the sort parameter on the search.
If the config "RDM_SEARCH_SORT_BY_VERIFIED" is set, then all the sorting is
prepended by the record's `is_verified` property.
"""
if current_app.config["RDM_SEARCH_SORT_BY_VERIFIED"]:
fields = self._compute_sort_fields(params)
fields.insert(0, "-parent.is_verified")
return search.sort(*fields)
return super().apply(identity, search, params)
14 changes: 13 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@
from invenio_requests.notifications.builders import (
CommentRequestEventCreateNotificationBuilder,
)
from invenio_users_resources.proxies import current_users_service
from invenio_requests.proxies import current_user_moderation_service as mod_service
from invenio_users_resources.permissions import user_management_action
from invenio_users_resources.proxies import current_users_service
from invenio_users_resources.records.api import UserAggregate
from invenio_users_resources.services.schemas import (
NotificationPreferences,
Expand Down Expand Up @@ -1618,6 +1618,18 @@ def test_user(UserFixture, app, db, index_users):
return u


@pytest.fixture()
def verified_user(UserFixture, app, db):
"""User meant to test 'verified' property of records."""
u = UserFixture(
email="testuser@inveniosoftware.org",
password="testuser",
)
u.create(app, db)
u.user.verified_at = datetime.utcnow()
return u


@pytest.fixture()
def uploader(UserFixture, app, db, index_users):
"""Uploader."""
Expand Down
27 changes: 16 additions & 11 deletions tests/requests/test_user_moderation_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,29 @@
# # Invenio-RDM is free software; you can redistribute it and/or modify
# # it under the terms of the MIT License; see LICENSE file for more details.
"""Test user moderation actions."""
from time import sleep

from invenio_access.permissions import system_identity
from invenio_requests.proxies import current_requests_service

from invenio_rdm_records.proxies import current_rdm_records_service as records_service


def fetch_user_records(user_id, allversions=False):
"""Fetches records from a given user."""
user_records_q = f"parent.access.owned_by.user:{user_id}"
return records_service.search(
system_identity, params={"q": user_records_q, "allversions": allversions}
)


def test_user_moderation_approve(
running_app,
mod_request_create,
mod_identity,
uploader,
test_user,
es_clear,
minimal_record,
):
"""Tests moderation action after user approval.
The user records, and all its versions, should have a "is_verified" field set to "True".
"""

user_records_q = f"parent.access.owned_by.user:{uploader.id}"

# Create a record
draft = records_service.create(uploader.identity, minimal_record)
record = records_service.publish(id_=draft.id, identity=uploader.identity)
Expand All @@ -42,7 +37,10 @@ def test_user_moderation_approve(
records_service.update_draft(uploader.identity, new_version.id, minimal_record)
new_record = records_service.publish(identity=uploader.identity, id_=new_version.id)

pre_approval_records = fetch_user_records(uploader.id, allversions=True)
pre_approval_records = records_service.search(
system_identity, params={"q": user_records_q, "allversions": True}
)

assert pre_approval_records.total == 2
hits = pre_approval_records.to_dict()["hits"]["hits"]
is_verified = all([hit["parent"]["is_verified"] for hit in hits])
Expand All @@ -56,7 +54,14 @@ def test_user_moderation_approve(
mod_identity, id_=moderation_request.id, action="accept"
)

post_approval_records = fetch_user_records(uploader.id, allversions=True)
# Records are re-indexed in bulk. We need to force it for tests, otherwise it's done by a celery beat task
records_service.indexer.process_bulk_queue()
records_service.record_cls.index.refresh()

post_approval_records = records_service.search(
system_identity, params={"q": user_records_q, "allversions": True}
)

assert post_approval_records.total == 2
hits = post_approval_records.to_dict()["hits"]["hits"]
is_verified = all([hit["parent"]["is_verified"] for hit in hits])
Expand Down
Loading

0 comments on commit e9383c1

Please sign in to comment.