diff --git a/.env.example b/.env.example index b7d7cb00a..03bf4f9e0 100644 --- a/.env.example +++ b/.env.example @@ -28,5 +28,7 @@ SEND_LICENCE_TO_CHIEF=False SET_INACTIVE_APP_TYPES_ACTIVE=False -# To send emails set GOV_NOTIFY_API_KEY to the development API key in the dev vault -GOV_NOTIFY_API_KEY= \ No newline at end of file +# To send/receive emails (which are implemented using gov notify) set the following +GOV_NOTIFY_API_KEY="" # use the development API key which can be found in the dev vault +EMAIL_BACKEND="web.mail.backends.GovNotifyEmailBackend" +SEND_ALL_EMAILS_TO=""# Need to have registered with GOV Notify and have been invited to the ICMS project diff --git a/config/settings/base.py b/config/settings/base.py index f910758c5..00d8602dc 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -130,12 +130,7 @@ # Email GOV_NOTIFY_API_KEY = env.str("GOV_NOTIFY_API_KEY", default="") -EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend" - -AWS_SES_ACCESS_KEY_ID = env.str("AWS_SES_ACCESS_KEY_ID", default="") -AWS_SES_SECRET_ACCESS_KEY = env.str("AWS_SES_SECRET_ACCESS_KEY", default="") -AWS_SES_REGION_NAME = "eu-west-1" -AWS_SES_REGION_ENDPOINT = "email.eu-west-1.amazonaws.com" +EMAIL_BACKEND = env.str("EMAIL_BACKEND", default="django.core.mail.backends.console.EmailBackend") # Email/phone contacts EMAIL_FROM = env.str("ICMS_EMAIL_FROM", default="") @@ -228,6 +223,7 @@ CELERY_ACCEPT_CONTENT = ["application/json"] CELERY_RESULT_SERIALIZER = "json" CELERY_TASK_SERIALIZER = "json" +CELERY_RESULT_EXTENDED = True # Django cache with Redis CACHES = { diff --git a/config/settings/non_prod_base.py b/config/settings/non_prod_base.py index b22976550..eea68bd79 100644 --- a/config/settings/non_prod_base.py +++ b/config/settings/non_prod_base.py @@ -6,11 +6,7 @@ from .base import * # Email settings for all non prod environments. -EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend" -AWS_SES_ACCESS_KEY_ID = "" -AWS_SES_SECRET_ACCESS_KEY = "" -AWS_SES_REGION_NAME = "" -AWS_SES_REGION_ENDPOINT = "" +SEND_ALL_EMAILS_TO = env.list("SEND_ALL_EMAILS_TO", default=[]) # Email/phone contacts EMAIL_FROM = env.str("ICMS_EMAIL_FROM", "enquiries.ilb@icms.trade.dev.uktrade.io") # /PS-IGNORE diff --git a/config/settings/test.py b/config/settings/test.py index f12b86cf1..cf6888efb 100644 --- a/config/settings/test.py +++ b/config/settings/test.py @@ -57,3 +57,4 @@ ICMS_V1_REPLICA_DSN = "" GOV_NOTIFY_API_KEY = "fakekey-11111111-1111-1111-1111-111111111111-22222222-2222-2222-222222222222" +EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend" diff --git a/docker-compose.yml b/docker-compose.yml index c7cb74f4d..1c438554c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,8 +41,6 @@ services: - ICMS_SILENCED_SYSTEM_CHECKS - ICMS_SECRET_KEY - ICMS_EMAIL_FROM - - AWS_SES_ACCESS_KEY_ID - - AWS_SES_SECRET_ACCESS_KEY - DJANGO_SETTINGS_MODULE - ELASTIC_APM_SECRET_TOKEN - ELASTIC_APM_ENVIRONMENT @@ -64,6 +62,8 @@ services: - SEND_LICENCE_TO_CHIEF - SET_INACTIVE_APP_TYPES_ACTIVE - GOV_NOTIFY_API_KEY + - EMAIL_BACKEND + - SEND_ALL_EMAILS_TO # stdin_open: true # tty: true ports: @@ -96,14 +96,14 @@ services: command: celery --app=config.celery:app worker --loglevel=info environment: - DJANGO_SETTINGS_MODULE - - AWS_SES_ACCESS_KEY_ID - - AWS_SES_SECRET_ACCESS_KEY - ICMS_HMRC_DOMAIN - ICMS_HMRC_UPDATE_LICENCE_ENDPOINT - HAWK_AUTH_ID - HAWK_AUTH_KEY - SEND_LICENCE_TO_CHIEF - GOV_NOTIFY_API_KEY + - EMAIL_BACKEND + - SEND_ALL_EMAILS_TO depends_on: - redis volumes: diff --git a/requirements-base.txt b/requirements-base.txt index 699385ac8..62855fbee 100644 --- a/requirements-base.txt +++ b/requirements-base.txt @@ -1,5 +1,5 @@ boto3==1.21.29 -celery[redis]==5.2.7 +celery[redis]==5.3.1 django-celery-results==2.4.0 django-chunk-upload-handlers==0.0.11 django-compressor==4.1 @@ -11,7 +11,6 @@ django-phonenumber-field==5.0.0 django-ratelimit==3.0.1 django-redis==5.2.0 django-select2==7.10.1 -django-ses==3.5.0 django-structlog==1.6.2 Django==4.1.10 elastic-apm==6.15.1 @@ -26,6 +25,7 @@ notifications-python-client==8.0.1 openpyxl==3.0.7 oracledb==1.2.0 phonenumbers==8.12.12 +pytz==2023.3 psycogreen==1.0.2 psycopg2-binary==2.9.3 pydantic==1.10.2 diff --git a/requirements-dev.txt b/requirements-dev.txt index dd63053ad..81fb00178 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -29,7 +29,7 @@ pip-check pre-commit==2.20.0 boto3-stubs[essential] types-openpyxl==3.1.0.0 -types-pytz==2021.1.0 +types-pytz==2023.3 types-python-dateutil==2.8.19.4 types-Pygments==2.14.0.6 types-requests==2.25.0 diff --git a/web/domains/case/access/views.py b/web/domains/case/access/views.py index 0e968bf52..f189d8a9f 100644 --- a/web/domains/case/access/views.py +++ b/web/domains/case/access/views.py @@ -17,6 +17,7 @@ ) from web.domains.case.services import case_progress, reference from web.flow.models import ProcessTypes +from web.mail import emails from web.models import ( AccessRequest, ExporterAccessRequest, @@ -115,7 +116,7 @@ def importer_access_request(request: AuthenticatedHttpRequest) -> HttpResponse: process=access_request, task_type=Task.TaskType.PROCESS, owner=request.user ) - notify.send_access_requested_email(access_request) + emails.send_access_requested_email(access_request) if request.user.has_perm(Perms.sys.importer_access) or request.user.has_perm( Perms.sys.exporter_access @@ -157,7 +158,7 @@ def exporter_access_request(request: AuthenticatedHttpRequest) -> HttpResponse: process=access_request, task_type=Task.TaskType.PROCESS, owner=request.user ) - notify.send_access_requested_email(access_request) + emails.send_access_requested_email(access_request) if request.user.has_perm(Perms.sys.importer_access) or request.user.has_perm( Perms.sys.exporter_access diff --git a/web/mail/api.py b/web/mail/api.py index a4669ee51..79286b03a 100644 --- a/web/mail/api.py +++ b/web/mail/api.py @@ -4,6 +4,8 @@ from notifications_python_client import NotificationsAPIClient from notifications_python_client.errors import HTTPError +from config.celery import app + def get_gov_notify_client() -> NotificationsAPIClient: return NotificationsAPIClient(settings.GOV_NOTIFY_API_KEY) @@ -25,3 +27,11 @@ def is_valid_template_id(template_id: UUID) -> bool: except ValueError: return False return gov_notify_template_id == template_id + + +@app.task +def send_email(template_id: UUID, personalisation: dict, email_address: str) -> dict: + client = get_gov_notify_client() + return client.send_email_notification( + email_address, str(template_id), personalisation=personalisation + ) diff --git a/web/mail/backends.py b/web/mail/backends.py new file mode 100644 index 000000000..36e4f9e67 --- /dev/null +++ b/web/mail/backends.py @@ -0,0 +1,20 @@ +from uuid import UUID + +import structlog as logging +from django.core.mail.backends.base import BaseEmailBackend + +from web.mail.api import send_email +from web.mail.messages import GOVNotifyEmailMessage + +logger = logging.getLogger(__name__) + + +class GovNotifyEmailBackend(BaseEmailBackend): + def send_messages(self, email_messages: list[GOVNotifyEmailMessage]) -> None: + for message in email_messages: + for recipient in message.recipients(): + logger.info("Sending %s email to %s", message.name.label, recipient) + self._send_message(message.template_id, message.personalisation, recipient) + + def _send_message(self, template_id: UUID, personalisation: dict, recipient: str) -> None: + send_email.delay(template_id, personalisation, recipient) diff --git a/web/mail/decorators.py b/web/mail/decorators.py new file mode 100644 index 000000000..cd53d7d3b --- /dev/null +++ b/web/mail/decorators.py @@ -0,0 +1,18 @@ +from functools import wraps + +from django.conf import settings + + +def override_recipients(f): + """Helper decorator to override the email addresses returned by the wrapped function. + If APP_ENV is dev or local and SEND_ALL_EMAILS_TO is set in django settings all emails will be sent to + the specified email addresses. + """ + + @wraps(f) + def wrapper(*args, **kwargs): + if settings.APP_ENV in ("local", "dev") and settings.SEND_ALL_EMAILS_TO: + return settings.SEND_ALL_EMAILS_TO + return f(*args, **kwargs) + + return wrapper diff --git a/web/mail/emails.py b/web/mail/emails.py new file mode 100644 index 000000000..45e8d165e --- /dev/null +++ b/web/mail/emails.py @@ -0,0 +1,11 @@ +from web.models import AccessRequest + +from .messages import AccessRequestEmail +from .recipients import get_ilb_case_officers_email_addresses + + +def send_access_requested_email(access_request: AccessRequest) -> None: + recipients = get_ilb_case_officers_email_addresses() + for recipient in recipients: + email = AccessRequestEmail(access_request=access_request, to=[recipient]) + email.send() diff --git a/web/mail/messages.py b/web/mail/messages.py new file mode 100644 index 000000000..7ce6106cb --- /dev/null +++ b/web/mail/messages.py @@ -0,0 +1,51 @@ +from typing import ClassVar +from uuid import UUID + +from django.conf import settings +from django.core.mail import EmailMessage, SafeMIMEMultipart + +from web.models import AccessRequest + +from .constants import EmailTypes +from .models import EmailTemplate + + +class GOVNotifyEmailMessage(EmailMessage): + name: ClassVar[EmailTypes] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.template_id = self.get_template_id() + self.personalisation = self.get_personalisation() + + def message(self) -> SafeMIMEMultipart: + """Adds the personalisation data to the message header, so it is visible when using the console backend.""" + message = super().message() + message["Personalisation"] = self.personalisation + return message + + def get_context(self) -> dict: + raise NotImplementedError + + def get_personalisation(self) -> dict: + return { + "icms_url": settings.DEFAULT_DOMAIN, + "icms_contact_email": settings.ILB_CONTACT_EMAIL, + "icms_contact_phone": settings.ILB_CONTACT_PHONE, + "subject": self.subject, + "body": self.body, + } | self.get_context() + + def get_template_id(self) -> UUID: + return EmailTemplate.objects.get(name=self.name).gov_notify_template_id + + +class AccessRequestEmail(GOVNotifyEmailMessage): + name = EmailTypes.ACCESS_REQUEST + + def __init__(self, *args, access_request: AccessRequest, **kwargs): + self.access_request = access_request + super().__init__(*args, **kwargs) + + def get_context(self) -> dict: + return {"reference": self.access_request.reference} diff --git a/web/mail/recipients.py b/web/mail/recipients.py new file mode 100644 index 000000000..0e877247e --- /dev/null +++ b/web/mail/recipients.py @@ -0,0 +1,19 @@ +from django.db.models import QuerySet + +from web.mail.decorators import override_recipients +from web.models import User +from web.notify.utils import get_notification_emails +from web.permissions import get_ilb_case_officers + + +def get_ilb_case_officers_email_addresses() -> list[str]: + users = get_ilb_case_officers() + return get_email_addresses_for_users(users) + + +@override_recipients +def get_email_addresses_for_users(users: QuerySet[User]) -> list[str]: + emails = [] + for user in users: + emails.extend(get_notification_emails(user)) + return list(set(emails)) diff --git a/web/notify/notify.py b/web/notify/notify.py index 94d74138c..6c388639f 100644 --- a/web/notify/notify.py +++ b/web/notify/notify.py @@ -26,7 +26,7 @@ User, VariationRequest, ) -from web.permissions import SysPerms, get_ilb_case_officers +from web.permissions import SysPerms from . import email, utils @@ -81,15 +81,6 @@ def register(user, password): ) -def send_access_requested_email(access_request): - context = {"subject": f"Access Request {access_request.reference}"} - email.send_html_email( - "email/access/access_requested.html", - context, - list(get_ilb_case_officers()), - ) - - def access_request_closed(access_request): requester = access_request.submitted_by subject = "Import Case Management System Account" diff --git a/web/tests/conftest.py b/web/tests/conftest.py index 327923c98..d7f6b06ec 100644 --- a/web/tests/conftest.py +++ b/web/tests/conftest.py @@ -1,4 +1,5 @@ import datetime +from unittest import mock import pytest from django.conf import settings @@ -8,6 +9,7 @@ from django.test.client import Client from django.urls import reverse from jinja2 import Template as Jinja2Template +from notifications_python_client import NotificationsAPIClient from pytest_django.asserts import assertRedirects from web.domains.case.services import case_progress, document_pack @@ -726,6 +728,22 @@ def strict_templates(): yield None +@pytest.fixture +def enable_gov_notify_backend(): + with override_settings(EMAIL_BACKEND="web.mail.backends.GovNotifyEmailBackend"): + yield None + + +@pytest.fixture +def mock_gov_notify_client(enable_gov_notify_backend): + with mock.patch("web.mail.api.get_gov_notify_client") as client: + mock_gov_notify_client = mock.create_autospec( + spec=NotificationsAPIClient(settings.GOV_NOTIFY_API_KEY), instance=True + ) + client.return_value = mock_gov_notify_client + yield mock_gov_notify_client + + def _set_valid_licence(app): licence = document_pack.pack_draft_get(app) licence.case_completion_datetime = datetime.datetime(2020, 1, 1, tzinfo=datetime.UTC) diff --git a/web/tests/domains/case/access/test_views.py b/web/tests/domains/case/access/test_views.py index 9e1262443..ac242af38 100644 --- a/web/tests/domains/case/access/test_views.py +++ b/web/tests/domains/case/access/test_views.py @@ -5,23 +5,12 @@ from django.urls import reverse from pytest_django.asserts import assertInHTML, assertRedirects, assertTemplateUsed +from web.mail.constants import EmailTypes from web.models import AccessRequest, ExporterAccessRequest, ImporterAccessRequest from web.permissions import organisation_get_contacts from web.tests.auth import AuthTestCase from web.tests.conftest import LOGIN_URL -from web.tests.helpers import check_email_was_sent, get_test_client - - -def check_access_request_email_was_sent( - exp_num_emails: int, - exp_sent_to_list: list[str], - exp_subject: str, - exp_in_body: str | None = None, -): - check_email_was_sent(exp_num_emails, exp_sent_to_list[0], exp_subject, exp_in_body) - outbox = mail.outbox - sent_to = [_email.to[0] for _email in outbox if _email.subject == exp_subject] - assert set(exp_sent_to_list) == set(sent_to) +from web.tests.helpers import check_gov_notify_email_was_sent, get_test_client def test_list_importer_access_request_ok( @@ -112,14 +101,14 @@ def test_post(self): self.access_request_user.refresh_from_db() assert self.access_request_user.submitted_access_requests.count() == 3 - check_access_request_email_was_sent( + check_gov_notify_email_was_sent( 2, [ "ilb_admin_user@example.com", # /PS-IGNORE "ilb_admin_two@example.com", # /PS-IGNORE ], - "Access Request IAR/1", - "An Access Request has been delivered", + EmailTypes.ACCESS_REQUEST, + {"reference": "IAR/1"}, ) @@ -180,14 +169,14 @@ def test_post(self): self.access_request_user.refresh_from_db() assert self.access_request_user.submitted_access_requests.count() == 3 - check_access_request_email_was_sent( + check_gov_notify_email_was_sent( 2, [ "ilb_admin_user@example.com", # /PS-IGNORE "ilb_admin_two@example.com", # /PS-IGNORE ], - "Access Request EAR/1", - "An Access Request has been delivered", + EmailTypes.ACCESS_REQUEST, + {"reference": "EAR/1"}, ) diff --git a/web/tests/helpers.py b/web/tests/helpers.py index 338649f36..ca8dac06a 100644 --- a/web/tests/helpers.py +++ b/web/tests/helpers.py @@ -1,3 +1,4 @@ +from django.conf import settings from django.core import mail from django.test.client import Client from django.urls import reverse @@ -5,6 +6,7 @@ from web.domains.case.types import ImpOrExp from web.flow.models import ProcessTypes +from web.mail.constants import EmailTypes from web.models import ( ApprovalRequest, Exporter, @@ -70,6 +72,37 @@ def check_email_was_sent( assert exp_attachments == sent_email.attachments +def check_gov_notify_email_was_sent( + exp_num_emails: int, + exp_sent_to: list, + exp_email_name: EmailTypes, + exp_personalisation: dict, + exp_subject: str | None = "", + exp_in_body: str | None = "", +) -> None: + outbox = mail.outbox + assert len(outbox) == exp_num_emails + sent_to = [_email.to[0] for _email in outbox] + assert sorted(sent_to) == sorted(exp_sent_to) + + if exp_num_emails: + sent_email = outbox[exp_num_emails - 1] + assert sent_email.subject == exp_subject + assert exp_in_body in sent_email.body + + assert sent_email.name == exp_email_name + assert_common_email_personalisation(sent_email.personalisation, exp_subject, exp_in_body) + assert sent_email.personalisation == exp_personalisation + + +def assert_common_email_personalisation(personalisation: dict, exp_subject: str, exp_in_body: str): + assert personalisation.pop("icms_contact_email") == settings.ILB_CONTACT_EMAIL + assert personalisation.pop("icms_contact_phone") == settings.ILB_CONTACT_PHONE + assert personalisation.pop("icms_url") == settings.DEFAULT_DOMAIN + assert personalisation.pop("subject") == exp_subject + assert exp_in_body in personalisation.pop("body") + + def add_variation_request_to_app( application: ImpOrExp, user: User, diff --git a/web/tests/mail/test_api.py b/web/tests/mail/test_api.py index c214d8a64..b26235d5f 100644 --- a/web/tests/mail/test_api.py +++ b/web/tests/mail/test_api.py @@ -51,3 +51,14 @@ def test_get_template_by_id_error(mock_gov_notify_get_template): api.get_template_by_id(UUID("fb9a1023-3901-44e8-a7d3-a0e309e93951")) == HTTP_404_NOT_FOUND_ERROR ) + + +@mock.patch("notifications_python_client.NotificationsAPIClient.send_email_notification") +def test_send_email_error(mock_gov_notify_send_email_notification): + fake_response = mock.Mock(status_code=404, json=lambda: HTTP_404_NOT_FOUND_ERROR) + fake_error = mock.Mock(response=fake_response) + mock_gov_notify_send_email_notification.side_effect = HTTPError.create(fake_error) + with pytest.raises(HTTPError): + api.send_email( + UUID("fb9a1023-3901-44e8-a7d3-a0e309e93951"), {}, "tester@example.com" # /PS-IGNORE + ) diff --git a/web/tests/mail/test_emails.py b/web/tests/mail/test_emails.py new file mode 100644 index 000000000..e0b3f49cf --- /dev/null +++ b/web/tests/mail/test_emails.py @@ -0,0 +1,43 @@ +import pytest +from django.conf import settings + +from web.mail.constants import EmailTypes +from web.mail.emails import send_access_requested_email +from web.models import EmailTemplate +from web.tests.auth.auth import AuthTestCase + + +def default_personalisation() -> dict: + return { + "icms_url": settings.DEFAULT_DOMAIN, + "icms_contact_email": settings.ILB_CONTACT_EMAIL, + "icms_contact_phone": settings.ILB_CONTACT_PHONE, + "subject": "", + "body": "", + } + + +class TestEmails(AuthTestCase): + @pytest.fixture(autouse=True) + def setup(self, _setup, mock_gov_notify_client): + self.mock_gov_notify_client = mock_gov_notify_client + + def test_access_request_email(self, importer_access_request): + exp_template_id = str( + EmailTemplate.objects.get(name=EmailTypes.ACCESS_REQUEST).gov_notify_template_id + ) + expected_personalisation = default_personalisation() | { + "reference": importer_access_request.reference + } + send_access_requested_email(importer_access_request) + assert self.mock_gov_notify_client.send_email_notification.call_count == 2 + self.mock_gov_notify_client.send_email_notification.assert_any_call( + self.ilb_admin_two_user.email, + exp_template_id, + personalisation=expected_personalisation, + ) + self.mock_gov_notify_client.send_email_notification.assert_any_call( + self.ilb_admin_user.email, + exp_template_id, + personalisation=expected_personalisation, + ) diff --git a/web/tests/mail/test_recipients.py b/web/tests/mail/test_recipients.py new file mode 100644 index 000000000..b9eb555ae --- /dev/null +++ b/web/tests/mail/test_recipients.py @@ -0,0 +1,22 @@ +import pytest +from django.test import override_settings + +from web.mail.recipients import get_ilb_case_officers_email_addresses + + +@pytest.mark.django_db +def test_get_ilb_case_officers_email_addresses(): + assert sorted(get_ilb_case_officers_email_addresses()) == sorted( + [ + "ilb_admin_user@example.com", # /PS-IGNORE + "ilb_admin_two@example.com", # /PS-IGNORE + ] + ) + + +@pytest.mark.django_db +def test_get_ilb_case_officers_email_addresses_override_recipients(): + with override_settings( + APP_ENV="dev", SEND_ALL_EMAILS_TO=["test_user@example.com"] # /PS-IGNORE + ): + assert get_ilb_case_officers_email_addresses() == ["test_user@example.com"] # /PS-IGNORE