Bug 18723 - Dot not recognized as decimal separator on receive
Summary: Dot not recognized as decimal separator on receive
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical (vote)
Assignee: Josef Moravec
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-03 19:30 UTC by Séverine Queune
Modified: 2020-11-30 21:44 UTC (History)
16 users (show)

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


Attachments
Bug 18723: Do not unformat price (3.07 KB, patch)
2018-10-03 14:18 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18723: (Alternate Solution) Block form submission (5.12 KB, patch)
2019-01-15 14:39 UTC, José-Mario Monteiro-Santos
Details | Diff | Splinter Review
Bug 18723: (Alternate Solution) Block form submission (5.18 KB, patch)
2019-01-17 18:42 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 18723: Change dot into comma (4.16 KB, patch)
2019-01-17 21:55 UTC, José-Mario Monteiro-Santos
Details | Diff | Splinter Review
Bug 18723: Change dot into comma (4.22 KB, patch)
2019-01-22 11:15 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 18723: Change dot into comma (4.29 KB, patch)
2019-02-07 06:57 UTC, Katrin Fischer
Details | Diff | Splinter Review
Backport of the patch to 18.05.x (3.13 KB, patch)
2019-03-05 14:21 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Séverine Queune 2017-06-03 19:30:48 UTC
Since we work on 16.11, we can't use dot when we receive orders but only comma (for example : a price 11.25€ will be changed in 1125,00€ after receiving the order).
It seems linked to syspref "Currencyformat" because the behaviour is different when we change it from FR to US format (a price 15,43€ will be changed in 15,430.00€).

When we worked on 3.20 until this april, we could use both dot and comma when receiving books.
This is quite disturbing for staff users because a dot is required when adding an order to a basket, whereas a comma must be used when receiving the book but there is no warning like it exist in the basket.

I don't know if it's a similar problem to the "Bug 18186 - Amounts with decimal comma instead of decimal dot breaking payments" about the circulation module.
We also work with plack, but even without it, we still have this bug.
Comment 1 Fridolin Somers 2017-10-24 12:36:36 UTC
Duplicate of Bug 18471 ?
Comment 2 Fridolin Somers 2017-11-17 15:18:47 UTC
You answered in Bug 19521 is it a mistake ?
Comment 3 Victor Grousset/tuxayo 2018-03-14 13:03:45 UTC
Linked but not a duplicate. Needs an independent patch due to the specificities of the two different pages.

The current plan is to make the order page take into account the CurrencyFormat syspref. Server side that means using Koha::Number::Price->unformat()
But it's likely to be done client-side.
Because currently, on the client-side there are calculations to convert the amount from one currency to another.

With that the behavior would be consistent between the order and reception.
Comment 4 Victor Grousset/tuxayo 2018-03-14 16:58:14 UTC
caused by Bug 18471
Comment 5 Victor Grousset/tuxayo 2018-03-26 09:53:43 UTC
The current plan is to make bug 12310 go in master and then in this bug do a follow up in cgi-bin/koha/acqui/orderreceive.pl
Comment 6 Katrin Fischer 2018-08-14 12:49:28 UTC
If we alreay know, that this was caused by 18471 - could we not bring the old behaviour back somehow to make things consistent again? A solution that could be backported.

And then bug 12310 as a new feature/enh for the next release?
Comment 7 Victor Grousset/tuxayo 2018-08-14 17:18:38 UTC
If Bug 18471 causes more harm than good, then that's a nice idea.

If so, then no one should feel blocked to submit such a patch because of the ongoing work on bug 12310 and this bug.

Unfortunately (or fortunately ;) ) all the issues that are reported to me are about fines, so I'll focus on fixing 12310 to not let it sink. Along with this bug, because it's a nice enhancement and it's won't be to much time.

@Séverine Queune, maybe you could rollback bug 18471 on your instance while bug 12310 and this bug are fixed?
Comment 8 Katrin Fischer 2018-09-14 21:22:47 UTC
To test:
- Set CurrencyFormat to FR
- Order something, verify you can use . on input there
- Receive it, verify that if you use . there, it turns into thousands
Comment 9 Caroline Cyr La Rose 2018-10-01 20:01:13 UTC
While testing bug 9775, I came upon a decimal problem in orderreceive.pl. I don't know if it's the same bug. This happens in the "Actual cost" box.

To replicate:

1- Go to Acquisitions
2- Search for a Vendor
3- Click on "New basket"
4- Give basket a name and click "Save"
5- Click on "Add to basket"
6- Add an order through preferred method
7- In Accounting details, enter a vendor price with dot decimal (i.e. 19.44)
8- Save your order line
9- Click on "Close this basket"
10- Confirm closing of basket by clicking on "Yes, close"
11- Click on "Receive shipment"
12- Enter an invoice number and click "Next"
13- Click on "Receive" to the right of your order
14- In Accounting details, notice the Actual cost is written with a decimal dot.
15- Change the Actual cost, using a dot decimal (i.e. 20.99)
16- Receive the order
17- Click on "Save"
18- In "Already received" notice the price is multiplied by 100 (i.e. 2099,00)
19- Click on "Cancel receipt"
20- Click on "Receive" to the right of your order
21- In Accounting details, change the Actual cost, using a comma decimal (i.e. 20,99)
22- Receive the order
23- Click on "Save"
24- In "Already received", notice the price is correct.
Comment 10 Josef Moravec 2018-10-03 14:18:56 UTC
Created attachment 79892 [details] [review]
Bug 18723: Do not unformat price

Test plan:
a) Replicate the issue:
    0- Set CurrencyFormat to FR
    1- Go to Acquisitions
    2- Search for a Vendor
    3- Click on "New basket"
    4- Give basket a name and click "Save"
    5- Click on "Add to basket"
    6- Add an order through preferred method
    7- In Accounting details, enter a vendor price with dot decimal (i.e. 19.44)
    8- Save your order line
    9- Click on "Close this basket"
    10- Confirm closing of basket by clicking on "Yes, close"
    11- Click on "Receive shipment"
    12- Enter an invoice number and click "Next"
    13- Click on "Receive" to the right of your order
    14- In Accounting details, notice the Actual cost is written with a decimal dot.
    15- Change the Actual cost, using a dot decimal (i.e. 20.99)
    16- Receive the order
    17- Click on "Save"
    18- In "Already received" notice the price is multiplied by 100 (i.e. 2099,00)
    19- Click on "Cancel receipt"
    20- Click on "Receive" to the right of your order
    21- In Accounting details, change the Actual cost, using a comma decimal (i.e. 20,99)
    22- Receive the order
    23- Click on "Save"
    24- In "Already received", notice the price is correct.
b) apply the patch
c) try again, should be fixe now
Comment 11 Caroline Cyr La Rose 2018-10-03 15:25:56 UTC
Hi Josef,

Thanks for the patch!

I don't know if I didn't notice it before or if it's new, but when I change the actual cost to 20,99 (with a comma), the price once it's received is 20,00. The cents are removed completely.
Comment 12 Caroline Cyr La Rose 2018-10-03 15:31:38 UTC
(In reply to Caroline Cyr La Rose from comment #11)
> Hi Josef,
> 
> Thanks for the patch!
> 
> I don't know if I didn't notice it before or if it's new, but when I change
> the actual cost to 20,99 (with a comma), the price once it's received is
> 20,00. The cents are removed completely.

That's steps 20-24
Comment 13 Victor Grousset/tuxayo 2018-10-06 18:56:54 UTC
While bug 21507 is implemented, here was the work in progress on this bug:
https://gitlab.com/tuxayo/Koha/commit/3562c8708c820d46595640459915e0a97a6a9fad

If it takes too much time. I hope that the patch of Josef Moravec will be functional enough to be integrated and improve the current situation.
Comment 14 Victor Grousset/tuxayo 2018-11-26 16:30:32 UTC
Quite complete discussion on the issue in the IRC channel: http://irc.koha-community.org/koha/2018-11-26#i_2100176

The current plan (bug 21507) is still valid from what I understand. And various followups have been discussed.
Comment 15 José-Mario Monteiro-Santos 2019-01-15 14:39:21 UTC
Created attachment 83971 [details] [review]
Bug 18723: (Alternate Solution) Block form submission

When entering an amount in "Actual Cost" on the orderreceive page,
it is multiplied by 100 if the entered number's format does not
match the "CurrencyFormat" syspref.

This patch adds a value check before submitting the form. It uses
regexps based on the "CurrencyFormat" syspref to validate that the
number will be processed correctly.

Test Plan:
a)Replicate the issue:
    0- Set CurrencyFormat to FR
    1- Go to Acquisitions
    2- Search for a Vendor
    3- Click on "New basket"
    4- Give basket a name and click "Save"
    5- Click on "Add to basket"
    6- Add an order through preferred method
    7- In Accounting details, enter a vendor price with dot decimal (i.e. 19.44)
    8- Save your order line
    9- Click on "Close this basket"
    10- Confirm closing of basket by clicking on "Yes, close"
    11- Click on "Receive shipment"
    12- Enter an invoice number and click "Next"
    13- Click on "Receive" to the right of your order
    14- In Accounting details, notice the Actual cost is written with a decimal dot.
    15- Change the Actual cost, using a dot decimal (i.e 20.99)
    16- Receive the order
    17- Click on "Save"
    18- In "Already received" notice the price is multiplied by 100 (i.e. 2099,00)
    19- Click on "Cancel receipt"
    20- Click on "Receive" to the right of your order
    21- In Accounting details, change the Actual cost, using a comma decimal (i.e. 20,99)
    22- Receive the order
    23- Click on "Save"
    24- In "Already received", notice the price is correct.
b)Apply the patch
c)Test the patch:
    1- Click on "Cancel receipt"
    2- Click on "Receive to the right of your order
    3- In Accounting Details, notice the Actual cost is formatted correctly (comma decimal)
    4- Change the Actual cost, using a dot decimal (21.99)
    5- Receive the order
    6- Click on "Save"
    7- Notice an error message explaining the proper format for Actual cost (click "Ok")
    8- Change the Actual cost, using a comma decimal (21,99)
    9- Click on "Save"
    10- In "Already received", notice the price is correct.
    11- Test the other CurrencyFormat settings
Comment 16 Victor Grousset/tuxayo 2019-01-17 18:42:39 UTC
Hi, thanks for your help :)

Something is a bit weird:

>    14- In Accounting details, notice the Actual cost is written with a decimal dot.

«Alright»

>    15- Change the Actual cost, using a dot decimal (i.e 20.99)

«Wait that was already the case»

Anyway, I kept going.
Comment 17 Victor Grousset/tuxayo 2019-01-17 18:42:54 UTC
Created attachment 84150 [details] [review]
Bug 18723: (Alternate Solution) Block form submission

When entering an amount in "Actual Cost" on the orderreceive page,
it is multiplied by 100 if the entered number's format does not
match the "CurrencyFormat" syspref.

This patch adds a value check before submitting the form. It uses
regexps based on the "CurrencyFormat" syspref to validate that the
number will be processed correctly.

Test Plan:
a)Replicate the issue:
    0- Set CurrencyFormat to FR
    1- Go to Acquisitions
    2- Search for a Vendor
    3- Click on "New basket"
    4- Give basket a name and click "Save"
    5- Click on "Add to basket"
    6- Add an order through preferred method
    7- In Accounting details, enter a vendor price with dot decimal (i.e. 19.44)
    8- Save your order line
    9- Click on "Close this basket"
    10- Confirm closing of basket by clicking on "Yes, close"
    11- Click on "Receive shipment"
    12- Enter an invoice number and click "Next"
    13- Click on "Receive" to the right of your order
    14- In Accounting details, notice the Actual cost is written with a decimal dot.
    15- Change the Actual cost, using a dot decimal (i.e 20.99)
    16- Receive the order
    17- Click on "Save"
    18- In "Already received" notice the price is multiplied by 100 (i.e. 2099,00)
    19- Click on "Cancel receipt"
    20- Click on "Receive" to the right of your order
    21- In Accounting details, change the Actual cost, using a comma decimal (i.e. 20,99)
    22- Receive the order
    23- Click on "Save"
    24- In "Already received", notice the price is correct.
b)Apply the patch
c)Test the patch:
    1- Click on "Cancel receipt"
    2- Click on "Receive to the right of your order
    3- In Accounting Details, notice the Actual cost is formatted correctly (comma decimal)
    4- Change the Actual cost, using a dot decimal (21.99)
    5- Receive the order
    6- Click on "Save"
    7- Notice an error message explaining the proper format for Actual cost (click "Ok")
    8- Change the Actual cost, using a comma decimal (21,99)
    9- Click on "Save"
    10- In "Already received", notice the price is correct.
    11- Test the other CurrencyFormat settings

Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>
Comment 18 José-Mario Monteiro-Santos 2019-01-17 18:49:55 UTC
(In reply to Victor Grousset/tuxayo from comment #16)
> Hi, thanks for your help :)
> 
> Something is a bit weird:
> 
> >    14- In Accounting details, notice the Actual cost is written with a decimal dot.
> 
> «Alright»
> 
> >    15- Change the Actual cost, using a dot decimal (i.e 20.99)
> 
> «Wait that was already the case»
> 
> Anyway, I kept going.

I see how it can be a little confusing. What line 14 shows is that even though the CurrencyFormat uses a comma, the form uses a period when it is being edited. When that price (or a different price) is entered at step 15, the period "breaks" the formatting and leads to the unwanted multiplication.
Comment 19 Victor Grousset/tuxayo 2019-01-17 18:53:16 UTC
According to the QA and functionnal feedback that I got on bug 21507 and bug 12310:

- here we must also use jQuery.validator
  - https://gitlab.com/tuxayo/Koha/compare/master...bug%2F21507%2Fdecimal-separators-fines#0d520d86f436b1c7c08ed14dc1ac60e58fa114a4_0_19
  - can we copy the code from the above patch? So when both will be merged, there will be a consistence between payments/fines and acquisitions
- thousands separators shouldn't be allowed, which means the code that prefills  the field shouldn't add it.


(I'll reread again to see if there are other things to mention)
Comment 20 Katrin Fischer 2019-01-17 19:44:30 UTC
Can we just get a patch for now that makes this form behave like the rest of Koha  again and keep elaborate better fixes for the next step?

We have still not been able to fix the regression!
Comment 21 Katrin Fischer 2019-01-17 20:10:33 UTC
I think what we want for the stable releases is consistent behaviour. If I have to add my data everywhere with a decimal dot (independent of the CurrencyFormat setting!) then I want that on the receive form as well. Currently only this one input behaves badly which causes a lot of confusion and frustration.

And if we are going to make a change here - we need to do it for all input fields in ACQ (or in Koha) at once and release that with a major version. 

I think while the new patch might be a way forward, it might not work for fixing the regression.
Comment 22 José-Mario Monteiro-Santos 2019-01-17 21:55:13 UTC
Created attachment 84166 [details] [review]
Bug 18723: Change dot into comma

This fixes the regression that multiplies the amount entered by 100
when CurrencyFormat is set to FR. It replaces the last dot with a
comma before dealing with the value of ActualCost and ReplacementCost.

Test Plan:
a)Replicate the issue:
    0- Set CurrencyFormat to FR
    1- Go to Acquisitions
    2- Search for a Vendor
    3- Click on "New basket"
    4- Give basket a name and click "Save"
    5- Click on "Add to basket"
    6- Add an order through preferred method
    7- In Accounting details, enter a vendor price with dot decimal (i.e. 19.44)
    8- Save your order line
    9- Click on "Close this basket"
    10- Confirm closing of basket by clicking on "Yes, close"
    11- Click on "Receive shipment"
    12- Enter an invoice number and click "Next"
    13- Click on "Receive" to the right of your order
    14- In Accounting details, notice the Actual cost is written with a decimal dot.
    15- Change the Actual cost, using a dot decimal (i.e 20.99)
    16- Receive the order
    17- Click on "Save"
    18- In "Already received" notice the price is multiplied by 100 (i.e. 2099,00)
    19- Click on "Cancel receipt"
    20- Click on "Receive" to the right of your order
    21- In Accounting details, change the Actual cost, using a comma decimal (i.e. 20,99)
    22- Receive the order
    23- Click on "Save"
    24- In "Already received", notice the price is correct.
b)Apply the patch
c)Test the patch:
    1- Click on "Cancel receipt"
    2- Click on "Receive to the right of your order
    3- Change the Actual cost/Replacement cost, using a dot decimal (21.99)
    4- Receive the order
    5- Click on "Save"
    6- Notice that the Actual cost and the Replacement cost use commas
    7- Change the Actual cost, using a comma decimal (21,99)
    8- Click on "Save"
    9- In "Already received", notice the price is still correct.
Comment 23 Victor Grousset/tuxayo 2019-01-22 11:15:34 UTC
Created attachment 84280 [details] [review]
Bug 18723: Change dot into comma

This fixes the regression that multiplies the amount entered by 100
when CurrencyFormat is set to FR. It replaces the last dot with a
comma before dealing with the value of ActualCost and ReplacementCost.

Test Plan:
a)Replicate the issue:
    0- Set CurrencyFormat to FR
    1- Go to Acquisitions
    2- Search for a Vendor
    3- Click on "New basket"
    4- Give basket a name and click "Save"
    5- Click on "Add to basket"
    6- Add an order through preferred method
    7- In Accounting details, enter a vendor price with dot decimal (i.e. 19.44)
    8- Save your order line
    9- Click on "Close this basket"
    10- Confirm closing of basket by clicking on "Yes, close"
    11- Click on "Receive shipment"
    12- Enter an invoice number and click "Next"
    13- Click on "Receive" to the right of your order
    14- In Accounting details, notice the Actual cost is written with a decimal dot.
    15- Change the Actual cost, using a dot decimal (i.e 20.99)
    16- Receive the order
    17- Click on "Save"
    18- In "Already received" notice the price is multiplied by 100 (i.e. 2099,00)
    19- Click on "Cancel receipt"
    20- Click on "Receive" to the right of your order
    21- In Accounting details, change the Actual cost, using a comma decimal (i.e. 20,99)
    22- Receive the order
    23- Click on "Save"
    24- In "Already received", notice the price is correct.
b)Apply the patch
c)Test the patch:
    1- Click on "Cancel receipt"
    2- Click on "Receive to the right of your order
    3- Change the Actual cost/Replacement cost, using a dot decimal (21.99)
    4- Receive the order
    5- Click on "Save"
    6- Notice that the Actual cost and the Replacement cost use commas
    7- Change the Actual cost, using a comma decimal (21,99)
    8- Click on "Save"
    9- In "Already received", notice the price is still correct.

Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>
Comment 24 Victor Grousset/tuxayo 2019-01-22 11:30:37 UTC
Here are the result with the 3 formats, is that the expected behavior?

CurrencyFormat:
- FR:
  - dot: ok (act as decimal separator)
  - comma: ok (act as decimal separator)
- US
  - dot: ok
  - comma: acts as a thousand separator (so multiply by 100 if one expected it to act as a decimal separator)
- CH
  - dot: ok
  - comma: acts as a thousand separator (so multiply by 100 if one expected it to act as a decimal separator)
Comment 25 José-Mario Monteiro-Santos 2019-01-22 13:29:46 UTC
(In reply to Victor Grousset/tuxayo from comment #24)
> Here are the result with the 3 formats, is that the expected behavior?
> 
> CurrencyFormat:
> - FR:
>   - dot: ok (act as decimal separator)
>   - comma: ok (act as decimal separator)
> - US
>   - dot: ok
>   - comma: acts as a thousand separator (so multiply by 100 if one expected
> it to act as a decimal separator)
> - CH
>   - dot: ok
>   - comma: acts as a thousand separator (so multiply by 100 if one expected
> it to act as a decimal separator)

The other 2 formats use commas (or apostrophes for CH) as thousands separators, so that behavior is kind of normal.
Comment 26 Victor Grousset/tuxayo 2019-01-22 15:39:31 UTC
That's what I understood, good :)
Comment 27 Victor Grousset/tuxayo 2019-01-24 16:26:14 UTC
@José-Mario The patch "(Alternate Solution) Block form submission" is obsolete right? The previous functional discussion points toward using "Change dot into comma" if I understand correctly.
Comment 28 Katrin Fischer 2019-01-24 16:57:58 UTC
Sorry, quite confused here by now :( 

I'll try to test myself, but not sure when time will allow. If someone knows right away, what would be the expected behaviour for entering the actual price on receive and CurrencyFormat CH with the patch? Will it expect 10.00 or 10,00?
Comment 29 Victor Grousset/tuxayo 2019-01-24 17:06:19 UTC
> I'll try to test myself, but not sure when time will allow.

Don't worry, that's quite quick. It took me less than 10 min to test with the 3 syspref values. Including some time to get lost due to fall into some traps due to not knowing well this part of acq.

> what would be the expected behaviour for entering the actual price on receive and CurrencyFormat CH with the patch? Will it expect 10.00 or 10,00?

Both! It depend on what the user expects :)
input 10.00 => 10.00
input 10,00 => 1000 (treated a thousands sep and discarded)
Comment 30 José-Mario Monteiro-Santos 2019-01-24 17:23:52 UTC
@Victor the patch is indeed obsolete, I simply forgot to mark it as such.

@Katrin As Victor said, in CH format, 10,00 would be treated as 1000 since commas are thousand separators. I do not think anybody using CH would expect 10,00 to be treated as 10.00 due to commas having a different meaning in that format, but I could be wrong.
Comment 31 Katrin Fischer 2019-01-25 06:46:33 UTC
(In reply to José-Mario Monteiro-Santos from comment #30)
> @Victor the patch is indeed obsolete, I simply forgot to mark it as such.
> 
> @Katrin As Victor said, in CH format, 10,00 would be treated as 1000 since
> commas are thousand separators. I do not think anybody using CH would expect
> 10,00 to be treated as 10.00 due to commas having a different meaning in
> that format, but I could be wrong.

Still confused, but the test plan reads well (too tired from travelling) I will try to have  a look during the weekend!
Comment 32 Katrin Fischer 2019-02-07 06:56:52 UTC
This patch works fine for master, but I wonder about the lower versions.
Maybe the addition of replacement cost for 18.11 changed something? It's not there in 17.11, so the same fix will not apply I think.

On 18.11:

1) Set CurrencyFormat to FR
2) Receive item in acq:
   Quantity received: 1
   Actual cost and replacement cost: 11.11
3) Verify _both_ turned into: 111.10

On Master:
0) - 2) as above
3) Same with the patch: 111.10
4) Repeat with patch: all good.
Comment 33 Katrin Fischer 2019-02-07 06:57:35 UTC
Created attachment 84843 [details] [review]
Bug 18723: Change dot into comma

This fixes the regression that multiplies the amount entered by 100
when CurrencyFormat is set to FR. It replaces the last dot with a
comma before dealing with the value of ActualCost and ReplacementCost.

Test Plan:
a)Replicate the issue:
    0- Set CurrencyFormat to FR
    1- Go to Acquisitions
    2- Search for a Vendor
    3- Click on "New basket"
    4- Give basket a name and click "Save"
    5- Click on "Add to basket"
    6- Add an order through preferred method
    7- In Accounting details, enter a vendor price with dot decimal (i.e. 19.44)
    8- Save your order line
    9- Click on "Close this basket"
    10- Confirm closing of basket by clicking on "Yes, close"
    11- Click on "Receive shipment"
    12- Enter an invoice number and click "Next"
    13- Click on "Receive" to the right of your order
    14- In Accounting details, notice the Actual cost is written with a decimal dot.
    15- Change the Actual cost, using a dot decimal (i.e 20.99)
    16- Receive the order
    17- Click on "Save"
    18- In "Already received" notice the price is multiplied by 100 (i.e. 2099,00)
    19- Click on "Cancel receipt"
    20- Click on "Receive" to the right of your order
    21- In Accounting details, change the Actual cost, using a comma decimal (i.e. 20,99)
    22- Receive the order
    23- Click on "Save"
    24- In "Already received", notice the price is correct.
b)Apply the patch
c)Test the patch:
    1- Click on "Cancel receipt"
    2- Click on "Receive to the right of your order
    3- Change the Actual cost/Replacement cost, using a dot decimal (21.99)
    4- Receive the order
    5- Click on "Save"
    6- Notice that the Actual cost and the Replacement cost use commas
    7- Change the Actual cost, using a comma decimal (21,99)
    8- Click on "Save"
    9- In "Already received", notice the price is still correct.

Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 34 Katrin Fischer 2019-02-07 07:08:36 UTC
Sorry, I am probably confusing (too early still). 

This should work for master and 18.11. Will just need to have the one line for replacement cost removed for lower versions.

I know this is not the perfect fix - it's a regression fix that will need a better global solution at some point. But please don't hold it up any longer. I just received another ticket about this issue yesterday from a library and I feel it needs solving more than perfect right now.
Comment 35 Katrin Fischer 2019-02-07 07:09:51 UTC
Thx Jose-Mario and Victor!
Comment 36 Nick Clemens 2019-02-11 18:37:44 UTC
Awesome work all!

Pushed to master for 19.05
Comment 37 Martin Renvoize 2019-02-12 11:32:15 UTC
Pushed to 18.11.x for 18.11.03
Comment 38 Lucas Gass 2019-02-12 21:59:14 UTC
can't cleanly apply to 18.05.x. please rebase if needed in 18.05.x
Comment 39 Katrin Fischer 2019-02-12 22:02:18 UTC
The issue goes back as far as 17.11. Maybe a separate patch without the lines for replacement cost would work best (18.11 feature).
Comment 40 Martin Renvoize 2019-02-13 13:10:51 UTC
I think you're right there cait.. should be reasonably trivial to rebase along those lines.. did you want me to have a punt at it for you Lucas?
Comment 41 Victor Grousset/tuxayo 2019-03-05 14:21:55 UTC
Created attachment 86082 [details] [review]
Backport of the patch to 18.05.x

Cherry-picked from 18.11.x branch on top of current 18.05.x (0dd262a02e)
Removed replacement cost stuff which didn't exist in 18.05
Test plan "ran" successfully.
Comment 42 Victor Grousset/tuxayo 2019-03-05 16:49:15 UTC
Applying the previous patch (Backport for 18.05) on top of current 17.11.x (tag v17.11.16) works.
Test plan "ran" successfully.

Lucas, Frido: is there anything else doable to help landing in 18.05.x and 17.11.x ?
Comment 43 Lucas Gass 2019-03-13 16:36:09 UTC
Thanks Victor,

backported to 18.05.x for 18.05.11
Comment 44 Fridolin Somers 2019-03-13 16:47:59 UTC
Pushed to 17.11.x for 17.11.17
Comment 45 Victor Grousset/tuxayo 2019-03-13 16:54:58 UTC
Thanks Lucas and Fridolin! :D
Comment 46 Victor Grousset/tuxayo 2019-11-15 20:04:48 UTC
Hi :)

Here is an SQL query to detect orders and items (items.price) that were affected by this bug.
And 2 others to fix these cases.
And finally a large price check query that should point any case of price difference > 5 (this is arbitrary) between prices that should be close to each other (unitprice, unitprice_tax_excluded, ecost, items.price, etc)

The check queries were applied to a dozen of instances that were reporting price issue with acquisition receiving and thus the budget and funds total
And the fix queries in there final version were applied to a bit more than half of them.



=== BZ 18723 specific price check query ===
Should only detect cases related to this bug.

All values except ecost are x100 time bigger
- unitprice
- items.price
- unitprice_tax_excluded
- unitprice_tax_included
- tax_value_on_receiving

```
SELECT invoicenumber,
       link.ordernumber,
       items.itemnumber,
       ecost,       
       items.price,
       unitprice,
       unitprice_tax_excluded,
       unitprice_tax_included,
       tax_value_on_receiving,
       unitprice_tax_excluded/ecost as ratio
 FROM aqorders
      LEFT JOIN aqinvoices
      ON aqinvoices.invoiceid = aqorders.invoiceid
      LEFT JOIN aqorders_items as link
      ON link.ordernumber=aqorders.ordernumber
      LEFT JOIN items
      ON link.itemnumber=items.itemnumber
WHERE
       unitprice_tax_excluded    / ecost BETWEEN 50 AND 150
   AND unitprice_tax_included    / ecost BETWEEN 50 AND 150
   AND unitprice                 / ecost BETWEEN 50 AND 150
ORDER BY ratio DESC;
-- BETWEEN 50 AND 150 to get the differences between order cost and final cost.
-- This is arbitrary.
```


=== BZ 18723 specific data fix queries ===
Caveat, this might be not exhaustive, there might be remaining deriving data that isn't updated.

divide by 100
- unitprice
- items.price
- unitprice_tax_excluded
- unitprice_tax_included
- tax_value_on_receiving (don't forget this one)

```
UPDATE aqorders
SET unitprice=unitprice                           / 100,
    unitprice_tax_excluded=unitprice_tax_excluded / 100,
    unitprice_tax_included=unitprice_tax_included / 100,
    tax_value_on_receiving=tax_value_on_receiving / 100
WHERE
      -- we get only the x100 cases
      unitprice_tax_excluded / ecost BETWEEN 50 AND 150
  AND unitprice_tax_included / ecost BETWEEN 50 AND 150
  AND unitprice              / ecost BETWEEN 50 AND 150;
```

```
UPDATE items set price=price/100
-- wider criteria than the issue detection request
-- because a JOIN in the update would have made it less readable and error prone.
-- so it might fix more cases than the only those related to bug 18723
WHERE price/replacementprice between 50 and 150; -- we get only the x100 cases
-- caveat; we have seen replacementprice being NULL and thus the fix doesn't happen
-- we consider this to not be related to bug 18723
-- and it will be caught be the general analysis query.
```

Hopefully we can finally have a relief about this issue. 

=== general price check query ===
Compare all the values in a systematic way.
It can be summarized by
A not more than 5x B
B not more than 5x C
C not more than 5x A

5 is arbitrary, any feedback welcome.

```
SELECT invoicenumber,
       link.ordernumber,
       items.itemnumber,
       ecost,
       items.replacementprice AS items_replacementprice,
       aqorders.replacementprice AS aqorders_replacementprice,
       items.price,
       unitprice,
       unitprice_tax_excluded,
       unitprice_tax_included,
       tax_value_on_receiving,
       unitprice_tax_excluded/ecost as ratio
 FROM aqorders
      LEFT JOIN aqinvoices
      ON aqinvoices.invoiceid = aqorders.invoiceid
      LEFT JOIN aqorders_items as link
      ON link.ordernumber=aqorders.ordernumber
      LEFT JOIN items
      ON link.itemnumber=items.itemnumber
WHERE
      unitprice_tax_excluded    / unitprice_tax_included    > 5
   OR unitprice_tax_included    / unitprice    > 5
   OR unitprice                 / aqorders.replacementprice > 5
   OR aqorders.replacementprice / ecost                     > 5
   OR ecost                     / items.price               > 5
   OR items.price               / unitprice_tax_excluded    > 5
ORDER BY ratio DESC;
```

Thanks Arthur for you help in finally getting rid of this!