[REF] Complete decommissioning of CRM/Contribute/Form/Task/PDFLetterCommon.php

[eileenmcnaughton]

Overview

[REF] Complete decommissioning of CRM/Contribute/Form/Task/PDFLetterCommon.php - a class which caused the functionality of CRM/Contribute/Form/Task/PDFLetter to be 'spread' across 2 classes

This removes the last function fromCRM/Contribute/Form/Task/PDFLetterCommon.php, allowing the file to be deleted.

Before

Resolve tokens on CRM/Contribute/Form/Task/PDFLetterCommon.php

After

Resolve tokens on CRM/Contribute/Form/Task/PDFLetter.php , CRM/Contribute/Form/Task/PDFLetterCommon.php deleted

Technical Details

The function was calling a protected function on the parent which had to be unravelled.

I used the method we used in

civicrm-core/CRM/SMS/Form/Upload.php

Line 341 in 5e67eb7

$str = CRM_Core_BAO_MessageTemplate::renderMessageTemplate(['text' => $str, 'html' => '', 'subject' => ''], TRUE, CRM_Core_Session::getLoggedInContactID(), [])['text'];

to replace the code with the part that we have tested as being done in via render - ie replacing
hook, contact, domain tokens & smarty parsing.

However, there are quite a few tests on this code & one checked that we don't over-call
the token hook. I added some caching for the results of this hook so we can start to
eliminate these but I also allowed the calls to increment by 1 because it is not 1 per row

I made the results of this hook optional to replaceHookTokens & in general I think that
is the only place it needs to be called - perhaps once it used to pass more parameters &
the results were dynamic but now they really aren't

Comments

@seamuslee001 this change is pretty similar to the other you merged, test cover is good

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

[seamuslee001]

This looks fine to me and the test coverage here should be sufficient