From 7e2b1957dc06aed28e56bf6fc3b5eb505d7564d8 Mon Sep 17 00:00:00 2001 From: Karolina Przerwa Date: Wed, 6 Nov 2024 18:13:45 +0100 Subject: [PATCH] records: upgrade jsonschema version * adds internal notes --- invenio_rdm_records/config.py | 15 +- invenio_rdm_records/records/api.py | 2 +- .../jsonschemas/records/record-v6.0.0.json | 18 - .../jsonschemas/records/record-v7.0.0.json | 463 ++++++++++++++++++ .../services/components/internal_notes.py | 30 +- invenio_rdm_records/services/config.py | 8 - invenio_rdm_records/services/permissions.py | 3 +- .../services/schemas/metadata.py | 1 - tests/records/test_jsonschema.py | 2 +- tests/records/tombstone.json | 2 +- tests/resources/test_resources.py | 4 +- 11 files changed, 499 insertions(+), 49 deletions(-) create mode 100644 invenio_rdm_records/records/jsonschemas/records/record-v7.0.0.json diff --git a/invenio_rdm_records/config.py b/invenio_rdm_records/config.py index 7dfbd5bb0..9ea88dddd 100644 --- a/invenio_rdm_records/config.py +++ b/invenio_rdm_records/config.py @@ -13,11 +13,12 @@ from datetime import timedelta import idutils -from invenio_administration.permissions import administration_permission +from invenio_access.permissions import system_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, + RestrictedTermValue, SearchFieldTransformer, ) @@ -274,12 +275,12 @@ def always_valid(identifier): ], "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 + "internal_notes.note": RestrictedTerm(system_permission), + "internal_notes.id": RestrictedTerm(system_permission), + "internal_notes.added_by": RestrictedTerm(system_permission), + "internal_notes.timestamp": RestrictedTerm(system_permission), + "_exists_": RestrictedTermValue( + system_permission, word=word_internal_notes ), }, tree_transformer_cls=SearchFieldTransformer, diff --git a/invenio_rdm_records/records/api.py b/invenio_rdm_records/records/api.py index 0348f53bb..327ab1860 100644 --- a/invenio_rdm_records/records/api.py +++ b/invenio_rdm_records/records/api.py @@ -113,7 +113,7 @@ class CommonFieldsMixin: # Remember to update INDEXER_DEFAULT_INDEX in Invenio-App-RDM if you # update the JSONSchema and mappings to a new version. - schema = ConstantField("$schema", "local://records/record-v6.0.0.json") + schema = ConstantField("$schema", "local://records/record-v7.0.0.json") dumper = SearchDumper( extensions=[ 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 592f5db23..d426401cc 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 @@ -379,24 +379,6 @@ } } }, - "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/jsonschemas/records/record-v7.0.0.json b/invenio_rdm_records/records/jsonschemas/records/record-v7.0.0.json new file mode 100644 index 000000000..6ae6aa409 --- /dev/null +++ b/invenio_rdm_records/records/jsonschemas/records/record-v7.0.0.json @@ -0,0 +1,463 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "id": "local://records/record-v7.0.0.json", + "title": "InvenioRDM Record Schema v7.0.0", + "type": "object", + "additionalProperties": false, + "properties": { + "$schema": { + "$ref": "local://definitions-v1.0.0.json#/$schema" + }, + "id": { + "$ref": "local://definitions-v1.0.0.json#/identifier" + }, + "pid": { + "$ref": "local://definitions-v1.0.0.json#/internal-pid" + }, + "pids": { + "type": "object", + "description": "External persistent identifiers for a record including e.g. OAI-PMH identifier, minted DOIs and more. PIDs are registered in the PIDStore.", + "additionalProperties": { + "$ref": "local://records/definitions-v2.0.0.json#/external-pid" + }, + "propertyNames": { + "pattern": "^[a-z_-][a-z0-9_-]*$" + }, + "uniqueItems": true + }, + "metadata": { + "type": "object", + "description": "Resource metadata.", + "additionalProperties": false, + "properties": { + "resource_type": { + "$ref": "local://records/definitions-v2.0.0.json#/resource_type" + }, + "creators": { + "description": "Creators of the resource.", + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "person_or_org": { + "$ref": "local://records/definitions-v2.0.0.json#/person_or_org" + }, + "role": { + "$ref": "local://records/definitions-v2.0.0.json#/role" + }, + "affiliations": { + "$ref": "local://records/definitions-v2.0.0.json#/affiliations" + } + } + } + }, + "title": { + "description": "Primary title of the record.", + "type": "string" + }, + "additional_titles": { + "description": "Additional record titles.", + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "title": { + "description": "Additional title of the record.", + "type": "string" + }, + "type": { + "$ref": "local://records/definitions-v2.0.0.json#/title_type" + }, + "lang": { + "$ref": "local://records/definitions-v2.0.0.json#/language" + } + } + } + }, + "publisher": { + "type": "string" + }, + "publication_date": { + "description": "Record publication date (EDTF level 0 format).", + "type": "string" + }, + "subjects": { + "$ref": "local://records/definitions-v2.0.0.json#/subjects" + }, + "contributors": { + "description": "Contributors in order of importance.", + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "person_or_org": { + "$ref": "local://records/definitions-v2.0.0.json#/person_or_org" + }, + "role": { + "$ref": "local://records/definitions-v2.0.0.json#/role" + }, + "affiliations": { + "$ref": "local://records/definitions-v2.0.0.json#/affiliations" + } + } + } + }, + "dates": { + "description": "Date, datetime or date interval.", + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "date": { + "description": "Date, datetime or date interval in EDTF level 0 format", + "type": "string" + }, + "type": { + "$ref": "local://records/definitions-v2.0.0.json#/date_type" + }, + "description": { + "description": "Description of the date, datetime or date interval e.g. 'Accepted' or 'Available' (CV).", + "type": "string" + } + } + } + }, + "languages": { + "description": "The primary languages of the resource. ISO 639-3 language code.", + "type": "array", + "items": { + "$ref": "local://records/definitions-v2.0.0.json#/language" + } + }, + "identifiers": { + "description": "Alternate identifiers for the record.", + "type": "array", + "items": { + "$ref": "local://definitions-v1.0.0.json#/identifiers_with_scheme" + }, + "uniqueItems": true + }, + "related_identifiers": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "identifier": { + "$ref": "local://definitions-v1.0.0.json#/identifier" + }, + "scheme": { + "$ref": "local://definitions-v1.0.0.json#/scheme" + }, + "relation_type": { + "$ref": "local://records/definitions-v2.0.0.json#/relation_type" + }, + "resource_type": { + "$ref": "local://records/definitions-v2.0.0.json#/resource_type" + } + } + } + }, + "sizes": { + "type": "array", + "items": { + "type": "string" + } + }, + "formats": { + "type": "array", + "items": { + "type": "string" + } + }, + "version": { + "description": "Record version tag.", + "type": "string" + }, + "rights": { + "description": "Any license or copyright information for this resource.", + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "id": { + "$ref": "local://definitions-v1.0.0.json#/identifier" + }, + "title": { + "description": "The license name or license itself. Free text.", + "type": "object" + }, + "description": { + "description": "The license description Free text.", + "type": "object" + }, + "link": { + "type": "string", + "format": "uri" + } + } + } + }, + "description": { + "description": "Description for record (may contain HTML).", + "type": "string" + }, + "additional_descriptions": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "description": { + "description": "Description for record.", + "type": "string" + }, + "type": { + "$ref": "local://records/definitions-v2.0.0.json#/description_type" + }, + "lang": { + "$ref": "local://records/definitions-v2.0.0.json#/language" + } + } + } + }, + "locations": { + "description": "Geographical locations relevant to this record.", + "$comment": "This part of the schema is modelled on GeoJSON, but without the scope to embed arbitrary metadata.\n\nWe nest `features` within `locations` to give scope later to say something about the locations as a whole.", + "type": "object", + "additionalProperties": false, + "properties": { + "features": { + "type": "array", + "minItems": 1, + "items": { + "allOf": [ + { + "type": "object", + "additionalProperties": false, + "properties": { + "geometry": { + "allOf": [ + { + "$ref": "local://records/definitions-v2.0.0.json#/GeoJSON-Geometry" + }, + { + "$comment": "Bounding boxes (`bbox`) are forbidden on our geometries, even though they're allowed on GeoJSON geometries", + "properties": { + "type": {}, + "coordinates": {} + }, + "additionalProperties": false + } + ] + }, + "identifiers": { + "type": "array", + "items": { + "$ref": "local://definitions-v1.0.0.json#/identifiers_with_scheme" + }, + "uniqueItems": true + }, + "place": { + "description": "Place of the location", + "type": "string", + "minLength": 1 + }, + "description": { + "description": "Description of the location", + "type": "string", + "minLength": 1 + } + } + } + ] + } + } + } + }, + "funding": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "funder": { + "type": "object", + "additionalProperties": false, + "properties": { + "name": { + "type": "string" + }, + "id": { + "$ref": "local://definitions-v1.0.0.json#/identifier" + } + } + }, + "award": { + "type": "object", + "additionalProperties": false, + "properties": { + "title": { + "type": "object" + }, + "number": { + "type": "string" + }, + "id": { + "$ref": "local://definitions-v1.0.0.json#/identifier" + }, + "identifiers": { + "type": "array", + "items": { + "$ref": "local://definitions-v1.0.0.json#/identifiers_with_scheme" + } + } + } + } + } + } + }, + "references": { + "type": "array", + "minItems": 0, + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "reference": { + "type": "string", + "description": "A reference string." + }, + "identifier": { + "$ref": "local://definitions-v1.0.0.json#/identifier" + }, + "scheme": { + "$ref": "local://definitions-v1.0.0.json#/scheme" + } + } + } + } + } + }, + "custom_fields": { + "type": "object", + "description": "Configured additional metadata" + }, + "tombstone": { + "type": "object", + "description": "Tombstone information for the record.", + "additionalProperties": false, + "properties": { + "removal_reason": { + "$ref": "local://records/definitions-v2.0.0.json#/removal_reason", + "description": "Reason for record removal." + }, + "note": { + "type": "string", + "description": "Public note about the removal." + }, + "removed_by": { + "$ref": "local://records/definitions-v2.0.0.json#/agent" + }, + "removal_date": { + "type": "string", + "description": "ISO8601 formatted timestamp in UTC.", + "format": "date-time" + }, + "citation_text": { + "type": "string", + "description": "The record citation text to be displayed on the tombstone page." + }, + "is_visible": { + "type": "boolean", + "description": "Whether or not the tombstone page is publicly visible." + } + } + }, + "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.", + "additionalProperties": false, + "properties": { + "created_by": { + "$ref": "local://records/definitions-v2.0.0.json#/agent" + }, + "on_behalf_of": { + "$ref": "local://records/definitions-v2.0.0.json#/agent" + } + } + }, + "access": { + "type": "object", + "description": "Record access control and ownership.", + "additionalProperties": false, + "properties": { + "record": { + "description": "Record visibility (public or restricted)", + "type": "string", + "enum": ["public", "restricted"] + }, + "files": { + "description": "Files visibility (public or restricted)", + "type": "string", + "enum": ["public", "restricted"] + }, + "embargo": { + "description": "Description of the embargo on the record.", + "type": "object", + "additionalProperties": false, + "properties": { + "active": { + "description": "Whether or not the embargo is (still) active.", + "type": ["boolean", "null"] + }, + "until": { + "description": "Embargo date of record (ISO8601 formatted date time in UTC). At this time both metadata and files will be made public.", + "type": ["string", "null"], + "format": "date" + }, + "reason": { + "description": "The reason why the record is under embargo.", + "type": ["string", "null"] + } + } + } + } + }, + "files": { + "$ref": "local://definitions-v2.0.0.json#/files-simple" + }, + "media_files": { + "$ref": "local://definitions-v2.0.0.json#/files-simple" + }, + "notes": { + "type": "array", + "items": { + "type": "string" + } + } + } +} diff --git a/invenio_rdm_records/services/components/internal_notes.py b/invenio_rdm_records/services/components/internal_notes.py index d249965a2..33f125466 100644 --- a/invenio_rdm_records/services/components/internal_notes.py +++ b/invenio_rdm_records/services/components/internal_notes.py @@ -9,6 +9,7 @@ import uuid from copy import copy, deepcopy from datetime import datetime, timezone + from invenio_drafts_resources.services.records.components import ServiceComponent @@ -31,29 +32,40 @@ def create(self, identity, data=None, record=None, **kwargs): 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] + """Update internal notes field on draft update. + + Takes list of internal notes. Adds timestamp and user to new notes. + Ignores updates of existing notes. Removes a note if missing from data + Ex. if existing record has record["internal_notes"] = [{"id": "uuuid1", "note": "abc"}] + data: [{"id": "uuuid1", "note": "abc"}, {"note: "edf"}] <--- adds a new note "edf" + data: [] <--- removes existing note of id = uuid1 + data: [{"id": "uuuid1", "note": "new note"}] <--- does not modify existing note + We don't allow updates to existing notes since each one + has a user and timestamp - therefore should be "locked" in time + """ + notes_to_update = deepcopy(data.get(self.field, [])) + existing_notes = deepcopy(record.get(self.field, [])) + ids_to_check = [note["id"] for note in existing_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 + # skip already existing IDs from incoming data + # because we don't allow updating existing notes notes_to_update.remove(note) + # we remove checked ids. The ones remaining on this list will be deleted 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: + for note in existing_notes: if note["id"] in to_delete: - notes.remove(note) + existing_notes.remove(note) - record.update({"internal_notes": notes + notes_to_update}) + record.update({"internal_notes": existing_notes + notes_to_update}) def publish(self, identity, draft=None, record=None, **kwargs): """Update draft metadata.""" diff --git a/invenio_rdm_records/services/config.py b/invenio_rdm_records/services/config.py index 60ed81a1f..160071860 100644 --- a/invenio_rdm_records/services/config.py +++ b/invenio_rdm_records/services/config.py @@ -65,17 +65,9 @@ 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 -from werkzeug.exceptions import NotFound from werkzeug.local import LocalProxy from invenio_rdm_records.records.processors.tiles import TilesProcessor diff --git a/invenio_rdm_records/services/permissions.py b/invenio_rdm_records/services/permissions.py index d77ccafb5..8d6f06477 100644 --- a/invenio_rdm_records/services/permissions.py +++ b/invenio_rdm_records/services/permissions.py @@ -63,7 +63,8 @@ class RDMRecordPermissionPolicy(RecordPermissionPolicy): # permission meant for global curators of the instance # (for now applies to internal notes field only - can_manage_internal = [Administration()] + # to be replaced with an adequate permission when it is defined) + can_manage_internal = [SystemProcess()] # # High-level permissions (used by low-level) # diff --git a/invenio_rdm_records/services/schemas/metadata.py b/invenio_rdm_records/services/schemas/metadata.py index ab2fdf3d7..eeab1370d 100644 --- a/invenio_rdm_records/services/schemas/metadata.py +++ b/invenio_rdm_records/services/schemas/metadata.py @@ -39,7 +39,6 @@ from marshmallow_utils.schemas import GeometryObjectSchema, IdentifierSchema from werkzeug.local import LocalProxy - record_personorg_schemes = LocalProxy( lambda: current_app.config["RDM_RECORDS_PERSONORG_SCHEMES"] ) diff --git a/tests/records/test_jsonschema.py b/tests/records/test_jsonschema.py index 40974ad32..ea69d147a 100644 --- a/tests/records/test_jsonschema.py +++ b/tests/records/test_jsonschema.py @@ -23,7 +23,7 @@ # def validates(data): """Assertion function used to validate according to the schema.""" - data["$schema"] = "local://records/record-v6.0.0.json" + data["$schema"] = "local://records/record-v7.0.0.json" Record(data).validate() return True diff --git a/tests/records/tombstone.json b/tests/records/tombstone.json index 4df0f54d2..9a622e6a7 100644 --- a/tests/records/tombstone.json +++ b/tests/records/tombstone.json @@ -1,5 +1,5 @@ { - "$schema": "local://records/record-v6.0.0.json", + "$schema": "local://records/record-v7.0.0.json", "id": "abcde-12345", "pid": { "pk": 1, diff --git a/tests/resources/test_resources.py b/tests/resources/test_resources.py index 6e7b39c9f..d9d06ad91 100644 --- a/tests/resources/test_resources.py +++ b/tests/resources/test_resources.py @@ -949,7 +949,7 @@ def _create_and_include_in_community(): def test_search_internal_notes_fields( - running_app, client, minimal_record, headers, search_clear, admin, users + running_app, client, minimal_record, headers, search_clear, superuser, users ): # login regular user @@ -964,7 +964,7 @@ def test_search_internal_notes_fields( # login admin logout_user(client) - user = admin.user + user = superuser.user login_user(client, user) resp = client.post("/records", json={**minimal_record_w_int_notes}, headers=headers)