Feature/minor pre release updates (#156)

[Update logic to make cyber assessor
readonly](81c87ed)

 - Previous solution prevented the assessor from viewing the
   outcome details as the next step was disabled in the
   indicator stage. Implemented a new route by enabling the
   assessor to submit, but not save and just pass to the
   
[Rename download xls to download
data](1ec20cd)

- This is to avoid the confusion of the users assuming that this is the
complete assessment data in excel format.
 
 Peer review excel report corrections

Author: dhvander

tests/test_utils/test_to_spreadsheet_peer_review.py

@@ -206,15 +206,15 @@ def test_recommendations_tab_has_risk_columns_for_independent_review(self):
         self.assertIn("Recommendation number", headers)
 
     def test_recommendations_tab_no_risk_columns_for_peer_review(self):
-        """Test that recommendations tab does NOT include 'Risk' and 'Recommendation number' for peer review."""
+        """Test that recommendations tab does NOT include 'Risk' and 'Risk number' for peer review."""
         excel_bytes = review_to_excel(self.peer_review)
         wb = load_workbook(BytesIO(excel_bytes))
         ws = wb["Recommendations"]
 
         # Get header row
         headers = [cell.value for cell in ws[1]]
         self.assertNotIn("Risk", headers)
-        self.assertNotIn("Recommendation number", headers)
+        self.assertNotIn("Risk number", headers)
 
     def test_recommendations_tab_column_count_differs_by_review_type(self):
         """Test that recommendations tab has different column counts for different review types."""
@@ -251,7 +251,7 @@ def test_all_sheets_present_for_both_review_types(self):
 
     def test_recommendations_tab_common_columns_present(self):
         """Test that common columns are present in recommendations tab for both review types."""
-        common_columns = ["Contributing outcome", "Target CAF profile", "Risk number", "Recommendation"]
+        common_columns = ["Contributing outcome", "Target CAF profile", "Recommendation number", "Recommendation"]
 
         # Test independent review
         excel_bytes = review_to_excel(self.independent_review)
@@ -346,14 +346,14 @@ def test_peer_review_does_not_have_review_comments_column(self):
         self.assertNotIn("Review comments", headers)
 
     def test_peer_review_does_not_have_risk_columns(self):
-        """Test that peer_review does NOT have 'Risk' and 'Recommendation number' columns."""
+        """Test that peer_review does NOT have 'Risk' and 'Risk number' columns."""
         review = self.reviews["peer_review"]
         excel_bytes = review_to_excel(review)
         wb = load_workbook(BytesIO(excel_bytes))
         ws = wb["Recommendations"]
         headers = [cell.value for cell in ws[1]]
         self.assertNotIn("Risk", headers)
-        self.assertNotIn("Recommendation number", headers)
+        self.assertNotIn("Risk number", headers)
 
 
 class TestReviewToExcelDataIntegrity(TestCase):

tests/test_views/test_outcome_indicators_view.py

@@ -93,6 +93,41 @@ def test_post_outcome_indicators_status(self, form_data, expected_outcome):
         self.assessment.refresh_from_db()
         self.assertEqual(form_data, self.assessment.assessments_data["A1.a"]["indicators"])
 
+    def test_cyber_advisor_does_not_save_indicator_data(self):
+        cyber_advisor_profile = UserProfile.objects.get(role="cyber_advisor", organisation__name="Big organisation")
+        self.client.force_login(cyber_advisor_profile.user)
+        session = self.client.session
+        session["current_profile_id"] = cyber_advisor_profile.id
+        session["draft_assessment"] = {"assessment_id": self.assessment.id}
+        session.save()
+
+        original_data = {}
+        self.assessment.assessments_data = original_data
+        self.assessment.last_updated_by = None
+        self.assessment.save()
+
+        form_data = {
+            "achieved_A1.a.5": True,
+            "achieved_A1.a.6": True,
+            "achieved_A1.a.7": True,
+            "achieved_A1.a.8": True,
+            "not-achieved_A1.a.1": False,
+            "not-achieved_A1.a.2": False,
+            "not-achieved_A1.a.3": False,
+            "not-achieved_A1.a.4": False,
+            "achieved_A1.a.5_comment": "Achieved comment one",
+            "achieved_A1.a.6_comment": "",
+            "achieved_A1.a.7_comment": "",
+            "achieved_A1.a.8_comment": "",
+        }
+        response = self.client.post(self.url, data=form_data, follow=False)
+
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response["Location"], self.confirmation_url)
+        self.assessment.refresh_from_db()
+        self.assertEqual(self.assessment.assessments_data, original_data)
+        self.assertIsNone(self.assessment.last_updated_by)
+
     def test_post_confirmation_with_no_summary(self):
         """
         Summary:

webcaf/webcaf/caf/views/factory.py

@@ -18,6 +18,7 @@
 from webcaf.webcaf.forms.general import ContinueForm, NextActionForm
 from webcaf.webcaf.utils import mask_email
 from webcaf.webcaf.utils.caf import CafFormUtil
+from webcaf.webcaf.utils.permission import PermissionUtil
 from webcaf.webcaf.utils.session import SessionUtil
 from webcaf.webcaf.views.general import FormViewWithBreadcrumbs, assessment_required
 
@@ -141,9 +142,16 @@ def form_valid(self, form):
             validation.
         :return: The HTTP response returned by the parent class's form_valid method.
         """
+
+        current_user_profile = SessionUtil.get_current_user_profile(self.request)
+        # If the user does not have the edit permissions, then skip the
+        # update and show the next page.
+        if not PermissionUtil.current_user_can_edit_assessments(current_user_profile):
+            self.logger.info("Current user only has view permission, so not saving any data")
+            return FormView.form_valid(self, form)
+
         assessment = SessionUtil.get_current_assessment(self.request)
         if assessment:
-            current_user_profile = SessionUtil.get_current_user_profile(self.request)
             if self.class_id not in assessment.assessments_data:
                 assessment.assessments_data[self.class_id] = {}
             if self.stage not in assessment.assessments_data[self.class_id]:

webcaf/webcaf/templates/caf/confirmation.html

@@ -1,4 +1,5 @@
 {% extends "base.html" %}
+{% load permission_extras %}
 {% load static %}
 {% load govuk_frontend_django %}
 {% load csp %}
@@ -153,7 +154,9 @@ <h1 class="govuk-heading-l">{{ view.class_id }} {{ title }} Outcome</h1>
                     </div>
                 </div>
                 <div class="govuk-button-group">
-                    <button type="submit" class="govuk-button" data-module="govuk-button" data-govuk-button-init="">
+                    {% current_user_can_edit_assessments current_profile as can_edit_assessments %}
+                    <button type="submit" class="govuk-button" data-module="govuk-button" data-govuk-button-init=""
+                            {% if not can_edit_assessments %}disabled{% endif %}>
                         Save and continue
                     </button>
                 </div>

webcaf/webcaf/templates/caf/indicators.html

@@ -101,9 +101,8 @@ <h1 class="govuk-heading-l">{{ view.class_id }} {{ title }}</h1>
                 <div class="govuk-button-group">
                     {% current_user_can_edit_assessments current_profile as can_edit_assessments %}
                     <button type="submit" class="govuk-button" data-module="govuk-button"
-                            {% if not can_edit_assessments %} disabled {% endif %}
                             data-govuk-button-init="">
-                        Save and continue
+                        {% if not can_edit_assessments %}Continue{% else %}Save and continue{% endif %}
                     </button>
                     <a class="govuk-link" href="{% url back_url %}">Back to {{ objective_name }} </a>
                 </div>

webcaf/webcaf/templates/review/revisions.html

@@ -64,8 +64,8 @@ <h1 class="govuk-heading-l">Report history</h1>
                                href="{% url 'download-report' pk=object.id version=revision_number %}">Download
                                 (PDF)</a> |
                             <a class="govuk-link"
-                               href="{% url 'download-excel-report' pk=object.id version=revision_number %}">Download
-                                (XLS)</a>
+                               href="{% url 'download-excel-report' pk=object.id version=revision_number %}">Download data
+                                </a>
                         {% endwith %}
                     </p>
                 </div>
@@ -121,8 +121,8 @@ <h1 class="govuk-heading-l">Report history</h1>
                                href="{% url 'download-report' pk=revision.instance.id version=forloop.revcounter %}">Download
                                 (PDF)</a><br/>
                             <a class="govuk-link" target="_blank"
-                               href="{% url 'download-excel-report' pk=revision.instance.id version=forloop.revcounter %}">Download
-                                (XLS)</a>
+                               href="{% url 'download-excel-report' pk=revision.instance.id version=forloop.revcounter %}">Download data
+                                </a>
                         </td>
                     </tr>
                 {% endfor %}

webcaf/webcaf/utils/to_spreadsheet.py

@@ -276,17 +276,17 @@ def _add_recommendations_tab(wb: Workbook, review: Review):
         [
             "Contributing outcome",
             "Target CAF profile",
-            "Risk number",
         ]
         + (
             [
+                "Risk number",
                 "Risk",
-                "Recommendation number",
             ]
             if review.assessment.review_type != "peer_review"
             else []
         )
         + [
+            "Recommendation number",
             "Recommendation",
         ]
     )
@@ -312,17 +312,17 @@ def _add_recommendations_tab(wb: Workbook, review: Review):
                     [
                         contributing_outcome_titles[recommendation.outcome],
                         profile_met,
-                        f"{prefix}{recommendation_group.group_index}",
                     ]
                     + (
                         [
+                            f"{prefix}{recommendation_group.group_index}",
                             recommendation.title,
-                            recommendation.id,
                         ]
                         if review.assessment.review_type != "peer_review"
                         else []
                     )
                     + [
+                        recommendation.id,
                         recommendation.text,
                     ]
                 )