Switch pledge_acknowledgement to parse through the processor only

[eileenmcnaughton]

Overview

Switch pledge_acknowledgement to parse through the processor only

This is a bit more than the other changes as the code was parsing a contact through hooks 'as if' for tokens but then assigns it to the template. The $contact part of this was last used in the shipped templates in 2019 which explains why it was there. The $domain part is still used

This PR actually STOPS assigning $contact to the template. I think our options are to either keep assigning it forever in case
anyone uses it or to remove the assign in this 5.43 release when we are already communicating heavily about template & token changes.

This is a less used template

Before

legacy function used to render a contact to the template, creating maintenance issues & confusion

After

Poof

Technical Details

I think doing this will add clarity - and I can add to our upgrade notes - but in addition I could add a pre-upgrade check to message people if their template has $contact in it

Comments

Includes #21788 because of the test function it adds

[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).

[totten]

The $contact part of this was last used in the shipped templates in 2019 which explains why it was there. The $domain part is still used

Looks like it was specifically 5.15.

I guess there's no empirical way to assess how effectively deployments made the transition (back in 2019).

My only concern about removing the Smarty $contact is that (AFAICS) we never gave any real warning or heads-up. For any site which edited pledge_acknowledgement before 5.15 -- wouldn't they get silently blank content now?

Here's way to log a clear warning bbcd76b (depends on GhostArray and ErrorTestTrait via this branch).

$form->assign('contact', new CRM_Utils_GhostArray(function($op, $field) {
  $badExpr = '{$contact.' . $field . '}';
  CRM_Core_Error::deprecatedWarning("The automated message (\"pledge_acknowledgement\") attempted to use $badExpr. This is no longer supported. Please convert to a {contact.*} token.");
}));

And then capture the warning during the unit-test:

[$log] = $this->captureErrors(E_USER_DEPRECATED, function() use ($loggedInUser) {
  $this->submitExamplePledge($loggedInUser);
);
$this->assertNotEmpty(preg_grep('/\$contact\.first_name.*is no longer supported/', $log));

[eileenmcnaughton]

This is moved to master - note that our tools for warning are generally the upgrade script, the release notes & status checks. In this case the upgrade script could be in 'pre' & warn before upgrading. However, I note that we have a couple of similar templates & it would probably be better to remove the $contact variable from them at the same time

[eileenmcnaughton]

@totten so revisiting this - we actually have a process where we update templates on upgrade & give notification to users that have customised their templates that they need to update them. Users of this template would have had that back in 5.15.

More recently we've also been using upgrade scripts to do this with less manual intervention required.

However, we have around 5 templates left that are all lower usage and less likely to be customised. I think it makes sense just to use the old way with perhaps the addition of some status check backup.

The main change this thinking makes is that it makes it more worth updating the templates so that they no longer refer to $domain or $contact at all and making this change across the 2 pledge templates & 3 participant templates in 5.44

[eileenmcnaughton]

the original still works but since we are changing we should update

[eileenmcnaughton]

resolved in another pr