Merge pull request #656 from opensafely-core/c2-instructions

Add instructions for context and controls
opensafely-core · Sep 6, 2024 · cd1bcae · cd1bcae
2 parents c722892 + b8e36f9
commit cd1bcae
Show file tree

Hide file tree

Showing 46 changed files with 56 additions and 5 deletions.
diff --git a/airlock/forms.py b/airlock/forms.py
@@ -1,5 +1,6 @@
 from django import forms
 from django.forms.formsets import BaseFormSet, formset_factory
+from django.utils.safestring import mark_safe
 
 from airlock.enums import RequestFileType, Visibility
 from airlock.models import FileGroup
@@ -124,8 +125,45 @@ def clean(self):
 
 
 class GroupEditForm(forms.Form):
-    context = forms.CharField(required=False)
-    controls = forms.CharField(required=False)
+    context = forms.CharField(
+        required=False,
+        help_text=mark_safe(
+            """
+            <div class="c2-help">
+            Please describe the contents of these outputs to help reviewers. This includes:
+            <ul style="margin-top: 0;">
+                <li>Variable descriptions</li>
+                <li>A description and count of the underlying population for each output if it is not included in the output.</li>
+                <li>Specify population size and degrees of freedom for all regression outputs.</li>
+                <li>Relationship to other data/tables which through combination may introduce secondary disclosive risks.</li>
+            </ul>
+            </div>
+            """
+        ),
+    )
+    controls = forms.CharField(
+        required=False,
+        help_text=mark_safe(
+            """
+            <div class="c2-help">
+            Please describe the statistical disclosure controls you have applied to these outputs. This includes:
+            <ul>
+                <li>The threshold used for low number suppression.</li>
+                <li>The rounding approach used.</li>
+            </ul>
+            </div>
+            """
+        ),
+    )
+
+    def __init__(self, *args, **kwargs):
+        inline = kwargs.pop("inline", False)
+        super().__init__(*args, **kwargs)
+        if inline:
+            # Hide the help text if the form is inlined (i.e. it's)
+            # being opened as readonly from a file view
+            for field in ["context", "controls"]:
+                self.fields[field].help_text = ""
 
     @classmethod
     def from_filegroup(cls, filegroup: FileGroup, *args, **kwargs):

diff --git a/airlock/templates/file_browser/request/filegroup.html b/airlock/templates/file_browser/request/filegroup.html
@@ -6,17 +6,24 @@
   .comments .comment_blinded {
     background-color: darkgrey;
   }
+
+  .c2-help {
+    line-height: 1.6;
+    font-size: small;
+  }
+
 </style>
 {% #card title=group.title container=True class="group_modal" %}
   <div data-testid="c3">
     <form action="{{ group.c2_edit_url }}" method="POST" aria-label="group-edit-form">
       {% csrf_token %}
       <div style="display: flex; flex-direction: row; width: 100%; flex-wrap: wrap; gap: 1rem;">
         <div style="display: flex: flex-direction: column; flex-basis: 100%; flex: 1;">
-          {% form_textarea field=group.c2_edit_form.context show_placeholder=True resize=True placeholder="Describe the data to be released in this group of files" class="w-full max-w-lg mx-auto" rows=6 disabled=group.c2_readonly readonly=group.c2_readonly %}
+
+          {% form_textarea field=group.c2_edit_form.context resize=True hint_below=True show_placeholder=True placeholder="Describe the data to be released in this group of files" class="w-full max-w-lg mx-auto" rows=6 disabled=group.c2_readonly readonly=group.c2_readonly %}
         </div>
         <div style="display: flex: flex-direction: column; flex-basis: 100%; flex: 1; gap: 1rem;">
-          {% form_textarea field=group.c2_edit_form.controls resize=True show_placeholder=True placeholder="Describe the disclosure controls that have been applied to these files" class="w-full max-w-lg mx-auto" rows=6 disabled=group.c2_readonly readonly=group.c2_readonly %}
+          {% form_textarea field=group.c2_edit_form.controls resize=True hint_below=True show_placeholder=True placeholder="Describe the disclosure controls that have been applied to these files" class="w-full max-w-lg mx-auto" rows=6 disabled=group.c2_readonly readonly=group.c2_readonly %}
         </div>
       </div>
       {% if not group.c2_readonly %}

diff --git a/airlock/views/request.py b/airlock/views/request.py
@@ -386,7 +386,7 @@ def group_presenter(release_request, relpath, request):
         "inline": inline,
         # context/controls editing
         "c2_readonly": c2_readonly,
-        "c2_edit_form": GroupEditForm.from_filegroup(filegroup),
+        "c2_edit_form": GroupEditForm.from_filegroup(filegroup, inline=inline),
         "c2_edit_url": reverse(
             "group_edit",
             kwargs={"request_id": release_request.id, "group": group},

diff --git a/docs/screenshots/add_file_button.png b/docs/screenshots/add_file_button.png
diff --git a/docs/screenshots/add_file_modal.png b/docs/screenshots/add_file_modal.png
diff --git a/docs/screenshots/changed_tree_file.png b/docs/screenshots/changed_tree_file.png
diff --git a/docs/screenshots/context_and_controls.png b/docs/screenshots/context_and_controls.png
diff --git a/docs/screenshots/context_modal.png b/docs/screenshots/context_modal.png
diff --git a/docs/screenshots/file_approved.png b/docs/screenshots/file_approved.png
diff --git a/docs/screenshots/file_group.png b/docs/screenshots/file_group.png
diff --git a/docs/screenshots/file_review.png b/docs/screenshots/file_review.png
diff --git a/docs/screenshots/file_update.png b/docs/screenshots/file_update.png
diff --git a/docs/screenshots/file_update_modal copy.png b/docs/screenshots/file_update_modal copy.png
diff --git a/docs/screenshots/file_update_modal.png b/docs/screenshots/file_update_modal.png
diff --git a/docs/screenshots/files_released.png b/docs/screenshots/files_released.png
diff --git a/docs/screenshots/more_dropdown.png b/docs/screenshots/more_dropdown.png
diff --git a/docs/screenshots/more_dropdown_el.png b/docs/screenshots/more_dropdown_el.png
diff --git a/docs/screenshots/more_dropdown_el_request_file.png b/docs/screenshots/more_dropdown_el_request_file.png
diff --git a/docs/screenshots/multiselect_update.png b/docs/screenshots/multiselect_update.png
diff --git a/docs/screenshots/ready_to_release.png b/docs/screenshots/ready_to_release.png
diff --git a/docs/screenshots/request_independent_review_file_icons.png b/docs/screenshots/request_independent_review_file_icons.png
diff --git a/docs/screenshots/request_independent_review_researcher_file_icons.png b/docs/screenshots/request_independent_review_researcher_file_icons.png
diff --git a/docs/screenshots/request_overview.png b/docs/screenshots/request_overview.png
diff --git a/docs/screenshots/request_reviewed_file_icons.png b/docs/screenshots/request_reviewed_file_icons.png
diff --git a/docs/screenshots/request_tree.png b/docs/screenshots/request_tree.png
diff --git a/docs/screenshots/request_tree_post_voting.png b/docs/screenshots/request_tree_post_voting.png
diff --git a/docs/screenshots/requests_index.png b/docs/screenshots/requests_index.png
diff --git a/docs/screenshots/returned_request_comments.png b/docs/screenshots/returned_request_comments.png
diff --git a/docs/screenshots/returned_tree.png b/docs/screenshots/returned_tree.png
diff --git a/docs/screenshots/reviewed_request_comment_in_progress.png b/docs/screenshots/reviewed_request_comment_in_progress.png
diff --git a/docs/screenshots/reviewed_request_comments.png b/docs/screenshots/reviewed_request_comments.png
diff --git a/docs/screenshots/submit_request.png b/docs/screenshots/submit_request.png
diff --git a/docs/screenshots/submit_review.png b/docs/screenshots/submit_review.png
diff --git a/docs/screenshots/submitted_request.png b/docs/screenshots/submitted_request.png
diff --git a/docs/screenshots/submitted_review.png b/docs/screenshots/submitted_review.png
diff --git a/docs/screenshots/withdraw_request.png b/docs/screenshots/withdraw_request.png
diff --git a/docs/screenshots/withdraw_request_modal.png b/docs/screenshots/withdraw_request_modal.png
diff --git a/docs/screenshots/withdrawn_file.png b/docs/screenshots/withdrawn_file.png
diff --git a/docs/screenshots/workspace_directory_content.png b/docs/screenshots/workspace_directory_content.png
diff --git a/docs/screenshots/workspace_directory_view.png b/docs/screenshots/workspace_directory_view.png
diff --git a/docs/screenshots/workspace_file_icons.png b/docs/screenshots/workspace_file_icons.png
diff --git a/docs/screenshots/workspace_file_view.png b/docs/screenshots/workspace_file_view.png
diff --git a/docs/screenshots/workspace_view.png b/docs/screenshots/workspace_view.png
diff --git a/docs/screenshots/workspaces_index.png b/docs/screenshots/workspaces_index.png
diff --git a/tests/functional/test_e2e.py b/tests/functional/test_e2e.py
@@ -282,6 +282,9 @@ def test_e2e_release_files(
     filegroup_link = page.get_by_role("link").locator(".filegroup:scope")
     find_and_click(filegroup_link)
 
+    # context and controls instruction help text is shown
+    expect(page.get_by_test_id("c3")).to_contain_text("Please describe")
+
     context_input = page.locator("#id_context")
     find_and_click(context_input)
     context_input.fill("some context")

diff --git a/tests/functional/test_request_pages.py b/tests/functional/test_request_pages.py
@@ -81,6 +81,9 @@ def test_request_file_group_context_modal(live_server, context, page):
     expect(dialog).to_contain_text("This is some testing context")
     expect(dialog).to_contain_text("I got rid of all the small numbers")
 
+    # context and controls instruction help text is not shown in the modal
+    expect(page.get_by_test_id("c3")).not_to_contain_text("Please describe")
+
     dialog.locator("button[type=cancel]").click()
     expect(dialog).not_to_be_visible()