Create separate views for researchers and output checkers

airlock/nav.py

@@ -5,6 +5,8 @@
 from django.http import HttpRequest
 from django.urls import reverse
 
+from airlock import permissions
+
 
 def default_predicate(request):
     return True
@@ -33,9 +35,13 @@ def iter_nav(items, request):
 def menu(request):
     items = [
         NavItem(name="Workspaces", url_name="workspace_index"),
-        NavItem(name="Requests", url_name="request_index"),
+        NavItem(name="Requests", url_name="requests_for_researcher"),
         NavItem(name="Docs", url_name="docs_home"),
     ]
+
+    if request.user and permissions.user_can_review(request.user):
+        reviews_menu = NavItem(name="Reviews", url_name="requests_for_output_checker")
+        items.insert(2, reviews_menu)
     return {"nav": list(iter_nav(items, request))}
 
 

airlock/templates/requests.html → airlock/templates/requests_for_output_checker.html

@@ -2,23 +2,12 @@
 
 {% load airlock %}
 
-{% block metatitle %}Requests |  Airlock{% endblock metatitle %}
+{% block metatitle %}Reviews |  Airlock{% endblock metatitle %}
 
 {% block content %}
   <div class="flex flex-col gap-4 max-w-3xl">
     {% airlock_header title="Requests" %}
 
-    {% #card title="Requests by "|add:request.user.username %}
-      {% #list_group id="authored-requests" %}
-        {% for release_request in authored_requests %}
-          {% #list_group_rich_item url=release_request.get_url status_text=release_request.status.description title=release_request.workspace %}
-          {% /list_group_rich_item %}
-        {% empty %}
-          {% list_group_empty title="No requests" description="You do not have any authored requests" %}
-        {% endfor %}
-      {% /list_group %}
-    {% /card %}
-
     {% if request.user.output_checker %}
       {% #card title="Outstanding requests awaiting review" %}
         {% #list_group id="outstanding-requests" %}

airlock/templates/requests_for_researcher.html

@@ -0,0 +1,23 @@
+{% extends "base.html" %}
+
+{% load airlock %}
+
+{% block metatitle %}Requests |  Airlock{% endblock metatitle %}
+
+{% block content %}
+  <div class="flex flex-col gap-4 max-w-3xl">
+    {% airlock_header title="Requests" %}
+
+    {% #card title="Requests by "|add:request.user.username %}
+      {% #list_group id="authored-requests" %}
+        {% for release_request in authored_requests %}
+          {% #list_group_rich_item url=release_request.get_url status_text=release_request.status.description title=release_request.workspace %}
+          {% /list_group_rich_item %}
+        {% empty %}
+          {% list_group_empty title="No requests" description="You do not have any authored requests" %}
+        {% endfor %}
+      {% /list_group %}
+    {% /card %}
+
+  </div>
+{% endblock content %}

airlock/urls.py

@@ -69,8 +69,18 @@
     # requests
     path(
         "requests/",
-        airlock.views.request_index,
-        name="request_index",
+        RedirectView.as_view(pattern_name="requests_for_researcher"),
+        name="requests",
+    ),
+    path(
+        "requests/researcher",
+        airlock.views.requests_for_researcher,
+        name="requests_for_researcher",
+    ),
+    path(
+        "requests/output_checker",
+        airlock.views.requests_for_output_checker,
+        name="requests_for_output_checker",
     ),
     path(
         "requests/view/<str:request_id>/",

airlock/views/__init__.py

@@ -10,7 +10,6 @@
     group_comment_visibility_public,
     group_edit,
     request_contents,
-    request_index,
     request_multiselect,
     request_reject,
     request_release_files,
@@ -19,6 +18,8 @@
     request_submit,
     request_view,
     request_withdraw,
+    requests_for_output_checker,
+    requests_for_researcher,
     requests_for_workspace,
 )
 from .workspace import (
@@ -39,7 +40,8 @@
     "file_request_changes",
     "file_withdraw",
     "request_contents",
-    "request_index",
+    "requests_for_output_checker",
+    "requests_for_researcher",
     "request_multiselect",
     "request_reject",
     "file_reset_review",

airlock/views/request.py

@@ -45,9 +45,7 @@
 
 
 @instrument
-def request_index(request):
-    authored_requests = bll.get_requests_authored_by_user(request.user)
-
+def requests_for_output_checker(request):
     outstanding_requests = []
     returned_requests = []
     approved_requests = []
@@ -67,19 +65,33 @@ def get_reviewer_progress(release_request):
         ]
         returned_requests = bll.get_returned_requests(request.user)
         approved_requests = bll.get_approved_requests(request.user)
+    else:
+        return redirect("requests_for_researcher")
 
     return TemplateResponse(
         request,
-        "requests.html",
+        "requests_for_output_checker.html",
         {
-            "authored_requests": authored_requests,
             "outstanding_requests": outstanding_requests,
             "returned_requests": returned_requests,
             "approved_requests": approved_requests,
         },
     )
 
 
+@instrument
+def requests_for_researcher(request):
+    authored_requests = bll.get_requests_authored_by_user(request.user)
+
+    return TemplateResponse(
+        request,
+        "requests_for_researcher.html",
+        {
+            "authored_requests": authored_requests,
+        },
+    )
+
+
 def _get_request_button_context(user, release_request):
     # default context for request-level actions with everything hidden
     # author actions

tests/functional/test_e2e.py

@@ -309,7 +309,7 @@ def test_e2e_release_files(
     # Before we log the researcher out and continue, let's just check
     # their requests
     find_and_click(page.get_by_test_id("nav-requests"))
-    expect(page).to_have_url(live_server.url + "/requests/")
+    expect(page).to_have_url(live_server.url + "/requests/researcher")
 
     authored_request_locator = page.locator("#authored-requests")
     expect(authored_request_locator).to_contain_text("SUBMITTED")
@@ -330,7 +330,7 @@ def test_e2e_release_files(
     login_as(live_server, page, "output_checker")
 
     # View requests
-    find_and_click(page.get_by_test_id("nav-requests"))
+    find_and_click(page.get_by_test_id("nav-reviews"))
 
     # View submitted request (in the output-checker's outstanding requests for review)
     find_and_click(page.locator("#outstanding-requests").get_by_role("link"))

tests/integration/views/test_request.py

@@ -659,18 +659,14 @@ def test_request_download_file_permissions(
 def test_request_index_user_permitted_requests(airlock_client):
     airlock_client.login(workspaces=["test1"])
     release_request = factories.create_release_request("test1", airlock_client.user)
-    response = airlock_client.get("/requests/")
+    response = airlock_client.get("/requests/researcher")
     authored_ids = {r.id for r in response.context["authored_requests"]}
-    outstanding_ids = {r[0].id for r in response.context["outstanding_requests"]}
-    returned_ids = {r.id for r in response.context["returned_requests"]}
-    approved_ids = {r.id for r in response.context["approved_requests"]}
+
     assert authored_ids == {release_request.id}
-    assert outstanding_ids == set()
-    assert returned_ids == set()
-    assert approved_ids == set()
 
 
-def test_request_index_user_output_checker(airlock_client):
+# reviews page
+def test_review_user_output_checker(airlock_client):
     airlock_client.login(workspaces=["test_workspace"], output_checker=True)
     other = factories.create_user(
         "other",
@@ -682,40 +678,65 @@ def test_request_index_user_output_checker(airlock_client):
         ],
     )
     r1 = factories.create_request_at_status(
-        "test_workspace",
-        author=airlock_client.user,
-        status=RequestStatus.SUBMITTED,
-        files=[factories.request_file()],
-    )
-    r2 = factories.create_request_at_status(
         "other_workspace",
         author=other,
         status=RequestStatus.SUBMITTED,
         files=[factories.request_file()],
     )
-    r3 = factories.create_request_at_status(
+    r2 = factories.create_request_at_status(
         "other_other_workspace",
         author=other,
         status=RequestStatus.RETURNED,
         files=[factories.request_file(changes_requested=True)],
     )
-    r4 = factories.create_request_at_status(
+    r3 = factories.create_request_at_status(
         "other_other1_workspace",
         author=other,
         status=RequestStatus.APPROVED,
         files=[factories.request_file(approved=True)],
     )
-    response = airlock_client.get("/requests/")
+    response = airlock_client.get("/requests/output_checker")
 
-    authored_ids = {r.id for r in response.context["authored_requests"]}
     outstanding_ids = {r[0].id for r in response.context["outstanding_requests"]}
     returned_ids = {r.id for r in response.context["returned_requests"]}
     approved_ids = {r.id for r in response.context["approved_requests"]}
 
+    assert outstanding_ids == {r1.id}
+    assert returned_ids == {r2.id}
+    assert approved_ids == {r3.id}
+
+
+# To confirm that the request page displays for an output checker
+def test_request_index_user_output_checker(airlock_client):
+    airlock_client.login(workspaces=["test_workspace"], output_checker=True)
+    r1 = factories.create_request_at_status(
+        "test_workspace",
+        author=airlock_client.user,
+        status=RequestStatus.SUBMITTED,
+        files=[factories.request_file()],
+    )
+
+    response = airlock_client.get("/requests/researcher")
+
+    authored_ids = {r.id for r in response.context["authored_requests"]}
+
     assert authored_ids == {r1.id}
-    assert outstanding_ids == {r2.id}
-    assert returned_ids == {r3.id}
-    assert approved_ids == {r4.id}
+
+
+# Check that the reviews page redirects to requests for researchers
+def test_request_index_redirect_user_researcher(airlock_client):
+    airlock_client.login(workspaces=["test_workspace"], output_checker=False)
+
+    response = airlock_client.get("/requests/output_checker")
+    redirected_url = "/requests/researcher"
+    assert response.url == redirected_url
+
+
+def test_no_outstanding_request_output_checker(airlock_client):
+    airlock_client.login(workspaces=["test_workspace"], output_checker=True)
+    response = airlock_client.get("/requests/output_checker")
+    outstanding_ids = {r[0].id for r in response.context["outstanding_requests"]}
+    assert len(outstanding_ids) == 0
 
 
 def test_request_index_user_request_progress(airlock_client):
@@ -809,7 +830,7 @@ def generate_files(reviewed=False, checkers_a=None, checkers_b=None):
         ),
     )
 
-    response = airlock_client.get("/requests/")
+    response = airlock_client.get("/requests/output_checker")
     assert response.context["outstanding_requests"] == [
         (r0, "Your review: 0/2 files (incomplete)"),
         (r1, "Your review: 2/2 files (incomplete)"),

tests/unit/test_nav.py

@@ -8,7 +8,7 @@ def predicate_false(request):
 def test_iter_nav(rf):
     items = [
         nav.NavItem("Workspace", "workspace_index"),
-        nav.NavItem("Requests", "request_index", predicate_false),
+        nav.NavItem("Requests", "requests_for_researcher", predicate_false),
     ]
 
     request = rf.get("/workspaces/")