The OPACFineNoRenewals (and consequently the OPACFineNoRenewalsBlockAutoRenew) feature uses the patrons' account balance (which combines outstanding credits and debits) as opposed to their outstanding debts. This should either be configurable (or perhaps coded to use outstanding debts instead) as it does not accurately reflect what a patron may owe when AccountAutoReconcile is set to 'Do not' apply.
Created attachment 91448 [details] [review] Bug 23293: Add system preference
Created attachment 91449 [details] [review] Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference
Test plan 1) Apply the patches 2) Run the DB update 3) Set the new systempreference to 'Do not' 4) Check the OPACFineNoRenewals functionality now takes account of only outstanding debts and not ignores outstanding credits.
Martin, what do you think of adding something to Koha::Account for this? In the lines of $patron->account->outstanding_amount({ for => 'renewals' }); that encapsulates each required case and uses the sysprefs?
Sounds sane to me on first thoughts. [U+1F60A]
This one lacks tests and have minor issues (syspref inserted in the wrong spot)
Created attachment 93149 [details] [review] Bug 23293: Add system preference
Created attachment 93150 [details] [review] Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference
Created attachment 93151 [details] [review] Bug 23293: (follow-up) Add tests for C4::Circulation change
(In reply to Tomás Cohen Arazi from comment #4) > Martin, what do you think of adding something to Koha::Account for this? > In the lines of > > $patron->account->outstanding_amount({ for => 'renewals' }); > > that encapsulates each required case and uses the sysprefs? Not so sure after working back through this.. I think perhaps we could do this sort of change down the line, but somehow it doesn't feel quit right here.. needs more pondering but I'd rather not hold this patch up for it.
(In reply to Tomás Cohen Arazi from comment #6) > This one lacks tests and have minor issues (syspref inserted in the wrong > spot) Tests added and first patch amended to correct syspref order.
Created attachment 95121 [details] [review] Bug 23293: Add system preference Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 95122 [details] [review] Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 95123 [details] [review] Bug 23293: (follow-up) Add tests for C4::Circulation change Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 95124 [details] [review] Bug 23293: (QA follow-up) Minor fix for tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 95143 [details] [review] Bug 23293: Add system preference Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 95144 [details] [review] Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 95145 [details] [review] Bug 23293: (follow-up) Add tests for C4::Circulation change Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 95146 [details] [review] Bug 23293: (QA follow-up) Minor fix for tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work! Pushed to master for 19.11.00