Refactor TemplateEmailFile to use same sorting strategy as other models

app/main/views/template_email_files.py

@@ -82,7 +82,7 @@ def manage_a_template_email_file(service_id, template_id, template_email_file_id
     )
     delete = bool(request.args.get("delete"))
     template_email_file = TemplateEmailFile.get_by_id(
-        template_email_file_id=template_email_file_id, service_id=service_id, template_id=template_id
+        template_email_file_id=template_email_file_id, service_id=service_id, template=template
     )
     if request.method == "POST" and delete:
         new_content = PlainTextField(
@@ -121,7 +121,7 @@ def make_file_live(service_id, template_id, template_email_file_id):
         must_be_of_type="email",
     )
     template_email_file = TemplateEmailFile.get_by_id(
-        template_email_file_id=template_email_file_id, service_id=service_id, template_id=template_id
+        template_email_file_id=template_email_file_id, service_id=service_id, template=template
     )
     if template_email_file.filename not in InsensitiveSet(template.placeholders):
         new_content = template.content + f"\n\n(({template_email_file.filename}))"
@@ -185,7 +185,7 @@ def change_link_text(service_id, template_id, template_email_file_id):
         must_be_of_type="email",
     )
     template_email_file = TemplateEmailFile.get_by_id(
-        template_email_file_id=template_email_file_id, service_id=service_id, template_id=template_id
+        template_email_file_id=template_email_file_id, service_id=service_id, template=template
     )
     form = TemplateEmailFileLinkTextForm(link_text=template_email_file.link_text)
 
@@ -221,7 +221,7 @@ def change_data_retention_period(service_id, template_id, template_email_file_id
         must_be_of_type="email",
     )
     template_email_file = TemplateEmailFile.get_by_id(
-        template_email_file_id=template_email_file_id, service_id=service_id, template_id=template_id
+        template_email_file_id=template_email_file_id, service_id=service_id, template=template
     )
     form = TemplateEmailFileRetentionPeriodForm(retention_period=template_email_file.retention_period)
 
@@ -258,7 +258,7 @@ def change_email_validation(service_id, template_id, template_email_file_id):
         must_be_of_type="email",
     )
     template_email_file = TemplateEmailFile.get_by_id(
-        template_email_file_id=template_email_file_id, service_id=service_id, template_id=template_id
+        template_email_file_id=template_email_file_id, service_id=service_id, template=template
     )
     form = OnOffSettingForm(
         "Ask recipient for their email address",

app/models/template_email_file.py

@@ -1,5 +1,7 @@
+import math
 import mimetypes
 import uuid
+from contextlib import suppress
 from datetime import UTC, datetime, timedelta
 from typing import Any
 
@@ -8,6 +10,7 @@
 from notifications_utils.base64_uuid import uuid_to_base64
 from notifications_utils.s3 import s3download
 from notifications_utils.serialised_model import SerialisedModelCollection
+from notifications_utils.template import Template
 
 from app.models import JSONModel
 from app.s3_client.s3_template_email_file_upload_client import upload_template_email_file_to_s3
@@ -20,14 +23,14 @@ def _get_file_location(file_id: uuid, service_id: uuid) -> str:
 class TemplateEmailFile(JSONModel):
     id: Any
     service_id: Any
-    template_id: Any
+    template: Any
     filename: str
     link_text: str
     retention_period: int
     validate_users_email: bool
     pending: bool
 
-    __sort_attribute__ = "filename"
+    __sort_attribute__ = "position_in_template"
 
     @staticmethod
     def create(*, filename, file_contents, template_id, pending=True):
@@ -57,15 +60,16 @@ def update(self, **kwargs):
         } | kwargs
 
         return template_email_file_client.update_file(
-            service_id=self.service_id, template_id=self.template_id, file_id=self.id, **data
+            service_id=self.service_id, template_id=self.template.id, file_id=self.id, **data
         )
 
     @classmethod
-    def get_by_id(cls, template_email_file_id: str, service_id: str, template_id: str):
+    def get_by_id(cls, template_email_file_id: str, service_id: str, template: Template):
         from app import template_email_file_client
 
-        template_email_file = template_email_file_client.get_file_by_id(template_email_file_id, service_id, template_id)
+        template_email_file = template_email_file_client.get_file_by_id(template_email_file_id, service_id, template.id)
         template_email_file["data"]["service_id"] = service_id
+        template_email_file["data"]["template"] = template
         return cls(template_email_file.get("data"))
 
     @property
@@ -74,7 +78,7 @@ def link_as_markdown(self):
             "main.document_download_landing",
             service_id=self.service_id,
             document_id=self.id,
-            key=uuid_to_base64(self.template_id),
+            key=uuid_to_base64(self.template.id),
             _external=True,
         )
         if self.link_text:
@@ -108,6 +112,12 @@ def file_contents(self):
             f"{self.service_id}/{self.id}",
         )
 
+    @property
+    def position_in_template(self):
+        with suppress(KeyError):
+            return self.template.all_placeholders.index(self.filename)
+        return math.inf
+
 
 class TemplateEmailFiles(SerialisedModelCollection):
     model = TemplateEmailFile
@@ -116,16 +126,12 @@ def __init__(self, template):
         from app import current_service
 
         self.service_id = current_service.id
-        self.template_id = template.id
-
-        email_files = template._template.get("email_files", [])
-        super().__init__(email_files)
+        self.template = template
 
-        position_in_template = (template.index_of_placeholder(email_file.filename) for email_file in self)
-        self.items = sorted(email_files, key=lambda _: next(position_in_template))
+        super().__init__(template._template.get("email_files", []))
 
     def __getitem__(self, index):
-        return self.model(self.items[index] | {"service_id": self.service_id, "template_id": self.template_id})
+        return self.model(self.items[index] | {"service_id": self.service_id, "template": self.template})
 
     @property
     def as_personalisation(self):

app/templates/views/templates/email-template-files/files-list.html

@@ -20,7 +20,7 @@
 
    {% set files = [] %}
     {%  if template.email_files %}
-    {% for item in template.email_files %}
+    {% for item in template.email_files | sort %}
       {% do files.append(
         {
           "key": {

app/utils/templates.py

@@ -1,6 +1,4 @@
 import json
-import math
-from contextlib import suppress
 from typing import Any
 
 from flask import current_app, render_template, url_for
@@ -315,11 +313,6 @@ def all_placeholders(self):
     def placeholders(self):
         return OrderedSet([placeholder for placeholder in self.all_placeholders if placeholder not in self.filenames])
 
-    def index_of_placeholder(self, placeholder):
-        with suppress(KeyError):
-            return InsensitiveSet(self.all_placeholders).index(placeholder)
-        return math.inf
-
 
 class LetterAttachment(JSONModel):
     id: Any