Proposed structure for new email templating system

[neekfenwick]

This discussion arose on #2695

It occurred to me that instead of strings for the emails being in language defines, like a made up example:

• includes/languages/english/lang_email.php - defines EMAIL_GREETING 'Hello'
• includes/languages/spanish/lang_email.php - defines EMAIL_GREETING 'Hola'
• email/email_template_welcome.html

The difference in language would instead be in each email template, and you'd need a template per supported language, e.g.

• includes/languages/english/html_defines/email_template_welcome.html - contains "Hello ${customer_name}"
• includes/languages/spanish/html_defines/email_template_welcome.html - contains "Hola ${customer_name}"

Is this an acceptable direction? Any other suggestions for how to structure it?

On the subject of data, I'm a fan of putting conditional logic, loops etc in my Smarty templates, so having a order details and a list of products would be by something like:

$tmpl_data = [
    'orders_id' => $orders_id,
    'some_config_switch' => SOME_CONFIG_SWITCH,
    'products' => $products_array
];

and in the template, have Smarty functions like if and foreach, instead of writing PHP code that acts on configuration and logic before settings strings that go into the template:

<p>Greetings {$customer_name},</p>

{if some_config_switch}
<p>We also provide service xyz.</p>
{/if}

<p>Your invoice number is {$orders_id}, for the following products:</p>
<table><thead>blah blah</thead>
<tbody>
{foreach from=products item=product}
  <tr><td>{$product.products_name}</td><td>{$product.final_price}</td></tr>
{/foreach}
</tbody></table>

This is standard templating technique and very different to how Zen Cart currently builds its email content, we'd be moving a lot of PHP logic into the Smarty template. Any feedback on this general approach?

[torvista]

It is encouraging to see some new enthusiasm and effort, albeit misguided in my opinion.

Where is the demand for the email template system to be rewritten?

Will the description of an up-to-date method tempt potential users to try Zen Cart?
Does the current method make potential new users not use Zen Cart after trying it?
Do current users leave Zen Cart because of the current method as it does not meet their needs?

Who looks at their email system once their shop is set up and working?

Does Zen Cart have more serious problems? I guess you have read this #4805
My opinion has not changed and neither has the random approach here.
Fettling with code won't change the big picture/decline.

[neekfenwick]

Where is the demand for the email template system to be rewritten?

The requirements were laid out by a user request in #2695. To my mind they were:

• ability to change core email content by an easy editing method such as a define page, rather than code modification, by which we can assume things like 'EMAIL_GREETING' being a constant defined by a language file is not easy to edit for your non-techie user who wants to be able to use a web interface to edit everything.
• actually, that's about it.

I did read #4805 and I think referencing a huge post like that when trying to discuss improvements to the system is a little underhanded :)

Who looks at their email system once their shop is set up and working?

People like the OP who wants to edit content in emails when offers change, seasonal messages change, customer engagements chances change.

Will the description of an up-to-date method tempt potential users to try Zen Cart?

I would say yes. But currently we don't make it clear how hard it is to change this content. In this very focused example of ZC content, I'd say people try zen cart, find out it's really annoying to manage their email content (by editing PHP files with ugly => symbols and, ugh, geeks, amirite?) and move on. (I did raise the issue of the Smarty syntax also being relatively unfriendly, too, I'm not blind to these things, just answering your question)

Does the current method make potential new users not use Zen Cart after trying it?

See last point, we don't make it clear how annoying it is to manage until people really get into it, I can see how some would want to migrate away because of its weirdness.

Do current users leave Zen Cart because of the current method as it does not meet their needs?

No idea, this is a bit of a fluff question as none of us have specific metrics and you know it :)

My opinion has not changed and neither has the random approach here.

It's not a random approach, I'm a bit disappointed that you think this is somehow misguided. Do you think I'm just trying to distract people as some kind of guerrilla operation, or that I simply don't know what I'm talking about? You really don't think that a more modern templating system for email generation would be better than what we have? I've lived with it for 12+ years and will keep on living with it, but please don't be so dismissive.

In conclusion you don't seem to think a change like this would help but you're overshadowing it with #4805 where everything is awful. I'd like to focus on specific problems and talk about solving them and would appreciate your constructive input.

[scottcwilson]

I have had a number of clients request the ability to customize email. This is a worthwhile effort.

[torvista]

I think anyone who tries to make a contribution however small, should be applauded and encouraged. Not everyone agrees with that.

But contributors are not even in double figures and getting less, if that's possible.
I think your enthusiasm, ability and potential input is hence particularly important.

But, with no roadmap to focus development efforts nor any attempt being made to arrest the overall decline or market share, is your time well-spent?
Probably there is just no-one in this little developer group who feels they can take on that big non-coding role. Understandably.

It's not all awful at all. But is making such a big effort to improve an already-great product the best use of your input, if fewer and fewer people are thinking of using it?

I'd like to focus on specific problems and talk about solving them and would appreciate your constructive input.

I've been constructive for 12 years and as I am not a developer I have no profit to make from the product. In my current economic situation I can't justify the time it takes me to compose these posts to reinforce what I've already said, nor to justify my efforts to contribute.
I'd just like this project to succeed.

[neekfenwick]

Fair enough, and I respect your contributions over the years, more than mine to be sure. One thing I would say is that a set of Issues on github is a good start towards a roadmap, and from there it needs a concerted effort from a project manager to understand the development effort available to him/her and assign it appropriately to each issue, which I don't see happening, which I think is due to what seem to be well understood limitations in the available developer resources. #4805 is great and good luck to it but it's also large and complicated and I don't see any of the points raised as one I can contribute to, I don't use 70% of the features mentioned by lat9 in May, others I implemented myself. What I have done is bump into a user requirement that sounds like a good idea and tried to move it forward. This is such a large change it would probably end up as an item on #4805 at some point.

I think there are things about my proposed solution here that suck, from various angles, which any reasonable dev would be able to recognise, so I'm still looking for constructive feedback on the points raised. I can see that if I went ahead with implementation as proposed it would likely meet stiff resistance and require rewriting, and I've done enough dev work to see that it's not sensible to proceed without more discussion. (then again I've done some quite silly things in my past that led to multi year contracts and life changing decisions so I'm open to all options :) )

[neekfenwick]

A thought on the proposed solution, that it seems silly to repeat the general structure of an email template for each language, e.g. every single english, spanish etc. template has to contain the entire HTML structure. This raises a high likelihood of error, by corrupting the HTML in one template and not noticing it, or making a change in one and not propagating it to the others. So, this is very inefficient, it makes more sense to have a single template defining the structure.

What I'm coming around to is actually something a lot more like the current system with its language strings like EMAIL_GREETING, and having made that decision, we have at least two optional directions to go in. I still like both, but the use of a templating system isn't absolutely necessary to achieve the requirements from #2695.

The sticking points with the requirements was that the contents aren't easily editable, both that the email template file like email_template_order_status.html is a static file on disk and not editable, and the language files that define the strings aren't easily editable. Also factoring into this is the way the PHP code is hardcoded to combine some of these strings, such as:

$email_order = EMAIL_TEXT_HEADER . EMAIL_TEXT_FROM . STORE_NAME . "\n\n" .

I like the way the old system has a single template with a placeholder like EMAIL_GREETING, and it's up to the language files to define that content. Imagine with a Smarty (or similar, I'm going to stick with Smarty until someone proposes an alternative that makes sense) template that's designed to be language agnostic, unlike the one I proposed at the top. This solution has split into two directions:

1. Based on original requirements requested in [Suggestion] Edit Email default text from Admin #2695, make the overall email content easily editable by the store owner. I see this being solved by allowing editing of the email templates, similar to the Define Pages editor, possibly by renaming and extending that to allow editing of define pages and email content, and also some new way to define language strings that doesn't put them in unfriendly PHP language files, against maybe a new editor ability that knows how to present them using a friendly UI. Where we currently have PHP logic in a language file could be replaced by making them Smarty template snippets, single strings that would be processed through Smarty to achieve concatenation or branching logic as required.
2. Use a 'modern' templating system such as Smarty to generate emails, instead of simple text substitution as is currently used. This does bring a lot of power to the table, over plain text substitution.

What I'm trying to achieve here is:

• Single email template that defines HTML structure, without having to replicate it for each language (like we have now)
• Keep the benefit of having language strings defined outside the email template
• Make it all editable by the user via a UI

For example, an email template that is language agnostic:

email_template_order_details.html:

<p>EMAIL_GREETING,</p>

{if some_config_switch}
<p>${SOME_CONFIG_TEXT_CONTENT</p>
{/if}

<p>EMAIL_ORDER_DESC</p>
<table><thead>blah blah</thead>
<tbody>
{foreach from=products item=product}
  <tr><td>{$product.products_name}</td><td>{$product.final_price}</td></tr>
{/foreach}
</tbody></table>

Associated defines brought in from the current language files lang.email.php:

languages/english/lang.email.php which may be editable by the user by some mechanism like the one I described (they may have to move out of this directory structure, so the Editor page knows which files are editable and which aren't, although if we make some language strings editable via a UI, why not extend this to all?):

EMAIL_GREETING: Greetings {$customer_name}
EMAIL_ORDER_DESC: Your invoice number is {$orders_id}, for the following products:

Any file that's editable by the user should be considered fluid and subject to change. Currently I manage all email templates and lang.whatever.php files in git and deploy tagged versions of our website overwriting current, whereas define pages in includes/languages/english/html_includes/** are considered uncontrolled and are excluded from our deploy process.

Has there been an effort in the past to extend the Define Pages admin edit page to edit more than just that? Any other feedback? @scottcwilson you say you've had similar requests from clients, how far down each of these paths would you be willing to entertain? Let's presume that developer resources are infinite at this point and just talk about what we would want.

[torvista]

Single email template that defines HTML structure, without having to replicate it for each language (like we have now)

There is no need to currently replicate the email-templates per language, unless a different format is required per language.

[neekfenwick]

Apologies, that was poorly worded. I meant 'like we have now' as in 'in my current proposal'. Of course current ZC does not do this.

[scottcwilson]

I think this would be a mistake and would make the change much less useful. Remember, most Zen Cart storeowners support only one language (English). Optimize for English, with the ability to go multilingual.

[scottcwilson]

Have you considered Twig instead of Smarty?

I'm attaching the email templates (Twig based) that OpenCart uses for your consideration
mail.zip

[neekfenwick]

Have you considered Twig instead of Smarty?

Ah Twig, I think I used that around 2005 in Perl. I had not considered it :) Do you mention it for a specific reason over Smarty? From quick research, Smarty seems older, but the pages I found comparing the two are really poorly written (e.g. https://www.freelancinggig.com/blog/2017/03/25/pros-cons-twig-vs-smarty-web-templates/ ugh) .. I'd really have to play around with Twig before casting an opinion. I've used Smarty for years and am happy with it (including custom template functions written in PHP in our Zen Cart codebase, seems good) but am happy to consider alternatives.

[edit] Looking at those Twig templates it seems they take the latter of my approaches, to let the code building the template data model do the localization work, and the template is language agnostic, e.g. affiliate.twig expresses {{ text_welcome }} .. makes sense. There's more to talk about on the subject of making the two parts of ZC's email system editable by admin users via a UI, as I mentioned earlier, 1/ the template and 2/ the language strings.

I also ran into Blade, a part of Laravel (ref: https://laravel.com/docs/9.x/blade). I noticed the latest Zen Cart has a laravel folder and seems to use a bunch of its Model classes e.g. laravel/app/Models/Customer.php. I've heard of Laravel but never used it. Can you fill me in on how integral Laravel is to Zen Cart these days, roughly where the inspiration came from to use it and integrate it, with a view to saying that if we're talking templating systems we ought to be using the one that comes with Laravel simply to cut down on dependencies (i.e. don't add Smarty or Twig or a.n.other)?

[scottcwilson]

Blade is another option too. I don't have a strong preference, just wanted to be sure you have considered a few options.

[mc12345678]

Blade is another option too. I don't have a strong preference, just wanted to be sure you have considered a few options.

Maybe this uses the same language templating as already discussed. I haven't torn it apart in a few years and don't personally recall, but there is an open source project that uses a sort of insert this text where it appears the translation is in the admin. I don't know or recall if that translation is then passed back to a central sharing area or not, but....

PhpMyFaq: https://www.phpmyfaq.de/ which is also in Github at: https://github.com/thorsten/phpMyFAQ

[scottcwilson]

One small tweak to your original post - please don't use includes/languages/english/html_defines/.
Let's either put them in email/ or a new folder.

Your original design would be much easier for non-technical storeowners to tweak their emails.

Is your vision of text email that we'd simply do a strip_tags on the HTML email?
(We have to send both.)

[scottcwilson]

Correct - you have to produce both but I'd like to have only one template so the store owner doesn't have to edit multiple files. We could do this using strip_tags or perhaps there's some other clever way to do it.

[neekfenwick]

Agreed, email templates should still live in their old folder, probably staying in the email folder they're currently in.

I looked at Blade template syntax, I'm not a big fan of its syntax but it's ok. My big problem is Laravel. Laravel seems to have been copy/pasted into Zen Cart piecemeal, not via a dependency management system like Composer, and has ZC specific class files scattered throughout the tree. I tried replacing laravel with a composer.json directive but am having trouble making it work.

[neekfenwick]

strip_tags works well for some cases. I like the option of having an optional text template for when it doesn't do a good job, such as complex order content where HTML would use a <table> that does not strip nicely into TEXT.

Some kind of preview on the editor page would be a nice-to-have, too, using dummy data.

[scottcwilson]

I had forgotten about that - Blade might be more trouble than it's worth, but it's your call.

[neekfenwick]

My big problem is Laravel. Laravel seems to have been copy/pasted into Zen Cart piecemeal, not via a dependency management system like Composer, and has ZC specific class files scattered throughout the tree. I tried replacing laravel with a composer.json directive but am having trouble making it work.

I was talking bollocks here, turns out there's a laravel/composer.json as well as the top-level composer.json, and Blade does seem to be in place, which is a relief. I'm just quite ignorant of Laravel and its structure. I've got a basic template being generated, faffing around with that now. I feel the other discussion about editing language strings is a major blocker on this discussion.

[scottcwilson]

The goals I'd like to accomplish in this work are:

• make it much easier to modify both HTML and TEXT email.
• If possible, preserve backwards compatibility for plugins that haven't migrated.

[neekfenwick]

Agreed. Focusing on English but supporting other languages is sensible, and reminds me that while html_defines live in a language specific subfolder, email templates do not. We rely on the language strings to do 100% of the localization and message content definition. So when we talk about editing 'email content' I think we're talking more about being able to edit the language strings, even more than the other important factor of editing the template. However, editing the email template is relatively simple, whereas editing language strings is a much greater challenge given the way ZC strings are currently defined, often in complex PHP files with conditional logic. Is it an acceptable 'cheap charlie' effort to just make the template editable for now, and assume a store is likely to be single language or would be happy to do the usual legwork to edit language strings? I see this being a bit disappointing for some store owners who do want easy content editing for multiple languages.

[lat9]

Actually, the email templates can have language-specific versions. The function zen_build_html_email_from_template uses the current $_SESSION['languages_code'], e.g. 'es' for Spanish, to locate a language-specific template in the site's /email[/lang] directory.

[scottcwilson]

Is it an acceptable 'cheap charlie' effort to just make the template editable for now, and assume a store is likely to be single language or would be happy to do the usual legwork to edit language strings? I see this being a bit disappointing for some store owners who do want easy content editing for multiple languages.

Translating email templates is just another task for language translators. This is a nice step forward.

while html_defines live in a language specific subfolder, email templates do not.

They do. https://docs.zen-cart.com/user/email/html_email_templates/

[neekfenwick]

Is it an acceptable 'cheap charlie' effort to just make the template editable for now, and assume a store is likely to be single language or would be happy to do the usual legwork to edit language strings? I see this being a bit disappointing for some store owners who do want easy content editing for multiple languages.

Translating email templates is just another task for language translators. This is a nice step forward.

But the email templates themselves don't contain translated strings. We have e.g. email/email_template_welcome.html but there's no English text in it, that all comes from language defines. If we don't make the language defines easily editable, we don't get easy editing of email content, unless as I say you're happy to put translated text in the email template and thus break the multilingual support provided by ZC.

while html_defines live in a language specific subfolder, email templates do not.

They do. https://docs.zen-cart.com/user/email/html_email_templates/

Oops you're right, I just ran into the line in zen_build_html_email_from_template where it does actually look for DIR_FS_EMAIL_TEMPLATES . $langfolder . "email_template_"; so emails can reside in language specific subfolders, they just don't do so in the core release of ZC. Quite a surprise to me, given that as discussed above the language strings already provide localisation, but OK. I'm learning as I go on this one :)

[scottcwilson]

But the email templates themselves don't contain translated strings.

We're going to have to use more precise language moving forward to prevent confusion. The new email templates will contain translated strings.

<p>Greetings {$customer_name},</p>

{if some_config_switch}
<p>We also provide service xyz.</p>
{/if}

<p>Your invoice number is {$orders_id}, for the following products:</p>
<table><thead>blah blah</thead>
<tbody>
...

[neekfenwick]

Ah ok, I see you're sticking with your reply to my rather long comment above where I proposed sticking with the single template and translated strings being substituted in, but you said this was unwise and to assume most stores are only supporting one language anyway. OK, I can certainly see that if we have a template file per language, it makes the job of providing an admin editor to edit email content more straightforward.

In the new system, we still have the option of using define'd language strings, such as STORE_NAME in the template. I'm working on the Order Status Update template first, as it's easy to trigger, and will see how many of the strings are still useful. There's an awful lot of little logic statements in the functions_email.php code that have been put together over the years, like the way it looks in several places for the template to use including the current page's name, and I'm loathe to start ripping them all out, so am trying to keep things as unchanged as possible. Fortunately, the way zen_mail and friends builds the $block array and then substitutes it into the template is pretty identical to the way other systems work.

In your opinion does the new system want to keep a compatible zen_mail() function, so that legacy code can call it and expect identical behaviour to the old system, or would we be able to completely rewrite it in core ZC and assume everyone else follows suit? This sounds pretty dangerous to me, with enough legacy addons etc popping up with the 1.5.8 update having trouble with the new system. We'd want to try to keep code backward compatible while keeping the core using the system system throughout.

[scottcwilson]

Keep compatible with the old way for plugins if possible and not too much work. There are not a lot of plugins that define their own templates, so if it's too hard to do, let's not do it. But for now focus on the new stuff and let's see how much better we can make it; then we'll worry about legacy.

[neekfenwick]

I feel you, but I wasn't very clear in what I wrote, the new zen_mail function does not take an $email_text parameter, its signature in my WIP has changed from:

function zen_mail($to_name, $to_address, $email_subject, $email_text, $from_email_name, $from_email_address, $block=array(), $module='default', $attachments_list='', $email_reply_to_name = '', $email_reply_to_address = '' ) {

to the same but without $email_text:

function zen_mail($to_name, $to_address, $email_subject, $from_email_name, $from_email_address, $block=array(), $module='default', $attachments_list='', $email_reply_to_name = '', $email_reply_to_address = '' ) {

because of course if the templating system is taking care of producing the HTML or TEXT formatted email there's no need for the calling code to construct the text version of the email in PHP code and pass it in. This would be a backwardly breaking change, and any plugins that call zen_mail, even if they do not define a new template and just use email_template_default would break. Unfortunately that means we would need to keep the code that deals with the $email_text variable when it is passed, and bypass the new template lookup, it ends up being quite messy.

I realise this is mostly my problem, I'm just feeling out the larger project requirements guiding the work. As you say, if possible and not too much work :)

[scottcwilson]

Bear in mind too that you could hypothetically provide an editor for language strings (for email only). Look at (for example) includes/languages/english/lang.email_extras.php. There's no logic in this file, so you could create a screen that allowed you to edit the values for each key, then re-write the file on a save. Would this make more sense than just embedding the text in the new template? Not sure, but it's an idea. If you add it a little javascript that mocks up the email at the bottom of the file, it could be just as easy as providing a template with hardcoded strings (and much easier for translators to deal with).

[scottcwilson]

I'm hoping you (somehow) can bake in the functionality of Preview Email too.

https://www.zen-cart.com/downloads.php?do=file&id=2220

[neekfenwick]

I have pushed work so far to https://github.com/neekfenwick/zencart/tree/discussion-5499 .. initial work in Feb/March went pretty well but I was discouraged by the small amount of interest expressed here so stopped work for a while. It is very much a Work In Progress so please be kind, treat this as a curiosity rather than any kind of formal review, but comments on the broad approach are welcome. I'll need to iterate quite a bit as I focus on more than the 2 emails I've done so far.

Some notes:

• I've tried to keep the old zen_email function identical to before for back-compat with third party addons, and introduced a new function for sending emails using the new system. The big difference is that the old function took the text version of the email as a parameter, which is now redundant.
• Uses Laravel's Blade engine to turn both text and HTML templates into email bodies. No new third party dependencies were introduced.
• Tries to find templates using that same language based lookup the old system used. It's pretty ugly code.
• We now ideally need two template files per email, because the text and HTML emails are typically so very different. Stripping an HTML email to text generally goes pretty badly. More likely, a text body could be patched with <br> etc to make HTML.
• Templates are presumed to be named email_template_foo.blade.php and email_template_foo_text.blade.php. The _blade.php bit is hardcoded into Blade and can be changed if we need to, it's just more fiddly. Hopefully with an admin editor the user won't see the filenames much.
• Only two emails have been looked at so far: welcome and order_status, because they are relatively easy to generate while testing and have an interesting data model and lang string content.
• I started building an admin page for editing emails but got stuck trying to make a RESTful endpoint work to make the editing more seamless. Laravel's framework doesn't seem complete, or simply can't be used standalone, I'm not sure. There are some files in the illuminate/routing folder that I couldn't get to work. Help would be appreciated.

Some changes to note:

• The old system might have some duplication, such as a lang string used for the text version of the template and some hardcoded text in the email template file for the HTML version. The new system aims to have only one lang string, used in both the text and html versions of the template. Lang strings are put into the data model and given to the engine for expansion, much like the old string replacement system did.
• As far as I can see the Blade engine doesn't have access to all global strings defined when it is invoked, so you have to put all lang strings into the model for the template.

Example data model:
The model might start with some strings:

$model = [
  'WELCOME_TEXT' => $WELCOME_TEXT'
];

Additionally, put data required by the template:

$model['customers_name'] = $customers_name;
$model['products'] = $products_array; // can be an array or object, iterated in the template

This model is then processed by the Blade engine as it expands the template.

[neekfenwick]

In moving the construction of the order confirmation email ('checkout') I ran into this beauty:

zencart/includes/classes/order.php

Lines 1225 to 1243 in f10838c

	$this->notify('NOTIFY_ORDER_DURING_CREATE_ADD_PRODUCTS', $i, $custom_insertable_text);
	/* START: ADD MY CUSTOM DETAILS
	* 1. calculate/prepare custom information to be added to this product entry in order-confirmation, perhaps as a function call to custom code to build a serial number etc:
	* Possible parameters to pass to custom functions at this point:
	* Product ID ordered (for this line item): $this->products[$i]['id']
	* Quantity ordered (of this line-item): $this->products[$i]['qty']
	* Order number: $zf_insert_id
	* Attribute Option Name ID: (int)$this->products[$i]['attributes'][$j]['option_id']
	* Attribute Option Value ID: (int)$this->products[$i]['attributes'][$j]['value_id']
	* Attribute Filename: $attributes_values->fields['products_attributes_filename']
	*
	* 2. Add that data to the $this->products_ordered_attributes variable, using this sort of format:
	* $this->products_ordered_attributes .= {INSERT CUSTOM INFORMATION HERE};
	*/
	$this->products_ordered_attributes .= $custom_insertable_text;
	/* END: ADD MY CUSTOM DETAILS */

Can anyone say if NOTIFY_ORDER_DURING_CREATE_ADD_PRODUCTS is used by anyone to set up $custom_insertable_text?

I'm moving all text/html construction out of functions like create_add_products and into send_order_email where it belongs. This $custom_insertable_text variable is set up in create_add_products and presumably may rely on the state of the $order object at the time (since listeners can access the calling object). I can move the call to the notifier into send_order_email but will have to keep its name containing DURING_CREATE_ADD_PRODUCTS so it's going to look pretty silly.

[torvista]

I'm using it....but I don't care, if changes are made that are logical in the context, and it's documented.

[scottcwilson]

Woe be to the contributor who changes a notifier. :)

[neekfenwick]

I'm performing veritable open heart surgery on this email system, guess I'm a glutton for punishment 😆 .. WIP

[drbyte]

That $custom_insertable_text is used along with (actually I think, updated by, the notifier) in cases where for example a store sells prepaid phone cards or software serial numbers, which are generated in real time when the order is saved and emailed. This is how those numbers are inserted into the confirmation email.

[drbyte]

I'm performing veritable open heart surgery on this email system, guess I'm a glutton for punishment 😆 .. WIP

Indeed you are! My apologies for the gremlins you find in there. That whole system was one of my early contributions to the project. I've learned a lot since then! 😊

[neekfenwick]

Minor point: Text version of 'checkout' order confirmation email was missed 'Shipping method'. I've added it.

I'm also adding a common header to the text versions of emails that wasn't there before, the way text bodies were built was a bit Wild West, for example checkout dove straight in with a Order confirmation from... message. I'm using EMAIL_LOGO_ALT_TEXT as the header string so it matches what's put in the HTML email header.

[drbyte]

I'm also adding a common header to the text versions of emails that wasn't there before, the way text bodies were built was a bit Wild West, for example checkout dove straight in with a Order confirmation from... message.

I "think" that was intentional, at least in some cases, to get-to-the-point quickly. Many email clients (especially mobile devices) show a preview of the text-only content of an HTML message, and if the first lines of the text-only portion are just branding stuff, it makes the preview useless.

Of course, the storeowner can tweak it to their liking, so I guess they can remove what they don't see is needed. The subject line will hopefully contain useful info as well.

[neekfenwick]

FYI my dev branch is at https://github.com/neekfenwick/zencart/tree/discussion-5499 - I'm maintaining a local .md file to track progress, I'll put it here and Edit it as I go, if that helps track where I'm at.
Edit Update 13 Jul 2023 - all completed.

Location / Description	Module Used	Status
admin/coupon_admin
- action 'send_email_to_user'	coupon	Done
- copy to admin	coupon_extra	Done
admin/customers.php
- case 'status' changing customer status	default	Done
- case 'pwdresetconfirm'	default	Done
- again in 'pwdresetconfirm'	default	Done
admin/gv_mail.php
- action 'send_email_to_user'	gv_mail	Done
- copy to admin 'gv_mail_extra'	Done
admin/gv_queue.php
- action 'confirmrelease'	gv_queue	Done
admin/mail.php
- action 'send_email_to_user'	direct_email	Done
admin/modules.php
- case 'save'	admin_settings_changed	Done
- case 'install'	admin_settings_changed	Done
- case 'removeconfirm'	admin_settings_changed	Done
admin/password_forgotten.php
- action 'update' success case	password_forgotten_admin	Done
- action 'update' failure case	password_forgotten_admin	Done
admin/includes/functions/admin_access.php
- zen_delete_user()	admin_settings_changed	Done
- zen_insert_user()	admin_settings_changed	Done
- zen_update_user() to old user	admin_settings_changed	Done
- zen_update_user() to new user	admin_settings_changed	Done
- zen_validate_user_login() too many login attempts	no_archive
admin/includes/modules/newsletters/newsletter.php
- send() function	newsletters	Done
admin/includes/modules/newsletters/product_notification.php
- send() function	product_notification	Done
includes/classes/order.php
- send_order_email() function - low stock email to admin	low_stock
- send_order_email() function - order confirmation email to customer	checkout	Done
- send_order_email() function - extra order confirmation email to admin	checkout_extra	Done
includes/classes/observers/auto.downloads_via_streaming.php
- updateNotifyDownloadReadyToStream() function - admin warning email	default	Done
includes/modules/create_account.php
- welcome email	welcome	Done
- extra welcome email to admin	welcome_extra	Done
includes/modules/pages/ask_a_question/header_php.php
- email to store admin	ask_a_question	Done
includes/modules/pages/contact_us/header_php.php
- email to store admin	contact_us	Done
includes/modules/pages/download/header_php.php
- 3 warning emails to admin	default	Done
includes/modules/pages/gv_send/header_php.php
- customer email	gv_send	Done
- extra admin email	gv_send_extra	Done
includes/modules/pages/password_forgotten/header_php.php
- customer email	password_forgotten	Done
includes/modules/pages/product_reviews_write/header_php.php
- review notification email to admin	reviews_extra	Done
includes/modules/payment/authorizenet_aim.php
includes/modules/payment/authorizenet.php
includes/modules/payment/firstdata_hco.php
- all 3 above, admin warning email	debug	Done
includes/modules/payment/paypaldp.php
- about 12 warning emails to admin	paymentalert	Done
includes/modules/payment/paypalwpp.php
- about 6 warning or notification emails to admin	paymentalert	Done
includes/modules/payment/square.php
-	payment_module_error	Done
-	debug	Done
includes/modules/payment/paypal/paypal_curl.php
-	debug	Done
includes/modules/payment/paypal/paypal_functions.php
-	debug	Done

[neekfenwick]

Wanted to point out the NEW_SIGNUP_GIFT_VOUCHER_AMOUNT code in includes/modules/create_account.php only effects the TEXT version of the welcome email. It looks like it's buggy and does not put the info into the HTML version of the email. I'm going to fix it, but this may surprise store admins who haven't noticed they still have this value set because they only check the HTML versions of emails.

https://github.com/zencart/zencart/blame/8a7ef921623758b2afa10823b6a54bd3aba95322/includes/modules/create_account.php#L325-L361

Describing the bug: The coupon stuff seems well handled, zen_mail() looks for COUPON_TEXT_VOUCHER_IS and sets up COUPON_BLOCK which is then used in the welcome email. But, the NEW_SIGNUP_GIFT_VOUCHER_AMOUNT code in create_account.php sets up GV_WORTH, GV_REDEEM etc and there's code in the zen_mail function that looks for GV_ANNOUNCE in the $block of data, which is not found, so it won't result in welcome HTML email content. The text version is there, because it's built in create_account, but the HTML body won't have anything.

Some dev context: My new system under development does not rely on zen_mail() sniffing out values and populating other members of $block .. rather, the email templates themselves react to the template data. So instead of the old system where the building of text and HTML content was split between the create_account module and code in zen_mail():

• create_account puts together $email_text with a text version of the data, and puts values like GV_WORTH, GV_REDEEM into $block`.
• zen_mail() sniffs out GV_STUFF and sets up GV_BLOCK with HTML content.
• email_template_welcome.html includes GV_BLOCK, which is either empty string or the HTML content.

The new system:

• create_account puts values like GV_WORTH, GV_REDEEM into $block`.
• The text and html versions of the template build content according to these values.

[drbyte]

this may surprise store admins who haven't noticed they still have this value set because they only check the HTML versions of emails.

Good catch.

Many don't know the feature even exists.
Nowadays storeowners are more inclined to want to send such a coupon only after the customer has placed their first order. But we don't have such a feature in core presently.

[neekfenwick]

To note another old bit of cruft I ran into: Gift Vouchers, 'Send Gift Voucher' feature, lang.email_extras.php contains a string EMAIL_ADVISORY which is only ever used in the gv_send situation, and even then only in the TEXT version of the email.

    'EMAIL_ADVISORY' => '-----' . "\n" . '<strong>IMPORTANT:</strong> For your protection and to prevent malicious use, all emails sent via this web site are logged and the contents recorded and available to the store owner. If you feel that you have received this email in error, please send an email to ' . STORE_OWNER_EMAIL_ADDRESS . "\n\n",

This is only true if EMAIL_ARCHIVE is true so emails are logged, and that setting is false by default. Opinions?

I do see the sense in such a warning, given that this one gv_send email is when a customer has chosen to cause the store to send an email to an unknown third party in order to send them a gift certificate. In which case it should be in the HTML email too.

On top of this, the tpl_gv_send_default page tells the user EMAIL_ADVISORY_INCLUDED_WARNING:

    'EMAIL_ADVISORY_INCLUDED_WARNING' => '<strong>This message is included with all emails sent from this site:</strong>',

This isn't true, that advisory is only ever sent in the gv_send case (not gv_queue or any other email).

So, I'll fix the ...INCLUDED_WARNING to just say it's only sent in the gv_send email, and include the advisory in the HTML email, but only if EMAIL_ARCHIVE is true.

[mc12345678]

Not copying all the above content. First thing that comes to mind is to look at the history of that messaging. E.g., when was it added that all messaging was to include particular text and when and why did that get removed if it was previously included in more than just gv_send?

In my opinion I would think that the status of recording messaging shouldn't omit the applicable statement. Instead I would think it may offer some other unique data to help a store owner to validate the message truly came from their site and wasn't just some sort of spoof.

I agree that content in the text only message identified above ought to be in the html also, though that history also is worth looking into.

[neekfenwick]

@mc12345678 I see your point but if EMAIL_ARCHIVE is false what else can it say? The message seems purely to placate the recipient, it's doesn't help the store owner validate it in any way. It looks like something either added to look reassuring, knowing it may not be true, or simply an oversight that's crept in over the years. Probably not worth investigating, just fix it and move forward.

[mc12345678]

Ah, I see the fallacy in my statement. I had latched onto the latter part of:

    'EMAIL_ADVISORY' => '-----' . "\n" . '<strong>IMPORTANT:</strong> For your protection and to prevent malicious use, all emails sent via this web site are logged and the contents recorded and available to the store owner. If you feel that you have received this email in error, please send an email to ' . STORE_OWNER_EMAIL_ADDRESS . "\n\n",

The first part of that message related to logging, really only applies as stated. It's the if you have received in error part that I was thinking should be on all.

The aspect associated to research would be more about:

    'EMAIL_ADVISORY_INCLUDED_WARNING' => '<strong>This message is included with all emails sent from this site:</strong>',

Was this ever true? Was it an inadvertently dropped action to include? Was it being removed in steps and this one occurrence was not addressed? I would think that there would be interest in ensuring appropriate or sufficient coverage.

[drbyte]

There is indeed some history as to when certain of these "advisory" messages were coded to no longer be displayed, or are displayed in certain circumstances.

I would say that the current timing of when certain things is sent is correct, but perhaps the wording could be improved to be less emphatic that "all emails are logged", so that even if logging is not turned on, the intended sentiment is still conveyed.

Remember: hackers and bad-actors read-into what is said or not said. We don't have to say "hey, we've got logging turned off, so don't bother telling us you've been sent stuff you don't want, cuz we won't be able to help you". Instead say the other part that focuses on "please contact us if you want us to investigate...".

[scottcwilson]

Lots of great progress here. I will spend some more time looking at this next week. Thanks!

[neekfenwick]

FYI @scottcwilson a note on the admin email template editor, which is half baked at present. While working on One Page Checkout I ran into the ajax.php files that facilitate async calls from the UI. I spent a bunch of time trying to figure out if the Laravel laravel/routes/routes.php files are functional but couldn't get it to work and saw no response to my comment #5499 (comment) .. I'd really like to use a well developed RESTful system with routing, middleware etc, but I'm going to presume zencart does not plan to employ Laravel for this task and instead focus on this more home grown ajax.php approach. It's a shame, because ajax.php bakes the name of the class and method into the URL, so it's very brittle. Even the term 'ajax' seems archaic at this point :)

So, in time I'll update the admin editor to use async requests to generate email previews and possibly strings editing. I halted work to get feedback from people such as yourself on the core template changes I've made, before building further on that.

[drbyte]

I spent a bunch of time trying to figure out if the Laravel laravel/routes/routes.php files are functional but couldn't get it to work and saw no response to my comment #5499 (comment) .. I'd really like to use a well developed RESTful system with routing, middleware etc, but I'm going to presume zencart does not plan to employ Laravel for this task

Some of the open Pull Requests that are in Draft mode/status are work-in-progress efforts to build a RESTful API service into Zen Cart, leveraging the Laravel framework. I'm not expecting it to be fully ready for the 2.0 release though.

A few years ago I'd built a very basic web-hooks thing for a Zen Cart site that handled real-time inventory updates, order status updates for shipping notices and tracking numbers, enabling of sales/specials and adding/deleting products ... all pushed/pulled from an ERP system with JSON data calls. It wasn't really an API. More just web hooks.
But that's not exactly AJAX-y to handle on-screen previews of generated content like email templates would require.
I didn't use the ajax.php file, but did take some inspiration from it. There was no "routing" based on URL; instead, like "ajax.php" I think the handler for it was /hooks.php and it basically loaded whatever classes were needed based on the incoming payload request type, and after authenticating that the request was from a trusted source, took action on the directives in the JSON that was POSTed in the request.
But again, while it served a purpose, it wasn't a RESTful API with routing, middleware, etc.

That said, the reason I mention it is because even though "...ajax.php bakes the name of the class and method into the URL, so it's very brittle", you could do all your email template preview/etc things through one or more of said (created by you) classes and methods and have it return the desired responses.

[neekfenwick]

@drbyte fair enough. For my day job I use https://github.com/peej/tonic with some modifications as it appears to be dead. ajax.php is working fine, it just lacks abstraction. As I said I tried to use the Laravel stuff but every document I found involved running their own web server and every use of templates I could fine presumed you were rendering a View within their MVC framework, so I have used https://github.com/PhiloNL/Laravel-Blade in my Zen Cart email template work to be able to generate output from a template at will.

@scottcwilson nudge did you have any time to review after your mid August return? I have things to say on topics I'd expect you to raise on a number of issues - exposing users to HTML, inability to edit templates using WYSIWYG editor, what to do about the old email template .html files, etc.

[scottcwilson]

Sorry, no. I'm pretty busy these days and now that @drbyte has re-engaged, I'm stepping back a bit. You're in good hands.

[neekfenwick]

@drbyte have you re-engaged? :) This is a large piece of work and I don't want to carry the can for ages and then face a backlash that makes it all a waste of time. I'm feeling rather out on a limb, here.

[neekfenwick]

By the way I ran into the PHPUnit tests in not_for_release today, and will try to build a test that exercises the stock email templates, as they are effectively PHP code now and have the potential for compile failure in a way the old templates did not.

[neekfenwick]

Could do with some advice. To what extent can the unit tests use data that normally resides in the configuration database table? edited for clarity

I've run into a circular dependency problem due to the way zcUnitTestCase sets itself up rather manually without requiring application_top.php and the usual bootstrapping. To build an email, you need lang strings, the lang strings sometimes refer to database constants (like ENTRY_CITY_MIN_LENGTH in lang.english.php), and database constants are loaded by includes/init_includes/init_database.php which uses symbols like DB_TYPE which are normally set by configure.php, but requiring that runs into issues with HTTP_SERVER already defined.

I'm not trying to access the database myself, just needing to include files (the lang.* strings) that use DB constants.

I'm not sure if I can mock the database during unit testing or is this whole attempt untested waters?

[drbyte]

I'm not sure if I can mock the database during unit testing or is this whole attempt untested waters?

Before moving the test suite to Laravel, the (currently named) testDbLogWriterInstantiation and other tests for the Database LogWriter were fully implemented. Presently the db-mock hasn't been implemented for those tests.

@zcwilt were you able to get those db-mocks implemented in another branch?

[drbyte]

circular dependency problem
I'm not trying to access the database myself, just needing to include files (the lang.* strings) that use DB constants.

Ironically most of the constants that the email templates would need are few, and could probably be passed as the constant-name as far as the test-suite goes, since many of the tests won't rely on the values of those constants. Not always, but in many cases.

I wonder if the language system that processes the lang.* strings could be called in some sort of test mode that allowed passing of dummy or mocked data for necessary constants, and unneeded ones just ignored instead of triggering errors? Or could a helper be created to "do" that somehow, for the sake of the test suite.

Also ironic: the email templating system is the only place in ZC where replaceable parameters are parsed instead of being executed, meaning any undeclared replacements are just ignored without error (for better or worse). But the language file system is still tied to constants :(

[neekfenwick]

Am I right in thinking the xml_record email template is not used in ZC Core, but may be used by addons to send plain text emails containing XML, so the behaviour needs to be maintained?

Until now I had presumed all emails should look 'pretty', and so even passing a plain string as the mail body should use the 'default' template and put a header/footer on it. Now I look at the old behaviour, passing a plain string specifically omitted the HTML portion and just sends a non-MIME plain text email with that string, so I've introduced a regression. I think I've seen store owners comment on the forum they like this behaviour, to just get a simple text admin email without any faff.

I'll look at reverting some of my work to make this old behaviour the norm with the new template system.

[drbyte]

Am I right in thinking the xml_record email template is not used in ZC Core, but may be used by addons to send plain text emails containing XML, so the behaviour needs to be maintained?

Yes that is correct.

[drbyte]

I think I've seen store owners comment on the forum they like this behaviour, to just get a simple text admin email without any faff.

This is correct.

But, if we're able to generate both HTML and TEXT components of an email (which is required when sending HTML emails: you must supply a non-HTML version, for maximum deliverability), then we can certainly skip the HTML portion if we want to send a TEXT-only version based on whatever situation requires it.

[neekfenwick]

I've got the admin page for editing email templates working to a good degree and am hitting some nitty gritty details that need sorting out. When editing an email template on the admin page, we want to present a set of editable strings that are used to build the email. I'd like some advice on how to handle the fact that the PHP files containing the strings are often not directly related to the email being edited.

Taking a meaty example, the checkout email template is used in includes/classes/order.php called from includes/modules/checkout_process.php, which naturally loads lang.checkout_process.php due to the way the require_languages.php module works. So with the current file structure, when editing the checkout template we would want to load and make editable the strings in checkout_process.php. In this case, the filename containing strings checkout_process.php is not directly deducible from the name of the email template being edited, checkout.

Some possible solutions:

• hardcode in the editor the knowledge of what lang files are used for each template, so it knows which to load. Con: hardcoding is bad.
• rename the lang files to match the email template name, so the lang filename can be directly deduced from the email template. Con: enforces a one-to-one mapping problem, if two templates use the same strings they can't share a lang filename.
• reorganise strings in the lang files into a new arrangement so the strings are in some other deducible location. Con: migration effort, and possibly another one-to-one mapping problem.

What do you guys with a longer view of the project think? Has it been irksome for some time that the lang strings sometimes come from rather hard to deduce places, and is there a plan fomenting anywhere to reorganise them, in which case now would be a great time to do it?

Side note: email generation actually generally depends on two data sources, 1/ the lang strings mentioned above and 2/ raw data specific to the task e.g. INTRO_ORDER_NUMBER which holds the order ID. These two sources are merged into one $block array passed into zen_mail(). When previewing emails during editing I'm trying to take both sources into account while holding the lang strings as a unique case where the user needs to be able to edit and save them, while the transient data is only necessary as dummy data for generating the preview. I'm currently planning to maintain a set of json formatted files in a data directory somewhere to hold this dummy data, so it will be static for an installation of Zen Cart but maintainable by developers or store owners if the email templates change to need new or different dummy data.

[drbyte]

Yes, language stuff is hard. It's easily one of the most annoying things to have to constantly code for (in any platform, although some environments abstract a lot of it away, differently).

One of the challenges you've been trying various approaches to solve, is rendering the placeholder text contents in the editor, both to show what the placeholder "says", and also to retain the placeholder so that the template can remain language-independent. It's definitely a tough problem. Exacerbated by the complexity of the end-user's temptation to actually type language-dependent ("hardcoded") text into those templates.

(Obviously a storeowner who knows/supports only one language will hate that they can't type hardcoded text in the email template editor. And a multilingual storeowner will hate that they can't see their email in both languages at once, and can't split strings up to read "better" in one language vs another.)

The only way to "please" everyone would be to provide an editor which doesn't use language-strings at all. Just provides a default set of text (per language), that they then edit directly, and save "that" content for all their future emails.
BUT, obviously there is still need for placeholders for dynamic data (non-language-defines) like order numbers, coupon codes, product data in an order, etc.
AND the HTML markup.

It was these challenges that relegated the templates to developers and code-editors instead of storeowners and admin template-editor UIs.

[drbyte]

Your tracking of required keys/relations via a json file (presumable read by the Editor) seems useful. It certainly lets you load "some" instead of "all" language files, on demand based on whatever template is selected.

Thought:
Since v1.5.8 has language strings in the new array-based files (where duplicates aren't fatal errors), instead of constants (where duplicates are fatal), I would imagine it's much easier to compile a complete list of available strings by loading all the needed arrays into memory, even keyed on their filename (or reasonable portion thereof); or not.
So instead of calling require_languages, perhaps something can be written to load "everything" or at least "everything that could be email-related". This could be used generally, or could be used as a fallback for non-core email templates that are added by a store and don't have corresponding json data available to aid the mapping.

[drbyte]

As you said elsewhere, refactoring language string files is another huge undertaking. And there's no panacea there either, since not all language-strings are there only for email purposes. Many are shared, intentionally so that there is less duplication and also less translation.

I agree with deferring that topic to a separate project. The email templating portion is already massive enough.

[neekfenwick]

This work has sat on a branch for a long time and I feel it needs a serious push to reach PR status and get some serious attention from core devs or it might miss a major ZC release and become too difficult to release. I'm planning to cut back on the scope of my work on this to remove the editing of lang strings. It's just too thorny, and I haven't had clear answers to a couple of questions about refactoring lang string files, it's simpler to leave it out here and add as a new feature later on.

One thing I do need to finalise here is the file structure for email templates. Old templates are of the form email/email_template_foo.html, and I think a lot of stores may want to retain the old templates for any addons that still use the old zen_mail() call to send emails. The new templates are in HTML/TEXT pairs and I feel they should be in a unique subfolder of their own. Offhand, I guess I'll use a folder email/templates and put files in there, omitting the email_template_ part of each filename. Ought to be cleaner.

[drbyte]

I guess I'll use a folder email/templates and put files in there, omitting the email_template_ part of each filename.

I support this approach.

[drbyte]

Remember, the system first looks for /email/[lang]/foo.html and then falls back to /email/foo.html, where [lang] is the 2-letter language code.
Probably good to maintain that. So /email/[lang]/templates/...

the email templates can have language-specific versions. The function zen_build_html_email_from_template uses the current $_SESSION['languages_code'], e.g. 'es' for Spanish, to locate a language-specific template in the site's /email[/lang] directory