Merge pull request #538 from opensafely-core/disable-context-controls

Disable editing of context and controls for author when the request is under review

airlock/business_logic.py

@@ -1074,6 +1074,16 @@ def is_final(self):
             BusinessLogicLayer.STATUS_OWNERS[self.status] == RequestStatusOwner.SYSTEM
         )
 
+    def is_under_review(self):
+        return (
+            BusinessLogicLayer.STATUS_OWNERS[self.status] == RequestStatusOwner.REVIEWER
+        )
+
+    def is_editing(self):
+        return (
+            BusinessLogicLayer.STATUS_OWNERS[self.status] == RequestStatusOwner.AUTHOR
+        )
+
 
 def store_file(release_request: ReleaseRequest, abspath: Path) -> str:
     # Make a "staging" copy of the file under a temporary name so we know it can't be

airlock/templates/file_browser/group.html

@@ -50,30 +50,32 @@
             {% endif %}
           {% /list_group_rich_item %}
         {% endfor %}
-        {% #list_group_item %}
-          <form action="{{ group_comment_create_url }}" method="POST" aria-label="group-comment-form">
-            {% csrf_token %}
-            {% if request.user.output_checker and release_request.get_turn_phase.name == "INDEPENDENT" %}
-              {% #alert variant="warning" title="Comments are hidden" dismissible=True %}
-                Only you will see this comment until two independent reviews have been completed
-              {% /alert %}
-            {% else %}
-              {% #alert variant="info" title="Comments are pending" no_icon=True %}
-                Any comments will be shown to other users once you submit or return a request
-              {% /alert %}
-            {% endif %}
+        {% if can_comment %}
+          {% #list_group_item %}
+            <form action="{{ group_comment_create_url }}" method="POST" aria-label="group-comment-form">
+              {% csrf_token %}
+              {% if request.user.output_checker and release_request.get_turn_phase.name == "INDEPENDENT" %}
+                {% #alert variant="warning" title="Comments are hidden" dismissible=True %}
+                  Only you will see this comment until two independent reviews have been completed
+                {% /alert %}
+              {% else %}
+                {% #alert variant="info" title="Comments are pending" no_icon=True %}
+                  Any comments will be shown to other users once you submit or return a request
+                {% /alert %}
+              {% endif %}
 
-            {% form_textarea field=group_comment_form.comment placeholder=" " label="Add Comment" show_placeholder=True class="w-full max-w-lg" rows=6 required=False %}
-            {% if group_comment_form.visibility.field.choices|length == 1 %}
-              <input type="hidden" name="visibility" value="{{ group_comment_form.visibility.field.choices.0.0 }}"/>
-            {% else %}
-              {% form_radios field=group_comment_form.visibility choices=group_comment_form.visibility.field.choices class="w-full max-w-lg" %}
-            {% endif%}
-            <div class="mt-2">
-              {% #button type="submit" variant="success" id="edit-comment-button" %}Comment{% /button %}
-            </div>
-          </form>
-        {% /list_group_item %}
+              {% form_textarea field=group_comment_form.comment placeholder=" " label="Add Comment" show_placeholder=True class="w-full max-w-lg" rows=6 required=False %}
+              {% if group_comment_form.visibility.field.choices|length == 1 %}
+                <input type="hidden" name="visibility" value="{{ group_comment_form.visibility.field.choices.0.0 }}"/>
+              {% else %}
+                {% form_radios field=group_comment_form.visibility choices=group_comment_form.visibility.field.choices class="w-full max-w-lg" %}
+              {% endif%}
+              <div class="mt-2">
+                {% #button type="submit" variant="success" id="edit-comment-button" %}Comment{% /button %}
+              </div>
+            </form>
+          {% /list_group_item %}
+        {% endif %}
       {% /list_group %}
     </div>
   {% endif %}

airlock/users.py

@@ -48,10 +48,13 @@ def has_permission(self, workspace_name):
             self.output_checker or workspace_name in self.workspaces
         )
 
+    def can_access_workspace(self, workspace_name):
+        return workspace_name in self.workspaces
+
     def verify_can_action_request(self, workspace_name):
         # Only users with explict access to the workspace can create/modify release
         # requests.
-        if workspace_name not in self.workspaces:
+        if not self.can_access_workspace(workspace_name):
             raise ActionDenied(f"you do not have permission for {workspace_name}")
         # Requests for archived workspaces cannot be created/modified
         if self.workspaces[workspace_name]["archived"]:

airlock/views/request.py

@@ -18,7 +18,6 @@
     RequestFileType,
     RequestFileVote,
     RequestStatus,
-    RequestStatusOwner,
     bll,
 )
 from airlock.file_browser_api import get_request_tree
@@ -90,10 +89,7 @@ def request_view(request, request_id: str, path: str = ""):
     file_withdraw_url = None
     code_url = None
 
-    if (
-        release_request.status_owner() == RequestStatusOwner.AUTHOR
-        and not is_directory_url
-    ):
+    if release_request.is_editing() and not is_directory_url:
         # A file can only be withdrawn from a request that is currently
         # editable by the author
         file_withdraw_url = reverse(
@@ -121,7 +117,31 @@ def request_view(request, request_id: str, path: str = ""):
     group_comment_form = None
     group_comment_create_url = None
     group_comment_delete_url = None
-    group_readonly = release_request.is_final() or not is_author
+
+    can_edit_group = (
+        # no-one can edit context/controls for final requests
+        not release_request.is_final()
+        # only authors can edit context/controls
+        and is_author
+        # and only if the request is in draft i.e. in an author-owned turn
+        and release_request.is_editing()
+    )
+
+    can_comment = (
+        # no-one can comment on final requests
+        not release_request.is_final()
+        # user who can review can comment if the request is under review
+        and (
+            release_request.user_can_review(request.user)
+            and release_request.is_under_review()
+        )
+        or
+        # any user with access to the workspace can comment if the request is in draft
+        (
+            request.user.can_access_workspace(release_request.workspace)
+            and release_request.is_editing()
+        )
+    )
 
     activity = []
     group_activity = []
@@ -277,7 +297,8 @@ def request_view(request, request_id: str, path: str = ""):
         "group_comments": comments,
         "group_comment_form": group_comment_form,
         "group_comment_create_url": group_comment_create_url,
-        "group_readonly": group_readonly,
+        "group_readonly": not can_edit_group,
+        "can_comment": can_comment,
         "group_activity": group_activity,
         "show_c3": settings.SHOW_C3,
         # TODO, but for now stops template variable errors

airlock/views/workspace.py

@@ -11,7 +11,6 @@
 
 from airlock.business_logic import (
     RequestFileType,
-    RequestStatusOwner,
     bll,
 )
 from airlock.file_browser_api import get_workspace_tree
@@ -85,8 +84,7 @@ def workspace_view(request, workspace_name: str, path: str = ""):
         can_action_request = False
 
     multiselect_add = can_action_request and (
-        workspace.current_request is None
-        or workspace.current_request.status_owner() == RequestStatusOwner.AUTHOR
+        workspace.current_request is None or workspace.current_request.is_editing()
     )
 
     valid_states_to_add = [

tests/functional/test_request_pages.py

@@ -50,7 +50,9 @@ def test_request_file_withdraw(live_server, context, page, bll):
     expect(page.locator("#withdraw-file-button")).not_to_be_visible()
 
 
-def test_request_group_edit_comment(live_server, context, page, bll, settings):
+def test_request_group_edit_comment_for_author(
+    live_server, context, page, bll, settings
+):
     settings.SHOW_C3 = False  # context and controls visible, comments hidden
     author = login_as_user(
         live_server,
@@ -61,20 +63,30 @@ def test_request_group_edit_comment(live_server, context, page, bll, settings):
                 "workspace": {
                     "project_details": {"name": "Project 2", "ongoing": True},
                     "archived": False,
-                }
+                },
+                "pending": {
+                    "project_details": {"name": "Project 2", "ongoing": True},
+                    "archived": False,
+                },
             },
             "output_checker": False,
         },
     )
 
-    release_request = factories.create_request_at_status(
+    submitted_release_request = factories.create_request_at_status(
         "workspace",
         author=author,
         files=[factories.request_file(group="group")],
         status=RequestStatus.SUBMITTED,
     )
+    pending_release_request = factories.create_request_at_status(
+        "pending",
+        author=author,
+        files=[factories.request_file(group="group")],
+        status=RequestStatus.PENDING,
+    )
 
-    page.goto(live_server.url + release_request.get_url("group"))
+    page.goto(live_server.url + pending_release_request.get_url("group"))
     contents = page.locator("#selected-contents")
 
     group_edit_locator = contents.get_by_role("form", name="group-edit-form")
@@ -84,7 +96,8 @@ def test_request_group_edit_comment(live_server, context, page, bll, settings):
     context_locator.fill("test context")
     controls_locator.fill("test controls")
 
-    group_edit_locator.get_by_role("button", name="Save").click()
+    group_save_button = group_edit_locator.get_by_role("button", name="Save")
+    group_save_button.click()
 
     expect(context_locator).to_have_value("test context")
     expect(controls_locator).to_have_value("test controls")
@@ -99,11 +112,71 @@ def test_request_group_edit_comment(live_server, context, page, bll, settings):
     comment_locator = group_comment_locator.get_by_role("textbox", name="comment")
 
     comment_locator.fill("test comment")
-    group_comment_locator.get_by_role("button", name="Comment").click()
+    comment_button = group_comment_locator.get_by_role("button", name="Comment")
+    comment_button.click()
 
     comments_locator = contents.locator(".comments")
     expect(comments_locator).to_contain_text("test comment")
 
+    # cannot edit context/controls for submitted request or add comment
+    page.goto(live_server.url + submitted_release_request.get_url("group"))
+    expect(context_locator).not_to_be_editable()
+    expect(controls_locator).not_to_be_editable()
+    expect(group_save_button).not_to_be_visible()
+    expect(comment_button).not_to_be_visible()
+
+
+def test_request_group_edit_comment_for_checker(
+    live_server, context, page, bll, settings
+):
+    settings.SHOW_C3 = True
+    login_as_user(
+        live_server,
+        context,
+        user_dict={
+            "username": "checker",
+            "workspaces": {},
+            "output_checker": True,
+        },
+    )
+
+    submitted_release_request = factories.create_request_at_status(
+        "workspace",
+        files=[factories.request_file(group="group")],
+        status=RequestStatus.SUBMITTED,
+    )
+    pending_release_request = factories.create_request_at_status(
+        "pending",
+        files=[factories.request_file(group="group")],
+        status=RequestStatus.PENDING,
+    )
+
+    page.goto(live_server.url + submitted_release_request.get_url("group"))
+    contents = page.locator("#selected-contents")
+
+    group_edit_locator = contents.get_by_role("form", name="group-edit-form")
+    context_locator = group_edit_locator.get_by_role("textbox", name="context")
+    controls_locator = group_edit_locator.get_by_role("textbox", name="controls")
+    group_save_button = group_edit_locator.get_by_role("button", name="Save")
+
+    group_comment_locator = contents.get_by_role("form", name="group-comment-form")
+    comment_button = group_comment_locator.get_by_role("button", name="Comment")
+
+    # only author can edit context/controls
+    expect(context_locator).not_to_be_editable()
+    expect(controls_locator).not_to_be_editable()
+    expect(group_save_button).not_to_be_visible()
+    # in submitted status, output-checker can comment
+    expect(comment_button).to_be_visible()
+
+    # cannot edit context/controls for submitted request or add comment
+    page.goto(live_server.url + pending_release_request.get_url("group"))
+    expect(context_locator).not_to_be_editable()
+    expect(controls_locator).not_to_be_editable()
+    expect(group_save_button).not_to_be_visible()
+    # in pending status, output-checker cannot comment
+    expect(comment_button).not_to_be_visible()
+
 
 def _workspace_dict():
     return {