From e09819da8a1ab2c3fa76d10888787e78f1c011f0 Mon Sep 17 00:00:00 2001 From: David Eckhard Date: Mon, 7 Oct 2024 15:05:08 +0200 Subject: [PATCH 1/3] components: Update request title if draft title has changed --- invenio_curations/services/components.py | 27 +++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/invenio_curations/services/components.py b/invenio_curations/services/components.py index 4eb943e..51459f9 100644 --- a/invenio_curations/services/components.py +++ b/invenio_curations/services/components.py @@ -71,6 +71,22 @@ def delete_draft(self, identity, draft=None, record=None, force=False): system_identity, request["id"], "cancel" ) + def _check_update_request( + self, identity, request, data=None, record=None, errors=None + ): + """Update request title if record title has changed.""" + updated_draft_title = (data or {}).get("metadata", {}).get("title") + current_draft_title = (record or {}).get("metadata", {}).get("title") + if current_draft_title != updated_draft_title: + request["title"] = "RDM Curation: {title}".format( + title=updated_draft_title or record["id"] + ) + # Using system identity, to not have to update the default request can_update permission. + # Data will be checked in the requests service. + current_requests_service.update( + system_identity, request["id"], request, uow=self.uow + ) + def update_draft(self, identity, data=None, record=None, errors=None): """Update draft handler.""" has_published_record = record is not None and record.is_published @@ -97,6 +113,14 @@ def update_draft(self, identity, data=None, record=None, errors=None): ) return + current_draft = self.service.draft_cls.pid.resolve( + record["id"], registered_only=False + ) + + self._check_update_request( + identity, request, data=data, record=current_draft, errors=errors + ) + # TODO: Should updates be disallowed if the record/request is currently being reviewed? # It could be possible that the record gets updated while a curator performs a review. The curator would be looking at an outdated record and the review might not be correct. @@ -105,9 +129,6 @@ def update_draft(self, identity, data=None, record=None, errors=None): return # Compare metadata of current draft and updated draft. - current_draft = self.service.draft_cls.pid.resolve( - record["id"], registered_only=False - ) # Sometimes the metadata differs between the passed `record` and resolved # `current_draft` in references (e.g. in the `record` object, the creator's From b7c817337afa314876423c6835416cb9845348b5 Mon Sep 17 00:00:00 2001 From: David Eckhard Date: Mon, 7 Oct 2024 16:17:44 +0200 Subject: [PATCH 2/3] generators: Handle case when entity can not be resolved --- invenio_curations/services/generators.py | 30 ++++++++++++++++++----- invenio_curations/services/permissions.py | 1 + 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/invenio_curations/services/generators.py b/invenio_curations/services/generators.py index 9954156..dba94e4 100644 --- a/invenio_curations/services/generators.py +++ b/invenio_curations/services/generators.py @@ -12,6 +12,7 @@ from flask_principal import RoleNeed from invenio_access.permissions import system_identity +from invenio_pidstore.errors import PIDDoesNotExistError from invenio_records_permissions.generators import ConditionalGenerator, Generator from ..proxies import current_curations_service @@ -73,22 +74,32 @@ def __init__(self, permission_name, **kwargs): assert self.entity_field is not None, "Subclass must define entity_field." super().__init__() - def _get_permission(self, request): + def _get_permission(self, entity): """Get the specified permission from the request entity service config.""" - entity = getattr(request, self.entity_field) permission_policy_cls = ( entity.get_resolver().get_service().config.permission_policy_cls ) return getattr(permission_policy_cls, self.permission_name) + def _get_entity(self, request): + """Get the specified entity of the request.""" + return getattr(request, self.entity_field) + def needs(self, request=None, **kwargs): """Set of needs granting permission.""" if request is None: return set() - permission = self._get_permission(request) - record = request.topic.resolve() + entity = self._get_entity(request) + permission = self._get_permission(entity) + try: + record = entity.resolve() + except PIDDoesNotExistError: + # Could not resolve topic. This may happen when trying to serialize a request and checking its permissions. + # The referenced entity could be deleted, which would result in not being able to serialize instead. Instead, + # an empty set is returned for this permission. + return set() popped_record = kwargs.pop("record") needs = [g.needs(record=record, **kwargs) for g in permission] @@ -100,8 +111,15 @@ def excludes(self, request=None, **kwargs): if request is None: return set() - permission = self._get_permission(request) - record = request.topic.resolve() + entity = self._get_entity(request) + permission = self._get_permission(entity) + try: + record = entity.resolve() + except PIDDoesNotExistError: + # Could not resolve topic. This may happen when trying to serialize a request and checking its permissions. + # The referenced entity could be deleted, which would result in not being able to serialize instead. Instead, + # an empty set is returned for this permission. + return set() popped_record = kwargs.pop("record") excludes = [g.excludes(record=record, **kwargs) for g in permission] diff --git a/invenio_curations/services/permissions.py b/invenio_curations/services/permissions.py index 2c6251b..b09f0cd 100644 --- a/invenio_curations/services/permissions.py +++ b/invenio_curations/services/permissions.py @@ -107,6 +107,7 @@ class CurationRDMRequestsPermissionPolicy(RDMRequestsPermissionPolicy): Creator(), Receiver(), TopicPermission(permission_name="can_review"), + SystemProcess(), ], else_=RDMRequestsPermissionPolicy.can_read, ) From e7ab8cb06de53600f79150403a42802baa02842e Mon Sep 17 00:00:00 2001 From: David Eckhard Date: Mon, 7 Oct 2024 16:58:28 +0200 Subject: [PATCH 3/3] components: Pass components uow to service calls * For some reason, we could end up in a state where a request's status is "resubmitted" in the database while it is `accepted` in opensearch. --- invenio_curations/services/components.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/invenio_curations/services/components.py b/invenio_curations/services/components.py index 51459f9..9b3ae16 100644 --- a/invenio_curations/services/components.py +++ b/invenio_curations/services/components.py @@ -68,7 +68,7 @@ def delete_draft(self, identity, draft=None, record=None, force=False): # Delete draft for a published record. # Since only one request per record should exist, it is not deleted. Instead, put it back to accepted. current_requests_service.execute_action( - system_identity, request["id"], "cancel" + system_identity, request["id"], "cancel", uow=self.uow ) def _check_update_request( @@ -161,4 +161,6 @@ def update_draft(self, identity, data=None, record=None, errors=None): # Request is closed but draft was updated with new data. Put back for review if diff_list: - current_requests_service.execute_action(identity, request["id"], "resubmit") + current_requests_service.execute_action( + identity, request["id"], "resubmit", uow=self.uow + )