Bug 18805 - Add ability to use up account credits
Summary: Add ability to use up account credits
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Target Milestone: 19.05
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Whiteboard:
Keywords:
Depends on: 21896 2696
Blocks: 14825
  Show dependency treegraph
 
Reported: 2017-06-14 18:08 UTC by Kyle M Hall
Modified: 2018-12-17 10:07 UTC (History)
18 users (show)

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


Attachments
Bug 18805 - Add ability to use up account credits (8.80 KB, patch)
2017-06-14 18:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18805 - Add ability to use up account credits (8.66 KB, patch)
2018-02-21 15:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18805 - Add ability to use up account credits (8.88 KB, patch)
2018-10-11 18:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18805: Add ability to use up account credits (9.00 KB, patch)
2018-10-30 03:44 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18805: Add ability to use up account credits (3.89 KB, patch)
2018-11-27 23:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18805: Add ability to use up account credits (4.06 KB, patch)
2018-11-28 19:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18805: Add ability to use up account credits (4.14 KB, patch)
2018-11-28 19:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18805: Add ability to use up account credits (4.19 KB, patch)
2018-11-29 16:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
[18.05.x] Bug 18805: Add ability to use up account credits (4.20 KB, patch)
2018-11-30 20:19 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2017-06-14 18:08:40 UTC
Right now, in Koha, credits always have a static amount outstanding. So if a patron owes $10 in fines, and has a $10 credit, they technically owe nothing, but yet they still owe $10. This seems rather confusing.

Instead, we should add the ability for credits to be used to pay down closed out fines and other fees.
Comment 1 Kyle M Hall 2017-06-14 18:17:05 UTC
Created attachment 64297 [details] [review]
Bug 18805 - Add ability to use up account credits

Right now, in Koha, credits always have a static amount outstanding. So if a patron owes $10 in fines, and has a $10 credit, they technically owe nothing, but yet they still owe $10. This seems rather confusing.

Instead, we should add the ability for credits to be used to pay down closed out fines and other fees.

Test Plan:
1) Apply this patch and dependencies
2) Create a manual credit of some amount
3) Create a manual invoice
4) Note the manual invoice was automatically paid down using
   the balance from the manual credit!
Comment 2 Lisette Scheer 2017-08-11 21:12:58 UTC
After applying the patch when we applied a manual credit we got the following error:
SOFTWARE ERROR
DBIx::Class:Storage::DBI::_dbh_execute():table 'koha_sandbox1.accountoffsets' doesn't exist doesn't exist at /var/lib/koha/sandbox/1.kohaclone/koha/object.pm line 121.

The credit did apply. 

Same error when applying the manual credit.
Behavior appears to be the same both before and after patch.
Comment 3 Caroline Cyr La Rose 2017-09-08 20:51:33 UTC
Error message when trying to apply patch.

fatal: sha1 information is lacking or useless (C4/Accounts.pm)
Comment 4 Colin Campbell 2017-10-10 10:08:15 UTC
I think this is a change in behaviour that was inadvertantly added into - see bug 18580 for one case of it
Comment 5 Katrin Fischer 2017-10-14 20:54:10 UTC
This is at the bottom of a rather long dependency tree where most of the items are not yet pushed to master:

18805: Add ability to use up account credits 

depends on:

2696: Fine Payments Should Show What Was Paid For 
18790: Add ability to void payments 
18786: Add ability to create custom payment types 
14826: Resurrect account offsets table
Comment 6 Katrin Fischer 2017-10-14 22:49:47 UTC
When trying to suspend a hold from the holds list in staff, there is an internal server error that translates to:

Can't call method "ymd" on an undefined value at /home/vagrant/kohaclone/Koha/Hold.pm line 65.
Comment 7 Kyle M Hall 2018-02-21 15:12:40 UTC
Created attachment 72025 [details] [review]
Bug 18805 - Add ability to use up account credits

Right now, in Koha, credits always have a static amount outstanding. So if a patron owes $10 in fines, and has a $10 credit, they technically owe nothing, but yet they still owe $10. This seems rather confusing.

Instead, we should add the ability for credits to be used to pay down closed out fines and other fees.

Test Plan:
1) Apply this patch and dependencies
2) Create a manual credit of some amount
3) Create a manual invoice
4) Note the manual invoice was automatically paid down using
   the balance from the manual credit!
Comment 8 Kyle M Hall 2018-02-21 15:15:44 UTC
(In reply to Katrin Fischer from comment #6)
> When trying to suspend a hold from the holds list in staff, there is an
> internal server error that translates to:
> 
> Can't call method "ymd" on an undefined value at
> /home/vagrant/kohaclone/Koha/Hold.pm line 65.

Can you test again? I can't recreate your error. My guess is it was an unrelated bug.
Comment 9 Katrin Fischer 2018-02-22 22:18:02 UTC
(In reply to Kyle M Hall from comment #8)
> (In reply to Katrin Fischer from comment #6)
> > When trying to suspend a hold from the holds list in staff, there is an
> > internal server error that translates to:
> > 
> > Can't call method "ymd" on an undefined value at
> > /home/vagrant/kohaclone/Koha/Hold.pm line 65.
> 
> Can you test again? I can't recreate your error. My guess is it was an
> unrelated bug.

I wonder if this comment ended up on the wrong bug report? It appears really unrelated to the bug topic now and I don't remember. Please discount it.
Comment 10 Tomás Cohen Arazi 2018-06-14 16:32:09 UTC
Right now, we are able to create credits (mancredit.pl) but there's no way to 'use' the existing credit against existing outstanding (debit) lines. This is odd in the interface and worth working on.

I don't think this should be implemented as an automatic feature in the first place. I would rather implement a way to assign remaining credits to specific lines, and THEN add the option to do it automatically, maybe syspref-controlled (better by payment type ;-))
Comment 11 Lisette Scheer 2018-09-14 23:14:43 UTC
Applying: Bug 18805 - Add ability to use up account credits
fatal: sha1 information is lacking or useless (Koha/Account.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 18805 - Add ability to use up account credits
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
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-18805---Add-ability-to-use-up-account-credits-P_PACB.patch

Lisette
Comment 12 Kyle M Hall 2018-10-11 18:07:43 UTC
Created attachment 80415 [details] [review]
Bug 18805 - Add ability to use up account credits

Right now, in Koha, credits always have a static amount outstanding. So if a patron owes $10 in fines, and has a $10 credit, they technically owe nothing, but yet they still owe $10. This seems rather confusing.

Instead, we should add the ability for credits to be used to pay down closed out fines and other fees.

Test Plan:
1) Apply this patch and dependencies
2) Create a manual credit of some amount
3) Create a manual invoice
4) Note the manual invoice was automatically paid down using
   the balance from the manual credit!
Comment 13 Alex Buckley 2018-10-30 03:44:08 UTC
Created attachment 81609 [details] [review]
Bug 18805: Add ability to use up account credits

Right now, in Koha, credits always have a static amount outstanding. So if a patron owes $10 in fines, and has a $10 credit, they technically owe nothing, but yet they still owe $10. This seems rather confusing.

Instead, we should add the ability for credits to be used to pay down closed out fines and other fees.

Test Plan:
1) Apply this patch and dependencies
2) Create a manual credit of some amount
3) Create a manual invoice
4) Note the manual invoice was automatically paid down using
   the balance from the manual credit!

Followed test plan, patch worked as described. I also ran
t/db_dependent/Accounts.t test which passed

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 14 Alex Buckley 2018-10-30 03:45:58 UTC
Patch works as described and I also ran t/db_dependent/Accounts.t unit test which also passes. 

I have corrected the commit message to be in the format 'Bug: <bugnumber>' so it matches commit message guidelines.
Comment 15 Barton Chittenden 2018-11-26 17:57:58 UTC
I'm marking this as critical -- this is causing old credits (which existed before the account offsets table) not to apply, which means that the debits show as unpaid on the pay fines tab.
Comment 16 Tomás Cohen Arazi 2018-11-27 23:36:44 UTC
I'm taking this one with Kyle's permission.

I rewrite and moved the Koha::Account::normalize_balance method to bug 21896. It is fully covered by tests there.
Comment 17 Tomás Cohen Arazi 2018-11-27 23:38:23 UTC
Created attachment 82686 [details] [review]
Bug 18805: Add ability to use up account credits

This patch introduces a new button in the 'Pay fines' tab  on the
patron's account page. This button, labeled 'Normalize account' is only
displayed when outstanding credits are available, that could be used to
pay up existing debts.

When clicking the button, the Koha::Account::normalize_balance method
(bug 21896) is used for the purpose.

To test:
- Apply this patch (on top of 21896)
- On a patron's 'Fines' tab, create a 'manual invoice' (maybe more than
  one)
- Go to the 'Pay fines' tab
=> SUCCESS: No 'Normalize account' button is displayed.
- On the 'create manual credit' tab, create a couple credits.
- Go to the 'Pay fines'
=> SUCCESS: 'Normalize account' button is displayed
- Click on 'Normalize account'
=> SUCCESS: Outstanding credits have been used to pay up debts.
- Play with different options (credit excedes debts and vice-versa, they
        match, etc)
=> SUCCESS: They all work as expected
- Sign off :-D
Comment 18 Christopher Brannon 2018-11-28 00:13:01 UTC
(In reply to Tomás Cohen Arazi from comment #10)
> Right now, we are able to create credits (mancredit.pl) but there's no way
> to 'use' the existing credit against existing outstanding (debit) lines.
> This is odd in the interface and worth working on.
> 
> I don't think this should be implemented as an automatic feature in the
> first place. I would rather implement a way to assign remaining credits to
> specific lines, and THEN add the option to do it automatically, maybe
> syspref-controlled (better by payment type ;-))

I do hope an automatic setting is in the near future.  One would assume credit would be used automatically.  I don't know of many places that don't automatically use your credit.
Comment 19 Christopher Brannon 2018-11-28 00:31:51 UTC
Can't locate object method "normalize_balance" via package "Koha::Account" at /home/vagrant/kohaclone/members/pay.pl line 290
Comment 20 Christopher Brannon 2018-11-28 00:48:48 UTC
Sorry, needed a restart.
Comment 21 Christopher Brannon 2018-11-28 01:04:59 UTC
Passes.  I just can't figure out how to get the sign off to work!
Comment 22 Kyle M Hall 2018-11-28 19:29:32 UTC
(In reply to Christopher Brannon from comment #21)
> Passes.  I just can't figure out how to get the sign off to work!

I can add your signoff to this patch.
Comment 23 Tomás Cohen Arazi 2018-11-28 19:40:36 UTC
Created attachment 82711 [details] [review]
Bug 18805: Add ability to use up account credits

This patch introduces a new button in the 'Pay fines' tab  on the
patron's account page. This button, labeled 'Normalize account' is only
displayed when outstanding credits are available, that could be used to
pay up existing debts.

When clicking the button, the Koha::Account::normalize_balance method
(bug 21896) is used for the purpose.

To test:
- Apply this patch (on top of 21896)
- On a patron's 'Fines' tab, create a 'manual invoice' (maybe more than
  one)
- Go to the 'Pay fines' tab
=> SUCCESS: No 'Apply credits' button is displayed.
- On the 'create manual credit' tab, create a couple credits.
- Go to the 'Pay fines'
=> SUCCESS: 'Apply credits' button is displayed
- Click on 'Apply credits'
=> SUCCESS: Outstanding credits have been used to pay up debts.
- Play with different options (credit excedes debts and vice-versa, they
        match, etc)
=> SUCCESS: They all work as expected
- Sign off :-D
Comment 24 Kyle M Hall 2018-11-28 19:43:34 UTC
Created attachment 82712 [details] [review]
Bug 18805: Add ability to use up account credits

This patch introduces a new button in the 'Pay fines' tab  on the
patron's account page. This button, labeled 'Normalize account' is only
displayed when outstanding credits are available, that could be used to
pay up existing debts.

When clicking the button, the Koha::Account::normalize_balance method
(bug 21896) is used for the purpose.

To test:
- Apply this patch (on top of 21896)
- On a patron's 'Fines' tab, create a 'manual invoice' (maybe more than
  one)
- Go to the 'Pay fines' tab
=> SUCCESS: No 'Apply credits' button is displayed.
- On the 'create manual credit' tab, create a couple credits.
- Go to the 'Pay fines'
=> SUCCESS: 'Apply credits' button is displayed
- Click on 'Apply credits'
=> SUCCESS: Outstanding credits have been used to pay up debts.
- Play with different options (credit excedes debts and vice-versa, they
        match, etc)
=> SUCCESS: They all work as expected
- Sign off :-D

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Comment 25 Martin Renvoize 2018-11-29 16:57:15 UTC
Created attachment 82758 [details] [review]
Bug 18805: Add ability to use up account credits

This patch introduces a new button in the 'Pay fines' tab  on the
patron's account page. This button, labeled 'Normalize account' is only
displayed when outstanding credits are available, that could be used to
pay up existing debts.

When clicking the button, the Koha::Account::normalize_balance method
(bug 21896) is used for the purpose.

To test:
- Apply this patch (on top of 21896)
- On a patron's 'Fines' tab, create a 'manual invoice' (maybe more than
  one)
- Go to the 'Pay fines' tab
=> SUCCESS: No 'Apply credits' button is displayed.
- On the 'create manual credit' tab, create a couple credits.
- Go to the 'Pay fines'
=> SUCCESS: 'Apply credits' button is displayed
- Click on 'Apply credits'
=> SUCCESS: Outstanding credits have been used to pay up debts.
- Play with different options (credit excedes debts and vice-versa, they
        match, etc)
=> SUCCESS: They all work as expected
- Sign off :-D

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2018-11-29 16:58:11 UTC
The qa script throws an error:

'Argument "" isn't numeric in numeric gt (>) at /home/martin/kohaclone/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc line 84.'

But, I think it's a false catch as .count numifies.

Passing QA.
Comment 27 Nick Clemens 2018-11-29 19:15:15 UTC
Awesome work everyone!

Pushed to master for 19.05
Comment 28 Kyle M Hall 2018-11-30 20:19:57 UTC
Created attachment 82816 [details] [review]
[18.05.x] Bug 18805: Add ability to use up account credits

This patch introduces a new button in the 'Pay fines' tab  on the
patron's account page. This button, labeled 'Normalize account' is only
displayed when outstanding credits are available, that could be used to
pay up existing debts.

When clicking the button, the Koha::Account::normalize_balance method
(bug 21896) is used for the purpose.

To test:
- Apply this patch (on top of 21896)
- On a patron's 'Fines' tab, create a 'manual invoice' (maybe more than
  one)
- Go to the 'Pay fines' tab
=> SUCCESS: No 'Apply credits' button is displayed.
- On the 'create manual credit' tab, create a couple credits.
- Go to the 'Pay fines'
=> SUCCESS: 'Apply credits' button is displayed
- Click on 'Apply credits'
=> SUCCESS: Outstanding credits have been used to pay up debts.
- Play with different options (credit excedes debts and vice-versa, they
        match, etc)
=> SUCCESS: They all work as expected
- Sign off :-D

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize 2018-12-10 09:59:22 UTC
Pushed to 18.11.x for 18.11.01