diff --git a/invenio_rdm_records/config.py b/invenio_rdm_records/config.py index 80ee5d106..7dfbd5bb0 100644 --- a/invenio_rdm_records/config.py +++ b/invenio_rdm_records/config.py @@ -13,7 +13,13 @@ from datetime import timedelta import idutils +from invenio_administration.permissions import administration_permission from invenio_i18n import lazy_gettext as _ +from invenio_records_resources.services.records.queryparser import QueryParser +from invenio_records_resources.services.records.queryparser.transformer import ( + RestrictedTerm, + SearchFieldTransformer, +) import invenio_rdm_records.services.communities.moderation as communities_moderation from invenio_rdm_records.services.components.verified import UserModerationHandler @@ -24,6 +30,7 @@ from .services.config import lock_edit_published_files from .services.permissions import RDMRecordPermissionPolicy from .services.pids import providers +from .services.queryparser import word_internal_notes # Invenio-RDM-Records # =================== @@ -254,6 +261,7 @@ def always_valid(identifier): """ + RDM_SEARCH = { "facets": ["access_status", "file_type", "resource_type"], "sort": [ @@ -264,6 +272,18 @@ def always_valid(identifier): "mostviewed", "mostdownloaded", ], + "query_parser_cls": QueryParser.factory( + mapping={ + "internal_notes.note": RestrictedTerm(administration_permission), + "internal_notes.id": RestrictedTerm(administration_permission), + "internal_notes.added_by": RestrictedTerm(administration_permission), + "internal_notes.timestamp": RestrictedTerm(administration_permission), + "_exists_": RestrictedTerm( + administration_permission, word=word_internal_notes + ), + }, + tree_transformer_cls=SearchFieldTransformer, + ), } """Record search configuration. diff --git a/invenio_rdm_records/records/jsonschemas/records/record-v6.0.0.json b/invenio_rdm_records/records/jsonschemas/records/record-v6.0.0.json index 16d96fd58..592f5db23 100644 --- a/invenio_rdm_records/records/jsonschemas/records/record-v6.0.0.json +++ b/invenio_rdm_records/records/jsonschemas/records/record-v6.0.0.json @@ -341,22 +341,6 @@ } } } - }, - "_internal_notes": { - "properties": { - "note": { - "type": "string" - }, - "timestamp": { - "type": "string", - "description": "ISO8601 formatted timestamp in UTC.", - "format": "date-time" - }, - "user": { - "type": "string", - "description": "User id of the person who created the note" - } - } } } }, @@ -395,6 +379,24 @@ } } }, + "internal_notes": { + "properties": { + "id": { + "type": "string" + }, + "note": { + "type": "string" + }, + "timestamp": { + "type": "string", + "description": "ISO8601 formatted timestamp in UTC.", + "format": "date-time" + }, + "added_by": { + "$ref": "local://records/definitions-v2.0.0.json#/agent" + } + } + }, "provenance": { "type": "object", "description": "Record provenance.", diff --git a/invenio_rdm_records/records/mappings/os-v2/rdmrecords/drafts/draft-v6.0.0.json b/invenio_rdm_records/records/mappings/os-v2/rdmrecords/drafts/draft-v6.0.0.json index 3c532097e..0d93f5410 100644 --- a/invenio_rdm_records/records/mappings/os-v2/rdmrecords/drafts/draft-v6.0.0.json +++ b/invenio_rdm_records/records/mappings/os-v2/rdmrecords/drafts/draft-v6.0.0.json @@ -1571,6 +1571,28 @@ "type": "boolean" } } + }, + "internal_notes": { + "type": "object", + "properties": { + "id": { + "type": "keyword" + }, + "note": { + "type": "text" + }, + "timestamp": { + "type": "date" + }, + "added_by": { + "type": "object", + "properties": { + "user": { + "type": "keyword" + } + } + } + } } } } diff --git a/invenio_rdm_records/records/mappings/os-v2/rdmrecords/records/record-v7.0.0.json b/invenio_rdm_records/records/mappings/os-v2/rdmrecords/records/record-v7.0.0.json index 9291eec90..de3b0a3de 100644 --- a/invenio_rdm_records/records/mappings/os-v2/rdmrecords/records/record-v7.0.0.json +++ b/invenio_rdm_records/records/mappings/os-v2/rdmrecords/records/record-v7.0.0.json @@ -45,8 +45,13 @@ "accent_analyzer": { "tokenizer": "standard", "type": "custom", - "char_filter": ["strip_special_chars"], - "filter": ["lowercase", "asciifolding"] + "char_filter": [ + "strip_special_chars" + ], + "filter": [ + "lowercase", + "asciifolding" + ] } } } @@ -703,19 +708,6 @@ "type": "object", "enabled": false }, - "_internal_notes": { - "properties": { - "note": { - "type": "text" - }, - "timestamp": { - "type": "date" - }, - "user": { - "type": "keyword" - } - } - }, "contact": { "type": "keyword" }, @@ -1595,6 +1587,28 @@ "type": "boolean" } } + }, + "internal_notes": { + "type": "object", + "properties": { + "id": { + "type": "keyword" + }, + "note": { + "type": "text" + }, + "timestamp": { + "type": "date" + }, + "added_by": { + "type": "object", + "properties": { + "user": { + "type": "keyword" + } + } + } + } } } } diff --git a/invenio_rdm_records/services/components/__init__.py b/invenio_rdm_records/services/components/__init__.py index 09c52bee0..1bd491a7d 100644 --- a/invenio_rdm_records/services/components/__init__.py +++ b/invenio_rdm_records/services/components/__init__.py @@ -16,6 +16,7 @@ from .access import AccessComponent from .custom_fields import CustomFieldsComponent +from .internal_notes import InternalNotesComponent from .metadata import MetadataComponent from .pids import ParentPIDsComponent, PIDsComponent from .record_deletion import RecordDeletionComponent @@ -40,6 +41,7 @@ RelationsComponent, ReviewComponent, ContentModerationComponent, + InternalNotesComponent, ] diff --git a/invenio_rdm_records/services/components/internal_notes.py b/invenio_rdm_records/services/components/internal_notes.py new file mode 100644 index 000000000..d249965a2 --- /dev/null +++ b/invenio_rdm_records/services/components/internal_notes.py @@ -0,0 +1,68 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2024 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. + +"""RDM service component for custom fields.""" +import uuid +from copy import copy, deepcopy +from datetime import datetime, timezone +from invenio_drafts_resources.services.records.components import ServiceComponent + + +class InternalNotesComponent(ServiceComponent): + """Service component for custom fields.""" + + field = "internal_notes" + + def create(self, identity, data=None, record=None, **kwargs): + """Inject note to the record.""" + notes = data.get(self.field, []) + for note in notes: + note.update( + { + "added_by": {"user": identity.id}, + "timestamp": datetime.now(timezone.utc).isoformat(), + "id": str(uuid.uuid4()), + } + ) + record.update({"internal_notes": notes}) + + def update_draft(self, identity, data=None, record=None, **kwargs): + """Inject parsed metadata to the record.""" + notes_to_update = data.get(self.field, []) + notes = deepcopy(record.get(self.field, [])) + ids_to_check = [note["id"] for note in notes] + + # reverse list to keep proper iterator reference after remove + for note in reversed(notes_to_update): + note.setdefault("id", str(uuid.uuid4())) + if note["id"] in ids_to_check: + # exclude existing IDs from incoming data + # because we don't modify existing notes + notes_to_update.remove(note) + ids_to_check.remove(note["id"]) + continue + note.setdefault("added_by", {"user": identity.id}) + note.setdefault("timestamp", datetime.now(timezone.utc).isoformat()) + + to_delete = ids_to_check + for note in notes: + if note["id"] in to_delete: + notes.remove(note) + + record.update({"internal_notes": notes + notes_to_update}) + + def publish(self, identity, draft=None, record=None, **kwargs): + """Update draft metadata.""" + record.update({"internal_notes": draft.get(self.field, [])}) + + def edit(self, identity, draft=None, record=None, **kwargs): + """Update draft metadata.""" + draft.update({"internal_notes": record.get(self.field, [])}) + + def new_version(self, identity, draft=None, record=None, **kwargs): + """Update draft metadata.""" + draft.update({"internal_notes": copy(record.get(self.field, []))}) diff --git a/invenio_rdm_records/services/config.py b/invenio_rdm_records/services/config.py index b95a54d52..60ed81a1f 100644 --- a/invenio_rdm_records/services/config.py +++ b/invenio_rdm_records/services/config.py @@ -18,6 +18,7 @@ from pathlib import Path from flask import current_app +from invenio_administration.permissions import administration_permission from invenio_communities.communities.records.api import Community from invenio_drafts_resources.services.records.components import ( DraftMediaFilesComponent, @@ -64,6 +65,13 @@ PaginationParam, QueryStrParam, ) +from invenio_records_resources.services.records.queryparser import ( + QueryParser, + SearchFieldTransformer, +) +from invenio_records_resources.services.records.queryparser.transformer import ( + RestrictedTerm, +) from invenio_requests.services.requests import RequestItem, RequestList from invenio_requests.services.requests.config import RequestSearchOptions from requests import Request diff --git a/invenio_rdm_records/services/permissions.py b/invenio_rdm_records/services/permissions.py index 5cc3243cb..d77ccafb5 100644 --- a/invenio_rdm_records/services/permissions.py +++ b/invenio_rdm_records/services/permissions.py @@ -61,6 +61,9 @@ class RDMRecordPermissionPolicy(RecordPermissionPolicy): "object-read": "read_files", } + # permission meant for global curators of the instance + # (for now applies to internal notes field only + can_manage_internal = [Administration()] # # High-level permissions (used by low-level) # diff --git a/invenio_rdm_records/services/queryparser.py b/invenio_rdm_records/services/queryparser.py new file mode 100644 index 000000000..f5edf58f9 --- /dev/null +++ b/invenio_rdm_records/services/queryparser.py @@ -0,0 +1,16 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2024 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. + +"""Query Parser module for InvenioRdmRecords.""" +from luqum.tree import Word + + +def word_internal_notes(node): + """Rewrite the internal notes.""" + if not node.value.startswith("internal_notes"): + return node + return Word(" ") diff --git a/invenio_rdm_records/services/schemas/metadata.py b/invenio_rdm_records/services/schemas/metadata.py index dd08ca1eb..ab2fdf3d7 100644 --- a/invenio_rdm_records/services/schemas/metadata.py +++ b/invenio_rdm_records/services/schemas/metadata.py @@ -8,7 +8,6 @@ # it under the terms of the MIT License; see LICENSE file for more details. """RDM record schemas.""" -from datetime import timezone from functools import partial from urllib import parse @@ -35,12 +34,11 @@ EDTFDateTimeString, IdentifierSet, SanitizedHTML, - SanitizedUnicode, TZDateTime, + SanitizedUnicode, ) from marshmallow_utils.schemas import GeometryObjectSchema, IdentifierSchema from werkzeug.local import LocalProxy -from invenio_rdm_records.services.schemas.parent.access import Agent record_personorg_schemes = LocalProxy( lambda: current_app.config["RDM_RECORDS_PERSONORG_SCHEMES"] @@ -353,13 +351,6 @@ class FeatureSchema(Schema): features = fields.List(fields.Nested(LocationSchema)) -class InternalNoteSchema(Schema): - """Schema for internal notes.""" - timestamp = TZDateTime(timezone=timezone.utc, format="iso", dump_only=True) - user = fields.Nested(Agent, dump_only=True) - note = SanitizedUnicode() - - class MetadataSchema(Schema): """Schema for the record metadata.""" @@ -398,4 +389,3 @@ class MetadataSchema(Schema): locations = fields.Nested(FeatureSchema) funding = fields.List(fields.Nested(FundingSchema)) references = fields.List(fields.Nested(ReferenceSchema)) - _internal_notes = fields.List(fields.Nested(InternalNoteSchema)) diff --git a/invenio_rdm_records/services/schemas/record.py b/invenio_rdm_records/services/schemas/record.py index 57c69c91e..9abcb7c30 100644 --- a/invenio_rdm_records/services/schemas/record.py +++ b/invenio_rdm_records/services/schemas/record.py @@ -9,21 +9,29 @@ # it under the terms of the MIT License; see LICENSE file for more details. """RDM record schemas.""" - +from datetime import datetime, timezone from functools import partial from flask import current_app from invenio_drafts_resources.services.records.schema import RecordSchema from invenio_i18n import lazy_gettext as _ +from invenio_pidstore.providers.recordid_v2 import RecordIdProviderV2 from invenio_records_resources.services.custom_fields import CustomFieldsSchema -from marshmallow import EXCLUDE, ValidationError, fields, post_dump -from marshmallow_utils.fields import NestedAttribute, SanitizedUnicode +from marshmallow import EXCLUDE, Schema, ValidationError, fields, post_dump, pre_load +from marshmallow_utils.fields import ( + EDTFDateTimeString, + NestedAttribute, + SanitizedHTML, + SanitizedUnicode, + TZDateTime, +) from marshmallow_utils.permissions import FieldPermissionsMixin from .access import AccessSchema from .files import FilesSchema from .metadata import MetadataSchema from .parent import RDMParentSchema +from .parent.access import Agent from .pids import PIDSchema from .stats import StatsSchema from .tombstone import DeletionStatusSchema, TombstoneSchema @@ -36,6 +44,20 @@ def validate_scheme(scheme): raise ValidationError(_("Invalid persistent identifier scheme.")) +class InternalNoteSchema(Schema): + """Schema for internal notes.""" + + id = SanitizedUnicode() + timestamp = EDTFDateTimeString(dump_only=True) + added_by = fields.Nested(Agent, dump_only=True) + note = SanitizedHTML() + + class Meta: + """Meta attributes for the schema.""" + + unknown = EXCLUDE + + class RDMRecordSchema(RecordSchema, FieldPermissionsMixin): """Record schema.""" @@ -68,10 +90,18 @@ class Meta: status = fields.String(dump_only=True) tombstone = fields.Nested(TombstoneSchema, dump_only=True) deletion_status = fields.Nested(DeletionStatusSchema, dump_only=True) - + internal_notes = fields.List(fields.Nested(InternalNoteSchema)) stats = NestedAttribute(StatsSchema, dump_only=True) # schema_version = fields.Integer(dump_only=True) + field_dump_permissions = { + "internal_notes": "manage_internal", + } + + field_load_permissions = { + "internal_notes": "manage_internal", + } + def default_nested(self, data): """Serialize fields as empty dict for partial drafts. @@ -86,7 +116,6 @@ def default_nested(self, data): data["pids"] = {} if not data.get("custom_fields"): data["custom_fields"] = {} - return data def hide_tombstone(self, data): diff --git a/tests/resources/test_resources.py b/tests/resources/test_resources.py index c1e307b95..6e7b39c9f 100644 --- a/tests/resources/test_resources.py +++ b/tests/resources/test_resources.py @@ -10,15 +10,18 @@ """Module tests.""" import json +from copy import deepcopy from datetime import datetime, timedelta, timezone from io import BytesIO import arrow import pytest from invenio_requests import current_requests_service +from marshmallow_utils.permissions import FieldPermissionError from invenio_rdm_records.records import RDMDraft, RDMRecord from invenio_rdm_records.requests import CommunitySubmission +from tests.helpers import login_user, logout_user @pytest.fixture() @@ -943,3 +946,52 @@ def _create_and_include_in_community(): res = client.get(f"/communities/{community['id']}/records", headers=headers) assert res.json["hits"]["total"] == 1 + + +def test_search_internal_notes_fields( + running_app, client, minimal_record, headers, search_clear, admin, users +): + + # login regular user + login_user(client, users[0]) + minimal_record_w_int_notes = deepcopy(minimal_record) + minimal_record_w_int_notes["internal_notes"] = [{"note": "abc"}] + + with pytest.raises(FieldPermissionError): + resp = client.post( + "/records", json={**minimal_record_w_int_notes}, headers=headers + ) + + # login admin + logout_user(client) + user = admin.user + login_user(client, user) + + resp = client.post("/records", json={**minimal_record_w_int_notes}, headers=headers) + assert resp.status_code == 201 + + recid = resp.json["id"] + response = client.post( + "/records/{}/draft/actions/publish".format(recid), headers=headers + ) + + resp = client.get(f"/records/{recid}") + assert resp.json["id"] == recid + assert resp.json["internal_notes"][0]["note"] == "abc" + resp = client.get("/records?q=abc") + assert resp.json["hits"]["total"] == 0 + resp = client.get("/records?q=internal_notes.note:abc") + assert resp.json["hits"]["total"] == 1 + + # login user to check search and field perms + logout_user(client) + login_user(client, users[0]) + + resp = client.get(f"/records/{recid}") + assert "internal_notes" not in resp.json + + resp = client.get("/records?q=abc") + assert resp.json["hits"]["total"] == 0 + resp = client.get("/records?q=internal_notes.note:abc") + assert resp.json["hits"]["total"] == 0 + logout_user(client) diff --git a/tests/services/components/test_internal_notes_component.py b/tests/services/components/test_internal_notes_component.py new file mode 100644 index 000000000..36c5ca2a4 --- /dev/null +++ b/tests/services/components/test_internal_notes_component.py @@ -0,0 +1,110 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2021 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. + +"""Tests for the service Metadata component.""" +from copy import deepcopy + +from invenio_rdm_records.proxies import current_rdm_records +from invenio_rdm_records.records import RDMRecord +from invenio_rdm_records.records.api import RDMDraft +from invenio_rdm_records.services.components.internal_notes import ( + InternalNotesComponent, +) + + +def test_notes_component(minimal_record, parent, identity_simple, location): + """Test the metadata component.""" + record = RDMRecord.create(minimal_record, parent=parent) + draft = RDMDraft.new_version(record) + + assert "publication_date" in record.metadata + assert "title" in record.metadata + + component = InternalNotesComponent(current_rdm_records.records_service) + new_data = deepcopy(draft) + new_data["internal_notes"] = [{"note": "abc"}] + # check if internal notes have correct attributes + component.update_draft(identity_simple, data=new_data, record=draft) + assert "internal_notes" in draft + notes = draft["internal_notes"] + assert "id" in notes[0] + assert "timestamp" in notes[0] + assert "added_by" in notes[0] + + # check if new internal note is added properly + new_data["internal_notes"] = notes + [{"note": "def"}] + component.update_draft(identity_simple, data=new_data, record=draft) + assert "internal_notes" in draft + updated_notes = draft["internal_notes"] + assert len(updated_notes) == len(notes) + 1 + + # check if internal note can be removed + existing_id = updated_notes[0]["id"] + del updated_notes[0] + new_data["internal_notes"] = updated_notes + component.update_draft(identity_simple, data=new_data, record=draft) + assert "internal_notes" in draft + updated_notes = draft["internal_notes"] + assert len(updated_notes) == 1 + assert updated_notes[0]["id"] != existing_id + + # check if internal note can be removed and add another one + existing_id = updated_notes[0]["id"] + del updated_notes[0] + new_data["internal_notes"] = updated_notes + [{"note": "def"}] + component.update_draft(identity_simple, data=new_data, record=draft) + assert "internal_notes" in draft + updated_notes = draft["internal_notes"] + assert len(updated_notes) == 1 + assert updated_notes[0]["id"] != existing_id + + # check if internal note once added, is immutable + new_data["internal_notes"] = [ + { + "note": "this text should not be taken into account", + "id": updated_notes[0]["id"], + } + ] + component.update_draft(identity_simple, data=new_data, record=draft) + assert "internal_notes" in draft + updated_notes = draft["internal_notes"] + assert len(updated_notes) == 1 + assert updated_notes[0]["id"] == updated_notes[0]["id"] + assert updated_notes[0]["note"] == "def" + + # test create + record = RDMRecord.create(minimal_record, parent=parent) + data = deepcopy(record) + data["internal_notes"] = [{"note": "abc"}] + component.create(identity_simple, data=data, record=record) + assert "internal_notes" in record + notes = record["internal_notes"] + assert "id" in notes[0] + assert "timestamp" in notes[0] + assert "added_by" in notes[0] + assert notes[0]["note"] == "abc" + + # test new version + draft = RDMDraft.new_version(record) + component.new_version(identity_simple, draft=draft, record=record) + assert "internal_notes" in draft + notes = draft["internal_notes"] + assert "id" in notes[0] + assert "timestamp" in notes[0] + assert "added_by" in notes[0] + assert notes[0]["note"] == "abc" + + # test publish + record = RDMRecord.publish(draft) + + component.publish(identity_simple, draft=draft, record=record) + assert "internal_notes" in record + notes = record["internal_notes"] + assert "id" in notes[0] + assert "timestamp" in notes[0] + assert "added_by" in notes[0] + assert notes[0]["note"] == "abc"