From 88315cd62b82dca0ad58f95ac3dc8f0958e28b61 Mon Sep 17 00:00:00 2001 From: Ronald Krist Date: Thu, 12 Dec 2024 15:26:53 +0100 Subject: [PATCH 1/2] cancel transition fix --- oarepo_requests/actions/components.py | 10 ++--- oarepo_requests/actions/generic.py | 9 ++++- setup.cfg | 2 +- tests/conftest.py | 25 ++++++++++--- tests/test_requests/test_workflows.py | 53 +++++++++++++++++++++++++-- 5 files changed, 82 insertions(+), 17 deletions(-) diff --git a/oarepo_requests/actions/components.py b/oarepo_requests/actions/components.py index 4e52a61..130308e 100644 --- a/oarepo_requests/actions/components.py +++ b/oarepo_requests/actions/components.py @@ -14,11 +14,7 @@ import abc import contextlib -from typing import ( - TYPE_CHECKING, - Any, - override, -) +from typing import TYPE_CHECKING, Any, override from invenio_requests.customizations import RequestAction, RequestActions, RequestType from invenio_requests.errors import CannotExecuteActionError @@ -114,6 +110,10 @@ def apply( from sqlalchemy.exc import NoResultFound yield + if ( + not topic + ): # for example if we are cancelling requests after deleting draft, it does not make sense to attempt changing the state of the draft + return try: transitions = ( current_oarepo_workflows.get_workflow(topic) diff --git a/oarepo_requests/actions/generic.py b/oarepo_requests/actions/generic.py index 43f5083..7802827 100644 --- a/oarepo_requests/actions/generic.py +++ b/oarepo_requests/actions/generic.py @@ -94,7 +94,10 @@ def execute( """Execute the action.""" request: Request = self.request # type: ignore request_type = request.type - topic = request.topic.resolve() + try: + topic = request.topic.resolve() + except: + topic = None self._execute_with_components( self.components, identity, request_type, topic, uow, *args, **kwargs ) @@ -154,8 +157,10 @@ class OARepoAcceptAction(OARepoGenericActionMixin, actions.AcceptAction): name = _("Accept") -class OARepoCancelAction(actions.CancelAction): +class OARepoCancelAction(OARepoGenericActionMixin, actions.CancelAction): """Cancel action extended for oarepo requests.""" + name = _("Cancel") + status_from = ["created", "submitted"] status_to = "cancelled" diff --git a/setup.cfg b/setup.cfg index a98e710..8b11ef8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = oarepo-requests -version = 2.3.11 +version = 2.3.12 description = authors = Ronald Krist readme = README.md diff --git a/tests/conftest.py b/tests/conftest.py index 87977a7..1a2f00a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -106,14 +106,20 @@ class DefaultRequests(WorkflowRequestPolicy): requesters=[IfInState("draft", [RecordOwners()])], recipients=[UserGenerator(2)], transitions=WorkflowTransitions( - submitted="publishing", accepted="published", declined="draft" + submitted="publishing", + accepted="published", + declined="draft", + cancelled="draft", ), ) delete_published_record = WorkflowRequest( requesters=[IfInState("published", [RecordOwners()])], recipients=[UserGenerator(2)], transitions=WorkflowTransitions( - submitted="deleting", accepted="deleted", declined="published" + submitted="deleting", + accepted="deleted", + declined="published", + cancelled="published", ), ) delete_draft = WorkflowRequest( @@ -158,7 +164,10 @@ class RequestsWithApprove(WorkflowRequestPolicy): requesters=[IfInState("approved", [AutoRequest()])], recipients=[UserGenerator(1)], transitions=WorkflowTransitions( - submitted="publishing", accepted="published", declined="approved" + submitted="publishing", + accepted="published", + declined="approved", + cancelled="approved", ), events=events_only_receiver_can_comment, ) @@ -166,7 +175,10 @@ class RequestsWithApprove(WorkflowRequestPolicy): requesters=[IfInState("draft", [RecordOwners()])], recipients=[UserGenerator(2)], transitions=WorkflowTransitions( - submitted="approving", accepted="approved", declined="draft" + submitted="approving", + accepted="approved", + declined="draft", + cancelled="draft", ), events=events_only_receiver_can_comment, ) @@ -174,7 +186,10 @@ class RequestsWithApprove(WorkflowRequestPolicy): requesters=[IfInState("published", [RecordOwners()])], recipients=[UserGenerator(2)], transitions=WorkflowTransitions( - submitted="deleting", accepted="deleted", declined="published" + submitted="deleting", + accepted="deleted", + declined="published", + cancelled="published", ), events=events_only_receiver_can_comment, ) diff --git a/tests/test_requests/test_workflows.py b/tests/test_requests/test_workflows.py index c1d5a9e..91edeba 100644 --- a/tests/test_requests/test_workflows.py +++ b/tests/test_requests/test_workflows.py @@ -461,10 +461,12 @@ def test_workflow_events_resource( request_id = read_from_record.json["expanded"]["requests"][0]["id"] json = {**events_resource_data, "type": TestEventType.type_id} create_event_u1 = user1_client.post( - f"{urls['BASE_URL_REQUESTS']}{request_id}/timeline/{TestEventType.type_id}", json=json + f"{urls['BASE_URL_REQUESTS']}{request_id}/timeline/{TestEventType.type_id}", + json=json, ) create_event_u2 = user2_client.post( - f"{urls['BASE_URL_REQUESTS']}{request_id}/timeline/{TestEventType.type_id}", json=json + f"{urls['BASE_URL_REQUESTS']}{request_id}/timeline/{TestEventType.type_id}", + json=json, ) assert create_event_u1.status_code == 403 @@ -493,10 +495,12 @@ def test_workflow_events_resource( request_id = publish_request["id"] create_event_u1 = user1_client.post( - f"{urls['BASE_URL_REQUESTS']}{request_id}/timeline/{TestEventType.type_id}", json=json + f"{urls['BASE_URL_REQUESTS']}{request_id}/timeline/{TestEventType.type_id}", + json=json, ) create_event_u2 = user2_client.post( - f"{urls['BASE_URL_REQUESTS']}{request_id}/timeline/{TestEventType.type_id}", json=json + f"{urls['BASE_URL_REQUESTS']}{request_id}/timeline/{TestEventType.type_id}", + json=json, ) assert create_event_u1.status_code == 201 assert create_event_u2.status_code == 403 @@ -558,3 +562,44 @@ def test_delete_log( break else: assert False + + +def test_cancel_transition( + vocab_cf, + logged_client, + users, + urls, + publish_request_data_function, + create_draft_via_resource, + search_clear, +): + creator = users[0] + creator_client = logged_client(creator) + + draft1 = create_draft_via_resource(creator_client) + + resp_request_create = creator_client.post( + urls["BASE_URL_REQUESTS"], + json=publish_request_data_function(draft1.json["id"]), + ) + resp_request_submit = creator_client.post( + link2testclient(resp_request_create.json["links"]["actions"]["submit"]), + ) + + record = creator_client.get( + f"{urls['BASE_URL']}{draft1.json['id']}/draft?expand=true" + ) + assert record.json["expanded"]["requests"][0]["links"]["actions"].keys() == { + "cancel", + } + assert record.json["state"] == "publishing" + creator_client.post( + link2testclient( + record.json["expanded"]["requests"][0]["links"]["actions"]["cancel"] + ), + ) + + record = creator_client.get( + f"{urls['BASE_URL']}{draft1.json['id']}/draft?expand=true" + ) + assert record.json["state"] == "draft" From 517adece38176799e923d8379280f644ec4f3050 Mon Sep 17 00:00:00 2001 From: Ronald Krist Date: Tue, 17 Dec 2024 16:46:14 +0100 Subject: [PATCH 2/2] catching PersistentIdentifierError in topic resolve error cases --- oarepo_requests/actions/generic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/oarepo_requests/actions/generic.py b/oarepo_requests/actions/generic.py index 7802827..30645b1 100644 --- a/oarepo_requests/actions/generic.py +++ b/oarepo_requests/actions/generic.py @@ -16,7 +16,7 @@ from oarepo_runtime.i18n import lazy_gettext as _ from oarepo_requests.proxies import current_oarepo_requests - +from invenio_pidstore.errors import PersistentIdentifierError if TYPE_CHECKING: from flask_babel.speaklater import LazyString from flask_principal import Identity @@ -96,7 +96,7 @@ def execute( request_type = request.type try: topic = request.topic.resolve() - except: + except PersistentIdentifierError: topic = None self._execute_with_components( self.components, identity, request_type, topic, uow, *args, **kwargs