Use tokens in offline contribution receipt (new installs) #22560
Conversation
This gets us away from the empty checks & the isset that causes a crash with smarty modifiers. There is no change at the moment as to what is assigned so we don't need to make any changes to existing sites. The goal is to specify the mappings on the workflow template class later. That will allow us to remove significant amounts of code (& leakage points) from the form layer. However we still need to resolve civicrm#22547 because at the moment values in this template expect raw values - which is not the case with mapped params at the moment. Note the biggest thing to contemplate in this PR is the addition of an -always-there-in-contribution-smarty-templates smarty value isShowTax. The way it is currently checking is that there should pretty much be 'something' assigned if invoicing is enabled & it makes sense that tax would show even if zero if enabled - which is what I *think* is already happening (with maybe some random variation). I added 'taxTerm' as a domain token and I do kinda prefer that but it is available more places (unless we change the audience). invoice_notes is another contribution setting that effects display.
|
(Standard links)
|
eileenmcnaughton
changed the title
|
@colemanw @seamuslee001 @demeritcowboy is someone able to look at this? It removes one blocker to getting tests to run with the secure smarty mode (as we still have 'isset' in a few templates - including this one) |
|
jenkins retest this please (double-checking) |
| * | ||
| * @param array $export | ||
| */ | ||
| protected function exportExtraTplParams(array &$export): void { |
There was a problem hiding this comment.
It's not blocking but I'm wondering if there's some way to phpdoc these to avoid somebody going "I grepped and couldn't find anywhere it's used so let's remove it". You have to hunt to find that there's reflection being used to find functions starting with exportExtra.
| @@ -131,13 +131,13 @@ | |||
| {/foreach} | |||
| {/if} | |||
|
|
|||
| {if isset($totalTaxAmount) && $totalTaxAmount !== 'null'} | |||
| {if $isShowTax} | |||
There was a problem hiding this comment.
It is a little weird that it now always shows $0 if there's no tax, but I don't think it's a big deal.
Financial Type: Donation
Total Tax Amount : $0.00
Total Amount : $123.45
Date Received: January 21st, 2022 4:40 PM
Receipt Date: January 21st, 2022 4:41 PM
Paid By: Check
There was a problem hiding this comment.
Oh wait it did before too if you have the tax setting turned on. But now it's adding a time component to the dates whereas it didn't before. I don't personally have a strong opinion on that.
Overview
Use tokens in offline receipt for new installs (no upgrade being pushed at this stage as likely more tweaks will be done)
Before
Default contribution receipt makes heavy use of smarty variables that have 'evolved;
After
Where possible tokens are used - tokens are more consistently available so less (problematic) empty & isset checks are needed, are more predictable and will in time allow us to remove a lot of code.
Technical Details
This gets us away from the empty checks & the isset that causes a crash with smarty modifiers.
There is no change at the moment as to what is assigned so we don't need to make any changes to existing sites.
The goal is to specify the mappings on the workflow template class later. That will allow
us to remove significant amounts of code (& leakage points) from the form layer.
However we still need to resolve #22547
because at the moment values in this template expect raw values - which is
not the case with mapped params at the moment.
Note the biggest thing to contemplate in this PR is the addition of an
-always-there-in-contribution-smarty-templates smarty value isShowTax.
The way it is currently checking is that there should pretty much
be 'something' assigned if invoicing is enabled & it makes sense
that tax would show even if zero if enabled - which is what I think
is already happening (with maybe some random variation).
I added 'taxTerm' as a domain token and I do kinda prefer that but
it is available more places (unless we change the audience).
invoice_notes is another contribution setting that effects display.
Comments