More tests

.coveragerc

@@ -0,0 +1,4 @@
+[report]
+exclude_lines =
+    pragma: no cover
+    if TYPE_CHECKING:

oarepo_workflows/errors.py

@@ -24,8 +24,11 @@ def _get_id_from_record(record: Record | dict) -> str:
     :return str: The id of the record.
     """
     # community record doesn't have id in dict form, only uuid
-    if "id" in record:
-        return str(record["id"])
+    try:
+        if "id" in record:
+            return str(record["id"])
+    except TypeError:
+        pass
     if hasattr(record, "id"):
         return str(record.id)
     return str(record)

oarepo_workflows/requests/generators.py

@@ -10,6 +10,7 @@
 from __future__ import annotations
 
 import dataclasses
+from collections.abc import Iterable
 from typing import TYPE_CHECKING, Any, Optional
 
 from invenio_access import SystemRoleNeed
@@ -50,19 +51,23 @@ def excludes(self, **context: Any) -> set[Need]:
             for exclude in generator.excludes(**context)
         }
 
-    def query_filters(self, **context: Any) -> list[dict]:
+    def query_filter(self, **context: Any) -> list[dict]:
         """Generate a list of opensearch query filters.
 
          These filters are used to filter objects. These objects are governed by a policy
          containing this generator.
 
         :param context: Context.
         """
-        return [
-            query_filter
-            for generator in self.generators
-            for query_filter in generator.query_filter(**context)
-        ]
+        ret = []
+        for generator in self.generators:
+            query_filter = generator.query_filter(**context)
+            if query_filter:
+                if isinstance(query_filter, Iterable):
+                    ret.extend(query_filter)
+                else:
+                    ret.append(query_filter)
+        return ret
 
 
 auto_request_need = SystemRoleNeed("auto_request")
@@ -89,7 +94,7 @@ def reference_receivers(
         record: Optional[Record] = None,
         request_type: Optional[RequestType] = None,
         **context: Any,
-    ) -> list[dict[str, str]]:
+    ) -> list[dict[str, str]]: # pragma: no cover
         """Return the reference receiver(s) of the request.
 
         This call requires the context to contain at least "record" and "request_type"
@@ -119,4 +124,19 @@ def reference_receivers(
 
         Returning "auto_approve" is a signal to the workflow that the request should be auto-approved.
         """
-        return [{"auto_approve": "true"}]
+        return [{"auto_approve": "True"}]
+
+    def needs(self, **kwargs):
+        """Get needs that signal workflow to automatically approve the request."""
+        raise ValueError("Auto-approve generator can not create needs and "
+                         "should be used only in `recipient` section of WorkflowRequest.")
+
+    def excludes(self, **kwargs):
+        """Get needs that signal workflow to automatically approve the request."""
+        raise ValueError("Auto-approve generator can not create needs and "
+                         "should be used only in `recipient` section of WorkflowRequest.")
+
+    def query_filter(self, **kwargs):
+        """Get needs that signal workflow to automatically approve the request."""
+        raise ValueError("Auto-approve generator can not create needs and "
+                         "should be used only in `recipient` section of WorkflowRequest.")

oarepo_workflows/requests/requests.py

@@ -109,13 +109,11 @@ class WorkflowTransitions:
 
     def __getitem__(self, transition_name: str):
         """Get the transition by name."""
-        try:
-            return getattr(self, transition_name)
-        except AttributeError:
+        if transition_name not in ["submitted", "accepted", "declined"]:
             raise KeyError(
                 f"Transition {transition_name} not defined in {self.__class__.__name__}"
-            ) from None
-
+            )
+        return getattr(self, transition_name)
 
 @dataclasses.dataclass
 class WorkflowRequestEscalation:

oarepo_workflows/services/components/workflow.py

@@ -37,12 +37,16 @@ def create(
     ) -> None:
         """Implement record creation checks and set the workflow on the created record."""
         if not data:
-            raise MissingWorkflowError("Workflow not defined in input.", record=data)
+            # sanity check, should be handled by policy before the component is called
+            raise MissingWorkflowError("Workflow not defined in input. As this should be handled by a policy, "
+                                       "make sure you are using workflow-enabled policy.", record=data)   # pragma: no cover
         try:
             workflow_id = data["parent"]["workflow"]
-        except KeyError as e:
-            raise MissingWorkflowError(
-                "Workflow not defined in input.", record=data
+        except KeyError as e:               # pragma: no cover
+            # sanity check, should be handled by policy before the component is called
+            raise MissingWorkflowError(     # pragma: no cover
+                "Workflow not defined in input. As this should be handled by a policy, "
+                "make sure you are using workflow-enabled policy.", record=data
             ) from e
 
         current_oarepo_workflows.set_workflow(

oarepo_workflows/services/permissions/generators.py

@@ -106,7 +106,11 @@ def needs(self, record: Optional[Record] = None, **context: Any) -> set[Need]:
         ).needs
 
     def query_filter(self, record: Optional[Record] = None, **context: Any) -> dict:
-        """Return query filters that are generated by the workflow permission."""
+        """Return query filters that are generated by the workflow permission.
+
+           Note: this implementation in fact will be called from WorkflowRecordPermissionPolicy.query_filters
+           for each registered workflow type. The query_filters are then combined into a single query.
+        """
         return self._get_permissions_from_workflow(
             record, self._action, **context
         ).query_filters

ruff.toml

@@ -1,3 +1,7 @@
+exclude = [
+    "tests"
+]
+
 [lint]
 extend-select = [
     "UP",   # pyupgrade

tests/test_auto_approve.py

@@ -0,0 +1,33 @@
+from oarepo_workflows.requests.generators import AutoApprove, auto_approve_need
+import pytest
+from invenio_requests.resolvers.registry import ResolverRegistry
+
+from oarepo_workflows.resolvers.auto_approve import AutoApproveProxy, AutoApproveEntity
+from invenio_access.permissions import system_identity
+
+
+def test_auto_approve_generator():
+    a = AutoApprove()
+
+    with pytest.raises(ValueError):
+        a.needs()
+
+    with pytest.raises(ValueError):
+        a.excludes()
+
+    with pytest.raises(ValueError):
+        a.query_filter()
+
+    assert a.reference_receivers() == [{"auto_approve": "True"}]
+
+
+def test_auto_approve_resolver(app):
+    resolved = ResolverRegistry.resolve_entity({"auto_approve": "True"})
+    assert isinstance(resolved, AutoApproveEntity)
+    #
+    # assert resolved.resolve() == AutoApprove()
+    # assert resolved.pick_resolved_fields(system_identity, {"id": "true"}) == {"auto_approve": "true"}
+    # assert resolved.get_needs() == []
+
+    entity_reference = ResolverRegistry.reference_entity(AutoApproveEntity())
+    assert entity_reference == {"auto_approve": "True"}

tests/test_auto_request.py

@@ -0,0 +1,6 @@
+from oarepo_workflows import AutoRequest
+from oarepo_workflows.requests.generators import auto_request_need
+
+
+def test_auto_request_needs(app):
+    assert AutoRequest().needs() == [auto_request_need]

tests/test_workflow.py

@@ -12,6 +12,16 @@
 from thesis.thesis.records.api import ThesisDraft, ThesisRecord
 
 
+def test_create_without_workflow(users, logged_client, default_workflow_json, search_clear):
+    # create draft
+    user_client1 = logged_client(users[0])
+
+    create_response = user_client1.post(
+        ThesisResourceConfig.url_prefix, json={}
+    )
+    assert create_response.status_code == 400
+    assert create_response.json["errors"][0]["messages"] == ['Workflow not defined in input.']
+
 def test_workflow_read(users, logged_client, default_workflow_json, search_clear):
     # create draft
     user_client1 = logged_client(users[0])

tests/test_workflow_permission.py

@@ -0,0 +1,28 @@
+from types import SimpleNamespace
+
+from oarepo_workflows import WorkflowPermission
+from oarepo_workflows.errors import MissingWorkflowError
+from thesis.thesis.records.api import ThesisRecord
+import pytest
+
+from flask_principal import Identity, UserNeed
+
+
+def test_get_workflow_id(users, logged_client, search_clear, record_service):
+    thesis = ThesisRecord.create({})
+    wp = WorkflowPermission("read")
+    with pytest.raises(MissingWorkflowError):
+        wp._get_workflow_id(record=thesis)
+
+    fake_thesis = SimpleNamespace(parent=SimpleNamespace(workflow=""))
+    with pytest.raises(MissingWorkflowError):
+        assert wp._get_workflow_id(record=fake_thesis)  # noqa
+
+def test_query_filter(users, logged_client, search_clear, record_service):
+    wp = WorkflowPermission("read")
+
+    id1 = Identity(id=1)
+    id1.provides.add(UserNeed(1))
+
+    with pytest.raises(MissingWorkflowError):
+        assert wp.query_filter(identity=id1) == {}

tests/test_workflow_requests.py

@@ -8,14 +8,15 @@
 from types import SimpleNamespace
 
 from invenio_records_permissions.generators import Generator
-from oarepo_runtime.services.generators import RecordOwners
+from oarepo_runtime.services.permissions import RecordOwners, UserWithRole
 
-from oarepo_workflows import WorkflowRequestPolicy
+from oarepo_workflows import WorkflowRequestPolicy, WorkflowTransitions
 from oarepo_workflows.requests import RecipientGeneratorMixin, WorkflowRequest
 from thesis.thesis.records.api import ThesisRecord
 
 from flask_principal import Identity, UserNeed, RoleNeed
-from oarepo_runtime.services.permissions.generators import UserWithRole
+import pytest
+from opensearch_dsl.query import Terms
 
 
 class TestRecipient(RecipientGeneratorMixin, Generator):
@@ -28,6 +29,40 @@ class NullRecipient(RecipientGeneratorMixin, Generator):
     def reference_receivers(self, record=None, request_type=None, **kwargs):
         return None
 
+class FailingGenerator(Generator):
+    def needs(self, **context):
+        raise ValueError("Failing generator")
+
+    def excludes(self, **context):
+        raise ValueError("Failing generator")
+
+    def query_filter(self, **context):
+        raise ValueError("Failing generator")
+
+class R(WorkflowRequestPolicy):
+    req = WorkflowRequest(
+        requesters=[RecordOwners()],
+        recipients=[NullRecipient(), TestRecipient()],
+        transitions=WorkflowTransitions(
+            submitted="pending",
+            accepted="accepted",
+            declined="declined",
+        ),
+    )
+    req1 = WorkflowRequest(
+        requesters=[
+            UserWithRole("administrator"),
+        ],
+        recipients=[NullRecipient(), TestRecipient()],
+    )
+    req2 = WorkflowRequest(
+        requesters = [],    # never applicable, must be created by, for example, system identity
+        recipients = []
+    )
+    req3 = WorkflowRequest(
+        requesters = [FailingGenerator()],
+        recipients = [NullRecipient(), TestRecipient()],
+    )
 
 def test_workflow_requests(users, logged_client, search_clear, record_service):
     req = WorkflowRequest(
@@ -37,6 +72,19 @@ def test_workflow_requests(users, logged_client, search_clear, record_service):
     rec = ThesisRecord.create({})
     assert req.recipient_entity_reference(record=rec) == {"user": "1"}
 
+def test_workflow_requests_no_recipient(users, logged_client, search_clear, record_service):
+    req1 = WorkflowRequest(
+        requesters=[RecordOwners()],
+        recipients=[NullRecipient()],
+    )
+    rec = ThesisRecord.create({})
+    assert req1.recipient_entity_reference(record=rec) is None
+
+    req2 = WorkflowRequest(
+        requesters=[RecordOwners()],
+        recipients=[],
+    )
+    assert req2.recipient_entity_reference(record=rec) is None
 
 def test_request_policy_access(app):
     request_policy = app.config["WORKFLOWS"]["my_workflow"].requests()
@@ -61,17 +109,7 @@ def test_is_applicable(users, logged_client, search_clear, record_service):
 
 
 def test_list_applicable_requests(users, logged_client, search_clear, record_service):
-    class R(WorkflowRequestPolicy):
-        req = WorkflowRequest(
-            requesters=[RecordOwners()],
-            recipients=[NullRecipient(), TestRecipient()],
-        )
-        req1 = WorkflowRequest(
-            requesters=[
-                UserWithRole("administrator"),
-            ],
-            recipients=[NullRecipient(), TestRecipient()],
-        )
+
 
     requests = R()
 
@@ -91,3 +129,30 @@ class R(WorkflowRequestPolicy):
     assert set(
         x[0] for x in requests.applicable_workflow_requests(id2, record=record)
     ) == {"req1"}
+
+
+def test_get_request_type(users, logged_client, search_clear, record_service):
+    requests = R()
+    assert requests["req"] == requests.req
+    assert requests["req1"] == requests.req1
+    with pytest.raises(KeyError):
+        requests["non_existing_request"]        # noqa
+
+def test_transition_getter(users, logged_client, search_clear, record_service):
+    requests = R()
+    assert requests.req.transitions['submitted'] == 'pending'
+    assert requests.req.transitions['accepted'] == 'accepted'
+    assert requests.req.transitions['declined'] == 'declined'
+    with pytest.raises(KeyError):
+        requests.req.transitions['non_existing_transition']        # noqa
+
+
+def test_requestor_filter(users, logged_client, search_clear, record_service):
+    requests = R()
+    sample_record = SimpleNamespace(parent=SimpleNamespace(owners=[SimpleNamespace(id=1)]))
+
+    id1 = Identity(id=1)
+    id1.provides.add(UserNeed(1))
+
+    generator = requests.req.requester_generator
+    assert generator.query_filter(identity=id1, record=sample_record) == [Terms(parent__owners__user=[1])]