Bug 31513 - NaN errors when using refund and payout with CurrencyFormat = FR
Summary: NaN errors when using refund and payout with CurrencyFormat = FR
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Joonas Kylmälä
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-09-02 13:24 UTC by Christian Stelzenmüller
Modified: 2023-06-08 22:26 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.07, 21.11.14


Attachments
Bug 31513: Remove price formatting form data- attributes (5.31 KB, patch)
2022-09-11 15:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31513: Remove price formatting form data- attributes (5.37 KB, patch)
2022-09-14 07:13 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 31513: Remove price formatting form data- attributes (5.51 KB, patch)
2022-09-15 18:16 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 31513: (QA follow-up) Keep price formatting on non-editable text fields (2.50 KB, patch)
2022-09-15 18:16 UTC, Joonas Kylmälä
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Stelzenmüller 2022-09-02 13:24:02 UTC
This happens with 21.11 with us.

"Issue refund" on a paid transaction.
In the field "Returned to patron" there is a prefilled text "NaN", i don't know why yet.
"Confirming" that without correcting it to the amount in numbers that you want to refund, you get an Internal Server Error.
Comment 1 Katrin Fischer 2022-09-07 14:00:51 UTC
Confirmed on master with CurrencyFormat FR. It works ok with US.

NaN = Not a Number often a JavaScript related error
Comment 2 Katrin Fischer 2022-09-11 14:27:11 UTC
The problem is not an easy fix on first glance. This is all done in JS, but I have a feeling it should use template variables with the unformatted data instead:

var amount = button.data('amount');
var amountoutstanding = button.data('amountoutstanding') || 0;
var paid = amount - amountoutstanding;
$("#paid + span").replaceWith(paid);
$("#returned").attr({ "value": paid, "max": paid, "min": 0 });
Comment 3 Katrin Fischer 2022-09-11 14:29:07 UTC
The Payout has the same issues :(

We really ought to add testing with other CurrencyFormats to our normal test plans for all money related patches. Maybe also test with != EN to start with. If it works then, it is likely to always work.

Tempted to mark this major actually... this breaks central money related workflows for a lot of libraries.
Comment 4 Katrin Fischer 2022-09-11 15:01:49 UTC
Created attachment 140405 [details] [review]
Bug 31513: Remove price formatting form data- attributes

We cannot use data formatted with a decimal comma for
calculations. So we need to make sure that the data- attributes
passed to the modals are in 'database format' with decimal .

This will fix the NaN errors on the forms, but might also
negatively effect formatting in some spots.
Formatting should be fixed separately as this is for sure the
right first step, but we need to put in more work for displays.

To test:
* Set CurrencyFormat system preference to FR
* Go to any patron account > accounting tab
* Create a credit
* On Transactions tab: Payout amount at bottom = Verify NaN in input
* On Transactions tab: Issue payout on the credit line = Verify NaN in input
* Create a manual debit and pay it
* On Transactions tab: Issue refund = Verify NaN in input
* Apply patch
* Repeat test plan
* Now all input fields should be set with the correct amounts

https://bugs.koha-community.org/show_bug.cgi?id=31486
Comment 5 Katrin Fischer 2022-09-11 15:03:05 UTC
This fixes calculations, but we might pay with formatting... I am not sure if there is JS way to format prices yet?

For example, if we stick with the JS, we'd want to format paid here for displaying it in the span:

$("#paid + span").replaceWith(paid);
Comment 6 ByWater Sandboxes 2022-09-14 07:13:32 UTC
Created attachment 140612 [details] [review]
Bug 31513: Remove price formatting form data- attributes

We cannot use data formatted with a decimal comma for
calculations. So we need to make sure that the data- attributes
passed to the modals are in 'database format' with decimal .

This will fix the NaN errors on the forms, but might also
negatively effect formatting in some spots.
Formatting should be fixed separately as this is for sure the
right first step, but we need to put in more work for displays.

To test:
* Set CurrencyFormat system preference to FR
* Go to any patron account > accounting tab
* Create a credit
* On Transactions tab: Payout amount at bottom = Verify NaN in input
* On Transactions tab: Issue payout on the credit line = Verify NaN in input
* Create a manual debit and pay it
* On Transactions tab: Issue refund = Verify NaN in input
* Apply patch
* Repeat test plan
* Now all input fields should be set with the correct amounts

https://bugs.koha-community.org/show_bug.cgi?id=31486

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Comment 7 Joonas Kylmälä 2022-09-15 18:16:32 UTC
Created attachment 140684 [details] [review]
Bug 31513: Remove price formatting form data- attributes

We cannot use data formatted with a decimal comma for
calculations. So we need to make sure that the data- attributes
passed to the modals are in 'database format' with decimal .

This will fix the NaN errors on the forms, but might also
negatively effect formatting in some spots.
Formatting should be fixed separately as this is for sure the
right first step, but we need to put in more work for displays.

To test:
* Set CurrencyFormat system preference to FR
* Go to any patron account > accounting tab
* Create a credit
* On Transactions tab: Payout amount at bottom = Verify NaN in input
* On Transactions tab: Issue payout on the credit line = Verify NaN in input
* Create a manual debit and pay it
* On Transactions tab: Issue refund = Verify NaN in input
* Apply patch
* Repeat test plan
* Now all input fields should be set with the correct amounts

https://bugs.koha-community.org/show_bug.cgi?id=31486

Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 8 Joonas Kylmälä 2022-09-15 18:16:36 UTC
Created attachment 140685 [details] [review]
Bug 31513: (QA follow-up) Keep price formatting on non-editable text fields

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 9 Katrin Fischer 2022-09-16 07:32:31 UTC
Thanks for the follow-up, Joonas! I'll keep that in mind for some other places I spotted issues with the formatting!
Comment 10 Tomás Cohen Arazi 2022-09-16 13:34:44 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 11 Katrin Fischer 2022-09-16 13:40:11 UTC
Please backport!
Comment 12 Katrin Fischer 2022-09-16 13:48:35 UTC
Thanks Tomas :)
Comment 13 Lucas Gass 2022-10-31 21:33:04 UTC
Backported to 22.05.x for upcoming 22.05.07 release
Comment 14 Arthur Suzuki 2022-11-14 14:21:23 UTC
applied to 21.11 for 21.11.14
Comment 15 Victor Grousset/tuxayo 2022-11-23 18:28:07 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.