TRIVIR-2320 | Extract HTML and CSS from Themes and Email Templates#115
TRIVIR-2320 | Extract HTML and CSS from Themes and Email Templates#115csong-hash wants to merge 1 commit into
Conversation
csong-hash
commented
Jun 9, 2026
csong-hash
commented
- feat: Add 'no-extract' command to email templates to prevent HTML and css from being separate files, which happens by default now
… css from being separate files, which happens by default now
There was a problem hiding this comment.
There are several mocks that need to be deleted that you either created or modified. Only include the mocks for the new tests. The existing test mocks should already work, since all we are doing is simply changing how we store the export files.
We also should verify that imports still work. The way to do this will be to update the test/e2e/exports/all-separate/forgeops/global/emailTemplate directory to be the same files but exported to separate files. Then we should expect that the import tests will pass without any changes if your changes are correct.
On the topic of imports, you also need to make sure that on import it can successfully read in the extracted files. Additionally, you need to make sure that config export and config import can extract and import the extracted files for email templates. Additionally, you need to make sure that idm export and idm import can also handle the extracted files (since email templates are technically an IDM thing). Each of these already have the -x, --no-extract flags now, so it should be relatively simple to apply what we are doing for those commands to email templates as well.
To help with this, I would follow what we do for custom nodes and for normal IDM exports/imports when we do script extraction. We have helpers that you can use for this, specifically extractDataToFile and getExtractedData for exports and imports respectively.
Edit (post-review): The other thing is that I noticed you are not extracting any of the files for themes either, we want to do this as well. This will probably have to be a separate PR, so I created a ClickUp ticket for it: https://app.clickup.com/t/45049292/TRIVIR-2644
There was a problem hiding this comment.
Delete this, shouldn't be in this PR
There was a problem hiding this comment.
Delete this, shouldn't be in this PR
There was a problem hiding this comment.
Delete this, shouldn't be in this PR
There was a problem hiding this comment.
Delete this, shouldn't be in this PR
There was a problem hiding this comment.
Delete this, shouldn't be in this PR
| options.extract, | ||
| options.templateId, | ||
| options.file, | ||
| options.metadata |
There was a problem hiding this comment.
Make sure to update the ordering here based on my other comments
| options.extract, | ||
| options.file, | ||
| options.metadata |
There was a problem hiding this comment.
Make sure to update the ordering here based on my other comments
| .addOption( | ||
| new Option( | ||
| '-x, --no-extract', | ||
| 'Do not extract HTML and CSS to a separate file' |
There was a problem hiding this comment.
Suggestion: I would reword this to Do not extract HTML and CSS to separate files
| if (formattedTemplate.html) delete formattedTemplate.html; | ||
| if (formattedTemplate.styles) delete formattedTemplate.styles; |
There was a problem hiding this comment.
This is wrong, we want to store the file reference to the extracted file. Note that I already have helpers that handle this, I would refer to how we do extracted files in NodeOps for custom nodes, as that will be a good guide to how we should be doing it here.
| if (extract && (templateData.html || templateData.styles)) { | ||
| const fileThemeName = file ? file.replace('.json', '') + '.theme.json' : getTypedFilename( | ||
| templateId, | ||
| EMAIL_TEMPLATE_FILE_TYPE + '.theme' | ||
| ); | ||
| const fileHTMLData = getFileDataHTMLTemplate(); | ||
| fileHTMLData.html[templateId] = templateData.html ?? {}; | ||
| fileHTMLData.css[templateId] = templateData.styles ?? {}; | ||
|
|
||
| saveJsonToFile(fileHTMLData, getFilePath(fileThemeName, true), includeMeta); | ||
|
|
||
| const formattedTemplate = templateData; | ||
|
|
||
| if (formattedTemplate.html) delete formattedTemplate.html; | ||
| if (formattedTemplate.styles) delete formattedTemplate.styles; | ||
|
|
||
| fileData.emailTemplate[templateId] = formattedTemplate; | ||
| } |
There was a problem hiding this comment.
I would throw this into a helper function, similiar to how we do it for NodeOps. Basically create a function called extractHtmlAndCssToFiles that will take in the export data and extract all the files. You can use extractCustomNodeScriptsToFiles as a guide for this. You can do the same for imports, create a function called getEmailTemplateExportFromFile that reads in a file along with it's extracted files. The reason you will want helpers is because for one, you use them multiple times just in this file, but you will probably want to use them as well for config export/import, and for IDM export/import as well.
The other thing is the way you are handling extraction isn't entirely correct. Note that there are two types of email templates: advanced editor templates and non-advanced editor templates. Also note that there can be multiple locales for an email template, each with their own CSS and HTML.
In other words, here's how I would expect exports to work. I'll give two examples which have locales en and fr for english and french respectively, one example for non-advanced editor templates, and one example for advanced editor templates (note the advancedEditor attribute on the email template that tells what type it is with a boolean of true and false):
- For non-advanced, we'll say it's id is
testI would expect to see 4 files
test.template.email.jsontest.template.email.cssfrom "styles" in the JSONtest.template.email.en.htmlfrom "html" in the JSONtest.template.email.fr.htmlfrom "html" in the JSON
- For advanced, we'll say it's id is
testadvancedI would expect to see 3 files:
testadvanced.template.email.jsontestadvanced.template.email.en.htmlfrom "message" in the JSONtestadvanced.template.email.fr.htmlfrom "message" in the JSON
The idea is that we only export the data we would be able to modify in the UI. In other words, for non-advanced the "message" part is not used in the JSON (although it is updated automatically in the UI, it's basically the preview that they show you on the page). However it is used for the advanced templates. Similarly, while "html" and "styles" are used in non-advanced, they are not used in advanced (in fact, they empty the "html" ones when it switches over). In other words, we want to only extract the data that can be modified in the UI since this is what we are ultimately tracking. If we extract the others, it will just be confusing.
What I would do is for advanced templates as well, I would just delete the "styles" from the JSON entirely. The email templates work without them, and we don't need to track them. For "html", you will get an error in the UI if you delete it even though it's not used, so I would set "html" to be {}, since even if you add a new locale this attribute will never be updated.
You need to make sure that everywhere that exports email templates does this. What I would do is create a helper that "processes" email template exports before it goes through extraction. The process function will clean up the export as I just describe to you, and then the extract function extractHtmlAndCssToFiles can focus just on extracting the files.
2 participants
