Merge pull request #99 from oarepo/krist/be-593-change-redirect_urls

Krist/be 593 change redirect urls

oarepo_requests/actions/edit_topic.py

@@ -13,7 +13,6 @@
 
 from oarepo_runtime.datastreams.utils import get_record_service_for_record
 
-from .cascade_events import update_topic
 from .generic import AddTopicLinksOnPayloadMixin, OARepoAcceptAction
 
 if TYPE_CHECKING:
@@ -44,5 +43,4 @@ def apply(
         if not topic_service:
             raise KeyError(f"topic {topic} service not found")
         edit_topic = topic_service.edit(identity, topic["id"], uow=uow)
-        update_topic(self.request, topic, edit_topic._record, uow)
         super().apply(identity, request_type, edit_topic, uow, *args, **kwargs)

oarepo_requests/actions/generic.py

@@ -52,7 +52,6 @@ def apply(
         **kwargs: Any,
     ) -> None:
         """Apply the action to the topic."""
-        pass
 
     def _execute_with_components(
         self,
@@ -127,8 +126,13 @@ def apply(
         # invenio does not allow non-string values in the payload, so using colon notation here
         # client will need to handle this and convert to links structure
         # can not use dot notation as marshmallow tries to be too smart and does not serialize dotted keys
-        request["payload"][self.self_link] = topic_dict["links"]["self"]
-        request["payload"][self.self_html_link] = topic_dict["links"]["self_html"]
+        if (
+            "self" in topic_dict["links"]
+        ):  # todo consider - this happens if receiver doesn't have read rights to the topic, like after a draft is created after edit
+            # if it's needed in all cases, we could do a system identity call here
+            request["payload"][self.self_link] = topic_dict["links"]["self"]
+        if "self_html" in topic_dict["links"]:
+            request["payload"][self.self_html_link] = topic_dict["links"]["self_html"]
         return topic._record
 
 

oarepo_requests/actions/new_version.py

@@ -13,7 +13,6 @@
 
 from oarepo_runtime.datastreams.utils import get_record_service_for_record
 
-from .cascade_events import update_topic
 from .generic import AddTopicLinksOnPayloadMixin, OARepoAcceptAction
 
 if TYPE_CHECKING:
@@ -50,7 +49,11 @@ def apply(
             and self.request["payload"]["keep_files"] == "true"
         ):
             topic_service.import_files(identity, new_version_topic.id)
-        update_topic(self.request, topic, new_version_topic._record, uow)
+
+        if "payload" not in self.request:
+            self.request["payload"] = {}
+        self.request["payload"]["draft_record:id"] = new_version_topic["id"]
+
         return super().apply(
             identity, request_type, new_version_topic, uow, *args, **kwargs
         )

oarepo_requests/resolvers/interface.py

@@ -0,0 +1,47 @@
+from __future__ import annotations
+
+from typing import Any, TYPE_CHECKING
+
+from invenio_pidstore.errors import PersistentIdentifierError
+
+from oarepo_requests.resolvers.ui import resolve
+import logging
+if TYPE_CHECKING:
+    from invenio_requests.records import Request
+log = logging.getLogger(__name__)
+
+# todo consider - we are not using this strictly in the ui context - so how should we separate these things in the future
+def resolve_entity(entity: str, obj: Request, ctx: dict[str, Any]) -> dict:
+    """Resolve the entity and put it into the context cache.
+
+    :param obj: Request object
+    :param ctx: Context cache
+    :return: The resolved entity
+    """
+    entity_field_value = getattr(obj, entity)
+    if not entity_field_value:
+        return {}
+
+    reference_dict: dict = entity_field_value.reference_dict
+
+    key = entity_context_key(reference_dict)
+    if key in ctx:
+        return ctx[key]
+    try:
+        entity = resolve(ctx["identity"], reference_dict)
+    except Exception as e:  # noqa
+        if not isinstance(e, PersistentIdentifierError):
+            log.exception(
+                "Error resolving %s for identity %s",
+                reference_dict,
+                ctx["identity"],
+            )
+        entity = {"links": {}}
+    ctx[key] = entity
+    return entity
+
+
+def entity_context_key(reference_dict: dict) -> str:
+    return "entity:" + ":".join(
+        f"{x[0]}:{x[1]}" for x in sorted(reference_dict.items())
+    )

oarepo_requests/services/oarepo/config.py

@@ -10,14 +10,13 @@
 from __future__ import annotations
 
 import logging
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING
 
-from invenio_pidstore.errors import PersistentIdentifierError
 from invenio_records_resources.services.base.links import Link
 from invenio_requests.services import RequestsServiceConfig
 from invenio_requests.services.requests import RequestLink
 
-from oarepo_requests.resolvers.ui import resolve
+from oarepo_requests.resolvers.interface import resolve_entity
 
 if TYPE_CHECKING:
     from invenio_requests.records.api import Request
@@ -27,36 +26,6 @@
 class RequestEntityLinks(Link):
     """Utility class for keeping track of and resolve links."""
 
-    def _resolve(self, obj: Request, ctx: dict[str, Any]) -> dict:
-        """Resolve the entity and put it into the context cache.
-
-        :param obj: Request object
-        :param ctx: Context cache
-        :return: The resolved entity
-        """
-        entity_field_value = getattr(obj, self._entity)
-        if not entity_field_value:
-            return {}
-
-        reference_dict: dict = entity_field_value.reference_dict
-        key = "entity:" + ":".join(
-            f"{x[0]}:{x[1]}" for x in sorted(reference_dict.items())
-        )
-        if key in ctx:
-            return ctx[key]
-        try:
-            entity = resolve(ctx["identity"], reference_dict)
-        except Exception as e:  # noqa
-            if not isinstance(e, PersistentIdentifierError):
-                log.exception(
-                    "Error resolving %s for identity %s",
-                    reference_dict,
-                    ctx["identity"],
-                )
-            entity = {"links": {}}
-        ctx[key] = entity
-        return entity
-
     def __init__(self, entity: str, when: callable = None):
         """Constructor."""
         self._entity = entity
@@ -65,36 +34,24 @@ def __init__(self, entity: str, when: callable = None):
     def expand(self, obj: Request, context: dict) -> dict:
         """Create the request links."""
         res = {}
-        resolved = self._resolve(obj, context)
+        resolved = resolve_entity(self._entity, obj, context)
         if "links" in resolved:
             res.update(resolved["links"])
 
-        if hasattr(obj.type, "extra_entity_links"):
-            entity = self._resolve(obj, context)
-            res.update(
-                obj.type.extra_entity_links(
-                    request=obj, entity=entity, entity_type=self._entity
-                )
-            )
-
         return res
 
 
-class RedirectLinks(Link):
+class RedirectLink(Link):
     def __init__(self, when: callable = None):
         """Constructor."""
         self._when_func = when
 
     def expand(self, obj: Request, context: dict) -> dict:
         """Create the request links."""
-        links = {}
-        available_statuses = {"accept", "decline", "submit"}
-        for status in available_statuses:
-            if hasattr(obj.type, f"{status}_redirect_url"):
-                links[status] = getattr(obj.type, f"{status}_redirect_url")(
-                    obj, context
-                )
-        return links
+        link = None
+        if hasattr(obj.type, "get_ui_redirect_url"):
+            link = getattr(obj.type, "get_ui_redirect_url")(obj, context)
+        return link
 
 
 class OARepoRequestsServiceConfig(RequestsServiceConfig):
@@ -110,5 +67,5 @@ class OARepoRequestsServiceConfig(RequestsServiceConfig):
         "topic": RequestEntityLinks(entity="topic"),
         "created_by": RequestEntityLinks(entity="created_by"),
         "receiver": RequestEntityLinks(entity="receiver"),
-        "redirect_urls": RedirectLinks(),
+        "ui_redirect_url": RedirectLink(),
     }

oarepo_requests/types/delete_draft.py

@@ -36,10 +36,11 @@ class DeleteDraftRequestType(NonDuplicableOARepoRequestType):
 
     dangerous = True
 
-    def accept_redirect_url(self, request: Request, context: dict, **kwargs):
-        topic_cls = request.topic.record_cls
-        service = get_record_service_for_record_class(topic_cls)
-        return service.config.links_search["self_html"].expand(None, context)
+    def get_ui_redirect_url(self, request: Request, context: dict) -> str:
+        if request.status == "accepted":
+            topic_cls = request.topic.record_cls
+            service = get_record_service_for_record_class(topic_cls)
+            return service.config.links_search["self_html"].expand(None, context)
 
     @classproperty
     def available_actions(cls) -> dict[str, type[RequestAction]]:

oarepo_requests/types/delete_published_record.py

@@ -38,10 +38,11 @@ class DeletePublishedRecordRequestType(NonDuplicableOARepoRequestType):
     type_id = "delete_published_record"
     name = _("Delete record")
 
-    def accept_redirect_url(self, request: Request, context: dict, **kwargs):
-        topic_cls = request.topic.record_cls
-        service = get_record_service_for_record_class(topic_cls)
-        return service.config.links_search["self_html"].expand(None, context)
+    def get_ui_redirect_url(self, request: Request, context: dict) -> str:
+        if request.status == "accepted":
+            topic_cls = request.topic.record_cls
+            service = get_record_service_for_record_class(topic_cls)
+            return service.config.links_search["self_html"].expand(None, context)
 
     dangerous = True
 

oarepo_requests/types/edit_record.py

@@ -12,9 +12,12 @@
 from typing import TYPE_CHECKING, Any
 
 import marshmallow as ma
+from invenio_drafts_resources.resources.records.errors import DraftNotCreatedError
+from invenio_records_resources.services.errors import PermissionDeniedError
 from invenio_records_resources.services.uow import RecordCommitOp, UnitOfWork
 from invenio_requests.proxies import current_requests_service
 from invenio_requests.records.api import Request
+from oarepo_runtime.datastreams.utils import get_record_service_for_record_class
 from oarepo_runtime.i18n import lazy_gettext as _
 from typing_extensions import override
 
@@ -54,11 +57,20 @@ class EditPublishedRecordRequestType(NonDuplicableOARepoRequestType):
         ),
     }
 
-    def extra_entity_links(self, request: Request, entity: dict, entity_type: str, **kwargs) -> dict:
-        if request.status == "accepted" and entity_type == "topic":
-            return {"topic_redirect_link": entity["links"]["edit_html"]}
-        else:
-            return {}
+    def get_ui_redirect_url(self, request: Request, ctx: dict) -> str:
+        if request.status == "accepted":
+            service = get_record_service_for_record_class(request.topic.record_cls)
+            try:
+                result_item = service.read_draft(
+                    ctx["identity"], request.topic._parse_ref_dict_id()
+                )
+            except (PermissionDeniedError, DraftNotCreatedError):
+                return None
+
+            if "edit_html" in result_item["links"]:
+                return result_item["links"]["edit_html"]
+            elif "self_html" in result_item["links"]:
+                return result_item["links"]["self_html"]
 
     @classproperty
     def available_actions(cls) -> dict[str, type[RequestAction]]:

oarepo_requests/types/new_version.py

@@ -12,9 +12,12 @@
 from typing import TYPE_CHECKING, Any
 
 import marshmallow as ma
+from invenio_drafts_resources.resources.records.errors import DraftNotCreatedError
+from invenio_records_resources.services.errors import PermissionDeniedError
 from invenio_records_resources.services.uow import RecordCommitOp, UnitOfWork
 from invenio_requests.proxies import current_requests_service
 from marshmallow.validate import OneOf
+from oarepo_runtime.datastreams.utils import get_record_service_for_record_class
 from oarepo_runtime.i18n import lazy_gettext as _
 from typing_extensions import override
 
@@ -47,14 +50,27 @@ class NewVersionRequestType(NonDuplicableOARepoRequestType):
             attribute="draft_record:links:self_html",
             data_key="draft_record:links:self_html",
         ),
+        "draft_record.id": ma.fields.Str(
+            attribute="draft_record:id",
+            data_key="draft_record:id",
+        ),
         "keep_files": ma.fields.String(validate=OneOf(["true", "false"])),
     }
 
-    def extra_entity_links(self, request: Request, entity: dict, entity_type: str, **kwargs) -> dict:
-        if request.status == "accepted" and entity_type == "topic":
-            return {"topic_redirect_link": entity["links"]["edit_html"]}
-        else:
-            return {}
+    def get_ui_redirect_url(self, request: Request, ctx: dict) -> str:
+        if request.status == "accepted":
+            service = get_record_service_for_record_class(request.topic.record_cls)
+            try:
+                result_item = service.read_draft(
+                    ctx["identity"], request["payload"]["draft_record:id"]
+                )
+            except (PermissionDeniedError, DraftNotCreatedError):
+                return None
+
+            if "edit_html" in result_item["links"]:
+                return result_item["links"]["edit_html"]
+            elif "self_html" in result_item["links"]:
+                return result_item["links"]["self_html"]
 
     @classproperty
     def available_actions(cls) -> dict[str, type[RequestAction]]:

setup.cfg

@@ -1,6 +1,6 @@
 [metadata]
 name = oarepo-requests
-version = 2.3.10
+version = 2.3.11
 description =
 authors = Ronald Krist <[email protected]>
 readme = README.md

tests/conftest.py

@@ -141,6 +141,16 @@ class RequestsWithDifferentRecipients(DefaultRequests):
         requesters=[AnyUser()],
         recipients=[UserGenerator(1)],
     )
+    edit_published_record = WorkflowRequest(
+        requesters=[IfNoEditDraft([IfInState("published", [RecordOwners()])])],
+        recipients=[UserGenerator(2)],
+        transitions=WorkflowTransitions(),
+    )
+    new_version = WorkflowRequest(
+        requesters=[IfNoNewVersionDraft([IfInState("published", [RecordOwners()])])],
+        recipients=[UserGenerator(2)],
+        transitions=WorkflowTransitions(),
+    )
 
 
 class RequestsWithApprove(WorkflowRequestPolicy):
@@ -638,7 +648,7 @@ def record(identity, custom_workflow=None, additional_data=None):
         json = copy.deepcopy(default_workflow_json)
         if custom_workflow:  # specifying this assumes use of workflows
             json["parent"]["workflow"] = custom_workflow
-        json = {
+        json_metadata = {
             "metadata": {
                 "creators": [
                     "Creator 1",
@@ -647,7 +657,7 @@ def record(identity, custom_workflow=None, additional_data=None):
                 "contributors": ["Contributor 1"],
             }
         }
-        json = always_merger.merge(json, default_workflow_json)
+        json = always_merger.merge(json, json_metadata)
         if additional_data:
             always_merger.merge(json, additional_data)
         draft = record_service.create(identity, json)

tests/test_requests/test_delete.py

@@ -51,8 +51,7 @@ def test_delete(
         ),
     )
     assert (
-            link2testclient(delete.json["links"]["redirect_urls"]["accept"], ui=True)
-            == "/thesis/"
+        link2testclient(delete.json["links"]["ui_redirect_url"], ui=True) == "/thesis/"
     )
 
     ThesisRecord.index.refresh()
@@ -134,7 +133,7 @@ def test_delete_draft(
     assert request_after.json["status"] == "accepted"
     assert request_after.json["is_closed"]
     assert (
-            link2testclient(request_after.json["links"]["redirect_urls"]["accept"], ui=True)
-            == "/thesis/"
+        link2testclient(request_after.json["links"]["ui_redirect_url"], ui=True)
+        == "/thesis/"
     )
     assert read_deleted.status_code == 404