Bug 21915 - Add a way to automatically reconcile balance for patrons
Summary: Add a way to automatically reconcile balance for patrons
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
: 18580 (view as bug list)
Depends on: 15899 21896 21969
Blocks: 24290 25146 25780 24266
  Show dependency tree / graph
 
Reported: 2018-11-29 16:33 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2021-06-14 21:28 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
In the past, if a patron had any credit existing on their account (newly added, or pre-existing), if debts were present then the credit balance would always be immediately applied to the debt. This functionality was inadvertently removed during refactoring efforts which debuted in 16.11. This patch adds code to restore the functionality and allows it to be optionally applied to the system via a new system preference, `AccountAutoReconcile`. Note: The new preference defaults to the post 16.11 behaviour, if you wish to restore the 16.11 functionality then you will need to update the preference after the upgrade.
Version(s) released in:
Circulation function:

Attachments
Bug 21915: Add AccountAutoReconciliate syspref (2.10 KB, patch)
2018-11-29 19:49 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call (5.89 KB, patch)
2018-11-29 19:50 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call (5.89 KB, patch)
2018-11-29 19:58 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconciliate syspref (3.00 KB, patch)
2018-11-29 19:58 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual invoice creation (1.31 KB, patch)
2018-11-30 12:53 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual credit creation (1.44 KB, patch)
2018-11-30 12:56 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Add a cli script to reconcile balances (4.04 KB, patch)
2018-11-30 13:34 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.00 KB, patch)
2018-11-30 13:44 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call (5.89 KB, patch)
2018-11-30 13:44 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual invoice creation (1.31 KB, patch)
2018-11-30 13:44 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual credit creation (1.44 KB, patch)
2018-11-30 13:44 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Add a cli script to reconcile balances (4.04 KB, patch)
2018-11-30 13:44 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.07 KB, patch)
2018-11-30 19:06 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call (5.96 KB, patch)
2018-11-30 19:06 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual invoice creation (1.38 KB, patch)
2018-11-30 19:06 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual credit creation (1.51 KB, patch)
2018-11-30 19:06 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 21915: Add a cli script to reconcile balances (4.11 KB, patch)
2018-11-30 19:06 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.04 KB, patch)
2018-12-05 14:16 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call (5.94 KB, patch)
2018-12-05 14:16 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual invoice creation (1.36 KB, patch)
2018-12-05 14:16 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual credit creation (1.50 KB, patch)
2018-12-05 14:16 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Add a cli script to reconcile balances (4.55 KB, patch)
2018-12-05 14:17 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.11 KB, patch)
2018-12-11 15:30 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.14 KB, patch)
2018-12-12 08:32 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call (5.30 KB, patch)
2018-12-12 08:32 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual invoice creation (1.46 KB, patch)
2018-12-12 08:32 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 21915: Call reconcile_balance on manual credit creation (1.25 KB, patch)
2018-12-12 08:32 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Bug 21915: Add a cli script to reconcile balances (4.65 KB, patch)
2018-12-12 08:32 UTC, Martin Renvoize (ashimema) 		Details | Diff | Splinter Review
Show Obsolete (23) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2018-11-29 16:33:03 UTC 
Bug 18805 introduced a way to use existing credits to cancel outstanding debts, from the staff interface.

Most users expressed they want existing credits to be applied to outstanding debt, in an automatic way.

We could add a syspref controlling this behaviour, and a cli script for people wanting all the balances reconciled at once, or nightly through a cronjob.
Comment 1 Tomás Cohen Arazi (tcohen) 2018-11-29 19:49:57 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi (tcohen) 2018-11-29 19:50:04 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi (tcohen) 2018-11-29 19:58:41 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi (tcohen) 2018-11-29 19:58:55 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi (tcohen) 2018-11-30 12:53:22 UTC Comment hidden (obsolete)
Comment 6 Tomás Cohen Arazi (tcohen) 2018-11-30 12:56:25 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi (tcohen) 2018-11-30 13:34:47 UTC Comment hidden (obsolete)
Comment 8 Tomás Cohen Arazi (tcohen) 2018-11-30 13:44:33 UTC Comment hidden (obsolete)
Comment 9 Tomás Cohen Arazi (tcohen) 2018-11-30 13:44:37 UTC Comment hidden (obsolete)
Comment 10 Tomás Cohen Arazi (tcohen) 2018-11-30 13:44:41 UTC Comment hidden (obsolete)
Comment 11 Tomás Cohen Arazi (tcohen) 2018-11-30 13:44:46 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi (tcohen) 2018-11-30 13:44:50 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall (khall) 2018-11-30 19:06:19 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall (khall) 2018-11-30 19:06:32 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall (khall) 2018-11-30 19:06:36 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall (khall) 2018-11-30 19:06:40 UTC Comment hidden (obsolete)
Comment 17 Kyle M Hall (khall) 2018-11-30 19:06:44 UTC Comment hidden (obsolete)
Comment 18 Tomás Cohen Arazi (tcohen) 2018-12-05 14:16:46 UTC 
Created attachment 82863 [details] [review]
Bug 21915: Add AccountAutoReconcile syspref

This syspref will be used to call the reconcile_balance routing as
needed.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Tomás Cohen Arazi (tcohen) 2018-12-05 14:16:50 UTC 
Created attachment 82864 [details] [review]
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call

This patch makes _FixAccountForLostAndReturned reconcile the patron's
account balance, when the AccountAutoReconcile syspref is set.

To test:
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Circulation.t
=> SUCCESS: Tests pass, peace \o/
- Sign off :-D

Sponsored-by: ByWater Solutions

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 20 Tomás Cohen Arazi (tcohen) 2018-12-05 14:16:55 UTC 
Created attachment 82865 [details] [review]
Bug 21915: Call reconcile_balance on manual invoice creation

This patch makes the manual invoice creation step call
Koha::Account::reconcile_balance when the AccountAutoReconcile syspref
is set.

To test:
- Apply the patch
- Have some outstanding credit
- Create a manual invoice
=> SUCCESS: Same behaviour as always
- Set AccountAutoReconcile
- Create a manual invoice
=> SUCCESS: Reconcillation happened
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 21 Tomás Cohen Arazi (tcohen) 2018-12-05 14:16:59 UTC 
Created attachment 82866 [details] [review]
Bug 21915: Call reconcile_balance on manual credit creation

This patch makes the manual credit creation step call
Koha::Account::reconcile_balance when the AccountAutoReconcile syspref
is set.

To test:
- Apply the patch
- Have some outstanding debits
- Create a manual credit
=> SUCCESS: Same behaviour as always
- Set AccountAutoReconcile
- Create a manual credit
=> SUCCESS: Reconcillation happened
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Tomás Cohen Arazi (tcohen) 2018-12-05 14:17:04 UTC 
Created attachment 82867 [details] [review]
Bug 21915: Add a cli script to reconcile balances

This patch adds a CLI script, reconcile_balances.pl that takes care
of calling the reconcile operation for all patrons that have outstanding
credits.

A weird situation has been spotted in the wild, in which a debit ends up
actually being a credit, because amount outstanding is tweaked in
dropbox-mode returns. Until we figure what to do about that (probably a
new account type, etc), we catch any exceptions and warn about the
situation.

To test:
- Have patrons with outstanding credits and debits
- Run the script:
  $ kshell
 k$ perl misc/cronjobs/reconcile_balances.pl --verbose
=> SUCCESS: Notice patrons got their balances reconciled, and useful
            information is output.
- Add new outstanding credits and debits to patrons you con easily
identify
- Run:
 k$ perl misc/cronjobs/reconcile_balances.pl
=> SUCCESS: Notice balances are reconciled, but no output
- Run:
 k$ perl misc/cronjobs/reconcile_balances.pl --help
=> SUCCESS: Usage information is printed.
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 23 Tomás Cohen Arazi (tcohen) 2018-12-06 19:09:31 UTC 
I've added bug 21969 as dependency, because some pathological use cases considered debits as credits. Those cases are explained in bug 14591, and should be dealt with on a separate bug report.
Comment 24 Kyle M Hall (khall) 2018-12-11 15:30:17 UTC 
Created attachment 83053 [details] [review]
Bug 21915: Add AccountAutoReconcile syspref

This syspref will be used to call the reconcile_balance routing as
needed.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Martin Renvoize (ashimema) 2018-12-12 08:32:22 UTC 
Created attachment 83062 [details] [review]
Bug 21915: Add AccountAutoReconcile syspref

This syspref will be used to call the reconcile_balance routing as
needed.

Sponsored-by: ByWater Solutions
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize (ashimema) 2018-12-12 08:32:26 UTC 
Created attachment 83063 [details] [review]
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call

This patch makes _FixAccountForLostAndReturned reconcile the patron's
account balance, when the AccountAutoReconcile syspref is set.

To test:
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Circulation.t
=> SUCCESS: Tests pass, peace \o/
- Sign off :-D

Sponsored-by: ByWater Solutions
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize (ashimema) 2018-12-12 08:32:31 UTC 
Created attachment 83064 [details] [review]
Bug 21915: Call reconcile_balance on manual invoice creation

This patch makes the manual invoice creation step call
Koha::Account::reconcile_balance when the AccountAutoReconcile syspref
is set.

To test:
- Apply the patch
- Have some outstanding credit
- Create a manual invoice
=> SUCCESS: Same behaviour as always
- Set AccountAutoReconcile
- Create a manual invoice
=> SUCCESS: Reconcillation happened
- Sign off :-D

Sponsored-by: ByWater Solutions
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize (ashimema) 2018-12-12 08:32:36 UTC 
Created attachment 83065 [details] [review]
Bug 21915: Call reconcile_balance on manual credit creation

This patch makes the manual credit creation step call
Koha::Account::reconcile_balance when the AccountAutoReconcile syspref
is set.

To test:
- Apply the patch
- Have some outstanding debits
- Create a manual credit
=> SUCCESS: Same behaviour as always
- Set AccountAutoReconcile
- Create a manual credit
=> SUCCESS: Reconcillation happened
- Sign off :-D

Sponsored-by: ByWater Solutions
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize (ashimema) 2018-12-12 08:32:40 UTC 
Created attachment 83066 [details] [review]
Bug 21915: Add a cli script to reconcile balances

This patch adds a CLI script, reconcile_balances.pl that takes care
of calling the reconcile operation for all patrons that have outstanding
credits.

A weird situation has been spotted in the wild, in which a debit ends up
actually being a credit, because amount outstanding is tweaked in
dropbox-mode returns. Until we figure what to do about that (probably a
new account type, etc), we catch any exceptions and warn about the
situation.

To test:
- Have patrons with outstanding credits and debits
- Run the script:
  $ kshell
 k$ perl misc/cronjobs/reconcile_balances.pl --verbose
=> SUCCESS: Notice patrons got their balances reconciled, and useful
            information is output.
- Add new outstanding credits and debits to patrons you con easily
identify
- Run:
 k$ perl misc/cronjobs/reconcile_balances.pl
=> SUCCESS: Notice balances are reconciled, but no output
- Run:
 k$ perl misc/cronjobs/reconcile_balances.pl --help
=> SUCCESS: Usage information is printed.
- Sign off :-D

Sponsored-by: ByWater Solutions
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Martin Renvoize (ashimema) 2018-12-12 08:34:25 UTC 
Minor rebase during QA was required...

All tests pass, qa scripts pass and the code looks solid.. I could not find any regressions and I could not think of anywhere that we may have missed.

Passing QA
Comment 31 Martin Renvoize (ashimema) 2018-12-12 08:40:33 UTC 
*** Bug 18580 has been marked as a duplicate of this bug. ***
Comment 32 Nick Clemens (kidclamp) 2019-01-09 01:15:32 UTC 
Awesome work all!

Pushed to master for 19.05
Comment 33 Nick Clemens (kidclamp) 2019-01-09 01:16:31 UTC 
We should have a dependent bug to call reconcile balance for fines (when closed, not accruing) but I think is beyond scope here
Comment 34 Tomás Cohen Arazi (tcohen) 2019-01-09 11:11:47 UTC 
(In reply to Nick Clemens from comment #33)
> We should have a dependent bug to call reconcile balance for fines (when
> closed, not accruing) but I think is beyond scope here

Feel free to file a bug and assign it to me to deal with that.
Comment 35 Martin Renvoize (ashimema) 2019-01-10 16:21:23 UTC 
Pushed to 18.11.x for 18.11.02
Comment 36 Jesse Maseto 2019-01-23 17:44:30 UTC 
Will not be in 18.05. Merge conflicts and doesn't apply cleanly.
Comment 37 Martin Renvoize (ashimema) 2019-12-20 18:04:10 UTC 
We need to build a tree of bugs to take notice of this system preference.. I keep finding cases where it's not being applied :(