From 33aba51cb936b51e41cd88392104ae42bdbc7a8c Mon Sep 17 00:00:00 2001 From: Stephen McMurtry Date: Tue, 28 Jan 2025 15:55:25 -0500 Subject: [PATCH] Improve the formatting of the support emails for PTs (#2429) This PR improves the formatting of the support emails for the "PT service skip Freshdesk" feature while still using the existing _generate_ticket method. This PR removes the product_id, 'priority, status, and tagsdata from the email.priorityandstatuswere being hardcoded in this case so those numbers weren't meaningful.product_idwill always be the same for the prod app. Andtags` are only meaningful to freshdesk. So I think it is safe to remove this data for this feature since it makes the email less confusing for the support team. --- app/clients/freshdesk.py | 27 ++++++++++++++++++++------- tests/app/clients/test_freshdesk.py | 4 ++-- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/app/clients/freshdesk.py b/app/clients/freshdesk.py index 4b277063ab..04f9ac8e54 100644 --- a/app/clients/freshdesk.py +++ b/app/clients/freshdesk.py @@ -142,8 +142,12 @@ def send_ticket(self) -> int: def email_freshdesk_ticket_freshdesk_down(self): if current_app.config["CONTACT_FORM_EMAIL_ADDRESS"] is None: current_app.logger.info("Cannot email contact us form, CONTACT_FORM_EMAIL_ADDRESS is empty") + content = self._generate_ticket() + content_str = json.dumps(content, indent=4) self.email_freshdesk_ticket( - current_app.config["CONTACT_FORM_EMAIL_ADDRESS"], current_app.config["CONTACT_FORM_DIRECT_EMAIL_TEMPLATE_ID"] + current_app.config["CONTACT_FORM_EMAIL_ADDRESS"], + current_app.config["CONTACT_FORM_DIRECT_EMAIL_TEMPLATE_ID"], + content_str, ) def email_freshdesk_ticket_pt_service(self): @@ -151,9 +155,21 @@ def email_freshdesk_ticket_pt_service(self): template_id = current_app.config.get("CONTACT_FORM_SENSITIVE_SERVICE_EMAIL_TEMPLATE_ID") if not email_address: current_app.logger.error("SENSITIVE_SERVICE_EMAIL not set") - self.email_freshdesk_ticket(email_address, template_id) + content = self._generate_ticket() + # This email will display the form data as a quote block + content_str_nice = "\n".join( + [ + f"^**Subject**: {content['subject']}", + "^", + "^**User-entered info**:", + f"^{content['description'].replace('
', '\n').replace('\n', '\n^')}", + "^", + f"^**User's Email**: {content['email']}", + ] + ) + self.email_freshdesk_ticket(email_address, template_id, content_str_nice) - def email_freshdesk_ticket(self, email_address, template_id) -> None: + def email_freshdesk_ticket(self, email_address: str, template_id: str, content_str: str) -> None: content = self._generate_ticket() try: template = dao_get_template_by_id(template_id) @@ -164,11 +180,8 @@ def email_freshdesk_ticket(self, email_address, template_id) -> None: template_version=template.version, recipient=email_address, service=notify_service, - # This email will be badly formatted, but this allows us to re-use the - # _generate_description fn without having to duplicate all of that logic to get the - # description in plain text. personalisation={ - "contact_us_content": json.dumps(content, indent=4), + "contact_us_content": content_str, }, notification_type=template.template_type, api_key_id=None, diff --git a/tests/app/clients/test_freshdesk.py b/tests/app/clients/test_freshdesk.py index be9e5d5236..7c995088a8 100644 --- a/tests/app/clients/test_freshdesk.py +++ b/tests/app/clients/test_freshdesk.py @@ -347,7 +347,7 @@ def test_email_freshdesk_ticket_pt_service_success(self, mocker, notify_api): freshdesk_client = freshdesk.Freshdesk(ContactRequest(email_address="user@example.com")) freshdesk_client.email_freshdesk_ticket_pt_service() - mock_email_ticket.assert_called_once_with("sensitive@test.gov.uk", "template-123") + mock_email_ticket.assert_called_once_with("sensitive@test.gov.uk", "template-123", mocker.ANY) def test_email_freshdesk_ticket_pt_service_no_email(self, mocker, notify_api): """Test handling when sensitive service email not configured""" @@ -362,4 +362,4 @@ def test_email_freshdesk_ticket_pt_service_no_email(self, mocker, notify_api): freshdesk_client.email_freshdesk_ticket_pt_service() mock_logger.assert_called_once_with("SENSITIVE_SERVICE_EMAIL not set") - mock_email_ticket.assert_called_once_with(None, "template-123") + mock_email_ticket.assert_called_once_with(None, "template-123", mocker.ANY)