Attachment 48679 Details for Bug 11565 – Bug 11565 - decreaseLoanHighHolds needs Override

[patch] Bug 11565 - decreaseLoanHighHolds needs Override

Bug-11565---decreaseLoanHighHolds-needs-Override.patch (text/plain), 11.99 KB, created by Kyle M Hall (khall) on 2016-03-04 12:49:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2016-03-04 12:49:22 UTC

Size: 11.99 KB

patch

obsolete

>From eeeb31869c7ecc4f56cb47956c882effa0672b09 Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Wed, 30 Sep 2015 11:48:14 -0400
>Subject: [PATCH] Bug 11565 - decreaseLoanHighHolds needs Override
>
>This patch allows the high holds loan length decrease to be overridden
>either by a checkbox that remembers its setting during a series of
>checkouts, or by a one time use override checkbox that will show
>in the warning popup if a checkout is affected by high holds.
>
>Test Plan:
>1) Set up a checkout that will be affected by decreaseLoanHighHolds
>2) Attempt to check out an item
>3) Check the override checkbox
>4) Note the checkout date is not reduced
>5) Return the item
>6) Start a new checkout for the patron
>7) Check the "Don't decrease lean length based on holds" checkbox
>8) Check out the item
>9) Note you are not warned about the high holds decrease
>   and that the checkout length is not reduced
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
>---
> C4/Circulation.pm                                  | 30 ++++++++---
> circ/circulation.pl                                | 17 ++++++-
> .../prog/en/modules/circ/circulation.tt            | 59 ++++++++++++++++++----
> t/db_dependent/DecreaseLoanHighHolds.t             | 20 ++++++--
> 4 files changed, 103 insertions(+), 23 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index d827b0a..ef91dab 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -660,7 +660,7 @@ sub itemissues {
> =head2 CanBookBeIssued
> 
>   ( $issuingimpossible, $needsconfirmation ) =  CanBookBeIssued( $borrower, 
>-                      $barcode, $duedate, $inprocess, $ignore_reserves );
>+                      $barcode, $duedate, $inprocess, $ignore_reserves, $params );
> 
> Check if a book can be issued.
> 
>@@ -678,6 +678,12 @@ C<$issuingimpossible> and C<$needsconfirmation> are some hashref.
> 
> =item C<$ignore_reserves> boolean switch
> 
>+=item C<$params> Hashref of additional parameters
>+
>+Available keys:
>+    override_high_holds - Ignore high holds
>+    onsite_checkout     - Checkout is an onsite checkout that will not leave the library
>+
> =back
> 
> Returns :
>@@ -758,7 +764,8 @@ sub CanBookBeIssued {
>     my %issuingimpossible;    # filled with problems that causes the issue to be IMPOSSIBLE
>     my %alerts;               # filled with messages that shouldn't stop issuing, but the librarian should be aware of.
> 
>-    my $onsite_checkout = $params->{onsite_checkout} || 0;
>+    my $onsite_checkout     = $params->{onsite_checkout}     || 0;
>+    my $override_high_holds = $params->{override_high_holds} || 0;
> 
>     my $item = GetItem(GetItemnumberFromBarcode( $barcode ));
>     my $issue = GetItemIssue($item->{itemnumber});
>@@ -1074,11 +1081,20 @@ sub CanBookBeIssued {
>         my $check = checkHighHolds( $item, $borrower );
> 
>         if ( $check->{exceeded} ) {
>-            $needsconfirmation{HIGHHOLDS} = {
>-                num_holds  => $check->{outstanding},
>-                duration   => $check->{duration},
>-                returndate => output_pref( $check->{due_date} ),
>-            };
>+            if ($override_high_holds) {
>+                $alerts{HIGHHOLDS} = {
>+                    num_holds  => $check->{outstanding},
>+                    duration   => $check->{duration},
>+                    returndate => output_pref( $check->{due_date} ),
>+                };
>+            }
>+            else {
>+                $needsconfirmation{HIGHHOLDS} = {
>+                    num_holds  => $check->{outstanding},
>+                    duration   => $check->{duration},
>+                    returndate => output_pref( $check->{due_date} ),
>+                };
>+            }
>         }
>     }
> 
>diff --git a/circ/circulation.pl b/circ/circulation.pl
>index ea07e29..e2381f0 100755
>--- a/circ/circulation.pl
>+++ b/circ/circulation.pl
>@@ -62,6 +62,9 @@ my $query = new CGI;
> my $sessionID = $query->cookie("CGISESSID") ;
> my $session = get_session($sessionID);
> 
>+my $override_high_holds     = $query->param('override_high_holds');
>+my $override_high_holds_tmp = $query->param('override_high_holds_tmp');
>+
> # branch and printer are now defined by the userenv
> # but first we have to check if someone has tried to change them
> 
>@@ -332,8 +335,17 @@ if (@$barcodes) {
>   for my $barcode ( @$barcodes ) {
>     my $template_params = { barcode => $barcode };
>     # always check for blockers on issuing
>-    my ( $error, $question, $alerts ) =
>-    CanBookBeIssued( $borrower, $barcode, $datedue , $inprocess, undef, { onsite_checkout => $onsite_checkout } );
>+    my ( $error, $question, $alerts ) = CanBookBeIssued(
>+        $borrower,
>+        $barcode, $datedue,
>+        $inprocess,
>+        undef,
>+        {
>+            onsite_checkout     => $onsite_checkout,
>+            override_high_holds => $override_high_holds || $override_high_holds_tmp || 0,
>+        }
>+    );
>+
>     my $blocker = $invalidduedate ? 1 : 0;
> 
>     $template_params->{alert} = $alerts;
>@@ -639,6 +651,7 @@ $template->param(
>     canned_bor_notes_loop     => $canned_notes,
>     debarments                => GetDebarments({ borrowernumber => $borrowernumber }),
>     todaysdate                => output_pref( { dt => dt_from_string()->set(hour => 23)->set(minute => 59), dateformat => 'sql' } ),
>+    override_high_holds       => $override_high_holds,
> );
> 
> output_html_with_http_headers $query, $cookie, $template->output;
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>index 36c44f5..51e3275 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>@@ -207,6 +207,10 @@ $(document).ready(function() {
>     <div class="dialog message">The patron has unpaid charges for holds, rentals etc of [% alert.OTHER_CHARGES %]</div>
> [% END %]
> 
>+[% IF alert.HIGHHOLDS %]
>+    <div class="dialog message">High demand item. Loan period <i>not</i> shortened to [% alert.HIGHHOLDS.duration %] days (due [% alert.HIGHHOLDS.returndate %]) due to override.</div>
>+[% END %]
>+
> [% IF ( NEEDSCONFIRMATION ) %]
> <div class="yui-g">
> 
>@@ -299,7 +303,7 @@ $(document).ready(function() {
>     </li>
> [% END %]
> 
>-[% IF  HIGHHOLDS %]
>+[% IF HIGHHOLDS %]
>     <li>High demand item. Loan period shortened to [% HIGHHOLDS.duration %] days (due [% HIGHHOLDS.returndate %]). Check out anyway?</li>
> [% END %]
> 
>@@ -314,16 +318,24 @@ $(document).ready(function() {
> </ul>
> 
> [% IF HIGHHOLDS %]
>-	<script language="JavaScript" type="text/javascript">
>-	$(document).ready(function() {
>-            $("input[name=duedatespec]:hidden").val('[% HIGHHOLDS.returndate %]');
>-            if ('[% duedatespec %]' === '') {
>-                $("input[name=restoreduedatespec]:hidden").val('highholds_empty');
>-            } else {
>-                $("input[name=restoreduedatespec]:hidden").val('[% duedatespec %]');
>-            }
>-	});
>-	</script>
>+<script language="JavaScript" type="text/javascript">
>+$(document).ready(function() {
>+    [% IF !override_high_holds %]
>+        $("input[name=duedatespec]:hidden").val('[% HIGHHOLDS.returndate %]');
>+        if ('[% duedatespec %]' === '') {
>+            $("input[name=restoreduedatespec]:hidden").val('highholds_empty');
>+        } else {
>+            $("input[name=restoreduedatespec]:hidden").val('[% duedatespec %]');
>+        }
>+    [% END %]
>+
>+    $("#override_high_holds_tmp").on( 'change', function() {
>+        if ( this.checked ) {
>+            $("input[name=duedatespec]:hidden").val('');
>+        }
>+    });
>+});
>+</script>
> [% END %]
> 
> [% IF CAN_user_circulate_force_checkout or HIGHHOLDS %]
>@@ -332,6 +344,13 @@ $(document).ready(function() {
> 
> [% IF (forceallow) %]<input type="hidden" name="forceallow" value="1">[% END %]
> 
>+[% IF HIGHHOLDS %]
>+    <p>
>+    <input type="checkbox" name="override_high_holds_tmp" id="override_high_holds_tmp" value="1" />
>+    <label for="override_high_holds_tmp">Don't decrease loan length based on holds</label>
>+    </p>
>+[% END %]
>+
> [% IF ( RESERVED ) %]
>     <p>
>     <input type="checkbox" id="cancelreserve" name="cancelreserve" value="cancel" />
>@@ -351,6 +370,7 @@ $(document).ready(function() {
>     <input type="hidden" name="barcode" value="[% barcode |html %]" />
>     <input type="hidden" name="borrowernumber" value="[% borrowernumber %]" />
>     <input type="hidden" name="issueconfirmed" value="1" />
>+    <input type="hidden" name="override_high_holds" value="[% override_high_holds %]"/>
>     [% IF ( DEBT ) %]<input type="hidden" name="debt_confirmed" value="1" />[% END %]
>     [% IF ( INVALID_DATE ) %]
>     <p>
>@@ -647,6 +667,23 @@ No patron matched <span class="ex">[% message %]</span>
>             [% END %]
> 
>             <label for="auto_renew">Automatic renewal</label>
>+
>+            [% IF Koha.Preference('decreaseLoanHighHolds') %]
>+                [% IF NEEDSCONFIRMATION %]
>+                    [% IF override_high_holds %]
>+                        <input type="checkbox" name="override_high_holds" id="override_high_holds" value="1" disabled="disabled" checked="checked"/>
>+                    [% ELSE %]
>+                        <input type="checkbox" name="override_high_holds" id="override_high_holds" value="1" disabled="disabled"/>
>+                    [% END %]
>+                [% ELSE %]
>+                    [% IF override_high_holds %]
>+                        <input type="checkbox" name="override_high_holds" id="override_high_holds" value="1" checked="checked" />
>+                    [% ELSE %]
>+                        <input type="checkbox" name="override_high_holds" id="override_high_holds" value="1" />
>+                    [% END %]
>+                [% END %]
>+                <label for="override_high_holds">Don't decrease loan length based on holds</label>
>+            [% END %]
>         </div>
>     [% END %]
> 
>diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t
>index e13917a..d42ab4a 100755
>--- a/t/db_dependent/DecreaseLoanHighHolds.t
>+++ b/t/db_dependent/DecreaseLoanHighHolds.t
>@@ -26,7 +26,7 @@ use Koha::Holds;
> use Koha::Hold;
> use t::lib::TestBuilder;
> 
>-use Test::More tests => 12;
>+use Test::More tests => 14;
> 
> my $dbh    = C4::Context->dbh;
> my $schema = Koha::Database->new()->schema();
>@@ -63,7 +63,13 @@ my $biblioitem =
> 
> my @items;
> for my $i ( 1 .. 10 ) {
>-    my $item = Koha::Item->new( { biblionumber => $biblio->id(), biblioitemnumber => $biblioitem->id(), } )->store();
>+    my $item = Koha::Item->new(
>+        {
>+            biblionumber     => $biblio->id(),
>+            biblioitemnumber => $biblioitem->id(),
>+            barcode          => $i
>+        }
>+    )->store();
>     push( @items, $item );
> }
> 
>@@ -101,7 +107,7 @@ C4::Context->set_preference( 'decreaseLoanHighHoldsValue',          1 );
> C4::Context->set_preference( 'decreaseLoanHighHoldsControl',        'static' );
> C4::Context->set_preference( 'decreaseLoanHighHoldsIgnoreStatuses', 'damaged,itemlost,notforloan,withdrawn' );
> 
>-my $item_hr = { itemnumber => $item->id, biblionumber => $biblio->id, homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode} };
>+my $item_hr = { itemnumber => $item->id, biblionumber => $biblio->id, homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, barcode => $item->barcode };
> my $patron_hr = { borrower => $patron->id, branchcode => $library->{branchcode} };
> 
> my $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr );
>@@ -160,5 +166,13 @@ $unholdable->store();
> $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr );
> is( $data->{exceeded}, 1, "Should exceed threshold with one withdrawn item" );
> 
>+C4::Context->set_preference('CircControl', 'PatronLibrary');
>+
>+my ( undef, $needsconfirmation ) = CanBookBeIssued( $patron_hr, $item->barcode );
>+ok( $needsconfirmation->{HIGHHOLDS}, "High holds checkout needs confirmation" );
>+
>+( undef, $needsconfirmation ) = CanBookBeIssued( $patron_hr, $item->barcode, undef, undef, undef, { override_high_holds => 1 } );
>+ok( !$needsconfirmation->{HIGHHOLDS}, "High holds checkout does not need confirmation" );
>+
> $schema->storage->txn_rollback();
> 1;
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 11565: 42976 | 43160 | 43600 | 48679 | 49409 | 49937 | 50798 | 50799 | 50800 | 50815 | 50817 | 50818 | 50820 | 50821 | 50822 | 50828 | 50829