Bug 23293 - OPACFineNoRenewals always compares against 'balance' not 'outstanding'
Summary: OPACFineNoRenewals always compares against 'balance' not 'outstanding'
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Martin Renvoize
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks: 24007
  Show dependency treegraph
 
Reported: 2019-07-10 11:08 UTC by Martin Renvoize
Modified: 2019-11-11 15:49 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
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


Attachments
Bug 23293: Add system preference (3.24 KB, patch)
2019-07-10 16:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference (2.19 KB, patch)
2019-07-10 16:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23293: Add system preference (3.22 KB, patch)
2019-09-25 09:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference (2.19 KB, patch)
2019-09-25 09:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23293: (follow-up) Add tests for C4::Circulation change (3.12 KB, patch)
2019-09-25 09:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23293: Add system preference (3.27 KB, patch)
2019-11-06 18:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference (2.24 KB, patch)
2019-11-06 18:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23293: (follow-up) Add tests for C4::Circulation change (3.08 KB, patch)
2019-11-06 18:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23293: (QA follow-up) Minor fix for tests (880 bytes, patch)
2019-11-06 18:48 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23293: Add system preference (3.36 KB, patch)
2019-11-07 11:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23293: Apply OPACFineNoRenewalsIncludeCredit preference (2.32 KB, patch)
2019-11-07 11:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23293: (follow-up) Add tests for C4::Circulation change (3.15 KB, patch)
2019-11-07 11:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23293: (QA follow-up) Minor fix for tests (954 bytes, patch)
2019-11-07 11:31 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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