Split API for authored requests vs requests to review

Previously, the same function retrieved both sets.  Now, there are
separate API functions, and the Eequests page lists them separatedly.

Specifically, as now an author who is also and output checker cannot
review their own request, then those requests are exclude from the "to
review" list.

Author: bloodearnest

airlock/api.py

@@ -224,8 +224,12 @@ def get_current_request(
         """
         raise NotImplementedError()
 
-    def get_requests_for_user(self, user: User) -> list[ReleaseRequest]:
-        """Get all current requests authored by user"""
+    def get_requests_authored_by_user(self, user: User) -> list[ReleaseRequest]:
+        """Get all current requests authored by user."""
+        raise NotImplementedError()
+
+    def get_outstanding_requests_for_review(self, user: User):
+        """Get all request that need review."""
         raise NotImplementedError()
 
     VALID_STATE_TRANSITIONS = {

airlock/templates/requests.html

@@ -1,11 +1,23 @@
 {% extends "base.html" %}
 
 {% block content %}
-{% #card title="Requests for "|add:request.user.username %}
+
+{% #card title="Requests by "|add:request.user.username %}
   {% #list_group %}
-    {% for request in requests %} 
-    {% #list_group_item href=request.get_absolute_url %}{{ request.id }}{% /list_group_item %}
+    {% for request in authored_requests %} 
+    {% #list_group_item href=request.get_absolute_url %}{{ request.workspace }}: {{ request.status.name }}{% /list_group_item %}
     {% endfor %}
   {% /list_group %}
 {% /card %}
+
+{% if request.user.output_checker %}
+{% #card title="Outstanding requests awaiting review" %}
+  {% #list_group %}
+    {% for request in outstanding_requests %} 
+    {% #list_group_item href=request.get_absolute_url %}{{ request.workspace }} by {{ request.author }}{% /list_group_item %}
+    {% endfor %}
+  {% /list_group %}
+{% /card %}
+{% endif %}
+
 {% endblock content %}

airlock/views.py

@@ -161,8 +161,20 @@ def workspace_add_file_to_request(request, workspace_name):
 
 
 def request_index(request):
-    requests = api.get_requests_for_user(request.user)
-    return TemplateResponse(request, "requests.html", {"requests": requests})
+    authored_requests = api.get_requests_authored_by_user(request.user)
+
+    outstanding_requests = []
+    if request.user.output_checker:
+        outstanding_requests = api.get_outstanding_requests_for_review(request.user)
+
+    return TemplateResponse(
+        request,
+        "requests.html",
+        {
+            "authored_requests": authored_requests,
+            "outstanding_requests": outstanding_requests,
+        },
+    )
 
 
 def request_view(request, request_id: str, path: str = ""):

local_db/api.py

@@ -60,17 +60,30 @@ def get_current_request(self, workspace: str, user: User, create=False):
                 author=user.username,
             )
 
-    def get_requests_for_user(self, user: User):
+    def get_requests_authored_by_user(self, user: User):
+        requests = []
+
+        for metadata in RequestMetadata.objects.filter(author=user.username).order_by(
+            "status"
+        ):
+            # to create a request, user *must* have explicit workspace
+            # permissions - being an output checker is not enough
+            if metadata.workspace in user.workspaces:
+                requests.append(self._request(metadata))
+
+        return requests
+
+    def get_outstanding_requests_for_review(self, user: User):
         requests = []
-        filter_args = {}
 
-        # if not output checker, can only see your own requests
         if not user.output_checker:
-            filter_args = {"author": user.username}
+            return []
 
-        for metadata in RequestMetadata.objects.filter(**filter_args):
-            if user.output_checker or metadata.workspace in user.workspaces:
+        for metadata in RequestMetadata.objects.filter(status=Status.SUBMITTED):
+            # do not show output_checker their own requests
+            if metadata.author != user.username:
                 requests.append(self._request(metadata))
+
         return requests
 
     def set_status(self, request: ReleaseRequest, status: Status, user: User):

tests/integration/test_views.py

@@ -248,17 +248,30 @@ def test_request_index_user_permitted_requests(client_with_user):
     user = User.from_session(permitted_client.session)
     release_request = factories.create_release_request("test1", user)
     response = permitted_client.get("/requests/")
-    request_ids = {r.id for r in response.context["requests"]}
-    assert request_ids == {release_request.id}
+    authored_ids = {r.id for r in response.context["authored_requests"]}
+    outstanding_ids = {r.id for r in response.context["outstanding_requests"]}
+    assert authored_ids == {release_request.id}
+    assert outstanding_ids == set()
 
 
 def test_request_index_user_output_checker(client_with_user):
-    r1 = factories.create_release_request("test1")
-    r2 = factories.create_release_request("test2")
-    permitted_client = client_with_user({"workspaces": [], "output_checker": True})
+    permitted_client = client_with_user(
+        {"workspaces": ["test_workspace"], "output_checker": True}
+    )
+    user = User.from_session(permitted_client.session)
+    other = User(1, "other")
+    r1 = factories.create_release_request(
+        "test_workspace", user=user, status=Status.SUBMITTED
+    )
+    r2 = factories.create_release_request(
+        "other_workspace", user=other, status=Status.SUBMITTED
+    )
     response = permitted_client.get("/requests/")
-    request_ids = {r.id for r in response.context["requests"]}
-    assert request_ids == {r1.id, r2.id}
+
+    authored_ids = {r.id for r in response.context["authored_requests"]}
+    outstanding_ids = {r.id for r in response.context["outstanding_requests"]}
+    assert authored_ids == {r1.id}
+    assert outstanding_ids == {r2.id}
 
 
 def test_request_submit_author(client_with_user):

tests/unit/test_api.py

@@ -101,29 +101,25 @@ def test_provider_request_release_files(mock_old_api):
     [
         ([], False, []),
         (["allowed"], False, ["r1"]),
-        (
-            [],
-            True,
-            [
-                "r1",
-                "r2",
-                "r3",
-            ],
-        ),
+        # output checkers can't create requests unless explit permission for workspace
+        ([], True, []),
+        (["allowed"], True, ["r1"]),
         (["allowed", "notexist"], False, ["r1"]),
-        (["notexist", "notexist"], False, []),
+        (["notexist"], False, []),
         (["no-request-dir", "notexist"], False, []),
     ],
 )
-def test_provider_get_requests_for_user(workspaces, output_checker, expected, api):
+def test_provider_get_requests_authored_by_user(
+    workspaces, output_checker, expected, api
+):
     user = User(1, "test", workspaces, output_checker)
     other_user = User(1, "other", [], False)
     factories.create_release_request("allowed", user, id="r1")
     factories.create_release_request("allowed", other_user, id="r2")
     factories.create_release_request("not-allowed", user, id="r3")
     factories.create_workspace("no-request-dir")
 
-    assert set(r.id for r in api.get_requests_for_user(user)) == set(expected)
+    assert set(r.id for r in api.get_requests_authored_by_user(user)) == set(expected)
 
 
 def test_provider_get_current_request_for_user(api):