Version Upgrade Routine for customized Advanced PHP templates in version <= v160213

[raamdev]

The Comment Mail codebase is being completely revamped to bring it up-to-date with PSR 2/4 standards (see https://github.com/websharks/comment-mail/issues/150). These changes will affect any Advanced PHP templates that have been customized in any version up to and including v160213.

If Comment Mail Advanced PHP templates were modified, the next release will produce fatal errors unless we can come up with a way to handle this gracefully.

What we need to do for the next release is to build a version upgrade routine that will detect and handle a scenario where a site owner has customized the Advanced PHP templates and then prevent a fatal error from being thrown, along with a Dashboard notice that links to a KB article explaining how to migrate Advanced PHP templates to the new codebase.

• [x] Build version upgrade routine to gracefully handle customized Advanced PHP templates
• [x] Write Dashboard notice that announces this issue to site owners who have customized their Advanced PHP templates

---

Referencing original discussion that led to this GitHub issue:

@jaswsinc writes in https://github.com/websharks/comment-mail/issues/150#issuecomment-184048340...

Noting that existing custom templates out-in-the-wild will be broken by these changes. Simple templates will remain functional, but Advanced PHP templates that have been customized in previous versions will no longer work after these changes.

@raamdev writes...

Hmm, that seems like a pretty big deal. What sort of failure will occur if someone did customize an Advanced PHP template in a prior release? Is there any way for us to detect if Advanced PHP templates have been modified and notify the site owner that they need to update their template?

@jaswsinc writes...

What sort of failure will occur if someone did customize an Advanced PHP template in a prior release?

They use a different PHP namespace and they call upon methods that were renamed to their camelCase equivalent. So using older PHP templates with the newly modified codebase will result in PHP fatal errors.

Is there any way for us to detect if Advanced PHP templates have been modified and notify the site owner that they need to update their template?

A version-specific upgrade routine might be able to handle this.

@raamdev writes...

Roger that. I think we should definitely work on this to avoid fatal PHP errors if at all possible. I'll open a separate issue to track this.

[raamdev]

@jaswsinc I could use some help here if you have some time over the next few days.

[jaswrks]

Sorry. I'm not seeing an easy way to deal with this issue. The only way for us to provide full back compat for the old advanced PHP templates would be to restore all sorts of old class method names and also put those under the older namespace too. That doesn't seem feasible to me. There were a lot of changes. Even if we wanted to do that, I think it would be very prone to errors.

What we could do instead is to detect a site that has altered the advanced templates in some way, and if that's the case, save that data before we update their options, and then provide a warning/message.

... Hmm, that seems problematic also. I'll give it some thought.

[jaswrks]

Another option would to do an interim release (i.e., using the old codebase/structure) and add a routine to the automatic updater that will check if the site owner has customized the advanced PHP template files. If they have, the pro update message could inform them of the situation before they do the upgrade.

[raamdev]

Another option would to be an interim release (i.e., using the old codebase/structure) and add a routine to the automatic updater that will check if the site owner has customized the advanced PHP template files. If they have, the pro update message could inform them of the situation before they do the upgrade.

That sounds like the best option here.

So would that just be a Dashboard message that tells them the next update will revert their customized Advanced Templates? Or is there something else we can do to make this transition more smooth (i.e., since we'd be releasing a version that is compatible with their current advanced templates, is there anything else we could do)?

[raamdev]

@jaswsinc Ping. ↑

[raamdev]

@jaswsinc Ping. ↑↑

[jaswrks]

So would that just be a Dashboard message that tells them the next update will revert their customized Advanced Templates? Or is there something else we can do to make this transition more smooth (i.e., since we'd be releasing a version that is compatible with their current advanced templates, is there anything else we could do)?

Thanks for the ping. I was thinking about it being a notice that would magically detect if they are using advanced templates a, and then if so, check if they have made any changes to those templates that deviate from the defaults; i.e., if any of these option keys are !empty() they have customized one or more of their advanced templates.

That notice could then be displayed whenever an upgrade of the pro version is about to take place. Instead of doing the upgrade, we redirect and ask them to save any advanced template customizations, and then empty the fields they customized and try again; i.e., emptying the templates they customized should automatically revert those templates back to their default value, which would allow the upgrade to proceed then when they try it again.

[jaswrks]

Referencing these lines of code that already handle the logic behind detecting changes in the template files. https://github.com/websharks/comment-mail-pro/blob/000000-dev/src/includes/classes/Plugin.php#L1084-L1095

That's why I say, if any of these options keys are not empty, they have made changes.

[jaswrks]

Or any of these options keys. Sorry, I left these out before: https://github.com/websharks/comment-mail-pro/blob/000000-dev/src/includes/classes/Plugin.php#L714-L726

[jaswrks]

In other words, anything that has strpos($option_key, 'template__type_a__') === 0

[raamdev]

@jaswsinc I was just thinking about this problem some more (and about how I'm not looking forward to doing an interim release with the old codebase and hoping that everybody upgrades to that to see the Dashboard message before we can do another release with the new codebase) and I had an idea for another way we might be able to handle this:

What if we just detect, as part of the new release, when someone has customized their Advanced Templates, and then simply not load those incompatible templates. Instead, we show a Dashboard notice letting the site owner know that until they update their templates the original default templates will be used.

That seems like a safer plan than to hope that people update to the interim release and see the Dashboard notice. (If they don't update to the interim release and instead wait until the following release with the new codebase, then that update would produce fatal errors.)

[jaswrks]

That sounds great. The only question I have is how will we allow them to see/revise their old templates exactly? I guess that's not impossible, but I'm curious to know what you feel would work best for that part; i.e., when they get a notice about having outdated templates, how do they access those old templates?

[raamdev]

@jaswsinc writes...

how will we allow them to see/revise their old templates exactly

We could backup the contents of the customized Advanced Templates to new option keys in the database. Any templates they modified will then simply revert to their defaults in the plugin, so that the core functionality of the plugin doesn't break. We then show a Dashboard message like this:

You are using customized Advanced Templates that are no longer compatible with his version of Comment Mail. Your customized templates have been backed up and the Advanced Templates have been restored to their defaults to allow Comment Mail to continue working.

To continue using your customized templates, you'll need to reapply your customizations to the new default Advanced Templates. For more information about how to access your old customized templates and update them, please see this article.

To access the contents of the old customized templates, we simply check for the presence of those backed up option keys and have a new link that appears somewhere on each Advanced Template panel that will load the contents of the backed up template (which is stored in the new backed up option key). The KB Article will explain (with a screenshot) how to access the old template and describe how (and why) the Advanced Templates changed in an incompatible way.

[raamdev]

@jaswsinc Any thoughts on the above? ↑

[jaswrks]

Cool idea. Hmm, those AC template files are written in PHP. Maybe it would even be OK to just append the old template to the new one and then comment all of that out; e.g., /* ... */ at the bottom of the new template so they can see and edit both copies and compare the differences.

[raamdev]

Cool idea. Hmm, those AC template files are written in PHP. Maybe it would even be OK to just append the old template to the new one and then comment all of that out; e.g., /* ... */ at the bottom of the new template so they can see and edit both copies and compare the differences.

Hmm, so that would basically look like this?

1. Upgrade Comment Mail Pro
2. VS routine detects customized Advanced Templates
3. VS routine copies customized Advanced Template(s) from the database to the bottom of the default PHP template file as a comment
4. VS routine resets Advanced Templates to their defaults, which now include a copy of the customized template at the bottom in a comment
5. Site owner can then visit the Advanced Templates in the Comment Mail options and update them accordingly, all the while being able to see a copy of their old customized template at the very bottom.

Do I have that right?

I guess my only concern with going this route is that we're backing up their customizations by modifying template files inside the plugin itself, which means a future upgrade will simply overwrite those default templates with a copy that doesn't include their customizations. Or if something goes wrong (e.g., maybe they had some bad code in their template that somehow broke our attempt to append it to the bottom of the default template PHP file as a comment) and they need to replace the plugin files altogether, they'll have no way of recovering the backup of their custom advanced templates (since that backup would've been destroyed by deleting and reinstalling Comment Mail Pro).

That was why backing things up to the database seemed safer... if they somehow delete the Comment Mail Pro database options, well, then all of their Comment Mail configuration would be lost and it would be expected at the Advanced Template customizations would be lost too.

[raamdev]

@jaswsinc Ping. ↑

[jaswrks]

Do I have that right?

Yes. Sounds good.

I guess my only concern with going this route is that we're backing up their customizations by modifying template files inside the plugin itself, which means a future upgrade will simply overwrite those default templates with a copy that doesn't include their customizations

Now that you say that it makes me think of that a bonus with this technique, because if we go that route we are also forcing the updated template file to be different from the default template value; i.e., it will contain /* ... old template ... */ at the bottom. Which means that a future upgrade would require us to preserve that file, because it would contain what would be considered custom code; i.e., something other than the default template file. In short, I'm not seeing a problem with this. If anything, it sounds like it could work to help us avoid problems during future upgrades, because the old template is being added to the bottom of the new one. Not something I would want to do all the time, but in the case of this drastic reorganization that rendered old templates invalid it seems like a plan to me.

Or if something goes wrong (e.g., maybe they had some bad code in their template that somehow broke our attempt to append it to the bottom of the default template PHP file as a comment) and they need to replace the plugin files altogether

Hmm. Well, if they have bad code in an advanced template file the site was already broken before the upgrade, and the upgrade would be more likely to fix the issue than it would be to break something further. So long as we wrap the old template in /* ... */ comment markers then it should be fine.

That was why backing things up to the database seemed safer... if they somehow delete the Comment Mail Pro database options, well, then all of their Comment Mail configuration would be lost and it would be expected at the Advanced Template customizations would be lost too.

I'm not against that. If that seems easier to you then I'm fine with that :-)

[raamdev]

@jaswsinc writes...

Maybe it would even be OK to just append the old template to the new one and then comment all of that out; e.g., /* ... */ at the bottom of the new template so they can see and edit both copies and compare the differences.

The problem with that is you'd have to first strip out all of the multiline comments from the existing [modified by the user] template, and the user may have added their own multiline comments to help remind themselves of their own changes, so we wouldn't want to strip those out when making a backup of their modified template.

Why would you have to strip out multiline comments? Because PHP doesn't support nested multiline comments. See the PHP Manual:

'C' style comments end at the first */ encountered. Make sure you don't nest 'C' style comments. It is easy to make this mistake if you are trying to comment out a large block of code.

<?php
 /*
    echo 'This is a test'; /* This comment will cause a problem */
 */
?>

[jaswrks]

Why would you have to strip out multiline comments? Because PHP doesn't support nested multiline comments. See the PHP Manual:

Ah, good thinking! I totally forgot about that. Maybe we could instead do something like:

<?php
$original_template = '....';
$new_template .= preg_replace('/^/um', '#', $original_template);

[raamdev]

We'd also need to deal with the <?php and ?> tags in the templates...

[raamdev]

Maybe just ignore any lines that start with <?php or ?>?

<?php
# namespace WebSharks\CommentMail\Pro;
# 
# /*
#  * @var Plugin           $plugin              Plugin class.
#  * @var Template         $template            Template class.
# ...
#  * @note In addition to plugin-specific variables & functionality,
#  *    you may also use any WordPress functions that you like.
#  */
?>
<?php // Sets document <title> tag via `%%title%%` replacement code in header.
# echo str_replace('%%title%%', __('Confirmation Request', SLUG_TD), $email_header); ?>

[raamdev]

↑ This is why it seemed easier to me to just copy any template__type_a_* option keys to their own backup option keys (e.g., template__backup_type_a_*), then somehow provide access to the backed up data... even just a link that loaded the template as plain-text in the browser.

[raamdev]

Though I do agree that it will be a lot easier from a support-perspective to be able to say "your customized template was copied to the bottom of the reset template; just go there to see your old version and you can then compare that to the reset version".

So, I'm all for solving the problem of copying the old template to the bottom of the reset template, so long as we can do so in a reliable way that doesn't break things (which is what we're trying to avoid in the first place). Also, it's important to keep in mind that the user may have modified their template in any number of ways—we have no idea what it looks like now.

[jaswrks]

We'd also need to deal with the <?php and ?> tags in the templates...

Yikes. Yep, I see what you mean. Well, here is another idea.

$original_template = preg_replace_callback('/\/\*(.*?)\*\//u', function($m) {
    return preg_replace('/^/um', '# ', $m[1]);
}, $original_template);

$new_template .= "\n\n";
$new_template .= '/*';
$new_template .= $original_template."\n";
$new_template .= '*/';

In other words...

• Convert C-style comments into single-line comments in the original template.
• Then, append the original template and wrap it in /* ... */

[jaswrks]

Hmm... some of the templates contain CSS/JS also, so it's not just PHP that we are dealing with. Yeah, that's getting a bit hairy now. Not sure I still like that idea either.

[raamdev]

some of the templates contain CSS/JS also, so it's not just PHP that we are dealing with.

Hmm, I hadn't thought of that either. Yeah, I'd say we should just go with my original idea of backing up the existing template__type_a_* option keys to new template__backup_type_a_* keys, then deleting the existing `template__type_a_* keys (which will effectively reset the templates to their default).

Then we just need to give the user access to that backed up data.

[jaswrks]

Then we just need to give the user access to that backed up data.

Agree.

Maybe there could be a toggle button in the template editor that would allow for the old version to be appended (like we have been discussing) as best we can. That way they will gain access to both the old and new together in the same file, but only for the purpose of reclaiming the old data whenever they explicitly ask for it; i.e., when editing templates.

So we could try working from this idea and improve it. If we can get that somewhere close to 99% accurate, then it could be used in that toggle functionality, but not as a default VS upgrade.

[raamdev]

I suppose to the best way to present the backed up data from the user's perspective would be to have a link/button that expands a second code box underneath the one that contains the (now reset) template. That way they can easily compare the two, with syntax highlighting.

Then another button/link that allows them to delete the backed up version, getting rid of that backed up option key.

[jaswrks]

Yeah, that would be ideal. The downside is that it opens the door for new bugs in the UI, it will take more time to implement it, and it's all for the purpose of providing back compat. to only just a few people.

[jaswrks]

If we do append the old template to the new in some sort of toggled mode that allows a site owner to reclaim the old data, we could also append a marker just before the old data. That way the old data can be excluded entirely whenever they save any changes from that page where templates are being edited.

<?php
blah blah blah in the new template...
/* --- Original Template --- */
/*
Old template code.
*/

Whenever the templates are saved, we strip the marker and everything after it before saving.

[raamdev]

to only just a few people.

Hmm, I hadn't thought of that. I forgot this was Pro-only, which means right now at most we're affecting ~40 people with this, and even then not all 40 of them are going to have necessarily customized the Advanced templates...

[raamdev]

Whenever the templates are saved, we strip the marker and everything after it before saving.

I really like that idea. That way the old template is never really being mixed with the live template.

[jaswrks]

Maybe that's a better idea then. Instead of commenting out the old template, we just append it and add a marker so that the old templates do contain a copy of the old, but whenever the template is read and used by Comment Mail, we don't actually use anything after the marker and of course the marker itself would also be excluded too.

If we went that route, there wouldn't need to be a toggle mode or anything, the VS upgrade routine would simply append the old to the new (if it had been customized), along with a marker. The template system can then be updated to look for that marker and automatically exclude it whenever the template is read and parsed. See: https://github.com/websharks/comment-mail-pro/blob/000000-dev/src/includes/classes/Template.php#L325

[raamdev]

we strip the marker and everything after it before saving.

That would also mean that we don't even need to worry about commenting out the old code... all we need is a marker that indicates what comes next is just a copy of their old modified template.

/* --------------------------- Original Template ---------------------------
 * Comment Mail v16xxxx included changes that were not
 * backwards compatible with your customized Advanced Template.
 * To prevent problems with the upgrade to v16xxxx, we reset
 * the Advanced Templates to their (new) default and backed up
 * your customized template, a copy of which is included below.
 * You can reference your original template below to reapply
 * those changes to the new default template above.
 * When you are ready to discard this backup, simply delete
 * this comment, and everything below it, and save the template.
 * If you leave this comment here, your backup will be retained.
 */

<?php
namespace WebSharks\CommentMail\Pro;

/*
 * @var Plugin           $plugin      Plugin class.
 * @var Template         $template    Template class.
 *
 * Other variables made available in this template file:
 *
 * @var \stdClass        $sub         Subscription object

[jaswrks]

Great minds think alike :-) 😸

[jaswrks]

Actually, it would need to be implement here instead. https://github.com/websharks/comment-mail-pro/blob/000000-dev/src/includes/classes/Template.php#L337

[raamdev]

If we went that route, there wouldn't need to be a toggle mode or anything, the VS upgrade routine would simply append the old to the new (if it had been customized), along with a marker. The template system can then be updated to look for that marker and automatically exclude it whenever the template is read and parsed. See: https://github.com/websharks/comment-mail-pro/blob/000000-dev/src/includes/classes/Template.php#L325

That would work, but that does seem more complicated than simply backing up the option keys, injecting the backed up content into the textarea and then stripping it out whenever the templates are saved.

Any reason you like your idea better?

[jaswrks]

I like both of those ideas. However, yes, I do like my idea slightly better. The reason is that I see it taking fewer modifications to the code to bring it all together. For instance, no changes in the menu page class file would be needed so long as we don't introduce any new option keys that contain backup data.

Here's a quick look at how I'd try to implement my idea:

• Create a VS upgrade routine that automatically detects old templates that were customized and append those old templates to the new templates whenever the upgrade occurs. No new option keys, just use the existing template option keys and append the old to the new w/ marker.
• Then, update the getFileContents() method in the Template class to look for and strip the marker that separates the old from the new. https://github.com/websharks/comment-mail-pro/blob/000000-dev/src/includes/classes/Template.php#L337

---

With your idea, it provides a better separation, but then we would need to add some more steps.

• Alter the way template option keys are read whenever the data is being manipulated from the menu page class file; i.e., whenever a site owner is editing the template files we need to include data from the old templates via backup option keys. Not a huge deal, but there would need to be some logic integrated into the UI that would be capable of reading that additional data when it exists.

• Whenever template files are being saved, we need to strip the marker that separates the old from the new and put the old template file contents back into the backup option keys where it belongs, instead of being saved together with the new possibly-modified (or not) template file.

  That's assuming that the old template file would be edited in the same textarea as the new; i.e., appended at runtime for editing. If there is a separate field instead of appending to the existing textarea, then it would be easier, but there again, that calls for edits to the menu page class file i.e., a new UI component that displays the old version of the template under the new one.

[raamdev]

Example screenshot of Dashboard message that appears when Comment Mail Pro detects incompatible Advanced Templates. The notice will include a list of the specific templates it has reset so that you can review each one. Your backed up customizations will be appended to the bottom of each template.

[raamdev]

Next Release Changelog:

• Comment Mail Pro Upgrade Notice: Incompatible Advanced Templates. This version of Comment Mail includes a rewritten and improved codebase. This rewrite, however, came with the unfortunate side effect of breaking backwards compatibility with Advanced Templates that were customized in a previous version of Comment Mail Pro.

  If you are currently using Comment Mail Pro and you've customized your Advanced Templates, all of your customized Advanced Templates will be backed up and the templates will then be reset to their new defaults. You will find the backup of your old customized template appended to the bottom of the new template, separated with a Legacy Template Backup PHP comment. See example screenshots.

  Note: This change has no effect on Simple templates—only Advanced Templates are affected. Advanced Templates are a Pro-only feature, so this notice only applies to Comment Mail Pro. See Issue #238.

[raamdev]

Comment Mail v160618 has been released and includes changes from this GitHub Issue. See the v160618 announcement for further details.

---

This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#238).