Bug 18471 added number formating in order receive. I found a bug generated by this. Koha::Number::Price defines methods to (un)format price depending on CurrencyFormat syspref. The bug is that for CurrencyFormat US, some params are not defined in this module so there are retrieved from system locale. If system locale is not en_US, the params will be wrong. For example fr_FR will define , as decimal and so multiple price value by 1000.
18471
I have a UT that reproduces the bug. For testing in acquisition, one must use Plack mode, I don't understand why.
Created attachment 64838 [details] [review] Bug 18900 - add UT to Number/Price.t This patch adds unit tests to t/Number/Price.t in order to valid the problem. It sets in the script locale "en_US.UTF-8" by default so that those tests can be run with any system locale. And adds a test dedicated to this bug. (cherry picked from commit 20b3141bdf750bebb50e837771724324f7a67c1b)
Created attachment 64839 [details] [review] Bug 18900 - add UT to Number/Price.t This patch adds unit tests to t/Number/Price.t in order to valid the problem.
Created attachment 64840 [details] [review] Bug 18900 - wrong number format in receiving order Bug 18471 added number formating in order receive. I found a bug generated by this. Koha::Number::Price defines methods to (un)format price depending on CurrencyFormat syspref. The bug is that for CurrencyFormat US, some params are not defined in this module so there are retrieved from system locale. If system locale is not en_US, the params will be wrong. For example fr_FR will define ',' as decimal and so saved unit price is divided by 1000. To recreate: - On a server with locale 'fr_FR' - With Plack mode - Set CurrencyFormat on 'US' - Open a basket - Place an order for an item - Close basket - Receive order with changing 'Actual Cost' to '1,234' - Note the 'Actual Cost' is now '1.23' - Run unit test t/Number/Price.t without and with patch
Hi Fridolin, we also had some bad luck with Plack and amounts - see bug 18186!
Created attachment 65627 [details] [review] [SIGNED OFF] Bug 18900 - add UT to Number/Price.t This patch adds unit tests to t/Number/Price.t in order to valid the problem. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 65628 [details] [review] [SIGNED OFF] Bug 18900 - wrong number format in receiving order Bug 18471 added number formating in order receive. I found a bug generated by this. Koha::Number::Price defines methods to (un)format price depending on CurrencyFormat syspref. The bug is that for CurrencyFormat US, some params are not defined in this module so there are retrieved from system locale. If system locale is not en_US, the params will be wrong. For example fr_FR will define ',' as decimal and so saved unit price is divided by 1000. To recreate: - On a server with locale 'fr_FR' - With Plack mode - Set CurrencyFormat on 'US' - Open a basket - Place an order for an item - Close basket - Receive order with changing 'Actual Cost' to '1,234' - Note the 'Actual Cost' is now '1.23' - Run unit test t/Number/Price.t without and with patch Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Tests pass ok, no regressions found. We also have seen some strange phenomenons with prices and Plack, this might help in other cases too.
Created attachment 65759 [details] [review] Bug 18900: add UT to Number/Price.t This patch adds unit tests to t/Number/Price.t in order to valid the problem. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 65760 [details] [review] Bug 18900: (QA followup) Make tests conditional to locale existence It is important to highlight that tests have been skipped instead of having them pass on the absence of the required fr_FR.UTF-8 locale installed on the system. This patch does that check and effectively skips them. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 65761 [details] [review] Bug 18900: wrong number format in receiving order Bug 18471 added number formating in order receive. I found a bug generated by this. Koha::Number::Price defines methods to (un)format price depending on CurrencyFormat syspref. The bug is that for CurrencyFormat US, some params are not defined in this module so there are retrieved from system locale. If system locale is not en_US, the params will be wrong. For example fr_FR will define ',' as decimal and so saved unit price is divided by 1000. To recreate: - On a server with locale 'fr_FR' - With Plack mode - Set CurrencyFormat on 'US' - Open a basket - Place an order for an item - Close basket - Receive order with changing 'Actual Cost' to '1,234' - Note the 'Actual Cost' is now '1.23' - Run unit test t/Number/Price.t without and with patch Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, will be in 17.05.04.
These patches have been pushed to 16.11.x and will be in 16.11.12.
Pushed to 16.05.x, for 16.05.17 release