Bug 21915 - Add a way to automatically reconcile balance for patrons
Summary: Add a way to automatically reconcile balance for patrons
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: 19.05
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Whiteboard:
Keywords:
: 18580 (view as bug list)
Depends on: 15899 21896 21969
Blocks: 14825
  Show dependency treegraph
 
Reported: 2018-11-29 16:33 UTC by Tomás Cohen Arazi
Modified: 2019-01-23 17:44 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
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:


Attachments
Bug 21915: Add AccountAutoReconciliate syspref (2.10 KB, patch)
2018-11-29 19:49 UTC, Tomás Cohen Arazi
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
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
Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconciliate syspref (3.00 KB, patch)
2018-11-29 19:58 UTC, Tomás Cohen Arazi
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
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
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
Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.00 KB, patch)
2018-11-30 13:44 UTC, Tomás Cohen Arazi
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
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
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
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
Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.07 KB, patch)
2018-11-30 19:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call (5.96 KB, patch)
2018-11-30 19:06 UTC, Kyle M Hall
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
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
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
Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.04 KB, patch)
2018-12-05 14:16 UTC, Tomás Cohen Arazi
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
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
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
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
Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.11 KB, patch)
2018-12-11 15:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21915: Add AccountAutoReconcile syspref (3.14 KB, patch)
2018-12-12 08:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21915: Reconcile balance on _FixAccountForLostAndReturned call (5.30 KB, patch)
2018-12-12 08:32 UTC, Martin Renvoize
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
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
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
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 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 2018-11-29 19:49:57 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2018-11-29 19:50:04 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2018-11-29 19:58:41 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2018-11-29 19:58:55 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2018-11-30 12:53:22 UTC Comment hidden (obsolete)
Comment 6 Tomás Cohen Arazi 2018-11-30 12:56:25 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2018-11-30 13:34:47 UTC Comment hidden (obsolete)
Comment 8 Tomás Cohen Arazi 2018-11-30 13:44:33 UTC Comment hidden (obsolete)
Comment 9 Tomás Cohen Arazi 2018-11-30 13:44:37 UTC Comment hidden (obsolete)
Comment 10 Tomás Cohen Arazi 2018-11-30 13:44:41 UTC Comment hidden (obsolete)
Comment 11 Tomás Cohen Arazi 2018-11-30 13:44:46 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2018-11-30 13:44:50 UTC Comment hidden (obsolete)
Comment 13 Kyle M Hall 2018-11-30 19:06:19 UTC Comment hidden (obsolete)
Comment 14 Kyle M Hall 2018-11-30 19:06:32 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall 2018-11-30 19:06:36 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2018-11-30 19:06:40 UTC Comment hidden (obsolete)
Comment 17 Kyle M Hall 2018-11-30 19:06:44 UTC Comment hidden (obsolete)
Comment 18 Tomás Cohen Arazi 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 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 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 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 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 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 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 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 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 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 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 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 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 2018-12-12 08:40:33 UTC
*** Bug 18580 has been marked as a duplicate of this bug. ***
Comment 32 Nick Clemens 2019-01-09 01:15:32 UTC
Awesome work all!

Pushed to master for 19.05
Comment 33 Nick Clemens 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 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 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.