Bug 27290

Summary: Cash register allows for 'amount tendered' less than amount being paid
Product: Koha Reporter: Nick Clemens <nick>
Component: Fines and feesAssignee: Martin Renvoize <martin.renvoize>
Status: Pushed to stable --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: andrew, caroline.cyr-la-rose, fridolin.somers, katrin.fischer
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.03
Bug Depends on: 27289    
Bug Blocks: 13985, 26926    
Attachments: Bug 27290: Update 'type=number' to 'type=text inputmode=numeric'
Bug 27290: Update 'type=number' to 'type=text inputmode=numeric'
Bug 27290: (follow-up) fix filters - make register required
Bug 27290: Update 'type=number' to 'type=text inputmode=numeric'
Bug 27290: (follow-up) fix filters - make register required
Bug 27290: Clean up validation
Bug 27290: Clean up validation
Bug 27290: Clean up validation
Bug 27290: Clean up validation

Description Nick Clemens 2020-12-21 14:31:16 UTC
It seems amount tendered is only used to calculate change - if there is a value for the change then there is a confirmation. If there isn't then the transaction simply goes through

To test:
1 - Enable 'useCashRegisters' and 'EnablePointofSale'
2 - Add a cash register
3 - Add a debit type that can be sold
4 - Go to point of sale and sell the item multiple times
5 - Enter 'Amount tendered' less than amount being paid
6 - Click confirm
7 - Transaction is processed as if full funds received
8 - Try with a negative number - goes through again
Comment 1 Martin Renvoize 2020-12-21 16:28:25 UTC
Created attachment 114568 [details] [review]
Bug 27290: Update 'type=number' to 'type=text inputmode=numeric'

This patch updates the inputs for the pay form on the point of sale page
to use numeric inputmode. This appears to also fix the validation issue
described in the bug report.

To test:
1 - Enable 'useCashRegisters' and 'EnablePointofSale'
2 - Add a cash register
3 - Add a debit type that can be sold
4 - Go to point of sale and sell the item multiple times
5 - Enter 'Amount tendered' less than amount being paid
6 - Click confirm
7 - Transaction is processed as if full funds received
8 - Try with a negative number - goes through again
Comment 2 Martin Renvoize 2020-12-21 16:29:41 UTC
I think there's some browser ambiguity here.. I tested on latest Chrome, Firefox and Brave browsers and all worked as expected with this patch... but I'm not entirely confident in my fix... it's a step in the right direction accessibility wise though :)
Comment 3 Nick Clemens 2020-12-23 14:11:32 UTC
Created attachment 114670 [details] [review]
Bug 27290: Update 'type=number' to 'type=text inputmode=numeric'

This patch updates the inputs for the pay form on the point of sale page
to use numeric inputmode. This appears to also fix the validation issue
described in the bug report.

To test:
1 - Enable 'useCashRegisters' and 'EnablePointofSale'
2 - Add a cash register
3 - Add a debit type that can be sold
4 - Go to point of sale and sell the item multiple times
5 - Enter 'Amount tendered' less than amount being paid
6 - Click confirm
7 - Transaction is processed as if full funds received
8 - Try with a negative number - goes through again

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 4 Nick Clemens 2020-12-23 14:11:34 UTC
Created attachment 114671 [details] [review]
Bug 27290: (follow-up) fix filters - make register required

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Nick Clemens 2020-12-23 14:12:16 UTC
(In reply to Martin Renvoize from comment #2)
> I think there's some browser ambiguity here.. I tested on latest Chrome,
> Firefox and Brave browsers and all worked as expected with this patch... but
> I'm not entirely confident in my fix... it's a step in the right direction
> accessibility wise though :)

I can also no longer recreate - though now I was able to submit the form without a register selected? Follow-up attached, but maybe I am having other issues :-)
Comment 6 Martin Renvoize 2020-12-30 15:21:54 UTC
Created attachment 114747 [details] [review]
Bug 27290: Update 'type=number' to 'type=text inputmode=numeric'

This patch updates the inputs for the pay form on the point of sale page
to use numeric inputmode. This appears to also fix the validation issue
described in the bug report.

To test:
1 - Enable 'useCashRegisters' and 'EnablePointofSale'
2 - Add a cash register
3 - Add a debit type that can be sold
4 - Go to point of sale and sell the item multiple times
5 - Enter 'Amount tendered' less than amount being paid
6 - Click confirm
7 - Transaction is processed as if full funds received
8 - Try with a negative number - goes through again

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Martin Renvoize 2020-12-30 15:21:57 UTC
Created attachment 114748 [details] [review]
Bug 27290: (follow-up) fix filters - make register required

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Martin Renvoize 2020-12-30 15:22:00 UTC
Created attachment 114749 [details] [review]
Bug 27290: Clean up validation

It seems we had the validator instantiated twice and we used a mix of
validation of required at form element level and in JS instantiator.

This patch moves the rules to consistently apply at the instantiation
and removes the duplicate lines.
Comment 9 Martin Renvoize 2020-12-30 15:24:16 UTC
OK, reading the followup highlighted some further issues.. I think during a rebase somewhere we ended up with duplication of code blocks causing random issues.

I've cleaned up the duplication in the final patch.

I'm not entirely sure we need the 'on_editing' option added to the $Price filter of the readonly field, but I may be wrong there.. I not Nick you added it back in so I'm happy to defer.. not sure it makes much difference in this case.

So.. all in all.. back to NQA :)
Comment 10 Martin Renvoize 2020-12-31 12:07:09 UTC
Created attachment 114757 [details] [review]
Bug 27290: Clean up validation

It seems we had the validator instantiated twice and we used a mix of
validation of required at form element level and in JS instantiator.

This patch moves the rules to consistently apply at the instantiation
and removes the duplicate lines.

To test:
1 - Enable 'useCashRegisters' and 'EnablePointofSale'
2 - Add a cash register
3 - Add a debit type that can be sold
4 - Go to point of sale and sell the item multiple times
5 - Enter 'Amount tendered' less than amount being paid
6 - Click confirm
7 - Transaction is processed as if full funds received
8 - Try with a negative number - goes through again

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2020-12-31 12:08:50 UTC
Looks like Owen was working on similar improvements at the same time.. so I've tested and signed off his work on bug 27282 and made this dependant upon it and squashed the final outcome here..
Comment 12 Katrin Fischer 2021-01-10 22:34:57 UTC
Currently blocked by a minor issue on dependent bug 7289, hopefully resolved soon.
Comment 13 Martin Renvoize 2021-02-05 11:14:27 UTC
Created attachment 116389 [details] [review]
Bug 27290: Clean up validation

It seems we had the validator instantiated twice and we used a mix of
validation of required at form element level and in JS instantiator.

This patch moves the rules to consistently apply at the instantiation
and removes the duplicate lines.

To test:
1 - Enable 'useCashRegisters' and 'EnablePointofSale'
2 - Add a cash register
3 - Add a debit type that can be sold
4 - Go to point of sale and sell the item multiple times
5 - Enter 'Amount tendered' less than amount being paid
6 - Click confirm
7 - Transaction is processed as if full funds received
8 - Try with a negative number - goes through again

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2021-02-05 11:17:09 UTC
This bug now also disables the immediate call to validate the amount collected against the amount owed when items are added to the basket... as requested by Katrin in bug 27289.
Comment 15 Katrin Fischer 2021-02-07 12:40:36 UTC
This is so much better, thx!
Comment 16 Katrin Fischer 2021-02-07 12:43:51 UTC
Created attachment 116472 [details] [review]
Bug 27290: Clean up validation

It seems we had the validator instantiated twice and we used a mix of
validation of required at form element level and in JS instantiator.

This patch moves the rules to consistently apply at the instantiation
and removes the duplicate lines.

To test:
1 - Enable 'useCashRegisters' and 'EnablePointofSale'
2 - Add a cash register
3 - Add a debit type that can be sold
4 - Go to point of sale and sell the item multiple times
5 - Enter 'Amount tendered' less than amount being paid
6 - Click confirm
7 - Transaction is processed as if full funds received
8 - Try with a negative number - goes through again

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Jonathan Druart 2021-02-08 13:57:36 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 18 Fridolin Somers 2021-02-12 13:11:43 UTC
Pushed to 20.11.x for 20.11.03
Comment 19 Andrew Fuerste-Henry 2021-02-17 20:38:43 UTC
Missing dependency from 27289, which needs a rebase for 20.05