Fixed dev/core#2505 fix formatting of numbers in civireport

[jaapjansma]

Overview

Localize the display of custom fields of type number.

Before

When a report displayed a custom field of type number it wont localize the decimal separator.

After

Display of custom field of type number correctly shows the decimal separator. This is on the report screen as on other screens in CiviCRM.

Technical Details

• The function in CRM_Utils_Number::formatLocaleNumeric is added (which is almost a copy from CRM_Utils_Money::formatLocaleNumeric but without the precision ($numberOfPlaces) paramater.
• In the function CRM_Core_BAO_CustomField::formatDisplayValue the formatting of number fields is added.
• Also added a couple of test for checking the localized numbers.

Comments

See https://lab.civicrm.org/dev/core/-/issues/2505

[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]

I think you would need to do something like call getDecimalPlacesForAmount() and pass that in as numberOfPlaces, or since formatLocaleNumeric is the function that wasn't used anywhere, change the default of 2 in formatLocaleNumeric to null and then have it call getDecimalPlacesForAmount when numberOfPlaces is null (similar to how formatLocaleNumericRoundedByOptionalPrecision does it).

Something like that.

[jaapjansma]

I have updated the failing tests and added tests for checking with localized settings.

[eileenmcnaughton]

jenkins issue is code formatting

https://test.civicrm.org/job/CiviCRM-Core-PR/40501/checkstyleResult/new/

[jaapjansma]

I have squashed all commits into one.

[demeritcowboy]

• General standards
  • [r-explain] PASS
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
• Developer standards
  • [r-tech] PASS
  • [r-code] PASS
  • [r-maint] PASS
    • A nice-to-have would be a second variation of the test that changes the system locale but not a blocker.
  • [r-test] PASS

@eileenmcnaughton you had some initial concerns in the ticket - is this ok here?

[eileenmcnaughton]

@jaapjansma @demeritcowboy my concern is that ig we pass the locale to the formatter we shouldn't be specifiying the separaotrs - that should be determined from the locale - but did you try without?

[jaapjansma]

Yes i try that out and the test also does not pass the locale.

[demeritcowboy]

@eileenmcnaughton Yes without the symbol lines it uses the locale symbols, but then it would ignore your separators if you wanted something different than the locale, which seems like a possibility in some regions. But I'll leave that discussion to people who are in locales where it's mixed.

[jaapjansma]

And again I don´t know where as an site administrator I should set the locale. I do know where to set the decimal and thousand separators.

[eileenmcnaughton]

OK - well this is well tested and I think it will avoid the double convert pitfall so I'm OK going with it

[demeritcowboy]

@jaapjansma locale for these purposes gets set from "Default language" on the localization settings, which also seems a little bit wrong, so that's another reason it might not match the desired separators.