Bug 14577

Summary: Allow restriction of checkouts based on fines of guarantor/guarantee
Product: Koha Reporter: Jesse Weaver <jweaver>
Component: CirculationAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: amy, brendan, gmcharlt, gwilliams, jonathan.druart, kyle.m.hall, kyle, nicole
Version: master   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 15656    
Bug Blocks: 14570, 16405, 19382    
Attachments: Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Bug 14577 [QA Followup]
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Bug 14577 [QA Followup]
Bug 14577 - Allow restriction of checkouts based on fines of guarantor's guarantees
Bug 14577 [QA Followup]

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!