Bug 11373 - Add a "cash register" feature to the fine payment form.
Summary: Add a "cash register" feature to the fine payment form.
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 1 vote (vote)
Assignee: Maxime Beaulieu
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-10 22:09 UTC by Frédérick Capovilla
Modified: 2017-10-25 13:37 UTC (History)
21 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Add a "cash register" feature to the fine payment form. (10.95 KB, patch)
2013-12-10 22:09 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Bug 11373: Add a "cash register" feature to the fine payment form. (10.96 KB, patch)
2014-03-11 11:54 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 11373: Add a "cash register" feature to the fine payment form. (11.00 KB, patch)
2014-03-11 13:10 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 11373 - Unit tests for Accounts.pm (4.59 KB, patch)
2014-07-16 14:36 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11373 - Fixed: 'Change to give back' not displaying. (2.48 KB, patch)
2014-07-21 17:32 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11373: Add a "cash register" feature to the fine payment form. (11.18 KB, patch)
2015-03-13 19:35 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11373 - Unit tests for Accounts.pm (4.59 KB, patch)
2015-03-13 19:36 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11373 - Fixed: 'Change to give back' not displaying. (2.48 KB, patch)
2015-03-13 19:36 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11373: Add a "cash register" feature to the fine payment form. (8.70 KB, patch)
2017-02-21 15:47 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 11373 - Fixed: 'Change to give back' not displaying. (2.49 KB, patch)
2017-02-21 15:48 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 11373: Add a "cash register" feature to the fine payment form. (8.75 KB, patch)
2017-03-17 21:08 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 11373 - Fixed: 'Change to give back' not displaying. (2.54 KB, patch)
2017-03-17 21:08 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 11373: Add a "cash register" feature to the fine payment form. (7.34 KB, patch)
2017-09-25 18:30 UTC, David Bourgault
Details | Diff | Splinter Review
Bug 11373: Add a "cash register" feature to the fine payment form. (7.24 KB, patch)
2017-10-19 12:47 UTC, Simon Pouchol
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frédérick Capovilla 2013-12-10 22:09:11 UTC
Created attachment 23382 [details] [review]
Add a "cash register" feature to the fine payment form.

This patch improves the fines payment form to allow automatic calculation of the amount to give back to the patron once a fine payment is received.

For example, if a patron wants to pay a 3$ fine with a 5$ bill, you can set "Amount paid" to 3 and "Collect from patron" to 5. A message will then tell you to give back 2$ to the patron.

Sponsored-by: CCSR ( http://www.ccsr.qc.ca )
Comment 1 Kyle M Hall 2013-12-13 12:56:29 UTC
Excellent! I would like to note that the accounts rewrite ( Bug 6427 ) also has this feature.

Kyle
Comment 2 Julian Maurice 2014-03-11 11:54:03 UTC Comment hidden (obsolete)
Comment 3 sandboxes@biblibre.com 2014-03-11 13:10:09 UTC
Patch tested with a sandbox, by Joel Aloi <aloi54@live.fr>
Comment 4 sandboxes@biblibre.com 2014-03-11 13:10:33 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2014-04-18 22:13:49 UTC
I am worried about the changes to Accounts.pm. We need unit tests to go with this patch.
Comment 6 Maxime Beaulieu 2014-07-16 14:36:54 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2014-07-17 08:18:10 UTC
This last patch does not need a signoff (will be done by QA).
Comment 8 Katrin Fischer 2014-07-20 19:03:38 UTC
I am sorry, but this doesn't seem to work for me. I have added various fines and fees to my patron account. Then I chose "Pay amount" and changed the collected from patron value to be higher than the total amount owed. I didn't see anything telling me how much change to give back to the patron. The open fines were paid and the additional money was 'lost'.

The same happens for paying individual fines.

Please retest.
Comment 9 Maxime Beaulieu 2014-07-21 17:32:54 UTC Comment hidden (obsolete)
Comment 10 Nicole C. Engard 2015-03-06 10:41:19 UTC
Am I supposed to apply to all these patches? I got the following:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 11373: Add a "cash register" feature to the fine payment form.
/home/nengard/kohaclone/.git/rebase-apply/patch:242: trailing whitespace.
        $template->param( 
/home/nengard/kohaclone/.git/rebase-apply/patch:247: trailing whitespace.
        $template->param( 
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
<stdin>:242: trailing whitespace.
        $template->param( 
<stdin>:247: trailing whitespace.
        $template->param( 
warning: 2 lines applied after fixing whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging members/paycollect.pl
Auto-merging C4/Accounts.pm
CONFLICT (content): Merge conflict in C4/Accounts.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 11373: Add a "cash register" feature to the fine payment form.
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-11373-Add-a-cash-register-feature-to-the-fine--I0lypc.patch
Comment 11 Maxime Beaulieu 2015-03-13 19:35:09 UTC
Created attachment 36875 [details] [review]
Bug 11373: Add a "cash register" feature to the fine payment form.

If the amount received is greater than the amount to pay, a message is
shown to tell the employee how much money to give back.

Signed-off-by: Joel Aloi <aloi54@live.fr>
Comment 12 Maxime Beaulieu 2015-03-13 19:36:11 UTC
Created attachment 36876 [details] [review]
Bug 11373 - Unit tests for Accounts.pm

I added unit tests for the modified subs in C4/Accounts.pm.
    makepartialpayment()
    WriteOffFee()

	modified:   t/db_dependent/Accounts.t
Comment 13 Maxime Beaulieu 2015-03-13 19:36:38 UTC
Created attachment 36877 [details] [review]
Bug 11373 - Fixed: 'Change to give back' not displaying.

I tested the last patch and found that the case 'amount oustanding == amount paid' was not handled in paycollect.p .
The confirmation box should now appear when those values are equal and the amount collected is higher than the amount paid.

I have found another problem while testing.
When paying all fines or selected fines, the 'The amount collected is greater than the total amount paid.' message would appear even when the entered data was correct.

Both issues were adressed in this patch.
	modified:   koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
	modified:   members/paycollect.pl
Comment 14 Maxime Beaulieu 2015-03-13 19:43:48 UTC
Fixed the conflicts.
Comment 15 M. Tompsett 2015-10-09 18:25:57 UTC
Comment on attachment 36875 [details] [review]
Bug 11373: Add a "cash register" feature to the fine payment form.

Review of attachment 36875 [details] [review]:
-----------------------------------------------------------------

Changing the sign of what was there before would need something done during upgrade to change it to a new way of processing. This wasn't done, so frankly, I think the logic I highlighted is broken. Though, admittedly, I did not run this code yet.

::: C4/Accounts.pm
@@ +722,5 @@
>      my $insert = 'INSERT INTO accountlines (borrowernumber, accountno, date, amount, '
>      .  'description, accounttype, amountoutstanding, itemnumber, manager_id, note) '
>      . ' VALUES (?, ?, now(), ?, ?, ?, 0, ?, ?, ?)';
>  
> +    $dbh->do(  $insert, undef, $borrowernumber, $nextaccntno, $payment,

0-$amount is the negative of $amount. This is a vast change! Is this correct?

@@ +728,5 @@
>  
>      UpdateStats({
>                  branch => $user,
>                  type => 'payment',
> +                amount => $payment,

This is 0-$amount which is the negative of what was put here before.

@@ +739,4 @@
>              action            => 'create_payment',
>              borrowernumber    => $user,
>              accountno         => $nextaccntno,
> +            amount            => $payment,

On a positive note, by putting $payment earlier, it can be used nicely here.

@@ +820,4 @@
>      UpdateStats({
>                  branch => $branch,
>                  type => 'writeoff',
> +                amount => $payment,

Again, 0-$amount is negative of what was there before.
Comment 16 Marc Véron 2016-04-17 13:10:18 UTC
Patch does not apply.
Auto-merging members/paycollect.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
Auto-merging C4/Accounts.pm
CONFLICT (content): Merge conflict in C4/Accounts.pm
Comment 17 Mehdi Hamidi 2017-02-21 15:47:57 UTC
Created attachment 60528 [details] [review]
Bug 11373: Add a "cash register" feature to the fine payment form.

If the amount received is greater than the amount to pay, a message is
shown to tell the employee how much money to give back.

Signed-off-by: Joel Aloi <aloi54@live.fr>
Comment 18 Mehdi Hamidi 2017-02-21 15:48:10 UTC
Created attachment 60529 [details] [review]
Bug 11373 - Fixed: 'Change to give back' not displaying.

I tested the last patch and found that the case 'amount oustanding == amount paid' was not handled in paycollect.p .
The confirmation box should now appear when those values are equal and the amount collected is higher than the amount paid.

I have found another problem while testing.
When paying all fines or selected fines, the 'The amount collected is greater than the total amount paid.' message would appear even when the entered data was correct.

Both issues were adressed in this patch.
	modified:   koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
	modified:   members/paycollect.pl
Comment 19 Mehdi Hamidi 2017-02-21 16:22:28 UTC
I rebased the patches and now they apply properly.
I removed the changes in the file C4/Accounts.pm since the sub makepartialpayment was moved and renamed to sub pay in Koha/Accounts.pm, the patch works fine with the new changes.

I obsoleted the second patch which contained the tests for C4/Accounts.pm, because the module has it's own test files.

I added a test plan everyone can test it.

Test Plan
1) Apply patch
2) Select a patron with a fine.
3) Go to the selected details patron.
4) Go to the fines tab.
5) Click the button pay amount or pay selected.
6) Choose the amount paid equal to the outstanding amount(exemple : 3$).
7) Choose the amount collected to be more than the outstanding amount(exemple :5$)
8) Click the button confirm.
- You should be prompted with a message saying : "Change to give back: 2.00"
9) Click on the button above Confirm payment.

The selected patron should not have an outstanding fines left.
Comment 20 sandboxes@biblibre.com 2017-03-17 21:08:13 UTC
Patch tested with a sandbox, by Candice Hope <candiceh@payson.org>
Comment 21 sandboxes@biblibre.com 2017-03-17 21:08:46 UTC
Created attachment 61233 [details] [review]
Bug 11373: Add a "cash register" feature to the fine payment form.

If the amount received is greater than the amount to pay, a message is
shown to tell the employee how much money to give back.

Signed-off-by: Joel Aloi <aloi54@live.fr>

Signed-off-by: Candice Hope <candiceh@payson.org>
Comment 22 sandboxes@biblibre.com 2017-03-17 21:08:51 UTC
Created attachment 61234 [details] [review]
Bug 11373 - Fixed: 'Change to give back' not displaying.

I tested the last patch and found that the case 'amount oustanding == amount paid' was not handled in paycollect.p .
The confirmation box should now appear when those values are equal and the amount collected is higher than the amount paid.

I have found another problem while testing.
When paying all fines or selected fines, the 'The amount collected is greater than the total amount paid.' message would appear even when the entered data was correct.

Both issues were adressed in this patch.
	modified:   koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
	modified:   members/paycollect.pl

Signed-off-by: Candice Hope <candiceh@payson.org>
Comment 23 Rozenn Davenel 2017-03-21 09:56:27 UTC
It works.
Comment 24 Philippe Audet-Fortin 2017-05-17 15:44:06 UTC
It looks perfects!
Comment 25 David Bourgault 2017-09-25 18:30:11 UTC
Created attachment 67348 [details] [review]
Bug 11373: Add a "cash register" feature to the fine payment form.

If the amount received is greater than the amount to pay, a message is
shown to tell the employee how much money to give back.

This obsoletes (both) previous patches that did not apply
Comment 26 Simon Pouchol 2017-10-19 12:47:36 UTC
Created attachment 68296 [details] [review]
Bug 11373: Add a "cash register" feature to the fine payment form.

Works as intended.

Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>