Skip to content

Refactor TemplateEmailFile to use same sorting strategy as other models#5841

Open
quis wants to merge 5 commits intomainfrom
refactor-email-file-sorting
Open

Refactor TemplateEmailFile to use same sorting strategy as other models#5841
quis wants to merge 5 commits intomainfrom
refactor-email-file-sorting

Conversation

@quis
Copy link
Member

@quis quis commented Mar 16, 2026

Other models sort based on their __sort_attribute__. This commit moves the calculation of each files position in the template into the TemplateEmailFile object, so that its sorting can also work in this way.


The actual sort order is tested here:

(
# Content order matches database order
"((test_file_1.csv)) ((test_file_2.png))",
["test_file_1.csv", "test_file_2.png"],
),
(
# Content order does not match database order
"((test_file_2.png)) ((test_file_1.csv))",
["test_file_2.png", "test_file_1.csv"],
),
(
# Content order does not match database order (case differs)
"((TEST FILE 2.PNG)) ((TEST FILE 1.CSV))",
["test_file_2.png", "test_file_1.csv"],
),
(
# Content order does not match database order (extra, non-file placeholders)
"((test_file_2.png)) ((first name)) ((last name)) ((test_file_1.csv))",
["test_file_2.png", "test_file_1.csv"],
),

@quis quis force-pushed the refactor-email-file-sorting branch from cd8fd91 to 09887bc Compare March 16, 2026 10:47
quis added 5 commits March 17, 2026 12:34
This is a form of dependency injection. It will be needed so the files
can know their own position in the template.
Other models sort based on their `__sort_attribute__`. This commit moves
the calculation of each files position in the template into the
`TemplateEmailFile` object, so that its sorting can also work in this
way.
Don’t need the wrapper method on the `EmailPreviewTemplate` object now
that placeholders are an `InsensitiveSet` so we can do
`placeholders.index` directly
@quis quis force-pushed the refactor-email-file-sorting branch from 09887bc to e89f5bd Compare March 17, 2026 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant