MessageTemplate - Add renderTemplate(). Deprecate renderMessageTemplate().

[totten]

Overview

This deprecates one recent/internal helper, CRM_Core_BAO_MessageTemplate::renderMessageTemplate(). As alternatives, use CRM_Core_BAO_MessageTemplate::renderTemplate() or CRM_Core_TokenSmarty::render().

Before

// Internal APIs
CRM_Core_BAO_MessageTemplate::sendTemplate(array $params): array;
CRM_Core_BAO_MessageTemplate::renderMessageTemplate(array $mailContent, bool $disableSmarty, $contactID,
                                                    array $smartyAssigns, array $tokenContext = []): array;

After

// Internal APIs
CRM_Core_BAO_MessageTemplate::sendTemplate(array $params): array;
CRM_Core_BAO_MessageTemplate::renderTemplate(array $params): array;
CRM_Core_TokenSmarty::render(array $messages, array $tokenContext, array $smartyContext): array;

Use CRM_Core_TokenSmarty::render() if you want a low-level utility to render a template with the hybrid Token-Smarty template format. This function was recently extracted (#20870), so it is functionally the same. (The remaining bits in renderMessageTemplate merely juggle the names for smarty/disableSmarty and contactID/contactId).
would be appropriate if you need to use the same notation (Token-Smarty) but with a different process (template-loading/validation/hooks).

Use CRM_Core_BAO_MessageTemplate::renderTemplate() if you want a high-level trial-run of sendTemplate(). This is a new function with broader scope, and it is better suited to generating preview-GUI. The renderTemplate($params) and sendTemplate($params) use the same list of options+behaviors for template-loading, data-loading, data-validation, and hooks (Hook::alterMailContent, Hook::alterMailParams), etc. (This function is a pre-req for #20975.)

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@totten we should remove not deprecate that method - it is a recent method with explicit code comments saying not to call from outside of core

[totten]

@eileenmcnaughton I don't mind posting the code change - but removal requires more work wrt r-run/QA. So I'll send that as a separate PR. (This part - adding renderTemplate() and deprecating renderMessageTemplate() - is safer/easier to QA.)

[eileenmcnaughton]

@totten OK - but note the only other place it's called from has solid enough test cover that I don't think r-run is actually required

[eileenmcnaughton]

I hate this pattern of calling 'send' within 'render'

[eileenmcnaughton]

I guess what I would imagine is a shared function that this calls and send calls with the shared parts in it - having said that - this is hardly a new creation in this PR so not blocking

[totten]

Agree that's a code-smell. Really, it's about the first half of sendTemplate() which is shared (excl a few comments+defns). It might also make sense to flip the relationship (s.t. sendTemplate() delegates to renderTemplate()). For the moment, it is easier to juggle pending patches if sendTemplate()'s structure remains about the same. But we have (and will have) decent test coverage here, and I'd agree with rejiggering it in the not-distant-future.

[eileenmcnaughton]

The approach seems fine - I'm confident that the test cover on all code that calls the deprecated function is exhaustive so I haven't r-run