Skip to content

Fix: Sanitize fee recovery amount before saving to DB.#8254

Open
joshAppDev wants to merge 1 commit into
impress-org:developfrom
joshAppDev:sanitize-fee-amount
Open

Fix: Sanitize fee recovery amount before saving to DB.#8254
joshAppDev wants to merge 1 commit into
impress-org:developfrom
joshAppDev:sanitize-fee-amount

Conversation

@joshAppDev

Copy link
Copy Markdown
Contributor

Description

Background

The give-fee-recovery plugin stores the fee amount in the donation meta under '_give_fee_amount'. It uses the give_sanitize_amount_for_db() function before saving. This results in the amount having 6 decimal places. (E.g. "4000.000000".)

However, GiveWP automatically re-saves donation meta values if the donation is updated. For example, if the status changes from PROCESSING to COMPLETED. For '_give_fee_amount', it uses only the formatToDecimal() method before saving. This has 2 decimal places for the amount. (e.g. "4000.00".)

The problem

When a currency uses . as a thousands separator, it can change how GiveWP parses and formats amounts. This is more noticeable when large amounts are common with the currency, such as with VND or IDR.

Here is an example, from one of our Give sites running 3.22:
image

The amount was initially correct when the donation status was still PROCESSING. But after it changed to COMPLETED, the displayed fee amount became wrong.

Parsing issue

GiveWP uses several different functions when formatting the fee amount for display, including give_maybe_santize_amount(). When this function sees a fee amount with 6 decimal places, it parses correctly. But when the function sees the fee amount with 2 decimal places, it thinks that "4000.00" == "400000". Perhaps there is a way to fix this parsing algorithm, but not in this pull request.

( See give-fee-recovery/includes/admin/class-give-fee-recovery-admin.php :: donation_detail() )

Sanitizing solution

By adding give_sanitize_amount_for_db() to DonationRepository.php, we ensure the fee amount is consistently formatted for the DB across both give core, and give-fee-recovery. As a bonus, it also works around the parsing issue above.

Testing Instructions

For the IDR currency, we configure zero decimal places in the GiveWP settings. Additionally, we use the give_get_zero_based_currencies and give_fee_zero_based_currency filters to set IDR as zero based.

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

It affects how the amount is parsed and formatted in currencies using "." as the thousands separator.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant