Fix PHP8 tax_rate warning on Participant

[larssandergreen]

Overview

tax_amount could be zero while tax_rate was not set, resulting in warnings after form submission. This happens if you enable tax and invoicing, but don't actually set tax accounts and financial type. I guess this would happen if you wanted invoicing but not tax.

We can't add to $dataArray if $value['tax_rate'] is not set, so we can skip all that code. We do want to add to $dataArray if the tax rate is 0, but we do not want to add anything if the tax rate is not set.

I also simplified the code a little and added a comment explaining what the strange $dataArray actually does in the template, because it is not intuitive at all.

[civibot]

(Standard links)

• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[mattwire]

@larssandergreen Just wondering how this would display when you have lineitems with different amounts of tax and some that are 0%. In that case you probably want to show "0%" as a tax amount for that line item.

[larssandergreen]

@mattwire Good question to look at here. This is actually not for the tax rate and amount for line items, it's for the tax totals here (below the line items, there is a subtotal, then a breakdown of tax rates and amounts for that subtotal (or parts of that subtotal that might have different tax rates). It's unclear to me why we'd want to show something like:

Subtotal $100
Tax 5.00% $2.50
Tax 0.00% $0

Do you see any reason we would want to show that 0.00% $0 part? Makes sense beside the line items, but for the totals, I don't think we need it.

There is also:
No Tax $0
but I don't think we can ever reach that because $priceset || $priceset == 0 is always true.

So I think there is some simplification that can happen here.

[larssandergreen]

I have pushed a new commit and updated the description up top.

I'll take the template updates to a new PR.

[eileenmcnaughton]

@larssandergreen FYI I've been working on removing dataArray from the templates - it looks like this one still has it although it is now gone from the equivalent online receipt - replaced with

          {foreach from=$taxRateBreakdown item=taxDetail key=taxRate}
              <tr>
                <td {$labelStyle}>{if $taxRate == 0}{ts}No{/ts} {$taxTerm}{else}{$taxTerm} {$taxDetail.percentage}%{/if}</td>
                <td {$valueStyle}>{$taxDetail.amount|crmMoney:'{contribution.currency}'}</td>
              </tr>
            {/foreach}

[larssandergreen]

@eileenmcnaughton OK, that sounds great, it's terrible. But it seems like we might as well merge this for now as we aren't going to be able to get rid of it for a while, presumably, unless we want to break customized templates.

[eileenmcnaughton]

@larssandergreen ok this does seem good to merge

FYI we have 2 upgrade mechanisms for templates

• regular upgrades - each upgrade updates all the reserved templates & any un-customised default templates
• push upgrades - anyone with customised templates gets a message to tell them to upgrade their templates (or they may stop working)

We try to not do the push upgrades too often - there are quite a few changes we haven't pushed as yet. I was thinking that maybe when we were fully rid of $dataArray we might, at least for people with invoicing enabled & then we could start removing it.