Bug 14577 - Allow restriction of checkouts based on fines of guarantor/guarantee
Summary: Allow restriction of checkouts based on fines of guarantor/guarantee
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 15656
Blocks: 19382 14570 16405
  Show dependency treegraph
 
Reported: 2015-07-21 03:39 UTC by Jesse Weaver
Modified: 2017-09-28 13:01 UTC (History)
8 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees (14.30 KB, patch)
2016-02-11 19:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees (17.75 KB, patch)
2016-03-03 16:22 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees (16.47 KB, patch)
2016-03-03 16:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees (16.46 KB, patch)
2016-03-07 16:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees (16.51 KB, patch)
2016-03-08 14:00 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees (16.58 KB, patch)
2016-03-11 15:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees (16.58 KB, patch)
2016-03-16 18:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14577 [QA Followup] (5.53 KB, patch)
2016-03-16 18:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees (16.65 KB, patch)
2016-03-18 10:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14577 [QA Followup] (5.60 KB, patch)
2016-03-18 10:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees (16.65 KB, patch)
2016-04-27 16:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14577 [QA Followup] (5.61 KB, patch)
2016-04-27 16:51 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Weaver 2015-07-21 03:39:48 UTC
... with the amount controlled by a system preference.
Comment 1 Kyle M Hall 2016-02-11 19:30:23 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2016-03-03 16:22:20 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2016-03-03 16:58:07 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2016-03-07 16:41:26 UTC Comment hidden (obsolete)
Comment 5 Nicole C. Engard 2016-03-08 13:43:06 UTC
Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Using index info to reconstruct a base tree...
M	C4/Circulation.pm
M	installer/data/mysql/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging C4/Circulation.pm
Auto packing the repository in background for optimum performance.
See "git help gc" for manual housekeeping.
Comment 6 Nicole C. Engard 2016-03-08 14:00:35 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-03-11 09:27:19 UTC
I have added the Koha::Patron->guarantees method for bug 15656. Do you mind rebasing this new enhancement on top of it?
I have provided much more tests and it has reached the QA queue 1 month ago.
Comment 8 Kyle M Hall 2016-03-11 15:47:25 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2016-03-14 17:28:58 UTC
Comment on attachment 49036 [details] [review]
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees

Review of attachment 49036 [details] [review]:
-----------------------------------------------------------------

- circ/circulation_batch_checkouts.tt will need to be modified too

- The tests in Patron.t are not necessary. It's covered by t/db_dependent/Koha/Patrons.t

::: C4/Circulation.pm
@@ +852,5 @@
>      my $allfinesneedoverride = C4::Context->preference("AllFinesNeedOverride");
> +
> +    # Check the debt of this patrons guarantees
> +    my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees");
> +    $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees );

Is there a difference with 'defined $a && $a > 0'?

@@ +867,5 @@
> +            $issuingimpossible{DEBT_GUARANTEES} = sprintf( "%.2f", $guarantees_non_issues_charges );
> +        } elsif ( $guarantees_non_issues_charges > $no_issues_charge_guarantees && !$inprocess && $allowfineoverride) {
> +            $needsconfirmation{DEBT_GUARANTEES} = sprintf( "%.2f", $guarantees_non_issues_charges );
> +        } elsif ( $allfinesneedoverride && $guarantees_non_issues_charges > 0 && $guarantees_non_issues_charges <= $no_issues_charge_guarantees && !$inprocess ) {
> +            $needsconfirmation{DEBT_GUARANTEES} = sprintf( "%.2f", $guarantees_non_issues_charges );

It would be better to use the Price TT plugin to display these values.
Comment 10 Kyle M Hall 2016-03-16 18:09:39 UTC Comment hidden (obsolete)
Comment 11 Kyle M Hall 2016-03-16 18:09:48 UTC Comment hidden (obsolete)
Comment 12 Kyle M Hall 2016-03-16 18:11:19 UTC
(In reply to Jonathan Druart from comment #9)
 
> - circ/circulation_batch_checkouts.tt will need to be modified too

Fixed!
 
> - The tests in Patron.t are not necessary. It's covered by
> t/db_dependent/Koha/Patrons.t

Fixed!

> ::: C4/Circulation.pm
> @@ +852,5 @@
> >      my $allfinesneedoverride = C4::Context->preference("AllFinesNeedOverride");
> > +
> > +    # Check the debt of this patrons guarantees
> > +    my $no_issues_charge_guarantees = C4::Context->preference("NoIssuesChargeGuarantees");
> > +    $no_issues_charge_guarantees = undef unless looks_like_number( $no_issues_charge_guarantees );
> 
> Is there a difference with 'defined $a && $a > 0'?

It's a safety check in case some puts a non-number in the system preference ( since it's free text ).

> @@ +867,5 @@
> > +            $issuingimpossible{DEBT_GUARANTEES} = sprintf( "%.2f", $guarantees_non_issues_charges );
> > +        } elsif ( $guarantees_non_issues_charges > $no_issues_charge_guarantees && !$inprocess && $allowfineoverride) {
> > +            $needsconfirmation{DEBT_GUARANTEES} = sprintf( "%.2f", $guarantees_non_issues_charges );
> > +        } elsif ( $allfinesneedoverride && $guarantees_non_issues_charges > 0 && $guarantees_non_issues_charges <= $no_issues_charge_guarantees && !$inprocess ) {
> > +            $needsconfirmation{DEBT_GUARANTEES} = sprintf( "%.2f", $guarantees_non_issues_charges );
> 
> It would be better to use the Price TT plugin to display these values.

Fixed!
Comment 13 Jonathan Druart 2016-03-18 10:10:59 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2016-03-18 10:11:05 UTC Comment hidden (obsolete)
Comment 15 Brendan Gallagher 2016-03-31 20:27:36 UTC
Didn't apply cleanly
Comment 16 Kyle M Hall 2016-04-18 17:05:53 UTC
(In reply to Brendan Gallagher from comment #15)
> Didn't apply cleanly

Odd, it seems to apply cleanly now!
Comment 17 Kyle M Hall 2016-04-27 16:51:52 UTC
Created attachment 50834 [details] [review]
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees

This enhancment allows a library to prevent patrons from checking out
items if his or her guarantees own too much.

Test Plan:
1) Apply this patch
2) Find or create a patron with a guarantor
3) Add a fine to the patron's account
4) Set the new system preference NoIssuesChargeGuarantees to be less
   than the amount owed by the patron
4) Attempt to check out an item to the guarantor, you will either
   be warned or prevented from checking out based on your system
   settings.

Signed-off-by: Cathi Wiggin <CWIGGINS@arcadiaca.gov>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Kyle M Hall 2016-04-27 16:51:59 UTC
Created attachment 50835 [details] [review]
Bug 14577 [QA Followup]

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Kyle M Hall 2016-04-29 11:54:53 UTC
Pushed to master for the May 2016 release!