diff --git a/oarepo_requests/actions/edit_topic.py b/oarepo_requests/actions/edit_topic.py index f6246dda..cad6088b 100644 --- a/oarepo_requests/actions/edit_topic.py +++ b/oarepo_requests/actions/edit_topic.py @@ -1,4 +1,3 @@ -# from .generic import AcceptAction from invenio_requests.customizations import actions from ..utils import get_matching_service_for_record @@ -12,5 +11,5 @@ def execute(self, identity, uow): topic_service = get_matching_service_for_record(topic) if not topic_service: raise KeyError(f"topic {topic} service not found") - topic_service.edit(identity, topic.id, uow=uow) + topic_service.edit(identity, topic["id"], uow=uow) super().execute(identity, uow) diff --git a/oarepo_requests/actions/generic.py b/oarepo_requests/actions/generic.py index fe1d88d4..b79d9397 100644 --- a/oarepo_requests/actions/generic.py +++ b/oarepo_requests/actions/generic.py @@ -1,3 +1,19 @@ +from invenio_requests.customizations import actions +from invenio_requests.customizations.actions import RequestActions +from invenio_requests.errors import CannotExecuteActionError + + +class AutoAcceptSubmitAction(actions.SubmitAction): + log_event = True + + def execute(self, identity, uow): + super().execute(identity, uow) + action_obj = RequestActions.get_action(self.request, "accept") + if not action_obj.can_execute(): + raise CannotExecuteActionError("accept") + action_obj.execute(identity, uow) + + """ not needed for now diff --git a/oarepo_requests/resources/record/config.py b/oarepo_requests/resources/record/config.py index f3f09578..4c3bbfaf 100644 --- a/oarepo_requests/resources/record/config.py +++ b/oarepo_requests/resources/record/config.py @@ -10,14 +10,14 @@ class RecordRequestsResourceConfig: request_view_args = RecordResourceConfig.request_view_args | { "request_type": ma.fields.Str() } + """ @property def response_handlers(self): return { - **RequestsResourceConfig.routes, + **RecordResourceConfig.response_handlers, "application/vnd.inveniordm.v1+json": ResponseHandler( OARepoRequestsUIJSONSerializer() ), - **super().response_handlers, } """ diff --git a/oarepo_requests/types/delete_record.py b/oarepo_requests/types/delete_record.py index 1335d264..f6935851 100644 --- a/oarepo_requests/types/delete_record.py +++ b/oarepo_requests/types/delete_record.py @@ -3,10 +3,10 @@ from oarepo_requests.actions.delete_topic import DeleteTopicAcceptAction -from .generic import OARepoRequestType +from .generic import NonDuplicableOARepoRequestType -class DeleteRecordRequestType(OARepoRequestType): +class DeleteRecordRequestType(NonDuplicableOARepoRequestType): available_actions = { **RequestType.available_actions, "accept": DeleteTopicAcceptAction, diff --git a/oarepo_requests/types/edit_record.py b/oarepo_requests/types/edit_record.py index 84807777..5ed98d77 100644 --- a/oarepo_requests/types/edit_record.py +++ b/oarepo_requests/types/edit_record.py @@ -2,13 +2,15 @@ from oarepo_runtime.i18n import lazy_gettext as _ from oarepo_requests.actions.edit_topic import EditTopicAcceptAction +from oarepo_requests.actions.generic import AutoAcceptSubmitAction -from .generic import OARepoRequestType +from .generic import NonDuplicableOARepoRequestType -class EditRecordRequestType(OARepoRequestType): +class EditRecordRequestType(NonDuplicableOARepoRequestType): available_actions = { **RequestType.available_actions, + "submit": AutoAcceptSubmitAction, "accept": EditTopicAcceptAction, } description = _("Request re-opening of published record") diff --git a/oarepo_requests/types/generic.py b/oarepo_requests/types/generic.py index 5ad968b4..3aef81bd 100644 --- a/oarepo_requests/types/generic.py +++ b/oarepo_requests/types/generic.py @@ -2,6 +2,9 @@ from invenio_requests.customizations import RequestType from invenio_requests.proxies import current_requests_service +from oarepo_requests.errors import OpenRequestAlreadyExists +from oarepo_requests.utils import open_request_exists + class OARepoRequestType(RequestType): def can_create(self, identity, data, receiver, topic, creator, *args, **kwargs): @@ -21,3 +24,29 @@ def can_possibly_create(self, identity, topic, *args, **kwargs): except PermissionDeniedError: return False return True + + +class NonDuplicableOARepoRequestType(OARepoRequestType): + + def can_create(self, identity, data, receiver, topic, creator, *args, **kwargs): + if open_request_exists(topic, self.type_id): + raise OpenRequestAlreadyExists(self, topic) + current_requests_service.require_permission(identity, "create") + + @classmethod + def can_possibly_create(self, identity, topic, *args, **kwargs): + """ + used for checking whether there is any situation where the client can create a request of this type + it's different to just using can create with no receiver and data because that checks specifically + for situation without them while this method is used to check whether there is a possible situation + a user might create this request + eg. for the purpose of serializing a link on associated record + """ + + if open_request_exists(topic, self.type_id): + return False + try: + current_requests_service.require_permission(identity, "create") + except PermissionDeniedError: + return False + return True diff --git a/oarepo_requests/types/publish_draft.py b/oarepo_requests/types/publish_draft.py index 2ef92ad5..7df30d7f 100644 --- a/oarepo_requests/types/publish_draft.py +++ b/oarepo_requests/types/publish_draft.py @@ -3,10 +3,10 @@ from oarepo_requests.actions.publish_draft import PublishDraftAcceptAction -from .generic import OARepoRequestType +from .generic import NonDuplicableOARepoRequestType -class PublishDraftRequestType(OARepoRequestType): +class PublishDraftRequestType(NonDuplicableOARepoRequestType): available_actions = { **RequestType.available_actions, "accept": PublishDraftAcceptAction, diff --git a/oarepo_requests/utils.py b/oarepo_requests/utils.py index a346753f..75614556 100644 --- a/oarepo_requests/utils.py +++ b/oarepo_requests/utils.py @@ -4,8 +4,6 @@ from invenio_requests.resolvers.registry import ResolverRegistry from invenio_search.engine import dsl -from oarepo_requests.errors import OpenRequestAlreadyExists - def allowed_request_types_for_record(record): request_types = current_request_type_registry._registered_types @@ -53,7 +51,7 @@ def _reference_query_term(term, reference): ) -def request_exists( +def search_requests( identity, type_id, topic_reference=None, @@ -83,16 +81,15 @@ def request_exists( must=must, ), ) - return next(results.hits)["id"] if results.total > 0 else None + return results.hits def open_request_exists(topic_or_reference, type_id): topic_reference = ResolverRegistry.reference_entity(topic_or_reference, raise_=True) - existing_request = request_exists( + existing_requests = search_requests( system_identity, type_id, topic_reference=topic_reference, is_open=True ) - if existing_request: - raise OpenRequestAlreadyExists(existing_request, topic_or_reference) + return bool(list(existing_requests)) # TODO these things are related and possibly could be approached in a less convoluted manner? For example, global model->services map would help diff --git a/run-tests.sh b/run-tests.sh index ad20d7de..6b0d7f38 100755 --- a/run-tests.sh +++ b/run-tests.sh @@ -36,7 +36,6 @@ pip install "oarepo[tests]==$OAREPO_VERSION.*" pip install -e "./$BUILD_TEST_DIR/${MODEL}" pip install oarepo-ui pip install -e . -pip install -e ./tests/mock_requests pytest $BUILD_TEST_DIR/test_requests pytest $BUILD_TEST_DIR/test_ui \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index 6e89659e..e03fa9ef 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -29,6 +29,17 @@ def ret_data(record_id): return ret_data +@pytest.fixture() +def edit_record_data_function(): + def ret_data(record_id): + return { + "request_type": "thesis_edit_record", + "topic": {"thesis": record_id}, + } + + return ret_data + + @pytest.fixture() def delete_record_data_function(): def ret_data(record_id): @@ -140,10 +151,14 @@ def app_config(app_config): def default_receiver(*args, **kwargs): return {"user": "2"} + def none_receiver(*args, **kwargs): + return None + app_config["OAREPO_REQUESTS_DEFAULT_RECEIVER"] = { "thesis_publish_draft": default_receiver, "thesis_delete_record": default_receiver, "thesis_non_duplicable": default_receiver, + "thesis_edit_record": none_receiver, } """ diff --git a/tests/mock_requests/mock_requests/__init__.py b/tests/mock_requests/mock_requests/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/mock_requests/mock_requests/type.py b/tests/mock_requests/mock_requests/type.py deleted file mode 100644 index aca57f31..00000000 --- a/tests/mock_requests/mock_requests/type.py +++ /dev/null @@ -1,33 +0,0 @@ -from invenio_records_resources.services.errors import PermissionDeniedError -from invenio_requests.proxies import current_requests_service - -from oarepo_requests.errors import OpenRequestAlreadyExists -from oarepo_requests.types.generic import OARepoRequestType -from oarepo_requests.utils import open_request_exists - - -class NonDuplicableRequest(OARepoRequestType): - receiver_can_be_none = True - - def can_create(self, identity, data, receiver, topic, creator, *args, **kwargs): - open_request_exists(topic, self.type_id) - current_requests_service.require_permission(identity, "create") - - @classmethod - def can_possibly_create(self, identity, topic, *args, **kwargs): - """ - used for checking whether there is any situation where the client can create a request of this type - it's different to just using can create with no receiver and data because that checks specifically - for situation without them while this method is used to check whether there is a possible situation - a user might create this request - eg. for the purpose of serializing a link on associated record - """ - try: - open_request_exists(topic, self.type_id) - except OpenRequestAlreadyExists: - return False - try: - current_requests_service.require_permission(identity, "create") - except PermissionDeniedError: - return False - return True diff --git a/tests/mock_requests/setup.py b/tests/mock_requests/setup.py deleted file mode 100644 index 12065b4b..00000000 --- a/tests/mock_requests/setup.py +++ /dev/null @@ -1,10 +0,0 @@ -from setuptools import setup - -setup( - name="mock_requests", - packages=["mock_requests"], - install_requires=[ - "oarepo_requests", - "invenio_requests", - ], -) diff --git a/tests/test_requests/test_create_conditions.py b/tests/test_requests/test_create_conditions.py index 40dc774c..de01cb55 100644 --- a/tests/test_requests/test_create_conditions.py +++ b/tests/test_requests/test_create_conditions.py @@ -5,59 +5,112 @@ from .utils import link_api2testclient -def data(record_id): - return { - "request_type": "thesis_non_duplicable", - "topic": {"thesis_draft": record_id}, - } - - -def test_can_create(client_logged_as, identity_simple, users, urls, search_clear): - creator_client = client_logged_as(users[0].email) +def test_can_create( + logged_client_request, + identity_simple, + users, + urls, + publish_request_data_function, + search_clear, +): + creator = users[0] receiver = users[1] - draft1 = creator_client.post(urls["BASE_URL"], json={}) - draft2 = creator_client.post(urls["BASE_URL"], json={}) + draft1 = logged_client_request(creator, "post", urls["BASE_URL"], json={}) + draft2 = logged_client_request(creator, "post", urls["BASE_URL"], json={}) - resp_request_create = creator_client.post( - urls["BASE_URL_REQUESTS"], json=data(draft1.json["id"]) + resp_request_create = logged_client_request( + creator, + "post", + urls["BASE_URL_REQUESTS"], + json=publish_request_data_function(draft1.json["id"]), ) - resp_request_submit = creator_client.post( - link_api2testclient(resp_request_create.json["links"]["actions"]["submit"]) + resp_request_submit = logged_client_request( + creator, + "post", + link_api2testclient(resp_request_create.json["links"]["actions"]["submit"]), ) with pytest.raises(OpenRequestAlreadyExists): - resp_request_create_duplicated = creator_client.post( - urls["BASE_URL_REQUESTS"], json=data(draft1.json["id"]) + resp_request_create_duplicated = logged_client_request( + creator, + "post", + urls["BASE_URL_REQUESTS"], + json=publish_request_data_function(draft1.json["id"]), ) # should still be creatable for draft2 - create_for_request_draft2 = creator_client.post( - urls["BASE_URL_REQUESTS"], json=data(draft2.json["id"]) + create_for_request_draft2 = logged_client_request( + creator, + "post", + urls["BASE_URL_REQUESTS"], + json=publish_request_data_function(draft2.json["id"]), ) assert create_for_request_draft2.status_code == 201 + # try declining the request for draft2, we should be able to create again then + resp_request_submit = logged_client_request( + creator, + "post", + link_api2testclient( + create_for_request_draft2.json["links"]["actions"]["submit"] + ), + ) + + with pytest.raises(OpenRequestAlreadyExists): + create_for_request_draft2 = logged_client_request( + creator, + "post", + urls["BASE_URL_REQUESTS"], + json=publish_request_data_function(draft2.json["id"]), + ) + record = logged_client_request( + receiver, "get", f"{urls['BASE_URL']}{draft2.json['id']}/draft" + ) + decline = logged_client_request( + receiver, + "post", + link_api2testclient(record.json["requests"][0]["links"]["actions"]["decline"]), + ) + + resp_request_create_again = logged_client_request( + creator, + "post", + urls["BASE_URL_REQUESTS"], + json=publish_request_data_function(draft2.json["id"]), + ) + assert resp_request_create_again.status_code == 201 + def test_can_possibly_create( - client_logged_as, identity_simple, users, urls, search_clear + logged_client_request, + identity_simple, + users, + urls, + publish_request_data_function, + search_clear, ): - creator_client = client_logged_as(users[0].email) - receiver_client = client_logged_as(users[1].email) + creator = users[0] receiver = users[1] - draft1 = creator_client.post(urls["BASE_URL"], json={}) - draft2 = creator_client.post(urls["BASE_URL"], json={}) + draft1 = logged_client_request(creator, "post", urls["BASE_URL"], json={}) + draft2 = logged_client_request(creator, "post", urls["BASE_URL"], json={}) - record_resp_no_request = receiver_client.get( - f"{urls['BASE_URL']}{draft1.json['id']}/draft" + record_resp_no_request = logged_client_request( + receiver, "get", f"{urls['BASE_URL']}{draft1.json['id']}/draft" ) - resp_request_create = creator_client.post( - urls["BASE_URL_REQUESTS"], json=data(draft1.json["id"]) + resp_request_create = logged_client_request( + creator, + "post", + urls["BASE_URL_REQUESTS"], + json=publish_request_data_function(draft1.json["id"]), ) - resp_request_submit = creator_client.post( - link_api2testclient(resp_request_create.json["links"]["actions"]["submit"]) + resp_request_submit = logged_client_request( + creator, + "post", + link_api2testclient(resp_request_create.json["links"]["actions"]["submit"]), ) def find_request_type(requests, type): @@ -66,21 +119,21 @@ def find_request_type(requests, type): return request return None - record_resp_with_request = receiver_client.get( - f"{urls['BASE_URL']}{draft1.json['id']}/draft" + record_resp_with_request = logged_client_request( + receiver, "get", f"{urls['BASE_URL']}{draft1.json['id']}/draft" ) - record_resp_draft2 = receiver_client.get( - f"{urls['BASE_URL']}{draft2.json['id']}/draft" + record_resp_draft2 = logged_client_request( + receiver, "get", f"{urls['BASE_URL']}{draft2.json['id']}/draft" ) assert find_request_type( - record_resp_no_request.json["request_types"], "thesis_non_duplicable" + record_resp_no_request.json["request_types"], "thesis_publish_draft" ) assert find_request_type( - record_resp_draft2.json["request_types"], "thesis_non_duplicable" + record_resp_draft2.json["request_types"], "thesis_publish_draft" ) assert ( find_request_type( - record_resp_with_request.json["request_types"], "thesis_non_duplicable" + record_resp_with_request.json["request_types"], "thesis_publish_draft" ) is None ) diff --git a/tests/test_requests/test_edit.py b/tests/test_requests/test_edit.py new file mode 100644 index 00000000..0cf64741 --- /dev/null +++ b/tests/test_requests/test_edit.py @@ -0,0 +1,51 @@ +from thesis.records.api import ThesisDraft, ThesisRecord + +from tests.test_requests.utils import link_api2testclient + + +def test_edit_autoaccept( + logged_client_request, + identity_simple, + users, + urls, + edit_record_data_function, + record_factory, + search_clear, +): + creator = users[0] + record1 = record_factory() + + resp_request_create = logged_client_request( + creator, + "post", + urls["BASE_URL_REQUESTS"], + json=edit_record_data_function(record1["id"]), + ) + resp_request_submit = logged_client_request( + creator, + "post", + link_api2testclient(resp_request_create.json["links"]["actions"]["submit"]), + ) + # is request accepted and closed? + request = logged_client_request( + creator, + "get", + f'{urls["BASE_URL_REQUESTS"]}{resp_request_create.json["id"]}', + ).json + + assert request["status"] == "accepted" + assert not request["is_open"] + assert request["is_closed"] + + ThesisRecord.index.refresh() + ThesisDraft.index.refresh() + # edit action worked? + search = logged_client_request( + creator, + "get", + f'user{urls["BASE_URL"]}', + ).json[ + "hits" + ]["hits"] + assert len(search) == 1 + assert search[0]["links"]["self"].endswith("/draft") diff --git a/tests/thesis.yaml b/tests/thesis.yaml index 887abbb0..12e0424c 100644 --- a/tests/thesis.yaml +++ b/tests/thesis.yaml @@ -14,11 +14,6 @@ record: - PublishDraftRequestType imports: - import: oarepo_requests.types.publish_draft.PublishDraftRequestType - non-duplicable: - base-classes: - - NonDuplicableRequest - imports: - - import: mock_requests.type.NonDuplicableRequest requests: types: delete-record: @@ -26,7 +21,11 @@ record: - DeleteRecordRequestType imports: - import: oarepo_requests.types.delete_record.DeleteRecordRequestType - generic-request: {} + edit-record: + base-classes: + - EditRecordRequestType + imports: + - import: oarepo_requests.types.edit_record.EditRecordRequestType settings: schema-server: 'local://'