False positive message about missing INTL PHP extension on membership type form

Happens on dmaster.demo too so it's not windows or anything weird.

1. Click add membership type.
2. Click cancel. It happens if you fill it out too but this is quicker and is enough to see it.

There might be a couple things here.

One is that the message is tied partly to whether the value is numeric, which doesn't really have anything to do with the missing php extension.

The second is that for whatever reason the $amount value is being passed in as <input size="6" maxlength="14" name="minimum_fee" type="text" id="minimum_fee" class="six crm-form-text" />. I'm not sure yet what the mismatch is there.

It's kind of a regression because of the numeric part, but the form value might be old.

added regression label

So the form problem is this line:

https://github.com/civicrm/civicrm-core/blob/77c3b7089c290880a7599cc77cdbe87fa6958c13/templates/CRM/Core/Form/Field.tpl#L20

{if $fieldSpec.formatter === 'crmMoney'}{$form.$fieldName.html|crmMoney:$fieldSpec.formatterParam}

Does it ever make sense to pass .html to crmMoney?

@DaveD no it doesn't - our code has been unreasonably forgiving in the past

Ok. If I take it out then it still seems to have the correct decimal separator if you're editing an existing one, so that's good. The currency symbol doesn't get displayed, which is ok on this form since you can't choose it anyway here, but the Field.tpl subform is generic.

So there's a possibility that there are some places expecting that these will happen automatically:

• That the template will display the currency symbol.
• That the template will do the replacing of the decimal separators for the form field.

I'm also seeing a variation of the false positive when you have a custom field of type money with a dropdown list and the labels include a currency symbol. If there's an existing value then when you edit it gives you the INTL message. This must be coming from somewhere else since it happens even with the Field.tpl crmMoney removed.

@DaveD I think thats ok because it will flush them out & we can work through the places where we are doing odd things - I think without the check in the format function we get a few test fails that flush some out

Ok. For the variation, it's coming from https://github.com/civicrm/civicrm-core/blob/1a813170fa7f3b06df46a6753bb0852e72bf39a0/templates/CRM/Contact/Page/View/CustomDataFieldView.tpl#L35. I think there it needs to take html_type into account and not try to reformat it if it's an option value label.

First part: https://github.com/civicrm/civicrm-core/pull/19876

mentioned in commit core@8ada9143

Second part: https://github.com/civicrm/civicrm-core/pull/19880

I don't think html_type actually matters here, because it's formatting it wrong even for text.

mentioned in commit core@cd3523ad

I got another false positive but it's not a crmMoney template issue. When you add a price field, and you're setting up a radio choice, then when you choose the membership type some ajax happens to fill out the line. That calls CRM_Member_Page_AJAX::getMemberTypeDefaults which looks up the minimum fee and passes it to CRM_Utils_Money::formatLocaleNumericRoundedForDefaultCurrency. If you have no minimum fee it's null and this triggers the missing INTL message. The default for that field is DEFAULT 0.000000000 but it allows null which is what it seems to put when you have no minimum fee. In addition this gives TypeError: Return value of CRM_Utils_Money::formatUSLocaleNumericRounded() must be of the type string, null returned in CRM_Utils_Money::formatUSLocaleNumericRounded(), but it's hidden because ajax.

Hmm - we might want to do some casting

changed title from False positive message about missing INTL extension on membership type form to False positive message about missing INTL PHP extension on membership type form

@DaveD if it's not the INTL PHP extension that this issue is about, then please correct me

Correct. The thing in php.ini where you set extension=intl. This thing: https://www.php.net/manual/en/book.intl.php.

Although this issue is mostly turned out to be about places where non-numeric values are being passed to money formatting. The popup message is a new symptom, only in master/5.37 at the moment, although the problems may date back further.

added sig:bug triaged labels

PR for the price field part: https://github.com/civicrm/civicrm-core/pull/19929

@DaveD we added the second part of this if due to test failures

    if (!extension_loaded('intl') || !is_numeric($amount)) {

If we remove it & throw that at the test suite we might flush out any you haven't found yet?

mentioned in commit core@5bf266bd

Oh ok I get it now why it's there. Thoughts:

There should still be some kind of message logged somewhere if non-numeric because most of what's going to come up is form-related so test coverage is unlikely to pick it up.

Maybe this a job for trigger_error(), which will disappear as far as most users see, but will come up in tests or for anyone who is set up to make it crash for those.

I'll take a look at making a PR.