|
Description
Katrin Fischer
2022-02-19 20:58:58 UTC
To reproduce: - Set CurrencyFormat pref to FR - Activate Poit of sale and cash register features - Add cash register - Add debit type that 'can be sold' with a price of 10.00 - Go to point of sale, click on the item to purchase - Verify the Amount to be paid shows the wrong amount and can't be fixed It is caused by Bug 27801. I don't dare to suggest a modification since we had no problem prior to that fix, but obviously it was meant to fix something. It's the interaction between format_price and moneyFormat.. moneyFormat appears to be hard coded to '.' decimal separators.. but then Koha's internals expect point decimal separators in inputs too. Created attachment 134830 [details] [review] Bug 30139: Fix javascript moneyFormat to be compatible with FR format New way of formatting money amount to make it work with FR format Test plan: 1- Set CurrencyFormat pref to FR 2- Activate Point of sale and cash register features 3- Add cash register 4- Add debit type that 'can be sold' with a price of 10.00 5- Go to point of sale, click on the item to purchase 6- Verify the Amount to be paid shows the wrong amount and can't be fixed on the interface 7- Apply the patch 8- Refresh the page and do step 5 again 9- Notice it now shows the right amount If this is good. We can copy paste the changes to paycollect.tt moneyFormat function as well. Created attachment 134842 [details] [review] Bug 30139: Fix javascript moneyFormat to be compatible with FR format New way of formatting money amount to make it work with FR format Test plan: 1- Set CurrencyFormat pref to FR 2- Activate Point of sale and cash register features 3- Add cash register 4- Add debit type that 'can be sold' with a price of 10.00 5- Go to point of sale, click on the item to purchase 6- Verify the Amount to be paid shows the wrong amount and can't be fixed on the interface 7- Apply the patch 8- Refresh the page and do step 5 again 9- Notice it now shows the right amount Signed-off-by: David Nind <david@davidnind.com> Created attachment 134843 [details] Screenshot - after patch applied I've attached a screenshot after the patch was applied. I'm assuming that for the cost column, amount being paid, and amount tendered will be fixed to show a comma rather than a period once changes to paycollect.tt moneyFormat function are made (comment #5). Note: it was a little tricky to capture the screen with the amount tendered entered as 10,00 - once you click out it defaults to being 10.00 as the amount tendered. I'm also not sure whether you would normally enter the amount tendered as 10.00 or 10,00. Starting QA here I've tested POS with the 3 different currency formats (FR, CH and US) and found it all working now. I also did a quick run through some other monetary actions, like a cash-up etc., no regressions spotted. @David: In input fields you should always use . right now, independent from the CurrencyFormat setting, this only applies for display. Created attachment 134957 [details] [review] Bug 30139: Fix javascript moneyFormat to be compatible with FR format New way of formatting money amount to make it work with FR format Test plan: 1- Set CurrencyFormat pref to FR 2- Activate Point of sale and cash register features 3- Add cash register 4- Add debit type that 'can be sold' with a price of 10.00 5- Go to point of sale, click on the item to purchase 6- Verify the Amount to be paid shows the wrong amount and can't be fixed on the interface 7- Apply the patch 8- Refresh the page and do step 5 again 9- Notice it now shows the right amount Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Created attachment 134969 [details] [review] Bug 30139: Fix javascript moneyFormat to be compatible with FR format New way of formatting money amount to make it work with FR format Test plan: 1- Set CurrencyFormat pref to FR 2- Activate Point of sale and cash register features 3- Add cash register 4- Add debit type that 'can be sold' with a price of 10.00 5- Go to point of sale, click on the item to purchase 6- Verify the Amount to be paid shows the wrong amount and can't be fixed on the interface 7- Apply the patch 8- Refresh the page and do step 5 again 9- Notice it now shows the right amount Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Copy pasted the same function into paycollect.tt (In reply to Shi Yao Wang from comment #13) > Copy pasted the same function into paycollect.tt Hi Shi, please don't add changes to your patches after the patches have been signed and QA. This is not what people tested - please always provide the changes in a separate patch. I'd also be inclined to say it should maybe be a separate bug and also have a new test plan. I had tested paying fines and had no issue - where and why is the new change needed? I've restored the previous patch for now - awaiting follow-up patch here or on a separate bug. (In reply to Katrin Fischer from comment #14) > (In reply to Shi Yao Wang from comment #13) > > Copy pasted the same function into paycollect.tt > > Hi Shi, please don't add changes to your patches after the patches have been > signed and QA. This is not what people tested - please always provide the > changes in a separate patch. > > I'd also be inclined to say it should maybe be a separate bug and also have > a new test plan. I had tested paying fines and had no issue - where and why > is the new change needed? Noted, thanks for letting me know. I sent the changes because both files had the exact same function. It seems inconsistent to have 2 functions with the same name, but with different code. I guess it doesn't really matter if the other one doesn't have any issue in the first place. Created attachment 134973 [details] [review] Bug 30139: Same fix for the other javascript moneyFormat function Consistency is always an argument - I'll try to do some testing on paycollect for any regressions. Created attachment 134974 [details] [review] Bug 30139: Same fix for the other javascript moneyFormat function This patch makes javascript moneyFormat function in paycollect.tt consistent with the one in pay.tt Created attachment 134976 [details] [review] Bug 30139: Same fix for the other javascript moneyFormat function This patch makes javascript moneyFormat function in paycollect.tt consistent with the one in pay.tt Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Pushed to master for 22.05, thanks to everybody involved [U+1F984] |