From ab4300a72a2141d9c3b345431d7c8284fa2dcff0 Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Mon, 2 Dec 2024 11:01:56 +0000 Subject: [PATCH 1/9] Ensure unit tests can be added alongside caseworker code instead of in unit_tests directory --- .circleci/config.yml | 2 +- caseworker/conftest.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 caseworker/conftest.py diff --git a/.circleci/config.yml b/.circleci/config.yml index 921b63e0c..d69e8d563 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -236,7 +236,7 @@ jobs: environment: <<: *common_env_vars PIPENV_DOTENV_LOCATION: tests.caseworker.env - PYTEST_ADDOPTS: unit_tests/caseworker lite_forms/tests.py --capture=no --nomigrations + PYTEST_ADDOPTS: caseworker unit_tests/caseworker lite_forms/tests.py --capture=no --nomigrations FILE_UPLOAD_HANDLERS: django.core.files.uploadhandler.MemoryFileUploadHandler,django.core.files.uploadhandler.TemporaryFileUploadHandler steps: - backend_unit_tests: diff --git a/caseworker/conftest.py b/caseworker/conftest.py new file mode 100644 index 000000000..e9ffc9cd3 --- /dev/null +++ b/caseworker/conftest.py @@ -0,0 +1,3 @@ +# pylint: disable=unused-wildcard-import,wildcard-import +from unit_tests.conftest import * +from unit_tests.caseworker.conftest import * From e0cd7c561967bc55802c49036633742ebb911d6c Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Mon, 2 Dec 2024 11:07:57 +0000 Subject: [PATCH 2/9] Ensure LU consolidation applies to all OGDs --- caseworker/advice/services.py | 37 +++-- .../templates/advice/advice_details.html | 4 +- caseworker/advice/tests/test_services.py | 149 ++++++++++++++++++ pyproject.toml | 1 + 4 files changed, 176 insertions(+), 15 deletions(-) create mode 100644 caseworker/advice/tests/test_services.py diff --git a/caseworker/advice/services.py b/caseworker/advice/services.py index 40342f407..921576928 100644 --- a/caseworker/advice/services.py +++ b/caseworker/advice/services.py @@ -40,21 +40,27 @@ FCDO_TEAM = "FCO" LICENSING_UNIT_TEAM = "LICENSING_UNIT" MOD_ECJU_TEAM = "MOD_ECJU" +MOD_DI_TEAM = "MOD_DI" +MOD_DSR_TEAM = "MOD_DSR" +MOD_DSTL_TEAM = "MOD_DSTL" +MOD_CAPPROT_TEAM = "MOD_CAPPROT" MOD_CONSOLIDATE_TEAMS = [ - "MOD_DI", - "MOD_DSR", - "MOD_DSTL", - "MOD_CAPPROT", + MOD_DI_TEAM, + MOD_DSR_TEAM, + MOD_DSTL_TEAM, + MOD_CAPPROT_TEAM, ] MOD_TEAMS = [MOD_ECJU_TEAM, *MOD_CONSOLIDATE_TEAMS] -LU_CONSOLIDATE_TEAMS = [FCDO_TEAM, MOD_ECJU_TEAM] NCSC_TEAM = "NCSC" -OGD_TEAMS = [ +OGD_TEAMS_EXCLUDING_MOD = [ *DESNZ_TEAMS, FCDO_TEAM, - *MOD_TEAMS, NCSC_TEAM, ] +OGD_TEAMS = [ + *OGD_TEAMS_EXCLUDING_MOD, + *MOD_TEAMS, +] # Flags LU_COUNTERSIGN_REQUIRED_ID = "bbf29b42-0aae-4ebc-b77a-e502ddea30a8" # /PS-IGNORE @@ -180,11 +186,12 @@ def group_advice_by_user(advice): return result -def group_advice_by_team(advice): +def group_advice_by_team(advice, exclude_good_advice=True): result = defaultdict(list) for item in advice: - if not item.get("good"): - result[item["team"]["id"]].append(item) + if exclude_good_advice and item.get("good"): + continue + result[item["team"]["id"]].append(item) return result @@ -269,16 +276,20 @@ def get_advice_to_consolidate(advice, user_team_alias): """ if user_team_alias == LICENSING_UNIT_TEAM: - # LU needs to review the consolidated advice given by MOD which is at team level user_team_advice = filter_advice_by_level(advice, [constants.AdviceLevel.USER, constants.AdviceLevel.TEAM]) - advice_from_teams = filter_advice_by_teams(user_team_advice, LU_CONSOLIDATE_TEAMS) + lu_consolidate_teams = OGD_TEAMS.copy() + mod_ecju_in_advice = any(advice["team"]["alias"] == MOD_ECJU_TEAM for advice in user_team_advice) + # If MOD ECJU advice is present, this will supersede all MOD advice + if mod_ecju_in_advice: + lu_consolidate_teams = [*OGD_TEAMS_EXCLUDING_MOD, MOD_ECJU_TEAM] + advice_from_teams = filter_advice_by_teams(user_team_advice, lu_consolidate_teams) elif user_team_alias == MOD_ECJU_TEAM: user_advice = filter_advice_by_level(advice, [constants.AdviceLevel.USER]) advice_from_teams = filter_advice_by_teams(user_advice, MOD_CONSOLIDATE_TEAMS) else: raise Exception(f"Consolidate/combine operation not allowed for team {user_team_alias}") - return group_advice_by_user(advice_from_teams) + return group_advice_by_team(advice_from_teams, exclude_good_advice=False) def order_by_party_type(all_advice): diff --git a/caseworker/advice/templates/advice/advice_details.html b/caseworker/advice/templates/advice/advice_details.html index 6a31e35d2..88da2ba70 100644 --- a/caseworker/advice/templates/advice/advice_details.html +++ b/caseworker/advice/templates/advice/advice_details.html @@ -4,12 +4,12 @@ {% if decision == 'approve' or decision == 'proviso' %}

- {% if team %} Approved by {{ user.team.name }} {% else %} Approved by {{ user|full_name }} {% endif %} + {% if team %} Approved by {{ advice.0.team.name }} {% else %} Approved by {{ user|full_name }} {% endif %} {{ advice.0.created_at|parse_date|date:"d F Y" }}

{% elif decision == 'refuse' %}

- {% if team %} Refused by {{ user.team.name }} {% else %} Refused by {{ user|full_name }} {% endif %} + {% if team %} Refused by {{ advice.0.team.name }} {% else %} Refused by {{ user|full_name }} {% endif %}

{% endif %} diff --git a/caseworker/advice/tests/test_services.py b/caseworker/advice/tests/test_services.py new file mode 100644 index 000000000..93d86c45e --- /dev/null +++ b/caseworker/advice/tests/test_services.py @@ -0,0 +1,149 @@ +import pytest + +from caseworker.advice.services import ( + get_advice_to_consolidate, + LICENSING_UNIT_TEAM, + FCDO_TEAM, + NCSC_TEAM, + DESNZ_CHEMICAL, + DESNZ_NUCLEAR, + MOD_DI_TEAM, + MOD_DSR_TEAM, + MOD_DSTL_TEAM, + MOD_CAPPROT_TEAM, + MOD_ECJU_TEAM, +) + + +def test_get_advice_to_consolidate_unrecognized_team_raises_exception(): + with pytest.raises(Exception): + get_advice_to_consolidate([], "some unknown team") + + +@pytest.mark.parametrize( + "advice, expected_grouping", + ( + # No advice to group + ([], {}), + # FCDO advice only + ( + [ + {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + ], + {"id-fcdo": [{"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}]}, + ), + # FCDO and MOD-DI advice only + ( + [ + {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-3", "level": "team", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + ], + { + "id-fcdo": [ + {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + ], + "id-mod-di": [ + {"id": "advice-3", "level": "team", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + ], + }, + ), + # All OGDs advising with MOD collected under MOD-ECJU + ( + [ + {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-3", "level": "team", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + {"id": "advice-4", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + {"id": "advice-5", "level": "user", "team": {"alias": DESNZ_NUCLEAR, "id": "id-desnz-nuclear"}}, + {"id": "advice-6", "level": "user", "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}}, + {"id": "advice-7", "level": "user", "team": {"alias": NCSC_TEAM, "id": "id-ncsc"}}, + {"id": "advice-8", "level": "user", "team": {"alias": MOD_CAPPROT_TEAM, "id": "id-mod-capprot"}}, + {"id": "advice-9", "level": "user", "team": {"alias": MOD_DSR_TEAM, "id": "id-mod-dsr"}}, + {"id": "advice-10", "level": "user", "team": {"alias": MOD_DSTL_TEAM, "id": "id-mod-dstl"}}, + {"id": "advice-11", "level": "team", "team": {"alias": MOD_ECJU_TEAM, "id": "id-mod-ecju"}}, + ], + { + "id-fcdo": [ + {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + ], + "id-desnz-nuclear": [ + {"id": "advice-5", "level": "user", "team": {"alias": DESNZ_NUCLEAR, "id": "id-desnz-nuclear"}}, + ], + "id-desnz-chemical": [ + {"id": "advice-6", "level": "user", "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}}, + ], + "id-ncsc": [ + {"id": "advice-7", "level": "user", "team": {"alias": NCSC_TEAM, "id": "id-ncsc"}}, + ], + "id-mod-ecju": [ + {"id": "advice-11", "level": "team", "team": {"alias": MOD_ECJU_TEAM, "id": "id-mod-ecju"}}, + ], + }, + ), + ), +) +def test_get_advice_to_consolidate_lu(advice, expected_grouping): + assert get_advice_to_consolidate(advice, LICENSING_UNIT_TEAM) == expected_grouping + + +@pytest.mark.parametrize( + "advice, expected_grouping", + ( + # No advice to group + ([], {}), + # FCDO advice only + ( + [ + {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + ], + {}, + ), + # FCDO and MOD-DI advice only + ( + [ + {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-3", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + ], + { + "id-mod-di": [ + {"id": "advice-3", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + ] + }, + ), + # All OGDs advising with MOD collected under MOD-ECJU + ( + [ + {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + {"id": "advice-3", "level": "team", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + {"id": "advice-4", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + {"id": "advice-5", "level": "user", "team": {"alias": DESNZ_NUCLEAR, "id": "id-desnz-nuclear"}}, + {"id": "advice-6", "level": "user", "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}}, + {"id": "advice-7", "level": "user", "team": {"alias": NCSC_TEAM, "id": "id-ncsc"}}, + {"id": "advice-8", "level": "user", "team": {"alias": MOD_CAPPROT_TEAM, "id": "id-mod-capprot"}}, + {"id": "advice-9", "level": "user", "team": {"alias": MOD_DSR_TEAM, "id": "id-mod-dsr"}}, + {"id": "advice-10", "level": "user", "team": {"alias": MOD_DSTL_TEAM, "id": "id-mod-dstl"}}, + ], + { + "id-mod-di": [ + {"id": "advice-4", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + ], + "id-mod-capprot": [ + {"id": "advice-8", "level": "user", "team": {"alias": MOD_CAPPROT_TEAM, "id": "id-mod-capprot"}}, + ], + "id-mod-dsr": [ + {"id": "advice-9", "level": "user", "team": {"alias": MOD_DSR_TEAM, "id": "id-mod-dsr"}}, + ], + "id-mod-dstl": [ + {"id": "advice-10", "level": "user", "team": {"alias": MOD_DSTL_TEAM, "id": "id-mod-dstl"}}, + ], + }, + ), + ), +) +def test_get_advice_to_consolidate_mod_ecju(advice, expected_grouping): + assert get_advice_to_consolidate(advice, MOD_ECJU_TEAM) == expected_grouping diff --git a/pyproject.toml b/pyproject.toml index 6cfcbb95e..70a9eac5f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,4 +27,5 @@ select = ["F401", "S", "G", "LOG"] [tool.ruff.lint.per-file-ignores] "./{unit_tests,ui_tests,tests_common}/*" = ["S101", "S311", "S108", "S314"] +"*/tests/*" = ["S101"] "./lite_forms/tests.py" = ["S101"] From 491101c9686933993c042713ca822183339ee1a1 Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Mon, 2 Dec 2024 17:48:29 +0000 Subject: [PATCH 3/9] Break decision selection out of ReviewConsolidateView --- caseworker/advice/urls.py | 6 +- caseworker/advice/views/__init__.py | 0 caseworker/advice/views/consolidate_advice.py | 61 +++++ caseworker/advice/views/tests/__init__.py | 0 .../views/tests/test_consolidate_advice.py | 237 ++++++++++++++++++ caseworker/advice/{ => views}/views.py | 15 +- unit_tests/caseworker/advice/test_services.py | 4 +- .../advice/views/test_consolidate.py | 3 - 8 files changed, 307 insertions(+), 19 deletions(-) create mode 100644 caseworker/advice/views/__init__.py create mode 100644 caseworker/advice/views/consolidate_advice.py create mode 100644 caseworker/advice/views/tests/__init__.py create mode 100644 caseworker/advice/views/tests/test_consolidate_advice.py rename caseworker/advice/{ => views}/views.py (98%) diff --git a/caseworker/advice/urls.py b/caseworker/advice/urls.py index 4e443631b..8677dec6e 100644 --- a/caseworker/advice/urls.py +++ b/caseworker/advice/urls.py @@ -1,6 +1,6 @@ from django.urls import path -from caseworker.advice import views +from caseworker.advice.views import views, consolidate_advice urlpatterns = [ path("", views.AdviceView.as_view(), name="advice_view"), @@ -28,8 +28,8 @@ name="countersign_decision_edit", ), path("consolidate/", views.ConsolidateAdviceView.as_view(), name="consolidate_advice_view"), - path("consolidate/review//", views.ReviewConsolidateView.as_view()), - path("consolidate/review/", views.ReviewConsolidateView.as_view(), name="consolidate_review"), + path("consolidate/review//", views.ReviewConsolidateView.as_view(), name="consolidate"), + path("consolidate/review/", consolidate_advice.ConsolidateSelectDecisionView.as_view(), name="consolidate_review"), path("consolidate/edit/", views.ConsolidateEditView.as_view(), name="consolidate_edit"), path("consolidate/view-advice/", views.ViewConsolidatedAdviceView.as_view(), name="consolidate_view"), path("assess-trigger-list-products/", views.DESNZProductAssessmentView.as_view(), name="assess_trigger_list"), diff --git a/caseworker/advice/views/__init__.py b/caseworker/advice/views/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/caseworker/advice/views/consolidate_advice.py b/caseworker/advice/views/consolidate_advice.py new file mode 100644 index 000000000..5b2b5aba4 --- /dev/null +++ b/caseworker/advice/views/consolidate_advice.py @@ -0,0 +1,61 @@ +from django.views.generic import FormView +from django.shortcuts import redirect +from django.urls import reverse + +from core.auth.views import LoginRequiredMixin + +from caseworker.advice import forms +from caseworker.advice.views.views import CaseContextMixin +from caseworker.advice import services + + +class BaseConsolidationView(LoginRequiredMixin, CaseContextMixin, FormView): + + def get_title(self): + return f"Review and combine case recommendation - {self.case.reference_code} - {self.case.organisation['name']}" + + def get_context(self, **kwargs): + context = super().get_context() + team_alias = ( + self.caseworker["team"]["alias"] if self.caseworker["team"]["alias"] else self.caseworker["team"]["id"] + ) + advice_to_consolidate = services.get_advice_to_consolidate(self.case.advice, team_alias) + context["advice_to_consolidate"] = list(advice_to_consolidate.values()) + context["denial_reasons_display"] = self.denial_reasons_display + context["security_approvals_classified_display"] = self.security_approvals_classified_display + context["title"] = self.get_title() + return context + + +class ConsolidateSelectDecisionView(BaseConsolidationView): + """ + Initial selection of consolidated decision between approve and refuse. + This will redirect to the consolidate approval or consolidate refusal flows as appropriate. + """ + + template_name = "advice/review_consolidate.html" + form_class = forms.ConsolidateSelectAdviceForm + + def dispatch(self, request, *args, **kwargs): + approve_advice_types = ("approve", "proviso", "no_licence_required") + is_all_advice_approval = all(a["type"]["key"] in approve_advice_types for a in self.case.advice) + if is_all_advice_approval: + self.decision = "approve" + return redirect(self.get_success_url()) + return super().dispatch(request, *args, **kwargs) + + def get_form_kwargs(self): + form_kwargs = super().get_form_kwargs() + team_name = self.caseworker["team"]["name"] + form_kwargs.update({"team_name": team_name}) + return form_kwargs + + def form_valid(self, form): + self.decision = form.cleaned_data["recommendation"] + return super().form_valid(form) + + def get_success_url(self): + return reverse( + "cases:consolidate", + kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"], "advice_type": self.decision}, + ) diff --git a/caseworker/advice/views/tests/__init__.py b/caseworker/advice/views/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/caseworker/advice/views/tests/test_consolidate_advice.py b/caseworker/advice/views/tests/test_consolidate_advice.py new file mode 100644 index 000000000..4d2f3e06c --- /dev/null +++ b/caseworker/advice/views/tests/test_consolidate_advice.py @@ -0,0 +1,237 @@ +import pytest + +from django.urls import reverse + +from core import client +from caseworker.advice.services import ( + LICENSING_UNIT_TEAM, + MOD_ECJU_TEAM, +) + + +@pytest.fixture(autouse=True) +def setup( + mock_queue, + mock_case, + mock_approval_reason, + mock_denial_reasons, + mock_proviso, + mock_footnote_details, + mock_finalise_advice_documents, +): + return + + +@pytest.fixture +def consolidate_select_decision_url(data_queue, data_standard_case): + return reverse( + "cases:consolidate_review", kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]} + ) + + +@pytest.fixture +def consolidate_approve_url(data_queue, data_standard_case): + return reverse( + "cases:consolidate", + kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"], "advice_type": "approve"}, + ) + + +@pytest.fixture +def consolidate_refuse_url(data_queue, data_standard_case): + return reverse( + "cases:consolidate", + kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"], "advice_type": "refuse"}, + ) + + +@pytest.fixture +def advice_data(current_user, admin_team): + return { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE + "country": None, + "created_at": "2021-10-16T23:48:39.486679+01:00", + "denial_reasons": [], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote": "footnotes", + "good": None, + "id": "429c5596-fe8b-4540-988b-c37805cd08de", # /PS-IGNORE + "level": "user", + "note": "additional notes", + "proviso": "no conditions", + "text": "meets the criteria", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": {"key": "approve", "value": "Approve"}, + "ultimate_end_user": None, + "user": current_user, + "team": admin_team, + } + + +@pytest.fixture +def approval_advice(advice_data): + return [ + {**advice_data.copy(), "good": good_id} + for good_id in ("0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", "6daad1c3-cf97-4aad-b711-d5c9a9f4586e") + ] + + +@pytest.fixture +def mixed_advice(advice_data): + return [ + { + **advice_data.copy(), + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "type": {"key": "refuse", "value": "Refuse"}, + }, + { + **advice_data.copy(), + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "type": {"key": "approve", "value": "Approve"}, + }, + ] + + +@pytest.fixture +def gov_user(): + return { + "user": { + "id": "2a43805b-c082-47e7-9188-c8b3e1a83cb0", # /PS-IGNORE + "team": { + "id": "211111b-c111-11e1-1111-1111111111a", + "name": "Test", + "alias": "TEST_1", + }, + } + } + + +@pytest.fixture +def lu_gov_user(requests_mock, gov_user): + gov_user["user"]["team"]["name"] = "Licensing Unit" + gov_user["user"]["team"]["alias"] = LICENSING_UNIT_TEAM + + requests_mock.get( + client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), # /PS-IGNORE + json=gov_user, + ) + + +@pytest.fixture +def mod_ecju_gov_user(requests_mock, gov_user): + gov_user["user"]["team"]["name"] = "MOD-ECJU" + gov_user["user"]["team"]["alias"] = MOD_ECJU_TEAM + + requests_mock.get( + client._build_absolute_uri("/gov-users/2a43805b-c082-47e7-9188-c8b3e1a83cb0"), # /PS-IGNORE + json=gov_user, + ) + + +def test_ConsolidateSelectDecisionView_GET_team_not_allowed_raises_exception( + authorized_client, + consolidate_select_decision_url, + mixed_advice, + data_standard_case, +): + data_standard_case["case"]["advice"] = mixed_advice + + with pytest.raises(Exception) as err: + authorized_client.get(consolidate_select_decision_url) + + assert str(err.value) == "Consolidate/combine operation not allowed for team 00000000-0000-0000-0000-000000000001" + + +def test_ConsolidateSelectDecisionView_POST_team_not_allowed_raises_exception( + authorized_client, + consolidate_select_decision_url, + mixed_advice, + data_standard_case, +): + data_standard_case["case"]["advice"] = mixed_advice + + with pytest.raises(Exception) as err: + authorized_client.post(consolidate_select_decision_url) + + assert str(err.value) == "Consolidate/combine operation not allowed for team 00000000-0000-0000-0000-000000000001" + + +def test_ConsolidateSelectDecisionView_all_advice_approve_redirects( + authorized_client, + consolidate_select_decision_url, + approval_advice, + data_standard_case, + consolidate_approve_url, +): + data_standard_case["case"]["advice"] = approval_advice + response = authorized_client.get(consolidate_select_decision_url, follow=False) + assert response.status_code == 302 + assert response.url == consolidate_approve_url + + +def test_ConsolidateSelectDecisionView_lu_gov_user_GET( + authorized_client, + consolidate_select_decision_url, + mixed_advice, + data_standard_case, + lu_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.get(consolidate_select_decision_url, follow=False) + assert response.status_code == 200 + assert response.context["title"] == "Review and combine case recommendation - GBSIEL/2020/0002687/T - jim" + + +def test_ConsolidateSelectDecisionView_mod_ecju_gov_user_GET( + authorized_client, + consolidate_select_decision_url, + mixed_advice, + data_standard_case, + mod_ecju_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.get(consolidate_select_decision_url, follow=False) + assert response.status_code == 200 + assert response.context["title"] == "Review and combine case recommendation - GBSIEL/2020/0002687/T - jim" + + +def test_ConsolidateSelectDecisionView_POST_bad_data( + authorized_client, + consolidate_select_decision_url, + mixed_advice, + data_standard_case, + lu_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post(consolidate_select_decision_url, data={}) + assert response.status_code == 200 + form = response.context["form"] + assert form.errors == {"recommendation": ["Select if you approve or refuse"]} + + +def test_ConsolidateSelectDecisionView_POST_approve_success( + authorized_client, + consolidate_select_decision_url, + mixed_advice, + data_standard_case, + lu_gov_user, + consolidate_approve_url, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post(consolidate_select_decision_url, data={"recommendation": "approve"}, follow=False) + assert response.status_code == 302 + assert response.url == consolidate_approve_url + + +def test_ConsolidateSelectDecisionView_POST_refuse_success( + authorized_client, + consolidate_select_decision_url, + mixed_advice, + data_standard_case, + lu_gov_user, + consolidate_refuse_url, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post(consolidate_select_decision_url, data={"recommendation": "refuse"}, follow=False) + assert response.status_code == 302 + assert response.url == consolidate_refuse_url diff --git a/caseworker/advice/views.py b/caseworker/advice/views/views.py similarity index 98% rename from caseworker/advice/views.py rename to caseworker/advice/views/views.py index 66bb2ee2c..2e864a4be 100644 --- a/caseworker/advice/views.py +++ b/caseworker/advice/views/views.py @@ -649,6 +649,7 @@ def get_context(self, **kwargs): return {**super().get_context(**kwargs), "countersign": True} +# TODO: Move to views/consolidate_advice.py on change class ConsolidateAdviceView(AdviceView): def get_context(self, **kwargs): return {**super().get_context(**kwargs), "consolidate": True} @@ -662,10 +663,6 @@ def get_context_data(self, *args, **kwargs): class ReviewConsolidateView(LoginRequiredMixin, CaseContextMixin, FormView): template_name = "advice/review_consolidate.html" - def is_advice_approve_only(self): - approve_advice_types = ("approve", "proviso", "no_licence_required") - return all(a["type"]["key"] in approve_advice_types for a in self.case.advice) - def get_form(self): form_kwargs = self.get_form_kwargs() @@ -679,7 +676,7 @@ def get_form(self): ) return forms.RefusalAdviceForm(choices, **form_kwargs) - if self.kwargs.get("advice_type") == AdviceType.APPROVE or self.is_advice_approve_only(): + if self.kwargs.get("advice_type") == AdviceType.APPROVE: form_kwargs["approval_reason"] = get_picklists_list( self.request, type="standard_advice", disable_pagination=True, show_deactivated=False ) @@ -737,15 +734,10 @@ def form_valid(self, form): return super().form_valid(form) def get_success_url(self): - if self.kwargs.get("advice_type") is None: - recommendation = self.request.POST.get("recommendation") - if recommendation == "approve": - return f"{self.request.path}approve/" - if recommendation == "refuse": - return f"{self.request.path}refuse/" return reverse("cases:consolidate_view", kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"]}) +# TODO: Move to views/consolidate_advice.py on change class ConsolidateEditView(ReviewConsolidateView): """ Form to edit consolidated advice. @@ -828,6 +820,7 @@ def get_success_url(self): return reverse("cases:consolidate_view", kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"]}) +# TODO: Move to views/consolidate_advice.py on change class ViewConsolidatedAdviceView(AdviceView, FormView): form_class = forms.MoveCaseForwardForm diff --git a/unit_tests/caseworker/advice/test_services.py b/unit_tests/caseworker/advice/test_services.py index 3ae91ad24..d10a1400c 100644 --- a/unit_tests/caseworker/advice/test_services.py +++ b/unit_tests/caseworker/advice/test_services.py @@ -262,7 +262,7 @@ def test_update_countersign_decision_advice( ] -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_update_advice_by_team_other_than_LU_raises_error( mock_get_gov_user, advice, @@ -284,7 +284,7 @@ def test_update_advice_by_team_other_than_LU_raises_error( update_advice(requests_mock, case, current_user, "refuse", {}, "final-advice") -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_update_advice_not_supported_advice_type_raises_error( mock_get_gov_user, advice, diff --git a/unit_tests/caseworker/advice/views/test_consolidate.py b/unit_tests/caseworker/advice/views/test_consolidate.py index ceea436a7..30e150c09 100644 --- a/unit_tests/caseworker/advice/views/test_consolidate.py +++ b/unit_tests/caseworker/advice/views/test_consolidate.py @@ -306,8 +306,6 @@ def gov_user(): @pytest.mark.parametrize( "path, form_class, team_alias, team_name", ( - ("", forms.ConsolidateApprovalForm, LICENSING_UNIT_TEAM, "LU Team"), - ("", forms.ConsolidateApprovalForm, MOD_ECJU_TEAM, "MOD Team"), ("approve/", forms.ConsolidateApprovalForm, LICENSING_UNIT_TEAM, "LU Team"), ("refuse/", forms.LUConsolidateRefusalForm, LICENSING_UNIT_TEAM, "LU Team"), ("approve/", forms.ConsolidateApprovalForm, MOD_ECJU_TEAM, "MOD Team"), @@ -766,7 +764,6 @@ def test_view_consolidate_refuse_outcome( @pytest.mark.parametrize( "path, form_class", ( - ("", forms.ConsolidateApprovalForm), ("approve/", forms.ConsolidateApprovalForm), ("refuse/", forms.RefusalAdviceForm), ), From ba4f94b51fce6a726028a311af2d2ae5c6569206 Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Tue, 3 Dec 2024 12:28:45 +0000 Subject: [PATCH 4/9] Fix unit tests following relocation of advice views.py --- .../advice/views/test_consolidate_edit.py | 16 ++++++++-------- .../advice/views/test_countersign.py | 12 ++++++------ .../advice/views/test_countersign_edit.py | 2 +- .../advice/views/test_countersign_view.py | 8 ++++---- .../advice/views/test_edit_advice.py | 4 ++-- .../views/test_give_approval_advice_view.py | 18 +++++++++--------- .../advice/views/test_lu_consolidate.py | 12 ++++++------ .../advice/views/test_refusal_advice_view.py | 6 +++--- .../advice/views/test_select_advice_view.py | 2 +- .../advice/views/test_view_my_advice_view.py | 2 +- .../advice/views/test_view_ogd_advice.py | 2 +- 11 files changed, 42 insertions(+), 42 deletions(-) diff --git a/unit_tests/caseworker/advice/views/test_consolidate_edit.py b/unit_tests/caseworker/advice/views/test_consolidate_edit.py index 0e843c73c..c936a3c50 100644 --- a/unit_tests/caseworker/advice/views/test_consolidate_edit.py +++ b/unit_tests/caseworker/advice/views/test_consolidate_edit.py @@ -240,7 +240,7 @@ def test_edit_refuse_advice_post( "team, advice_level", ((services.LICENSING_UNIT_TEAM, "final"), (services.MOD_ECJU_TEAM, "team")), ) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_edit_advice_get( mock_get_gov_user, team, @@ -269,7 +269,7 @@ def test_edit_advice_get( assert isinstance(form, forms.ConsolidateApprovalForm) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_edit_consolidated_advice_approve_by_lu_put( mock_get_gov_user, authorized_client, @@ -298,7 +298,7 @@ def test_edit_consolidated_advice_approve_by_lu_put( ] -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_edit_consolidated_advice_approve__with_nlr_products_by_lu_put( mock_get_gov_user, authorized_client, @@ -337,7 +337,7 @@ def test_edit_consolidated_advice_approve__with_nlr_products_by_lu_put( ) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_edit_consolidated_advice_refuse_note_by_lu_put( mock_get_gov_user, authorized_client, @@ -375,7 +375,7 @@ def test_edit_consolidated_advice_refuse_note_by_lu_put( ] -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_edit_consolidated_advice_by_LU_error_from_API( mock_get_gov_user, authorized_client, @@ -410,7 +410,7 @@ def test_edit_consolidated_advice_by_LU_error_from_API( ("MOD", "Countersigned by MOD User"), ), ) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_edit_advice_get_displays_correct_counteradvice( mock_get_gov_user, authorized_client, @@ -458,7 +458,7 @@ def test_edit_advice_get_displays_correct_counteradvice( assert countersignatures[0].find("p").text == fcdo_or_mod_advice[0]["countersign_comments"] -@patch("caseworker.advice.views.get_gov_user") # Pass to the mock version; mock_get_gov_user +@patch("caseworker.advice.views.views.get_gov_user") # Pass to the mock version; mock_get_gov_user def test_edit_refusal_note_exists( mock_get_gov_user, authorized_client, @@ -483,7 +483,7 @@ def test_edit_refusal_note_exists( assert note_element.get_text(strip=True) == "The refusal note assess_1_2" -@patch("caseworker.advice.views.get_gov_user") # Pass to the mock version; mock_get_gov_user +@patch("caseworker.advice.views.views.get_gov_user") # Pass to the mock version; mock_get_gov_user def test_mod_ecju_edit_exists( mock_get_gov_user, authorized_client, diff --git a/unit_tests/caseworker/advice/views/test_countersign.py b/unit_tests/caseworker/advice/views/test_countersign.py index a1a5a2e73..9e60ed9ed 100644 --- a/unit_tests/caseworker/advice/views/test_countersign.py +++ b/unit_tests/caseworker/advice/views/test_countersign.py @@ -468,7 +468,7 @@ def test_lu_countersign_decision_post_success( ] -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_lu_countersign_get_shows_previous_countersignature( mock_get_gov_user, authorized_client, @@ -523,7 +523,7 @@ def user_not_allowed_to_countersign(response): return True -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") @patch("caseworker.core.rules.get_logged_in_caseworker") def test_case_officer_cannot_countersign_as_licensing_manager( mock_caseworker, @@ -546,7 +546,7 @@ def test_case_officer_cannot_countersign_as_licensing_manager( assert user_not_allowed_to_countersign(response) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") @patch("caseworker.core.rules.get_logged_in_caseworker") def test_licensing_manager_countersigner_not_same_as_case_officer( mock_caseworker, @@ -575,7 +575,7 @@ def test_licensing_manager_countersigner_not_same_as_case_officer( # Senior Licensing manager tests -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") @patch("caseworker.core.rules.get_logged_in_caseworker") def test_case_officer_cannot_countersign_as_senior_licensing_manager( mock_caseworker, @@ -598,7 +598,7 @@ def test_case_officer_cannot_countersign_as_senior_licensing_manager( assert user_not_allowed_to_countersign(response) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") @patch("caseworker.core.rules.get_logged_in_caseworker") def test_licensing_manager_cannot_countersign_as_senior_licensing_manager( mock_caseworker, @@ -621,7 +621,7 @@ def test_licensing_manager_cannot_countersign_as_senior_licensing_manager( assert user_not_allowed_to_countersign(response) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") @patch("caseworker.core.rules.get_logged_in_caseworker") def test_senior_manager_countersigner_not_same_as_case_officer_or_countersigner( mock_caseworker, diff --git a/unit_tests/caseworker/advice/views/test_countersign_edit.py b/unit_tests/caseworker/advice/views/test_countersign_edit.py index 6007b65a6..1eddd2e68 100644 --- a/unit_tests/caseworker/advice/views/test_countersign_edit.py +++ b/unit_tests/caseworker/advice/views/test_countersign_edit.py @@ -272,7 +272,7 @@ def test_lu_countersign_decision_edit_post_success( ] -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_lu_countersign_edit_get_shows_previous_countersignature( mock_get_gov_user, authorized_client, diff --git a/unit_tests/caseworker/advice/views/test_countersign_view.py b/unit_tests/caseworker/advice/views/test_countersign_view.py index 19c8c54d6..287723b10 100644 --- a/unit_tests/caseworker/advice/views/test_countersign_view.py +++ b/unit_tests/caseworker/advice/views/test_countersign_view.py @@ -52,7 +52,7 @@ def test_countersign_view_trigger_list_products( assert response.context["assessed_trigger_list_goods"] == expected_context -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_single_lu_countersignature( mock_get_gov_user, authorized_client, @@ -89,7 +89,7 @@ def test_single_lu_countersignature( assert not rejected_warning -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_double_lu_countersignature( mock_get_gov_user, authorized_client, @@ -140,7 +140,7 @@ def test_double_lu_countersignature( ], ), ) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_single_lu_rejected_countersignature( mock_get_gov_user, countersigning_data, @@ -205,7 +205,7 @@ def test_single_lu_rejected_countersignature( ], ), ) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_lu_rejected_senior_countersignature( mock_get_gov_user, countersigning_data, diff --git a/unit_tests/caseworker/advice/views/test_edit_advice.py b/unit_tests/caseworker/advice/views/test_edit_advice.py index f1f809c5f..fdaa26b50 100644 --- a/unit_tests/caseworker/advice/views/test_edit_advice.py +++ b/unit_tests/caseworker/advice/views/test_edit_advice.py @@ -245,7 +245,7 @@ def test_DESNZ_give_approval_advice_post_valid( }, None, ) - mocker.patch("caseworker.advice.views.get_gov_user", return_value=get_gov_user_value) + mocker.patch("caseworker.advice.views.views.get_gov_user", return_value=get_gov_user_value) case_data = deepcopy(data_standard_case) case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"] case_data["case"]["advice"] = standard_case_with_advice["advice"] @@ -355,7 +355,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional( }, None, ) - mocker.patch("caseworker.advice.views.get_gov_user", return_value=get_gov_user_value) + mocker.patch("caseworker.advice.views.views.get_gov_user", return_value=get_gov_user_value) case_data = deepcopy(data_standard_case) case_data["case"]["data"]["goods"] = standard_case_with_advice["data"]["goods"] case_data["case"]["advice"] = standard_case_with_advice["advice"] 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 ab2c56f13..a8d240e89 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 @@ -33,7 +33,7 @@ def test_select_advice_post(authorized_client, requests_mock, data_standard_case assert response.status_code == 302 -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_fco_give_approval_advice_get(mock_get_gov_user, authorized_client, url): mock_get_gov_user.return_value = ( {"user": {"team": {"id": "67b9a4a3-6f3d-4511-8a19-23ccff221a74", "name": "FCO", "alias": services.FCDO_TEAM}}}, @@ -48,7 +48,7 @@ def test_fco_give_approval_advice_get(mock_get_gov_user, authorized_client, url) ] -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_fco_give_approval_advice_existing_get(mock_get_gov_user, authorized_client, url, data_standard_case): mock_get_gov_user.return_value = ( {"user": {"team": {"id": "67b9a4a3-6f3d-4511-8a19-23ccff221a74", "name": "FCO", "alias": services.FCDO_TEAM}}}, @@ -93,8 +93,8 @@ def test_fco_give_approval_advice_existing_get(mock_get_gov_user, authorized_cli ([], "", 200), ], ) -@mock.patch("caseworker.advice.views.get_gov_user") -def test_fcdo_give_approval_advice_post( +@mock.patch("caseworker.advice.views.views.get_gov_user") +def test_fco_give_approval_advice_post( mock_get_gov_user, authorized_client, requests_mock, @@ -127,7 +127,7 @@ def post_to_step(post_to_step_factory, url_desnz): return post_to_step_factory(url_desnz) -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_DESNZ_give_approval_advice_post_valid( mock_get_gov_user, authorized_client, @@ -160,7 +160,7 @@ def test_DESNZ_give_approval_advice_post_valid( assert response.status_code == 302 -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_DESNZ_give_approval_advice_post_valid_add_conditional( mock_get_gov_user, authorized_client, @@ -212,7 +212,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional( assert add_instructions_response.status_code == 302 -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_DESNZ_give_approval_advice_post_valid_add_conditional_optional( mock_get_gov_user, authorized_client, @@ -265,7 +265,7 @@ def test_DESNZ_give_approval_advice_post_valid_add_conditional_optional( assert add_instructions_response.status_code == 302 -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_DESNZ_give_approval_advice_post_invalid( mock_get_gov_user, authorized_client, @@ -300,7 +300,7 @@ def test_DESNZ_give_approval_advice_post_invalid( soup = beautiful_soup(response.content) -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_DESNZ_give_approval_advice_post_invalid_user( mock_get_gov_user, authorized_client, diff --git a/unit_tests/caseworker/advice/views/test_lu_consolidate.py b/unit_tests/caseworker/advice/views/test_lu_consolidate.py index 1c4987b29..58ef8b3fd 100644 --- a/unit_tests/caseworker/advice/views/test_lu_consolidate.py +++ b/unit_tests/caseworker/advice/views/test_lu_consolidate.py @@ -22,7 +22,7 @@ def url(request, data_queue, data_standard_case): ) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_no_advice_summary_for_lu( mock_get_gov_user, url, @@ -53,7 +53,7 @@ def test_no_advice_summary_for_lu( ], ), ) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_lu_consolidate_check_countersignatures_other_recommendations( mock_get_gov_user, countersigning_data, @@ -104,14 +104,14 @@ def test_lu_consolidate_check_countersignatures_other_recommendations( def advice(current_user): return [ { - "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE "country": None, "created_at": "2021-10-16T23:48:39.486679+01:00", "denial_reasons": [], "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", "footnote": "footnotes", "good": good_id, - "id": "429c5596-fe8b-4540-988b-c37805cd08de", + "id": "429c5596-fe8b-4540-988b-c37805cd08de", # /PS-IGNORE "level": "user", "note": "additional notes", "proviso": "no conditions", @@ -132,7 +132,7 @@ def url_consolidate_review(data_queue, data_standard_case): ) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_refusal_note_post( mock_get_gov_user, requests_mock, authorized_client, data_standard_case, url_consolidate_review ): @@ -162,7 +162,7 @@ def test_refusal_note_post( "type": "refuse", "text": "test", "footnote_required": False, - "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE "denial_reasons": ["1a"], "is_refusal_note": True, }, diff --git a/unit_tests/caseworker/advice/views/test_refusal_advice_view.py b/unit_tests/caseworker/advice/views/test_refusal_advice_view.py index e61b6ea3f..fe57174fc 100644 --- a/unit_tests/caseworker/advice/views/test_refusal_advice_view.py +++ b/unit_tests/caseworker/advice/views/test_refusal_advice_view.py @@ -41,7 +41,7 @@ def test_refuse_all_post(authorized_client, url, denial_reasons, refusal_reasons assert response.status_code == expected_status_code -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_fco_give_refusal_advice_get(mock_get_gov_user, authorized_client, url): mock_get_gov_user.return_value = ( {"user": {"team": {"id": "67b9a4a3-6f3d-4511-8a19-23ccff221a74", "name": "FCO", "alias": services.FCDO_TEAM}}}, @@ -56,7 +56,7 @@ def test_fco_give_refusal_advice_get(mock_get_gov_user, authorized_client, url): ] -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_fco_give_refusal_advice_existing_get(mock_get_gov_user, authorized_client, url, data_standard_case): mock_get_gov_user.return_value = ( {"user": {"team": {"id": "67b9a4a3-6f3d-4511-8a19-23ccff221a74", "name": "FCO", "alias": services.FCDO_TEAM}}}, @@ -101,7 +101,7 @@ def test_fco_give_refusal_advice_existing_get(mock_get_gov_user, authorized_clie ([], "", 200), ], ) -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_fco_give_approval_advice_post( mock_get_gov_user, authorized_client, 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 04c57c544..91e293fdf 100644 --- a/unit_tests/caseworker/advice/views/test_select_advice_view.py +++ b/unit_tests/caseworker/advice/views/test_select_advice_view.py @@ -46,7 +46,7 @@ def test_select_advice_post_desnz(authorized_client, url, data_standard_case, mo }, None, ) - mocker.patch("caseworker.advice.views.get_gov_user", return_value=get_gov_user_value) + mocker.patch("caseworker.advice.views.views.get_gov_user", return_value=get_gov_user_value) response = authorized_client.post(url, data={"recommendation": "approve_all"}) assert response.status_code == 302 assert ( diff --git a/unit_tests/caseworker/advice/views/test_view_my_advice_view.py b/unit_tests/caseworker/advice/views/test_view_my_advice_view.py index a2623c401..36072d26b 100644 --- a/unit_tests/caseworker/advice/views/test_view_my_advice_view.py +++ b/unit_tests/caseworker/advice/views/test_view_my_advice_view.py @@ -267,7 +267,7 @@ def test_move_case_forward_permission( assert len(soup.find_all("form")) == (1 if is_user_case_advisor else 0) -@patch("caseworker.advice.views.get_gov_user") +@patch("caseworker.advice.views.views.get_gov_user") def test_lu_countersignatures_not_shown( mock_get_gov_user, authorized_client, diff --git a/unit_tests/caseworker/advice/views/test_view_ogd_advice.py b/unit_tests/caseworker/advice/views/test_view_ogd_advice.py index 4ed15b35e..71927ba21 100644 --- a/unit_tests/caseworker/advice/views/test_view_ogd_advice.py +++ b/unit_tests/caseworker/advice/views/test_view_ogd_advice.py @@ -78,7 +78,7 @@ def test_advice_view_heading_ogd_advice( assert team_headings == {"A team has approved and refused", "B team has approved"} -@mock.patch("caseworker.advice.views.get_gov_user") +@mock.patch("caseworker.advice.views.views.get_gov_user") def test_fcdo_cannot_advice_when_all_destinations_covered( mock_get_gov_user, authorized_client, data_queue, data_standard_case ): From 2ea1ae99762f83a449b6fc6dfcae9a8240a9a87b Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Tue, 3 Dec 2024 17:25:14 +0000 Subject: [PATCH 5/9] Break approval flow out of ReviewConsolidateView --- caseworker/advice/urls.py | 3 + caseworker/advice/views/consolidate_advice.py | 55 ++- .../views/tests/test_consolidate_advice.py | 420 +++++++++++++++++- .../advice/views/test_consolidate.py | 208 --------- 4 files changed, 473 insertions(+), 213 deletions(-) diff --git a/caseworker/advice/urls.py b/caseworker/advice/urls.py index 8677dec6e..d28d8e0c4 100644 --- a/caseworker/advice/urls.py +++ b/caseworker/advice/urls.py @@ -28,6 +28,9 @@ name="countersign_decision_edit", ), path("consolidate/", views.ConsolidateAdviceView.as_view(), name="consolidate_advice_view"), + path( + "consolidate/review/approve/", consolidate_advice.ConsolidateApproveView.as_view(), name="consolidate_approve" + ), path("consolidate/review//", views.ReviewConsolidateView.as_view(), name="consolidate"), path("consolidate/review/", consolidate_advice.ConsolidateSelectDecisionView.as_view(), name="consolidate_review"), path("consolidate/edit/", views.ConsolidateEditView.as_view(), name="consolidate_edit"), diff --git a/caseworker/advice/views/consolidate_advice.py b/caseworker/advice/views/consolidate_advice.py index 5b2b5aba4..b3eb4c806 100644 --- a/caseworker/advice/views/consolidate_advice.py +++ b/caseworker/advice/views/consolidate_advice.py @@ -2,11 +2,14 @@ from django.shortcuts import redirect from django.urls import reverse +from requests.exceptions import HTTPError + from core.auth.views import LoginRequiredMixin from caseworker.advice import forms from caseworker.advice.views.views import CaseContextMixin from caseworker.advice import services +from caseworker.picklists.services import get_picklists_list class BaseConsolidationView(LoginRequiredMixin, CaseContextMixin, FormView): @@ -55,7 +58,53 @@ def form_valid(self, form): return super().form_valid(form) def get_success_url(self): - return reverse( - "cases:consolidate", - kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"], "advice_type": self.decision}, + if self.decision == "approve": + return reverse( + "cases:consolidate_approve", + kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"]}, + ) + else: + return reverse( + "cases:consolidate", + kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"], "advice_type": "refuse"}, + ) + + +class ConsolidateApproveView(BaseConsolidationView): + """ + Consolidate advice and approve. + """ + + template_name = "advice/review_consolidate.html" + form_class = forms.ConsolidateApprovalForm + + def setup(self, *args, **kwargs): + super().setup(*args, **kwargs) + self.team_alias = self.caseworker["team"].get("alias", None) + + def get_form_kwargs(self): + form_kwargs = super().get_form_kwargs() + form_kwargs["approval_reason"] = get_picklists_list( + self.request, type="standard_advice", disable_pagination=True, show_deactivated=False + ) + form_kwargs["proviso"] = get_picklists_list( + self.request, type="proviso", disable_pagination=True, show_deactivated=False + ) + form_kwargs["footnote_details"] = get_picklists_list( + self.request, type="footnotes", disable_pagination=True, show_deactivated=False ) + form_kwargs["team_alias"] = self.team_alias + return form_kwargs + + def form_valid(self, form): + level = "final-advice" if self.team_alias == services.LICENSING_UNIT_TEAM else "team-advice" + try: + services.post_approval_advice(self.request, self.case, form.cleaned_data, level=level) + except HTTPError as e: + errors = e.response.json()["errors"] + form.add_error(None, errors) + return super().form_invalid(form) + return super().form_valid(form) + + def get_success_url(self): + return reverse("cases:consolidate_view", kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"]}) diff --git a/caseworker/advice/views/tests/test_consolidate_advice.py b/caseworker/advice/views/tests/test_consolidate_advice.py index 4d2f3e06c..072693a9d 100644 --- a/caseworker/advice/views/tests/test_consolidate_advice.py +++ b/caseworker/advice/views/tests/test_consolidate_advice.py @@ -32,8 +32,8 @@ def consolidate_select_decision_url(data_queue, data_standard_case): @pytest.fixture def consolidate_approve_url(data_queue, data_standard_case): return reverse( - "cases:consolidate", - kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"], "advice_type": "approve"}, + "cases:consolidate_approve", + kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}, ) @@ -45,6 +45,14 @@ def consolidate_refuse_url(data_queue, data_standard_case): ) +@pytest.fixture +def consolidate_view_url(data_queue, data_standard_case): + return reverse( + "cases:consolidate_view", + kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}, + ) + + @pytest.fixture def advice_data(current_user, admin_team): return { @@ -162,6 +170,7 @@ def test_ConsolidateSelectDecisionView_all_advice_approve_redirects( approval_advice, data_standard_case, consolidate_approve_url, + lu_gov_user, ): data_standard_case["case"]["advice"] = approval_advice response = authorized_client.get(consolidate_select_decision_url, follow=False) @@ -235,3 +244,410 @@ def test_ConsolidateSelectDecisionView_POST_refuse_success( response = authorized_client.post(consolidate_select_decision_url, data={"recommendation": "refuse"}, follow=False) assert response.status_code == 302 assert response.url == consolidate_refuse_url + + +def test_ConsolidateApproveView_GET_team_not_allowed_raises_exception( + authorized_client, + consolidate_approve_url, + mixed_advice, + data_standard_case, +): + data_standard_case["case"]["advice"] = mixed_advice + + with pytest.raises(Exception) as err: + authorized_client.get(consolidate_approve_url) + + assert str(err.value) == "Consolidate/combine operation not allowed for team 00000000-0000-0000-0000-000000000001" + + +def test_ConsolidateApproveView_POST_team_not_allowed_raises_exception( + authorized_client, + consolidate_approve_url, + mixed_advice, + data_standard_case, +): + data_standard_case["case"]["advice"] = mixed_advice + pass + + with pytest.raises(Exception) as err: + authorized_client.post(consolidate_approve_url) + + assert str(err.value) == "Consolidate/combine operation not allowed for team 00000000-0000-0000-0000-000000000001" + + +def test_ConsolidateApproveView_lu_gov_user_GET( + authorized_client, + consolidate_approve_url, + mixed_advice, + data_standard_case, + lu_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.get(consolidate_approve_url, follow=False) + assert response.status_code == 200 + assert response.context["title"] == "Review and combine case recommendation - GBSIEL/2020/0002687/T - jim" + + +def test_ConsolidateApproveView_mod_ecju_gov_user_GET( + authorized_client, + consolidate_approve_url, + mixed_advice, + data_standard_case, + mod_ecju_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.get(consolidate_approve_url, follow=False) + assert response.status_code == 200 + assert response.context["title"] == "Review and combine case recommendation - GBSIEL/2020/0002687/T - jim" + + +def test_ConsolidateApproveView_POST_bad_input( + authorized_client, + consolidate_approve_url, + mixed_advice, + data_standard_case, + lu_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post(consolidate_approve_url, data={}) + assert response.status_code == 200 + form = response.context["form"] + assert form.errors == {"approval_reasons": ["Enter a reason for approving"]} + + +@pytest.fixture +def mock_post_approval_final_advice(requests_mock, data_standard_case): + return requests_mock.post( + client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}/final-advice/"), + json={}, + ) + + +@pytest.fixture +def mock_post_approval_team_advice(requests_mock, data_standard_case): + return requests_mock.post( + client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}/team-advice/"), + json={}, + ) + + +@pytest.mark.parametrize( + "approval_data, expected_post_data", + ( + ( + {"approval_reasons": "yep, go for it"}, + [ + { + "denial_reasons": [], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote": "", + "footnote_required": False, + "note": "", + "proviso": "", + "text": "yep, go for it", + "type": "approve", + }, + { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "", + "proviso": "", + "text": "yep, go for it", + "type": "approve", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "", + "proviso": "", + "text": "yep, go for it", + "type": "approve", + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "", + "proviso": "", + "text": "yep, go for it", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": "approve", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "yep, go for it", + "type": "approve", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "", + "type": "no_licence_required", + }, + ], + ), + ( + { + "approval_reasons": "yep, go for it", + "proviso": "just consider this", + "instructions_to_exporter": "and this", + "footnote": "some footnote", + }, + [ + { + "denial_reasons": [], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote": "", + "footnote_required": False, + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "type": "proviso", + }, + { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "type": "proviso", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "type": "proviso", + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": "proviso", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "type": "proviso", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "", + "type": "no_licence_required", + }, + ], + ), + ), +) +def test_ConsolidateApproveView_mod_ecju_gov_user_POST_success( + approval_data, + expected_post_data, + authorized_client, + consolidate_approve_url, + mixed_advice, + data_standard_case, + lu_gov_user, + mock_post_approval_final_advice, + consolidate_view_url, +): + data_standard_case["case"]["advice"] = mixed_advice + data_standard_case["case"]["data"]["goods"][0]["is_good_controlled"] = {"key": "True", "value": "Yes"} + + response = authorized_client.post(consolidate_approve_url, data=approval_data) + assert response.status_code == 302 + assert response.url == consolidate_view_url + assert len(mock_post_approval_final_advice.request_history) == 1 + assert mock_post_approval_team_advice.request_history[0].json() == expected_post_data + + +@pytest.mark.parametrize( + "approval_data, expected_post_data", + ( + ( + {"approval_reasons": "yep, go for it"}, + [ + { + "denial_reasons": [], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote": "", + "footnote_required": False, + "note": "", + "proviso": "", + "text": "yep, go for it", + "type": "approve", + }, + { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "", + "proviso": "", + "text": "yep, go for it", + "type": "approve", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "", + "proviso": "", + "text": "yep, go for it", + "type": "approve", + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "", + "proviso": "", + "text": "yep, go for it", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": "approve", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "yep, go for it", + "type": "approve", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "", + "type": "no_licence_required", + }, + ], + ), + ( + { + "approval_reasons": "yep, go for it", + "proviso": "just consider this", + "instructions_to_exporter": "and this", + "footnote": "some footnote", + }, + [ + { + "denial_reasons": [], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote": "", + "footnote_required": False, + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "type": "proviso", + }, + { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "type": "proviso", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "type": "proviso", + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": "proviso", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "and this", + "proviso": "just consider this", + "text": "yep, go for it", + "type": "proviso", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "", + "type": "no_licence_required", + }, + ], + ), + ), +) +def test_ConsolidateApproveView_mod_ecju_gov_user_POST_success( + approval_data, + expected_post_data, + authorized_client, + consolidate_approve_url, + mixed_advice, + data_standard_case, + mod_ecju_gov_user, + mock_post_approval_team_advice, + consolidate_view_url, +): + data_standard_case["case"]["advice"] = mixed_advice + data_standard_case["case"]["data"]["goods"][0]["is_good_controlled"] = {"key": "True", "value": "Yes"} + + response = authorized_client.post(consolidate_approve_url, data=approval_data) + assert response.status_code == 302 + assert response.url == consolidate_view_url + assert len(mock_post_approval_team_advice.request_history) == 1 + assert mock_post_approval_team_advice.request_history[0].json() == expected_post_data diff --git a/unit_tests/caseworker/advice/views/test_consolidate.py b/unit_tests/caseworker/advice/views/test_consolidate.py index 30e150c09..36373d6aa 100644 --- a/unit_tests/caseworker/advice/views/test_consolidate.py +++ b/unit_tests/caseworker/advice/views/test_consolidate.py @@ -306,9 +306,7 @@ def gov_user(): @pytest.mark.parametrize( "path, form_class, team_alias, team_name", ( - ("approve/", forms.ConsolidateApprovalForm, LICENSING_UNIT_TEAM, "LU Team"), ("refuse/", forms.LUConsolidateRefusalForm, LICENSING_UNIT_TEAM, "LU Team"), - ("approve/", forms.ConsolidateApprovalForm, MOD_ECJU_TEAM, "MOD Team"), ("refuse/", forms.RefusalAdviceForm, MOD_ECJU_TEAM, "MOD Team"), ), ) @@ -347,139 +345,6 @@ def test_consolidate_review( 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, forms.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, forms.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, forms.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", [ @@ -551,79 +416,6 @@ def test_consolidate_review_refusal_advice_recommendation_label( assert form.fields["recommendation"].label == recommendation_label -def test_consolidate_review_approve(requests_mock, authorized_client, data_standard_case, url, advice): - data_standard_case["case"]["advice"] = advice - data = {"approval_reasons": "test", "countries": ["GB"]} - - response = authorized_client.post(url + "approve/", data=data) - assert response.status_code == 302 - request = requests_mock.request_history.pop() - assert request.method == "POST" - assert "team-advice" in request.url - assert request.json() == [ - { - "type": "approve", - "text": "test", - "proviso": "", - "note": "", - "footnote_required": False, - "footnote": "", - "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", - "denial_reasons": [], - }, - { - "type": "approve", - "text": "test", - "proviso": "", - "note": "", - "footnote_required": False, - "footnote": "", - "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", - "denial_reasons": [], - }, - { - "denial_reasons": [], - "footnote": "", - "footnote_required": False, - "note": "", - "proviso": "", - "text": "test", - "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", - "type": "approve", - }, - { - "type": "approve", - "text": "test", - "proviso": "", - "note": "", - "footnote_required": False, - "footnote": "", - "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", - "denial_reasons": [], - }, - { - "denial_reasons": [], - "footnote": "", - "footnote_required": False, - "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", - "note": "", - "proviso": "", - "text": "", - "type": "no_licence_required", - }, - { - "denial_reasons": [], - "footnote": "", - "footnote_required": False, - "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", - "note": "", - "proviso": "", - "text": "", - "type": "no_licence_required", - }, - ] - - def test_consolidate_review_refuse(requests_mock, authorized_client, data_standard_case, url, advice): data_standard_case["case"]["advice"] = advice data = {"denial_reasons": ["1"], "refusal_reasons": "test", "countries": ["GB"]} From 32f3c2396b4a4b286b846c82f9c2bc9a7e64dfe7 Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Wed, 4 Dec 2024 14:48:21 +0000 Subject: [PATCH 6/9] Break refusal flows out of ReviewConsolidateView --- caseworker/advice/urls.py | 9 +- caseworker/advice/views/consolidate_advice.py | 94 +++- .../views/tests/test_consolidate_advice.py | 464 +++++++++++++++++- .../advice/views/test_consolidate.py | 44 -- .../advice/views/test_lu_consolidate.py | 80 --- 5 files changed, 547 insertions(+), 144 deletions(-) diff --git a/caseworker/advice/urls.py b/caseworker/advice/urls.py index d28d8e0c4..9414505c4 100644 --- a/caseworker/advice/urls.py +++ b/caseworker/advice/urls.py @@ -28,11 +28,16 @@ name="countersign_decision_edit", ), path("consolidate/", views.ConsolidateAdviceView.as_view(), name="consolidate_advice_view"), + path("consolidate/review/", consolidate_advice.ConsolidateSelectDecisionView.as_view(), name="consolidate_review"), path( "consolidate/review/approve/", consolidate_advice.ConsolidateApproveView.as_view(), name="consolidate_approve" ), - path("consolidate/review//", views.ReviewConsolidateView.as_view(), name="consolidate"), - path("consolidate/review/", consolidate_advice.ConsolidateSelectDecisionView.as_view(), name="consolidate_review"), + path("consolidate/review/refuse/", consolidate_advice.ConsolidateRefuseView.as_view(), name="consolidate_refuse"), + path( + "consolidate/review/lu-refuse/", + consolidate_advice.LUConsolidateRefuseView.as_view(), + name="consolidate_refuse_lu", + ), path("consolidate/edit/", views.ConsolidateEditView.as_view(), name="consolidate_edit"), path("consolidate/view-advice/", views.ViewConsolidatedAdviceView.as_view(), name="consolidate_view"), path("assess-trigger-list-products/", views.DESNZProductAssessmentView.as_view(), name="assess_trigger_list"), diff --git a/caseworker/advice/views/consolidate_advice.py b/caseworker/advice/views/consolidate_advice.py index b3eb4c806..012f9daa8 100644 --- a/caseworker/advice/views/consolidate_advice.py +++ b/caseworker/advice/views/consolidate_advice.py @@ -9,6 +9,7 @@ from caseworker.advice import forms from caseworker.advice.views.views import CaseContextMixin from caseworker.advice import services +from caseworker.core.services import get_denial_reasons, group_denial_reasons from caseworker.picklists.services import get_picklists_list @@ -40,6 +41,7 @@ class ConsolidateSelectDecisionView(BaseConsolidationView): form_class = forms.ConsolidateSelectAdviceForm def dispatch(self, request, *args, **kwargs): + self.team_alias = self.caseworker["team"].get("alias", None) approve_advice_types = ("approve", "proviso", "no_licence_required") is_all_advice_approval = all(a["type"]["key"] in approve_advice_types for a in self.case.advice) if is_all_advice_approval: @@ -57,17 +59,19 @@ def form_valid(self, form): self.decision = form.cleaned_data["recommendation"] return super().form_valid(form) - def get_success_url(self): + def get_success_url_name(self): if self.decision == "approve": - return reverse( - "cases:consolidate_approve", - kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"]}, - ) - else: - return reverse( - "cases:consolidate", - kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"], "advice_type": "refuse"}, - ) + return "cases:consolidate_approve" + if self.team_alias == services.LICENSING_UNIT_TEAM: + return "cases:consolidate_refuse_lu" + return "cases:consolidate_refuse" + + def get_success_url(self): + url_name = self.get_success_url_name() + return reverse( + url_name, + kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"]}, + ) class ConsolidateApproveView(BaseConsolidationView): @@ -108,3 +112,73 @@ def form_valid(self, form): def get_success_url(self): return reverse("cases:consolidate_view", kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"]}) + + +class BaseConsolidateRefuseView(BaseConsolidationView): + """ + Base view to consolidate advice and refuse. + """ + + template_name = "advice/review_consolidate.html" + advice_level = None + + def get_title(self): + return f"Licence refused for case - {self.case.reference_code} - {self.case.organisation['name']}" + + def form_valid(self, form): + try: + data = self.build_refusal_payload(form.cleaned_data) + services.post_refusal_advice(self.request, self.case, data, level=self.advice_level) + except HTTPError as e: + errors = e.response.json()["errors"] + form.add_error(None, errors) + return super().form_invalid(form) + return super().form_valid(form) + + def get_success_url(self): + return reverse("cases:consolidate_view", kwargs={"queue_pk": self.kwargs["queue_pk"], "pk": self.kwargs["pk"]}) + + +class LUConsolidateRefuseView(BaseConsolidateRefuseView): + """ + Consolidate advice and refuse for LU. + """ + + form_class = forms.LUConsolidateRefusalForm + advice_level = "final-advice" + + def get_form_kwargs(self): + form_kwargs = super().get_form_kwargs() + denial_reasons = get_denial_reasons(self.request) + choices = group_denial_reasons(denial_reasons) + form_kwargs["choices"] = choices + return form_kwargs + + def build_refusal_payload(self, cleaned_data): + data = cleaned_data + data["text"] = data["refusal_note"] + data["is_refusal_note"] = True + return data + + +class ConsolidateRefuseView(BaseConsolidateRefuseView): + """ + Consolidate advice and refuse for non-LU. Currently MOD-ECJU. + """ + + form_class = forms.RefusalAdviceForm + advice_level = "team-advice" + + def get_form_kwargs(self): + form_kwargs = super().get_form_kwargs() + denial_reasons = get_denial_reasons(self.request) + form_kwargs["choices"] = group_denial_reasons(denial_reasons) + form_kwargs["refusal_reasons"] = get_picklists_list( + self.request, type="standard_advice", disable_pagination=True, show_deactivated=False + ) + return form_kwargs + + def build_refusal_payload(self, cleaned_data): + data = cleaned_data + data["text"] = data["refusal_reasons"] + return data diff --git a/caseworker/advice/views/tests/test_consolidate_advice.py b/caseworker/advice/views/tests/test_consolidate_advice.py index 072693a9d..6289feedf 100644 --- a/caseworker/advice/views/tests/test_consolidate_advice.py +++ b/caseworker/advice/views/tests/test_consolidate_advice.py @@ -17,7 +17,6 @@ def setup( mock_denial_reasons, mock_proviso, mock_footnote_details, - mock_finalise_advice_documents, ): return @@ -40,8 +39,16 @@ def consolidate_approve_url(data_queue, data_standard_case): @pytest.fixture def consolidate_refuse_url(data_queue, data_standard_case): return reverse( - "cases:consolidate", - kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"], "advice_type": "refuse"}, + "cases:consolidate_refuse", + kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}, + ) + + +@pytest.fixture +def lu_consolidate_refuse_url(data_queue, data_standard_case): + return reverse( + "cases:consolidate_refuse_lu", + kwargs={"queue_pk": data_queue["id"], "pk": data_standard_case["case"]["id"]}, ) @@ -188,7 +195,10 @@ def test_ConsolidateSelectDecisionView_lu_gov_user_GET( data_standard_case["case"]["advice"] = mixed_advice response = authorized_client.get(consolidate_select_decision_url, follow=False) assert response.status_code == 200 - assert response.context["title"] == "Review and combine case recommendation - GBSIEL/2020/0002687/T - jim" + assert ( + response.context["title"] + == f"Review and combine case recommendation - {data_standard_case['case']['reference_code']} - {data_standard_case['case']['data']['organisation']['name']}" + ) def test_ConsolidateSelectDecisionView_mod_ecju_gov_user_GET( @@ -201,7 +211,10 @@ def test_ConsolidateSelectDecisionView_mod_ecju_gov_user_GET( data_standard_case["case"]["advice"] = mixed_advice response = authorized_client.get(consolidate_select_decision_url, follow=False) assert response.status_code == 200 - assert response.context["title"] == "Review and combine case recommendation - GBSIEL/2020/0002687/T - jim" + assert ( + response.context["title"] + == f"Review and combine case recommendation - {data_standard_case['case']['reference_code']} - {data_standard_case['case']['data']['organisation']['name']}" + ) def test_ConsolidateSelectDecisionView_POST_bad_data( @@ -237,7 +250,7 @@ def test_ConsolidateSelectDecisionView_POST_refuse_success( consolidate_select_decision_url, mixed_advice, data_standard_case, - lu_gov_user, + mod_ecju_gov_user, consolidate_refuse_url, ): data_standard_case["case"]["advice"] = mixed_advice @@ -246,6 +259,20 @@ def test_ConsolidateSelectDecisionView_POST_refuse_success( assert response.url == consolidate_refuse_url +def test_ConsolidateSelectDecisionView_POST_lu_refuse_success( + authorized_client, + consolidate_select_decision_url, + mixed_advice, + data_standard_case, + lu_gov_user, + lu_consolidate_refuse_url, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post(consolidate_select_decision_url, data={"recommendation": "refuse"}, follow=False) + assert response.status_code == 302 + assert response.url == lu_consolidate_refuse_url + + def test_ConsolidateApproveView_GET_team_not_allowed_raises_exception( authorized_client, consolidate_approve_url, @@ -285,7 +312,10 @@ def test_ConsolidateApproveView_lu_gov_user_GET( data_standard_case["case"]["advice"] = mixed_advice response = authorized_client.get(consolidate_approve_url, follow=False) assert response.status_code == 200 - assert response.context["title"] == "Review and combine case recommendation - GBSIEL/2020/0002687/T - jim" + assert ( + response.context["title"] + == f"Review and combine case recommendation - {data_standard_case['case']['reference_code']} - {data_standard_case['case']['data']['organisation']['name']}" + ) def test_ConsolidateApproveView_mod_ecju_gov_user_GET( @@ -298,7 +328,10 @@ def test_ConsolidateApproveView_mod_ecju_gov_user_GET( data_standard_case["case"]["advice"] = mixed_advice response = authorized_client.get(consolidate_approve_url, follow=False) assert response.status_code == 200 - assert response.context["title"] == "Review and combine case recommendation - GBSIEL/2020/0002687/T - jim" + assert ( + response.context["title"] + == f"Review and combine case recommendation - {data_standard_case['case']['reference_code']} - {data_standard_case['case']['data']['organisation']['name']}" + ) def test_ConsolidateApproveView_POST_bad_input( @@ -651,3 +684,418 @@ def test_ConsolidateApproveView_mod_ecju_gov_user_POST_success( assert response.url == consolidate_view_url assert len(mock_post_approval_team_advice.request_history) == 1 assert mock_post_approval_team_advice.request_history[0].json() == expected_post_data + + +def test_ConsolidateRefuseView_GET_team_not_allowed_raises_exception( + authorized_client, + consolidate_refuse_url, + mixed_advice, + data_standard_case, +): + data_standard_case["case"]["advice"] = mixed_advice + + with pytest.raises(Exception) as err: + authorized_client.get(consolidate_refuse_url) + + assert str(err.value) == "Consolidate/combine operation not allowed for team 00000000-0000-0000-0000-000000000001" + + +def test_ConsolidateRefuseView_POST_team_not_allowed_raises_exception( + authorized_client, + consolidate_refuse_url, + mixed_advice, + data_standard_case, +): + data_standard_case["case"]["advice"] = mixed_advice + pass + + with pytest.raises(Exception) as err: + authorized_client.post(consolidate_refuse_url) + + assert str(err.value) == "Consolidate/combine operation not allowed for team 00000000-0000-0000-0000-000000000001" + + +def test_ConsolidateRefuseView_GET( + authorized_client, + consolidate_refuse_url, + mixed_advice, + data_standard_case, + mod_ecju_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.get(consolidate_refuse_url, follow=False) + assert response.status_code == 200 + assert ( + response.context["title"] + == f"Licence refused for case - {data_standard_case['case']['reference_code']} - {data_standard_case['case']['data']['organisation']['name']}" + ) + + +def test_ConsolidateRefuseView_POST_bad_input( + authorized_client, + consolidate_refuse_url, + mixed_advice, + data_standard_case, + mod_ecju_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post(consolidate_refuse_url, data={}) + assert response.status_code == 200 + form = response.context["form"] + assert form.errors == { + "denial_reasons": ["Select at least one refusal criteria"], + "refusal_reasons": ["Enter a reason for refusing"], + } + + +@pytest.fixture +def mock_post_refusal_team_advice(requests_mock, data_standard_case): + return requests_mock.post( + client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}/team-advice/"), + json={}, + ) + + +@pytest.mark.parametrize( + "refusal_data, expected_post_data", + ( + ( + {"denial_reasons": ["1"], "refusal_reasons": "you can't do that"}, + [ + { + "denial_reasons": ["1"], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote_required": False, + "is_refusal_note": False, + "text": "you can't do that", + "type": "refuse", + }, + { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE + "denial_reasons": ["1"], + "footnote_required": False, + "is_refusal_note": False, + "text": "you can't do that", + "type": "refuse", + }, + { + "denial_reasons": ["1"], + "footnote_required": False, + "is_refusal_note": False, + "text": "you can't do that", + "type": "refuse", + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + }, + { + "denial_reasons": ["1"], + "footnote_required": False, + "is_refusal_note": False, + "text": "you can't do that", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": "refuse", + }, + { + "denial_reasons": ["1"], + "footnote_required": False, + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "is_refusal_note": False, + "text": "you can't do that", + "type": "refuse", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "", + "type": "no_licence_required", + }, + ], + ), + ( + {"denial_reasons": ["1", "2a"], "refusal_reasons": "you can't do that"}, + [ + { + "denial_reasons": ["1", "2a"], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote_required": False, + "is_refusal_note": False, + "text": "you can't do that", + "type": "refuse", + }, + { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE + "denial_reasons": ["1", "2a"], + "footnote_required": False, + "is_refusal_note": False, + "text": "you can't do that", + "type": "refuse", + }, + { + "denial_reasons": ["1", "2a"], + "footnote_required": False, + "is_refusal_note": False, + "text": "you can't do that", + "type": "refuse", + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + }, + { + "denial_reasons": ["1", "2a"], + "footnote_required": False, + "is_refusal_note": False, + "text": "you can't do that", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": "refuse", + }, + { + "denial_reasons": ["1", "2a"], + "footnote_required": False, + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "is_refusal_note": False, + "text": "you can't do that", + "type": "refuse", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "", + "type": "no_licence_required", + }, + ], + ), + ), +) +def test_ConsolidateRefuseView_POST_success( + refusal_data, + expected_post_data, + authorized_client, + consolidate_refuse_url, + mixed_advice, + data_standard_case, + mod_ecju_gov_user, + mock_post_refusal_team_advice, + consolidate_view_url, +): + data_standard_case["case"]["advice"] = mixed_advice + data_standard_case["case"]["data"]["goods"][0]["is_good_controlled"] = {"key": "True", "value": "Yes"} + + response = authorized_client.post(consolidate_refuse_url, data=refusal_data) + assert response.status_code == 302 + assert response.url == consolidate_view_url + assert len(mock_post_refusal_team_advice.request_history) == 1 + assert mock_post_refusal_team_advice.request_history[0].json() == expected_post_data + + +# LU refusals... + + +def test_LUConsolidateRefuseView_GET_team_not_allowed_raises_exception( + authorized_client, + lu_consolidate_refuse_url, + mixed_advice, + data_standard_case, +): + data_standard_case["case"]["advice"] = mixed_advice + + with pytest.raises(Exception) as err: + authorized_client.get(lu_consolidate_refuse_url) + + assert str(err.value) == "Consolidate/combine operation not allowed for team 00000000-0000-0000-0000-000000000001" + + +def test_LUConsolidateRefuseView_POST_team_not_allowed_raises_exception( + authorized_client, + lu_consolidate_refuse_url, + mixed_advice, + data_standard_case, +): + data_standard_case["case"]["advice"] = mixed_advice + pass + + with pytest.raises(Exception) as err: + authorized_client.post(lu_consolidate_refuse_url) + + assert str(err.value) == "Consolidate/combine operation not allowed for team 00000000-0000-0000-0000-000000000001" + + +def test_LUConsolidateRefuseView_GET( + authorized_client, + lu_consolidate_refuse_url, + mixed_advice, + data_standard_case, + lu_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.get(lu_consolidate_refuse_url, follow=False) + assert response.status_code == 200 + assert ( + response.context["title"] + == f"Licence refused for case - {data_standard_case['case']['reference_code']} - {data_standard_case['case']['data']['organisation']['name']}" + ) + + +def test_LUConsolidateRefuseView_POST_bad_input( + authorized_client, + lu_consolidate_refuse_url, + mixed_advice, + data_standard_case, + lu_gov_user, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post(lu_consolidate_refuse_url, data={}) + assert response.status_code == 200 + form = response.context["form"] + assert form.errors == { + "denial_reasons": ["Select at least one refusal criteria"], + "refusal_note": ["Enter the refusal meeting note"], + } + + +@pytest.fixture +def mock_post_refusal_final_advice(requests_mock, data_standard_case): + return requests_mock.post( + client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}/final-advice/"), + json={}, + ) + + +@pytest.mark.parametrize( + "refusal_data, expected_post_data", + ( + ( + {"denial_reasons": ["1"], "refusal_note": "LU says no"}, + [ + { + "denial_reasons": ["1"], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote_required": False, + "is_refusal_note": True, + "text": "LU says no", + "type": "refuse", + }, + { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE + "denial_reasons": ["1"], + "footnote_required": False, + "is_refusal_note": True, + "text": "LU says no", + "type": "refuse", + }, + { + "denial_reasons": ["1"], + "footnote_required": False, + "is_refusal_note": True, + "text": "LU says no", + "type": "refuse", + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + }, + { + "denial_reasons": ["1"], + "footnote_required": False, + "is_refusal_note": True, + "text": "LU says no", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": "refuse", + }, + { + "denial_reasons": ["1"], + "footnote_required": False, + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "is_refusal_note": True, + "text": "LU says no", + "type": "refuse", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "", + "type": "no_licence_required", + }, + ], + ), + ( + {"denial_reasons": ["1", "2a"], "refusal_note": "LU says no"}, + [ + { + "denial_reasons": ["1", "2a"], + "end_user": "95d3ea36-6ab9-41ea-a744-7284d17b9cc5", + "footnote_required": False, + "is_refusal_note": True, + "text": "LU says no", + "type": "refuse", + }, + { + "consignee": "cd2263b4-a427-4f14-8552-505e1d192bb8", # /PS-IGNORE + "denial_reasons": ["1", "2a"], + "footnote_required": False, + "is_refusal_note": True, + "text": "LU says no", + "type": "refuse", + }, + { + "denial_reasons": ["1", "2a"], + "footnote_required": False, + "is_refusal_note": True, + "text": "LU says no", + "type": "refuse", + "ultimate_end_user": "9f077b3c-6116-4111-b9a0-b2491198aa72", + }, + { + "denial_reasons": ["1", "2a"], + "footnote_required": False, + "is_refusal_note": True, + "text": "LU says no", + "third_party": "95c2d6b7-5cfd-47e8-b3c8-dc76e1ac9747", + "type": "refuse", + }, + { + "denial_reasons": ["1", "2a"], + "footnote_required": False, + "good": "0bedd1c3-cf97-4aad-b711-d5c9a9f4586e", + "is_refusal_note": True, + "text": "LU says no", + "type": "refuse", + }, + { + "denial_reasons": [], + "footnote": "", + "footnote_required": False, + "good": "6daad1c3-cf97-4aad-b711-d5c9a9f4586e", + "note": "", + "proviso": "", + "text": "", + "type": "no_licence_required", + }, + ], + ), + ), +) +def test_LUConsolidateRefuseView_POST_success( + refusal_data, + expected_post_data, + authorized_client, + lu_consolidate_refuse_url, + mixed_advice, + data_standard_case, + lu_gov_user, + mock_post_refusal_final_advice, + consolidate_view_url, +): + data_standard_case["case"]["advice"] = mixed_advice + data_standard_case["case"]["data"]["goods"][0]["is_good_controlled"] = {"key": "True", "value": "Yes"} + + response = authorized_client.post(lu_consolidate_refuse_url, data=refusal_data) + assert response.status_code == 302 + assert response.url == consolidate_view_url + assert len(mock_post_refusal_final_advice.request_history) == 1 + assert mock_post_refusal_final_advice.request_history[0].json() == expected_post_data diff --git a/unit_tests/caseworker/advice/views/test_consolidate.py b/unit_tests/caseworker/advice/views/test_consolidate.py index 36373d6aa..7c324ca89 100644 --- a/unit_tests/caseworker/advice/views/test_consolidate.py +++ b/unit_tests/caseworker/advice/views/test_consolidate.py @@ -6,9 +6,7 @@ from caseworker.advice import forms from caseworker.advice import services from caseworker.advice.services import ( - FCDO_TEAM, LICENSING_UNIT_TEAM, - MOD_CONSOLIDATE_TEAMS, MOD_ECJU_TEAM, MANPADS_ID, AP_LANDMINE_ID, @@ -303,48 +301,6 @@ def gov_user(): } -@pytest.mark.parametrize( - "path, form_class, team_alias, team_name", - ( - ("refuse/", forms.LUConsolidateRefusalForm, LICENSING_UNIT_TEAM, "LU Team"), - ("refuse/", forms.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 - - @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 58ef8b3fd..206308d7f 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.views.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", # /PS-IGNORE - "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 6bf123ba6489c8611f14d3754f47ec803f5ca44e Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Wed, 4 Dec 2024 16:36:55 +0000 Subject: [PATCH 7/9] Fix coverage complaint --- caseworker/advice/views/consolidate_advice.py | 10 +- .../views/tests/test_consolidate_advice.py | 93 ++++++++++++++++++- 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/caseworker/advice/views/consolidate_advice.py b/caseworker/advice/views/consolidate_advice.py index 012f9daa8..08c1ef093 100644 --- a/caseworker/advice/views/consolidate_advice.py +++ b/caseworker/advice/views/consolidate_advice.py @@ -104,9 +104,8 @@ def form_valid(self, form): level = "final-advice" if self.team_alias == services.LICENSING_UNIT_TEAM else "team-advice" try: services.post_approval_advice(self.request, self.case, form.cleaned_data, level=level) - except HTTPError as e: - errors = e.response.json()["errors"] - form.add_error(None, errors) + except HTTPError: + form.add_error(None, ["An error occurred when saving consolidated advice"]) return super().form_invalid(form) return super().form_valid(form) @@ -129,9 +128,8 @@ def form_valid(self, form): try: data = self.build_refusal_payload(form.cleaned_data) services.post_refusal_advice(self.request, self.case, data, level=self.advice_level) - except HTTPError as e: - errors = e.response.json()["errors"] - form.add_error(None, errors) + except HTTPError: + form.add_error(None, ["An error occurred when saving consolidated advice"]) return super().form_invalid(form) return super().form_valid(form) diff --git a/caseworker/advice/views/tests/test_consolidate_advice.py b/caseworker/advice/views/tests/test_consolidate_advice.py index 6289feedf..ecfb2d29c 100644 --- a/caseworker/advice/views/tests/test_consolidate_advice.py +++ b/caseworker/advice/views/tests/test_consolidate_advice.py @@ -143,6 +143,11 @@ def mod_ecju_gov_user(requests_mock, gov_user): ) +################################# +# ConsolidateSelectDecisionView +################################# + + def test_ConsolidateSelectDecisionView_GET_team_not_allowed_raises_exception( authorized_client, consolidate_select_decision_url, @@ -273,6 +278,11 @@ def test_ConsolidateSelectDecisionView_POST_lu_refuse_success( assert response.url == lu_consolidate_refuse_url +######################## +# ConsolidateApproveView +######################## + + def test_ConsolidateApproveView_GET_team_not_allowed_raises_exception( authorized_client, consolidate_approve_url, @@ -686,6 +696,35 @@ def test_ConsolidateApproveView_mod_ecju_gov_user_POST_success( assert mock_post_approval_team_advice.request_history[0].json() == expected_post_data +@pytest.fixture +def mock_post_approval_team_advice_server_error(requests_mock, data_standard_case): + return requests_mock.post( + client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}/team-advice/"), + json={}, + status_code=500, + ) + + +def test_ConsolidateApproveView_POST_server_error( + authorized_client, + consolidate_approve_url, + mixed_advice, + data_standard_case, + mod_ecju_gov_user, + mock_post_approval_team_advice_server_error, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post(consolidate_approve_url, data={"approval_reasons": "yep, go for it"}) + assert response.status_code == 200 + form = response.context["form"] + assert form.errors == {"__all__": ["An error occurred when saving consolidated advice"]} + + +######################## +# ConsolidateRefuseView +######################## + + def test_ConsolidateRefuseView_GET_team_not_allowed_raises_exception( authorized_client, consolidate_refuse_url, @@ -892,7 +931,33 @@ def test_ConsolidateRefuseView_POST_success( assert mock_post_refusal_team_advice.request_history[0].json() == expected_post_data -# LU refusals... +@pytest.fixture +def mock_post_refusal_team_advice_server_error(requests_mock, data_standard_case): + return requests_mock.post( + client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}/team-advice/"), + json={}, + status_code=500, + ) + + +def test_ConsolidateRefuseView_POST_server_error( + authorized_client, + consolidate_refuse_url, + mixed_advice, + data_standard_case, + mod_ecju_gov_user, + mock_post_refusal_team_advice_server_error, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post( + consolidate_refuse_url, data={"denial_reasons": ["1", "2a"], "refusal_reasons": "you can't do that"} + ) + assert response.status_code == 200 + form = response.context["form"] + assert form.errors == {"__all__": ["An error occurred when saving consolidated advice"]} + + +# LUConsolidatRefuseView def test_LUConsolidateRefuseView_GET_team_not_allowed_raises_exception( @@ -1099,3 +1164,29 @@ def test_LUConsolidateRefuseView_POST_success( assert response.url == consolidate_view_url assert len(mock_post_refusal_final_advice.request_history) == 1 assert mock_post_refusal_final_advice.request_history[0].json() == expected_post_data + + +@pytest.fixture +def mock_post_refusal_final_advice_server_error(requests_mock, data_standard_case): + return requests_mock.post( + client._build_absolute_uri(f"/cases/{data_standard_case['case']['id']}/final-advice/"), + json={}, + status_code=500, + ) + + +def test_LUConsolidateRefuseView_POST_server_error( + authorized_client, + lu_consolidate_refuse_url, + mixed_advice, + data_standard_case, + lu_gov_user, + mock_post_refusal_final_advice_server_error, +): + data_standard_case["case"]["advice"] = mixed_advice + response = authorized_client.post( + lu_consolidate_refuse_url, data={"denial_reasons": ["1", "2a"], "refusal_note": "you can't do that"} + ) + assert response.status_code == 200 + form = response.context["form"] + assert form.errors == {"__all__": ["An error occurred when saving consolidated advice"]} From 927e3be1c74c366c9e7f557cc462fb95d9b4df30 Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Wed, 4 Dec 2024 17:39:40 +0000 Subject: [PATCH 8/9] Add TODO --- caseworker/advice/views/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/caseworker/advice/views/views.py b/caseworker/advice/views/views.py index 2e864a4be..dec7db031 100644 --- a/caseworker/advice/views/views.py +++ b/caseworker/advice/views/views.py @@ -660,6 +660,7 @@ def get_context_data(self, *args, **kwargs): return context +# TODO: Delete this when ConsolidateEditView (below) is moved over/refactored in views/consolidate_advice.py class ReviewConsolidateView(LoginRequiredMixin, CaseContextMixin, FormView): template_name = "advice/review_consolidate.html" From 31068345a0e845b5e61f1120047308392e280b6c Mon Sep 17 00:00:00 2001 From: Brendan Smith Date: Fri, 6 Dec 2024 15:49:20 +0000 Subject: [PATCH 9/9] Group consolidation advice by team AND decision to avoid grouping conflicting advice --- caseworker/advice/services.py | 16 +- caseworker/advice/tests/test_services.py | 386 +++++++++++++++++++---- 2 files changed, 340 insertions(+), 62 deletions(-) diff --git a/caseworker/advice/services.py b/caseworker/advice/services.py index 921576928..2882cbe15 100644 --- a/caseworker/advice/services.py +++ b/caseworker/advice/services.py @@ -186,12 +186,18 @@ def group_advice_by_user(advice): return result -def group_advice_by_team(advice, exclude_good_advice=True): +def group_advice_by_team(advice): result = defaultdict(list) for item in advice: - if exclude_good_advice and item.get("good"): - continue - result[item["team"]["id"]].append(item) + if not item.get("good"): + result[item["team"]["id"]].append(item) + return result + + +def group_advice_by_team_and_decision(advice): + result = defaultdict(list) + for item in advice: + result[f"{item['team']['id']}-{item['type']['key']}"].append(item) return result @@ -289,7 +295,7 @@ def get_advice_to_consolidate(advice, user_team_alias): else: raise Exception(f"Consolidate/combine operation not allowed for team {user_team_alias}") - return group_advice_by_team(advice_from_teams, exclude_good_advice=False) + return group_advice_by_team_and_decision(advice_from_teams) def order_by_party_type(all_advice): diff --git a/caseworker/advice/tests/test_services.py b/caseworker/advice/tests/test_services.py index 93d86c45e..ea658a58a 100644 --- a/caseworker/advice/tests/test_services.py +++ b/caseworker/advice/tests/test_services.py @@ -28,58 +28,221 @@ def test_get_advice_to_consolidate_unrecognized_team_raises_exception(): # FCDO advice only ( [ - {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + { + "id": "advice-1", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, ], - {"id-fcdo": [{"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}]}, + { + "id-fcdo-approve": [ + { + "id": "advice-1", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + } + ] + }, ), # FCDO and MOD-DI advice only ( [ - {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-3", "level": "team", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + { + "id": "advice-1", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-2", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-3", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, ], { - "id-fcdo": [ - {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + "id-fcdo-approve": [ + { + "id": "advice-1", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-2", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, ], - "id-mod-di": [ - {"id": "advice-3", "level": "team", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + "id-mod-di-approve": [ + { + "id": "advice-3", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, ], }, ), # All OGDs advising with MOD collected under MOD-ECJU ( [ - {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-3", "level": "team", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, - {"id": "advice-4", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, - {"id": "advice-5", "level": "user", "team": {"alias": DESNZ_NUCLEAR, "id": "id-desnz-nuclear"}}, - {"id": "advice-6", "level": "user", "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}}, - {"id": "advice-7", "level": "user", "team": {"alias": NCSC_TEAM, "id": "id-ncsc"}}, - {"id": "advice-8", "level": "user", "team": {"alias": MOD_CAPPROT_TEAM, "id": "id-mod-capprot"}}, - {"id": "advice-9", "level": "user", "team": {"alias": MOD_DSR_TEAM, "id": "id-mod-dsr"}}, - {"id": "advice-10", "level": "user", "team": {"alias": MOD_DSTL_TEAM, "id": "id-mod-dstl"}}, - {"id": "advice-11", "level": "team", "team": {"alias": MOD_ECJU_TEAM, "id": "id-mod-ecju"}}, + { + "id": "advice-1", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-2", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-2", + "level": "team", + "type": {"key": "proviso"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-3", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, + { + "id": "advice-4", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, + { + "id": "advice-4", + "level": "user", + "type": {"key": "refuse"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, + { + "id": "advice-5", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": DESNZ_NUCLEAR, "id": "id-desnz-nuclear"}, + }, + { + "id": "advice-6", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}, + }, + { + "id": "advice-6", + "level": "user", + "type": {"key": "refuse"}, + "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}, + }, + { + "id": "advice-7", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": NCSC_TEAM, "id": "id-ncsc"}, + }, + { + "id": "advice-8", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_CAPPROT_TEAM, "id": "id-mod-capprot"}, + }, + { + "id": "advice-9", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DSR_TEAM, "id": "id-mod-dsr"}, + }, + { + "id": "advice-10", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DSTL_TEAM, "id": "id-mod-dstl"}, + }, + { + "id": "advice-11", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": MOD_ECJU_TEAM, "id": "id-mod-ecju"}, + }, ], { - "id-fcdo": [ - {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + "id-fcdo-approve": [ + { + "id": "advice-1", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-2", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + ], + "id-fcdo-proviso": [ + { + "id": "advice-2", + "level": "team", + "type": {"key": "proviso"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, ], - "id-desnz-nuclear": [ - {"id": "advice-5", "level": "user", "team": {"alias": DESNZ_NUCLEAR, "id": "id-desnz-nuclear"}}, + "id-desnz-nuclear-approve": [ + { + "id": "advice-5", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": DESNZ_NUCLEAR, "id": "id-desnz-nuclear"}, + }, ], - "id-desnz-chemical": [ - {"id": "advice-6", "level": "user", "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}}, + "id-desnz-chemical-approve": [ + { + "id": "advice-6", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}, + }, ], - "id-ncsc": [ - {"id": "advice-7", "level": "user", "team": {"alias": NCSC_TEAM, "id": "id-ncsc"}}, + "id-desnz-chemical-refuse": [ + { + "id": "advice-6", + "level": "user", + "type": {"key": "refuse"}, + "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}, + }, ], - "id-mod-ecju": [ - {"id": "advice-11", "level": "team", "team": {"alias": MOD_ECJU_TEAM, "id": "id-mod-ecju"}}, + "id-ncsc-approve": [ + { + "id": "advice-7", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": NCSC_TEAM, "id": "id-ncsc"}, + }, + ], + "id-mod-ecju-approve": [ + { + "id": "advice-11", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": MOD_ECJU_TEAM, "id": "id-mod-ecju"}, + }, ], }, ), @@ -97,49 +260,158 @@ def test_get_advice_to_consolidate_lu(advice, expected_grouping): # FCDO advice only ( [ - {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, + { + "id": "advice-1", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, ], {}, ), # FCDO and MOD-DI advice only ( [ - {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-3", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + { + "id": "advice-1", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-2", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-3", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, + { + "id": "advice-3", + "level": "user", + "type": {"key": "refuse"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, ], { - "id-mod-di": [ - {"id": "advice-3", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, - ] + "id-mod-di-approve": [ + { + "id": "advice-3", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, + ], + "id-mod-di-refuse": [ + { + "id": "advice-3", + "level": "user", + "type": {"key": "refuse"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, + ], }, ), # All OGDs advising with MOD collected under MOD-ECJU ( [ - {"id": "advice-1", "level": "user", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-2", "level": "team", "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}}, - {"id": "advice-3", "level": "team", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, - {"id": "advice-4", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, - {"id": "advice-5", "level": "user", "team": {"alias": DESNZ_NUCLEAR, "id": "id-desnz-nuclear"}}, - {"id": "advice-6", "level": "user", "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}}, - {"id": "advice-7", "level": "user", "team": {"alias": NCSC_TEAM, "id": "id-ncsc"}}, - {"id": "advice-8", "level": "user", "team": {"alias": MOD_CAPPROT_TEAM, "id": "id-mod-capprot"}}, - {"id": "advice-9", "level": "user", "team": {"alias": MOD_DSR_TEAM, "id": "id-mod-dsr"}}, - {"id": "advice-10", "level": "user", "team": {"alias": MOD_DSTL_TEAM, "id": "id-mod-dstl"}}, + { + "id": "advice-1", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-2", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": FCDO_TEAM, "id": "id-fcdo"}, + }, + { + "id": "advice-3", + "level": "team", + "type": {"key": "approve"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, + { + "id": "advice-4", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, + { + "id": "advice-5", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": DESNZ_NUCLEAR, "id": "id-desnz-nuclear"}, + }, + { + "id": "advice-6", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": DESNZ_CHEMICAL, "id": "id-desnz-chemical"}, + }, + { + "id": "advice-7", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": NCSC_TEAM, "id": "id-ncsc"}, + }, + { + "id": "advice-8", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_CAPPROT_TEAM, "id": "id-mod-capprot"}, + }, + { + "id": "advice-9", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DSR_TEAM, "id": "id-mod-dsr"}, + }, + { + "id": "advice-10", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DSTL_TEAM, "id": "id-mod-dstl"}, + }, ], { - "id-mod-di": [ - {"id": "advice-4", "level": "user", "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}}, + "id-mod-di-approve": [ + { + "id": "advice-4", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DI_TEAM, "id": "id-mod-di"}, + }, ], - "id-mod-capprot": [ - {"id": "advice-8", "level": "user", "team": {"alias": MOD_CAPPROT_TEAM, "id": "id-mod-capprot"}}, + "id-mod-capprot-approve": [ + { + "id": "advice-8", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_CAPPROT_TEAM, "id": "id-mod-capprot"}, + }, ], - "id-mod-dsr": [ - {"id": "advice-9", "level": "user", "team": {"alias": MOD_DSR_TEAM, "id": "id-mod-dsr"}}, + "id-mod-dsr-approve": [ + { + "id": "advice-9", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DSR_TEAM, "id": "id-mod-dsr"}, + }, ], - "id-mod-dstl": [ - {"id": "advice-10", "level": "user", "team": {"alias": MOD_DSTL_TEAM, "id": "id-mod-dstl"}}, + "id-mod-dstl-approve": [ + { + "id": "advice-10", + "level": "user", + "type": {"key": "approve"}, + "team": {"alias": MOD_DSTL_TEAM, "id": "id-mod-dstl"}, + }, ], }, ),