From f9ad68754856ca98bf7d3240803e3890c5b6d941 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 5 Dec 2024 11:08:05 +0000 Subject: [PATCH 01/16] minor changes to basesessionwizard and splitting up forms/views into folder structure --- caseworker/advice/conditionals.py | 8 + caseworker/advice/views/add_advice.py | 80 ++++++++ caseworker/advice/views/edit_advice.py | 42 ++++ .../advice/views/test_consolidate.py | 180 ++++++++++++++++++ .../advice/views/test_lu_consolidate.py | 80 ++++++++ 5 files changed, 390 insertions(+) create mode 100644 caseworker/advice/views/add_advice.py create mode 100644 caseworker/advice/views/edit_advice.py diff --git a/caseworker/advice/conditionals.py b/caseworker/advice/conditionals.py index dc86ebbab..f5a8a3630 100644 --- a/caseworker/advice/conditionals.py +++ b/caseworker/advice/conditionals.py @@ -13,3 +13,11 @@ def _get_form_field_boolean(wizard): def is_desnz_team(wizard): return wizard.caseworker["team"]["alias"] in services.DESNZ_TEAMS + + +def is_fcdo_team(wizard): + return wizard.caseworker["team"]["alias"] == services.FCDO_TEAM + + +def default_form(wizard): + return not (is_fcdo_team(wizard) or is_desnz_team(wizard)) diff --git a/caseworker/advice/views/add_advice.py b/caseworker/advice/views/add_advice.py new file mode 100644 index 000000000..fe2167214 --- /dev/null +++ b/caseworker/advice/views/add_advice.py @@ -0,0 +1,80 @@ +from http import HTTPStatus +from caseworker.advice.conditionals import form_add_licence_conditions, is_desnz_team +from caseworker.advice.forms.approval import ( + FootnotesApprovalAdviceForm, + LicenceConditionsForm, + RecommendAnApprovalForm, +) +from caseworker.advice.payloads import GiveApprovalAdvicePayloadBuilder +from caseworker.advice.picklist_helpers import approval_picklist, footnote_picklist, proviso_picklist +from core.wizard.views import BaseSessionWizardView +from core.wizard.conditionals import C +from django.shortcuts import redirect +from django.urls import reverse +from caseworker.advice.views.mixins import CaseContextMixin +from caseworker.advice import services + +from caseworker.advice.constants import AdviceSteps +from core.auth.views import LoginRequiredMixin +from core.decorators import expect_status + + +# class SelectAdviceView(LoginRequiredMixin, CaseContextMixin, FormView): +# template_name = "advice/select_advice.html" +# form_class = SelectAdviceForm + +# def get_success_url(self): +# recommendation = self.request.POST.get("recommendation") +# if recommendation == "approve_all": +# return reverse("cases:approve_all", kwargs=self.kwargs) +# else: +# return reverse("cases:refuse_all", kwargs=self.kwargs) + +# def get_context_data(self, **kwargs): +# context = super().get_context_data(**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), + ] + + condition_dict = { + AdviceSteps.RECOMMEND_APPROVAL: C(is_desnz_team), + AdviceSteps.LICENCE_CONDITIONS: C(form_add_licence_conditions(AdviceSteps.RECOMMEND_APPROVAL)), + AdviceSteps.LICENCE_FOOTNOTES: C(form_add_licence_conditions(AdviceSteps.RECOMMEND_APPROVAL)), + } + + step_kwargs = { + AdviceSteps.RECOMMEND_APPROVAL: approval_picklist, + AdviceSteps.LICENCE_CONDITIONS: proviso_picklist, + AdviceSteps.LICENCE_FOOTNOTES: footnote_picklist, + } + + 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", + "Unexpected error adding approval advice", + ) + 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 = self.get_payload(form_dict) + self.post_approval_advice(data) + return redirect(self.get_success_url()) diff --git a/caseworker/advice/views/edit_advice.py b/caseworker/advice/views/edit_advice.py new file mode 100644 index 000000000..750ef1575 --- /dev/null +++ b/caseworker/advice/views/edit_advice.py @@ -0,0 +1,42 @@ +from caseworker.advice.forms.approval import FootnotesApprovalAdviceForm, RecommendAnApprovalForm +from caseworker.advice.forms.edit import PicklistApprovalAdviceEditForm +from caseworker.advice.views.add_advice import GiveApprovalAdviceView +from caseworker.advice import services +from caseworker.advice.constants import AdviceSteps +from caseworker.advice.picklist_helpers import approval_picklist, footnote_picklist + + +class EditAdviceView(GiveApprovalAdviceView): + + form_list = [ + (AdviceSteps.RECOMMEND_APPROVAL, RecommendAnApprovalForm), + (AdviceSteps.LICENCE_CONDITIONS, PicklistApprovalAdviceEditForm), + (AdviceSteps.LICENCE_FOOTNOTES, FootnotesApprovalAdviceForm), + ] + + step_kwargs = { + AdviceSteps.RECOMMEND_APPROVAL: approval_picklist, + AdviceSteps.LICENCE_CONDITIONS: None, + AdviceSteps.LICENCE_FOOTNOTES: footnote_picklist, + } + + 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] + + # 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", ""), + "approval_radios": "other", + "proviso": advice.get("proviso"), + "instructions_to_exporter": advice.get("note", ""), + "footnote_details": advice.get("footnote", ""), + "footnote_details_radios": "other", + } diff --git a/unit_tests/caseworker/advice/views/test_consolidate.py b/unit_tests/caseworker/advice/views/test_consolidate.py index bb9c2f798..aa22e15c1 100644 --- a/unit_tests/caseworker/advice/views/test_consolidate.py +++ b/unit_tests/caseworker/advice/views/test_consolidate.py @@ -6,6 +6,7 @@ from caseworker.advice.forms.consolidate import ( ConsolidateApprovalForm, ConsolidateSelectAdviceForm, + LUConsolidateRefusalForm, ) from caseworker.advice import services from caseworker.advice.forms.refusal import RefusalAdviceForm @@ -305,6 +306,185 @@ def gov_user(): } +@pytest.mark.parametrize( + "path, form_class, team_alias, team_name", + ( + ("", ConsolidateApprovalForm, LICENSING_UNIT_TEAM, "LU Team"), + ("", ConsolidateApprovalForm, MOD_ECJU_TEAM, "MOD Team"), + ("approve/", ConsolidateApprovalForm, LICENSING_UNIT_TEAM, "LU Team"), + ("refuse/", LUConsolidateRefusalForm, LICENSING_UNIT_TEAM, "LU Team"), + ("approve/", ConsolidateApprovalForm, MOD_ECJU_TEAM, "MOD Team"), + ("refuse/", RefusalAdviceForm, MOD_ECJU_TEAM, "MOD Team"), + ), +) +def test_consolidate_review( + requests_mock, + authorized_client, + data_standard_case, + url, + advice_to_consolidate, + gov_user, + path, + form_class, + team_alias, + team_name, +): + data_standard_case["case"]["advice"] = advice_to_consolidate + gov_user["user"]["team"]["name"] = team_name + gov_user["user"]["team"]["alias"] = team_alias + + requests_mock.get( + client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), + json=gov_user, + ) + + response = authorized_client.get(url + path) + assert response.status_code == 200 + form = response.context["form"] + assert isinstance(form, form_class) + + advice_to_review = list(response.context["advice_to_consolidate"]) + advice_teams = {item[0]["user"]["team"]["alias"] for item in advice_to_review} + + if team_alias == LICENSING_UNIT_TEAM: + assert advice_teams == {FCDO_TEAM, MOD_ECJU_TEAM} + elif team_alias == MOD_ECJU_TEAM: + assert bool(advice_teams.intersection(MOD_CONSOLIDATE_TEAMS)) == True + + +def test_approval_reasons_mocked( + requests_mock, + authorized_client, + data_standard_case, + url, + advice_to_consolidate, + gov_user, +): + data_standard_case["case"]["advice"] = advice_to_consolidate + gov_user["user"]["team"]["name"] = "LU Team" + gov_user["user"]["team"]["alias"] = LICENSING_UNIT_TEAM + + requests_mock.get( + client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), + json=gov_user, + ) + + response = authorized_client.get(url + "approve/") + assert response.status_code == 200 + form = response.context["form"] + assert isinstance(form, GiveApprovalAdviceForm) + # this is built mock_approval_reason + response_choices = [list(choice) for choice in form.fields["approval_radios"].choices] + assert response_choices == [ + ["no_concerns", "no concerns"], + ["concerns", "concerns"], + ["wmd", "wmd"], + ["other", "Other"], + ] + assert form.approval_text == { + "no_concerns": "No Concerns Text", + "concerns": "Concerns Text", + "wmd": "Weapons of mass destruction Text", + "other": "", + } + + +def test_approval_reasons_manual( + requests_mock, + authorized_client, + data_standard_case, + url, + advice_to_consolidate, + gov_user, +): + data_standard_case["case"]["advice"] = advice_to_consolidate + gov_user["user"]["team"]["name"] = "LU Team" + gov_user["user"]["team"]["alias"] = LICENSING_UNIT_TEAM + + requests_mock.get( + client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), + json=gov_user, + ) + + requests_mock.get( + client._build_absolute_uri( + "/picklist/?type=standard_advice&page=1&disable_pagination=True&show_deactivated=False" + ), + json={ + "results": [ + {"name": "custom Value", "text": "This Casing is Maintained"}, + {"name": "another custom value with many spaces", "text": "Concerns Text"}, + {"name": "ALLCAPSNOSPACES", "text": "This is all caps text"}, + ] + }, + ) + response = authorized_client.get(url + "approve/") + assert response.status_code == 200 + form = response.context["form"] + assert isinstance(form, GiveApprovalAdviceForm) + response_choices = [list(choice) for choice in form.fields["approval_radios"].choices] + + assert list(response_choices) == [ + ["custom_value", "custom Value"], + ["another_custom_value_with_many_spaces", "another custom value with many spaces"], + ["allcapsnospaces", "ALLCAPSNOSPACES"], + ["other", "Other"], + ] + assert form.approval_text == { + "custom_value": "This Casing is Maintained", + "another_custom_value_with_many_spaces": "Concerns Text", + "allcapsnospaces": "This is all caps text", + "other": "", + } + + +def test_proviso_manual( + requests_mock, + authorized_client, + data_standard_case, + url, + advice_to_consolidate, + gov_user, +): + data_standard_case["case"]["advice"] = advice_to_consolidate + gov_user["user"]["team"]["name"] = "LU Team" + gov_user["user"]["team"]["alias"] = LICENSING_UNIT_TEAM + + requests_mock.get( + client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), + json=gov_user, + ) + + requests_mock.get( + client._build_absolute_uri("/picklist/?type=proviso&page=1&disable_pagination=True&show_deactivated=False"), + json={ + "results": [ + {"name": "custom Value", "text": "This Casing is Maintained"}, + {"name": "another custom value with many spaces", "text": "Concerns Text"}, + {"name": "ALLCAPSNOSPACES", "text": "This is all caps text"}, + ] + }, + ) + response = authorized_client.get(url + "approve/") + assert response.status_code == 200 + form = response.context["form"] + assert isinstance(form, GiveApprovalAdviceForm) + response_choices = [list(choice) for choice in form.fields["proviso_radios"].choices] + + assert list(response_choices) == [ + ["custom_value", "custom Value"], + ["another_custom_value_with_many_spaces", "another custom value with many spaces"], + ["allcapsnospaces", "ALLCAPSNOSPACES"], + ["other", "Other"], + ] + assert form.proviso_text == { + "custom_value": "This Casing is Maintained", + "another_custom_value_with_many_spaces": "Concerns Text", + "allcapsnospaces": "This is all caps text", + "other": "", + } + + @pytest.mark.parametrize( "team_alias, team_name, recommendation, redirect", [ diff --git a/unit_tests/caseworker/advice/views/test_lu_consolidate.py b/unit_tests/caseworker/advice/views/test_lu_consolidate.py index 381e2f00a..85821ec83 100644 --- a/unit_tests/caseworker/advice/views/test_lu_consolidate.py +++ b/unit_tests/caseworker/advice/views/test_lu_consolidate.py @@ -123,3 +123,83 @@ def advice(current_user): } for good_id in ("0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", "6daad1c3-cf97-4aad-b711-d5c9a9f4586e") ] + + +@pytest.fixture +def url_consolidate_review(data_queue, data_standard_case): + return reverse( + "cases:consolidate_review", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]} + ) + + +@patch("caseworker.advice.views.mixins.get_gov_user") +def test_refusal_note_post( + mock_get_gov_user, requests_mock, authorized_client, data_standard_case, url_consolidate_review +): + mock_get_gov_user.return_value = ( + {"user": {"team": {"id": "21313212-23123-3123-323wq2", "alias": LICENSING_UNIT_TEAM}}}, + None, + ) + + requests_mock.post(client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}/final-advice/"), json={}) + + response = authorized_client.post( + url_consolidate_review + "refuse/", data={"denial_reasons": ["1a"], "refusal_note": "test"} + ) + assert response.status_code == 302 + request = requests_mock.request_history.pop() + assert "final-advice" in request.url + assert request.json() == [ + { + "type": "refuse", + "text": "test", + "footnote_required": False, + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "denial_reasons": ["1a"], + "is_refusal_note": True, + }, + { + "type": "refuse", + "text": "test", + "footnote_required": False, + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", + "denial_reasons": ["1a"], + "is_refusal_note": True, + }, + { + "type": "refuse", + "text": "test", + "footnote_required": False, + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + "denial_reasons": ["1a"], + "is_refusal_note": True, + }, + { + "type": "refuse", + "text": "test", + "footnote_required": False, + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "denial_reasons": ["1a"], + "is_refusal_note": True, + }, + { + "type": "no_licence_required", + "text": "", + "proviso": "", + "note": "", + "footnote_required": False, + "footnote": "", + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "denial_reasons": [], + }, + { + "type": "no_licence_required", + "text": "", + "proviso": "", + "note": "", + "footnote_required": False, + "footnote": "", + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "denial_reasons": [], + }, + ] From 589a41d113ab780313a10a9dd0b92389047ddd8d Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 5 Dec 2024 11:29:53 +0000 Subject: [PATCH 02/16] removed unused code --- caseworker/advice/conditionals.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/caseworker/advice/conditionals.py b/caseworker/advice/conditionals.py index f5a8a3630..dc86ebbab 100644 --- a/caseworker/advice/conditionals.py +++ b/caseworker/advice/conditionals.py @@ -13,11 +13,3 @@ def _get_form_field_boolean(wizard): def is_desnz_team(wizard): return wizard.caseworker["team"]["alias"] in services.DESNZ_TEAMS - - -def is_fcdo_team(wizard): - return wizard.caseworker["team"]["alias"] == services.FCDO_TEAM - - -def default_form(wizard): - return not (is_fcdo_team(wizard) or is_desnz_team(wizard)) From 50bba37088788d38c602c11b7eb88637b37e5895 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 5 Dec 2024 14:34:28 +0000 Subject: [PATCH 03/16] initial commit --- caseworker/advice/conditionals.py | 8 +++- caseworker/advice/forms/approval.py | 61 ++++++++++++++++++++++++++- caseworker/advice/picklist_helpers.py | 8 ++++ caseworker/advice/urls.py | 3 +- caseworker/advice/views/add_advice.py | 36 +++++++++------- 5 files changed, 97 insertions(+), 19 deletions(-) diff --git a/caseworker/advice/conditionals.py b/caseworker/advice/conditionals.py index dc86ebbab..411fba16d 100644 --- a/caseworker/advice/conditionals.py +++ b/caseworker/advice/conditionals.py @@ -11,5 +11,9 @@ def _get_form_field_boolean(wizard): return _get_form_field_boolean -def is_desnz_team(wizard): - return wizard.caseworker["team"]["alias"] in services.DESNZ_TEAMS +def is_fcdo_team(wizard): + return wizard.caseworker["team"]["alias"] == services.FCDO_TEAM + + +def is_ogd_team(wizard): + return not is_fcdo_team(wizard) diff --git a/caseworker/advice/forms/approval.py b/caseworker/advice/forms/approval.py index 27e18e22e..c7fcb432b 100644 --- a/caseworker/advice/forms/approval.py +++ b/caseworker/advice/forms/approval.py @@ -10,6 +10,7 @@ ConditionalCheckboxesQuestion, RadioTextArea, ) +from core.forms.widgets import GridmultipleSelect class SelectAdviceForm(forms.Form): @@ -71,7 +72,65 @@ def get_layout_fields(self): ) -class PicklistLicenceConditionsForm(PicklistAdviceForm, BaseForm): +class FCDOApprovalAdviceForm(RecommendAnApprovalForm): + class Layout: + TITLE = "Recommend an approval" + + def __init__(self, countries, *args, **kwargs): + countries = kwargs.pop("countries") + super().__init__(*args, **kwargs) + self.fields["countries"] = forms.MultipleChoiceField( + choices=countries.items(), + widget=GridmultipleSelect(), + label="Select countries for which you want to give advice", + error_messages={"required": "Select the destinations you want to make recommendations for"}, + ) + + def get_layout_fields(self): + return ( + RadioTextArea("approval_radios", "approval_reasons", self.approval_text), + "countries", + "add_licence_conditions", + ) + + +class DESNZApprovalForm(PicklistAdviceForm, BaseForm): + 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_licence_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_licence_conditions", + ) + + +class LicenceConditionsForm(PicklistAdviceForm, BaseForm): class Layout: TITLE = "Add licence conditions (optional)" diff --git a/caseworker/advice/picklist_helpers.py b/caseworker/advice/picklist_helpers.py index 812ad0336..4ba00d8a8 100644 --- a/caseworker/advice/picklist_helpers.py +++ b/caseworker/advice/picklist_helpers.py @@ -9,6 +9,14 @@ def approval_picklist(self): } +def fcdo_picklist(self): + return { + "approval_reason": get_picklists_list( + self.request, type="standard_advice", disable_pagination=True, show_deactivated=False + ) + } + + def proviso_picklist(self): return { "proviso": get_picklists_list(self.request, type="proviso", disable_pagination=True, show_deactivated=False) diff --git a/caseworker/advice/urls.py b/caseworker/advice/urls.py index b104d7a08..4bfbcf717 100644 --- a/caseworker/advice/urls.py +++ b/caseworker/advice/urls.py @@ -1,13 +1,14 @@ from django.urls import path from caseworker.advice.views import consolidate, views +from caseworker.advice.views.add_advice import SelectAdviceView from caseworker.advice.views.approval import GiveApprovalAdviceView from caseworker.advice.views.edit import EditAdviceView urlpatterns = [ 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("select-advice/", SelectAdviceView.as_view(), name="select_advice"), path("approve-all-legacy/", views.GiveApprovalAdviceViewLegacy.as_view(), name="approve_all_legacy"), path("approve-all/", GiveApprovalAdviceView.as_view(), name="approve_all"), path("refuse-all/", views.RefusalAdviceView.as_view(), name="refuse_all"), diff --git a/caseworker/advice/views/add_advice.py b/caseworker/advice/views/add_advice.py index fe2167214..9cb1ef705 100644 --- a/caseworker/advice/views/add_advice.py +++ b/caseworker/advice/views/add_advice.py @@ -1,12 +1,14 @@ from http import HTTPStatus -from caseworker.advice.conditionals import form_add_licence_conditions, is_desnz_team +from caseworker.advice.conditionals import form_add_licence_conditions, is_fcdo_team, is_ogd_team from caseworker.advice.forms.approval import ( + FCDOApprovalAdviceForm, FootnotesApprovalAdviceForm, LicenceConditionsForm, RecommendAnApprovalForm, + SelectAdviceForm, ) from caseworker.advice.payloads import GiveApprovalAdvicePayloadBuilder -from caseworker.advice.picklist_helpers import approval_picklist, footnote_picklist, proviso_picklist +from caseworker.advice.picklist_helpers import approval_picklist, fcdo_picklist, footnote_picklist, proviso_picklist from core.wizard.views import BaseSessionWizardView from core.wizard.conditionals import C from django.shortcuts import redirect @@ -17,40 +19,44 @@ from caseworker.advice.constants import AdviceSteps from core.auth.views import LoginRequiredMixin from core.decorators import expect_status +from lite_forms.views import FormView -# class SelectAdviceView(LoginRequiredMixin, CaseContextMixin, FormView): -# template_name = "advice/select_advice.html" -# form_class = SelectAdviceForm +class SelectAdviceView(LoginRequiredMixin, CaseContextMixin, FormView): + template_name = "advice/select_advice.html" + form_class = SelectAdviceForm -# def get_success_url(self): -# recommendation = self.request.POST.get("recommendation") -# if recommendation == "approve_all": -# return reverse("cases:approve_all", kwargs=self.kwargs) -# else: -# return reverse("cases:refuse_all", kwargs=self.kwargs) + def get_success_url(self): + recommendation = self.request.POST.get("recommendation") + if recommendation == "approve_all": + return reverse("cases:approve_all", kwargs=self.kwargs) + else: + return reverse("cases:refuse_all", kwargs=self.kwargs) -# def get_context_data(self, **kwargs): -# context = super().get_context_data(**kwargs) -# return {**context, "security_approvals_classified_display": self.security_approvals_classified_display} + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + return {**context, "security_approvals_classified_display": self.security_approvals_classified_display} class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView): form_list = [ (AdviceSteps.RECOMMEND_APPROVAL, RecommendAnApprovalForm), + (AdviceSteps.FCDO_APPROVAL, FCDOApprovalAdviceForm), (AdviceSteps.LICENCE_CONDITIONS, LicenceConditionsForm), (AdviceSteps.LICENCE_FOOTNOTES, FootnotesApprovalAdviceForm), ] condition_dict = { - AdviceSteps.RECOMMEND_APPROVAL: C(is_desnz_team), + AdviceSteps.RECOMMEND_APPROVAL: C(is_ogd_team), + AdviceSteps.FCDO_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)), } step_kwargs = { AdviceSteps.RECOMMEND_APPROVAL: approval_picklist, + AdviceSteps.FCDO_APPROVAL: fcdo_picklist, AdviceSteps.LICENCE_CONDITIONS: proviso_picklist, AdviceSteps.LICENCE_FOOTNOTES: footnote_picklist, } From d22c54369fa9ed0899031d3779a6a39b284dabc9 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 5 Dec 2024 16:49:42 +0000 Subject: [PATCH 04/16] all non fcdo ogds using multi_step --- caseworker/advice/forms/approval.py | 59 ----- caseworker/advice/picklist_helpers.py | 8 - caseworker/advice/views/add_advice.py | 14 +- caseworker/advice/views/views.py | 37 +--- .../advice/views/test_edit_advice.py | 207 ++++++++++++------ .../views/test_give_approval_advice_view.py | 116 ++++++++-- .../advice/views/test_select_advice_view.py | 4 +- 7 files changed, 261 insertions(+), 184 deletions(-) diff --git a/caseworker/advice/forms/approval.py b/caseworker/advice/forms/approval.py index c7fcb432b..127518513 100644 --- a/caseworker/advice/forms/approval.py +++ b/caseworker/advice/forms/approval.py @@ -10,7 +10,6 @@ ConditionalCheckboxesQuestion, RadioTextArea, ) -from core.forms.widgets import GridmultipleSelect class SelectAdviceForm(forms.Form): @@ -72,64 +71,6 @@ def get_layout_fields(self): ) -class FCDOApprovalAdviceForm(RecommendAnApprovalForm): - class Layout: - TITLE = "Recommend an approval" - - def __init__(self, countries, *args, **kwargs): - countries = kwargs.pop("countries") - super().__init__(*args, **kwargs) - self.fields["countries"] = forms.MultipleChoiceField( - choices=countries.items(), - widget=GridmultipleSelect(), - label="Select countries for which you want to give advice", - error_messages={"required": "Select the destinations you want to make recommendations for"}, - ) - - def get_layout_fields(self): - return ( - RadioTextArea("approval_radios", "approval_reasons", self.approval_text), - "countries", - "add_licence_conditions", - ) - - -class DESNZApprovalForm(PicklistAdviceForm, BaseForm): - 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_licence_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_licence_conditions", - ) - - class LicenceConditionsForm(PicklistAdviceForm, BaseForm): class Layout: TITLE = "Add licence conditions (optional)" diff --git a/caseworker/advice/picklist_helpers.py b/caseworker/advice/picklist_helpers.py index 4ba00d8a8..812ad0336 100644 --- a/caseworker/advice/picklist_helpers.py +++ b/caseworker/advice/picklist_helpers.py @@ -9,14 +9,6 @@ def approval_picklist(self): } -def fcdo_picklist(self): - return { - "approval_reason": get_picklists_list( - self.request, type="standard_advice", disable_pagination=True, show_deactivated=False - ) - } - - def proviso_picklist(self): return { "proviso": get_picklists_list(self.request, type="proviso", disable_pagination=True, show_deactivated=False) diff --git a/caseworker/advice/views/add_advice.py b/caseworker/advice/views/add_advice.py index 9cb1ef705..d13b61ba7 100644 --- a/caseworker/advice/views/add_advice.py +++ b/caseworker/advice/views/add_advice.py @@ -1,25 +1,26 @@ from http import HTTPStatus -from caseworker.advice.conditionals import form_add_licence_conditions, is_fcdo_team, is_ogd_team +from caseworker.advice.conditionals import form_add_licence_conditions, is_ogd_team from caseworker.advice.forms.approval import ( - FCDOApprovalAdviceForm, FootnotesApprovalAdviceForm, LicenceConditionsForm, RecommendAnApprovalForm, SelectAdviceForm, ) from caseworker.advice.payloads import GiveApprovalAdvicePayloadBuilder -from caseworker.advice.picklist_helpers import approval_picklist, fcdo_picklist, footnote_picklist, proviso_picklist +from caseworker.advice.picklist_helpers import approval_picklist, footnote_picklist, proviso_picklist from core.wizard.views import BaseSessionWizardView from core.wizard.conditionals import C from django.shortcuts import redirect from django.urls import reverse -from caseworker.advice.views.mixins import CaseContextMixin from caseworker.advice import services from caseworker.advice.constants import AdviceSteps from core.auth.views import LoginRequiredMixin from core.decorators import expect_status -from lite_forms.views import FormView +from http import HTTPStatus +from caseworker.advice.forms.approval import SelectAdviceForm +from caseworker.advice.views.mixins import CaseContextMixin +from django.views.generic import FormView class SelectAdviceView(LoginRequiredMixin, CaseContextMixin, FormView): @@ -42,21 +43,18 @@ class GiveApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWi form_list = [ (AdviceSteps.RECOMMEND_APPROVAL, RecommendAnApprovalForm), - (AdviceSteps.FCDO_APPROVAL, FCDOApprovalAdviceForm), (AdviceSteps.LICENCE_CONDITIONS, LicenceConditionsForm), (AdviceSteps.LICENCE_FOOTNOTES, FootnotesApprovalAdviceForm), ] condition_dict = { AdviceSteps.RECOMMEND_APPROVAL: C(is_ogd_team), - AdviceSteps.FCDO_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)), } step_kwargs = { AdviceSteps.RECOMMEND_APPROVAL: approval_picklist, - AdviceSteps.FCDO_APPROVAL: fcdo_picklist, AdviceSteps.LICENCE_CONDITIONS: proviso_picklist, AdviceSteps.LICENCE_FOOTNOTES: footnote_picklist, } diff --git a/caseworker/advice/views/views.py b/caseworker/advice/views/views.py index 67a258e73..ef96d0a63 100644 --- a/caseworker/advice/views/views.py +++ b/caseworker/advice/views/views.py @@ -1,5 +1,5 @@ from http import HTTPStatus -from caseworker.advice.forms.approval import MoveCaseForwardForm, SelectAdviceForm +from caseworker.advice.forms.approval import MoveCaseForwardForm from caseworker.advice.forms.consolidate import ( ConsolidateApprovalForm, LUConsolidateRefusalForm, @@ -8,7 +8,6 @@ from caseworker.advice.forms.delete import DeleteAdviceForm from caseworker.advice.forms.forms import ( FCDOApprovalAdviceForm, - GiveApprovalAdviceForm, get_approval_advice_form_factory, get_formset, ) @@ -50,24 +49,6 @@ class CaseDetailView(LoginRequiredMixin, CaseContextMixin, TemplateView): template_name = "advice/case_detail_example.html" -class SelectAdviceView(LoginRequiredMixin, CaseContextMixin, FormView): - template_name = "advice/select_advice.html" - form_class = SelectAdviceForm - - 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", kwargs=self.kwargs) - return reverse("cases:approve_all_legacy", kwargs=self.kwargs) - else: - return reverse("cases:refuse_all", kwargs=self.kwargs) - - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - return {**context, "security_approvals_classified_display": self.security_approvals_classified_display} - - class GiveApprovalAdviceViewLegacy(LoginRequiredMixin, CaseContextMixin, FormView): """ Form to recommend approval advice for all products on the application @@ -76,13 +57,10 @@ class GiveApprovalAdviceViewLegacy(LoginRequiredMixin, CaseContextMixin, FormVie template_name = "advice/give-approval-advice.html" def get_form(self): - if self.caseworker["team"]["alias"] == services.FCDO_TEAM: - return FCDOApprovalAdviceForm( - services.unadvised_countries(self.caseworker, self.case), - **self.get_form_kwargs(), - ) - else: - return GiveApprovalAdviceForm(**self.get_form_kwargs()) + return FCDOApprovalAdviceForm( + services.unadvised_countries(self.caseworker, self.case), + **self.get_form_kwargs(), + ) def get_form_kwargs(self): kwargs = super().get_form_kwargs() @@ -245,9 +223,8 @@ def form_valid(self, form): # & therefore, we render the normal forms and not the FCO ones. # This means that here data here doesn't include the list of countries for which # the advice should be applied and so we pop that in using a method. - if self.caseworker["team"]["alias"] == services.FCDO_TEAM: - data["countries"] = self.advised_countries() - if isinstance(form, GiveApprovalAdviceForm): + data["countries"] = self.advised_countries() + if isinstance(form, FCDOApprovalAdviceForm): services.post_approval_advice(self.request, self.case, data) elif isinstance(form, RefusalAdviceForm): data["text"] = data["refusal_reasons"] diff --git a/unit_tests/caseworker/advice/views/test_edit_advice.py b/unit_tests/caseworker/advice/views/test_edit_advice.py index 7dc8fc90c..8cc384fc5 100644 --- a/unit_tests/caseworker/advice/views/test_edit_advice.py +++ b/unit_tests/caseworker/advice/views/test_edit_advice.py @@ -20,9 +20,20 @@ def url(data_queue, data_standard_case): ) -def test_edit_approve_advice_post(authorized_client, requests_mock, data_standard_case, standard_case_with_advice, url): +@pytest.fixture +def url_approve(data_queue, data_standard_case): + return reverse(f"cases:edit_advice", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}) + + +@pytest.fixture +def post_to_step(post_to_step_factory, url_approve): + return post_to_step_factory(url_approve) + + +def test_edit_approve_advice_post( + authorized_client, requests_mock, mock_post_advice, data_standard_case, standard_case_with_advice, post_to_step, url +): 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"] @@ -33,11 +44,11 @@ def test_edit_approve_advice_post(authorized_client, requests_mock, data_standar json={"user": {"id": "58e62718-e889-4a01-b603-e676b794b394"}}, ) - data = { - "approval_reasons": "meets the requirements updated", - "instructions_to_exporter": "no specific instructions", - } - response = authorized_client.post(url, data=data) + data = {"approval_reasons": "meets the requirements updated", "add_licence_conditions": False} + response = post_to_step( + AdviceSteps.RECOMMEND_APPROVAL, + data, + ) 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 @@ -49,7 +60,7 @@ def test_edit_approve_advice_post(authorized_client, requests_mock, data_standar "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", "footnote": "", "footnote_required": False, - "note": "no specific instructions", + "note": "", "proviso": "", "text": "meets the requirements updated", "type": "approve", @@ -59,7 +70,7 @@ def test_edit_approve_advice_post(authorized_client, requests_mock, data_standar "denial_reasons": [], "footnote": "", "footnote_required": False, - "note": "no specific instructions", + "note": "", "proviso": "", "text": "meets the requirements updated", "type": "approve", @@ -69,7 +80,7 @@ def test_edit_approve_advice_post(authorized_client, requests_mock, data_standar "denial_reasons": [], "footnote": "", "footnote_required": False, - "note": "no specific instructions", + "note": "", "proviso": "", "text": "meets the requirements updated", "type": "approve", @@ -78,7 +89,7 @@ def test_edit_approve_advice_post(authorized_client, requests_mock, data_standar "denial_reasons": [], "footnote": "", "footnote_required": False, - "note": "no specific instructions", + "note": "", "proviso": "", "text": "meets the requirements updated", "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", @@ -89,7 +100,7 @@ def test_edit_approve_advice_post(authorized_client, requests_mock, data_standar "footnote": "", "footnote_required": False, "good": "9fbffa7f-ef50-402e-93ac-2f3f37d09030", - "note": "no specific instructions", + "note": "", "proviso": "", "text": "meets the requirements updated", "type": "approve", @@ -107,6 +118,123 @@ def test_edit_approve_advice_post(authorized_client, requests_mock, data_standar ] +def test_give_approval_advice_post_valid_add_conditional( + authorized_client, + requests_mock, + data_standard_case, + url, + mock_approval_reason, + mock_proviso, + mock_footnote_details, + mock_post_advice, + 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"}}, + ) + + response = post_to_step( + AdviceSteps.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_licence_condition_response = post_to_step( + AdviceSteps.LICENCE_CONDITIONS, + {"proviso": "proviso updated"}, + ) + 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( + AdviceSteps.LICENCE_FOOTNOTES, + {"instructions_to_exporter": "instructions updated", "footnote_details": "footnotes updated"}, + ) + assert add_instructions_response.status_code == 302 + history = mock_post_advice.request_history + assert len(history) == 1 + history = history[0] + assert history.method == "POST" + 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": [], + }, + ] + + def test_edit_refuse_advice_post( authorized_client, requests_mock, @@ -138,55 +266,23 @@ def test_edit_refuse_advice_post( assert user_advice_create_url in history.url assert history.method == "POST" assert history.json() == [ - { - "denial_reasons": ["3", "4", "5", "5a", "5b"], - "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", - "footnote_required": False, - "text": "doesn't meet the requirement", - "type": "refuse", - "is_refusal_note": False, - }, - { - "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", - "denial_reasons": ["3", "4", "5", "5a", "5b"], - "footnote_required": False, - "text": "doesn't meet the requirement", - "type": "refuse", - "is_refusal_note": False, - }, { "type": "refuse", "text": "doesn't meet the requirement", "footnote_required": False, - "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", - "denial_reasons": ["3", "4", "5", "5a", "5b"], - "is_refusal_note": False, - }, - { + "good": "9fbffa7f-ef50-402e-93ac-2f3f37d09030", "denial_reasons": ["3", "4", "5", "5a", "5b"], - "footnote_required": False, - "text": "doesn't meet the requirement", - "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", - "type": "refuse", "is_refusal_note": False, }, { - "denial_reasons": ["3", "4", "5", "5a", "5b"], + "type": "no_licence_required", + "text": "", + "proviso": "", + "note": "", "footnote_required": False, - "good": "9fbffa7f-ef50-402e-93ac-2f3f37d09030", - "text": "doesn't meet the requirement", - "type": "refuse", - "is_refusal_note": False, - }, - { - "denial_reasons": [], "footnote": "", - "footnote_required": False, "good": "d4feac1e-851d-41a5-b833-eb28addb8547", - "note": "", - "proviso": "", - "text": "", - "type": "no_licence_required", + "denial_reasons": [], }, ] @@ -209,16 +305,6 @@ def test_edit_refuse_advice_get( 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", 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) - - def test_DESNZ_give_approval_advice_post_valid( authorized_client, requests_mock, @@ -230,7 +316,6 @@ def test_DESNZ_give_approval_advice_post_valid( mock_post_advice, standard_case_with_advice, post_to_step, - beautiful_soup, mocker, ): get_gov_user_value = ( 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 5a2a4504b..4c73d87c6 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 @@ -21,19 +21,115 @@ def url(data_queue, data_standard_case): ) +@pytest.fixture +def url_approve(data_queue, data_standard_case): + return reverse("cases:approve_all", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}) + + +@pytest.fixture +def post_to_step(post_to_step_factory, url_approve): + return post_to_step_factory(url_approve) + + def test_give_approval_advice_get(authorized_client, url): response = authorized_client.get(url) assert response.status_code == 200 -def test_select_advice_post(authorized_client, requests_mock, data_standard_case, url): - requests_mock.post(f"/cases/{data_standard_case['case']['id']}/user-advice/", json={}) - - data = {"approval_reasons": "meets the requirements", "instructions_to_exporter": "no specific instructions"} - response = authorized_client.post(url, data=data) +def test_approval_advice_post_valid( + authorized_client, + data_standard_case, + url, + mock_approval_reason, + mock_proviso, + mock_footnote_details, + mock_post_advice, + post_to_step, + beautiful_soup, +): + response = post_to_step( + AdviceSteps.RECOMMEND_APPROVAL, + {"approval_reasons": "Data"}, + ) assert response.status_code == 302 +def test_approval_advice_post_valid_add_conditional( + authorized_client, + data_standard_case, + url, + mock_approval_reason, + mock_proviso, + mock_footnote_details, + mock_post_advice, + post_to_step, + beautiful_soup, +): + response = post_to_step( + AdviceSteps.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( + AdviceSteps.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( + AdviceSteps.LICENCE_FOOTNOTES, + {"instructions_to_exporter": "instructions", "footnote_details": "footnotes"}, + ) + assert add_instructions_response.status_code == 302 + + +def test_approval_advice_post_valid_add_conditional_optional( + authorized_client, + data_standard_case, + url, + mock_approval_reason, + mock_proviso, + mock_footnote_details, + mock_post_advice, + post_to_step, + beautiful_soup, +): + response = post_to_step( + AdviceSteps.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( + AdviceSteps.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( + AdviceSteps.LICENCE_FOOTNOTES, + {}, + ) + assert add_instructions_response.status_code == 302 + + @mock.patch("caseworker.advice.views.mixins.get_gov_user") def test_fco_give_approval_advice_get(mock_get_gov_user, authorized_client, url): mock_get_gov_user.return_value = ( @@ -118,16 +214,6 @@ def test_fcdo_give_approval_advice_post( assert response.status_code == expected_status_code -@pytest.fixture -def url_desnz(data_queue, data_standard_case): - return reverse("cases:approve_all", 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.mixins.get_gov_user") def test_DESNZ_give_approval_advice_post_valid( mock_get_gov_user, 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 58155c632..0420722a3 100644 --- a/unit_tests/caseworker/advice/views/test_select_advice_view.py +++ b/unit_tests/caseworker/advice/views/test_select_advice_view.py @@ -21,9 +21,7 @@ def test_select_advice_get(authorized_client, url): assert response.status_code == 200 -@pytest.mark.parametrize( - "recommendation, redirect", [("approve_all", "approve-all-legacy"), ("refuse_all", "refuse-all")] -) +@pytest.mark.parametrize("recommendation, redirect", [("approve_all", "approve-all"), ("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 From 2fac10831193475b2172dd0bb2e7655c958c3ab6 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Mon, 9 Dec 2024 10:43:52 +0000 Subject: [PATCH 05/16] changes to rules --- caseworker/advice/rules.py | 12 +++---- .../templates/advice/view_my_advice.html | 4 +-- unit_tests/caseworker/advice/test_rules.py | 32 +++++++++---------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/caseworker/advice/rules.py b/caseworker/advice/rules.py index a0b34f3e7..7bdad37e3 100644 --- a/caseworker/advice/rules.py +++ b/caseworker/advice/rules.py @@ -33,8 +33,8 @@ def can_desnz_make_recommendation(user, case, queue_alias): return True -def can_desnz_make_edit(team): - return team in services.DESNZ_TEAMS +def can_ogd_make_edit(team): + return team not in services.FCDO_TEAM def case_has_approval_advice(advice): @@ -44,15 +44,15 @@ def case_has_approval_advice(advice): @rules.predicate -def can_user_make_desnz_edit(request, case): +def can_user_make_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) + advice = services.filter_urrent_user_advice(case.advice, user["id"]) + return can_ogd_make_edit(team) and case_has_approval_advice(advice) @rules.predicate @@ -84,4 +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) +rules.add_rule("can_user_make_edit", can_user_make_edit) diff --git a/caseworker/advice/templates/advice/view_my_advice.html b/caseworker/advice/templates/advice/view_my_advice.html index 8595c9af9..6773dd3a8 100644 --- a/caseworker/advice/templates/advice/view_my_advice.html +++ b/caseworker/advice/templates/advice/view_my_advice.html @@ -34,10 +34,10 @@

View recommendation

{% if my_advice %} - {% test_rule 'can_user_make_desnz_edit' request case as can_user_make_desnz_edit %} + {% test_rule 'can_user_make_edit' request case as can_user_make_edit %} {% if buttons.edit_recommendation %} - {% if can_user_make_desnz_edit %} + {% if can_user_make_edit %} Edit recommendation {% else %} Edit recommendation diff --git a/unit_tests/caseworker/advice/test_rules.py b/unit_tests/caseworker/advice/test_rules.py index 87922f445..417489d76 100644 --- a/unit_tests/caseworker/advice/test_rules.py +++ b/unit_tests/caseworker/advice/test_rules.py @@ -189,7 +189,7 @@ def test_can_user_allocate_and_approve(mock_gov_user, data_fake_queue, data_stan assert rules.test_rule("can_user_allocate_and_approve", request, case) -def test_can_user_make_desnz_edit_valid( +def test_can_user_make_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] @@ -198,44 +198,44 @@ def test_can_user_make_desnz_edit_valid( 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) + assert rules.test_rule("can_user_make_edit", request, data_assigned_case) -def test_can_user_make_desnz_edit_invalid_advice(mock_gov_user, data_fake_queue, data_assigned_case): +def test_can_user_make_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) + assert not rules.test_rule("can_user_make_edit", request, data_assigned_case) -def test_can_user_make_desnz_edit_invalid_user( +def test_can_user_make_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 + mock_gov_user["user"]["team"]["alias"] = services.FCDO_TEAM + data_fake_queue["alias"] = services.FCDO_CASES_TO_REVIEW_QUEUE 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) + assert not rules.test_rule("can_user_make_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 +def test_can_user_make_edit_invalid_advice_and_user(mock_gov_user, data_fake_queue, data_assigned_case): + mock_gov_user["user"]["team"]["alias"] = services.FCDO_TEAM + data_fake_queue["alias"] = services.FCDO_CASES_TO_REVIEW_QUEUE 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) + assert not rules.test_rule("can_user_make_edit", request, data_assigned_case) -def test_can_user_make_desnz_edit_request_missing_attributes(mock_gov_user, data_fake_queue, data_standard_case): +def test_can_user_make_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) + assert not advice_rules.can_user_make_edit(request, case) -def test_can_user_make_desnz_edit_user_not_allocated(mock_gov_user, data_fake_queue, data_standard_case): +def test_can_user_make_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) + assert not rules.test_rule("can_user_make_edit", request, case) From 209ffadde57e46537931182191527f9ab085a530 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Mon, 9 Dec 2024 15:41:42 +0000 Subject: [PATCH 06/16] resolve conflicts --- caseworker/advice/forms/forms.py | 2 +- caseworker/advice/rules.py | 4 +- caseworker/advice/urls.py | 3 +- caseworker/advice/views/add_advice.py | 84 -------- caseworker/advice/views/approval.py | 23 ++- caseworker/advice/views/edit_advice.py | 42 ---- .../advice/views/test_consolidate.py | 180 ------------------ .../advice/views/test_edit_advice.py | 3 +- .../advice/views/test_lu_consolidate.py | 80 -------- 9 files changed, 25 insertions(+), 396 deletions(-) delete mode 100644 caseworker/advice/views/add_advice.py delete mode 100644 caseworker/advice/views/edit_advice.py diff --git a/caseworker/advice/forms/forms.py b/caseworker/advice/forms/forms.py index d7cc103ee..505bba217 100644 --- a/caseworker/advice/forms/forms.py +++ b/caseworker/advice/forms/forms.py @@ -23,7 +23,7 @@ def get_approval_advice_form_factory(advice, approval_reason, proviso, footnote_ "instructions_to_exporter": advice["note"], "footnote_details": advice["footnote"], } - return GiveApprovalAdviceForm( + return FCDOApprovalAdviceForm( approval_reason=approval_reason, proviso=proviso, footnote_details=footnote_details, data=data ) diff --git a/caseworker/advice/rules.py b/caseworker/advice/rules.py index 7bdad37e3..091f5b82b 100644 --- a/caseworker/advice/rules.py +++ b/caseworker/advice/rules.py @@ -34,7 +34,7 @@ def can_desnz_make_recommendation(user, case, queue_alias): def can_ogd_make_edit(team): - return team not in services.FCDO_TEAM + return not team == services.FCDO_TEAM def case_has_approval_advice(advice): @@ -51,7 +51,7 @@ def can_user_make_edit(request, case): return False team = user["team"]["alias"] - advice = services.filter_urrent_user_advice(case.advice, user["id"]) + advice = services.filter_current_user_advice(case.advice, user["id"]) return can_ogd_make_edit(team) and case_has_approval_advice(advice) diff --git a/caseworker/advice/urls.py b/caseworker/advice/urls.py index 4bfbcf717..08ebf0bec 100644 --- a/caseworker/advice/urls.py +++ b/caseworker/advice/urls.py @@ -1,8 +1,7 @@ from django.urls import path from caseworker.advice.views import consolidate, views -from caseworker.advice.views.add_advice import SelectAdviceView -from caseworker.advice.views.approval import GiveApprovalAdviceView +from caseworker.advice.views.approval import GiveApprovalAdviceView, SelectAdviceView from caseworker.advice.views.edit import EditAdviceView urlpatterns = [ diff --git a/caseworker/advice/views/add_advice.py b/caseworker/advice/views/add_advice.py deleted file mode 100644 index d13b61ba7..000000000 --- a/caseworker/advice/views/add_advice.py +++ /dev/null @@ -1,84 +0,0 @@ -from http import HTTPStatus -from caseworker.advice.conditionals import form_add_licence_conditions, is_ogd_team -from caseworker.advice.forms.approval import ( - FootnotesApprovalAdviceForm, - LicenceConditionsForm, - RecommendAnApprovalForm, - SelectAdviceForm, -) -from caseworker.advice.payloads import GiveApprovalAdvicePayloadBuilder -from caseworker.advice.picklist_helpers import approval_picklist, footnote_picklist, proviso_picklist -from core.wizard.views import BaseSessionWizardView -from core.wizard.conditionals import C -from django.shortcuts import redirect -from django.urls import reverse -from caseworker.advice import services - -from caseworker.advice.constants import AdviceSteps -from core.auth.views import LoginRequiredMixin -from core.decorators import expect_status -from http import HTTPStatus -from caseworker.advice.forms.approval import SelectAdviceForm -from caseworker.advice.views.mixins import CaseContextMixin -from django.views.generic import FormView - - -class SelectAdviceView(LoginRequiredMixin, CaseContextMixin, FormView): - template_name = "advice/select_advice.html" - form_class = SelectAdviceForm - - def get_success_url(self): - recommendation = self.request.POST.get("recommendation") - if recommendation == "approve_all": - return reverse("cases:approve_all", kwargs=self.kwargs) - else: - return reverse("cases:refuse_all", kwargs=self.kwargs) - - def get_context_data(self, **kwargs): - context = super().get_context_data(**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), - ] - - condition_dict = { - AdviceSteps.RECOMMEND_APPROVAL: C(is_ogd_team), - AdviceSteps.LICENCE_CONDITIONS: C(form_add_licence_conditions(AdviceSteps.RECOMMEND_APPROVAL)), - AdviceSteps.LICENCE_FOOTNOTES: C(form_add_licence_conditions(AdviceSteps.RECOMMEND_APPROVAL)), - } - - step_kwargs = { - AdviceSteps.RECOMMEND_APPROVAL: approval_picklist, - AdviceSteps.LICENCE_CONDITIONS: proviso_picklist, - AdviceSteps.LICENCE_FOOTNOTES: footnote_picklist, - } - - 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", - "Unexpected error adding approval advice", - ) - 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 = self.get_payload(form_dict) - self.post_approval_advice(data) - return redirect(self.get_success_url()) diff --git a/caseworker/advice/views/approval.py b/caseworker/advice/views/approval.py index 4565fb8cc..e4fa5cd64 100644 --- a/caseworker/advice/views/approval.py +++ b/caseworker/advice/views/approval.py @@ -1,10 +1,11 @@ from http import HTTPStatus -from caseworker.advice.conditionals import form_add_licence_conditions, is_desnz_team +from caseworker.advice.conditionals import form_add_licence_conditions, is_ogd_team from caseworker.advice.forms.approval import ( FootnotesApprovalAdviceForm, PicklistLicenceConditionsForm, SimpleLicenceConditionsForm, RecommendAnApprovalForm, + SelectAdviceForm, ) from caseworker.advice.payloads import GiveApprovalAdvicePayloadBuilder from caseworker.advice.picklist_helpers import approval_picklist, footnote_picklist, proviso_picklist @@ -14,16 +15,32 @@ from django.urls import reverse from caseworker.advice.views.mixins import CaseContextMixin from caseworker.advice import services - from caseworker.advice.constants import AdviceSteps from core.auth.views import LoginRequiredMixin from core.decorators import expect_status +from django.views.generic import FormView + + +class SelectAdviceView(LoginRequiredMixin, CaseContextMixin, FormView): + template_name = "advice/select_advice.html" + form_class = SelectAdviceForm + + def get_success_url(self): + recommendation = self.request.POST.get("recommendation") + if recommendation == "approve_all": + return reverse("cases:approve_all", kwargs=self.kwargs) + else: + return reverse("cases:refuse_all", kwargs=self.kwargs) + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + return {**context, "security_approvals_classified_display": self.security_approvals_classified_display} class BaseApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView): condition_dict = { - AdviceSteps.RECOMMEND_APPROVAL: C(is_desnz_team), + AdviceSteps.RECOMMEND_APPROVAL: C(is_ogd_team), AdviceSteps.LICENCE_CONDITIONS: C(form_add_licence_conditions(AdviceSteps.RECOMMEND_APPROVAL)), AdviceSteps.LICENCE_FOOTNOTES: C(form_add_licence_conditions(AdviceSteps.RECOMMEND_APPROVAL)), } diff --git a/caseworker/advice/views/edit_advice.py b/caseworker/advice/views/edit_advice.py deleted file mode 100644 index 750ef1575..000000000 --- a/caseworker/advice/views/edit_advice.py +++ /dev/null @@ -1,42 +0,0 @@ -from caseworker.advice.forms.approval import FootnotesApprovalAdviceForm, RecommendAnApprovalForm -from caseworker.advice.forms.edit import PicklistApprovalAdviceEditForm -from caseworker.advice.views.add_advice import GiveApprovalAdviceView -from caseworker.advice import services -from caseworker.advice.constants import AdviceSteps -from caseworker.advice.picklist_helpers import approval_picklist, footnote_picklist - - -class EditAdviceView(GiveApprovalAdviceView): - - form_list = [ - (AdviceSteps.RECOMMEND_APPROVAL, RecommendAnApprovalForm), - (AdviceSteps.LICENCE_CONDITIONS, PicklistApprovalAdviceEditForm), - (AdviceSteps.LICENCE_FOOTNOTES, FootnotesApprovalAdviceForm), - ] - - step_kwargs = { - AdviceSteps.RECOMMEND_APPROVAL: approval_picklist, - AdviceSteps.LICENCE_CONDITIONS: None, - AdviceSteps.LICENCE_FOOTNOTES: footnote_picklist, - } - - 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] - - # 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", ""), - "approval_radios": "other", - "proviso": advice.get("proviso"), - "instructions_to_exporter": advice.get("note", ""), - "footnote_details": advice.get("footnote", ""), - "footnote_details_radios": "other", - } diff --git a/unit_tests/caseworker/advice/views/test_consolidate.py b/unit_tests/caseworker/advice/views/test_consolidate.py index aa22e15c1..bb9c2f798 100644 --- a/unit_tests/caseworker/advice/views/test_consolidate.py +++ b/unit_tests/caseworker/advice/views/test_consolidate.py @@ -6,7 +6,6 @@ from caseworker.advice.forms.consolidate import ( ConsolidateApprovalForm, ConsolidateSelectAdviceForm, - LUConsolidateRefusalForm, ) from caseworker.advice import services from caseworker.advice.forms.refusal import RefusalAdviceForm @@ -306,185 +305,6 @@ def gov_user(): } -@pytest.mark.parametrize( - "path, form_class, team_alias, team_name", - ( - ("", ConsolidateApprovalForm, LICENSING_UNIT_TEAM, "LU Team"), - ("", ConsolidateApprovalForm, MOD_ECJU_TEAM, "MOD Team"), - ("approve/", ConsolidateApprovalForm, LICENSING_UNIT_TEAM, "LU Team"), - ("refuse/", LUConsolidateRefusalForm, LICENSING_UNIT_TEAM, "LU Team"), - ("approve/", ConsolidateApprovalForm, MOD_ECJU_TEAM, "MOD Team"), - ("refuse/", RefusalAdviceForm, MOD_ECJU_TEAM, "MOD Team"), - ), -) -def test_consolidate_review( - requests_mock, - authorized_client, - data_standard_case, - url, - advice_to_consolidate, - gov_user, - path, - form_class, - team_alias, - team_name, -): - data_standard_case["case"]["advice"] = advice_to_consolidate - gov_user["user"]["team"]["name"] = team_name - gov_user["user"]["team"]["alias"] = team_alias - - requests_mock.get( - client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), - json=gov_user, - ) - - response = authorized_client.get(url + path) - assert response.status_code == 200 - form = response.context["form"] - assert isinstance(form, form_class) - - advice_to_review = list(response.context["advice_to_consolidate"]) - advice_teams = {item[0]["user"]["team"]["alias"] for item in advice_to_review} - - if team_alias == LICENSING_UNIT_TEAM: - assert advice_teams == {FCDO_TEAM, MOD_ECJU_TEAM} - elif team_alias == MOD_ECJU_TEAM: - assert bool(advice_teams.intersection(MOD_CONSOLIDATE_TEAMS)) == True - - -def test_approval_reasons_mocked( - requests_mock, - authorized_client, - data_standard_case, - url, - advice_to_consolidate, - gov_user, -): - data_standard_case["case"]["advice"] = advice_to_consolidate - gov_user["user"]["team"]["name"] = "LU Team" - gov_user["user"]["team"]["alias"] = LICENSING_UNIT_TEAM - - requests_mock.get( - client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), - json=gov_user, - ) - - response = authorized_client.get(url + "approve/") - assert response.status_code == 200 - form = response.context["form"] - assert isinstance(form, GiveApprovalAdviceForm) - # this is built mock_approval_reason - response_choices = [list(choice) for choice in form.fields["approval_radios"].choices] - assert response_choices == [ - ["no_concerns", "no concerns"], - ["concerns", "concerns"], - ["wmd", "wmd"], - ["other", "Other"], - ] - assert form.approval_text == { - "no_concerns": "No Concerns Text", - "concerns": "Concerns Text", - "wmd": "Weapons of mass destruction Text", - "other": "", - } - - -def test_approval_reasons_manual( - requests_mock, - authorized_client, - data_standard_case, - url, - advice_to_consolidate, - gov_user, -): - data_standard_case["case"]["advice"] = advice_to_consolidate - gov_user["user"]["team"]["name"] = "LU Team" - gov_user["user"]["team"]["alias"] = LICENSING_UNIT_TEAM - - requests_mock.get( - client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), - json=gov_user, - ) - - requests_mock.get( - client._build_absolute_uri( - "/picklist/?type=standard_advice&page=1&disable_pagination=True&show_deactivated=False" - ), - json={ - "results": [ - {"name": "custom Value", "text": "This Casing is Maintained"}, - {"name": "another custom value with many spaces", "text": "Concerns Text"}, - {"name": "ALLCAPSNOSPACES", "text": "This is all caps text"}, - ] - }, - ) - response = authorized_client.get(url + "approve/") - assert response.status_code == 200 - form = response.context["form"] - assert isinstance(form, GiveApprovalAdviceForm) - response_choices = [list(choice) for choice in form.fields["approval_radios"].choices] - - assert list(response_choices) == [ - ["custom_value", "custom Value"], - ["another_custom_value_with_many_spaces", "another custom value with many spaces"], - ["allcapsnospaces", "ALLCAPSNOSPACES"], - ["other", "Other"], - ] - assert form.approval_text == { - "custom_value": "This Casing is Maintained", - "another_custom_value_with_many_spaces": "Concerns Text", - "allcapsnospaces": "This is all caps text", - "other": "", - } - - -def test_proviso_manual( - requests_mock, - authorized_client, - data_standard_case, - url, - advice_to_consolidate, - gov_user, -): - data_standard_case["case"]["advice"] = advice_to_consolidate - gov_user["user"]["team"]["name"] = "LU Team" - gov_user["user"]["team"]["alias"] = LICENSING_UNIT_TEAM - - requests_mock.get( - client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), - json=gov_user, - ) - - requests_mock.get( - client._build_absolute_uri("/picklist/?type=proviso&page=1&disable_pagination=True&show_deactivated=False"), - json={ - "results": [ - {"name": "custom Value", "text": "This Casing is Maintained"}, - {"name": "another custom value with many spaces", "text": "Concerns Text"}, - {"name": "ALLCAPSNOSPACES", "text": "This is all caps text"}, - ] - }, - ) - response = authorized_client.get(url + "approve/") - assert response.status_code == 200 - form = response.context["form"] - assert isinstance(form, GiveApprovalAdviceForm) - response_choices = [list(choice) for choice in form.fields["proviso_radios"].choices] - - assert list(response_choices) == [ - ["custom_value", "custom Value"], - ["another_custom_value_with_many_spaces", "another custom value with many spaces"], - ["allcapsnospaces", "ALLCAPSNOSPACES"], - ["other", "Other"], - ] - assert form.proviso_text == { - "custom_value": "This Casing is Maintained", - "another_custom_value_with_many_spaces": "Concerns Text", - "allcapsnospaces": "This is all caps text", - "other": "", - } - - @pytest.mark.parametrize( "team_alias, team_name, recommendation, redirect", [ diff --git a/unit_tests/caseworker/advice/views/test_edit_advice.py b/unit_tests/caseworker/advice/views/test_edit_advice.py index 8cc384fc5..f962abd9e 100644 --- a/unit_tests/caseworker/advice/views/test_edit_advice.py +++ b/unit_tests/caseworker/advice/views/test_edit_advice.py @@ -31,7 +31,7 @@ def post_to_step(post_to_step_factory, url_approve): def test_edit_approve_advice_post( - authorized_client, requests_mock, mock_post_advice, data_standard_case, standard_case_with_advice, post_to_step, url + authorized_client, requests_mock, mock_post_advice, data_standard_case, standard_case_with_advice, post_to_step ): user_advice_create_url = f"/cases/{data_standard_case['case']['id']}/user-advice/" case_data = deepcopy(data_standard_case) @@ -122,7 +122,6 @@ def test_give_approval_advice_post_valid_add_conditional( authorized_client, requests_mock, data_standard_case, - url, mock_approval_reason, mock_proviso, mock_footnote_details, diff --git a/unit_tests/caseworker/advice/views/test_lu_consolidate.py b/unit_tests/caseworker/advice/views/test_lu_consolidate.py index 85821ec83..381e2f00a 100644 --- a/unit_tests/caseworker/advice/views/test_lu_consolidate.py +++ b/unit_tests/caseworker/advice/views/test_lu_consolidate.py @@ -123,83 +123,3 @@ def advice(current_user): } for good_id in ("0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", "6daad1c3-cf97-4aad-b711-d5c9a9f4586e") ] - - -@pytest.fixture -def url_consolidate_review(data_queue, data_standard_case): - return reverse( - "cases:consolidate_review", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]} - ) - - -@patch("caseworker.advice.views.mixins.get_gov_user") -def test_refusal_note_post( - mock_get_gov_user, requests_mock, authorized_client, data_standard_case, url_consolidate_review -): - mock_get_gov_user.return_value = ( - {"user": {"team": {"id": "21313212-23123-3123-323wq2", "alias": LICENSING_UNIT_TEAM}}}, - None, - ) - - requests_mock.post(client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}/final-advice/"), json={}) - - response = authorized_client.post( - url_consolidate_review + "refuse/", data={"denial_reasons": ["1a"], "refusal_note": "test"} - ) - assert response.status_code == 302 - request = requests_mock.request_history.pop() - assert "final-advice" in request.url - assert request.json() == [ - { - "type": "refuse", - "text": "test", - "footnote_required": False, - "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", - "denial_reasons": ["1a"], - "is_refusal_note": True, - }, - { - "type": "refuse", - "text": "test", - "footnote_required": False, - "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", - "denial_reasons": ["1a"], - "is_refusal_note": True, - }, - { - "type": "refuse", - "text": "test", - "footnote_required": False, - "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", - "denial_reasons": ["1a"], - "is_refusal_note": True, - }, - { - "type": "refuse", - "text": "test", - "footnote_required": False, - "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", - "denial_reasons": ["1a"], - "is_refusal_note": True, - }, - { - "type": "no_licence_required", - "text": "", - "proviso": "", - "note": "", - "footnote_required": False, - "footnote": "", - "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", - "denial_reasons": [], - }, - { - "type": "no_licence_required", - "text": "", - "proviso": "", - "note": "", - "footnote_required": False, - "footnote": "", - "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", - "denial_reasons": [], - }, - ] From 5b2d865d01e14e2a400e672cb2fa38277a823fd9 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Wed, 11 Dec 2024 14:34:26 +0000 Subject: [PATCH 07/16] ui tests complete --- caseworker/advice/forms/approval.py | 3 +- caseworker/advice/forms/forms.py | 2 +- caseworker/advice/views/approval.py | 2 + caseworker/advice/views/views.py | 6 +- tests_common/constants.py | 5 + tests_common/functions.py | 13 +- ui_tests/caseworker/conftest.py | 26 ++-- .../give_advice/mod_approve_advice.feature | 15 ++- .../give_advice/mod_clear_advice.feature | 16 ++- .../give_advice/mod_edit_advice.feature | 29 +++-- .../give_advice/ogd_approve_advice.feature | 35 +++++ ui_tests/caseworker/pages/BasePage.py | 5 +- .../pages/add_denial_records_page.py | 3 +- ui_tests/caseworker/pages/advice.py | 41 +++++- ui_tests/caseworker/pages/application_page.py | 13 +- ui_tests/caseworker/pages/case_list_page.py | 5 +- ui_tests/caseworker/pages/case_page.py | 5 +- ui_tests/caseworker/pages/header_page.py | 3 +- .../caseworker/pages/product_assessment.py | 5 +- ui_tests/caseworker/pages/shared.py | 5 +- .../caseworker/step_defs/test_documents.py | 3 +- .../caseworker/step_defs/test_enforcement.py | 7 +- .../step_defs/test_give_advice/conftest.py | 40 +++++- .../test_ogd_approve_advice.py | 3 + .../step_defs/test_product_search.py | 3 +- ui_tests/exporter/conftest.py | 3 +- ui_tests/exporter/pages/shared.py | 3 +- .../advice/views/test_edit_advice.py | 121 ++++++++++++++++++ .../advice/views/test_select_advice_view.py | 25 ++++ 29 files changed, 369 insertions(+), 76 deletions(-) create mode 100644 tests_common/constants.py create mode 100644 ui_tests/caseworker/features/give_advice/ogd_approve_advice.feature create mode 100644 ui_tests/caseworker/step_defs/test_give_advice/test_ogd_approve_advice.py diff --git a/caseworker/advice/forms/approval.py b/caseworker/advice/forms/approval.py index 127518513..9a9a5fbeb 100644 --- a/caseworker/advice/forms/approval.py +++ b/caseworker/advice/forms/approval.py @@ -40,7 +40,7 @@ class Layout: TITLE = "Recommend an approval" approval_reasons = forms.CharField( - widget=forms.Textarea(attrs={"rows": 7}), + widget=forms.Textarea(attrs={"rows": 7, "class": "govuk-!-margin-top-4", "name": "approval_reasons"}), label="", error_messages={"required": "Enter a reason for approving"}, ) @@ -61,7 +61,6 @@ def __init__(self, *args, **kwargs): 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): diff --git a/caseworker/advice/forms/forms.py b/caseworker/advice/forms/forms.py index 505bba217..d7cc103ee 100644 --- a/caseworker/advice/forms/forms.py +++ b/caseworker/advice/forms/forms.py @@ -23,7 +23,7 @@ def get_approval_advice_form_factory(advice, approval_reason, proviso, footnote_ "instructions_to_exporter": advice["note"], "footnote_details": advice["footnote"], } - return FCDOApprovalAdviceForm( + return GiveApprovalAdviceForm( approval_reason=approval_reason, proviso=proviso, footnote_details=footnote_details, data=data ) diff --git a/caseworker/advice/views/approval.py b/caseworker/advice/views/approval.py index e4fa5cd64..4fe4a7d9b 100644 --- a/caseworker/advice/views/approval.py +++ b/caseworker/advice/views/approval.py @@ -28,6 +28,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"] == services.FCDO_TEAM: + return reverse("cases:approve_all_legacy", kwargs=self.kwargs) return reverse("cases:approve_all", kwargs=self.kwargs) else: return reverse("cases:refuse_all", kwargs=self.kwargs) diff --git a/caseworker/advice/views/views.py b/caseworker/advice/views/views.py index ef96d0a63..9fd9e8bd7 100644 --- a/caseworker/advice/views/views.py +++ b/caseworker/advice/views/views.py @@ -8,6 +8,7 @@ from caseworker.advice.forms.delete import DeleteAdviceForm from caseworker.advice.forms.forms import ( FCDOApprovalAdviceForm, + GiveApprovalAdviceForm, get_approval_advice_form_factory, get_formset, ) @@ -223,8 +224,9 @@ def form_valid(self, form): # & therefore, we render the normal forms and not the FCO ones. # This means that here data here doesn't include the list of countries for which # the advice should be applied and so we pop that in using a method. - data["countries"] = self.advised_countries() - if isinstance(form, FCDOApprovalAdviceForm): + if self.caseworker["team"]["alias"] == services.FCDO_TEAM: + data["countries"] = self.advised_countries() + if isinstance(form, GiveApprovalAdviceForm): services.post_approval_advice(self.request, self.case, data) elif isinstance(form, RefusalAdviceForm): data["text"] = data["refusal_reasons"] diff --git a/tests_common/constants.py b/tests_common/constants.py new file mode 100644 index 000000000..a94f0da5a --- /dev/null +++ b/tests_common/constants.py @@ -0,0 +1,5 @@ +class WebDriverDelay: + THIRTY = 30 + FORTYFIVE = 45 + TWENTY = 20 + SIXTY = 60 diff --git a/tests_common/functions.py b/tests_common/functions.py index 484e8ae23..9115e240d 100644 --- a/tests_common/functions.py +++ b/tests_common/functions.py @@ -7,6 +7,7 @@ from selenium.webdriver.remote.webdriver import WebDriver from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait +from tests_common.constants import WebDriverDelay def click_submit(driver: WebDriver): @@ -70,7 +71,7 @@ def select_multi_select_options(driver: WebDriver, element_selector: str, option element = driver.find_element(by=By.CSS_SELECTOR, value=element_selector) element.send_keys(option) element.send_keys(Keys.ENTER) - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located( (By.XPATH, f"//span[@class='selected-options__option-text' and contains(text(), '{option}')]") ), @@ -83,17 +84,17 @@ def click_apply_filters(driver: WebDriver): def open_case_filters(driver: WebDriver): if not driver.find_element(by=By.CLASS_NAME, value="case-filters").is_displayed(): - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, "show-filters-link")) ).click() - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.element_to_be_clickable((By.ID, "accordion-case-filters")) ).click() def try_open_filters(driver: WebDriver): if not driver.find_element(by=By.CLASS_NAME, value="lite-filter-bar").is_displayed(): - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, "show-filters-link")) ).click() @@ -115,7 +116,7 @@ def click_next_page(driver: WebDriver): def select_report_summary_subject_and_fill(driver, subject): suggestion_input_autocomplete = driver.find_element(by=By.ID, value="_report_summary_subject") suggestion_input_autocomplete.send_keys(subject) - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.text_to_be_present_in_element( (By.CSS_SELECTOR, ".lite-autocomplete__menu--visible #_report_summary_subject__option--0"), subject, @@ -128,7 +129,7 @@ def select_report_summary_subject_and_fill(driver, subject): def select_report_summary_prefix_and_fill(driver, prefix): suggestion_input_autocomplete = driver.find_element(by=By.ID, value="_report_summary_prefix") suggestion_input_autocomplete.send_keys(prefix) - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.text_to_be_present_in_element( (By.CSS_SELECTOR, ".lite-autocomplete__menu--visible #_report_summary_prefix__option--0"), prefix, diff --git a/ui_tests/caseworker/conftest.py b/ui_tests/caseworker/conftest.py index d42c1d3cc..295a5e15b 100644 --- a/ui_tests/caseworker/conftest.py +++ b/ui_tests/caseworker/conftest.py @@ -7,6 +7,7 @@ from selenium.webdriver.support.wait import WebDriverWait from core.constants import CaseStatusEnum +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.advice import FinalAdvicePage, RecommendationsAndDecisionPage, TeamAdvicePage from ui_tests.caseworker.pages.case_page import CasePage, CaseTabs from ui_tests.caseworker.pages.teams_pages import TeamsPages @@ -284,7 +285,7 @@ def prepare_case(api_test_client, nlr): # noqa def submit_form(driver): # noqa old_page = driver.find_element(by=By.TAG_NAME, value="html") Shared(driver).click_submit() - WebDriverWait(driver, 45).until(expected_conditions.staleness_of(old_page)) + WebDriverWait(driver, WebDriverDelay.FORTYFIVE).until(expected_conditions.staleness_of(old_page)) @when(parsers.parse('I click the text "{text}"')) @@ -295,7 +296,7 @@ def click_text(driver, text): # noqa @when(parsers.parse('I click "{button_text}"')) def click_button_with_text(driver, button_text): # noqa - WebDriverWait(driver, 20).until( + WebDriverWait(driver, WebDriverDelay.TWENTY).until( expected_conditions.presence_of_element_located( ( By.XPATH, @@ -396,21 +397,28 @@ def case_list_page(driver, internal_url): # noqa @when("I go to my profile page") # noqa def get_profile_page(driver): # noqa - WebDriverWait(driver, 30).until(expected_conditions.presence_of_element_located((By.ID, "link-profile"))).click() + WebDriverWait(driver, WebDriverDelay.THIRTY).until( + expected_conditions.presence_of_element_located((By.ID, "link-profile")) + ).click() @when(parsers.parse('I change my team to "{team}" and default queue to "{queue}"')) # noqa def go_to_team_edit_page(driver, team, queue): # noqa # we should already be on the profile page - WebDriverWait(driver, 30).until(expected_conditions.presence_of_element_located((By.ID, "link-edit-team"))).click() + WebDriverWait(driver, WebDriverDelay.THIRTY).until( + expected_conditions.presence_of_element_located((By.ID, "link-edit-team")) + ).click() teams_page = TeamsPages(driver) teams_page.select_team_from_dropdown(team) teams_page.select_default_queue_from_dropdown(queue) functions.click_submit(driver) # Ensure we return to the profile page - WebDriverWait(driver, 30).until(expected_conditions.presence_of_element_located((By.ID, "link-edit-team"))) + WebDriverWait(driver, WebDriverDelay.THIRTY).until( + expected_conditions.presence_of_element_located((By.ID, "link-edit-team")) + ) # Check that the team/queue change was applied successfully assert driver.find_element(by=By.ID, value="user-team-name").text == team + WebDriverWait(driver, WebDriverDelay.THIRTY) assert driver.find_element(by=By.ID, value="user-default-queue").text == queue @@ -432,7 +440,9 @@ def system_queue_shown_in_dropdown(driver, queue_name): # noqa @when(parsers.parse('I switch to "{queue_name}" queue')) # noqa def switch_to_queue(driver, queue_name): # noqa driver.find_element(by=By.ID, value="link-queue").click() - WebDriverWait(driver, 30).until(expected_conditions.presence_of_element_located((By.ID, "filter-queues"))) + WebDriverWait(driver, WebDriverDelay.THIRTY).until( + expected_conditions.presence_of_element_located((By.ID, "filter-queues")) + ) driver.find_element(by=By.ID, value="filter-queues").send_keys(queue_name) elements = [ item @@ -678,7 +688,7 @@ def click_edit_case_flags_link(driver, flag_name): old_page = driver.find_element(by=By.TAG_NAME, value="html") functions.click_submit(driver) - WebDriverWait(driver, 45).until(expected_conditions.staleness_of(old_page)) + WebDriverWait(driver, WebDriverDelay.FORTYFIVE).until(expected_conditions.staleness_of(old_page)) @given(parsers.parse('the status is set to "{status}"')) # noqa @@ -1005,7 +1015,7 @@ def approve_case_as_team(driver, team, queue, context, internal_url, internal_in recommendation_and_decisions_page.click_make_recommendation() recommendation_and_decisions_page.click_approve_all() Shared(driver).click_submit() - recommendation_and_decisions_page.enter_reasons_for_approving("approving") + recommendation_and_decisions_page.enter_approval_reasons("approving") functions.click_submit(driver) # Submit recommmendation functions.click_submit(driver) # Move case forward click_on_created_application(driver, context, internal_url) diff --git a/ui_tests/caseworker/features/give_advice/mod_approve_advice.feature b/ui_tests/caseworker/features/give_advice/mod_approve_advice.feature index 949cd0e65..0fe18d297 100644 --- a/ui_tests/caseworker/features/give_advice/mod_approve_advice.feature +++ b/ui_tests/caseworker/features/give_advice/mod_approve_advice.feature @@ -51,16 +51,19 @@ Feature: I want to record my user advice and any comments and conditions relatin And I click make recommendation And I click approve all And I click continue - And I enter "reason for approving" as the reasons for approving - And I click the text "Add a licence condition, instruction to exporter or footnote" - And I enter "licence condition" as the licence condition - And I enter "instruction for exporter" as the instructions for the exporter - And I enter "reporting footnote" as the reporting footnote - And I click submit recommendation + And I enter "reason for approving" as the approval reasons + And I click add licence condition + And I click continue + And I enter "licence condition" as the licence condition into the "other" checkbox + And I click continue + And I enter "instruction for exporter" as the instructions for the exporter on the instructions step + And I enter "reporting footnote" as the reporting footnote on the instructions step + And I click continue Then I see "reason for approving" as the reasons for approving And I see "licence condition" as the licence condition And I see "instruction for exporter" as the instructions for the exporter And I see "reporting footnote" as the reporting footnote + When I click move case forward Then I don't see previously created application diff --git a/ui_tests/caseworker/features/give_advice/mod_clear_advice.feature b/ui_tests/caseworker/features/give_advice/mod_clear_advice.feature index 2adb9f834..693be8e9a 100644 --- a/ui_tests/caseworker/features/give_advice/mod_clear_advice.feature +++ b/ui_tests/caseworker/features/give_advice/mod_clear_advice.feature @@ -19,13 +19,15 @@ Feature: I want to record my user advice and any comments and conditions relatin And I click make recommendation And I click approve all And I click continue - And I enter "reason for approving" as the reasons for approving - And I click the text "Add a licence condition, instruction to exporter or footnote" - And I enter "licence condition" as the licence condition - And I enter "instruction for exporter" as the instructions for the exporter - And I enter "reporting footnote" as the reporting footnote - And I click submit recommendation - Then I see "reason for approving" as the reasons for approving + And I enter "approval reason" as the approval reasons + And I click add licence condition + And I click continue + And I enter "licence condition" as the licence condition into the "other" checkbox + And I click continue + And I enter "instruction for exporter" as the instructions for the exporter on the instructions step + And I enter "reporting footnote" as the reporting footnote on the instructions step + And I click continue + Then I see "approval reason" as the reasons for approving And I see "licence condition" as the licence condition And I see "instruction for exporter" as the instructions for the exporter And I see "reporting footnote" as the reporting footnote diff --git a/ui_tests/caseworker/features/give_advice/mod_edit_advice.feature b/ui_tests/caseworker/features/give_advice/mod_edit_advice.feature index a3fe28166..1152ed597 100644 --- a/ui_tests/caseworker/features/give_advice/mod_edit_advice.feature +++ b/ui_tests/caseworker/features/give_advice/mod_edit_advice.feature @@ -19,23 +19,26 @@ Feature: I want to record my user advice and any comments and conditions relatin And I click make recommendation And I click approve all And I click continue - And I enter "reason for approving" as the reasons for approving - And I click the text "Add a licence condition, instruction to exporter or footnote" - And I enter "licence condition" as the licence condition - And I enter "instruction for exporter" as the instructions for the exporter - And I enter "reporting footnote" as the reporting footnote - And I click submit recommendation - Then I see "reason for approving" as the reasons for approving + And I enter "approval reason" as the approval reasons + And I click add licence condition + And I click continue + And I enter "licence condition" as the licence condition into the "other" checkbox + And I click continue + And I enter "instruction for exporter" as the instructions for the exporter on the instructions step + And I enter "reporting footnote" as the reporting footnote on the instructions step + And I click continue + Then I see "approval reason" as the reasons for approving And I see "licence condition" as the licence condition And I see "instruction for exporter" as the instructions for the exporter And I see "reporting footnote" as the reporting footnote When I click "Edit recommendation" - And I enter "reason for approving1" as the reasons for approving - And I click the text "Add a licence condition, instruction to exporter or footnote" - And I enter "licence condition1" as the licence condition - And I enter "instruction for exporter1" as the instructions for the exporter - And I enter "reporting footnote1" as the reporting footnote - And I click submit recommendation + And I enter "reason for approving1" as the approval reasons + And I click continue + And I enter "licence condition1" into the licence condition + And I click continue + And I enter "instruction for exporter1" as the instructions for the exporter on the instructions step + And I enter "reporting footnote1" as the reporting footnote on the instructions step + And I click continue Then I see "reason for approving1" as the reasons for approving And I see "licence condition1" as the licence condition And I see "instruction for exporter1" as the instructions for the exporter diff --git a/ui_tests/caseworker/features/give_advice/ogd_approve_advice.feature b/ui_tests/caseworker/features/give_advice/ogd_approve_advice.feature new file mode 100644 index 000000000..d7e4c8d69 --- /dev/null +++ b/ui_tests/caseworker/features/give_advice/ogd_approve_advice.feature @@ -0,0 +1,35 @@ +@all @internal @give_advice +Feature: I want to record my user advice and any comments and conditions relating to my recommendation + As a logged in government user working on a specific case that is assigned to me + I want to record my user advice and any comments and conditions relating to my recommendation + So that other users can see my decision and know that I have finished assessing this case + + @ogd_approve_advice + Scenario: DESNZ to approve advice journey + Given I sign in as Test UAT user + And I create standard application or standard application has been previously created + When I go to application previously created + And I assign myself to the case + And I assign the case to "DESNZ Chemical cases to review" queue + And I go to my profile page + And I change my team to "DESNZ Chemical" and default queue to "DESNZ Chemical cases to review" + And I go to my case list + And I click the application previously created + And I click the recommendations and decision tab + And I click make recommendation + And I click approve all + And I click continue + And I enter "Hello World" as the approval reasons + And I click add licence condition + And I click continue + And I enter "licence condition" as the licence condition into the "other" checkbox + And I click continue + And I enter "instruction for exporter" as the instructions for the exporter on the instructions step + And I enter "reporting footnote" as the reporting footnote on the instructions step + And I click continue + Then I see "Hello World" as the reasons for approving + And I see "licence condition" as the licence condition + And I see "instruction for exporter" as the instructions for the exporter + And I see "reporting footnote" as the reporting footnote + When I click move case forward + Then I don't see previously created application diff --git a/ui_tests/caseworker/pages/BasePage.py b/ui_tests/caseworker/pages/BasePage.py index 4f12d577d..c221f5553 100644 --- a/ui_tests/caseworker/pages/BasePage.py +++ b/ui_tests/caseworker/pages/BasePage.py @@ -4,6 +4,7 @@ from selenium.webdriver.support.wait import WebDriverWait from tests_common import functions +from tests_common.constants import WebDriverDelay class BasePage: @@ -12,7 +13,9 @@ def __init__(self, driver: WebDriver): # Wait for the cases list to load before interacting with the page if functions.element_with_id_exists(self.driver, "link-queue"): - WebDriverWait(driver, 60).until(expected_conditions.visibility_of_element_located((By.ID, "all-cases-tab"))) + WebDriverWait(driver, WebDriverDelay.SIXTY).until( + expected_conditions.visibility_of_element_located((By.ID, "all-cases-tab")) + ) # The case header is sticky and can often overlay elements preventing clicks, # therefore disable the stickyness of the header when running tests diff --git a/ui_tests/caseworker/pages/add_denial_records_page.py b/ui_tests/caseworker/pages/add_denial_records_page.py index c456219a1..5bcc1e281 100644 --- a/ui_tests/caseworker/pages/add_denial_records_page.py +++ b/ui_tests/caseworker/pages/add_denial_records_page.py @@ -7,6 +7,7 @@ from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.BasePage import BasePage @@ -14,7 +15,7 @@ class AddDenialRecordsPage(BasePage): CSV_FILE_LOCATION = "/tmp/downloads/example-denials.csv" def download_example_csv_file(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.LINK_TEXT, "Download an example .csv file")) ).click() diff --git a/ui_tests/caseworker/pages/advice.py b/ui_tests/caseworker/pages/advice.py index 47d685c6d..b019a469a 100644 --- a/ui_tests/caseworker/pages/advice.py +++ b/ui_tests/caseworker/pages/advice.py @@ -104,6 +104,11 @@ def click_approve_all(self): def click_refuse_all(self): self.driver.find_element(by=By.XPATH, value="//input[@type='radio' and @value='refuse_all']").click() + def click_add_licence_condition(self): + self.driver.find_element( + by=By.XPATH, value="//input[@id='id_recommend_approval-add_licence_conditions']" + ).click() + def select_country(self, country): self.driver.find_element(by=By.XPATH, value=f"//input[@type='checkbox' and @value='{country}']").click() @@ -115,6 +120,11 @@ def enter_reasons_for_approving(self, reasons): el.clear() el.send_keys(reasons) + def enter_approval_reasons(self, reasons): + el = self.driver.find_element(by=By.XPATH, value="//textarea[@name='recommend_approval-approval_reasons']") + el.clear() + el.send_keys(reasons) + def enter_reasons_for_refusal(self, reasons): el = self.driver.find_element(by=By.XPATH, value="//textarea[@name='refusal_reasons']") el.clear() @@ -125,17 +135,42 @@ def enter_refusal_note(self, note): el.clear() el.send_keys(note) - def enter_licence_condition(self, licence_condition): - el = self.driver.find_element(by=By.XPATH, value="//textarea[@name='proviso']") + def enter_licence_condition(self, licence_condition, condition_selected): + self.driver.find_element( + by=By.XPATH, value=f"//input[@type='checkbox' and @value='{condition_selected}']" + ).click() + el = self.driver.find_element(by=By.XPATH, value=f"//textarea[@name='licence_conditions-{condition_selected}']") + el.clear() + el.send_keys(licence_condition) + + def enter_licence_condition_edit(self, licence_condition): + el = self.driver.find_element(by=By.XPATH, value=f"//textarea[@name='licence_conditions-proviso']") el.clear() el.send_keys(licence_condition) def enter_instructions_for_exporter(self, instructions): - el = self.driver.find_element(by=By.XPATH, value="//textarea[@name='instructions_to_exporter']") + el = self.driver.find_element( + by=By.XPATH, value="//textarea[@name='licence_footnotes-instructions_to_exporter']" + ) el.clear() el.send_keys(instructions) def enter_reporting_footnote(self, footnote): + el = self.driver.find_element(by=By.XPATH, value="//textarea[@name='licence_footnotes-footnote_details']") + el.clear() + el.send_keys(footnote) + + def enter_licence_condition_legacy(self, licence_condition): + el = self.driver.find_element(by=By.XPATH, value="//textarea[@name='proviso']") + el.clear() + el.send_keys(licence_condition) + + def enter_instructions_for_exporter_legacy(self, instructions): + el = self.driver.find_element(by=By.XPATH, value="//textarea[@name='instructions_to_exporter']") + el.clear() + el.send_keys(instructions) + + def enter_reporting_footnote_legacy(self, footnote): el = self.driver.find_element(by=By.XPATH, value="//textarea[@name='footnote_details']") el.clear() el.send_keys(footnote) diff --git a/ui_tests/caseworker/pages/application_page.py b/ui_tests/caseworker/pages/application_page.py index efd6716fb..e13ed7a02 100644 --- a/ui_tests/caseworker/pages/application_page.py +++ b/ui_tests/caseworker/pages/application_page.py @@ -4,6 +4,7 @@ from selenium.webdriver.support.ui import Select from tests_common import functions +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.BasePage import BasePage from tests_common.tools.helpers import scroll_to_element_by_id from tests_common.tools.helpers import scroll_to_element_below_header_by_id @@ -67,7 +68,7 @@ def get_case_copy_of_field_href(self): return self.driver.find_element(by=By.ID, value=self.CASE_COPY_OF_ID).get_attribute("href") def click_visible_to_exporter_checkbox(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, self.IS_VISIBLE_TO_EXPORTER_CHECKBOX_ID)) ).click() @@ -82,7 +83,7 @@ def get_text_of_case_note_field(self): return self.driver.find_element(by=By.ID, value=self.INPUT_CASE_NOTE_ID).text def click_post_note_btn(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located( (By.CSS_SELECTOR, f"#{self.BUTTON_POST_NOTE_ID}:not([disabled])") ) @@ -90,7 +91,7 @@ def click_post_note_btn(self): functions.click_submit(self.driver) def click_cancel_btn(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, self.LINK_CANCEL_NOTE_ID)) ).click() @@ -150,7 +151,7 @@ def select_a_good(self): self.driver.execute_script("arguments[0].click();", element) def click_move_case_button(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, self.MOVE_CASE_BUTTON)) ).click() @@ -241,7 +242,7 @@ def click_assign_user_button(self): self.driver.find_element(by=By.ID, value=self.ASSIGN_USER_ID).click() def click_im_done_button(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, self.BUTTON_IM_DONE_ID)) ).click() @@ -305,7 +306,7 @@ def get_matches(self, match_type): """Return a list of names that have denial matches based on the supplied match_type - one of "PARTIAL MATCH" or "EXACT MATCH". """ - table = WebDriverWait(self.driver, 30).until( + table = WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.XPATH, "//table[@id='table-denial-matches']")) ) diff --git a/ui_tests/caseworker/pages/case_list_page.py b/ui_tests/caseworker/pages/case_list_page.py index cda3687c1..2eef7d638 100644 --- a/ui_tests/caseworker/pages/case_list_page.py +++ b/ui_tests/caseworker/pages/case_list_page.py @@ -4,6 +4,7 @@ from selenium.webdriver.support.wait import WebDriverWait from selenium.webdriver.support.select import Select +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.BasePage import BasePage from ui_tests.caseworker.pages.shared import Shared from tests_common import functions @@ -154,7 +155,7 @@ def click_on_queue_title(self): self.driver.find_element(by=By.ID, value=self.LINK_CHANGE_QUEUE_ID).click() def search_for_queue(self, queue_name): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, self.QUEUE_SEARCH_BOX)) ) self.driver.find_element(by=By.ID, value=self.QUEUE_SEARCH_BOX).send_keys(queue_name) @@ -216,6 +217,6 @@ def click_checkbox_to_show_team_ecju_query_and_hidden_cases(self): return self.driver.find_element(by=By.ID, value=self.SHOW_TEAM_ECJU_AND_HIDDEN_CASES).click() def click_export_enforcement_xml(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, self.EXPORT_ENFORCEMENT_XML_BUTTON_ID)) ).click() diff --git a/ui_tests/caseworker/pages/case_page.py b/ui_tests/caseworker/pages/case_page.py index 9323081a8..2e43ff3e9 100644 --- a/ui_tests/caseworker/pages/case_page.py +++ b/ui_tests/caseworker/pages/case_page.py @@ -1,6 +1,7 @@ from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.shared import Shared from ui_tests.caseworker.pages.BasePage import BasePage from tests_common import selectors @@ -128,7 +129,9 @@ def is_flag_applied(self, flag_name): self.driver.find_element(by=By.ID, value="candy-flags").click() - WebDriverWait(self.driver, 30).until(expected_conditions.presence_of_element_located((By.ID, POPUP_FLAGS_ID))) + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + expected_conditions.presence_of_element_located((By.ID, POPUP_FLAGS_ID)) + ) return flag_name in self.driver.find_element(by=By.ID, value=POPUP_FLAGS_ID).text diff --git a/ui_tests/caseworker/pages/header_page.py b/ui_tests/caseworker/pages/header_page.py index a91646d33..461c6609a 100644 --- a/ui_tests/caseworker/pages/header_page.py +++ b/ui_tests/caseworker/pages/header_page.py @@ -1,3 +1,4 @@ +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.BasePage import BasePage from tests_common.tools.wait import wait_until_page_is_loaded @@ -21,7 +22,7 @@ def click_lite_menu(self): self.driver.find_element(by=By.ID, value=self.MENU_BUTTON).click() def click_organisations(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.CSS_SELECTOR, self.ORGANISATIONS_LINK)) ).click() diff --git a/ui_tests/caseworker/pages/product_assessment.py b/ui_tests/caseworker/pages/product_assessment.py index ed3212dae..421468a29 100644 --- a/ui_tests/caseworker/pages/product_assessment.py +++ b/ui_tests/caseworker/pages/product_assessment.py @@ -4,6 +4,7 @@ from selenium.webdriver.support.wait import WebDriverWait from tests_common import functions +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.BasePage import BasePage @@ -23,7 +24,7 @@ def assess_report_summary_prefix(self, ars_prefix): ars_prefix_element = self.driver.find_element(by=By.ID, value="report_summary_prefix_container") ars_prefix_input = ars_prefix_element.find_element(by=By.ID, value="_report_summary_prefix") ars_prefix_input.send_keys(ars_prefix) - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, "_report_summary_prefix__listbox")) ) @@ -39,7 +40,7 @@ def assess_report_summary_subject(self, ars_subject): ars_subject_element = self.driver.find_element(by=By.ID, value="report_summary_subject_container") ars_subject_input = ars_subject_element.find_element(by=By.ID, value="_report_summary_subject") ars_subject_input.send_keys(ars_subject) - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, "_report_summary_subject__listbox")) ) diff --git a/ui_tests/caseworker/pages/shared.py b/ui_tests/caseworker/pages/shared.py index ebde39fac..7b42939fd 100644 --- a/ui_tests/caseworker/pages/shared.py +++ b/ui_tests/caseworker/pages/shared.py @@ -3,6 +3,7 @@ from selenium.webdriver.support.wait import WebDriverWait from tests_common import functions +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.BasePage import BasePage from tests_common.tools.helpers import scroll_to_element_by_id @@ -122,14 +123,14 @@ def click_first_link_in_row(self): def expand_govuk_details(self): self.driver.find_element(by=By.CLASS_NAME, value=self.GOVUK_DETAILS).click() - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located( (By.XPATH, f"//*[contains(@class, '{self.GOVUK_DETAILS}') and ancestor::details/@open]") ) ) def try_open_filters(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.CLASS_NAME, "lite-filter-bar")) ) diff --git a/ui_tests/caseworker/step_defs/test_documents.py b/ui_tests/caseworker/step_defs/test_documents.py index 747efc190..3f7be003e 100644 --- a/ui_tests/caseworker/step_defs/test_documents.py +++ b/ui_tests/caseworker/step_defs/test_documents.py @@ -3,6 +3,7 @@ from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.application_page import ApplicationPage from ui_tests.caseworker.pages.attach_document_page import AttachDocumentPage from ui_tests.caseworker.pages.documents_page import DocumentsPage @@ -28,7 +29,7 @@ def upload_a_file(driver, filename, description, tmp_path): old_page = driver.find_element(by=By.TAG_NAME, value="html") attach_document_page.click_submit_btn() - WebDriverWait(driver, 45).until(expected_conditions.staleness_of(old_page)) + WebDriverWait(driver, WebDriverDelay.FORTYFIVE).until(expected_conditions.staleness_of(old_page)) @then(parsers.parse('I see a file with filename "{filename}" is uploaded')) diff --git a/ui_tests/caseworker/step_defs/test_enforcement.py b/ui_tests/caseworker/step_defs/test_enforcement.py index 82180a921..49bf9cda0 100644 --- a/ui_tests/caseworker/step_defs/test_enforcement.py +++ b/ui_tests/caseworker/step_defs/test_enforcement.py @@ -8,6 +8,7 @@ from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.application_page import ApplicationPage from ui_tests.caseworker.pages.case_list_page import CaseListPage from ui_tests.caseworker.pages.shared import Shared @@ -146,9 +147,9 @@ def i_attach_updated_file(driver, enforcement_check_import_xml_file_path): # no upload_btn = driver.find_element(by=By.XPATH, value="//button[@type='submit']") upload_btn.click() - WebDriverWait(driver, 45).until(expected_conditions.staleness_of(old_page)) + WebDriverWait(driver, WebDriverDelay.THIRTY).until(expected_conditions.staleness_of(old_page)) - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.text_to_be_present_in_element( (By.CSS_SELECTOR, ".app-snackbar__content"), "Enforcement XML imported successfully", @@ -161,7 +162,7 @@ def i_attach_updated_file(driver, enforcement_check_import_xml_file_path): # no @then(parsers.parse('the application is removed from "{queue}" queue')) def application_removed_from_queue(driver, queue): ASSIGNED_QUEUES_ID = "assigned-queues" - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, ASSIGNED_QUEUES_ID)) ).is_enabled() queue_list = driver.find_element(by=By.ID, value=ASSIGNED_QUEUES_ID).text.split("\n") diff --git a/ui_tests/caseworker/step_defs/test_give_advice/conftest.py b/ui_tests/caseworker/step_defs/test_give_advice/conftest.py index d22b7034e..4b97614e6 100644 --- a/ui_tests/caseworker/step_defs/test_give_advice/conftest.py +++ b/ui_tests/caseworker/step_defs/test_give_advice/conftest.py @@ -39,6 +39,11 @@ def click_refuse_all(driver): # noqa RecommendationsAndDecisionPage(driver).click_refuse_all() +@when("I click add licence condition") +def click_add_licence_condition(driver): # noqa + RecommendationsAndDecisionPage(driver).click_add_licence_condition() + + @when(parsers.parse('I select refusal criteria "{criteria}"')) def select_refusal_criteria(driver, criteria): # noqa functions.select_multi_select_options( @@ -62,6 +67,11 @@ def enter_reasons_for_approving(driver, reasons, context): # noqa RecommendationsAndDecisionPage(driver).enter_reasons_for_approving(reasons) +@when(parsers.parse('I enter "{reasons}" as the approval reasons')) +def enter_approval_reasons(driver, reasons, context): # noqa + RecommendationsAndDecisionPage(driver).enter_approval_reasons(reasons) + + @when(parsers.parse('I enter "{reasons}" as the reasons for refusal')) def enter_reasons_for_refusal(driver, reasons, context): # noqa RecommendationsAndDecisionPage(driver).enter_reasons_for_refusal(reasons) @@ -72,21 +82,41 @@ def enter_refusal_note(driver, note, context): # noqa RecommendationsAndDecisionPage(driver).enter_refusal_note(note) -@when(parsers.parse('I enter "{licence_condition}" as the licence condition')) -def enter_licence_condition(driver, licence_condition, context): # noqa - RecommendationsAndDecisionPage(driver).enter_licence_condition(licence_condition) +@when(parsers.parse('I enter "{licence_condition}" as the licence condition into the "{condition_selected}" checkbox')) +def enter_licence_condition(driver, licence_condition, condition_selected, context): # noqa + RecommendationsAndDecisionPage(driver).enter_licence_condition(licence_condition, condition_selected) -@when(parsers.parse('I enter "{instructions}" as the instructions for the exporter')) +@when(parsers.parse('I enter "{licence_condition}" into the licence condition')) +def enter_licence_condition_edit(driver, licence_condition, context): # noqa + RecommendationsAndDecisionPage(driver).enter_licence_condition_edit(licence_condition) + + +@when(parsers.parse('I enter "{instructions}" as the instructions for the exporter on the instructions step')) def enter_instructions_for_exporter(driver, instructions, context): # noqa RecommendationsAndDecisionPage(driver).enter_instructions_for_exporter(instructions) -@when(parsers.parse('I enter "{footnote}" as the reporting footnote')) +@when(parsers.parse('I enter "{footnote}" as the reporting footnote on the instructions step')) def enter_reporting_footnote(driver, footnote, context): # noqa RecommendationsAndDecisionPage(driver).enter_reporting_footnote(footnote) +@when(parsers.parse('I enter "{licence_condition}" as the licence condition')) +def enter_licence_condition_legacy(driver, licence_condition, context): # noqa + RecommendationsAndDecisionPage(driver).enter_licence_condition_legacy(licence_condition) + + +@when(parsers.parse('I enter "{instructions}" as the instructions for the exporter')) +def enter_instructions_for_exporter_legacy(driver, instructions, context): # noqa + RecommendationsAndDecisionPage(driver).enter_instructions_for_exporter_legacy(instructions) + + +@when(parsers.parse('I enter "{footnote}" as the reporting footnote')) +def enter_reporting_footnote_legacy(driver, footnote, context): # noqa + RecommendationsAndDecisionPage(driver).enter_reporting_footnote_legacy(footnote) + + @then(parsers.parse('I should see my recommendation for "{countries}" with "{reasons}"')) def should_see_recommendation(driver, countries, reasons): # noqa text = driver.find_element(by=By.XPATH, value="//main[@class='govuk-main-wrapper']//*").text diff --git a/ui_tests/caseworker/step_defs/test_give_advice/test_ogd_approve_advice.py b/ui_tests/caseworker/step_defs/test_give_advice/test_ogd_approve_advice.py new file mode 100644 index 000000000..7d20f0486 --- /dev/null +++ b/ui_tests/caseworker/step_defs/test_give_advice/test_ogd_approve_advice.py @@ -0,0 +1,3 @@ +from pytest_bdd import scenarios + +scenarios("../../features/give_advice/ogd_approve_advice.feature", strict_gherkin=False) diff --git a/ui_tests/caseworker/step_defs/test_product_search.py b/ui_tests/caseworker/step_defs/test_product_search.py index 0559e53b9..009f293d1 100644 --- a/ui_tests/caseworker/step_defs/test_product_search.py +++ b/ui_tests/caseworker/step_defs/test_product_search.py @@ -8,6 +8,7 @@ from selenium.webdriver.support.wait import WebDriverWait from tests_common import functions +from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.product_search import ProductSearchPage @@ -16,7 +17,7 @@ @when("I go to product search page") def go_to_product_search_page(driver): - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, "link-product-search")) ).click() diff --git a/ui_tests/exporter/conftest.py b/ui_tests/exporter/conftest.py index ad435c4cc..5f55dcf9d 100644 --- a/ui_tests/exporter/conftest.py +++ b/ui_tests/exporter/conftest.py @@ -10,6 +10,7 @@ from selenium.webdriver.support.wait import WebDriverWait from selenium.common.exceptions import NoSuchElementException +from tests_common.constants import WebDriverDelay import tests_common.tools.helpers as utils from ui_tests.exporter.fixtures.add_end_user_advisory import add_end_user_advisory # noqa from ui_tests.exporter.fixtures.add_party import add_end_user_to_application # noqa @@ -1242,7 +1243,7 @@ def edit_good_details_in_application(driver, field_name, updated_value): # noqa @when(parsers.parse('I click on "{link_text}"')) # noqa def click_link_with_text(driver, link_text): # noqa - WebDriverWait(driver, 30).until( + WebDriverWait(driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located( ( By.LINK_TEXT, diff --git a/ui_tests/exporter/pages/shared.py b/ui_tests/exporter/pages/shared.py index 24a9b074f..444fc2e2d 100644 --- a/ui_tests/exporter/pages/shared.py +++ b/ui_tests/exporter/pages/shared.py @@ -1,3 +1,4 @@ +from tests_common.constants import WebDriverDelay from ui_tests.exporter.pages.BasePage import BasePage from selenium.webdriver.common.by import By @@ -33,7 +34,7 @@ def get_table_rows(self): return self.driver.find_elements_by_css_selector(self.GOV_TABLE_ROW) def get_text_of_organisation_heading(self): - WebDriverWait(self.driver, 30).until( + WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( expected_conditions.presence_of_element_located((By.ID, self.ORG_NAME_HEADING_ID)) ) diff --git a/unit_tests/caseworker/advice/views/test_edit_advice.py b/unit_tests/caseworker/advice/views/test_edit_advice.py index f962abd9e..e1fee1278 100644 --- a/unit_tests/caseworker/advice/views/test_edit_advice.py +++ b/unit_tests/caseworker/advice/views/test_edit_advice.py @@ -30,6 +30,95 @@ def post_to_step(post_to_step_factory, url_approve): return post_to_step_factory(url_approve) +def test_edit_approve_advice_post_legacy( + authorized_client, requests_mock, data_standard_case, standard_case_with_advice, url +): + 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"}}, + ) + + data = { + "approval_reasons": "meets the requirements updated", + "instructions_to_exporter": "no specific instructions", + } + response = authorized_client.post(url, data=data) + 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() == [ + { + "denial_reasons": [], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote": "", + "footnote_required": False, + "note": "no specific instructions", + "proviso": "", + "text": "meets the requirements updated", + "type": "approve", + }, + { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "no specific instructions", + "proviso": "", + "text": "meets the requirements updated", + "type": "approve", + }, + { + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "no specific instructions", + "proviso": "", + "text": "meets the requirements updated", + "type": "approve", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "no specific instructions", + "proviso": "", + "text": "meets the requirements updated", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": "approve", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "9fbffa7f-ef50-402e-93ac-2f3f37d09030", + "note": "no specific instructions", + "proviso": "", + "text": "meets the requirements updated", + "type": "approve", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "d4feac1e-851d-41a5-b833-eb28addb8547", + "note": "", + "proviso": "", + "text": "", + "type": "no_licence_required", + }, + ] + + def test_edit_approve_advice_post( authorized_client, requests_mock, mock_post_advice, data_standard_case, standard_case_with_advice, post_to_step ): @@ -265,6 +354,38 @@ def test_edit_refuse_advice_post( assert user_advice_create_url in history.url assert history.method == "POST" assert history.json() == [ + { + "type": "refuse", + "text": "doesn't meet the requirement", + "footnote_required": False, + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "denial_reasons": ["3", "4", "5", "5a", "5b"], + "is_refusal_note": False, + }, + { + "type": "refuse", + "text": "doesn't meet the requirement", + "footnote_required": False, + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", + "denial_reasons": ["3", "4", "5", "5a", "5b"], + "is_refusal_note": False, + }, + { + "type": "refuse", + "text": "doesn't meet the requirement", + "footnote_required": False, + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + "denial_reasons": ["3", "4", "5", "5a", "5b"], + "is_refusal_note": False, + }, + { + "type": "refuse", + "text": "doesn't meet the requirement", + "footnote_required": False, + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "denial_reasons": ["3", "4", "5", "5a", "5b"], + "is_refusal_note": False, + }, { "type": "refuse", "text": "doesn't meet the requirement", 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 0420722a3..23e73e5e6 100644 --- a/unit_tests/caseworker/advice/views/test_select_advice_view.py +++ b/unit_tests/caseworker/advice/views/test_select_advice_view.py @@ -31,6 +31,31 @@ def test_select_advice_post(authorized_client, url, recommendation, redirect, da ) +@pytest.mark.parametrize( + "recommendation, redirect", [("approve_all", "approve-all-legacy"), ("refuse_all", "refuse-all")] +) +def test_select_advice_post_fcdo(authorized_client, url, recommendation, redirect, data_standard_case, mocker): + get_gov_user_value = ( + { + "user": { + "team": { + "id": "56273dd4-4634-4ad7-a782-e480f85a85a9", + "name": "FCDO", + "alias": services.FCDO_TEAM, + } + } + }, + None, + ) + mocker.patch("caseworker.advice.views.mixins.get_gov_user", return_value=get_gov_user_value) + response = authorized_client.post(url, data={"recommendation": recommendation}) + assert response.status_code == 302 + assert ( + response.url + == f'/queues/00000000-0000-0000-0000-000000000001/cases/{data_standard_case["case"]["id"]}/advice/{redirect}/' + ) + + def test_select_advice_post_desnz(authorized_client, url, data_standard_case, mocker): get_gov_user_value = ( { From 1f8790d298439f738698266df36545ea67c08c48 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Tue, 7 Jan 2025 10:09:20 +0000 Subject: [PATCH 08/16] changes based on git comments, formvalid over request.POST, and a few other minor changes --- caseworker/advice/conditionals.py | 4 ---- caseworker/advice/forms/approval.py | 2 +- caseworker/advice/rules.py | 2 +- caseworker/advice/views/approval.py | 11 +++++++---- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/caseworker/advice/conditionals.py b/caseworker/advice/conditionals.py index 411fba16d..1d8fb9fad 100644 --- a/caseworker/advice/conditionals.py +++ b/caseworker/advice/conditionals.py @@ -13,7 +13,3 @@ def _get_form_field_boolean(wizard): def is_fcdo_team(wizard): return wizard.caseworker["team"]["alias"] == services.FCDO_TEAM - - -def is_ogd_team(wizard): - return not is_fcdo_team(wizard) diff --git a/caseworker/advice/forms/approval.py b/caseworker/advice/forms/approval.py index 9a9a5fbeb..227910008 100644 --- a/caseworker/advice/forms/approval.py +++ b/caseworker/advice/forms/approval.py @@ -40,7 +40,7 @@ class Layout: TITLE = "Recommend an approval" approval_reasons = forms.CharField( - widget=forms.Textarea(attrs={"rows": 7, "class": "govuk-!-margin-top-4", "name": "approval_reasons"}), + widget=forms.Textarea(attrs={"rows": 7, "class": "govuk-!-margin-top-4"}), label="", error_messages={"required": "Enter a reason for approving"}, ) diff --git a/caseworker/advice/rules.py b/caseworker/advice/rules.py index 091f5b82b..2f2568a34 100644 --- a/caseworker/advice/rules.py +++ b/caseworker/advice/rules.py @@ -34,7 +34,7 @@ def can_desnz_make_recommendation(user, case, queue_alias): def can_ogd_make_edit(team): - return not team == services.FCDO_TEAM + return team != services.FCDO_TEAM def case_has_approval_advice(advice): diff --git a/caseworker/advice/views/approval.py b/caseworker/advice/views/approval.py index 4fe4a7d9b..7c10c6093 100644 --- a/caseworker/advice/views/approval.py +++ b/caseworker/advice/views/approval.py @@ -1,5 +1,5 @@ from http import HTTPStatus -from caseworker.advice.conditionals import form_add_licence_conditions, is_ogd_team +from caseworker.advice.conditionals import form_add_licence_conditions, is_fcdo_team from caseworker.advice.forms.approval import ( FootnotesApprovalAdviceForm, PicklistLicenceConditionsForm, @@ -26,14 +26,17 @@ class SelectAdviceView(LoginRequiredMixin, CaseContextMixin, FormView): form_class = SelectAdviceForm def get_success_url(self): - recommendation = self.request.POST.get("recommendation") - if recommendation == "approve_all": + if self.recommendation == "approve_all": if self.caseworker["team"]["alias"] == services.FCDO_TEAM: return reverse("cases:approve_all_legacy", kwargs=self.kwargs) return reverse("cases:approve_all", kwargs=self.kwargs) else: return reverse("cases:refuse_all", kwargs=self.kwargs) + def form_valid(self, form): + self.recommendation = form.cleaned_data["recommendation"] + return super().form_valid(form) + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) return {**context, "security_approvals_classified_display": self.security_approvals_classified_display} @@ -42,7 +45,7 @@ def get_context_data(self, **kwargs): class BaseApprovalAdviceView(LoginRequiredMixin, CaseContextMixin, BaseSessionWizardView): condition_dict = { - AdviceSteps.RECOMMEND_APPROVAL: C(is_ogd_team), + 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)), } From 4179534c04b60432caaa32122c409a1c22135712 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Tue, 7 Jan 2025 15:36:54 +0000 Subject: [PATCH 09/16] rebase issue --- caseworker/advice/forms/forms.py | 2 +- unit_tests/caseworker/advice/views/test_edit_advice.py | 4 ++-- .../advice/views/test_give_approval_advice_view.py | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/caseworker/advice/forms/forms.py b/caseworker/advice/forms/forms.py index d7cc103ee..3758ccdc9 100644 --- a/caseworker/advice/forms/forms.py +++ b/caseworker/advice/forms/forms.py @@ -61,7 +61,7 @@ class GiveApprovalAdviceForm(PicklistAdviceForm): ) instructions_to_exporter = forms.CharField( widget=forms.Textarea(attrs={"rows": "3"}), - label="Add any instructions for the exporter (optional)", + label="Add instructions to the exporter, or a reporting footnote (optional)", help_text="These may be added to the licence cover letter, subject to review by the Licensing Unit.", required=False, ) diff --git a/unit_tests/caseworker/advice/views/test_edit_advice.py b/unit_tests/caseworker/advice/views/test_edit_advice.py index e1fee1278..437b66cb7 100644 --- a/unit_tests/caseworker/advice/views/test_edit_advice.py +++ b/unit_tests/caseworker/advice/views/test_edit_advice.py @@ -238,7 +238,7 @@ def test_give_approval_advice_post_valid_add_conditional( 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)" + assert header.text == "Add licence conditions (optional)" add_licence_condition_response = post_to_step( AdviceSteps.LICENCE_CONDITIONS, @@ -248,7 +248,7 @@ def test_give_approval_advice_post_valid_add_conditional( soup = beautiful_soup(add_licence_condition_response.content) # redirected to next form header = soup.find("h1") - assert header.text == "Instructions for the exporter (optional)" + assert header.text == "Add instructions to the exporter, or a reporting footnote (optional)" add_instructions_response = post_to_step( AdviceSteps.LICENCE_FOOTNOTES, 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 4c73d87c6..adc8fcb87 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 @@ -73,7 +73,7 @@ def test_approval_advice_post_valid_add_conditional( 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)" + assert header.text == "Add licence conditions (optional)" add_LC_response = post_to_step( AdviceSteps.LICENCE_CONDITIONS, @@ -83,7 +83,7 @@ def test_approval_advice_post_valid_add_conditional( soup = beautiful_soup(add_LC_response.content) # redirected to next form header = soup.find("h1") - assert header.text == "Instructions for the exporter (optional)" + assert header.text == "Add instructions to the exporter, or a reporting footnote (optional)" add_instructions_response = post_to_step( AdviceSteps.LICENCE_FOOTNOTES, @@ -111,7 +111,7 @@ def test_approval_advice_post_valid_add_conditional_optional( 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)" + assert header.text == "Add licence conditions (optional)" add_LC_response = post_to_step( AdviceSteps.LICENCE_CONDITIONS, @@ -121,7 +121,7 @@ def test_approval_advice_post_valid_add_conditional_optional( soup = beautiful_soup(add_LC_response.content) # redirected to next form header = soup.find("h1") - assert header.text == "Instructions for the exporter (optional)" + assert header.text == "Add instructions to the exporter, or a reporting footnote (optional)" add_instructions_response = post_to_step( AdviceSteps.LICENCE_FOOTNOTES, From a340e7ac441bc41d0f8d109d7f2d034f8a0d8dd4 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Tue, 7 Jan 2025 15:57:32 +0000 Subject: [PATCH 10/16] converted constant to env setting --- conf/caseworker.py | 1 + conf/exporter.py | 1 + tests_common/constants.py | 5 ----- tests_common/functions.py | 13 ++++++------- ui_tests/caseworker/conftest.py | 18 ++++++++---------- ui_tests/caseworker/pages/BasePage.py | 4 ++-- .../pages/add_denial_records_page.py | 4 ++-- ui_tests/caseworker/pages/application_page.py | 14 +++++++------- ui_tests/caseworker/pages/case_list_page.py | 6 +++--- ui_tests/caseworker/pages/case_page.py | 4 ++-- ui_tests/caseworker/pages/header_page.py | 4 ++-- .../caseworker/pages/product_assessment.py | 6 +++--- ui_tests/caseworker/pages/shared.py | 6 +++--- .../caseworker/step_defs/test_documents.py | 4 ++-- .../caseworker/step_defs/test_enforcement.py | 8 ++++---- .../step_defs/test_product_search.py | 4 ++-- ui_tests/exporter/conftest.py | 3 +-- ui_tests/exporter/pages/shared.py | 4 ++-- 18 files changed, 51 insertions(+), 58 deletions(-) delete mode 100644 tests_common/constants.py diff --git a/conf/caseworker.py b/conf/caseworker.py index 70820fdbd..5816548d6 100644 --- a/conf/caseworker.py +++ b/conf/caseworker.py @@ -134,3 +134,4 @@ # using it here as some browsers still don't support CSP_REPORT_TO which replaces it CSP_REPORT_URI = env.tuple("CASEWORKER_CSP_REPORT_URI", default=("",)) +E2E_WAIT_MULTIPLIER = env.int("E2E_WAIT_MULTIPLIER", default=1) diff --git a/conf/exporter.py b/conf/exporter.py index 2879f0d03..c2baebadf 100644 --- a/conf/exporter.py +++ b/conf/exporter.py @@ -129,3 +129,4 @@ # using it here as some browsers still don't support CSP_REPORT_TO which replaces it CSP_REPORT_URI = env.tuple("EXPORTER_CSP_REPORT_URI", default=("",)) +E2E_WAIT_MULTIPLIER = env.int("E2E_WAIT_MULTIPLIER", default=1) diff --git a/tests_common/constants.py b/tests_common/constants.py deleted file mode 100644 index a94f0da5a..000000000 --- a/tests_common/constants.py +++ /dev/null @@ -1,5 +0,0 @@ -class WebDriverDelay: - THIRTY = 30 - FORTYFIVE = 45 - TWENTY = 20 - SIXTY = 60 diff --git a/tests_common/functions.py b/tests_common/functions.py index 9115e240d..783649d7c 100644 --- a/tests_common/functions.py +++ b/tests_common/functions.py @@ -7,7 +7,6 @@ from selenium.webdriver.remote.webdriver import WebDriver from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait -from tests_common.constants import WebDriverDelay def click_submit(driver: WebDriver): @@ -71,7 +70,7 @@ def select_multi_select_options(driver: WebDriver, element_selector: str, option element = driver.find_element(by=By.CSS_SELECTOR, value=element_selector) element.send_keys(option) element.send_keys(Keys.ENTER) - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located( (By.XPATH, f"//span[@class='selected-options__option-text' and contains(text(), '{option}')]") ), @@ -84,17 +83,17 @@ def click_apply_filters(driver: WebDriver): def open_case_filters(driver: WebDriver): if not driver.find_element(by=By.CLASS_NAME, value="case-filters").is_displayed(): - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, "show-filters-link")) ).click() - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.element_to_be_clickable((By.ID, "accordion-case-filters")) ).click() def try_open_filters(driver: WebDriver): if not driver.find_element(by=By.CLASS_NAME, value="lite-filter-bar").is_displayed(): - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, "show-filters-link")) ).click() @@ -116,7 +115,7 @@ def click_next_page(driver: WebDriver): def select_report_summary_subject_and_fill(driver, subject): suggestion_input_autocomplete = driver.find_element(by=By.ID, value="_report_summary_subject") suggestion_input_autocomplete.send_keys(subject) - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.text_to_be_present_in_element( (By.CSS_SELECTOR, ".lite-autocomplete__menu--visible #_report_summary_subject__option--0"), subject, @@ -129,7 +128,7 @@ def select_report_summary_subject_and_fill(driver, subject): def select_report_summary_prefix_and_fill(driver, prefix): suggestion_input_autocomplete = driver.find_element(by=By.ID, value="_report_summary_prefix") suggestion_input_autocomplete.send_keys(prefix) - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.text_to_be_present_in_element( (By.CSS_SELECTOR, ".lite-autocomplete__menu--visible #_report_summary_prefix__option--0"), prefix, diff --git a/ui_tests/caseworker/conftest.py b/ui_tests/caseworker/conftest.py index 295a5e15b..41e811801 100644 --- a/ui_tests/caseworker/conftest.py +++ b/ui_tests/caseworker/conftest.py @@ -4,10 +4,8 @@ from pytest_bdd import given, when, then, parsers from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions -from selenium.webdriver.support.wait import WebDriverWait from core.constants import CaseStatusEnum -from tests_common.constants import WebDriverDelay from ui_tests.caseworker.pages.advice import FinalAdvicePage, RecommendationsAndDecisionPage, TeamAdvicePage from ui_tests.caseworker.pages.case_page import CasePage, CaseTabs from ui_tests.caseworker.pages.teams_pages import TeamsPages @@ -285,7 +283,7 @@ def prepare_case(api_test_client, nlr): # noqa def submit_form(driver): # noqa old_page = driver.find_element(by=By.TAG_NAME, value="html") Shared(driver).click_submit() - WebDriverWait(driver, WebDriverDelay.FORTYFIVE).until(expected_conditions.staleness_of(old_page)) + WebDriverWait(driver, 45 * settings.E2E_WAIT_MULTIPLIER).until(expected_conditions.staleness_of(old_page)) @when(parsers.parse('I click the text "{text}"')) @@ -296,7 +294,7 @@ def click_text(driver, text): # noqa @when(parsers.parse('I click "{button_text}"')) def click_button_with_text(driver, button_text): # noqa - WebDriverWait(driver, WebDriverDelay.TWENTY).until( + WebDriverWait(driver, 20 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located( ( By.XPATH, @@ -397,7 +395,7 @@ def case_list_page(driver, internal_url): # noqa @when("I go to my profile page") # noqa def get_profile_page(driver): # noqa - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, "link-profile")) ).click() @@ -405,7 +403,7 @@ def get_profile_page(driver): # noqa @when(parsers.parse('I change my team to "{team}" and default queue to "{queue}"')) # noqa def go_to_team_edit_page(driver, team, queue): # noqa # we should already be on the profile page - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, "link-edit-team")) ).click() teams_page = TeamsPages(driver) @@ -413,12 +411,12 @@ def go_to_team_edit_page(driver, team, queue): # noqa teams_page.select_default_queue_from_dropdown(queue) functions.click_submit(driver) # Ensure we return to the profile page - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, "link-edit-team")) ) # Check that the team/queue change was applied successfully assert driver.find_element(by=By.ID, value="user-team-name").text == team - WebDriverWait(driver, WebDriverDelay.THIRTY) + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER) assert driver.find_element(by=By.ID, value="user-default-queue").text == queue @@ -440,7 +438,7 @@ def system_queue_shown_in_dropdown(driver, queue_name): # noqa @when(parsers.parse('I switch to "{queue_name}" queue')) # noqa def switch_to_queue(driver, queue_name): # noqa driver.find_element(by=By.ID, value="link-queue").click() - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, "filter-queues")) ) driver.find_element(by=By.ID, value="filter-queues").send_keys(queue_name) @@ -688,7 +686,7 @@ def click_edit_case_flags_link(driver, flag_name): old_page = driver.find_element(by=By.TAG_NAME, value="html") functions.click_submit(driver) - WebDriverWait(driver, WebDriverDelay.FORTYFIVE).until(expected_conditions.staleness_of(old_page)) + WebDriverWait(driver, 45 * settings.E2E_WAIT_MULTIPLIER).until(expected_conditions.staleness_of(old_page)) @given(parsers.parse('the status is set to "{status}"')) # noqa diff --git a/ui_tests/caseworker/pages/BasePage.py b/ui_tests/caseworker/pages/BasePage.py index c221f5553..c35cb4474 100644 --- a/ui_tests/caseworker/pages/BasePage.py +++ b/ui_tests/caseworker/pages/BasePage.py @@ -4,7 +4,7 @@ from selenium.webdriver.support.wait import WebDriverWait from tests_common import functions -from tests_common.constants import WebDriverDelay +from django.conf import settings class BasePage: @@ -13,7 +13,7 @@ def __init__(self, driver: WebDriver): # Wait for the cases list to load before interacting with the page if functions.element_with_id_exists(self.driver, "link-queue"): - WebDriverWait(driver, WebDriverDelay.SIXTY).until( + WebDriverWait(driver, 60 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.visibility_of_element_located((By.ID, "all-cases-tab")) ) diff --git a/ui_tests/caseworker/pages/add_denial_records_page.py b/ui_tests/caseworker/pages/add_denial_records_page.py index 5bcc1e281..6153a2573 100644 --- a/ui_tests/caseworker/pages/add_denial_records_page.py +++ b/ui_tests/caseworker/pages/add_denial_records_page.py @@ -7,7 +7,7 @@ from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.BasePage import BasePage @@ -15,7 +15,7 @@ class AddDenialRecordsPage(BasePage): CSV_FILE_LOCATION = "/tmp/downloads/example-denials.csv" def download_example_csv_file(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.LINK_TEXT, "Download an example .csv file")) ).click() diff --git a/ui_tests/caseworker/pages/application_page.py b/ui_tests/caseworker/pages/application_page.py index e13ed7a02..c3764fbe5 100644 --- a/ui_tests/caseworker/pages/application_page.py +++ b/ui_tests/caseworker/pages/application_page.py @@ -4,7 +4,7 @@ from selenium.webdriver.support.ui import Select from tests_common import functions -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.BasePage import BasePage from tests_common.tools.helpers import scroll_to_element_by_id from tests_common.tools.helpers import scroll_to_element_below_header_by_id @@ -68,7 +68,7 @@ def get_case_copy_of_field_href(self): return self.driver.find_element(by=By.ID, value=self.CASE_COPY_OF_ID).get_attribute("href") def click_visible_to_exporter_checkbox(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, self.IS_VISIBLE_TO_EXPORTER_CHECKBOX_ID)) ).click() @@ -83,7 +83,7 @@ def get_text_of_case_note_field(self): return self.driver.find_element(by=By.ID, value=self.INPUT_CASE_NOTE_ID).text def click_post_note_btn(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located( (By.CSS_SELECTOR, f"#{self.BUTTON_POST_NOTE_ID}:not([disabled])") ) @@ -91,7 +91,7 @@ def click_post_note_btn(self): functions.click_submit(self.driver) def click_cancel_btn(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, self.LINK_CANCEL_NOTE_ID)) ).click() @@ -151,7 +151,7 @@ def select_a_good(self): self.driver.execute_script("arguments[0].click();", element) def click_move_case_button(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, self.MOVE_CASE_BUTTON)) ).click() @@ -242,7 +242,7 @@ def click_assign_user_button(self): self.driver.find_element(by=By.ID, value=self.ASSIGN_USER_ID).click() def click_im_done_button(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, self.BUTTON_IM_DONE_ID)) ).click() @@ -306,7 +306,7 @@ def get_matches(self, match_type): """Return a list of names that have denial matches based on the supplied match_type - one of "PARTIAL MATCH" or "EXACT MATCH". """ - table = WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + table = WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.XPATH, "//table[@id='table-denial-matches']")) ) diff --git a/ui_tests/caseworker/pages/case_list_page.py b/ui_tests/caseworker/pages/case_list_page.py index 2eef7d638..5d4d9ae74 100644 --- a/ui_tests/caseworker/pages/case_list_page.py +++ b/ui_tests/caseworker/pages/case_list_page.py @@ -4,7 +4,7 @@ from selenium.webdriver.support.wait import WebDriverWait from selenium.webdriver.support.select import Select -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.BasePage import BasePage from ui_tests.caseworker.pages.shared import Shared from tests_common import functions @@ -155,7 +155,7 @@ def click_on_queue_title(self): self.driver.find_element(by=By.ID, value=self.LINK_CHANGE_QUEUE_ID).click() def search_for_queue(self, queue_name): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, self.QUEUE_SEARCH_BOX)) ) self.driver.find_element(by=By.ID, value=self.QUEUE_SEARCH_BOX).send_keys(queue_name) @@ -217,6 +217,6 @@ def click_checkbox_to_show_team_ecju_query_and_hidden_cases(self): return self.driver.find_element(by=By.ID, value=self.SHOW_TEAM_ECJU_AND_HIDDEN_CASES).click() def click_export_enforcement_xml(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, self.EXPORT_ENFORCEMENT_XML_BUTTON_ID)) ).click() diff --git a/ui_tests/caseworker/pages/case_page.py b/ui_tests/caseworker/pages/case_page.py index 2e43ff3e9..d3b205eaa 100644 --- a/ui_tests/caseworker/pages/case_page.py +++ b/ui_tests/caseworker/pages/case_page.py @@ -1,7 +1,7 @@ from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.shared import Shared from ui_tests.caseworker.pages.BasePage import BasePage from tests_common import selectors @@ -129,7 +129,7 @@ def is_flag_applied(self, flag_name): self.driver.find_element(by=By.ID, value="candy-flags").click() - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, POPUP_FLAGS_ID)) ) diff --git a/ui_tests/caseworker/pages/header_page.py b/ui_tests/caseworker/pages/header_page.py index 461c6609a..65330c08a 100644 --- a/ui_tests/caseworker/pages/header_page.py +++ b/ui_tests/caseworker/pages/header_page.py @@ -1,4 +1,4 @@ -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.BasePage import BasePage from tests_common.tools.wait import wait_until_page_is_loaded @@ -22,7 +22,7 @@ def click_lite_menu(self): self.driver.find_element(by=By.ID, value=self.MENU_BUTTON).click() def click_organisations(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.CSS_SELECTOR, self.ORGANISATIONS_LINK)) ).click() diff --git a/ui_tests/caseworker/pages/product_assessment.py b/ui_tests/caseworker/pages/product_assessment.py index 421468a29..afdc9e052 100644 --- a/ui_tests/caseworker/pages/product_assessment.py +++ b/ui_tests/caseworker/pages/product_assessment.py @@ -4,7 +4,7 @@ from selenium.webdriver.support.wait import WebDriverWait from tests_common import functions -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.BasePage import BasePage @@ -24,7 +24,7 @@ def assess_report_summary_prefix(self, ars_prefix): ars_prefix_element = self.driver.find_element(by=By.ID, value="report_summary_prefix_container") ars_prefix_input = ars_prefix_element.find_element(by=By.ID, value="_report_summary_prefix") ars_prefix_input.send_keys(ars_prefix) - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, "_report_summary_prefix__listbox")) ) @@ -40,7 +40,7 @@ def assess_report_summary_subject(self, ars_subject): ars_subject_element = self.driver.find_element(by=By.ID, value="report_summary_subject_container") ars_subject_input = ars_subject_element.find_element(by=By.ID, value="_report_summary_subject") ars_subject_input.send_keys(ars_subject) - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, "_report_summary_subject__listbox")) ) diff --git a/ui_tests/caseworker/pages/shared.py b/ui_tests/caseworker/pages/shared.py index 7b42939fd..d651f6667 100644 --- a/ui_tests/caseworker/pages/shared.py +++ b/ui_tests/caseworker/pages/shared.py @@ -3,7 +3,7 @@ from selenium.webdriver.support.wait import WebDriverWait from tests_common import functions -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.BasePage import BasePage from tests_common.tools.helpers import scroll_to_element_by_id @@ -123,14 +123,14 @@ def click_first_link_in_row(self): def expand_govuk_details(self): self.driver.find_element(by=By.CLASS_NAME, value=self.GOVUK_DETAILS).click() - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located( (By.XPATH, f"//*[contains(@class, '{self.GOVUK_DETAILS}') and ancestor::details/@open]") ) ) def try_open_filters(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.CLASS_NAME, "lite-filter-bar")) ) diff --git a/ui_tests/caseworker/step_defs/test_documents.py b/ui_tests/caseworker/step_defs/test_documents.py index 3f7be003e..958130176 100644 --- a/ui_tests/caseworker/step_defs/test_documents.py +++ b/ui_tests/caseworker/step_defs/test_documents.py @@ -3,7 +3,7 @@ from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.application_page import ApplicationPage from ui_tests.caseworker.pages.attach_document_page import AttachDocumentPage from ui_tests.caseworker.pages.documents_page import DocumentsPage @@ -29,7 +29,7 @@ def upload_a_file(driver, filename, description, tmp_path): old_page = driver.find_element(by=By.TAG_NAME, value="html") attach_document_page.click_submit_btn() - WebDriverWait(driver, WebDriverDelay.FORTYFIVE).until(expected_conditions.staleness_of(old_page)) + WebDriverWait(driver, 45 * settings.E2E_WAIT_MULTIPLIER).until(expected_conditions.staleness_of(old_page)) @then(parsers.parse('I see a file with filename "{filename}" is uploaded')) diff --git a/ui_tests/caseworker/step_defs/test_enforcement.py b/ui_tests/caseworker/step_defs/test_enforcement.py index 49bf9cda0..823f54f8e 100644 --- a/ui_tests/caseworker/step_defs/test_enforcement.py +++ b/ui_tests/caseworker/step_defs/test_enforcement.py @@ -8,7 +8,7 @@ from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.application_page import ApplicationPage from ui_tests.caseworker.pages.case_list_page import CaseListPage from ui_tests.caseworker.pages.shared import Shared @@ -147,9 +147,9 @@ def i_attach_updated_file(driver, enforcement_check_import_xml_file_path): # no upload_btn = driver.find_element(by=By.XPATH, value="//button[@type='submit']") upload_btn.click() - WebDriverWait(driver, WebDriverDelay.THIRTY).until(expected_conditions.staleness_of(old_page)) + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until(expected_conditions.staleness_of(old_page)) - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.text_to_be_present_in_element( (By.CSS_SELECTOR, ".app-snackbar__content"), "Enforcement XML imported successfully", @@ -162,7 +162,7 @@ def i_attach_updated_file(driver, enforcement_check_import_xml_file_path): # no @then(parsers.parse('the application is removed from "{queue}" queue')) def application_removed_from_queue(driver, queue): ASSIGNED_QUEUES_ID = "assigned-queues" - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, ASSIGNED_QUEUES_ID)) ).is_enabled() queue_list = driver.find_element(by=By.ID, value=ASSIGNED_QUEUES_ID).text.split("\n") diff --git a/ui_tests/caseworker/step_defs/test_product_search.py b/ui_tests/caseworker/step_defs/test_product_search.py index 009f293d1..9f505a6dd 100644 --- a/ui_tests/caseworker/step_defs/test_product_search.py +++ b/ui_tests/caseworker/step_defs/test_product_search.py @@ -8,7 +8,7 @@ from selenium.webdriver.support.wait import WebDriverWait from tests_common import functions -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.caseworker.pages.product_search import ProductSearchPage @@ -17,7 +17,7 @@ @when("I go to product search page") def go_to_product_search_page(driver): - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, "link-product-search")) ).click() diff --git a/ui_tests/exporter/conftest.py b/ui_tests/exporter/conftest.py index 5f55dcf9d..d0b454e28 100644 --- a/ui_tests/exporter/conftest.py +++ b/ui_tests/exporter/conftest.py @@ -10,7 +10,6 @@ from selenium.webdriver.support.wait import WebDriverWait from selenium.common.exceptions import NoSuchElementException -from tests_common.constants import WebDriverDelay import tests_common.tools.helpers as utils from ui_tests.exporter.fixtures.add_end_user_advisory import add_end_user_advisory # noqa from ui_tests.exporter.fixtures.add_party import add_end_user_to_application # noqa @@ -1243,7 +1242,7 @@ def edit_good_details_in_application(driver, field_name, updated_value): # noqa @when(parsers.parse('I click on "{link_text}"')) # noqa def click_link_with_text(driver, link_text): # noqa - WebDriverWait(driver, WebDriverDelay.THIRTY).until( + WebDriverWait(driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located( ( By.LINK_TEXT, diff --git a/ui_tests/exporter/pages/shared.py b/ui_tests/exporter/pages/shared.py index 444fc2e2d..ba9a9646a 100644 --- a/ui_tests/exporter/pages/shared.py +++ b/ui_tests/exporter/pages/shared.py @@ -1,4 +1,4 @@ -from tests_common.constants import WebDriverDelay +from django.conf import settings from ui_tests.exporter.pages.BasePage import BasePage from selenium.webdriver.common.by import By @@ -34,7 +34,7 @@ def get_table_rows(self): return self.driver.find_elements_by_css_selector(self.GOV_TABLE_ROW) def get_text_of_organisation_heading(self): - WebDriverWait(self.driver, WebDriverDelay.THIRTY).until( + WebDriverWait(self.driver, 30 * settings.E2E_WAIT_MULTIPLIER).until( expected_conditions.presence_of_element_located((By.ID, self.ORG_NAME_HEADING_ID)) ) From 60cf760a7414c0453777044cbaf4f7010a2787cb Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Wed, 8 Jan 2025 09:42:48 +0000 Subject: [PATCH 11/16] accidental removal --- ui_tests/caseworker/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ui_tests/caseworker/conftest.py b/ui_tests/caseworker/conftest.py index 41e811801..20dcdd35a 100644 --- a/ui_tests/caseworker/conftest.py +++ b/ui_tests/caseworker/conftest.py @@ -4,6 +4,7 @@ from pytest_bdd import given, when, then, parsers from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions +from selenium.webdriver.support.wait import WebDriverWait from core.constants import CaseStatusEnum from ui_tests.caseworker.pages.advice import FinalAdvicePage, RecommendationsAndDecisionPage, TeamAdvicePage From 293814a444887808a04a8854603e9cf5666bff9c Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Wed, 8 Jan 2025 10:04:06 +0000 Subject: [PATCH 12/16] update --- tests_common/functions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests_common/functions.py b/tests_common/functions.py index 783649d7c..c4ed6b12c 100644 --- a/tests_common/functions.py +++ b/tests_common/functions.py @@ -7,6 +7,7 @@ from selenium.webdriver.remote.webdriver import WebDriver from selenium.webdriver.support import expected_conditions from selenium.webdriver.support.wait import WebDriverWait +from django.conf import settings def click_submit(driver: WebDriver): From 7e5e71d87868b9870851b9c72cb437d0eeb4cf42 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Wed, 8 Jan 2025 14:16:24 +0000 Subject: [PATCH 13/16] re added name to approval_reasons widget --- caseworker/advice/forms/approval.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/caseworker/advice/forms/approval.py b/caseworker/advice/forms/approval.py index 227910008..9a9a5fbeb 100644 --- a/caseworker/advice/forms/approval.py +++ b/caseworker/advice/forms/approval.py @@ -40,7 +40,7 @@ class Layout: TITLE = "Recommend an approval" approval_reasons = forms.CharField( - widget=forms.Textarea(attrs={"rows": 7, "class": "govuk-!-margin-top-4"}), + widget=forms.Textarea(attrs={"rows": 7, "class": "govuk-!-margin-top-4", "name": "approval_reasons"}), label="", error_messages={"required": "Enter a reason for approving"}, ) From 5868c36209f8be24f55be80aaf76a8c69e384a5c Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 9 Jan 2025 10:20:47 +0000 Subject: [PATCH 14/16] fix e2e --- .../give_advice/lu_consolidate_advice.feature | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/ui_tests/caseworker/features/give_advice/lu_consolidate_advice.feature b/ui_tests/caseworker/features/give_advice/lu_consolidate_advice.feature index 25b69321f..ef6b7e69d 100644 --- a/ui_tests/caseworker/features/give_advice/lu_consolidate_advice.feature +++ b/ui_tests/caseworker/features/give_advice/lu_consolidate_advice.feature @@ -99,17 +99,20 @@ Feature: I want to record my user advice and any comments and conditions relatin And I click make recommendation And I click approve all And I click continue - And I enter "reason for approving" as the reasons for approving - And I click the text "Add a licence condition, instruction to exporter or footnote" - And I enter "MOD licence condition" as the licence condition - And I enter "instruction for exporter" as the instructions for the exporter - And I enter "reporting footnote" as the reporting footnote + And I enter "reason for approving" as the approval reasons + And I click add licence condition + And I click continue + And I enter "MOD licence condition" as the licence condition into the "other" checkbox + And I click continue + And I enter "instruction for exporter" as the instructions for the exporter on the instructions step + And I enter "reporting footnote" as the reporting footnote on the instructions step And I click submit recommendation Then I see "reason for approving" as the reasons for approving And I see "MOD licence condition" as the licence condition And I see "instruction for exporter" as the instructions for the exporter And I see "reporting footnote" as the reporting footnote + ##### FCDO Sub-advisor to give advice ##### When I go to my profile page And I change my team to "FCDO" and default queue to "FCDO Cases to Review" From 3156492d5394124ebf3e215bea1bb8a792deb963 Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Thu, 19 Dec 2024 10:29:55 +0000 Subject: [PATCH 15/16] Ensure checkbox licence conditions collapse to single textbox when there are no picklist conditions --- caseworker/advice/forms/approval.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/caseworker/advice/forms/approval.py b/caseworker/advice/forms/approval.py index 9a9a5fbeb..d43be45a7 100644 --- a/caseworker/advice/forms/approval.py +++ b/caseworker/advice/forms/approval.py @@ -70,7 +70,7 @@ def get_layout_fields(self): ) -class LicenceConditionsForm(PicklistAdviceForm, BaseForm): +class PicklistLicenceConditionsForm(PicklistAdviceForm, BaseForm): class Layout: TITLE = "Add licence conditions (optional)" @@ -119,7 +119,7 @@ class Layout: TITLE = "Add licence conditions (optional)" proviso = forms.CharField( - widget=forms.Textarea(attrs={"rows": 7}), + widget=forms.Textarea(attrs={"rows": 7, "class": "govuk-!-margin-top-4"}), label="Licence condition", required=False, ) From 5053276a348cd16bcfdb8e3a7748d82587c17fc9 Mon Sep 17 00:00:00 2001 From: Tomos Williams Date: Thu, 9 Jan 2025 16:20:20 +0000 Subject: [PATCH 16/16] Remove unecessary margin styling updating to reflect simple form actually saving fix e2e --- caseworker/advice/forms/approval.py | 2 +- .../features/give_advice/lu_consolidate_advice.feature | 2 +- .../caseworker/features/give_advice/mod_approve_advice.feature | 2 +- .../caseworker/features/give_advice/mod_clear_advice.feature | 2 +- .../caseworker/features/give_advice/mod_edit_advice.feature | 2 +- .../caseworker/features/give_advice/ogd_approve_advice.feature | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/caseworker/advice/forms/approval.py b/caseworker/advice/forms/approval.py index d43be45a7..85eacf507 100644 --- a/caseworker/advice/forms/approval.py +++ b/caseworker/advice/forms/approval.py @@ -119,7 +119,7 @@ class Layout: TITLE = "Add licence conditions (optional)" proviso = forms.CharField( - widget=forms.Textarea(attrs={"rows": 7, "class": "govuk-!-margin-top-4"}), + widget=forms.Textarea(attrs={"rows": 7}), label="Licence condition", required=False, ) diff --git a/ui_tests/caseworker/features/give_advice/lu_consolidate_advice.feature b/ui_tests/caseworker/features/give_advice/lu_consolidate_advice.feature index ef6b7e69d..72755b343 100644 --- a/ui_tests/caseworker/features/give_advice/lu_consolidate_advice.feature +++ b/ui_tests/caseworker/features/give_advice/lu_consolidate_advice.feature @@ -102,7 +102,7 @@ Feature: I want to record my user advice and any comments and conditions relatin And I enter "reason for approving" as the approval reasons And I click add licence condition And I click continue - And I enter "MOD licence condition" as the licence condition into the "other" checkbox + And I enter "MOD licence condition" into the licence condition And I click continue And I enter "instruction for exporter" as the instructions for the exporter on the instructions step And I enter "reporting footnote" as the reporting footnote on the instructions step diff --git a/ui_tests/caseworker/features/give_advice/mod_approve_advice.feature b/ui_tests/caseworker/features/give_advice/mod_approve_advice.feature index 0fe18d297..6cc069565 100644 --- a/ui_tests/caseworker/features/give_advice/mod_approve_advice.feature +++ b/ui_tests/caseworker/features/give_advice/mod_approve_advice.feature @@ -54,7 +54,7 @@ Feature: I want to record my user advice and any comments and conditions relatin And I enter "reason for approving" as the approval reasons And I click add licence condition And I click continue - And I enter "licence condition" as the licence condition into the "other" checkbox + And I enter "licence condition" into the licence condition And I click continue And I enter "instruction for exporter" as the instructions for the exporter on the instructions step And I enter "reporting footnote" as the reporting footnote on the instructions step diff --git a/ui_tests/caseworker/features/give_advice/mod_clear_advice.feature b/ui_tests/caseworker/features/give_advice/mod_clear_advice.feature index 693be8e9a..39e813b96 100644 --- a/ui_tests/caseworker/features/give_advice/mod_clear_advice.feature +++ b/ui_tests/caseworker/features/give_advice/mod_clear_advice.feature @@ -22,7 +22,7 @@ Feature: I want to record my user advice and any comments and conditions relatin And I enter "approval reason" as the approval reasons And I click add licence condition And I click continue - And I enter "licence condition" as the licence condition into the "other" checkbox + And I enter "licence condition" into the licence condition And I click continue And I enter "instruction for exporter" as the instructions for the exporter on the instructions step And I enter "reporting footnote" as the reporting footnote on the instructions step diff --git a/ui_tests/caseworker/features/give_advice/mod_edit_advice.feature b/ui_tests/caseworker/features/give_advice/mod_edit_advice.feature index 1152ed597..9de6d7340 100644 --- a/ui_tests/caseworker/features/give_advice/mod_edit_advice.feature +++ b/ui_tests/caseworker/features/give_advice/mod_edit_advice.feature @@ -22,7 +22,7 @@ Feature: I want to record my user advice and any comments and conditions relatin And I enter "approval reason" as the approval reasons And I click add licence condition And I click continue - And I enter "licence condition" as the licence condition into the "other" checkbox + And I enter "licence condition" into the licence condition And I click continue And I enter "instruction for exporter" as the instructions for the exporter on the instructions step And I enter "reporting footnote" as the reporting footnote on the instructions step diff --git a/ui_tests/caseworker/features/give_advice/ogd_approve_advice.feature b/ui_tests/caseworker/features/give_advice/ogd_approve_advice.feature index d7e4c8d69..f7a13d960 100644 --- a/ui_tests/caseworker/features/give_advice/ogd_approve_advice.feature +++ b/ui_tests/caseworker/features/give_advice/ogd_approve_advice.feature @@ -22,7 +22,7 @@ Feature: I want to record my user advice and any comments and conditions relatin And I enter "Hello World" as the approval reasons And I click add licence condition And I click continue - And I enter "licence condition" as the licence condition into the "other" checkbox + And I enter "licence condition" into the licence condition And I click continue And I enter "instruction for exporter" as the instructions for the exporter on the instructions step And I enter "reporting footnote" as the reporting footnote on the instructions step