Attachment 82220 Details for Bug 15565 – Bug 15565: (follow-up) Moved function form C4/Reserves to Koha/Biblio

[patch] Bug 15565: (follow-up) Moved function form C4/Reserves to Koha/Biblio

Bug-15565-follow-up-Moved-function-form-C4Reserves.patch (text/plain), 12.90 KB, created by Alex Buckley on 2018-11-12 10:11:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Buckley

Created: 2018-11-12 10:11:57 UTC

Size: 12.90 KB

patch

obsolete

>From ac2cf5f0120bd53111cfeef9488b75119157c268 Mon Sep 17 00:00:00 2001
>From: Alex Buckley <alexbuckley@catalyst.net.nz>
>Date: Mon, 12 Nov 2018 09:51:18 +0000
>Subject: [PATCH] Bug 15565: (follow-up) Moved function form C4/Reserves to
> Koha/Biblio
>
>Also updated t/db_dependen/Reserves/MultiplePerRecord.t test to reflect
>moving function.
>
>Added the display of the JS modal box informing OPAC borrower that
>they have exceeded hold limit after they click 'A specific item'
>radiobutton on opac-reserve if holds per day, holds per record or
>maxreserves is 1
>
>Test plan:
>1. Apply patches
>2. Restart memcached and plack
>3. Run t/db_dependent/Reserves/MultiplePerRecord.t
>4. Set your maxreserves syspref to 3, and a circ rules holds per record
>to 2 and holds per day to 1
>5. In the OPAC visit the 'Place hold' interface of item. Select the 'A
>specific item' radiobutton and observe a modal box loads to inform you
>that you have reached the maximum holds allowed today.
>6. Click 'OK' on the modal and notice the first checkbox is selected and
>other unchecked checkboxes are disabled.
>7. Repeat step 4 this time setting holds per day to 5
>8. Now reloading the OPAC 'Place hold' page notice after clicking 'A
>specific item' no modal displays and select another item checkbox (so
>you have 2 selected checkboxes). Observe a modal saying you have reached
>the maximum number of holds for the record is loaded
>9. Click 'OK' on modal and observe all unchecked item checkboxes are
>automatically disabled to prevent additional holds being placed
>10. Select 'Confirm hold'
>11. On your holds summary page confirm both holds are placed
>12. In the staff client set the 'AllowHoldPolicyOverride' syspref to
>"Don't allow"
>13. Visit a biblio reservation interface in the staff client
>14. Select 2 item checkboxes and observe the modal box explaining you
>have reached maximum holds for the record is displayed.
>15. Select 'OK' on the modal and notice all un-checked item checkboxes
>are automatically disabled
>16. Repeat steps 12-15 this time with the value of
>'AllowHoldPolicyOverride' syspref set to 'Allow' and notice that once
>you have clicked on the second item checkbox although the modal still
>loads the item checkboxes do not disable - you are able to override and
>continue selecting checkboxes each time with modal warning loading.
>
>17. Observe the allow_holds() function is in the Koha/Biblio.pm file and
>the GetAllowedHoldsForPatronToday() function has been removed from
>C4/Reserves.pm
>
>Sponsored-By: Brimbank Library, Australia
>---
> C4/Reserves.pm                                     | 34 ------------------
> Koha/Biblio.pm                                     | 31 +++++++++++++++++
> .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 40 ++++++++++++++--------
> opac/opac-reserve.pl                               |  4 ++-
> reserve/request.pl                                 |  5 ++-
> t/db_dependent/Reserves/MultiplePerRecord.t        |  9 +++--
> 6 files changed, 69 insertions(+), 54 deletions(-)
>
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index 15b79a5..77e4213 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -140,7 +140,6 @@ BEGIN {
>         IsItemOnHoldAndFound
> 
>         GetMaxPatronHoldsForRecord
>-        GetAllowedHoldsForPatronToday
>     );
>     @EXPORT_OK = qw( MergeHolds );
> }
>@@ -2122,39 +2121,6 @@ sub GetMaxPatronHoldsForRecord {
>     return $max;
> }
> 
>-=head2 GetAllowedHoldsForPatronToday
>-
>-my $holds_allowed_on_record_today = GetAllowedHoldsForPatronToday( $patron->borrowernumber, $biblionumber );
>-
>-Calculates and returns the number of item-level holds a borrower is allowed to place on the record on a single day.
>-
>-=cut
>-
>-sub GetAllowedHoldsForPatronToday {
>-    my ( $borrowernumber, $biblionumber ) = @_;
>-
>-    my $patron = Koha::Patrons->find($borrowernumber);
>-    my @items = Koha::Items->search( { biblionumber => $biblionumber } );
>-
>-    my $controlbranch = C4::Context->preference('ReservesControlBranch');
>-
>-    my $categorycode = $patron->categorycode;
>-    my $branchcode;
>-    $branchcode = $patron->branchcode if ( $controlbranch eq "PatronLibrary" );
>-
>-    my $holds_allowed = 0;
>-    foreach my $item (@items) {
>-        my $itemtype = $item->effective_itemtype();
>-
>-        $branchcode = $item->homebranch if ( $controlbranch eq "ItemHomeLibrary" );
>-
>-        my $rule = GetHoldRule( $categorycode, $itemtype, $branchcode );
>-        my $holds_per_day = $rule ? $rule->{holds_per_day} : 0;
>-        $holds_allowed = $holds_per_day if $holds_per_day > $holds_allowed;
>-    }
>-    return $holds_allowed;
>-}
>-
> =head2 GetHoldRule
> 
> my $rule = GetHoldRule( $categorycode, $itemtype, $branchcode );
>diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm
>index 1dca4d9..5bd7fe0 100644
>--- a/Koha/Biblio.pm
>+++ b/Koha/Biblio.pm
>@@ -425,6 +425,37 @@ sub has_items_waiting_or_intransit {
>     return 0;
> }
> 
>+=head3 allowed_holds
>+
>+    my $holds_allowed_on_record_today = $biblio->allowed_holds({ patron => $patron_obj );
>+
>+    Calculates and returns the number of item-level holds a borrower is allowed to place on the record on a single day.
>+
>+=cut
>+
>+sub allowed_holds {
>+    my ( $self, $patron ) = @_;
>+
>+    my $controlbranch = C4::Context->preference('ReservesControlBranch');
>+
>+    my $categorycode = $patron->categorycode;
>+    my $branchcode;
>+    $branchcode = $patron->branchcode if ( $controlbranch eq "PatronLibrary" );
>+
>+    my $holds_allowed = 0;
>+    foreach my $item ( $self->items()->as_list() ) {
>+       my $itemtype = $item->effective_itemtype();
>+       $branchcode = $item->homebranch if ( $controlbranch eq "ItemHomeLibrary" );
>+
>+       my $rule = C4::Reserves::GetHoldRule( $categorycode, $itemtype, $branchcode );
>+       my $holds_per_day = $rule ? $rule->{holds_per_day} : 0;
>+       if ( $holds_per_day ) {
>+            $holds_allowed = $holds_per_day if $holds_per_day > $holds_allowed;
>+       }
>+    }
>+    return $holds_allowed;
>+}
>+
> =head3 type
> 
> =cut
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>index e46609f..57c5bf8 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt
>@@ -463,7 +463,9 @@
> <script>
> // <![CDATA[
>     var MSG_NO_ITEM_SELECTED = _("Expecting a specific item selection.");
>-    var MSG_MAX_HOLDS_EXCEEDED = _("Maximum number of reserve exceded.");
>+    var MSG_MAX_HOLDS_EXCEEDED = _("Maximum number of reserves for this record reached.");
>+    var MSG_EXCEEDED_HOLDS_PER_DAY =  _("Maximum number of reserves allowed today reached.");
>+    var MSG_EXCEEDED_MAXRESERVES_SYSPREF = _("Maximum number of holds allowed at once reached");
> 
>         // Clear the contents of an input field
>         $(".clearfield").on("click",function(e){
>@@ -475,20 +477,6 @@
>     function select_first_available(id){
>         var checkboxes = $("input:checkbox[name='checkitem_" + id + "']");
>         $(checkboxes).first().attr("checked", "checked");
>-
>-        var nbCheked = 1;
>-        var remaining = $("input[id='remaining_holds_"+id+"']").val();
>-        var remaining_holds_today = $("input[id='remaining_holds_allowed_today"+id+"']").val();
>-        var maxreserves = "[% maxreserves | html %]";
>-        if (nbCheked == remaining) {
>-            $("input[name='checkitem_"+id+"']:not(:checked)").attr('disabled', 'disabled');
>-        }
>-        else if (nbCheked == remaining_holds_today) {
>-            $("input[name='checkitem_"+id+"']:not(:checked)").attr('disabled', 'disabled');
>-        }
>-        else if (nbCheked == maxreserves) {
>-            $("input[name='checkitem_"+id+"']:not(:checked)").attr('disabled', 'disabled');
>-        }
>     }
> 
>     $(document).ready(function() {
>@@ -516,12 +504,15 @@
>             var remaining_holds_today = $("input[id='remaining_holds_allowed_today"+bibNum+"']").val();
>             var maxreserves = "[% maxreserves | html %]";
>             if (nbCheked >= remaining) {
>+                alert(MSG_MAX_HOLDS_EXCEEDED);
>                 $("input[name='checkitem_"+bibNum+"']:not(:checked)").attr('disabled', 'disabled');
>             }
>             else if (nbCheked >= remaining_holds_today) {
>+                alert(MSG_EXCEEDED_HOLDS_PER_DAY);
>                 $("input[name='checkitem_"+bibNum+"']:not(:checked)").attr('disabled', 'disabled');
>             }
>             else if (nbCheked >= maxreserves) {
>+                alert(MSG_EXCEEDED_MAXRESERVES_SYSPREF);
>                 $("input[name='checkitem_"+bibNum+"']:not(:checked)").attr('disabled', 'disabled');
>             }
>             else {
>@@ -584,6 +575,25 @@
>             if ( $(checkbox).is(":checked") && select_specific ) {
>                 $(newCopiesRowId).show();
>                 $(holdsToPlaceCountId).hide();
>+
>+                //Check if maximum holds is reached and if it has then disable un-checked item checkboxes and display message
>+                var nbCheked = 1;
>+                var remaining = $("input[id='remaining_holds_"+biblioNum+"']").val();
>+                var remaining_holds_today = $("input[id='remaining_holds_allowed_today"+biblioNum+"']").val();
>+                var maxreserves = "[% maxreserves | html %]";
>+                if (nbCheked == remaining) {
>+                   alert(MSG_MAX_HOLDS_EXCEEDED);
>+                   $("input[name='checkitem_"+biblioNum+"']:not(:checked)").attr('disabled', 'disabled');
>+                }
>+                else if (nbCheked == remaining_holds_today) {
>+                   alert(MSG_EXCEEDED_HOLDS_PER_DAY);
>+                   $("input[name='checkitem_"+biblioNum+"']:not(:checked)").attr('disabled', 'disabled');
>+                }
>+                else if (nbCheked == maxreserves) {
>+                   alert(MSG_EXCEEDED_MAXRESERVES_SYSPREF);
>+                   $("input[name='checkitem_"+biblioNum+"']:not(:checked)").attr('disabled', 'disabled');
>+                }
>+
>             } else {
>                 $(newCopiesRowId).hide();
>                 $(holdsToPlaceCountId).show();
>diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
>index f9e4817..ecb40f6 100755
>--- a/opac/opac-reserve.pl
>+++ b/opac/opac-reserve.pl
>@@ -630,7 +630,9 @@ foreach my $biblioNum (@biblionumbers) {
>        reservedate    => dt_from_string->date
>     });
> 
>-    my $holds_allowed_on_record_today = GetAllowedHoldsForPatronToday( $borrowernumber, $biblioNum );
>+    my $biblio_record = Koha::Biblios->find( $biblioNum );
>+    my $patron_obj = Koha::Patrons->find( $borrowernumber );
>+    my $holds_allowed_on_record_today = $biblio_record->allowed_holds( $patron_obj );
>     my $remaining_holds_allowed_today = $holds_allowed_on_record_today - $today_holds->count();
> 
>     $biblioLoopIter{remaining_holds_for_record} = $remaining_holds_for_record;
>diff --git a/reserve/request.pl b/reserve/request.pl
>index d5fd6de..fbb9ad5 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -267,7 +267,10 @@ foreach my $biblionumber (@biblionumbers) {
>             reservedate    => dt_from_string->date
>         });
> 
>-        my $holds_allowed_on_record_today = GetAllowedHoldsForPatronToday( $patron->borrowernumber, $biblionumber );
>+
>+        my $biblio_record = Koha::Biblios->find( $biblionumber );
>+        my $patron_obj = Koha::Patrons->find( $patron->borrowernumber );
>+        my $holds_allowed_on_record_today = $biblio_record->allowed_holds( $patron_obj );
>         my $remaining_holds_allowed_today = $holds_allowed_on_record_today - $today_holds->count();
> 
>         my $maxreserves = C4::Context->preference('maxreserves');
>diff --git a/t/db_dependent/Reserves/MultiplePerRecord.t b/t/db_dependent/Reserves/MultiplePerRecord.t
>index 61f5833..f3a09a4 100755
>--- a/t/db_dependent/Reserves/MultiplePerRecord.t
>+++ b/t/db_dependent/Reserves/MultiplePerRecord.t
>@@ -24,7 +24,7 @@ use Test::More tests => 42;
> use t::lib::TestBuilder;
> use t::lib::Mocks;
> 
>-use C4::Reserves qw( GetMaxPatronHoldsForRecord GetAllowedHoldsForPatronToday AddReserve CanBookBeReserved );
>+use C4::Reserves qw( GetMaxPatronHoldsForRecord AddReserve CanBookBeReserved );
> use Koha::Database;
> use Koha::Holds;
> 
>@@ -205,7 +205,10 @@ my $rule4 = $rules_rs->new(
> $max = GetMaxPatronHoldsForRecord( $patron->{borrowernumber}, $biblio->{biblionumber} );
> is( $max, 4, 'GetMaxPatronHoldsForRecord returns max of 4' );
> 
>-my $holds_per_day = GetAllowedHoldsForPatronToday( $patron->{borrowernumber}, $biblio->{biblionumber} );
>+my $biblio_record = Koha::Biblios->find( $biblio->{biblionumber} );
>+my $patron_obj = Koha::Patrons->find( $patron->{borrowernumber} );
>+
>+my $holds_per_day = $biblio_record->allowed_holds( $patron_obj );
> is( $holds_per_day, 4, 'GetAllowedHoldsForPatronToday returns max of 4' );
> 
> $rule = C4::Reserves::GetHoldRule(
>@@ -234,7 +237,7 @@ my $rule5 = $rules_rs->new(
> $max = GetMaxPatronHoldsForRecord( $patron->{borrowernumber}, $biblio->{biblionumber} );
> is( $max, 5, 'GetMaxPatronHoldsForRecord returns max of 1' );
> 
>-$holds_per_day = GetAllowedHoldsForPatronToday( $patron->{borrowernumber}, $biblio->{biblionumber} );
>+$holds_per_day = $biblio_record->allowed_holds( $patron_obj );
> is( $holds_per_day, 5, 'GetAllowedHoldsForPatronToday returns max of 5' );
> 
> 
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 15565: 47527 | 47551 | 47552 | 50068 | 50069 | 50392 | 50393 | 56229 | 56230 | 56231 | 56232 | 56458 | 56510 | 56511 | 56512 | 56513 | 56514 | 56515 | 56517 | 56518 | 56519 | 56520 | 56521 | 56522 | 56523 | 56524 | 61244 | 61245 | 61246 | 61247 | 61248 | 61576 | 61577 | 61578 | 61579 | 61580 | 64590 | 64591 | 64592 | 64593 | 64594 | 66594 | 66595 | 66596 | 66597 | 66598 | 68759 | 68760 | 68761 | 68762 | 68763 | 71643 | 71644 | 71645 | 71646 | 71647 | 71648 | 72644 | 72645 | 72646 | 72647 | 72648 | 72649 | 78242 | 78243 | 78244 | 78245 | 78246 | 78247 | 78248 | 78266 | 78305 | 78417 | 78418 | 78419 | 78420 | 78421 | 78422 | 78423 | 78424 | 78425 | 78426 | 78712 | 78713 | 78714 | 78715 | 78716 | 78717 | 78718 | 78719 | 78720 | 78815 | 78816 | 78817 | 78818 | 78819 | 78820 | 78821 | 78822 | 78823 | 81419 | 81420 | 81421 | 81422 | 81423 | 81424 | 81425 | 81426 | 81427 | 81431 | 81449 | 81570 | 81571 | 81572 | 81573 | 81574 | 81575 | 81576 | 81577 | 81578 | 81579 | 81580 | 81581 | 82205 | 82206 | 82207 | 82208 | 82209 | 82210 | 82211 | 82212 | 82213 | 82214 | 82215 | 82216 | 82220 | 82221 | 83331 | 83332 | 83333 | 83334 | 83335 | 83336 | 83337 | 83338 | 83339 | 83340 | 83341 | 83342 | 83378 | 83380 | 83381 | 84541 | 84542 | 84543 | 84544 | 84545 | 84546 | 84547 | 84548 | 84549 | 84550 | 84551 | 84552 | 84553 | 87979 | 87980 | 87981 | 87982 | 87983 | 87984 | 87985 | 87986 | 87987 | 87988 | 87989 | 87990 | 87991 | 117128 | 117129 | 117130 | 117131 | 117132 | 117133 | 120365 | 120366 | 120367 | 120639 | 120640 | 120641 | 121941 | 122098 | 162071 | 162072 | 162073 | 162074 | 163052 | 163053 | 163054 | 163055 | 163092 | 163093 | 166005 | 166006 | 166007 | 166008 | 166009