dev/financial#171 - Trigger an error if non-numeric value passed to Money::format()

[demeritcowboy]

Overview

https://lab.civicrm.org/dev/financial/-/issues/171#note_56673

To trigger this you can do something like put {'aaa'|crmMoney} in any template and then visit that page. You should see an error if you have on-screen errors turned on (drupal).

Before

Confusing warning about missing INTL when non-numeric values are passed to Money::format()

After

User deprecated function: Formatting non-numeric values is no longer supported: <the offending value>

Technical Details

It was being intercepted before because it made tests fail on php 7.4 prematurely. No longer a need to do this so this will also see if any more tests fail now.

Comments

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

[demeritcowboy]

api_v3_ReportTemplateTest.testDeferredRevenueReport
CRM_Core_Page_HookTest.testFormsCallBuildFormOnce
CRM_Core_Smarty_plugins_CrmMoneyTest.testMoney with data set #2
CRM_Core_FormTest.testOpeningForms with data set "Fulltext search"
CRM_Core_InvokeTest.testInvokeDashboardForNonAdmin
CRM_Core_PseudoConstantTest.testOptionValues
CRM_Core_PseudoConstantTest.testContactTypes
CRM_Core_PseudoConstantTest.testGetTaxRates
CRM_Custom_Form_OptionTest.testEditCustomFieldOptionValue
CRM_Export_BAO_ExportTest.testAdditionalReturnProperties with data set #2
CRM_Export_BAO_ExportTest.testDefaultReturnProperties with data set #2
CRM_Export_BAO_ExportTest.testGetSQLColumnsAndHeaders with data set #2
CRM_Member_BAO_QueryTest.testMembershipDateWhereSearchBuilder with data set #0
CRM_Member_BAO_QueryTest.testMembershipDateWhereSearchBuilder with data set #1
CRM_Member_BAO_QueryTest.testMembershipDateWhereSearchBuilder with data set #2
CRM_Member_BAO_QueryTest.testConvertEntityFieldSingleValue
CRM_Member_BAO_QueryTest.testConvertEntityFieldMultipleValueEntityRef
CRM_Member_BAO_QueryTest.testConvertEntityFieldMultipleValueLegacy
CRM_Member_BAO_QueryTest.testConvertEntityFieldMultipleValueEntityRefDoubleRun
CRM_Report_Utils_ReportTest.testOutputPrint
CRM_Upgrade_Incremental_php_FiveTwentyTest.testChangeCaseTypeAutoassignee
CRM_Upgrade_Incremental_php_FiveTwentyTest.testConvertRoleLabelsToNames

[eileenmcnaughton]

@demeritcowboy more than I expected!

[demeritcowboy]

I was looking at some. I think several might be bad smarty that somehow ends up calling crmMoney indirectly but the error gets swallowed up somewhere so it's never come up before.

[eileenmcnaughton]

@demeritcowboy I took a look at that first one & it's pretty dodgey - what gets passed in is

'100, 100'

I think this is OK #19940

(I hope we don't have to go through every single one & several are failing on the same thing....)

[eileenmcnaughton]

Also #19941

[eileenmcnaughton]

test this please

[eileenmcnaughton]

I just gave this a very optimistic MOP

[eileenmcnaughton]

They didn't just melt away

Test Result (20 failures / +20)
CRM_Core_Smarty_plugins_CrmMoneyTest.testMoney with data set #2
CRM_Core_FormTest.testOpeningForms with data set "Fulltext search"
CRM_Core_InvokeTest.testInvokeDashboardForNonAdmin
CRM_Core_PseudoConstantTest.testOptionValues
CRM_Core_PseudoConstantTest.testContactTypes
CRM_Core_PseudoConstantTest.testGetTaxRates
CRM_Custom_Form_OptionTest.testEditCustomFieldOptionValue
CRM_Export_BAO_ExportTest.testAdditionalReturnProperties with data set #2
CRM_Export_BAO_ExportTest.testDefaultReturnProperties with data set #2
CRM_Export_BAO_ExportTest.testGetSQLColumnsAndHeaders with data set #2
CRM_Member_BAO_QueryTest.testMembershipDateWhereSearchBuilder with data set #0
CRM_Member_BAO_QueryTest.testMembershipDateWhereSearchBuilder with data set #1
CRM_Member_BAO_QueryTest.testMembershipDateWhereSearchBuilder with data set #2
CRM_Member_BAO_QueryTest.testConvertEntityFieldSingleValue
CRM_Member_BAO_QueryTest.testConvertEntityFieldMultipleValueEntityRef
CRM_Member_BAO_QueryTest.testConvertEntityFieldMultipleValueLegacy
CRM_Member_BAO_QueryTest.testConvertEntityFieldMultipleValueEntityRefDoubleRun
CRM_Report_Utils_ReportTest.testOutputPrint
CRM_Upgrade_Incremental_php_FiveTwentyTest.testChangeCaseTypeAutoassignee
CRM_Upgrade_Incremental_php_FiveTwentyTest.testConvertRoleLabelsToNames

[demeritcowboy]

The ones that trigger civicase errors are technically correct I think, I just have no idea why this PR causes that. In secure mode, smarty should reject loading a tpl that isn't under templates or an authorized directory. The fact that the case sample tpl lives under CRM is a weird oddity, cuz civicase. I'm really curious though how this PR triggers those errors.

[demeritcowboy]

Oh I think I know. Smarty is a static singleton. Calling fetch('string:...') sets secure mode. When it fails, secure mode never gets reset, so the other tests start running under secure mode.

[eileenmcnaughton]

ohhhh - go super-sleuth

[monishdeb]

Test failure seems related https://test.civicrm.org/job/CiviCRM-Core-PR/40101/

[demeritcowboy]

#19958

[demeritcowboy]

jenkins retest this please

[seamuslee001]

Well done all here

[eileenmcnaughton]

Oh wow - it passed!

[demeritcowboy]

Yay!