Bug 23293

Summary: OPACFineNoRenewals always compares against 'balance' not 'outstanding'
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: CirculationAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: major    
Priority: P5 - low CC: gmcharlt, katrin.fischer, kyle.m.hall, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24007
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
The patchset adds a new system preference, `OPACFineNoRenewalsIncludeCredits`, to control whether the `OPACFineNoRenewals` function uses the account balance or account amount outstanding for calculation.
Version(s) released in:
19.11.00
Bug Depends on:    
Bug Blocks: 24007    
Attachments: Bug 23293: Add system preference
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference
Bug 23293: Add system preference
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference
Bug 23293: (follow-up) Add tests for C4::Circulation change
Bug 23293: Add system preference
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference
Bug 23293: (follow-up) Add tests for C4::Circulation change
Bug 23293: (QA follow-up) Minor fix for tests
Bug 23293: Add system preference
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference
Bug 23293: (follow-up) Add tests for C4::Circulation change
Bug 23293: (QA follow-up) Minor fix for tests

Description Martin Renvoize 2019-07-10 11:08:54 UTC
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.
Comment 1 Martin Renvoize 2019-07-10 16:36:12 UTC
Created attachment 91448 [details] [review]
Bug 23293: Add system preference
Comment 2 Martin Renvoize 2019-07-10 16:36:15 UTC
Created attachment 91449 [details] [review]
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference
Comment 3 Martin Renvoize 2019-07-10 16:39:29 UTC
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.
Comment 4 Tomás Cohen Arazi 2019-09-19 19:01:58 UTC
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?
Comment 5 Martin Renvoize 2019-09-20 06:54:25 UTC
Sounds sane to me on first thoughts. 😊
Comment 6 Tomás Cohen Arazi 2019-09-20 13:01:12 UTC
This one lacks tests and have minor issues (syspref inserted in the wrong spot)
Comment 7 Martin Renvoize 2019-09-25 09:59:52 UTC
Created attachment 93149 [details] [review]
Bug 23293: Add system preference
Comment 8 Martin Renvoize 2019-09-25 09:59:55 UTC
Created attachment 93150 [details] [review]
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference
Comment 9 Martin Renvoize 2019-09-25 09:59:58 UTC
Created attachment 93151 [details] [review]
Bug 23293: (follow-up) Add tests for C4::Circulation change
Comment 10 Martin Renvoize 2019-09-25 10:01:02 UTC
(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.
Comment 11 Martin Renvoize 2019-09-25 10:01:33 UTC
(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.
Comment 12 Tomás Cohen Arazi 2019-11-06 18:48:47 UTC
Created attachment 95121 [details] [review]
Bug 23293: Add system preference

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2019-11-06 18:48:51 UTC
Created attachment 95122 [details] [review]
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi 2019-11-06 18:48:54 UTC
Created attachment 95123 [details] [review]
Bug 23293: (follow-up) Add tests for C4::Circulation change

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2019-11-06 18:48:57 UTC
Created attachment 95124 [details] [review]
Bug 23293: (QA follow-up) Minor fix for tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Jonathan Druart 2019-11-07 11:31:22 UTC
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>
Comment 17 Jonathan Druart 2019-11-07 11:31:27 UTC
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>
Comment 18 Jonathan Druart 2019-11-07 11:31:31 UTC
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>
Comment 19 Jonathan Druart 2019-11-07 11:31:36 UTC
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>
Comment 20 Martin Renvoize 2019-11-07 13:27:13 UTC
Nice work!

Pushed to master for 19.11.00