changes to -legacy suffix instead of DESNZ hoping to use the new vers…

…ion everywhere soon

caseworker/advice/conditionals.py

@@ -2,12 +2,14 @@
 
 
 def form_add_licence_conditions(step_name):
+    """Returns the boolean value for add_licence_conditions from the approval form"""
+
     def _get_form_field_boolean(wizard):
         cleaned_data = wizard.get_cleaned_data_for_step(step_name)
         return cleaned_data.get("add_licence_conditions")
 
     return _get_form_field_boolean
 
 
-def DESNZ_team(wizard):
+def is_desnz_team(wizard):
     return wizard.caseworker["team"]["alias"] in services.DESNZ_TEAMS

caseworker/advice/constants.py

@@ -29,6 +29,6 @@ class AdviceType:
 
 
 class AdviceView:
-    DESNZ_RECOMMEND_APPROVAL = "desnz_recommend_approval"
+    RECOMMEND_APPROVAL = "recommend_approval"
     LICENCE_CONDITIONS = "licence_conditions"
     LICENCE_FOOTNOTES = "licence_footnotes"

caseworker/advice/forms.py

@@ -494,7 +494,7 @@ def __init__(
         self.organisation_documents = organisation_documents
 
 
-class DESNZRecommendAnApproval(PicklistAdviceForm, BaseForm):
+class RecommendAnApproval(PicklistAdviceForm, BaseForm):
     DOCUMENT_TITLE = "Recommend approval for this case"
 
     class Layout:
@@ -540,7 +540,6 @@ class PicklistApprovalAdviceFormEdit(BaseForm):
     class Layout:
         TITLE = "Add licence conditions, instructions to exporter or footnotes (optional)"
 
-    DOCUMENT_TITLE = "Recommend approval for this case"
     proviso = forms.CharField(
         widget=forms.Textarea(attrs={"rows": 30, "class": "govuk-!-margin-top-4"}),
         label="",
@@ -557,13 +556,12 @@ def get_layout_fields(self):
         return ("proviso",)
 
 
-class PicklistApprovalAdviceForm(PicklistAdviceForm, BaseForm):
+class LicenceConditionsForm(PicklistAdviceForm, BaseForm):
     DOCUMENT_TITLE = "Recommend approval for this case"
 
     class Layout:
         TITLE = "Add licence conditions, instructions to exporter or footnotes (optional)"
 
-    DOCUMENT_TITLE = "Recommend approval for this case"
     proviso = forms.CharField(
         widget=forms.Textarea(attrs={"rows": 7, "class": "govuk-!-margin-top-4"}),
         label="",
@@ -576,7 +574,7 @@ class Layout:
         widget=forms.RadioSelect,
         choices=(),
     )
-    proviso_radios = forms.MultipleChoiceField(
+    proviso_checkboxes = forms.MultipleChoiceField(
         label="Add a licence condition (optional)",
         required=False,
         widget=forms.CheckboxSelectMultiple,
@@ -586,7 +584,7 @@ class Layout:
     def clean(self):
         cleaned_data = super().clean()
         # only return proviso (text) for selected radios, nothing else matters, join by 2 newlines
-        return {"proviso": "\r\n\r\n".join([cleaned_data[selected] for selected in cleaned_data["proviso_radios"]])}
+        return {"proviso": "\r\n\r\n".join([cleaned_data[selected] for selected in cleaned_data["proviso_checkboxes"]])}
 
     def __init__(self, *args, **kwargs):
         kwargs.pop("approval_reason")
@@ -602,7 +600,7 @@ def __init__(self, *args, **kwargs):
 
         super().__init__(*args, **kwargs)
 
-        self.fields["proviso_radios"].choices = proviso_choices
+        self.fields["proviso_checkboxes"].choices = proviso_choices
         for choices in proviso_choices:
             self.fields[choices.value] = forms.CharField(
                 widget=forms.Textarea(attrs={"rows": 3, "class": "govuk-!-margin-top-4"}),
@@ -613,7 +611,7 @@ def __init__(self, *args, **kwargs):
 
     def get_layout_fields(self):
 
-        return (ConditionalCheckboxes("proviso_radios", *self.conditional_checkbox_choices),)
+        return (ConditionalCheckboxes("proviso_checkboxes", *self.conditional_checkbox_choices),)
 
 
 class FootnotesApprovalAdviceForm(PicklistAdviceForm, BaseForm):

caseworker/advice/templates/advice/view_my_advice.html

@@ -35,10 +35,10 @@ <h2 class="govuk-error-summary__title" id="error-summary-title">
               <h1 class="govuk-heading-xl">View recommendation</h1>
               {% if my_advice %}
                 {% if buttons.edit_recommendation %}
-                  {% if caseworker|user_in_DESNZ_team%}
-                    <a role="button" draggable="false" class="govuk-button govuk-button--secondary" href="{% url 'cases:edit_advice_desnz' queue_pk case.id %}">Edit recommendation</a>
-                  {% else %}
+                  {% if caseworker|is_desnz_team and case|is_approval %}
                     <a role="button" draggable="false" class="govuk-button govuk-button--secondary" href="{% url 'cases:edit_advice' queue_pk case.id %}">Edit recommendation</a>
+                  {% else %}
+                    <a role="button" draggable="false" class="govuk-button govuk-button--secondary" href="{% url 'cases:edit_advice_legacy' queue_pk case.id %}">Edit recommendation</a>
                   {% endif %}
                 {% endif %}
                 {% if buttons.clear_recommendation %}

caseworker/advice/templatetags/advice_tags.py

@@ -225,5 +225,10 @@ def _add_team_decisions(grouped_advice):
 
 
 @register.filter
-def user_in_DESNZ_team(caseworker):
+def is_desnz_team(caseworker):
     return caseworker["team"]["alias"] in services.DESNZ_TEAMS
+
+
+@register.filter
+def is_approval(case):
+    return case["advice"][0]["type"]["key"] in ["proviso", "approve"]

caseworker/advice/urls.py

@@ -6,12 +6,12 @@
     path("", views.AdviceView.as_view(), name="advice_view"),
     path("case-details/", views.CaseDetailView.as_view(), name="case_details"),
     path("select-advice/", views.SelectAdviceView.as_view(), name="select_advice"),
+    path("approve-all-legacy/", views.GiveApprovalAdviceViewLegacy.as_view(), name="approve_all_legacy"),
     path("approve-all/", views.GiveApprovalAdviceView.as_view(), name="approve_all"),
-    path("approve-all-desnz/", views.GiveApprovalAdviceViewDESNZ.as_view(), name="approve_all_desnz"),
     path("refuse-all/", views.RefusalAdviceView.as_view(), name="refuse_all"),
     path("view-my-advice/", views.AdviceDetailView.as_view(), name="view_my_advice"),
+    path("edit-advice-legacy/", views.EditAdviceViewLegacy.as_view(), name="edit_advice_legacy"),
     path("edit-advice/", views.EditAdviceView.as_view(), name="edit_advice"),
-    path("edit-advice-desnz/", views.EditAdviceViewDESNZ.as_view(), name="edit_advice_desnz"),
     path("delete-advice/", views.DeleteAdviceView.as_view(), name="delete_advice"),
     path("countersign/", views.CountersignAdviceView.as_view(), name="countersign_advice_view"),
     path("countersign/review-advice/", views.ReviewCountersignView.as_view(), name="countersign_review"),

caseworker/advice/views.py

@@ -1,5 +1,5 @@
 from http import HTTPStatus
-from caseworker.advice.conditionals import form_add_licence_conditions, DESNZ_team
+from caseworker.advice.conditionals import form_add_licence_conditions, is_desnz_team
 from core.wizard.views import BaseSessionWizardView
 from core.wizard.conditionals import C
 import sentry_sdk
@@ -148,8 +148,8 @@ def get_success_url(self):
         recommendation = self.request.POST.get("recommendation")
         if recommendation == "approve_all":
             if self.caseworker["team"]["alias"] in services.DESNZ_TEAMS:
-                return reverse("cases:approve_all_desnz", kwargs=self.kwargs)
-            return reverse("cases:approve_all", kwargs=self.kwargs)
+                return reverse("cases:approve_all", kwargs=self.kwargs)
+            return reverse("cases:approve_all_legacy", kwargs=self.kwargs)
         else:
             return reverse("cases:refuse_all", kwargs=self.kwargs)
 
@@ -158,7 +158,7 @@ def get_context_data(self, **kwargs):
         return {**context, "security_approvals_classified_display": self.security_approvals_classified_display}
 
 
-class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, FormView):
+class GiveApprovalAdviceViewLegacy(LoginRequiredMixin, CaseContextMixin, FormView):
     """
     Form to recommend approval advice for all products on the application
     """
@@ -283,7 +283,7 @@ def get_success_url(self):
         return reverse("queues:cases", kwargs={"queue_pk": self.kwargs["queue_pk"]})
 
 
-class EditAdviceView(LoginRequiredMixin, CaseContextMixin, FormView):
+class EditAdviceViewLegacy(LoginRequiredMixin, CaseContextMixin, FormView):
     """
     Form to edit given advice for all products on the application
     """
@@ -356,19 +356,18 @@ def get_context_data(self, **kwargs):
         return context
 
 
-class GiveApprovalAdviceViewDESNZ(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView):
-    # on save combine everything into a single field
+class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView):
 
     form_list = [
-        (AdviceView.DESNZ_RECOMMEND_APPROVAL, forms.DESNZRecommendAnApproval),
-        (AdviceView.LICENCE_CONDITIONS, forms.PicklistApprovalAdviceForm),
+        (AdviceView.RECOMMEND_APPROVAL, forms.RecommendAnApproval),
+        (AdviceView.LICENCE_CONDITIONS, forms.LicenceConditionsForm),
         (AdviceView.LICENCE_FOOTNOTES, forms.FootnotesApprovalAdviceForm),
     ]
 
     condition_dict = {
-        AdviceView.DESNZ_RECOMMEND_APPROVAL: C(DESNZ_team),
-        AdviceView.LICENCE_CONDITIONS: C(form_add_licence_conditions("desnz_recommend_approval")),
-        AdviceView.LICENCE_FOOTNOTES: C(form_add_licence_conditions("desnz_recommend_approval")),
+        AdviceView.RECOMMEND_APPROVAL: C(is_desnz_team),
+        AdviceView.LICENCE_CONDITIONS: C(form_add_licence_conditions("recommend_approval")),
+        AdviceView.LICENCE_FOOTNOTES: C(form_add_licence_conditions("recommend_approval")),
     }
 
     def get_form_kwargs(self, step=None):
@@ -387,18 +386,27 @@ def get_form_kwargs(self, step=None):
     def get_success_url(self):
         return reverse("cases:view_my_advice", kwargs=self.kwargs)
 
+    @expect_status(
+        HTTPStatus.OK,
+        "Error adding approval advice",
+        "Unexpected error adding approval advice",
+    )
+    def post_approval_advice(self, data):
+        return services.post_approval_advice(self.request, self.case, data)
+
     def done(self, form_list, form_dict, **kwargs):
         data = {}
+        # flattens the form_dict
         for form_data in form_dict.values():
             data.update(form_data.cleaned_data)
-        services.post_approval_advice(self.request, self.case, data)
+        self.post_approval_advice(data)
         return redirect(self.get_success_url())
 
 
-class EditAdviceViewDESNZ(GiveApprovalAdviceViewDESNZ):
+class EditAdviceView(GiveApprovalAdviceView):
 
     form_list = [
-        (AdviceView.DESNZ_RECOMMEND_APPROVAL, forms.DESNZRecommendAnApproval),
+        (AdviceView.RECOMMEND_APPROVAL, forms.RecommendAnApproval),
         (AdviceView.LICENCE_CONDITIONS, forms.PicklistApprovalAdviceFormEdit),
         (AdviceView.LICENCE_FOOTNOTES, forms.FootnotesApprovalAdviceForm),
     ]
@@ -407,7 +415,8 @@ def get_form_initial(self, step):
         my_advice = services.filter_current_user_advice(self.case.advice, self.caseworker_id)
         advice = my_advice[0]
 
-        # default to other so that they're not likely to reselect, approval_radios isn't stored.
+        # When the form is prepopulated in the edit flow,
+        # the radio values are set to other because only the textfield values are stored it's not possible to replay the selected radio.
         return {
             "add_licence_conditions": bool(advice.get("proviso")),
             "approval_reasons": advice.get("text", ""),

caseworker/cases/helpers/case.py

@@ -108,7 +108,9 @@ def get_context_data(self, **kwargs):
                     "queue_id": self.queue_id,
                     "user_id": self.caseworker["id"],
                     "case_id": self.case_id,
-                    "return_to": reverse("cases:approve_all", kwargs={"queue_pk": self.queue_id, "pk": self.case_id}),
+                    "return_to": reverse(
+                        "cases:approve_all_legacy", kwargs={"queue_pk": self.queue_id, "pk": self.case_id}
+                    ),
                 },
             )
         )

unit_tests/caseworker/advice/templatetags/test_advice_tags.py

@@ -7,7 +7,8 @@
     is_case_pv_graded,
     get_denial_reason_display_values,
     format_serial_numbers,
-    user_in_DESNZ_team,
+    is_desnz_team,
+    is_approval,
 )
 from caseworker.cases.objects import Case
 
@@ -469,7 +470,17 @@ def test_format_serial_numbersformat_serial_numbers(serial_numbers, quantity, ex
     "alias, expected_result",
     ([("DESNZ_CHEMICAL", True), ("GIBBERISH", False)]),
 )
-def test_user_in_DESNZ_team(alias, expected_result):
+def test_is_desnz_team(alias, expected_result):
     caseworker = {"team": {"alias": alias}}
-    result = user_in_DESNZ_team(caseworker)
+    result = is_desnz_team(caseworker)
+    assert result == expected_result
+
+
+@pytest.mark.parametrize(
+    "advice, expected_result",
+    ([("approve", True), ("proviso", True), ("refuse", False)]),
+)
+def test_is_approval(advice, expected_result):
+    case = {"advice": [{"type": {"key": advice}}]}
+    result = is_approval(case)
     assert result == expected_result