Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix internal formatting of monetary values when handling non-decimal currencies #20277

Merged
merged 1 commit into from
May 12, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented May 12, 2021

Overview

As @mattwire pointed out on Mattermost, there is a regression in currency formatting. It arises when processing payments (a) with certain payment processors (e.g. Payflow Pro and iATS) and (b) with currencies that do not use decimal (".") notation.

The problem relates to #20074 (esp 78b338e#diff-3d2a990f0b1960f0b3e6671c645e18033883e6d1eaf535b8d19d085b67dde00bR1171).

Before

Currency separators incorrectly applied

After

The only reason to format money at this stage is to ensure there are 2 decimal places - we should be sticking with machine (US) formatting

Technical Details

Comments

@mattwire @seamuslee001

@civibot
Copy link

civibot bot commented May 12, 2021

(Standard links)

@civibot civibot bot added the 5.38 label May 12, 2021
@mattwire
Copy link
Contributor

@eileenmcnaughton Thanks - looks good. It will need backporting to 5.37 where it regressed

@eileenmcnaughton
Copy link
Contributor Author

@mattwire yep - @totten we should expedite this drop - on the bright side doing the drop yesterday did help someone n the Joomla! channel

@seamuslee001
Copy link
Contributor

merging as per review

@seamuslee001 seamuslee001 merged commit ca90123 into civicrm:5.38 May 12, 2021
@seamuslee001 seamuslee001 deleted the 538 branch May 12, 2021 22:53
@totten totten changed the title Fix money formatting opps Fix internal formatting of monetary values when handling non-decimal currencies May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants