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 8, 2023
1 parent a0a7219 commit 3927414
Show file tree
Hide file tree
Showing 20 changed files with 151 additions and 15 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
6 changes: 2 additions & 4 deletions invenio_rdm_records/records/systemfields/is_verified.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ def __init__(self, key=None):
super().__init__(key=key, use_cache=False)

def calculate(self, record):
"""Calculate the ``is_expired`` property of the request."""
"""Calculate the ``is_verified`` property of the record."""
owner = record.access.owner
if not owner:
return False
is_verified = (
owner.resolve().verified_at is not None
) # TODO property is_verified in user
is_verified = owner.resolve().verified_at is not None
return is_verified
17 changes: 17 additions & 0 deletions invenio_rdm_records/services/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@
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
from invenio_records_resources.services.records.params import (
FacetsParam,
PaginationParam,
QueryStrParam,
)
from invenio_drafts_resources.services.records.search_params import AllVersionsParam


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
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)
11 changes: 11 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1714,6 +1714,17 @@ def test_user(UserFixture, app, db):
u.create(app, db)
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):
Expand Down
26 changes: 15 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,13 @@ def test_user_moderation_approve(
mod_identity, id_=moderation_request.id, action="accept"
)

post_approval_records = fetch_user_records(uploader.id, allversions=True)
# We need to sleep for a little here because the records are re-indexed after user approval
sleep(1.25)

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
39 changes: 39 additions & 0 deletions tests/services/test_rdm_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,42 @@ def test_reindex_user_records(running_app, uploader, minimal_record, search_clea
reindex_success = service.reindex_user_records(
identity=uploader_identity, id_=uploader.id
)


def test_search_sort_verified_enabled(
running_app,
uploader,
minimal_record,
search_clear,
monkeypatch,
verified_user,
):
"""Tests sort by 'is_verified' field, when enabled.
The flag "RDM_SEARCH_SORT_BY_VERIFIED" is monkeypatched (only modified for this test).
"""
service = current_rdm_records.records_service
monkeypatch.setitem(running_app.app.config, "RDM_SEARCH_SORT_BY_VERIFIED", True)

# NV : non-verified
nv_user = uploader
# V : verified
v_user = verified_user

# Create two records for two distinct users (non-verified and verified)
nv_draft = service.create(nv_user.identity, minimal_record)
assert nv_draft
nv_record = service.publish(id_=nv_draft.id, identity=nv_user.identity)
assert nv_record

v_draft = service.create(v_user.identity, minimal_record)
assert v_draft
v_record = service.publish(id_=v_draft.id, identity=v_user.identity)
assert v_record

res = service.search(nv_user.identity)
assert res.total == 2
hits = res.to_dict()["hits"]["hits"]

expected_order = [v_record.id, nv_record.id]
assert expected_order == [h["id"] for h in hits]

0 comments on commit 3927414

Please sign in to comment.