Stop passing html to crmMoney #19941
Conversation
|
(Standard links)
|
| @@ -96,7 +96,7 @@ | |||
| {else} | |||
| <div class="display-block"> | |||
| <td class="label">{$form.total_amount.label}</td> | |||
| <td><span>{$form.total_amount.html|crmMoney} {if $taxAmount}{ts 1=$taxTerm 2=$taxAmount|crmMoney}(includes %1 of %2){/ts}{/if}</span></td> | |||
| <td><span>{$form.total_amount.html} {if $taxAmount}{ts 1=$taxTerm 2=$taxAmount|crmMoney}(includes %1 of %2){/ts}{/if}</span></td> | |||
There was a problem hiding this comment.
It took me a bit to figure out how to even get here. Make a contribution with pending status and then visit any contribution page and add ccid=44 to the url, where 44 is the contribution id you just made. The difference after the patch is the dollar sign is missing. Otherwise it still seems ok even with a different locale.
| <span class="description">{ts}The market value of this premium (e.g. retail price). For tax-deductible contributions, this amount will be used to set the non-deductible amount in the contribution record and receipt.{/ts}</span> | ||
| </td> | ||
| </tr> | ||
| <tr class="crm-contribution-form-block-cost"> | ||
| <td class="label">{$form.cost.label}</td> | ||
| <td class="html-adjust">{$form.cost.html|crmMoney}<br /> | ||
| <td class="html-adjust">{$form.cost.html}<br /> |
There was a problem hiding this comment.
Also the dollar sign goes missing but fixes the warning. There's also something else weird about this form both before and after the patch. In another locale it converts the separators to en_US when you go back in to edit, or maybe more accurately it doesn't convert them to the locale.
| @@ -68,7 +68,7 @@ | |||
| <td class="label">{$form.financial_type_id.label}<span class="crm-marker"> *</span></td> | |||
| <td>{$form.financial_type_id.html}<br /><span class="description">{ts}Select the appropriate financial type for this payment.{/ts}</span></td> | |||
| </tr> | |||
| <tr class="crm-event-eventfees-form-block-total_amount"><td class="label">{$form.total_amount.label}</td><td>{$form.total_amount.html|crmMoney:$currency}</td></tr> | |||
| <tr class="crm-event-eventfees-form-block-total_amount"><td class="label">{$form.total_amount.label}</td><td>{$form.total_amount.html}</td></tr> | |||
There was a problem hiding this comment.
This is the warning I saw yesterday (two days ago). Again just no dollar sign.
In another locale the javascript when you choose the price formats it with en_US separators but it accepts it either way. And if you have mixed currencies the whole form gets confused.
templates/CRM/Pledge/Form/Pledge.tpl
Outdated
| {elseif $action eq 2 and !$isPending} | ||
| {$eachPaymentAmount|crmMoney:$currency} | ||
| {$eachPaymentAmount} |
There was a problem hiding this comment.
The first one is ok, just the same dollar sign thing.
This second one that isn't a form element, it needs the crmMoney otherwise it's both missing the dollar sign and in another locale it doesn't reformat the separators properly.
|
@demeritcowboy what should we do - just merge as is & worry about the others or try to work throught them all making sure the $ is added at the qf level. They all see a bit obscure |
|
The very last one shouldn't be merged. The other ones I don't see a choice at the moment so yes. Where to format can be a separate discussion but I don't think currency formatting before sending to the forrm is right. It should be at the display level. |
eileenmcnaughton
force-pushed
the
html_to_money
branch
from
7002421 to
29a42d3
Compare
|
@demeritcowboy OK - I've fixed that so this should be good to merge now |
|
The last one line 56 still needs the currency. It's where it says "12 installments of $12.34 every month" when the pledge has been completely paid. If the pledge is in a non-default currency then without the $currency it gets the currency symbol wrong. |
eileenmcnaughton
force-pushed
the
html_to_money
branch
from
29a42d3 to
c6ee9b3
Compare
|
Looks good! |
|
@seamuslee001 if you're in a mergey mood this one looks good now. |
|
I'm going to merge this based on @demeritcowboy so we can unblock resolving these before the next rc is cut |
Overview
crmMoney should not receive html for format - this fixes
Before
See #19938
After
@demeritcowboy can't make the tests fail
Technical Details
@demeritcowboy I haven't checked how these impact the forms yet - in theory we should be handling in the QF layer
Comments