From 2976b3c32a04a550770ec247b07a42d11f2b169a Mon Sep 17 00:00:00 2001
From: Tomos Williams <tllewwilliams@gmail.com>
Date: Tue, 12 Nov 2024 16:20:23 +0000
Subject: [PATCH 1/6] Added sessionwizard for DESNZ with the goal of rolling
 this out to the other forms too

---
 caseworker/advice/conditionals.py             |  17 ++
 caseworker/advice/forms.py                    | 170 +++++++++++++++++-
 caseworker/advice/services.py                 |  10 +-
 .../templates/advice/view_my_advice.html      |  21 ++-
 caseworker/advice/templatetags/advice_tags.py |   5 +
 caseworker/advice/urls.py                     |   1 +
 caseworker/advice/views.py                    | 102 +++++++----
 7 files changed, 279 insertions(+), 47 deletions(-)
 create mode 100644 caseworker/advice/conditionals.py

diff --git a/caseworker/advice/conditionals.py b/caseworker/advice/conditionals.py
new file mode 100644
index 0000000000..4ad9b63844
--- /dev/null
+++ b/caseworker/advice/conditionals.py
@@ -0,0 +1,17 @@
+from caseworker.advice import services
+
+
+def add_conditions(step_name):
+    def _get_form_field_boolean(wizard):
+        cleaned_data = wizard.get_cleaned_data_for_step(step_name)
+        return cleaned_data.get("add_conditions")
+
+    return _get_form_field_boolean
+
+
+def FCDO_team(wizard):
+    return wizard.caseworker["team"]["alias"] == services.FCDO_TEAM
+
+
+def DESNZ_team(wizard):
+    return wizard.caseworker["team"]["alias"] in services.DESNZ_TEAMS
diff --git a/caseworker/advice/forms.py b/caseworker/advice/forms.py
index 2c661349da..9c7cf63e96 100644
--- a/caseworker/advice/forms.py
+++ b/caseworker/advice/forms.py
@@ -2,11 +2,19 @@
 from django.forms.formsets import formset_factory
 from django.utils.html import format_html
 
+from core.common.forms import BaseForm
 from crispy_forms_gds.helper import FormHelper
 from crispy_forms_gds.layout import Field, Layout, Submit
 from crispy_forms_gds.choices import Choice
 
-from core.forms.layouts import ConditionalRadios, ConditionalRadiosQuestion, ExpandingFieldset, RadioTextArea
+from core.forms.layouts import (
+    ConditionalCheckboxes,
+    ConditionalCheckboxesQuestion,
+    ConditionalRadios,
+    ConditionalRadiosQuestion,
+    ExpandingFieldset,
+    RadioTextArea,
+)
 from core.forms.utils import coerce_str_to_bool
 from caseworker.tau.summaries import get_good_on_application_tau_summary
 from caseworker.tau.widgets import GoodsMultipleSelect
@@ -83,7 +91,7 @@ def __init__(self, team_name, *args, **kwargs):
         self.fields["recommendation"].label = f"{recommendation_label}?"
 
 
-class PicklistAdviceForm(forms.Form):
+class PicklistAdviceForm(BaseForm):
     def _picklist_to_choices(self, picklist_data):
         reasons_choices = []
         reasons_text = {"other": ""}
@@ -484,3 +492,161 @@ def __init__(
         self.application_pk = application_pk
         self.is_user_rfd = is_user_rfd
         self.organisation_documents = organisation_documents
+
+
+class DESNZRecommendAnApproval(PicklistAdviceForm):
+    DOCUMENT_TITLE = "Recommend approval for this case"
+
+    class Layout:
+        TITLE = "Recommend an approval"
+
+    approval_reasons = forms.CharField(
+        widget=forms.Textarea(attrs={"rows": 7, "class": "govuk-!-margin-top-4"}),
+        label="",
+        error_messages={"required": "Enter a reason for approving"},
+    )
+    approval_radios = forms.ChoiceField(
+        label="What is your reason for approving?",
+        required=False,
+        widget=forms.RadioSelect,
+        choices=(),
+    )
+    add_conditions = forms.BooleanField(
+        label="Add licence conditions, instructions to exporter or footnotes (optional)",
+        required=False,
+    )
+
+    def __init__(self, *args, **kwargs):
+        approval_reason = kwargs.pop("approval_reason")
+        # this follows the same pattern as denial_reasons.
+        approval_choices, approval_text = self._picklist_to_choices(approval_reason)
+        self.approval_text = approval_text
+        super().__init__(*args, **kwargs)
+
+        self.fields["approval_radios"].choices = approval_choices
+
+    def get_layout_fields(self):
+        return (
+            RadioTextArea("approval_radios", "approval_reasons", self.approval_text),
+            "add_conditions",
+        )
+
+
+class PicklistApprovalAdviceFormEdit(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": 30, "class": "govuk-!-margin-top-4"}),
+        label="",
+        required=False,
+    )
+
+    def __init__(self, *args, **kwargs):
+        proviso = kwargs.pop("proviso")
+        super().__init__(*args, **kwargs)
+
+    def get_layout_fields(self):
+        return ("proviso",)
+
+
+class PicklistApprovalAdviceForm(PicklistAdviceForm):
+    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="",
+        required=False,
+    )
+
+    approval_radios = forms.ChoiceField(
+        label="What is your reason for approving?",
+        required=False,
+        widget=forms.RadioSelect,
+        choices=(),
+    )
+    proviso_radios = forms.MultipleChoiceField(
+        label="Add a licence condition (optional)",
+        required=False,
+        widget=forms.CheckboxSelectMultiple,
+        choices=(),
+    )
+
+    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"]])}
+
+    def __init__(self, *args, **kwargs):
+        proviso = kwargs.pop("proviso")
+
+        proviso_choices, proviso_text = self._picklist_to_choices(proviso)
+        self.proviso_text = proviso_text
+
+        self.conditional_checkbox_choices = (
+            ConditionalCheckboxesQuestion(choices.label, choices.value) for choices in proviso_choices
+        )
+
+        super().__init__(*args, **kwargs)
+
+        self.fields["proviso_radios"].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"}),
+                label="Description",
+                required=False,
+                initial=proviso_text[choices.value],
+            )
+
+    def get_layout_fields(self):
+
+        return (ConditionalCheckboxes("proviso_radios", *self.conditional_checkbox_choices),)
+
+
+class FootnotesApprovalAdviceForm(PicklistAdviceForm):
+
+    DOCUMENT_TITLE = "Recommend approval for this case"
+
+    class Layout:
+        TITLE = "Instructions for the exporter (optional)"
+
+    instructions_to_exporter = forms.CharField(
+        widget=forms.Textarea(attrs={"rows": "3"}),
+        label="Add any instructions for the exporter (optional)",
+        help_text="These may be added to the licence cover letter, subject to review by the Licensing Unit.",
+        required=False,
+    )
+
+    footnote_details_radios = forms.ChoiceField(
+        label="Add a reporting footnote (optional)",
+        required=False,
+        widget=forms.RadioSelect,
+        choices=(),
+    )
+    footnote_details = forms.CharField(
+        widget=forms.Textarea(attrs={"rows": 3, "class": "govuk-!-margin-top-4"}),
+        label="",
+        required=False,
+    )
+
+    def __init__(self, *args, **kwargs):
+        footnote_details = kwargs.pop("footnote_details")
+        footnote_details_choices, footnote_text = self._picklist_to_choices(footnote_details)
+        self.footnote_text = footnote_text
+
+        super().__init__(*args, **kwargs)
+
+        self.fields["footnote_details_radios"].choices = footnote_details_choices
+
+    def get_layout_fields(self):
+        return (
+            "instructions_to_exporter",
+            RadioTextArea("footnote_details_radios", "footnote_details", self.footnote_text),
+        )
diff --git a/caseworker/advice/services.py b/caseworker/advice/services.py
index 144f7124d2..40342f4076 100644
--- a/caseworker/advice/services.py
+++ b/caseworker/advice/services.py
@@ -321,12 +321,12 @@ def get_advice_subjects(case, countries=None):
 def post_approval_advice(request, case, data, level="user-advice"):
     json = [
         {
-            "type": "proviso" if data["proviso"] else "approve",
+            "type": "proviso" if data.get("proviso", False) else "approve",
             "text": data["approval_reasons"],
-            "proviso": data["proviso"],
-            "note": data["instructions_to_exporter"],
-            "footnote_required": True if data["footnote_details"] else False,
-            "footnote": data["footnote_details"],
+            "proviso": data.get("proviso", ""),
+            "note": data.get("instructions_to_exporter", ""),
+            "footnote_required": True if data.get("footnote_details") else False,
+            "footnote": data.get("footnote_details", ""),
             subject_name: subject_id,
             "denial_reasons": [],
         }
diff --git a/caseworker/advice/templates/advice/view_my_advice.html b/caseworker/advice/templates/advice/view_my_advice.html
index f8476e66e0..9546fdc34a 100644
--- a/caseworker/advice/templates/advice/view_my_advice.html
+++ b/caseworker/advice/templates/advice/view_my_advice.html
@@ -1,5 +1,6 @@
 {% extends 'layouts/case.html' %}
 {% load crispy_forms_tags advice_tags %}
+{% load static advice_tags %}
 {% load rules %}
 {% block header_tabs %}
   <div id="tab-bar" class="app-case-tab-bar">
@@ -33,16 +34,18 @@ <h2 class="govuk-error-summary__title" id="error-summary-title">
             <div class="govuk-grid-column-three-quarters">
               <h1 class="govuk-heading-xl">View recommendation</h1>
               {% if my_advice %}
-                  {% if buttons.edit_recommendation %}
-                    <a role="button" draggable="false" class="govuk-button govuk-button--secondary" href="{% url 'cases:edit_advice' queue_pk case.id %}">
-                        Edit recommendation
-                    </a>
-                  {% endif %}
-                  {% if buttons.clear_recommendation %}
-                    <a role="button" draggable="false" class="govuk-button govuk-button--secondary" href="{% url 'cases:delete_advice' queue_pk case.id %}">
-                        Clear recommendation
-                    </a>
+                {% 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 %}
+                    <a role="button" draggable="false" class="govuk-button govuk-button--secondary" href="{% url 'cases:edit_advice' queue_pk case.id %}">Edit recommendation</a>
                   {% endif %}
+                {% endif %}
+                {% if buttons.clear_recommendation %}
+                  <a role="button" draggable="false" class="govuk-button govuk-button--secondary" href="{% url 'cases:delete_advice' queue_pk case.id %}">
+                      Clear recommendation
+                  </a>
+                {% endif %}
                 {% for advice in my_advice %}
                   {% include "advice/advice_details.html" %}
                 {% endfor %}
diff --git a/caseworker/advice/templatetags/advice_tags.py b/caseworker/advice/templatetags/advice_tags.py
index 6db1e9e523..826adb4b35 100644
--- a/caseworker/advice/templatetags/advice_tags.py
+++ b/caseworker/advice/templatetags/advice_tags.py
@@ -222,3 +222,8 @@ def _add_team_decisions(grouped_advice):
                 team_advice["decision"] = constants.TEAM_DECISION_REFUSED
             else:
                 team_advice["decision"] = constants.TEAM_DECISION_APPROVED_REFUSED
+
+
+@register.filter
+def user_in_DESNZ_team(caseworker):
+    return caseworker["team"]["alias"] in services.DESNZ_TEAMS
diff --git a/caseworker/advice/urls.py b/caseworker/advice/urls.py
index 4278d69278..4ef66636f3 100644
--- a/caseworker/advice/urls.py
+++ b/caseworker/advice/urls.py
@@ -10,6 +10,7 @@
     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/", views.EditAdviceView.as_view(), name="edit_advice"),
+    path("edit-advice-desnz/", views.DESNZEditAdviceView.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"),
diff --git a/caseworker/advice/views.py b/caseworker/advice/views.py
index b59fb11a7f..208d8f6e88 100644
--- a/caseworker/advice/views.py
+++ b/caseworker/advice/views.py
@@ -1,4 +1,7 @@
 from http import HTTPStatus
+from caseworker.advice.conditionals import add_conditions, FCDO_team, DESNZ_team
+from core.wizard.views import BaseSessionWizardView
+from core.wizard.conditionals import C
 import sentry_sdk
 from django.http import Http404, HttpResponseRedirect
 from django.shortcuts import redirect
@@ -153,42 +156,42 @@ def get_context_data(self, **kwargs):
         return {**context, "security_approvals_classified_display": self.security_approvals_classified_display}
 
 
-class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, FormView):
-    """
-    Form to recommend approval advice for all products on the application
-    """
-
-    template_name = "advice/give-approval-advice.html"
-
-    def get_form(self):
-        if self.caseworker["team"]["alias"] == services.FCDO_TEAM:
-            return forms.FCDOApprovalAdviceForm(
-                services.unadvised_countries(self.caseworker, self.case),
-                **self.get_form_kwargs(),
+class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView):
+    # on save combine everything into a single field
+    form_list = [
+        ("desnz_recommend_approval", forms.DESNZRecommendAnApproval),
+        ("fcdo_recommend_approval", forms.FCDOApprovalAdviceForm),
+        ("default_recommend_approval", forms.GiveApprovalAdviceForm),
+        ("conditional", forms.PicklistApprovalAdviceForm),
+        ("footers", forms.FootnotesApprovalAdviceForm),
+    ]
+
+    condition_dict = {
+        "desnz_recommend_approval": C(DESNZ_team),
+        "fcdo_recommend_approval": C(FCDO_team),
+        "default_recommend_approval": not C(DESNZ_team) or C(FCDO_team),
+        "conditional": C(add_conditions("desnz_recommend_approval")),
+        "footers": C(add_conditions("desnz_recommend_approval")),
+    }
+
+    def get_form_kwargs(self, step=None):
+        kwargs = super().get_form_kwargs(step)
+        if "recommend_approval" in step or step == None:
+            kwargs["approval_reason"] = get_picklists_list(
+                self.request, type="standard_advice", disable_pagination=True, show_deactivated=False
+            )
+        if step == "conditional":
+            kwargs["proviso"] = get_picklists_list(
+                self.request, type="proviso", disable_pagination=True, show_deactivated=False
             )
-        else:
-            return forms.GiveApprovalAdviceForm(**self.get_form_kwargs())
-
-    def get_form_kwargs(self):
-        kwargs = super().get_form_kwargs()
-        approval_reason = get_picklists_list(
-            self.request, type="standard_advice", disable_pagination=True, show_deactivated=False
-        )
-        proviso = get_picklists_list(self.request, type="proviso", disable_pagination=True, show_deactivated=False)
 
-        footnote_details = get_picklists_list(
-            self.request, type="footnotes", disable_pagination=True, show_deactivated=False
-        )
+        if step == "footers":
+            kwargs["footnote_details"] = get_picklists_list(
+                self.request, type="footnotes", disable_pagination=True, show_deactivated=False
+            )
 
-        kwargs["approval_reason"] = approval_reason
-        kwargs["proviso"] = proviso
-        kwargs["footnote_details"] = footnote_details
         return kwargs
 
-    def form_valid(self, form):
-        services.post_approval_advice(self.request, self.case, form.cleaned_data)
-        return super().form_valid(form)
-
     def get_success_url(self):
         return reverse("cases:view_my_advice", kwargs=self.kwargs)
 
@@ -200,6 +203,13 @@ def get_context_data(self, **kwargs):
             "title": f"{self.get_form().DOCUMENT_TITLE} - {self.case.reference_code} - {self.case.organisation['name']}",
         }
 
+    def done(self, form_list, form_dict, **kwargs):
+        data = {}
+        for form_data in form_dict.values():
+            data.update(form_data.cleaned_data)
+        services.post_approval_advice(self.request, self.case, data)
+        return redirect(self.get_success_url())
+
 
 class RefusalAdviceView(LoginRequiredMixin, CaseContextMixin, FormView):
     template_name = "advice/refusal_advice.html"
@@ -248,6 +258,7 @@ def get_context_data(self, **kwargs):
         nlr_products = services.filter_nlr_products(self.case["data"]["goods"])
         advice_completed = services.unadvised_countries(self.caseworker, self.case) == {}
         title = f"View recommendation for this case - {self.case.reference_code} - {self.case.organisation['name']}"
+
         return {
             **context,
             "title": title,
@@ -350,6 +361,35 @@ def get_context_data(self, **kwargs):
         return context
 
 
+class DESNZEditAdviceView(GiveApprovalAdviceView):
+
+    form_list = [
+        ("desnz_recommend_approval", forms.DESNZRecommendAnApproval),
+        ("fcdo_recommend_approval", forms.FCDOApprovalAdviceForm),
+        ("default_recommend_approval", forms.GiveApprovalAdviceForm),
+        ("conditional", forms.PicklistApprovalAdviceFormEdit),
+        ("footers", forms.FootnotesApprovalAdviceForm),
+    ]
+
+    def get_form_initial(self, step):
+        my_advice = services.filter_current_user_advice(self.case.advice, self.caseworker_id)
+        if len(my_advice) > 0:
+            advice = my_advice[0]
+
+            # default to other so that they're not likely to reselect, approval_radios isn't stored.
+            return {
+                "add_conditions": bool(advice.get("proviso")),
+                "approval_reasons": advice.get("text", ""),
+                "approval_radios": "other",
+                "proviso": advice.get("proviso"),
+                "instructions_to_exporter": advice.get("note", ""),
+                "footnote_details": advice.get("footnote", ""),
+                "footnote_details_radios": "other",
+            }
+
+        return self.instance_dict.get(step, None)
+
+
 class DeleteAdviceView(LoginRequiredMixin, CaseContextMixin, FormView):
     template_name = "advice/delete-advice.html"
     form_class = forms.DeleteAdviceForm

From f398e4cf1346c570641053f025958b894d3d52be Mon Sep 17 00:00:00 2001
From: Tomos Williams <tllewwilliams@gmail.com>
Date: Wed, 20 Nov 2024 16:02:00 +0000
Subject: [PATCH 2/6] converted this to always use the original, only use DESNZ
 during DESNZ

---
 caseworker/advice/conditionals.py             |   8 +-
 caseworker/advice/constants.py                |   6 +
 caseworker/advice/forms.py                    |  22 +-
 caseworker/advice/urls.py                     |   3 +-
 caseworker/advice/views.py                    | 145 ++++++-----
 .../advice/templatetags/test_advice_tags.py   |  11 +
 .../advice/views/test_edit_advice.py          | 155 ++++++++++++
 .../views/test_give_approval_advice_view.py   | 227 +++++++++++++++++-
 .../advice/views/test_select_advice_view.py   |  22 ++
 9 files changed, 526 insertions(+), 73 deletions(-)

diff --git a/caseworker/advice/conditionals.py b/caseworker/advice/conditionals.py
index 4ad9b63844..ffce7812af 100644
--- a/caseworker/advice/conditionals.py
+++ b/caseworker/advice/conditionals.py
@@ -1,17 +1,13 @@
 from caseworker.advice import services
 
 
-def add_conditions(step_name):
+def form_add_licence_conditions(step_name):
     def _get_form_field_boolean(wizard):
         cleaned_data = wizard.get_cleaned_data_for_step(step_name)
-        return cleaned_data.get("add_conditions")
+        return cleaned_data.get("add_licence_conditions")
 
     return _get_form_field_boolean
 
 
-def FCDO_team(wizard):
-    return wizard.caseworker["team"]["alias"] == services.FCDO_TEAM
-
-
 def DESNZ_team(wizard):
     return wizard.caseworker["team"]["alias"] in services.DESNZ_TEAMS
diff --git a/caseworker/advice/constants.py b/caseworker/advice/constants.py
index 99ace1b55f..4de3eb64f0 100644
--- a/caseworker/advice/constants.py
+++ b/caseworker/advice/constants.py
@@ -26,3 +26,9 @@ class AdviceType:
     PROVISO = "proviso"
     REFUSE = "refuse"
     NO_LICENCE_REQUIRED = "no_licence_required"
+
+
+class AdviceView:
+    DESNZ_RECOMMEND_APPROVAL = "desnz_recommend_approval"
+    LICENCE_CONDITIONS = "licence_conditions"
+    LICENCE_FOOTNOTES = "licence_footnotes"
diff --git a/caseworker/advice/forms.py b/caseworker/advice/forms.py
index 9c7cf63e96..a15725aa45 100644
--- a/caseworker/advice/forms.py
+++ b/caseworker/advice/forms.py
@@ -91,7 +91,7 @@ def __init__(self, team_name, *args, **kwargs):
         self.fields["recommendation"].label = f"{recommendation_label}?"
 
 
-class PicklistAdviceForm(BaseForm):
+class PicklistAdviceForm(forms.Form):
     def _picklist_to_choices(self, picklist_data):
         reasons_choices = []
         reasons_text = {"other": ""}
@@ -494,7 +494,7 @@ def __init__(
         self.organisation_documents = organisation_documents
 
 
-class DESNZRecommendAnApproval(PicklistAdviceForm):
+class DESNZRecommendAnApproval(PicklistAdviceForm, BaseForm):
     DOCUMENT_TITLE = "Recommend approval for this case"
 
     class Layout:
@@ -511,12 +511,14 @@ class Layout:
         widget=forms.RadioSelect,
         choices=(),
     )
-    add_conditions = forms.BooleanField(
+    add_licence_conditions = forms.BooleanField(
         label="Add licence conditions, instructions to exporter or footnotes (optional)",
         required=False,
     )
 
     def __init__(self, *args, **kwargs):
+        kwargs.pop("proviso")
+        kwargs.pop("footnote_details")
         approval_reason = kwargs.pop("approval_reason")
         # this follows the same pattern as denial_reasons.
         approval_choices, approval_text = self._picklist_to_choices(approval_reason)
@@ -528,7 +530,7 @@ def __init__(self, *args, **kwargs):
     def get_layout_fields(self):
         return (
             RadioTextArea("approval_radios", "approval_reasons", self.approval_text),
-            "add_conditions",
+            "add_licence_conditions",
         )
 
 
@@ -546,14 +548,16 @@ class Layout:
     )
 
     def __init__(self, *args, **kwargs):
-        proviso = kwargs.pop("proviso")
+        kwargs.pop("approval_reason")
+        kwargs.pop("proviso")
+        kwargs.pop("footnote_details")
         super().__init__(*args, **kwargs)
 
     def get_layout_fields(self):
         return ("proviso",)
 
 
-class PicklistApprovalAdviceForm(PicklistAdviceForm):
+class PicklistApprovalAdviceForm(PicklistAdviceForm, BaseForm):
     DOCUMENT_TITLE = "Recommend approval for this case"
 
     class Layout:
@@ -585,6 +589,8 @@ def clean(self):
         return {"proviso": "\r\n\r\n".join([cleaned_data[selected] for selected in cleaned_data["proviso_radios"]])}
 
     def __init__(self, *args, **kwargs):
+        kwargs.pop("approval_reason")
+        kwargs.pop("footnote_details")
         proviso = kwargs.pop("proviso")
 
         proviso_choices, proviso_text = self._picklist_to_choices(proviso)
@@ -610,7 +616,7 @@ def get_layout_fields(self):
         return (ConditionalCheckboxes("proviso_radios", *self.conditional_checkbox_choices),)
 
 
-class FootnotesApprovalAdviceForm(PicklistAdviceForm):
+class FootnotesApprovalAdviceForm(PicklistAdviceForm, BaseForm):
 
     DOCUMENT_TITLE = "Recommend approval for this case"
 
@@ -637,6 +643,8 @@ class Layout:
     )
 
     def __init__(self, *args, **kwargs):
+        kwargs.pop("approval_reason")
+        kwargs.pop("proviso")
         footnote_details = kwargs.pop("footnote_details")
         footnote_details_choices, footnote_text = self._picklist_to_choices(footnote_details)
         self.footnote_text = footnote_text
diff --git a/caseworker/advice/urls.py b/caseworker/advice/urls.py
index 4ef66636f3..afd1cb7d85 100644
--- a/caseworker/advice/urls.py
+++ b/caseworker/advice/urls.py
@@ -7,10 +7,11 @@
     path("case-details/", views.CaseDetailView.as_view(), name="case_details"),
     path("select-advice/", views.SelectAdviceView.as_view(), name="select_advice"),
     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/", views.EditAdviceView.as_view(), name="edit_advice"),
-    path("edit-advice-desnz/", views.DESNZEditAdviceView.as_view(), name="edit_advice_desnz"),
+    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"),
diff --git a/caseworker/advice/views.py b/caseworker/advice/views.py
index 208d8f6e88..564381f3c3 100644
--- a/caseworker/advice/views.py
+++ b/caseworker/advice/views.py
@@ -1,5 +1,5 @@
 from http import HTTPStatus
-from caseworker.advice.conditionals import add_conditions, FCDO_team, DESNZ_team
+from caseworker.advice.conditionals import form_add_licence_conditions, DESNZ_team
 from core.wizard.views import BaseSessionWizardView
 from core.wizard.conditionals import C
 import sentry_sdk
@@ -22,7 +22,7 @@
 from caseworker.tau.summaries import get_good_on_application_tau_summary
 from caseworker.users.services import get_gov_user
 
-from caseworker.advice.constants import AdviceType
+from caseworker.advice.constants import AdviceType, AdviceView
 from core import client
 from core.auth.views import LoginRequiredMixin
 from core.constants import SecurityClassifiedApprovalsType, OrganisationDocumentType
@@ -147,6 +147,8 @@ class SelectAdviceView(LoginRequiredMixin, CaseContextMixin, FormView):
     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)
         else:
             return reverse("cases:refuse_all", kwargs=self.kwargs)
@@ -156,42 +158,42 @@ def get_context_data(self, **kwargs):
         return {**context, "security_approvals_classified_display": self.security_approvals_classified_display}
 
 
-class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView):
-    # on save combine everything into a single field
-    form_list = [
-        ("desnz_recommend_approval", forms.DESNZRecommendAnApproval),
-        ("fcdo_recommend_approval", forms.FCDOApprovalAdviceForm),
-        ("default_recommend_approval", forms.GiveApprovalAdviceForm),
-        ("conditional", forms.PicklistApprovalAdviceForm),
-        ("footers", forms.FootnotesApprovalAdviceForm),
-    ]
+class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, FormView):
+    """
+    Form to recommend approval advice for all products on the application
+    """
 
-    condition_dict = {
-        "desnz_recommend_approval": C(DESNZ_team),
-        "fcdo_recommend_approval": C(FCDO_team),
-        "default_recommend_approval": not C(DESNZ_team) or C(FCDO_team),
-        "conditional": C(add_conditions("desnz_recommend_approval")),
-        "footers": C(add_conditions("desnz_recommend_approval")),
-    }
+    template_name = "advice/give-approval-advice.html"
 
-    def get_form_kwargs(self, step=None):
-        kwargs = super().get_form_kwargs(step)
-        if "recommend_approval" in step or step == None:
-            kwargs["approval_reason"] = get_picklists_list(
-                self.request, type="standard_advice", disable_pagination=True, show_deactivated=False
-            )
-        if step == "conditional":
-            kwargs["proviso"] = get_picklists_list(
-                self.request, type="proviso", disable_pagination=True, show_deactivated=False
+    def get_form(self):
+        if self.caseworker["team"]["alias"] == services.FCDO_TEAM:
+            return forms.FCDOApprovalAdviceForm(
+                services.unadvised_countries(self.caseworker, self.case),
+                **self.get_form_kwargs(),
             )
+        else:
+            return forms.GiveApprovalAdviceForm(**self.get_form_kwargs())
 
-        if step == "footers":
-            kwargs["footnote_details"] = get_picklists_list(
-                self.request, type="footnotes", disable_pagination=True, show_deactivated=False
-            )
+    def get_form_kwargs(self):
+        kwargs = super().get_form_kwargs()
+        approval_reason = get_picklists_list(
+            self.request, type="standard_advice", disable_pagination=True, show_deactivated=False
+        )
+        proviso = get_picklists_list(self.request, type="proviso", disable_pagination=True, show_deactivated=False)
+
+        footnote_details = get_picklists_list(
+            self.request, type="footnotes", disable_pagination=True, show_deactivated=False
+        )
 
+        kwargs["approval_reason"] = approval_reason
+        kwargs["proviso"] = proviso
+        kwargs["footnote_details"] = footnote_details
         return kwargs
 
+    def form_valid(self, form):
+        services.post_approval_advice(self.request, self.case, form.cleaned_data)
+        return super().form_valid(form)
+
     def get_success_url(self):
         return reverse("cases:view_my_advice", kwargs=self.kwargs)
 
@@ -203,13 +205,6 @@ def get_context_data(self, **kwargs):
             "title": f"{self.get_form().DOCUMENT_TITLE} - {self.case.reference_code} - {self.case.organisation['name']}",
         }
 
-    def done(self, form_list, form_dict, **kwargs):
-        data = {}
-        for form_data in form_dict.values():
-            data.update(form_data.cleaned_data)
-        services.post_approval_advice(self.request, self.case, data)
-        return redirect(self.get_success_url())
-
 
 class RefusalAdviceView(LoginRequiredMixin, CaseContextMixin, FormView):
     template_name = "advice/refusal_advice.html"
@@ -361,33 +356,67 @@ def get_context_data(self, **kwargs):
         return context
 
 
-class DESNZEditAdviceView(GiveApprovalAdviceView):
+class GiveApprovalAdviceViewDESNZ(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView):
+    # on save combine everything into a single field
 
     form_list = [
-        ("desnz_recommend_approval", forms.DESNZRecommendAnApproval),
-        ("fcdo_recommend_approval", forms.FCDOApprovalAdviceForm),
-        ("default_recommend_approval", forms.GiveApprovalAdviceForm),
-        ("conditional", forms.PicklistApprovalAdviceFormEdit),
-        ("footers", forms.FootnotesApprovalAdviceForm),
+        (AdviceView.DESNZ_RECOMMEND_APPROVAL, forms.DESNZRecommendAnApproval),
+        (AdviceView.LICENCE_CONDITIONS, forms.PicklistApprovalAdviceForm),
+        (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")),
+    }
+
+    def get_form_kwargs(self, step=None):
+        kwargs = super().get_form_kwargs(step)
+        kwargs["approval_reason"] = get_picklists_list(
+            self.request, type="standard_advice", disable_pagination=True, show_deactivated=False
+        )
+        kwargs["proviso"] = get_picklists_list(
+            self.request, type="proviso", disable_pagination=True, show_deactivated=False
+        )
+        kwargs["footnote_details"] = get_picklists_list(
+            self.request, type="footnotes", disable_pagination=True, show_deactivated=False
+        )
+        return kwargs
+
+    def get_success_url(self):
+        return reverse("cases:view_my_advice", kwargs=self.kwargs)
+
+    def done(self, form_list, form_dict, **kwargs):
+        data = {}
+        for form_data in form_dict.values():
+            data.update(form_data.cleaned_data)
+        services.post_approval_advice(self.request, self.case, data)
+        return redirect(self.get_success_url())
+
+
+class EditAdviceViewDESNZ(GiveApprovalAdviceViewDESNZ):
+
+    form_list = [
+        (AdviceView.DESNZ_RECOMMEND_APPROVAL, forms.DESNZRecommendAnApproval),
+        (AdviceView.LICENCE_CONDITIONS, forms.PicklistApprovalAdviceFormEdit),
+        (AdviceView.LICENCE_FOOTNOTES, forms.FootnotesApprovalAdviceForm),
     ]
 
     def get_form_initial(self, step):
         my_advice = services.filter_current_user_advice(self.case.advice, self.caseworker_id)
-        if len(my_advice) > 0:
-            advice = my_advice[0]
-
-            # default to other so that they're not likely to reselect, approval_radios isn't stored.
-            return {
-                "add_conditions": bool(advice.get("proviso")),
-                "approval_reasons": advice.get("text", ""),
-                "approval_radios": "other",
-                "proviso": advice.get("proviso"),
-                "instructions_to_exporter": advice.get("note", ""),
-                "footnote_details": advice.get("footnote", ""),
-                "footnote_details_radios": "other",
-            }
+        advice = my_advice[0]
 
-        return self.instance_dict.get(step, None)
+        # default to other so that they're not likely to reselect, approval_radios isn't stored.
+        return {
+            "add_licence_conditions": bool(advice.get("proviso")),
+            "approval_reasons": advice.get("text", ""),
+            "approval_radios": "other",
+            "proviso": advice.get("proviso"),
+            "instructions_to_exporter": advice.get("note", ""),
+            "footnote_details": advice.get("footnote", ""),
+            "footnote_details_radios": "other",
+        }
 
 
 class DeleteAdviceView(LoginRequiredMixin, CaseContextMixin, FormView):
diff --git a/unit_tests/caseworker/advice/templatetags/test_advice_tags.py b/unit_tests/caseworker/advice/templatetags/test_advice_tags.py
index 07be109c31..be05fb3013 100644
--- a/unit_tests/caseworker/advice/templatetags/test_advice_tags.py
+++ b/unit_tests/caseworker/advice/templatetags/test_advice_tags.py
@@ -7,6 +7,7 @@
     is_case_pv_graded,
     get_denial_reason_display_values,
     format_serial_numbers,
+    user_in_DESNZ_team,
 )
 from caseworker.cases.objects import Case
 
@@ -462,3 +463,13 @@ def test_get_denial_reason_display_values():
 def test_format_serial_numbersformat_serial_numbers(serial_numbers, quantity, expected_result):
     result = format_serial_numbers(serial_numbers, quantity)
     assert result == expected_result
+
+
+@pytest.mark.parametrize(
+    "alias, expected_result",
+    ([("DESNZ_CHEMICAL", True), ("GIBBERISH", False)]),
+)
+def test_user_in_DESNZ_team(alias, expected_result):
+    caseworker = {"team": {"alias": alias}}
+    result = user_in_DESNZ_team(caseworker)
+    assert result == expected_result
diff --git a/unit_tests/caseworker/advice/views/test_edit_advice.py b/unit_tests/caseworker/advice/views/test_edit_advice.py
index 2ce7e02409..ecfbdae46a 100644
--- a/unit_tests/caseworker/advice/views/test_edit_advice.py
+++ b/unit_tests/caseworker/advice/views/test_edit_advice.py
@@ -1,9 +1,12 @@
+from unittest import mock
 import pytest
 
 from copy import deepcopy
 from django.urls import reverse
 
+from caseworker.advice import services
 from core import client
+from caseworker.advice.constants import AdviceView
 
 
 @pytest.fixture(autouse=True)
@@ -203,3 +206,155 @@ def test_edit_refuse_advice_get(
     assert response.context["security_approvals_classified_display"] == "F680"
     assert response.context["edit"] is True
     assert response.context["current_user"] == mock_gov_user["user"]
+
+
+@pytest.fixture
+def url_desnz(data_queue, data_standard_case):
+    return reverse(
+        f"cases:edit_advice_desnz", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}
+    )
+
+
+@pytest.fixture
+def post_to_step(post_to_step_factory, url_desnz):
+    return post_to_step_factory(url_desnz)
+
+
+@mock.patch("caseworker.advice.views.get_gov_user")
+def test_DESNZ_give_approval_advice_post_valid(
+    mock_get_gov_user,
+    authorized_client,
+    requests_mock,
+    data_standard_case,
+    url,
+    mock_approval_reason,
+    mock_proviso,
+    mock_footnote_details,
+    standard_case_with_advice,
+    post_to_step,
+    beautiful_soup,
+):
+    user_advice_create_url = f"/cases/{data_standard_case['case']['id']}/user-advice/"
+    requests_mock.post(user_advice_create_url, json={})
+    case_data = deepcopy(data_standard_case)
+    case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"]
+    case_data["case"]["advice"] = standard_case_with_advice["advice"]
+
+    requests_mock.get(client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}"), json=case_data)
+    requests_mock.get(
+        client._build_absolute_uri(f"/gov_users/{data_standard_case['case']['id']}"),
+        json={"user": {"id": "58e62718-e889-4a01-b603-e676b794b394"}},
+    )
+
+    mock_get_gov_user.return_value = (
+        {
+            "user": {
+                "team": {
+                    "id": "58e62718-e889-4a01-b603-e676b794b394",
+                    "name": "DESNZ Chemical",
+                    "alias": services.DESNZ_CHEMICAL,
+                }
+            }
+        },
+        None,
+    )
+
+    response = post_to_step(
+        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        {"approval_reasons": "reason updated", "add_licence_conditions": False},
+    )
+    assert response.status_code == 302
+    history = [item for item in requests_mock.request_history if user_advice_create_url in item.url]
+    assert len(history) == 1
+    history = history[0]
+    assert history.method == "POST"
+    assert history.json()[0] == {
+        "type": "approve",
+        "text": "reason updated",
+        "proviso": "",
+        "note": "",
+        "footnote_required": False,
+        "footnote": "",
+        "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5",
+        "denial_reasons": [],
+    }
+
+
+@mock.patch("caseworker.advice.views.get_gov_user")
+def test_DESNZ_give_approval_advice_post_valid_add_conditional(
+    mock_get_gov_user,
+    authorized_client,
+    requests_mock,
+    data_standard_case,
+    url,
+    mock_approval_reason,
+    mock_proviso,
+    mock_footnote_details,
+    standard_case_with_advice,
+    post_to_step,
+    beautiful_soup,
+):
+    user_advice_create_url = f"/cases/{data_standard_case['case']['id']}/user-advice/"
+    requests_mock.post(user_advice_create_url, json={})
+    case_data = deepcopy(data_standard_case)
+    case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"]
+    case_data["case"]["advice"] = standard_case_with_advice["advice"]
+
+    requests_mock.get(client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}"), json=case_data)
+    requests_mock.get(
+        client._build_absolute_uri(f"/gov_users/{data_standard_case['case']['id']}"),
+        json={"user": {"id": "58e62718-e889-4a01-b603-e676b794b394"}},
+    )
+
+    mock_get_gov_user.return_value = (
+        {
+            "user": {
+                "team": {
+                    "id": "58e62718-e889-4a01-b603-e676b794b394",
+                    "name": "DESNZ Chemical",
+                    "alias": services.DESNZ_CHEMICAL,
+                }
+            }
+        },
+        None,
+    )
+
+    response = post_to_step(
+        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        {"approval_reasons": "reason updated", "add_licence_conditions": True},
+    )
+    assert response.status_code == 200
+    soup = beautiful_soup(response.content)
+    # redirected to next form
+    header = soup.find("h1")
+    assert header.text == "Add licence conditions, instructions to exporter or footnotes (optional)"
+
+    add_LC_response = post_to_step(
+        AdviceView.LICENCE_CONDITIONS,
+        {"proviso": "proviso updated"},
+    )
+    assert add_LC_response.status_code == 200
+    soup = beautiful_soup(add_LC_response.content)
+    # redirected to next form
+    header = soup.find("h1")
+    assert header.text == "Instructions for the exporter (optional)"
+
+    add_instructions_response = post_to_step(
+        AdviceView.LICENCE_FOOTNOTES,
+        {"instructions_to_exporter": "instructions updated", "footnote_details": "footnotes updated"},
+    )
+    assert add_instructions_response.status_code == 302
+    history = [item for item in requests_mock.request_history if user_advice_create_url in item.url]
+    assert len(history) == 1
+    history = history[0]
+    assert history.method == "POST"
+    assert history.json()[0] == {
+        "type": "proviso",
+        "text": "reason updated",
+        "proviso": "proviso updated",
+        "note": "instructions updated",
+        "footnote_required": True,
+        "footnote": "footnotes updated",
+        "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5",
+        "denial_reasons": [],
+    }
diff --git a/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py b/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
index d0117818ca..32d0d5e0e0 100644
--- a/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
+++ b/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
@@ -5,6 +5,7 @@
 from django.urls import reverse
 
 from caseworker.advice import services
+from caseworker.advice.constants import AdviceView
 
 
 @pytest.fixture(autouse=True)
@@ -91,7 +92,7 @@ def test_fco_give_approval_advice_existing_get(mock_get_gov_user, authorized_cli
     ],
 )
 @mock.patch("caseworker.advice.views.get_gov_user")
-def test_fco_give_approval_advice_post(
+def test_fcdo_give_approval_advice_post(
     mock_get_gov_user,
     authorized_client,
     requests_mock,
@@ -112,3 +113,227 @@ def test_fco_give_approval_advice_post(
     data = {"approval_reasons": approval_reasons, "countries": countries}
     response = authorized_client.post(url, data=data)
     assert response.status_code == expected_status_code
+
+
+@pytest.fixture
+def url_desnz(data_queue, data_standard_case):
+    return reverse(
+        "cases:approve_all_desnz", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}
+    )
+
+
+@pytest.fixture
+def post_to_step(post_to_step_factory, url_desnz):
+    return post_to_step_factory(url_desnz)
+
+
+@mock.patch("caseworker.advice.views.get_gov_user")
+def test_DESNZ_give_approval_advice_post_valid(
+    mock_get_gov_user,
+    authorized_client,
+    requests_mock,
+    data_standard_case,
+    url,
+    mock_approval_reason,
+    mock_proviso,
+    mock_footnote_details,
+    post_to_step,
+    beautiful_soup,
+):
+    mock_get_gov_user.return_value = (
+        {
+            "user": {
+                "team": {
+                    "id": "56273dd4-4634-4ad7-a782-e480f85a85a9",
+                    "name": "DESNZ Chemical",
+                    "alias": services.DESNZ_CHEMICAL,
+                }
+            }
+        },
+        None,
+    )
+    requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
+
+    response = post_to_step(
+        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        {"approval_reasons": "Data"},
+    )
+    assert response.status_code == 302
+
+
+@mock.patch("caseworker.advice.views.get_gov_user")
+def test_DESNZ_give_approval_advice_post_valid_add_conditional(
+    mock_get_gov_user,
+    authorized_client,
+    requests_mock,
+    data_standard_case,
+    url,
+    mock_approval_reason,
+    mock_proviso,
+    mock_footnote_details,
+    post_to_step,
+    beautiful_soup,
+):
+    mock_get_gov_user.return_value = (
+        {
+            "user": {
+                "team": {
+                    "id": "56273dd4-4634-4ad7-a782-e480f85a85a9",
+                    "name": "DESNZ Chemical",
+                    "alias": services.DESNZ_CHEMICAL,
+                }
+            }
+        },
+        None,
+    )
+    requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
+
+    response = post_to_step(
+        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        {"approval_reasons": "reason", "add_licence_conditions": True},
+    )
+    assert response.status_code == 200
+    soup = beautiful_soup(response.content)
+    # redirected to next form
+    header = soup.find("h1")
+    assert header.text == "Add licence conditions, instructions to exporter or footnotes (optional)"
+
+    add_LC_response = post_to_step(
+        AdviceView.LICENCE_CONDITIONS,
+        {"proviso": "proviso"},
+    )
+    assert add_LC_response.status_code == 200
+    soup = beautiful_soup(add_LC_response.content)
+    # redirected to next form
+    header = soup.find("h1")
+    assert header.text == "Instructions for the exporter (optional)"
+
+    add_instructions_response = post_to_step(
+        AdviceView.LICENCE_FOOTNOTES,
+        {"instructions_to_exporter": "instructions", "footnote_details": "footnotes"},
+    )
+    assert add_instructions_response.status_code == 302
+
+
+@mock.patch("caseworker.advice.views.get_gov_user")
+def test_DESNZ_give_approval_advice_post_valid_add_conditional_optional(
+    mock_get_gov_user,
+    authorized_client,
+    requests_mock,
+    data_standard_case,
+    url,
+    mock_approval_reason,
+    mock_proviso,
+    mock_footnote_details,
+    post_to_step,
+    beautiful_soup,
+):
+    mock_get_gov_user.return_value = (
+        {
+            "user": {
+                "team": {
+                    "id": "56273dd4-4634-4ad7-a782-e480f85a85a9",
+                    "name": "DESNZ Chemical",
+                    "alias": services.DESNZ_CHEMICAL,
+                }
+            }
+        },
+        None,
+    )
+    requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
+
+    response = post_to_step(
+        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        {"approval_reasons": "reason", "add_licence_conditions": True},
+    )
+    assert response.status_code == 200
+    soup = beautiful_soup(response.content)
+    # redirected to next form
+    header = soup.find("h1")
+    assert header.text == "Add licence conditions, instructions to exporter or footnotes (optional)"
+
+    add_LC_response = post_to_step(
+        AdviceView.LICENCE_CONDITIONS,
+        {},
+    )
+    assert add_LC_response.status_code == 200
+    soup = beautiful_soup(add_LC_response.content)
+    # redirected to next form
+    header = soup.find("h1")
+    assert header.text == "Instructions for the exporter (optional)"
+
+    add_instructions_response = post_to_step(
+        AdviceView.LICENCE_FOOTNOTES,
+        {},
+    )
+    assert add_instructions_response.status_code == 302
+
+
+@mock.patch("caseworker.advice.views.get_gov_user")
+def test_DESNZ_give_approval_advice_post_invalid(
+    mock_get_gov_user,
+    authorized_client,
+    requests_mock,
+    data_standard_case,
+    url,
+    mock_approval_reason,
+    mock_proviso,
+    mock_footnote_details,
+    post_to_step,
+    beautiful_soup,
+):
+    mock_get_gov_user.return_value = (
+        {
+            "user": {
+                "team": {
+                    "id": "56273dd4-4634-4ad7-a782-e480f85a85a9",
+                    "name": "DESNZ Chemical",
+                    "alias": services.DESNZ_CHEMICAL,
+                }
+            }
+        },
+        None,
+    )
+    requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
+
+    response = post_to_step(
+        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        {"approval_reasons": ""},
+    )
+    assert response.status_code == 200
+    soup = beautiful_soup(response.content)
+
+
+@mock.patch("caseworker.advice.views.get_gov_user")
+def test_DESNZ_give_approval_advice_post_invalid_user(
+    mock_get_gov_user,
+    authorized_client,
+    requests_mock,
+    data_standard_case,
+    url,
+    mock_approval_reason,
+    mock_proviso,
+    mock_footnote_details,
+    post_to_step,
+    beautiful_soup,
+):
+    mock_get_gov_user.return_value = (
+        {
+            "user": {
+                "team": {
+                    "id": "56273dd4-4634-4ad7-a782-e480f85a85a9",
+                    "name": "DESNZ Chemical",
+                    "alias": services.FCDO_TEAM,
+                }
+            }
+        },
+        None,
+    )
+    requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
+
+    # DESNZ only.
+    with pytest.raises(IndexError) as err:
+        response = post_to_step(
+            AdviceView.DESNZ_RECOMMEND_APPROVAL,
+            {"approval_reasons": ""},
+        )
diff --git a/unit_tests/caseworker/advice/views/test_select_advice_view.py b/unit_tests/caseworker/advice/views/test_select_advice_view.py
index 2ec69efb2b..f907efd385 100644
--- a/unit_tests/caseworker/advice/views/test_select_advice_view.py
+++ b/unit_tests/caseworker/advice/views/test_select_advice_view.py
@@ -1,8 +1,11 @@
+from unittest import mock
 import pytest
 from bs4 import BeautifulSoup
 
 from django.urls import reverse
 
+from caseworker.advice import services
+
 
 @pytest.fixture(autouse=True)
 def setup(mock_queue, mock_case):
@@ -26,6 +29,25 @@ def test_select_advice_post(authorized_client, url, recommendation, redirect):
     assert redirect in response.url
 
 
+@mock.patch("caseworker.advice.views.get_gov_user")
+def test_select_advice_post_desnz(mock_get_gov_user, authorized_client, url):
+    mock_get_gov_user.return_value = (
+        {
+            "user": {
+                "team": {
+                    "id": "56273dd4-4634-4ad7-a782-e480f85a85a9",
+                    "name": "DESNZ Chemical",
+                    "alias": services.DESNZ_CHEMICAL,
+                }
+            }
+        },
+        None,
+    )
+    response = authorized_client.post(url, data={"recommendation": "approve_all"})
+    assert response.status_code == 302
+    assert "approve-all-desnz" in response.url
+
+
 def test_view_serial_numbers_for_firearm_product_in_select_advice_view(authorized_client, data_standard_case, url):
     good_on_application = data_standard_case["case"]["data"]["goods"][0]
     assert good_on_application["firearm_details"]["serial_numbers"][0] == "12345"

From 3472d48efab60ef4089240ec3e32d3b0b771cdf0 Mon Sep 17 00:00:00 2001
From: Tomos Williams <tllewwilliams@gmail.com>
Date: Mon, 25 Nov 2024 09:31:33 +0000
Subject: [PATCH 3/6] changes to -legacy suffix instead of DESNZ hoping to use
 the new version everywhere soon

---
 caseworker/advice/conditionals.py             |   4 +-
 caseworker/advice/constants.py                |   2 +-
 caseworker/advice/forms.py                    |  14 +-
 .../templates/advice/view_my_advice.html      |   6 +-
 caseworker/advice/templatetags/advice_tags.py |   7 +-
 caseworker/advice/urls.py                     |   4 +-
 caseworker/advice/views.py                    |  41 +++--
 caseworker/cases/helpers/case.py              |   4 +-
 .../advice/templatetags/test_advice_tags.py   |  17 +-
 .../advice/views/test_edit_advice.py          | 156 +++++++++++++++---
 .../views/test_give_approval_advice_view.py   |  18 +-
 .../advice/views/test_select_advice_view.py   |   2 +-
 12 files changed, 203 insertions(+), 72 deletions(-)

diff --git a/caseworker/advice/conditionals.py b/caseworker/advice/conditionals.py
index ffce7812af..6466fc3134 100644
--- a/caseworker/advice/conditionals.py
+++ b/caseworker/advice/conditionals.py
@@ -2,6 +2,8 @@
 
 
 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")
@@ -9,5 +11,5 @@ def _get_form_field_boolean(wizard):
     return _get_form_field_boolean
 
 
-def DESNZ_team(wizard):
+def is_desnz_team(wizard):
     return wizard.caseworker["team"]["alias"] in services.DESNZ_TEAMS
diff --git a/caseworker/advice/constants.py b/caseworker/advice/constants.py
index 4de3eb64f0..80f77208f7 100644
--- a/caseworker/advice/constants.py
+++ b/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"
diff --git a/caseworker/advice/forms.py b/caseworker/advice/forms.py
index a15725aa45..5e6b992186 100644
--- a/caseworker/advice/forms.py
+++ b/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):
diff --git a/caseworker/advice/templates/advice/view_my_advice.html b/caseworker/advice/templates/advice/view_my_advice.html
index 9546fdc34a..974c50450b 100644
--- a/caseworker/advice/templates/advice/view_my_advice.html
+++ b/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 %}
diff --git a/caseworker/advice/templatetags/advice_tags.py b/caseworker/advice/templatetags/advice_tags.py
index 826adb4b35..740ed4d3e2 100644
--- a/caseworker/advice/templatetags/advice_tags.py
+++ b/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"]
diff --git a/caseworker/advice/urls.py b/caseworker/advice/urls.py
index afd1cb7d85..4e443631b2 100644
--- a/caseworker/advice/urls.py
+++ b/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"),
diff --git a/caseworker/advice/views.py b/caseworker/advice/views.py
index 564381f3c3..0db0d34dfb 100644
--- a/caseworker/advice/views.py
+++ b/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", ""),
diff --git a/caseworker/cases/helpers/case.py b/caseworker/cases/helpers/case.py
index d76903adb0..4392f1f6cb 100644
--- a/caseworker/cases/helpers/case.py
+++ b/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}
+                    ),
                 },
             )
         )
diff --git a/unit_tests/caseworker/advice/templatetags/test_advice_tags.py b/unit_tests/caseworker/advice/templatetags/test_advice_tags.py
index be05fb3013..1e66253405 100644
--- a/unit_tests/caseworker/advice/templatetags/test_advice_tags.py
+++ b/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
diff --git a/unit_tests/caseworker/advice/views/test_edit_advice.py b/unit_tests/caseworker/advice/views/test_edit_advice.py
index ecfbdae46a..eba569fcba 100644
--- a/unit_tests/caseworker/advice/views/test_edit_advice.py
+++ b/unit_tests/caseworker/advice/views/test_edit_advice.py
@@ -16,7 +16,9 @@ def setup(mock_queue, mock_case, mock_denial_reasons, mock_approval_reason, mock
 
 @pytest.fixture
 def url(data_queue, data_standard_case):
-    return reverse(f"cases:edit_advice", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]})
+    return reverse(
+        f"cases:edit_advice_legacy", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}
+    )
 
 
 def test_edit_approve_advice_post(authorized_client, requests_mock, data_standard_case, standard_case_with_advice, url):
@@ -210,9 +212,7 @@ def test_edit_refuse_advice_get(
 
 @pytest.fixture
 def url_desnz(data_queue, data_standard_case):
-    return reverse(
-        f"cases:edit_advice_desnz", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}
-    )
+    return reverse(f"cases:edit_advice", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]})
 
 
 @pytest.fixture
@@ -260,7 +260,7 @@ def test_DESNZ_give_approval_advice_post_valid(
     )
 
     response = post_to_step(
-        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        AdviceView.RECOMMEND_APPROVAL,
         {"approval_reasons": "reason updated", "add_licence_conditions": False},
     )
     assert response.status_code == 302
@@ -268,16 +268,68 @@ def test_DESNZ_give_approval_advice_post_valid(
     assert len(history) == 1
     history = history[0]
     assert history.method == "POST"
-    assert history.json()[0] == {
-        "type": "approve",
-        "text": "reason updated",
-        "proviso": "",
-        "note": "",
-        "footnote_required": False,
-        "footnote": "",
-        "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5",
-        "denial_reasons": [],
-    }
+    assert history.json() == [
+        {
+            "type": "approve",
+            "text": "reason updated",
+            "proviso": "",
+            "note": "",
+            "footnote_required": False,
+            "footnote": "",
+            "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5",
+            "denial_reasons": [],
+        },
+        {
+            "type": "approve",
+            "text": "reason updated",
+            "proviso": "",
+            "note": "",
+            "footnote_required": False,
+            "footnote": "",
+            "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8",
+            "denial_reasons": [],
+        },
+        {
+            "type": "approve",
+            "text": "reason updated",
+            "proviso": "",
+            "note": "",
+            "footnote_required": False,
+            "footnote": "",
+            "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72",
+            "denial_reasons": [],
+        },
+        {
+            "type": "approve",
+            "text": "reason updated",
+            "proviso": "",
+            "note": "",
+            "footnote_required": False,
+            "footnote": "",
+            "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747",
+            "denial_reasons": [],
+        },
+        {
+            "type": "approve",
+            "text": "reason updated",
+            "proviso": "",
+            "note": "",
+            "footnote_required": False,
+            "footnote": "",
+            "good": "9fbffa7f-ef50-402e-93ac-2f3f37d09030",
+            "denial_reasons": [],
+        },
+        {
+            "type": "no_licence_required",
+            "text": "",
+            "proviso": "",
+            "note": "",
+            "footnote_required": False,
+            "footnote": "",
+            "good": "d4feac1e-851d-41a5-b833-eb28addb8547",
+            "denial_reasons": [],
+        },
+    ]
 
 
 @mock.patch("caseworker.advice.views.get_gov_user")
@@ -320,7 +372,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
     )
 
     response = post_to_step(
-        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        AdviceView.RECOMMEND_APPROVAL,
         {"approval_reasons": "reason updated", "add_licence_conditions": True},
     )
     assert response.status_code == 200
@@ -348,13 +400,65 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
     assert len(history) == 1
     history = history[0]
     assert history.method == "POST"
-    assert history.json()[0] == {
-        "type": "proviso",
-        "text": "reason updated",
-        "proviso": "proviso updated",
-        "note": "instructions updated",
-        "footnote_required": True,
-        "footnote": "footnotes updated",
-        "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5",
-        "denial_reasons": [],
-    }
+    assert history.json() == [
+        {
+            "type": "proviso",
+            "text": "reason updated",
+            "proviso": "proviso updated",
+            "note": "instructions updated",
+            "footnote_required": True,
+            "footnote": "footnotes updated",
+            "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5",
+            "denial_reasons": [],
+        },
+        {
+            "type": "proviso",
+            "text": "reason updated",
+            "proviso": "proviso updated",
+            "note": "instructions updated",
+            "footnote_required": True,
+            "footnote": "footnotes updated",
+            "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8",
+            "denial_reasons": [],
+        },
+        {
+            "type": "proviso",
+            "text": "reason updated",
+            "proviso": "proviso updated",
+            "note": "instructions updated",
+            "footnote_required": True,
+            "footnote": "footnotes updated",
+            "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72",
+            "denial_reasons": [],
+        },
+        {
+            "type": "proviso",
+            "text": "reason updated",
+            "proviso": "proviso updated",
+            "note": "instructions updated",
+            "footnote_required": True,
+            "footnote": "footnotes updated",
+            "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747",
+            "denial_reasons": [],
+        },
+        {
+            "type": "proviso",
+            "text": "reason updated",
+            "proviso": "proviso updated",
+            "note": "instructions updated",
+            "footnote_required": True,
+            "footnote": "footnotes updated",
+            "good": "9fbffa7f-ef50-402e-93ac-2f3f37d09030",
+            "denial_reasons": [],
+        },
+        {
+            "type": "no_licence_required",
+            "text": "",
+            "proviso": "",
+            "note": "",
+            "footnote_required": False,
+            "footnote": "",
+            "good": "d4feac1e-851d-41a5-b833-eb28addb8547",
+            "denial_reasons": [],
+        },
+    ]
diff --git a/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py b/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
index 32d0d5e0e0..5f800d7e2a 100644
--- a/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
+++ b/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
@@ -15,7 +15,9 @@ def setup(mock_queue, mock_case, mock_approval_reason, mock_proviso, mock_footno
 
 @pytest.fixture
 def url(data_queue, data_standard_case):
-    return reverse("cases:approve_all", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]})
+    return reverse(
+        "cases:approve_all_legacy", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}
+    )
 
 
 def test_give_approval_advice_get(authorized_client, url):
@@ -117,9 +119,7 @@ def test_fcdo_give_approval_advice_post(
 
 @pytest.fixture
 def url_desnz(data_queue, data_standard_case):
-    return reverse(
-        "cases:approve_all_desnz", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}
-    )
+    return reverse("cases:approve_all", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]})
 
 
 @pytest.fixture
@@ -155,7 +155,7 @@ def test_DESNZ_give_approval_advice_post_valid(
     requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
 
     response = post_to_step(
-        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        AdviceView.RECOMMEND_APPROVAL,
         {"approval_reasons": "Data"},
     )
     assert response.status_code == 302
@@ -189,7 +189,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
     requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
 
     response = post_to_step(
-        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        AdviceView.RECOMMEND_APPROVAL,
         {"approval_reasons": "reason", "add_licence_conditions": True},
     )
     assert response.status_code == 200
@@ -243,7 +243,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional_optional(
     requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
 
     response = post_to_step(
-        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        AdviceView.RECOMMEND_APPROVAL,
         {"approval_reasons": "reason", "add_licence_conditions": True},
     )
     assert response.status_code == 200
@@ -297,7 +297,7 @@ def test_DESNZ_give_approval_advice_post_invalid(
     requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
 
     response = post_to_step(
-        AdviceView.DESNZ_RECOMMEND_APPROVAL,
+        AdviceView.RECOMMEND_APPROVAL,
         {"approval_reasons": ""},
     )
     assert response.status_code == 200
@@ -334,6 +334,6 @@ def test_DESNZ_give_approval_advice_post_invalid_user(
     # DESNZ only.
     with pytest.raises(IndexError) as err:
         response = post_to_step(
-            AdviceView.DESNZ_RECOMMEND_APPROVAL,
+            AdviceView.RECOMMEND_APPROVAL,
             {"approval_reasons": ""},
         )
diff --git a/unit_tests/caseworker/advice/views/test_select_advice_view.py b/unit_tests/caseworker/advice/views/test_select_advice_view.py
index f907efd385..6c0640b67c 100644
--- a/unit_tests/caseworker/advice/views/test_select_advice_view.py
+++ b/unit_tests/caseworker/advice/views/test_select_advice_view.py
@@ -45,7 +45,7 @@ def test_select_advice_post_desnz(mock_get_gov_user, authorized_client, url):
     )
     response = authorized_client.post(url, data={"recommendation": "approve_all"})
     assert response.status_code == 302
-    assert "approve-all-desnz" in response.url
+    assert "approve-all" in response.url
 
 
 def test_view_serial_numbers_for_firearm_product_in_select_advice_view(authorized_client, data_standard_case, url):

From b82c3a9b046f085da4e24c60d5b793545c6089b0 Mon Sep 17 00:00:00 2001
From: Brendan Smith <brendan.smith@digital.trade.gov.uk>
Date: Fri, 29 Nov 2024 11:16:30 +0000
Subject: [PATCH 4/6] Fix error on approval form submission

---
 caseworker/advice/views.py                             |  2 +-
 unit_tests/caseworker/advice/views/conftest.py         |  7 +++++++
 unit_tests/caseworker/advice/views/test_edit_advice.py | 10 ++++------
 .../advice/views/test_give_approval_advice_view.py     | 10 +++-------
 4 files changed, 15 insertions(+), 14 deletions(-)
 create mode 100644 unit_tests/caseworker/advice/views/conftest.py

diff --git a/caseworker/advice/views.py b/caseworker/advice/views.py
index 0db0d34dfb..cec7d1c0a8 100644
--- a/caseworker/advice/views.py
+++ b/caseworker/advice/views.py
@@ -387,7 +387,7 @@ def get_success_url(self):
         return reverse("cases:view_my_advice", kwargs=self.kwargs)
 
     @expect_status(
-        HTTPStatus.OK,
+        HTTPStatus.CREATED,
         "Error adding approval advice",
         "Unexpected error adding approval advice",
     )
diff --git a/unit_tests/caseworker/advice/views/conftest.py b/unit_tests/caseworker/advice/views/conftest.py
new file mode 100644
index 0000000000..c732667b92
--- /dev/null
+++ b/unit_tests/caseworker/advice/views/conftest.py
@@ -0,0 +1,7 @@
+import pytest
+
+
+@pytest.fixture
+def mock_post_advice(requests_mock, data_standard_case):
+    user_advice_create_url = f"/cases/{data_standard_case['case']['id']}/user-advice/"
+    return requests_mock.post(user_advice_create_url, json={}, status_code=201)
diff --git a/unit_tests/caseworker/advice/views/test_edit_advice.py b/unit_tests/caseworker/advice/views/test_edit_advice.py
index eba569fcba..c8a00f869e 100644
--- a/unit_tests/caseworker/advice/views/test_edit_advice.py
+++ b/unit_tests/caseworker/advice/views/test_edit_advice.py
@@ -230,12 +230,11 @@ def test_DESNZ_give_approval_advice_post_valid(
     mock_approval_reason,
     mock_proviso,
     mock_footnote_details,
+    mock_post_advice,
     standard_case_with_advice,
     post_to_step,
     beautiful_soup,
 ):
-    user_advice_create_url = f"/cases/{data_standard_case['case']['id']}/user-advice/"
-    requests_mock.post(user_advice_create_url, json={})
     case_data = deepcopy(data_standard_case)
     case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"]
     case_data["case"]["advice"] = standard_case_with_advice["advice"]
@@ -264,7 +263,7 @@ def test_DESNZ_give_approval_advice_post_valid(
         {"approval_reasons": "reason updated", "add_licence_conditions": False},
     )
     assert response.status_code == 302
-    history = [item for item in requests_mock.request_history if user_advice_create_url in item.url]
+    history = mock_post_advice.request_history
     assert len(history) == 1
     history = history[0]
     assert history.method == "POST"
@@ -342,12 +341,11 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
     mock_approval_reason,
     mock_proviso,
     mock_footnote_details,
+    mock_post_advice,
     standard_case_with_advice,
     post_to_step,
     beautiful_soup,
 ):
-    user_advice_create_url = f"/cases/{data_standard_case['case']['id']}/user-advice/"
-    requests_mock.post(user_advice_create_url, json={})
     case_data = deepcopy(data_standard_case)
     case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"]
     case_data["case"]["advice"] = standard_case_with_advice["advice"]
@@ -396,7 +394,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
         {"instructions_to_exporter": "instructions updated", "footnote_details": "footnotes updated"},
     )
     assert add_instructions_response.status_code == 302
-    history = [item for item in requests_mock.request_history if user_advice_create_url in item.url]
+    history = mock_post_advice.request_history
     assert len(history) == 1
     history = history[0]
     assert history.method == "POST"
diff --git a/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py b/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
index 5f800d7e2a..71fac5748f 100644
--- a/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
+++ b/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
@@ -131,12 +131,12 @@ def post_to_step(post_to_step_factory, url_desnz):
 def test_DESNZ_give_approval_advice_post_valid(
     mock_get_gov_user,
     authorized_client,
-    requests_mock,
     data_standard_case,
     url,
     mock_approval_reason,
     mock_proviso,
     mock_footnote_details,
+    mock_post_advice,
     post_to_step,
     beautiful_soup,
 ):
@@ -152,7 +152,6 @@ def test_DESNZ_give_approval_advice_post_valid(
         },
         None,
     )
-    requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
 
     response = post_to_step(
         AdviceView.RECOMMEND_APPROVAL,
@@ -165,12 +164,12 @@ def test_DESNZ_give_approval_advice_post_valid(
 def test_DESNZ_give_approval_advice_post_valid_add_conditional(
     mock_get_gov_user,
     authorized_client,
-    requests_mock,
     data_standard_case,
     url,
     mock_approval_reason,
     mock_proviso,
     mock_footnote_details,
+    mock_post_advice,
     post_to_step,
     beautiful_soup,
 ):
@@ -186,8 +185,6 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
         },
         None,
     )
-    requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
-
     response = post_to_step(
         AdviceView.RECOMMEND_APPROVAL,
         {"approval_reasons": "reason", "add_licence_conditions": True},
@@ -219,12 +216,12 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
 def test_DESNZ_give_approval_advice_post_valid_add_conditional_optional(
     mock_get_gov_user,
     authorized_client,
-    requests_mock,
     data_standard_case,
     url,
     mock_approval_reason,
     mock_proviso,
     mock_footnote_details,
+    mock_post_advice,
     post_to_step,
     beautiful_soup,
 ):
@@ -240,7 +237,6 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional_optional(
         },
         None,
     )
-    requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
 
     response = post_to_step(
         AdviceView.RECOMMEND_APPROVAL,

From f60aeccb33b556560e01fd81c9b09e46c28beed7 Mon Sep 17 00:00:00 2001
From: Tomos Williams <tllewwilliams@gmail.com>
Date: Mon, 2 Dec 2024 16:30:26 +0000
Subject: [PATCH 5/6] followed existing naming convention for forms, used
 get_payload, added a backlink and moved some logic to rules

---
 caseworker/advice/conditionals.py             |  2 +-
 caseworker/advice/constants.py                |  2 +-
 caseworker/advice/forms.py                    | 33 ++++-----
 caseworker/advice/payloads.py                 | 14 ++++
 caseworker/advice/rules.py                    | 23 +++++++
 .../templates/advice/view_my_advice.html      |  4 +-
 caseworker/advice/templatetags/advice_tags.py | 10 ---
 caseworker/advice/views.py                    | 39 +++++++----
 .../advice/templatetags/test_advice_tags.py   | 22 ------
 unit_tests/caseworker/advice/test_rules.py    | 38 +++++++++++
 .../advice/views/test_edit_advice.py          | 67 +++++++++----------
 .../views/test_give_approval_advice_view.py   | 20 +++---
 .../advice/views/test_select_advice_view.py   | 23 ++++---
 13 files changed, 175 insertions(+), 122 deletions(-)
 create mode 100644 caseworker/advice/payloads.py

diff --git a/caseworker/advice/conditionals.py b/caseworker/advice/conditionals.py
index 6466fc3134..dc86ebbabe 100644
--- a/caseworker/advice/conditionals.py
+++ b/caseworker/advice/conditionals.py
@@ -6,7 +6,7 @@ def form_add_licence_conditions(step_name):
 
     def _get_form_field_boolean(wizard):
         cleaned_data = wizard.get_cleaned_data_for_step(step_name)
-        return cleaned_data.get("add_licence_conditions")
+        return cleaned_data.get("add_licence_conditions", False)
 
     return _get_form_field_boolean
 
diff --git a/caseworker/advice/constants.py b/caseworker/advice/constants.py
index 80f77208f7..dc13a41352 100644
--- a/caseworker/advice/constants.py
+++ b/caseworker/advice/constants.py
@@ -28,7 +28,7 @@ class AdviceType:
     NO_LICENCE_REQUIRED = "no_licence_required"
 
 
-class AdviceView:
+class AdviceSteps:
     RECOMMEND_APPROVAL = "recommend_approval"
     LICENCE_CONDITIONS = "licence_conditions"
     LICENCE_FOOTNOTES = "licence_footnotes"
diff --git a/caseworker/advice/forms.py b/caseworker/advice/forms.py
index 5e6b992186..e496ce7fb6 100644
--- a/caseworker/advice/forms.py
+++ b/caseworker/advice/forms.py
@@ -494,9 +494,7 @@ def __init__(
         self.organisation_documents = organisation_documents
 
 
-class RecommendAnApproval(PicklistAdviceForm, BaseForm):
-    DOCUMENT_TITLE = "Recommend approval for this case"
-
+class RecommendAnApprovalForm(PicklistAdviceForm, BaseForm):
     class Layout:
         TITLE = "Recommend an approval"
 
@@ -517,8 +515,8 @@ class Layout:
     )
 
     def __init__(self, *args, **kwargs):
-        kwargs.pop("proviso")
-        kwargs.pop("footnote_details")
+        del kwargs["proviso"]
+        del kwargs["footnote_details"]
         approval_reason = kwargs.pop("approval_reason")
         # this follows the same pattern as denial_reasons.
         approval_choices, approval_text = self._picklist_to_choices(approval_reason)
@@ -534,9 +532,7 @@ def get_layout_fields(self):
         )
 
 
-class PicklistApprovalAdviceFormEdit(BaseForm):
-    DOCUMENT_TITLE = "Recommend approval for this case"
-
+class PicklistApprovalAdviceEditForm(BaseForm):
     class Layout:
         TITLE = "Add licence conditions, instructions to exporter or footnotes (optional)"
 
@@ -547,9 +543,9 @@ class Layout:
     )
 
     def __init__(self, *args, **kwargs):
-        kwargs.pop("approval_reason")
-        kwargs.pop("proviso")
-        kwargs.pop("footnote_details")
+        del kwargs["approval_reason"]
+        del kwargs["proviso"]
+        del kwargs["footnote_details"]
         super().__init__(*args, **kwargs)
 
     def get_layout_fields(self):
@@ -557,8 +553,6 @@ def get_layout_fields(self):
 
 
 class LicenceConditionsForm(PicklistAdviceForm, BaseForm):
-    DOCUMENT_TITLE = "Recommend approval for this case"
-
     class Layout:
         TITLE = "Add licence conditions, instructions to exporter or footnotes (optional)"
 
@@ -587,8 +581,9 @@ def clean(self):
         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")
-        kwargs.pop("footnote_details")
+        del kwargs["approval_reason"]
+        del kwargs["footnote_details"]
+
         proviso = kwargs.pop("proviso")
 
         proviso_choices, proviso_text = self._picklist_to_choices(proviso)
@@ -615,9 +610,6 @@ def get_layout_fields(self):
 
 
 class FootnotesApprovalAdviceForm(PicklistAdviceForm, BaseForm):
-
-    DOCUMENT_TITLE = "Recommend approval for this case"
-
     class Layout:
         TITLE = "Instructions for the exporter (optional)"
 
@@ -641,8 +633,9 @@ class Layout:
     )
 
     def __init__(self, *args, **kwargs):
-        kwargs.pop("approval_reason")
-        kwargs.pop("proviso")
+        del kwargs["approval_reason"]
+        del kwargs["proviso"]
+
         footnote_details = kwargs.pop("footnote_details")
         footnote_details_choices, footnote_text = self._picklist_to_choices(footnote_details)
         self.footnote_text = footnote_text
diff --git a/caseworker/advice/payloads.py b/caseworker/advice/payloads.py
new file mode 100644
index 0000000000..7b464dfe37
--- /dev/null
+++ b/caseworker/advice/payloads.py
@@ -0,0 +1,14 @@
+from caseworker.advice.constants import AdviceSteps
+from core.wizard.payloads import MergingPayloadBuilder
+
+
+def get_cleaned_data(form):
+    return form.cleaned_data
+
+
+class GiveApprovalAdvicePayloadBuilder(MergingPayloadBuilder):
+    payload_dict = {
+        AdviceSteps.RECOMMEND_APPROVAL: get_cleaned_data,
+        AdviceSteps.LICENCE_CONDITIONS: get_cleaned_data,
+        AdviceSteps.LICENCE_FOOTNOTES: get_cleaned_data,
+    }
diff --git a/caseworker/advice/rules.py b/caseworker/advice/rules.py
index cabc597733..a0b34f3e7e 100644
--- a/caseworker/advice/rules.py
+++ b/caseworker/advice/rules.py
@@ -33,6 +33,28 @@ def can_desnz_make_recommendation(user, case, queue_alias):
     return True
 
 
+def can_desnz_make_edit(team):
+    return team in services.DESNZ_TEAMS
+
+
+def case_has_approval_advice(advice):
+    if advice:
+        return advice[0]["type"]["key"] in ["proviso", "approve"]
+    return False
+
+
+@rules.predicate
+def can_user_make_desnz_edit(request, case):
+    try:
+        user = request.lite_user
+    except AttributeError:
+        return False
+
+    team = user["team"]["alias"]
+    advice = services.filter_current_user_advice(case.advice, user["id"])
+    return can_desnz_make_edit(team) and case_has_approval_advice(advice)
+
+
 @rules.predicate
 def can_user_make_recommendation(request, case):
     try:
@@ -62,3 +84,4 @@ def can_user_make_recommendation(request, case):
 
 rules.add_rule("can_user_make_recommendation", is_user_allocated & can_user_make_recommendation)
 rules.add_rule("can_user_allocate_and_approve", can_user_make_recommendation)
+rules.add_rule("can_user_make_desnz_edit", can_user_make_desnz_edit)
diff --git a/caseworker/advice/templates/advice/view_my_advice.html b/caseworker/advice/templates/advice/view_my_advice.html
index 974c50450b..8595c9af9b 100644
--- a/caseworker/advice/templates/advice/view_my_advice.html
+++ b/caseworker/advice/templates/advice/view_my_advice.html
@@ -34,8 +34,10 @@ <h2 class="govuk-error-summary__title" id="error-summary-title">
             <div class="govuk-grid-column-three-quarters">
               <h1 class="govuk-heading-xl">View recommendation</h1>
               {% if my_advice %}
+                {% test_rule 'can_user_make_desnz_edit' request case as can_user_make_desnz_edit %}
+
                 {% if buttons.edit_recommendation %}
-                  {% if caseworker|is_desnz_team and case|is_approval %}
+                  {% if can_user_make_desnz_edit %}
                     <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>
diff --git a/caseworker/advice/templatetags/advice_tags.py b/caseworker/advice/templatetags/advice_tags.py
index 740ed4d3e2..6db1e9e523 100644
--- a/caseworker/advice/templatetags/advice_tags.py
+++ b/caseworker/advice/templatetags/advice_tags.py
@@ -222,13 +222,3 @@ def _add_team_decisions(grouped_advice):
                 team_advice["decision"] = constants.TEAM_DECISION_REFUSED
             else:
                 team_advice["decision"] = constants.TEAM_DECISION_APPROVED_REFUSED
-
-
-@register.filter
-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"]
diff --git a/caseworker/advice/views.py b/caseworker/advice/views.py
index cec7d1c0a8..66bb2ee2c9 100644
--- a/caseworker/advice/views.py
+++ b/caseworker/advice/views.py
@@ -1,5 +1,6 @@
 from http import HTTPStatus
 from caseworker.advice.conditionals import form_add_licence_conditions, is_desnz_team
+from caseworker.advice.payloads import GiveApprovalAdvicePayloadBuilder
 from core.wizard.views import BaseSessionWizardView
 from core.wizard.conditionals import C
 import sentry_sdk
@@ -22,7 +23,7 @@
 from caseworker.tau.summaries import get_good_on_application_tau_summary
 from caseworker.users.services import get_gov_user
 
-from caseworker.advice.constants import AdviceType, AdviceView
+from caseworker.advice.constants import AdviceType, AdviceSteps
 from core import client
 from core.auth.views import LoginRequiredMixin
 from core.constants import SecurityClassifiedApprovalsType, OrganisationDocumentType
@@ -359,15 +360,15 @@ def get_context_data(self, **kwargs):
 class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView):
 
     form_list = [
-        (AdviceView.RECOMMEND_APPROVAL, forms.RecommendAnApproval),
-        (AdviceView.LICENCE_CONDITIONS, forms.LicenceConditionsForm),
-        (AdviceView.LICENCE_FOOTNOTES, forms.FootnotesApprovalAdviceForm),
+        (AdviceSteps.RECOMMEND_APPROVAL, forms.RecommendAnApprovalForm),
+        (AdviceSteps.LICENCE_CONDITIONS, forms.LicenceConditionsForm),
+        (AdviceSteps.LICENCE_FOOTNOTES, forms.FootnotesApprovalAdviceForm),
     ]
 
     condition_dict = {
-        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")),
+        AdviceSteps.RECOMMEND_APPROVAL: C(is_desnz_team),
+        AdviceSteps.LICENCE_CONDITIONS: C(form_add_licence_conditions("recommend_approval")),
+        AdviceSteps.LICENCE_FOOTNOTES: C(form_add_licence_conditions("recommend_approval")),
     }
 
     def get_form_kwargs(self, step=None):
@@ -386,6 +387,11 @@ def get_form_kwargs(self, step=None):
     def get_success_url(self):
         return reverse("cases:view_my_advice", kwargs=self.kwargs)
 
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context["back_link_url"] = reverse("cases:advice_view", kwargs=self.kwargs)
+        return context
+
     @expect_status(
         HTTPStatus.CREATED,
         "Error adding approval advice",
@@ -394,11 +400,11 @@ def get_success_url(self):
     def post_approval_advice(self, data):
         return services.post_approval_advice(self.request, self.case, data)
 
+    def get_payload(self, form_dict):
+        return GiveApprovalAdvicePayloadBuilder().build(form_dict)
+
     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)
+        data = self.get_payload(form_dict)
         self.post_approval_advice(data)
         return redirect(self.get_success_url())
 
@@ -406,11 +412,16 @@ def done(self, form_list, form_dict, **kwargs):
 class EditAdviceView(GiveApprovalAdviceView):
 
     form_list = [
-        (AdviceView.RECOMMEND_APPROVAL, forms.RecommendAnApproval),
-        (AdviceView.LICENCE_CONDITIONS, forms.PicklistApprovalAdviceFormEdit),
-        (AdviceView.LICENCE_FOOTNOTES, forms.FootnotesApprovalAdviceForm),
+        (AdviceSteps.RECOMMEND_APPROVAL, forms.RecommendAnApprovalForm),
+        (AdviceSteps.LICENCE_CONDITIONS, forms.PicklistApprovalAdviceEditForm),
+        (AdviceSteps.LICENCE_FOOTNOTES, forms.FootnotesApprovalAdviceForm),
     ]
 
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+        context["back_link_url"] = self.get_success_url()
+        return context
+
     def get_form_initial(self, step):
         my_advice = services.filter_current_user_advice(self.case.advice, self.caseworker_id)
         advice = my_advice[0]
diff --git a/unit_tests/caseworker/advice/templatetags/test_advice_tags.py b/unit_tests/caseworker/advice/templatetags/test_advice_tags.py
index 1e66253405..07be109c31 100644
--- a/unit_tests/caseworker/advice/templatetags/test_advice_tags.py
+++ b/unit_tests/caseworker/advice/templatetags/test_advice_tags.py
@@ -7,8 +7,6 @@
     is_case_pv_graded,
     get_denial_reason_display_values,
     format_serial_numbers,
-    is_desnz_team,
-    is_approval,
 )
 from caseworker.cases.objects import Case
 
@@ -464,23 +462,3 @@ def test_get_denial_reason_display_values():
 def test_format_serial_numbersformat_serial_numbers(serial_numbers, quantity, expected_result):
     result = format_serial_numbers(serial_numbers, quantity)
     assert result == expected_result
-
-
-@pytest.mark.parametrize(
-    "alias, expected_result",
-    ([("DESNZ_CHEMICAL", True), ("GIBBERISH", False)]),
-)
-def test_is_desnz_team(alias, expected_result):
-    caseworker = {"team": {"alias": alias}}
-    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
diff --git a/unit_tests/caseworker/advice/test_rules.py b/unit_tests/caseworker/advice/test_rules.py
index 0c305ebb34..45d08e5988 100644
--- a/unit_tests/caseworker/advice/test_rules.py
+++ b/unit_tests/caseworker/advice/test_rules.py
@@ -187,3 +187,41 @@ def test_can_user_allocate_and_approve(mock_gov_user, data_fake_queue, data_stan
     data_fake_queue["alias"] = services.NCSC_CASES_TO_REVIEW
     request = get_mock_request(mock_gov_user["user"], data_fake_queue)
     assert rules.test_rule("can_user_allocate_and_approve", request, case)
+
+
+def test_can_user_make_desnz_edit_valid(
+    mock_gov_user, data_fake_queue, data_assigned_case, standard_case_with_advice, mocker
+):
+    mock_gov_user["user"]["team"]["alias"] = services.DESNZ_TEAMS[0]
+    data_fake_queue["alias"] = services.DESNZ_CHEMICAL_CASES_TO_REVIEW
+    request = get_mock_request(mock_gov_user["user"], data_fake_queue)
+    mocker.patch(
+        "caseworker.advice.rules.services.filter_current_user_advice", return_value=standard_case_with_advice["advice"]
+    )
+    assert rules.test_rule("can_user_make_desnz_edit", request, data_assigned_case)
+
+
+def test_can_user_make_desnz_edit_invalid_advice(mock_gov_user, data_fake_queue, data_assigned_case):
+    mock_gov_user["user"]["team"]["alias"] = services.DESNZ_TEAMS[0]
+    data_fake_queue["alias"] = services.DESNZ_CHEMICAL_CASES_TO_REVIEW
+    data_assigned_case
+    request = get_mock_request(mock_gov_user["user"], data_fake_queue)
+    assert not rules.test_rule("can_user_make_desnz_edit", request, data_assigned_case)
+
+
+def test_can_user_make_desnz_edit_invalid_user(
+    mock_gov_user, data_fake_queue, data_assigned_case, standard_case_with_advice, mocker
+):
+    data_fake_queue["alias"] = services.DESNZ_CHEMICAL_CASES_TO_REVIEW
+    request = get_mock_request(mock_gov_user["user"], data_fake_queue)
+    mocker.patch(
+        "caseworker.advice.rules.services.filter_current_user_advice", return_value=standard_case_with_advice["advice"]
+    )
+    assert not rules.test_rule("can_user_make_desnz_edit", request, data_assigned_case)
+
+
+def test_can_user_make_desnz_edit_invalid_advice_and_user(mock_gov_user, data_fake_queue, data_assigned_case):
+    data_fake_queue["alias"] = services.DESNZ_CHEMICAL_CASES_TO_REVIEW
+    data_assigned_case
+    request = get_mock_request(mock_gov_user["user"], data_fake_queue)
+    assert not rules.test_rule("can_user_make_desnz_edit", request, data_assigned_case)
diff --git a/unit_tests/caseworker/advice/views/test_edit_advice.py b/unit_tests/caseworker/advice/views/test_edit_advice.py
index c8a00f869e..f1f809c5fd 100644
--- a/unit_tests/caseworker/advice/views/test_edit_advice.py
+++ b/unit_tests/caseworker/advice/views/test_edit_advice.py
@@ -1,4 +1,3 @@
-from unittest import mock
 import pytest
 
 from copy import deepcopy
@@ -6,7 +5,7 @@
 
 from caseworker.advice import services
 from core import client
-from caseworker.advice.constants import AdviceView
+from caseworker.advice.constants import AdviceSteps
 
 
 @pytest.fixture(autouse=True)
@@ -220,9 +219,7 @@ def post_to_step(post_to_step_factory, url_desnz):
     return post_to_step_factory(url_desnz)
 
 
-@mock.patch("caseworker.advice.views.get_gov_user")
 def test_DESNZ_give_approval_advice_post_valid(
-    mock_get_gov_user,
     authorized_client,
     requests_mock,
     data_standard_case,
@@ -234,18 +231,9 @@ def test_DESNZ_give_approval_advice_post_valid(
     standard_case_with_advice,
     post_to_step,
     beautiful_soup,
+    mocker,
 ):
-    case_data = deepcopy(data_standard_case)
-    case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"]
-    case_data["case"]["advice"] = standard_case_with_advice["advice"]
-
-    requests_mock.get(client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}"), json=case_data)
-    requests_mock.get(
-        client._build_absolute_uri(f"/gov_users/{data_standard_case['case']['id']}"),
-        json={"user": {"id": "58e62718-e889-4a01-b603-e676b794b394"}},
-    )
-
-    mock_get_gov_user.return_value = (
+    get_gov_user_value = (
         {
             "user": {
                 "team": {
@@ -257,9 +245,19 @@ def test_DESNZ_give_approval_advice_post_valid(
         },
         None,
     )
+    mocker.patch("caseworker.advice.views.get_gov_user", return_value=get_gov_user_value)
+    case_data = deepcopy(data_standard_case)
+    case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"]
+    case_data["case"]["advice"] = standard_case_with_advice["advice"]
+
+    requests_mock.get(client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}"), json=case_data)
+    requests_mock.get(
+        client._build_absolute_uri(f"/gov_users/{data_standard_case['case']['id']}"),
+        json={"user": {"id": "58e62718-e889-4a01-b603-e676b794b394"}},
+    )
 
     response = post_to_step(
-        AdviceView.RECOMMEND_APPROVAL,
+        AdviceSteps.RECOMMEND_APPROVAL,
         {"approval_reasons": "reason updated", "add_licence_conditions": False},
     )
     assert response.status_code == 302
@@ -331,9 +329,7 @@ def test_DESNZ_give_approval_advice_post_valid(
     ]
 
 
-@mock.patch("caseworker.advice.views.get_gov_user")
 def test_DESNZ_give_approval_advice_post_valid_add_conditional(
-    mock_get_gov_user,
     authorized_client,
     requests_mock,
     data_standard_case,
@@ -345,18 +341,9 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
     standard_case_with_advice,
     post_to_step,
     beautiful_soup,
+    mocker,
 ):
-    case_data = deepcopy(data_standard_case)
-    case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"]
-    case_data["case"]["advice"] = standard_case_with_advice["advice"]
-
-    requests_mock.get(client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}"), json=case_data)
-    requests_mock.get(
-        client._build_absolute_uri(f"/gov_users/{data_standard_case['case']['id']}"),
-        json={"user": {"id": "58e62718-e889-4a01-b603-e676b794b394"}},
-    )
-
-    mock_get_gov_user.return_value = (
+    get_gov_user_value = (
         {
             "user": {
                 "team": {
@@ -368,9 +355,19 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
         },
         None,
     )
+    mocker.patch("caseworker.advice.views.get_gov_user", return_value=get_gov_user_value)
+    case_data = deepcopy(data_standard_case)
+    case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"]
+    case_data["case"]["advice"] = standard_case_with_advice["advice"]
+
+    requests_mock.get(client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}"), json=case_data)
+    requests_mock.get(
+        client._build_absolute_uri(f"/gov_users/{data_standard_case['case']['id']}"),
+        json={"user": {"id": "58e62718-e889-4a01-b603-e676b794b394"}},
+    )
 
     response = post_to_step(
-        AdviceView.RECOMMEND_APPROVAL,
+        AdviceSteps.RECOMMEND_APPROVAL,
         {"approval_reasons": "reason updated", "add_licence_conditions": True},
     )
     assert response.status_code == 200
@@ -379,18 +376,18 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
     header = soup.find("h1")
     assert header.text == "Add licence conditions, instructions to exporter or footnotes (optional)"
 
-    add_LC_response = post_to_step(
-        AdviceView.LICENCE_CONDITIONS,
+    add_licence_condition_response = post_to_step(
+        AdviceSteps.LICENCE_CONDITIONS,
         {"proviso": "proviso updated"},
     )
-    assert add_LC_response.status_code == 200
-    soup = beautiful_soup(add_LC_response.content)
+    assert add_licence_condition_response.status_code == 200
+    soup = beautiful_soup(add_licence_condition_response.content)
     # redirected to next form
     header = soup.find("h1")
     assert header.text == "Instructions for the exporter (optional)"
 
     add_instructions_response = post_to_step(
-        AdviceView.LICENCE_FOOTNOTES,
+        AdviceSteps.LICENCE_FOOTNOTES,
         {"instructions_to_exporter": "instructions updated", "footnote_details": "footnotes updated"},
     )
     assert add_instructions_response.status_code == 302
diff --git a/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py b/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
index 71fac5748f..ab2c56f136 100644
--- a/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
+++ b/unit_tests/caseworker/advice/views/test_give_approval_advice_view.py
@@ -5,7 +5,7 @@
 from django.urls import reverse
 
 from caseworker.advice import services
-from caseworker.advice.constants import AdviceView
+from caseworker.advice.constants import AdviceSteps
 
 
 @pytest.fixture(autouse=True)
@@ -154,7 +154,7 @@ def test_DESNZ_give_approval_advice_post_valid(
     )
 
     response = post_to_step(
-        AdviceView.RECOMMEND_APPROVAL,
+        AdviceSteps.RECOMMEND_APPROVAL,
         {"approval_reasons": "Data"},
     )
     assert response.status_code == 302
@@ -186,7 +186,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
         None,
     )
     response = post_to_step(
-        AdviceView.RECOMMEND_APPROVAL,
+        AdviceSteps.RECOMMEND_APPROVAL,
         {"approval_reasons": "reason", "add_licence_conditions": True},
     )
     assert response.status_code == 200
@@ -196,7 +196,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
     assert header.text == "Add licence conditions, instructions to exporter or footnotes (optional)"
 
     add_LC_response = post_to_step(
-        AdviceView.LICENCE_CONDITIONS,
+        AdviceSteps.LICENCE_CONDITIONS,
         {"proviso": "proviso"},
     )
     assert add_LC_response.status_code == 200
@@ -206,7 +206,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional(
     assert header.text == "Instructions for the exporter (optional)"
 
     add_instructions_response = post_to_step(
-        AdviceView.LICENCE_FOOTNOTES,
+        AdviceSteps.LICENCE_FOOTNOTES,
         {"instructions_to_exporter": "instructions", "footnote_details": "footnotes"},
     )
     assert add_instructions_response.status_code == 302
@@ -239,7 +239,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional_optional(
     )
 
     response = post_to_step(
-        AdviceView.RECOMMEND_APPROVAL,
+        AdviceSteps.RECOMMEND_APPROVAL,
         {"approval_reasons": "reason", "add_licence_conditions": True},
     )
     assert response.status_code == 200
@@ -249,7 +249,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional_optional(
     assert header.text == "Add licence conditions, instructions to exporter or footnotes (optional)"
 
     add_LC_response = post_to_step(
-        AdviceView.LICENCE_CONDITIONS,
+        AdviceSteps.LICENCE_CONDITIONS,
         {},
     )
     assert add_LC_response.status_code == 200
@@ -259,7 +259,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional_optional(
     assert header.text == "Instructions for the exporter (optional)"
 
     add_instructions_response = post_to_step(
-        AdviceView.LICENCE_FOOTNOTES,
+        AdviceSteps.LICENCE_FOOTNOTES,
         {},
     )
     assert add_instructions_response.status_code == 302
@@ -293,7 +293,7 @@ def test_DESNZ_give_approval_advice_post_invalid(
     requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={})
 
     response = post_to_step(
-        AdviceView.RECOMMEND_APPROVAL,
+        AdviceSteps.RECOMMEND_APPROVAL,
         {"approval_reasons": ""},
     )
     assert response.status_code == 200
@@ -330,6 +330,6 @@ def test_DESNZ_give_approval_advice_post_invalid_user(
     # DESNZ only.
     with pytest.raises(IndexError) as err:
         response = post_to_step(
-            AdviceView.RECOMMEND_APPROVAL,
+            AdviceSteps.RECOMMEND_APPROVAL,
             {"approval_reasons": ""},
         )
diff --git a/unit_tests/caseworker/advice/views/test_select_advice_view.py b/unit_tests/caseworker/advice/views/test_select_advice_view.py
index 6c0640b67c..04c57c5443 100644
--- a/unit_tests/caseworker/advice/views/test_select_advice_view.py
+++ b/unit_tests/caseworker/advice/views/test_select_advice_view.py
@@ -1,4 +1,3 @@
-from unittest import mock
 import pytest
 from bs4 import BeautifulSoup
 
@@ -22,16 +21,20 @@ def test_select_advice_get(authorized_client, url):
     assert response.status_code == 200
 
 
-@pytest.mark.parametrize("recommendation, redirect", [("approve_all", "approve"), ("refuse_all", "refuse")])
-def test_select_advice_post(authorized_client, url, recommendation, redirect):
+@pytest.mark.parametrize(
+    "recommendation, redirect", [("approve_all", "approve-all-legacy"), ("refuse_all", "refuse-all")]
+)
+def test_select_advice_post(authorized_client, url, recommendation, redirect, data_standard_case):
     response = authorized_client.post(url, data={"recommendation": recommendation})
     assert response.status_code == 302
-    assert redirect in response.url
+    assert (
+        response.url
+        == f'/queues/00000000-0000-0000-0000-000000000001/cases/{data_standard_case["case"]["id"]}/advice/{redirect}/'
+    )
 
 
-@mock.patch("caseworker.advice.views.get_gov_user")
-def test_select_advice_post_desnz(mock_get_gov_user, authorized_client, url):
-    mock_get_gov_user.return_value = (
+def test_select_advice_post_desnz(authorized_client, url, data_standard_case, mocker):
+    get_gov_user_value = (
         {
             "user": {
                 "team": {
@@ -43,9 +46,13 @@ def test_select_advice_post_desnz(mock_get_gov_user, authorized_client, url):
         },
         None,
     )
+    mocker.patch("caseworker.advice.views.get_gov_user", return_value=get_gov_user_value)
     response = authorized_client.post(url, data={"recommendation": "approve_all"})
     assert response.status_code == 302
-    assert "approve-all" in response.url
+    assert (
+        response.url
+        == f'/queues/00000000-0000-0000-0000-000000000001/cases/{data_standard_case["case"]["id"]}/advice/approve-all/'
+    )
 
 
 def test_view_serial_numbers_for_firearm_product_in_select_advice_view(authorized_client, data_standard_case, url):

From 915e82ee99de7b90a9ee75e6afd6dafe0d7d0225 Mon Sep 17 00:00:00 2001
From: Tomos Williams <tllewwilliams@gmail.com>
Date: Mon, 2 Dec 2024 16:58:15 +0000
Subject: [PATCH 6/6] additional test cases

---
 unit_tests/caseworker/advice/test_rules.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/unit_tests/caseworker/advice/test_rules.py b/unit_tests/caseworker/advice/test_rules.py
index 45d08e5988..87922f445d 100644
--- a/unit_tests/caseworker/advice/test_rules.py
+++ b/unit_tests/caseworker/advice/test_rules.py
@@ -225,3 +225,17 @@ def test_can_user_make_desnz_edit_invalid_advice_and_user(mock_gov_user, data_fa
     data_assigned_case
     request = get_mock_request(mock_gov_user["user"], data_fake_queue)
     assert not rules.test_rule("can_user_make_desnz_edit", request, data_assigned_case)
+
+
+def test_can_user_make_desnz_edit_request_missing_attributes(mock_gov_user, data_fake_queue, data_standard_case):
+    case = Case(data_standard_case["case"])
+    request = None
+
+    assert not advice_rules.can_user_make_desnz_edit(request, case)
+
+
+def test_can_user_make_desnz_edit_user_not_allocated(mock_gov_user, data_fake_queue, data_standard_case):
+    case = Case(data_standard_case["case"])
+    request = get_mock_request(mock_gov_user["user"], data_fake_queue)
+
+    assert not rules.test_rule("can_user_make_desnz_edit", request, case)