From a145c501ab0aa0ee70bd7dbe541ae66ce4650341 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 26 Oct 2021 18:06:26 +0100
Subject: [PATCH 1/2] Use Jinja template for branding request tickets

Following the pattern established by
https://github.com/alphagov/notifications-admin/pull/4041/files
this commit move the ticket message from a Python f-string to a proper
templating language.
---
 app/main/views/service_settings.py            | 26 +++++--------------
 .../support-tickets/branding-request.txt      | 10 +++++++
 2 files changed, 17 insertions(+), 19 deletions(-)
 create mode 100644 app/templates/support-tickets/branding-request.txt

diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 497c6aa78..4a20f6888 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -1173,27 +1173,15 @@ def branding_request(service_id, branding_type):
     elif branding_type == "letter":
         branding_name = current_service.letter_branding_name
     if form.validate_on_submit():
+        ticket_message = render_template(
+            'support-tickets/branding-request.txt',
+            current_branding=branding_name,
+            branding_requested=dict(form.options.choices)[form.options.data],
+            detail=form.something_else.data,
+        )
         ticket = NotifySupportTicket(
             subject=f'{branding_type.capitalize()} branding request - {current_service.name}',
-            message=(
-                'Organisation: {organisation}\n'
-                'Service: {service_name}\n'
-                '{dashboard_url}\n'
-                '\n---'
-                '\nCurrent branding: {current_branding}'
-                '\nBranding requested: {branding_requested}'
-                '{new_paragraph}'
-                '{detail}'
-                '\n'
-            ).format(
-                organisation=current_service.organisation.as_info_for_branding_request(current_user.email_domain),
-                service_name=current_service.name,
-                dashboard_url=url_for('main.service_dashboard', service_id=current_service.id, _external=True),
-                current_branding=branding_name,
-                branding_requested=dict(form.options.choices)[form.options.data],
-                new_paragraph='\n\n' if form.something_else.data else '',
-                detail=form.something_else.data or ''
-            ),
+            message=ticket_message,
             ticket_type=NotifySupportTicket.TYPE_QUESTION,
             user_name=current_user.name,
             user_email=current_user.email_address,
diff --git a/app/templates/support-tickets/branding-request.txt b/app/templates/support-tickets/branding-request.txt
new file mode 100644
index 000000000..eeffdc67d
--- /dev/null
+++ b/app/templates/support-tickets/branding-request.txt
@@ -0,0 +1,10 @@
+Organisation: {{ current_service.organisation.as_info_for_branding_request(current_user.email_domain) }}
+Service: {{ current_service.name }}
+{{ url_for('main.service_dashboard', service_id=current_service.id, _external=True) }}
+
+---
+Current branding: {{ current_branding }}
+Branding requested: {{ branding_requested }}
+{% if detail %}
+{{ detail }}
+{% endif %}

From e675500ac206f39ba728cc1e97c54e006f06cc98 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 26 Oct 2021 18:11:27 +0100
Subject: [PATCH 2/2] Move branding info from model
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This was an awkward property to have in a model class because
- it’s presentation not data about the object
- it munged together data from a different object, so it wasn’t well
  encapsulated

This commit moves the logic to the template, where it can reference the
`Organisation` and `User` models directly.
---
 app/models/organisation.py                         | 3 ---
 app/templates/support-tickets/branding-request.txt | 6 +++++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/app/models/organisation.py b/app/models/organisation.py
index 5b5e5272a..5ddffaf88 100644
--- a/app/models/organisation.py
+++ b/app/models/organisation.py
@@ -101,9 +101,6 @@ class Organisation(JSONModel):
             self.request_to_go_live_notes = None
             self.email_branding_id = None
 
-    def as_info_for_branding_request(self, fallback_domain):
-        return self.name or 'Can’t tell (domain is {})'.format(fallback_domain)
-
     @property
     def organisation_type_label(self):
         return dict(self.TYPES).get(self.organisation_type)
diff --git a/app/templates/support-tickets/branding-request.txt b/app/templates/support-tickets/branding-request.txt
index eeffdc67d..da55691d0 100644
--- a/app/templates/support-tickets/branding-request.txt
+++ b/app/templates/support-tickets/branding-request.txt
@@ -1,4 +1,8 @@
-Organisation: {{ current_service.organisation.as_info_for_branding_request(current_user.email_domain) }}
+Organisation: {% if current_service.organisation -%}
+  {{ current_service.organisation.name }}
+{%- else -%}
+  Can’t tell (domain is {{ current_user.email_domain }})
+{%- endif %}
 Service: {{ current_service.name }}
 {{ url_for('main.service_dashboard', service_id=current_service.id, _external=True) }}