Merge branch 'dev' into LTD-5714-other-ogd-sessionwizard

caseworker/advice/constants.py

@@ -30,7 +30,5 @@ class AdviceType:
 
 class AdviceSteps:
     RECOMMEND_APPROVAL = "recommend_approval"
-    DESNZ_APPROVAL = "desnz_approval"
-    FCDO_APPROVAL = "fcdo_approval"
     LICENCE_CONDITIONS = "licence_conditions"
     LICENCE_FOOTNOTES = "licence_footnotes"

caseworker/advice/forms/approval.py

@@ -3,8 +3,8 @@
 from core.common.forms import BaseForm
 from crispy_forms_gds.helper import FormHelper
 from crispy_forms_gds.layout import Layout, Submit
-from crispy_forms_gds.choices import Choice
 
+from caseworker.advice.forms.forms import PicklistAdviceForm
 from core.forms.layouts import (
     ConditionalCheckboxes,
     ConditionalCheckboxesQuestion,
@@ -28,22 +28,6 @@ def __init__(self, *args, **kwargs):
         self.helper.add_input(Submit("submit", "Continue"))
 
 
-class PicklistAdviceForm(forms.Form):
-    def _picklist_to_choices(self, picklist_data):
-        reasons_choices = []
-        reasons_text = {"other": ""}
-
-        for result in picklist_data["results"]:
-            key = "_".join(result.get("name").lower().split())
-            choice = Choice(key, result.get("name"))
-            if result == picklist_data["results"][-1]:
-                choice = Choice(key, result.get("name"), divider="or")
-            reasons_choices.append(choice)
-            reasons_text[key] = result.get("text")
-        reasons_choices.append(Choice("other", "Other"))
-        return reasons_choices, reasons_text
-
-
 class MoveCaseForwardForm(forms.Form):
     def __init__(self, move_case_button_label="Move case forward", *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -86,22 +70,10 @@ def get_layout_fields(self):
         )
 
 
-class LicenceConditionsForm(PicklistAdviceForm, BaseForm):
+class PicklistLicenceConditionsForm(PicklistAdviceForm, BaseForm):
     class Layout:
         TITLE = "Add licence conditions (optional)"
 
-    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_checkboxes = forms.MultipleChoiceField(
         label="",
         required=False,
@@ -111,14 +83,17 @@ 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_checkboxes"]])}
+        # only return proviso (text) for selected checkboxes, nothing else matters, join by 2 newlines
+        return {
+            "proviso": "\n\n--------\n".join(
+                [cleaned_data[selected] for selected in cleaned_data["proviso_checkboxes"]]
+            )
+        }
 
     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
@@ -129,17 +104,30 @@ def __init__(self, *args, **kwargs):
         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"}),
+                widget=forms.Textarea(attrs={"rows": 3}),
                 label="Description",
                 required=False,
                 initial=proviso_text[choices.value],
             )
 
     def get_layout_fields(self):
-
         return (ConditionalCheckboxes("proviso_checkboxes", *self.conditional_checkbox_choices),)
 
 
+class SimpleLicenceConditionsForm(BaseForm):
+    class Layout:
+        TITLE = "Add licence conditions (optional)"
+
+    proviso = forms.CharField(
+        widget=forms.Textarea(attrs={"rows": 7}),
+        label="Licence condition",
+        required=False,
+    )
+
+    def get_layout_fields(self):
+        return ("proviso",)
+
+
 class FootnotesApprovalAdviceForm(PicklistAdviceForm, BaseForm):
     class Layout:
         TITLE = "Add instructions to the exporter, or a reporting footnote (optional)"
@@ -158,7 +146,7 @@ class Layout:
         choices=(),
     )
     footnote_details = forms.CharField(
-        widget=forms.Textarea(attrs={"rows": 3, "class": "govuk-!-margin-top-4"}),
+        widget=forms.Textarea(attrs={"rows": 3}),
         label="",
         required=False,
     )

caseworker/advice/forms/consolidate.py

@@ -59,7 +59,7 @@ class ConsolidateApprovalForm(PicklistAdviceForm):
     def __init__(self, approval_reason, proviso, **kwargs):
         super().__init__(**kwargs)
 
-        approval_choices, approval_text = self._picklist_to_choices(approval_reason, include_other=False)
+        approval_choices, approval_text = self._picklist_to_choices(approval_reason)
         self.approval_text = approval_text
         self.fields["approval_radios"].choices = approval_choices
 

caseworker/advice/forms/forms.py

@@ -31,7 +31,7 @@ def get_approval_advice_form_factory(advice, approval_reason, proviso, footnote_
 class PicklistAdviceForm(forms.Form):
     def _picklist_to_choices(self, picklist_data, include_other=True):
         reasons_choices = []
-        reasons_text = {"other": ""}
+        reasons_text = {}
 
         for result in picklist_data["results"]:
             key = "_".join(result.get("name").lower().split())
@@ -40,7 +40,9 @@ def _picklist_to_choices(self, picklist_data, include_other=True):
                 choice = Choice(key, result.get("name"), divider="or")
             reasons_choices.append(choice)
             reasons_text[key] = result.get("text")
-        if include_other:
+        picklist_choices = len(reasons_choices) > 0
+        if include_other and picklist_choices:
+            reasons_text["other"] = ""
             reasons_choices.append(Choice("other", "Other"))
         return reasons_choices, reasons_text
 

caseworker/advice/views/approval.py

@@ -2,7 +2,8 @@
 from caseworker.advice.conditionals import form_add_licence_conditions, is_fcdo_team
 from caseworker.advice.forms.approval import (
     FootnotesApprovalAdviceForm,
-    LicenceConditionsForm,
+    PicklistLicenceConditionsForm,
+    SimpleLicenceConditionsForm,
     RecommendAnApprovalForm,
     SelectAdviceForm,
 )
@@ -41,20 +42,20 @@ def get_context_data(self, **kwargs):
         return {**context, "security_approvals_classified_display": self.security_approvals_classified_display}
 
 
-class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView):
-
-    form_list = [
-        (AdviceSteps.RECOMMEND_APPROVAL, RecommendAnApprovalForm),
-        (AdviceSteps.LICENCE_CONDITIONS, LicenceConditionsForm),
-        (AdviceSteps.LICENCE_FOOTNOTES, FootnotesApprovalAdviceForm),
-    ]
+class BaseApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView):
 
     condition_dict = {
         AdviceSteps.RECOMMEND_APPROVAL: ~C(is_fcdo_team),
         AdviceSteps.LICENCE_CONDITIONS: C(form_add_licence_conditions(AdviceSteps.RECOMMEND_APPROVAL)),
         AdviceSteps.LICENCE_FOOTNOTES: C(form_add_licence_conditions(AdviceSteps.RECOMMEND_APPROVAL)),
     }
 
+    form_list = [
+        (AdviceSteps.RECOMMEND_APPROVAL, RecommendAnApprovalForm),
+        (AdviceSteps.LICENCE_CONDITIONS, PicklistLicenceConditionsForm),
+        (AdviceSteps.LICENCE_FOOTNOTES, FootnotesApprovalAdviceForm),
+    ]
+
     step_kwargs = {
         AdviceSteps.RECOMMEND_APPROVAL: approval_picklist,
         AdviceSteps.LICENCE_CONDITIONS: proviso_picklist,
@@ -84,3 +85,18 @@ def done(self, form_list, form_dict, **kwargs):
         data = self.get_payload(form_dict)
         self.post_approval_advice(data)
         return redirect(self.get_success_url())
+
+
+class GiveApprovalAdviceView(BaseApprovalAdviceView):
+
+    def get_form(self, step=None, data=None, files=None):
+
+        if step == AdviceSteps.LICENCE_CONDITIONS:
+            picklist_form_kwargs = self.step_kwargs[AdviceSteps.LICENCE_CONDITIONS](self)
+            picklist_options_exist = len(picklist_form_kwargs["proviso"]["results"]) > 0
+            if picklist_options_exist:
+                return PicklistLicenceConditionsForm(data=data, prefix=step, **picklist_form_kwargs)
+            else:
+                return SimpleLicenceConditionsForm(data=data, prefix=step)
+
+        return super().get_form(step, data, files)

caseworker/advice/views/edit.py

@@ -1,16 +1,19 @@
-from caseworker.advice.forms.approval import FootnotesApprovalAdviceForm, RecommendAnApprovalForm
-from caseworker.advice.forms.edit import PicklistApprovalAdviceEditForm
-from caseworker.advice.views.approval import GiveApprovalAdviceView
+from caseworker.advice.forms.approval import (
+    FootnotesApprovalAdviceForm,
+    RecommendAnApprovalForm,
+    SimpleLicenceConditionsForm,
+)
+from caseworker.advice.views.approval import BaseApprovalAdviceView
 from caseworker.advice import services
 from caseworker.advice.constants import AdviceSteps
 from caseworker.advice.picklist_helpers import approval_picklist, footnote_picklist
 
 
-class EditAdviceView(GiveApprovalAdviceView):
+class EditAdviceView(BaseApprovalAdviceView):
 
     form_list = [
         (AdviceSteps.RECOMMEND_APPROVAL, RecommendAnApprovalForm),
-        (AdviceSteps.LICENCE_CONDITIONS, PicklistApprovalAdviceEditForm),
+        (AdviceSteps.LICENCE_CONDITIONS, SimpleLicenceConditionsForm),
         (AdviceSteps.LICENCE_FOOTNOTES, FootnotesApprovalAdviceForm),
     ]
 

caseworker/advice/views/tests/test_consolidate_advice.py

@@ -401,10 +401,7 @@ def test_ConsolidateApproveView_GET_canned_snippets(
     soup = BeautifulSoup(response.content, "html.parser")
     assert "firearm serial numbers" in soup.find("div", {"id": "div_id_proviso_snippets"}).text
     assert soup.find("button", attrs={"data-snippet-key": "firearm_serial_numbers"}).text == "Add licence condition"
-    assert (
-        soup.find("script", {"id": "proviso"}).text
-        == '{"other": "", "firearm_serial_numbers": "Firearm serial numbers text"}'
-    )
+    assert soup.find("script", {"id": "proviso"}).text == '{"firearm_serial_numbers": "Firearm serial numbers text"}'
 
 
 def test_ConsolidateApproveView_POST_bad_input(

core/helpers.py

@@ -72,3 +72,9 @@ def stream_document_response(api_response):
     ]:
         response.headers[header_to_copy] = api_response.headers[header_to_copy]
     return response
+
+
+def remove_non_printable_characters(str):
+    # Given a string this will remove all non_printable_characters from ascii table
+    # Chars 9(/h), 10 (/n), 13(/r) are maintained
+    return "".join([c for c in str if ord(c) > 31 or ord(c) in [9, 10, 13]])

exporter/applications/forms/parties.py

@@ -1,3 +1,4 @@
+from core.helpers import remove_non_printable_characters
 from crispy_forms_gds.choices import Choice
 from crispy_forms_gds.helper import FormHelper
 from crispy_forms_gds.layout import Layout, Submit, HTML
@@ -225,6 +226,10 @@ class PartyNameForm(BaseForm):
         ],
     )
 
+    def clean_name(self):
+        name = self.cleaned_data["name"]
+        return remove_non_printable_characters(name)
+
     def get_layout_fields(self):
         return ("name",)
 
@@ -315,6 +320,10 @@ def __init__(self, *args, **kwargs):
         country_choices = [(country["id"], country["name"]) for country in countries]
         self.fields["country"].choices += country_choices
 
+    def clean_address(self):
+        address = self.cleaned_data["address"]
+        return remove_non_printable_characters(address)
+
     def get_layout_fields(self):
         return (
             "address",