diff --git a/web/domains/case/access/forms.py b/web/domains/case/access/forms.py index e419a647b..05c69bc2e 100644 --- a/web/domains/case/access/forms.py +++ b/web/domains/case/access/forms.py @@ -63,23 +63,7 @@ class Meta: ] -class LinkOrgAccessRequestFormBase(ModelForm): - instance: ImporterAccessRequest | ExporterAccessRequest - - def clean(self) -> dict[str, Any]: - cleaned_data = super().clean() - - if self.instance.is_agent_request: - link = cleaned_data.get("link") - agent_link = cleaned_data.get("agent_link") - - if agent_link and agent_link.get_main_org() != link: - self.add_error("agent_link", "Agent organisation is not linked to main org.") - - return cleaned_data - - -class LinkImporterAccessRequestForm(LinkOrgAccessRequestFormBase): +class LinkImporterAccessRequestForm(ModelForm): link = ModelChoiceField( label="Link Importer", help_text=( @@ -91,24 +75,12 @@ class LinkImporterAccessRequestForm(LinkOrgAccessRequestFormBase): widget=ImporterWidget, ) - agent_link = ModelChoiceField( - label="Link Agent Importer", - help_text=( - "Search an agent importer to link." - " Importers returned are matched against name, registerer number" - ", eori number and user name/email." - ), - queryset=Importer.objects.filter(is_active=True, main_importer__isnull=False), - widget=ImporterAgentWidget, - required=False, - ) - class Meta: model = ImporterAccessRequest - fields = ["link", "agent_link"] + fields = ["link"] -class LinkExporterAccessRequestForm(LinkOrgAccessRequestFormBase): +class LinkExporterAccessRequestForm(ModelForm): link = ModelChoiceField( label="Link Exporter", help_text=( @@ -119,20 +91,72 @@ class LinkExporterAccessRequestForm(LinkOrgAccessRequestFormBase): widget=ExporterWidget, ) + class Meta: + model = ExporterAccessRequest + fields = ["link"] + + +class LinkOrgAgentFormBase(ModelForm): + instance: ImporterAccessRequest | ExporterAccessRequest + + def clean(self) -> dict[str, Any]: + cleaned_data = super().clean() + + link = self.instance.link + agent_link = cleaned_data.get("agent_link") + + if agent_link and agent_link.get_main_org() != link: + self.add_error("agent_link", "Agent organisation is not linked to main organisation.") + + return cleaned_data + + +class LinkImporterAgentForm(LinkOrgAgentFormBase): + class Meta: + model = ImporterAccessRequest + fields = ["agent_link"] + + agent_link = ModelChoiceField( + label="Link Agent Importer", + help_text=( + "Search an agent importer to link." + " Importers returned are matched against name, registerer number" + ", eori number and user name/email." + ), + queryset=Importer.objects.none(), + widget=ImporterAgentWidget(attrs={"data-minimum-input-length": 0}), + required=True, + ) + + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self.fields["agent_link"].queryset = Importer.objects.filter( + is_active=True, main_importer=self.instance.link + ) + + +class LinkExporterAgentForm(LinkOrgAgentFormBase): + class Meta: + model = ExporterAccessRequest + fields = ["agent_link"] + agent_link = ModelChoiceField( label="Link Agent Exporter", help_text=( "Search an agent exporter to link." " Exporters returned are matched against name and registerer number." ), - queryset=Exporter.objects.filter(is_active=True, main_exporter__isnull=False), - widget=ExporterAgentWidget, - required=False, + queryset=Exporter.objects.none(), + widget=ExporterAgentWidget(attrs={"data-minimum-input-length": 0}), + required=True, ) - class Meta: - model = ExporterAccessRequest - fields = ["link", "agent_link"] + def __init__(self, *args: Any, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + + self.fields["agent_link"].queryset = Exporter.objects.filter( + is_active=True, main_exporter=self.instance.link + ) class CloseAccessRequestForm(ModelForm): diff --git a/web/domains/case/access/urls.py b/web/domains/case/access/urls.py index b35fe057a..7f5363fc3 100644 --- a/web/domains/case/access/urls.py +++ b/web/domains/case/access/urls.py @@ -17,6 +17,11 @@ views.link_access_request, name="link-request", ), + path( + "case///link-access-request-agent/", + views.LinkOrgAgentAccessRequestUpdateView.as_view(), + name="link-access-request-agent", + ), re_path( "^case/(?P[0-9]+)/(?Pimporter|exporter)/close-access-request/$", views.close_access_request, diff --git a/web/domains/case/access/views.py b/web/domains/case/access/views.py index c38d7f437..3e4bd57bf 100644 --- a/web/domains/case/access/views.py +++ b/web/domains/case/access/views.py @@ -2,14 +2,19 @@ from django.contrib import messages from django.contrib.auth.decorators import login_required, permission_required -from django.contrib.auth.mixins import PermissionRequiredMixin +from django.contrib.auth.mixins import ( + LoginRequiredMixin, + PermissionRequiredMixin, + UserPassesTestMixin, +) from django.db import transaction from django.db.models import QuerySet -from django.http import HttpResponse +from django.http import HttpResponse, HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.utils import timezone -from django.views.generic import DetailView, TemplateView +from django.utils.decorators import method_decorator +from django.views.generic import DetailView, TemplateView, UpdateView from django_ratelimit import UNSAFE from django_ratelimit.decorators import ratelimit @@ -18,6 +23,7 @@ ImporterAccessRequestFilter, ) from web.domains.case.services import case_progress, reference +from web.flow.errors import ProcessError from web.flow.models import ProcessTypes from web.mail import emails from web.models import ( @@ -237,9 +243,15 @@ def link_access_request( ) form = form_cls(request.POST, instance=access_request) + original_link_pk = access_request.link.pk if access_request.link else None if form.is_valid(): - form.save() + saved_ar = form.save() + + # Reset agent if org changes. + if saved_ar.link.pk != original_link_pk: + saved_ar.agent_link = None + saved_ar.save() messages.success( request, @@ -256,12 +268,25 @@ def link_access_request( else: form = form_cls(instance=access_request) + if access_request.is_agent_request: + agent_form_cls = ( + forms.LinkImporterAgentForm if entity == "importer" else forms.LinkExporterAgentForm + ) + agent_form = agent_form_cls(instance=access_request) + else: + agent_form = None + context = { "case_type": "access", "process": access_request, "has_approval_request": has_approval_request, "form": form, "show_agent_link": access_request.is_agent_request, + "link_access_request_agent_url": reverse( + "access:link-access-request-agent", + kwargs={"access_request_pk": access_request_pk, "entity": entity}, + ), + "agent_form": agent_form, "org": org, "create_org_url": reverse("importer-list" if entity == "importer" else "exporter-list"), } @@ -271,6 +296,57 @@ def link_access_request( ) +@method_decorator(transaction.atomic, name="post") +class LinkOrgAgentAccessRequestUpdateView( + LoginRequiredMixin, PermissionRequiredMixin, UserPassesTestMixin, UpdateView +): + permission_required = [Perms.sys.ilb_admin] + pk_url_kwarg = "access_request_pk" + model = AccessRequest + object: ImporterAccessRequest | ExporterAccessRequest + http_method_names = ["post"] + + def get_queryset(self): + # Call select_for_update for atomic post method + return super().get_queryset().select_for_update() + + def get_object( + self, queryset: QuerySet[ImporterAccessRequest | ExporterAccessRequest] | None = None + ) -> ImporterAccessRequest | ExporterAccessRequest: + obj = super().get_object(queryset) + + return obj.get_specific_model() + + def test_func(self): + # Override queryset so select_for_update is not called + access_request = self.get_object(AccessRequest.objects.all()) + + try: + case_progress.access_request_in_processing(access_request) + except ProcessError: + return False + + return True and access_request.is_agent_request + + def get_form_class(self): + entity = self.kwargs["entity"] + + return forms.LinkImporterAgentForm if entity == "importer" else forms.LinkExporterAgentForm + + def form_invalid( + self, form: forms.LinkImporterAgentForm | forms.LinkExporterAgentForm + ) -> HttpResponseRedirect: + # This is a post only endpoint so unable to display form errors like normal. + messages.warning(self.request, "Unable to link agent.") + return redirect(self.get_success_url()) + + def get_success_url(self) -> str: + return reverse( + "access:link-request", + kwargs={"access_request_pk": self.object.pk, "entity": self.kwargs["entity"]}, + ) + + @login_required @permission_required(Perms.sys.ilb_admin, raise_exception=True) def close_access_request( @@ -295,6 +371,8 @@ def close_access_request( is_active=True, status=ApprovalRequest.Statuses.OPEN ) + agent_missing = access_request.is_agent_request and not access_request.agent_link + if request.method == "POST": if has_open_approval_request: messages.error( @@ -311,6 +389,19 @@ def close_access_request( ) ) + if agent_missing: + messages.error( + request, + "You cannot close this Access Request because you need to link the agent.", + ) + + return redirect( + reverse( + "access:close-request", + kwargs={"access_request_pk": access_request.pk, "entity": entity}, + ) + ) + task = case_progress.get_expected_task(access_request, Task.TaskType.PROCESS) form = forms.CloseAccessRequestForm(request.POST, instance=access_request) @@ -345,6 +436,7 @@ def close_access_request( "process": access_request, "form": form, "has_open_approval_request": has_open_approval_request, + "agent_missing": agent_missing, } return render( @@ -354,7 +446,7 @@ def close_access_request( ) -class AccessRequestHistoryView(PermissionRequiredMixin, DetailView): +class AccessRequestHistoryView(LoginRequiredMixin, PermissionRequiredMixin, DetailView): # DetailView Config model = AccessRequest pk_url_kwarg = "access_request_pk" diff --git a/web/templates/web/domains/case/access/close-access-request.html b/web/templates/web/domains/case/access/close-access-request.html index 629e373ee..f7726ab19 100644 --- a/web/templates/web/domains/case/access/close-access-request.html +++ b/web/templates/web/domains/case/access/close-access-request.html @@ -15,6 +15,10 @@

Close Access Request

You cannot close this Access Request because you have already started the Approval Process. To close this Access Request you must first withdraw the Approval Request.
+ {% elif agent_missing %} +
+ You cannot close this Access Request because you need to link the agent. +
{% else %}
{% call forms.form(action='', method='post', csrf_input=csrf_input) -%} diff --git a/web/templates/web/domains/case/access/management.html b/web/templates/web/domains/case/access/management.html index 483712aa3..6ef2774d8 100644 --- a/web/templates/web/domains/case/access/management.html +++ b/web/templates/web/domains/case/access/management.html @@ -66,21 +66,37 @@

Link Importer to Access Request

You cannot re-link this Access Request because you have already started the Approval Process. You must first Withdraw / Restart the Approval Request.
{% else %} +

+ If the {{ org }} is not found in the below list, click this link + to create a new {{ org }} (opens in new tab) before returning here to complete the access request. You may need to reload the browser page to see the new {{ org }}. +

{% call forms.form(method='post', csrf_input=csrf_input) -%} - {{ fields.field(form.link) }} - {% if show_agent_link %} - {{ fields.field(form.agent_link, show_optional_indicator=False) }} - {% endif %} -

- If the {{ org }} is not found in the above list, click this link - to create a new {{ org }} (opens in new tab) before returning here to complete the access request. You may need to reload the browser page to see the new {{ org }}. -

- + {% for field in form %} + {{ fields.field(field) }} + {{ forms.submit_button(btn_label="Link") }} + {% endfor %} {% endcall %}
{% endif %} +{% if show_agent_link %} +
+

Link Agent to Access Request

+ {% if process.link %} +
+ {% call forms.form(action=link_access_request_agent_url, method='post', csrf_input=csrf_input) -%} + {% for field in agent_form %} + {{ fields.field(field) }} + {{ forms.submit_button(btn_label="Link agent") }} + {% endfor %} + {% endcall %} +
+ {% else %} +
You need to link the {{ org }} before linking the agent
+ {% endif %} +
+{% endif %} {% endblock %} {% block page_js %} diff --git a/web/tests/domains/case/access/test_views.py b/web/tests/domains/case/access/test_views.py index 0e02b1322..ad2440066 100644 --- a/web/tests/domains/case/access/test_views.py +++ b/web/tests/domains/case/access/test_views.py @@ -245,21 +245,6 @@ def test_link_importer_access_request(self): assert self.iar.link == self.importer - def test_link_importer_agent_access_request(self): - assert self.iar.link is None - assert self.iar.agent_link is None - - data = {"link": self.importer.pk, "agent_link": self.importer_agent.pk} - - response = self.ilb_admin_client.post(self.iar_url, data=data) - - assertRedirects(response, self.iar_url) - - self.iar.refresh_from_db() - - assert self.iar.link == self.importer - assert self.iar.agent_link == self.importer_agent - def test_link_exporter_access_request(self): assert self.ear.link is None @@ -273,21 +258,6 @@ def test_link_exporter_access_request(self): assert self.ear.link == self.exporter - def test_link_exporter_agent_access_request(self): - assert self.ear.link is None - assert self.ear.agent_link is None - - data = {"link": self.exporter.pk, "agent_link": self.exporter_agent.pk} - - response = self.ilb_admin_client.post(self.ear_url, data=data) - - assertRedirects(response, self.ear_url) - - self.ear.refresh_from_db() - - assert self.ear.link == self.exporter - assert self.ear.agent_link == self.exporter_agent - def test_unable_to_link_access_request_with_open_approval(self): # Link the access request data = {"link": self.importer.pk} @@ -315,6 +285,91 @@ def test_unable_to_link_access_request_with_open_approval(self): ) +class TestLinkOrgAgentAccessRequestUpdateView(AuthTestCase): + @pytest.fixture(autouse=True) + def setup(self, _setup, importer, importer_access_request, exporter, exporter_access_request): + self.iar = importer_access_request + self.iar.request_type = ImporterAccessRequest.AGENT_ACCESS + self.iar.link = importer + self.iar.save() + + self.iar_url = reverse( + "access:link-access-request-agent", + kwargs={"access_request_pk": importer_access_request.pk, "entity": "importer"}, + ) + + self.ear = exporter_access_request + self.ear.request_type = ExporterAccessRequest.AGENT_ACCESS + self.ear.link = exporter + self.ear.save() + + self.ear_url = reverse( + "access:link-access-request-agent", + kwargs={"access_request_pk": exporter_access_request.pk, "entity": "exporter"}, + ) + + def test_link_importer_agent_access_request(self, ilb_admin_user): + assert self.iar.agent_link is None + + data = {"agent_link": self.importer_agent.pk} + response = self.ilb_admin_client.post(self.iar_url, data=data, follow=True) + + assert response.resolver_match.view_name == "access:link-request" + + self.iar.refresh_from_db() + + assert self.iar.agent_link == self.importer_agent + + def test_link_exporter_agent_access_request(self): + assert self.ear.agent_link is None + + data = {"agent_link": self.exporter_agent.pk} + + response = self.ilb_admin_client.post(self.ear_url, data=data, follow=True) + assert response.resolver_match.view_name == "access:link-request" + + self.ear.refresh_from_db() + assert self.ear.agent_link == self.exporter_agent + + def test_invalid_importer_agent_chosen( + self, agent_importer, importer_two, agent_exporter, exporter_two + ): + # Link importer 1 agent to importer 2 + agent_importer.main_importer = importer_two + agent_importer.save() + + assert self.iar.agent_link is None + + data = {"agent_link": agent_importer.pk} + response = self.ilb_admin_client.post(self.iar_url, data=data, follow=True) + assert response.resolver_match.view_name == "access:link-request" + + messages = get_messages_from_response(response) + assert len(messages) == 1 + assert messages[0] == "Unable to link agent." + + self.iar.refresh_from_db() + assert self.iar.agent_link is None + + def test_invalid_exporter_agent_chosen(self, agent_exporter, exporter_two): + # Link exporter 1 agent to exporter 2 + agent_exporter.main_exporter = exporter_two + agent_exporter.save() + + assert self.ear.agent_link is None + + data = {"agent_link": agent_exporter.pk} + response = self.ilb_admin_client.post(self.ear_url, data=data, follow=True) + assert response.resolver_match.view_name == "access:link-request" + + messages = get_messages_from_response(response) + assert len(messages) == 1 + assert messages[0] == "Unable to link agent." + + self.ear.refresh_from_db() + assert self.ear.agent_link is None + + class TestCloseAccessRequest(AuthTestCase): @pytest.fixture(autouse=True) def setup(self, _setup, access_request_user, importer_access_request, exporter_access_request): @@ -652,6 +707,41 @@ def test_unable_to_close_access_request_with_open_approval(self): "Process. To close this Access Request you must first withdraw the Approval Request." ) + def test_unable_to_close_access_request_when_agent_not_linked(self): + # Setup the iar to be an agent access request. + self.iar.request_type = self.iar.AGENT_ACCESS + self.iar.agent_link = None + self.iar.save() + + # Link the access request + link_url = reverse( + "access:link-request", kwargs={"access_request_pk": self.iar.pk, "entity": "importer"} + ) + data = {"link": self.importer.pk} + response = self.ilb_admin_client.post(link_url, data=data) + assert response.status_code == HTTPStatus.FOUND + + # Go to close page and expect to see a message saying we can't close + response = self.ilb_admin_client.get(self.iar_url) + assert response.status_code == HTTPStatus.OK + + assertInHTML( + "You cannot close this Access Request because you need to link the agent.", + response.content.decode(), + ) + + # Check posting to the endpoint also prevents closing the access request. + response = self.ilb_admin_client.post( + self.iar_url, data={"response": AccessRequest.APPROVED} + ) + assert response.status_code == HTTPStatus.FOUND + + messages = get_messages_from_response(response) + assert len(messages) == 1 + assert messages[0] == ( + "You cannot close this Access Request because you need to link the agent." + ) + class TestAccessRequestHistoryView(AuthTestCase): @pytest.fixture(autouse=True)