Bugzilla – Attachment 32551 Details for
Bug 13116
Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates.
Bug-13116---Make-it-possible-to-propagate-errors-f.patch (text/plain), 11.98 KB, created by
Olli-Antti Kivilahti
on 2014-10-21 12:06:26 UTC
(
hide
)
Description:
Bug 13116 - Make it possible to propagate errors from C4::Reserves::CanItemBeReserved() to the web-templates.
Filename:
MIME Type:
Creator:
Olli-Antti Kivilahti
Created:
2014-10-21 12:06:26 UTC
Size:
11.98 KB
patch
obsolete
>From 9a37568a4d49ab13bb5358a1922da1d7f0bda08b Mon Sep 17 00:00:00 2001 >From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> >Date: Mon, 20 Oct 2014 19:04:51 +0300 >Subject: [PATCH] Bug 13116 - Make it possible to propagate errors from > C4::Reserves::CanItemBeReserved() to the web-templates. > >This patch changes the way CanBookBeReserved() and CanItemBeReserved() return error >messages and how they are dealt with in the templates. This change makes it possible >to distinguish between different types of reservation failure. > >Currently only two types of errors are handled, all the way to the user, from the CanItemBeReserved(): >-ageRestricted >-tooManyReserves which translates to maxreserves > > ############# > - TEST PLAN - > ############# >((-- AGE RESTRICTION --)) >STAFF CLIENT >1. Find a Record with Items, update the MARC Subfield 521a to "PEGI 16". >2. Get a Borrower who is younger than 16 years. >3. Place a hold for the underage Borrower for the ageRestricted Record. >4. You get a notification, that placing a hold on ageRestricted material is > forbidden. (previously you just got a notification about maximum amount of reserves reached) > >OPAC >A. In OPAC go to opac-detail.pl for the ageRestricted Biblio. > The "Place Hold" -button is missing on the right sidebar if the underage > user is logged in. >B. From the opac-search.pl a hold can still be placed, but it will fail giving > an error about age restrictions. > >((-- MAXIMUM RESERVES REACHED --)) >0. Set the maxreserves -syspref to 3 (or any low value) >STAFF CLIENT AND OPAC >1. Make a ton of reserves for one borrower. >2. Observe the notification about maximum reserves reached blocking your reservations. >--- > C4/Reserves.pm | 30 +++++++++++++------- > .../prog/en/modules/reserve/request.tt | 5 +++- > .../opac-tmpl/bootstrap/en/modules/opac-reserve.tt | 6 ++++ > opac/opac-detail.pl | 4 +-- > opac/opac-reserve.pl | 10 ++++++- > reserve/request.pl | 19 ++++++++++--- > t/db_dependent/Reserves.t | 6 ++-- > 7 files changed, 59 insertions(+), 21 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 97990ac..19a8e97 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -448,7 +448,10 @@ sub GetReservesFromBorrowernumber { > #------------------------------------------------------------------------------------- > =head2 CanBookBeReserved > >- $error = &CanBookBeReserved($borrowernumber, $biblionumber) >+ $canReserve = &CanBookBeReserved($borrowernumber, $biblionumber) >+ if ($canReserve eq 'OK') { #We can reserve this Item! } >+ >+See CanItemBeReserved() for possible return values. > > =cut > >@@ -462,17 +465,24 @@ sub CanBookBeReserved{ > push (@$items,@hostitems); > } > >+ my $canReserve; > foreach my $item (@$items){ >- return 1 if CanItemBeReserved($borrowernumber, $item); >+ $canReserve = CanItemBeReserved($borrowernumber, $item); >+ return 'OK' if $canReserve eq 'OK'; > } >- return 0; >+ return $canReserve; > } > > =head2 CanItemBeReserved > >- $error = &CanItemBeReserved($borrowernumber, $itemnumber) >+ $canReserve = &CanItemBeReserved($borrowernumber, $itemnumber) >+ if ($canReserve eq 'OK') { #We can reserve this Item! } > >-This function return 1 if an item can be issued by this borrower. >+@RETURNS OK, if the Item can be reserved. >+ ageRestricted, if the Item is age restricted for this borrower. >+ damaged, if the Item is damaged. >+ cannotReserveFromOtherBranches, if syspref 'canreservefromotherbranches' is OK. >+ tooManyReserves, if the borrower has exceeded his maximum reserve amount. > > =cut > >@@ -490,11 +500,11 @@ sub CanItemBeReserved{ > my $borrower = C4::Members::GetMember('borrowernumber'=>$borrowernumber); > > # If an item is damaged and we don't allow holds on damaged items, we can stop right here >- return 0 if ( $item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems') ); >+ return 'damaged' if ( $item->{damaged} && !C4::Context->preference('AllowHoldsOnDamagedItems') ); > > #Check for the age restriction > my ($ageRestriction, $daysToAgeRestriction) = C4::Circulation::GetAgeRestriction( $biblioData->{agerestriction}, $borrower ); >- return 0 if $daysToAgeRestriction && $daysToAgeRestriction > 0; >+ return 'ageRestricted' if $daysToAgeRestriction && $daysToAgeRestriction > 0; > > my $controlbranch = C4::Context->preference('ReservesControlBranch'); > my $itemtypefield = C4::Context->preference('item-level_itypes') ? "itype" : "itemtype"; >@@ -561,7 +571,7 @@ sub CanItemBeReserved{ > > # we check if it's ok or not > if( $reservecount >= $allowedreserves ){ >- return 0; >+ return 'tooManyReserves'; > } > > # If reservecount is ok, we check item branch if IndependentBranches is ON >@@ -571,11 +581,11 @@ sub CanItemBeReserved{ > { > my $itembranch = $item->{homebranch}; > if ($itembranch ne $borrower->{branchcode}) { >- return 0; >+ return 'cannotReserveFromOtherBranches'; > } > } > >- return 1; >+ return 'OK'; > } > > =head2 CanReserveBeCanceledFromOpac >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >index 6c061e4..ddec2b5 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >@@ -241,7 +241,7 @@ function checkMultiHold() { > </form> > [% ELSE %] > >-[% IF ( maxreserves || alreadyreserved || none_available || alreadypossession ) %] >+[% IF ( maxreserves || alreadyreserved || none_available || alreadypossession || ageRestricted ) %] > <div class="dialog alert"> > > [% UNLESS ( multi_hold ) %] >@@ -250,6 +250,9 @@ function checkMultiHold() { > [% IF ( maxreserves ) %] > <li><strong>Too many holds: </strong> <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %] </a> has too many holds.</li> > [% END %] >+ [% IF ( ageRestricted ) %] >+ <li><strong>Age restricted</strong></li> >+ [% END %] > [% IF ( alreadyreserved ) %] > <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> <strong>already has a hold</strong> on this item </li> > [% END %] >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 a0b86e8..e7e4ee7 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >@@ -52,6 +52,12 @@ > </div> > [% END %] > >+ [% IF ( ageRestricted ) %] >+ <div id="ageRestricted" class="alert"> >+ Sorry, you are too young to reserve this material. >+ </div> >+ [% END %] >+ > [% IF ( bad_biblionumber ) %] > <div id="bad_biblionumber" class="alert"> > ERROR: No biblio record found for biblionumber [% bad_biblionumber %].</div> >diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl >index b856304..a817fbc 100755 >--- a/opac/opac-detail.pl >+++ b/opac/opac-detail.pl >@@ -522,8 +522,8 @@ my $dat = &GetBiblioData($biblionumber); > > #Check if we can place a hold on this biblio. > my $canReserve = C4::Reserves::CanBookBeReserved( $biblionumber, $biblionumber ); >-unless ($canReserve ) { >- $template->param( cannotReserve => 1 ) ; >+unless ($canReserve eq 'OK' ) { >+ $template->param( cannotReserve => $canReserve ) ; > } > > >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 45d03bd..0e2d88c 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -548,9 +548,17 @@ foreach my $biblioNum (@biblionumbers) { > if ($biblioLoopIter{already_reserved}) { > $biblioLoopIter{holdable} = undef; > } >- if(not CanBookBeReserved($borrowernumber,$biblioNum)){ >+ my $canReserve = CanBookBeReserved($borrowernumber,$biblioNum); >+ if ($canReserve eq 'OK') { >+ #All is OK! >+ } >+ elsif ($canReserve eq 'ageRestricted') { > $biblioLoopIter{holdable} = undef; >+ $template->param( message => 1, $canReserve => 1 ); > } >+ else { >+ $biblioLoopIter{holdable} = undef; >+ } > if(not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowernumber,$biblioNum)) { > $biblioLoopIter{holdable} = undef; > $biblioLoopIter{already_patron_possession} = 1; >diff --git a/reserve/request.pl b/reserve/request.pl >index 14340c2..a679065 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -1,4 +1,4 @@ >-#!/usr/bin/perl >+#!/usr/bin/perl -d > > > #writen 2/1/00 by chris@katipo.oc.nz >@@ -191,9 +191,20 @@ foreach my $biblionumber (@biblionumbers) { > > my $dat = GetBiblioData($biblionumber); > >- unless ( CanBookBeReserved($borrowerinfo->{borrowernumber}, $biblionumber) ) { >- $maxreserves = 1; >+ my $canReserve = CanBookBeReserved($borrowerinfo->{borrowernumber}, $biblionumber); >+ if ($canReserve eq 'OK') { >+ #All is OK and we can continue >+ } >+ elsif ( $canReserve eq 'tooManyReserves' ) { >+ $maxreserves = 1; > } >+ elsif ( $canReserve eq 'ageRestricted' ) { >+ $template->param( $canReserve => 1 ); >+ $biblioloopiter{ $canReserve } = 1; >+ } >+ else { >+ $biblioloopiter{ $canReserve } = 1; >+ } > > my $alreadypossession; > if (not C4::Context->preference('AllowHoldsOnPatronsPossessions') and CheckIfIssuedToPatron($borrowerinfo->{borrowernumber},$biblionumber)) { >@@ -415,7 +426,7 @@ foreach my $biblionumber (@biblionumbers) { > && IsAvailableForItemLevelRequest($itemnumber) > && CanItemBeReserved( > $borrowerinfo->{borrowernumber}, $itemnumber >- ) >+ ) eq 'OK' > ) > { > $item->{available} = 1; >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 0bc763b..b666dfe 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -486,7 +486,7 @@ my ( $ageres_tagid, $ageres_subfieldid ) = GetMarcFromKohaField( "biblioitems.ag > $record->append_fields( MARC::Field->new($ageres_tagid, '', '', $ageres_subfieldid => 'PEGI 16') ); > C4::Biblio::ModBiblio( $record, $bibnum, '' ); > >-is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 1, "Reserving an ageRestricted Biblio without a borrower dateofbirth succeeds" ); >+is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'OK', "Reserving an ageRestricted Biblio without a borrower dateofbirth succeeds" ); > > #Set the dateofbirth for the Borrower making him "too young". > my $now = DateTime->now(); >@@ -494,14 +494,14 @@ my $duration_15years = DateTime::Duration->new(years => 15); > my $past15yearsAgo = DateTime->now()->subtract_duration($duration_15years); > C4::Members::ModMember( borrowernumber => $borrowernumber, dateofbirth => $past15yearsAgo->ymd() ); > >-is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 0, "Reserving a 'PEGI 16' Biblio by a 15 year old borrower fails"); >+is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'ageRestricted', "Reserving a 'PEGI 16' Biblio by a 15 year old borrower fails"); > > #Set the dateofbirth for the Borrower making him "too old". > my $duration_30years = DateTime::Duration->new(years => 30); > my $past30yearsAgo = DateTime->now()->subtract_duration($duration_30years); > C4::Members::ModMember( borrowernumber => $borrowernumber, dateofbirth => $past30yearsAgo->ymd() ); > >-is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 1, "Reserving a 'PEGI 16' Biblio by a 30 year old borrower succeeds"); >+is( C4::Reserves::CanBookBeReserved($borrowernumber, $biblionumber) , 'OK', "Reserving a 'PEGI 16' Biblio by a 30 year old borrower succeeds"); > #### > ####### EO Bug 13113 <<< > #### >-- >1.7.9.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 13116
:
32544
|
32550
|
32551
|
32552
|
32627
|
32662
|
32920
|
33075
|
33076
|
33077