Merge remote-tracking branch 'origin/main' into krist/be-443-configur…

…e-notifications

# Conflicts:
#	tests/conftest.py

oarepo_requests/actions/cascade_events.py

@@ -74,14 +74,19 @@ def update_topic(
     old_topic_ref = _get_topic_reference(old_topic)
     requests_with_topic = _get_requests_with_topic_reference(old_topic_ref)
     new_topic_ref = ResolverRegistry.reference_entity(new_topic)
-    for request_from_search in requests_with_topic:
+    for (
+        request_from_search
+    ) in (
+        requests_with_topic._results
+    ):  # result list links might crash before update of the topic
+        request_from_search_id = request_from_search["uuid"]
         request_type = current_request_type_registry.lookup(
             request_from_search["type"], quiet=True
         )
         if hasattr(request_type, "topic_change"):
             cur_request = (
-                Request.get_record(request_from_search["id"])
-                if request_from_search["id"] != str(request.id)
+                Request.get_record(request_from_search_id)
+                if request_from_search_id != str(request.id)
                 else request
             )  # request on which the action is executed is recommited later, the change must be done on the same instance
             request_type.topic_change(cur_request, new_topic_ref, uow)
@@ -103,14 +108,19 @@ def cancel_requests_on_topic_delete(
 
     topic_ref = _get_topic_reference(topic)
     requests_with_topic = _get_requests_with_topic_reference(topic_ref)
-    for request_from_search in requests_with_topic:
+    for (
+        request_from_search
+    ) in (
+        requests_with_topic._results
+    ):  # result list links might crash before update of the topic
+        request_from_search_id = request_from_search["uuid"]
         request_type = current_request_type_registry.lookup(
             request_from_search["type"], quiet=True
         )
         if hasattr(request_type, "on_topic_delete"):
-            if request_from_search["id"] == str(request.id):
+            if request_from_search_id == str(request.id):
                 continue
-            cur_request = Request.get_record(request_from_search["id"])
+            cur_request = Request.get_record(request_from_search_id)
             if cur_request.is_open:
                 request_type.on_topic_delete(
                     cur_request, uow

oarepo_requests/actions/components.py

@@ -14,20 +14,16 @@
 
 import abc
 import contextlib
-from typing import (
-    TYPE_CHECKING,
-    Any,
-    ContextManager,
-    Generator,
-    override,
-)
+from typing import TYPE_CHECKING, Any, override
 
 from invenio_requests.customizations import RequestAction, RequestActions, RequestType
 from invenio_requests.errors import CannotExecuteActionError
 
 from oarepo_requests.services.permissions.identity import request_active
 
 if TYPE_CHECKING:
+    from collections.abc import Generator
+
     from flask_principal import Identity
     from invenio_records_resources.services.uow import UnitOfWork
     from invenio_requests.records.api import Request
@@ -52,7 +48,7 @@ def apply(
         uow: UnitOfWork,
         *args: Any,
         **kwargs: Any,
-    ) -> ContextManager:
+    ) -> contextlib.AbstractContextManager:
         """Apply the component.
 
         Must return a context manager
@@ -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)

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

@@ -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
@@ -52,7 +52,6 @@ def apply(
         **kwargs: Any,
     ) -> None:
         """Apply the action to the topic."""
-        pass
 
     def _execute_with_components(
         self,
@@ -95,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 PersistentIdentifierError:
+            topic = None
         self._execute_with_components(
             self.components, identity, request_type, topic, uow, *args, **kwargs
         )
@@ -127,8 +129,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
 
 
@@ -150,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"

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:
@@ -47,10 +46,14 @@ def apply(
         if (
             "payload" in self.request
             and "keep_files" in self.request["payload"]
-            and self.request["payload"]["keep_files"] == "true"
+            and self.request["payload"]["keep_files"] == "yes"
         ):
             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/actions/publish_draft.py

@@ -33,35 +33,39 @@
     from invenio_requests.customizations import RequestType
     from invenio_requests.customizations.actions import RequestAction
 
+
 class PublishMixin:
     """Mixin for publish actions."""
 
     def can_execute(self: RequestAction) -> bool:
         """Check if the action can be executed."""
-        if not super().can_execute():       # type: ignore
+        if not super().can_execute():  # type: ignore
             return False
 
         try:
             from ..types.publish_draft import PublishDraftRequestType
+
             topic = self.request.topic.resolve()
             PublishDraftRequestType.validate_topic(system_identity, topic)
             return True
-        except: # noqa E722: used for displaying buttons, so ignore errors here
+        except:  # noqa E722: used for displaying buttons, so ignore errors here
             return False
 
+
 class PublishDraftSubmitAction(PublishMixin, OARepoSubmitAction):
     """Submit action for publishing draft requests."""
 
 
-class PublishDraftAcceptAction(PublishMixin, AddTopicLinksOnPayloadMixin, OARepoAcceptAction):
+class PublishDraftAcceptAction(
+    PublishMixin, AddTopicLinksOnPayloadMixin, OARepoAcceptAction
+):
     """Accept action for publishing draft requests."""
 
     self_link = "published_record:links:self"
     self_html_link = "published_record:links:self_html"
 
     name = _("Publish")
 
-
     def apply(
         self,
         identity: Identity,

oarepo_requests/config.py

@@ -7,6 +7,8 @@
 #
 """Default configuration of oarepo-requests."""
 
+from __future__ import annotations
+
 import invenio_requests.config
 import oarepo_workflows  # noqa
 from invenio_requests.customizations import CommentEventType, LogEventType

oarepo_requests/invenio_patches.py

@@ -9,12 +9,13 @@
 
 from __future__ import annotations
 
-from functools import cached_property
+from functools import cached_property, partial
 from typing import TYPE_CHECKING, Any, Callable
 
 from flask_resources import JSONSerializer, ResponseHandler
 from invenio_records_resources.resources.records.headers import etag_headers
 from invenio_records_resources.services.records.params import FilterParam
+from invenio_records_resources.services.records.params.base import ParamInterpreter
 from invenio_requests.resources.events.config import RequestCommentsResourceConfig
 from invenio_requests.resources.requests.config import (
     RequestSearchRequestArgsSchema,
@@ -29,13 +30,11 @@
 from marshmallow import fields
 from opensearch_dsl.query import Bool
 
-from oarepo_requests.proxies import current_oarepo_requests
 from oarepo_requests.resources.ui import (
     OARepoRequestEventsUIJSONSerializer,
     OARepoRequestsUIJSONSerializer,
 )
 from oarepo_requests.services.oarepo.config import OARepoRequestsServiceConfig
-from oarepo_requests.utils import create_query_term_for_reference
 
 if TYPE_CHECKING:
     from flask.blueprints import BlueprintSetupState
@@ -55,23 +54,40 @@ def apply(self, identity: Identity, search: Query, params: dict[str, str]) -> Qu
         return search
 
 
-class RequestReceiverFilterParam(FilterParam):
-    """Filter requests by receiver.
+class RequestAllAvailableFilterParam(ParamInterpreter):
+    """A no-op filter that returns all requests that are readable by the current user."""
 
-    Note: This is different from the invenio handling. Invenio requires receiver to be
-    a user, we handle it as a more generic reference.
+    def __init__(self, param_name, config):
+        """Initialize the filter."""
+        self.param_name = param_name
+        super().__init__(config)
+
+    @classmethod
+    def factory(cls, param=None):
+        """Create a new filter parameter."""
+        return partial(cls, param)
+
+    def apply(self, identity, search, params):
+        """Apply the filter to the search - does nothing."""
+        params.pop(self.param_name, None)
+        return search
+
+
+class RequestNotOwnerFilterParam(FilterParam):
+    """Filter requests that are not owned by the current user.
+
+    Note: invenio still does check that the user has the right to see the request,
+    so this is just a filter to narrow down the search to requests, that the user
+    can approve.
     """
 
     def apply(self, identity: Identity, search: Query, params: dict[str, str]) -> Query:
         """Apply the filter to the search."""
         value = params.pop(self.param_name, None)
-        terms = dsl.Q("match_none")
         if value is not None:
-            references = current_oarepo_requests.identity_to_entity_references(identity)
-            for reference in references:
-                query_term = create_query_term_for_reference(self.field_name, reference)
-                terms |= query_term
-            search = search.filter(Bool(filter=terms))
+            search = search.filter(
+                Bool(must_not=[dsl.Q("term", **{self.field_name: identity.id})])
+            )
         return search
 
 
@@ -92,7 +108,8 @@ class EnhancedRequestSearchOptions(RequestSearchOptions):
 
     params_interpreters_cls = RequestSearchOptions.params_interpreters_cls + [
         RequestOwnerFilterParam.factory("mine", "created_by.user"),
-        RequestReceiverFilterParam.factory("assigned", "receiver"),
+        RequestNotOwnerFilterParam.factory("assigned", "created_by.user"),
+        RequestAllAvailableFilterParam.factory("all"),
         IsClosedParam.factory("is_closed"),
     ]
 
@@ -102,6 +119,7 @@ class ExtendedRequestSearchRequestArgsSchema(RequestSearchRequestArgsSchema):
 
     mine = fields.Boolean()
     assigned = fields.Boolean()
+    all = fields.Boolean()
     is_closed = fields.Boolean()
 
 

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())
+    )