Bugzilla – Attachment 17448 Details for
Bug 7710
multiple holds per title
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 7710 - multiple holds per title
Bug-7710---multiple-holds-per-title.patch (text/plain), 20.72 KB, created by
Kyle M Hall (khall)
on 2013-04-15 11:57:06 UTC
(
hide
)
Description:
Bug 7710 - multiple holds per title
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2013-04-15 11:57:06 UTC
Size:
20.72 KB
patch
obsolete
>From e9a5d4427e6bc6bd4906518026fc5909fb3b274f Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Tue, 15 Jan 2013 10:18:03 -0500 >Subject: [PATCH] Bug 7710 - multiple holds per title > >Adds the ability to allow multiple holds >on the same record for the same borrower. > >Adds new system preference MaxHoldsPerRecord, >which controls how many holds one person can >have on items for the same record. > >Test Plan: >1) Apply patch for Bug 9394 >1) Apply this patch >2) Run updatedatabase.pl >3) Attempt to place 2 holds for the same patron on the same record, > you should not be able to ( default is 1 per record ) >4) Set MaxHoldsPerRecord to 3 >5) Attempt to place multiple holds for one patron on one record, > you should be able to place 3 holds for one patron on a given > record. A 4th hold should result in a message stating the patron > has placed the max number of holds allowed on this record. >--- > C4/Reserves.pm | 59 ++++++++++++-------- > installer/data/mysql/sysprefs.sql | 1 + > installer/data/mysql/updatedatabase.pl | 6 ++ > .../en/modules/admin/preferences/circulation.pref | 7 ++- > .../prog/en/modules/reserve/request.tt | 25 ++++---- > .../opac-tmpl/prog/en/modules/opac-reserve.tt | 6 ++- > opac/opac-reserve.pl | 29 ++++++---- > reserve/request.pl | 9 +++- > 8 files changed, 92 insertions(+), 50 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index c001b71..b7f0ecc 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -373,36 +373,47 @@ sub GetReservesFromItemnumber { > > =head2 GetReservesFromBorrowernumber > >- $borrowerreserv = GetReservesFromBorrowernumber($borrowernumber,$tatus); >- >-TODO :: Descritpion >+ $borrowerreserv = GetReservesFromBorrowernumber($borrowernumber[,$status][,$biblionumber][,$itemnumber]); > > =cut > > sub GetReservesFromBorrowernumber { >- my ( $borrowernumber, $status ) = @_; >- my $dbh = C4::Context->dbh; >- my $sth; >- if ($status) { >- $sth = $dbh->prepare(" >- SELECT * >- FROM reserves >- WHERE borrowernumber=? >- AND found =? >- ORDER BY reservedate >- "); >- $sth->execute($borrowernumber,$status); >- } else { >- $sth = $dbh->prepare(" >- SELECT * >- FROM reserves >- WHERE borrowernumber=? >- ORDER BY reservedate >- "); >- $sth->execute($borrowernumber); >+ my ( $borrowernumber, $found, $biblionumber, $itemnumber ) = @_; >+ >+ my @params; >+ >+ my $sql = "SELECT * FROM reserves WHERE borrowernumber = ?"; >+ push( @params, $borrowernumber ); >+ >+ if ( $biblionumber ) { >+ $sql .= " AND biblionumber = ? "; >+ push( @params, $biblionumber ); >+ } >+ >+ if ( $itemnumber ) { >+ $sql .= " AND itemnumber = ? "; >+ push( @params, $itemnumber ); >+ } >+ >+ if ( $found ) { >+ $sql .= " AND found = ? "; >+ push( @params, $found ); > } >+ >+ $sql .= " ORDER BY reservedate "; >+ >+ my $dbh = C4::Context->dbh; >+ my $sth = $dbh->prepare( $sql ); >+ >+ $sth->execute( @params ); >+ > my $data = $sth->fetchall_arrayref({}); >- return @$data; >+ >+ if ( scalar @$data ) { >+ return @$data; >+ } else { >+ return (); >+ } > } > #------------------------------------------------------------------------------------- > =head2 CanBookBeReserved >diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql >index 5d536c5..c0cb13d 100644 >--- a/installer/data/mysql/sysprefs.sql >+++ b/installer/data/mysql/sysprefs.sql >@@ -423,3 +423,4 @@ INSERT INTO systempreferences (variable,value,options,explanation,type) VALUES ( > INSERT INTO systempreferences (variable,value,options,explanation,type) VALUES('DisplayIconsXSLT', '1', '', 'If ON, displays the format, audience, and material type icons in XSLT MARC21 results and detail pages.', 'YesNo'); > INSERT INTO systempreferences (variable,value,options,explanation,type) VALUES ('HighlightOwnItemsOnOPAC','0','','If on, and a patron is logged into the OPAC, items from his or her home library will be emphasized and shown first in search results and item details.','YesNo'); > INSERT INTO systempreferences (variable,value,options,explanation,type) VALUES ('HighlightOwnItemsOnOPACWhich','PatronBranch','PatronBranch|OpacURLBranch','Decides which branch''s items to emphasize. If PatronBranch, emphasize the logged in user''s library''s items. If OpacURLBranch, highlight the items of the Apache var BRANCHCODE defined in Koha''s Apache configuration file.','Choice') >+INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES('MaxHoldsPerRecord', '1', 'The maximum number of holds allowed per bibliographic record per borrower', NULL, NULL); >diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl >index 7c97a51..b167f2a 100755 >--- a/installer/data/mysql/updatedatabase.pl >+++ b/installer/data/mysql/updatedatabase.pl >@@ -6771,6 +6771,12 @@ if ( CheckVersion($DBversion) ) { > SetVersion ($DBversion); > } > >+$DBversion = "3.11.00.XXX"; >+if (C4::Context->preference("Version") < TransformToNum($DBversion)) { >+ $dbh->do("INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES ('MaxHoldsPerRecord', '1', 'The maximum number of holds allowed per bibliographic record per borrower', NULL, NULL)"); >+ print "Upgrade to $DBversion done (Add syspref MaxHoldsPerRecord)\n"; >+ SetVersion ($DBversion); >+} > > =head1 FUNCTIONS > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >index f88d964..f06e425 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref >@@ -404,7 +404,12 @@ Circulation: > - Patrons can only have > - pref: maxreserves > class: integer >- - holds at once. >+ - total holds at a time. >+ - >+ - Patrons can only place >+ - pref: MaxHoldsPerRecord >+ class: integer >+ - holds on a single record at a time. > - > - pref: emailLibrarianWhenHoldIsPlaced > choices: >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 122b2d0..67f5a69 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt >@@ -55,7 +55,7 @@ function check() { > } > > if (alreadyreserved > "0"){ >- msg += (_("- This patron had already placed a hold on this item") + "\n" + _("Please cancel the previous hold first") + "\n"); >+ msg += (_("This patron has already placed the maximum number of holds for this record.") + "\n\n" + _("Please cancel a previous hold first.") + "\n"); > } > > if (msg == "") return(true); >@@ -208,7 +208,7 @@ function checkMultiHold() { > <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 ( 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> >+ <li><a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% borrowernumber %]">[% borrowerfirstname %] [% borrowersurname %]</a> <strong>has already placed the maximum allowed number of holds</strong> on this record</li> > [% END %] > [% IF ( none_available ) %] > <li> <strong>No copies are available</strong> to be placed on hold</li> >@@ -564,6 +564,7 @@ function checkMultiHold() { > [% FOREACH reserveloo IN biblioloo.reserveloop %] > [% UNLESS ( loop.odd ) %]<tr class="highlight">[% ELSE %]<tr>[% END %] > <td> >+ <input type="hidden" name="reserve_id" value="[% reserveloo.reserve_id %]" /> > <input type="hidden" name="borrowernumber" value="[% reserveloo.borrowernumber %]" /> > <input type="hidden" name="biblionumber" value="[% reserveloo.biblionumber %]" /> > <select name="rank-request"> >@@ -583,19 +584,19 @@ function checkMultiHold() { > > [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] > <td style="white-space:nowrap;"> >- <a title="Move Hold Up" href="request.pl?action=move&where=up&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Move Hold Up" href="request.pl?action=move&where=up&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/go-up.png" border="0" alt="Go up" /> > </a> > >- <a title="Move hold to top" href="request.pl?action=move&where=top&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Move Hold To Top" href="request.pl?action=move&where=top&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/go-top.png" border="0" alt="Go top" /> > </a> > >- <a title="Move hold to bottom" href="request.pl?action=move&where=bottom&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Move Hold To Bottom" href="request.pl?action=move&where=bottom&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/go-bottom.png" border="0" alt="Go bottom" /> > </a> > >- <a title="Move hold down" href="request.pl?action=move&where=down&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Move Hold Down" href="request.pl?action=move&where=down&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/go-down.png" border="0" alt="Go down" /> > </a> > </td> >@@ -675,7 +676,7 @@ function checkMultiHold() { > > [% IF ( CAN_user_reserveforothers_modify_holds_priority ) %] > <td> >- <a title="Toggle lowest priority" href="request.pl?action=setLowestPriority&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Toggle Lowest Priority" href="request.pl?action=setLowestPriority&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > [% IF ( reserveloo.lowestPriority ) %] > <img src="/intranet-tmpl/[% theme %]/img/go-bottom.png" border="0" alt="Unset Lowest Priority" /> > [% ELSE %] >@@ -686,7 +687,7 @@ function checkMultiHold() { > [% END %] > > <td> >- <a title="Cancel hold" href="request.pl?action=cancel&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]"> >+ <a title="Cancel Hold" href="request.pl?action=cancel&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&reserve_id=[% reserveloo.reserve_id %]&date=[% reserveloo.date %]"> > <img src="/intranet-tmpl/[% theme %]/img/x.png" border="0" alt="Cancel" /> > </a> > </td> >@@ -694,12 +695,12 @@ function checkMultiHold() { > [% IF SuspendHoldsIntranet %] > <td> > [% UNLESS ( reserveloo.wait ) %] >- <input type="button" value="[% IF ( reserveloo.suspend ) %]Unsuspend[% ELSE %]Suspend[% END %]" onclick="window.location.href='request.pl?action=toggleSuspend&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]&suspend_until=' + $('#suspend_until_[% reserveloo.borrowernumber %]').val()" /> >+ <input type="button" value="[% IF ( reserveloo.suspend ) %]Unsuspend[% ELSE %]Suspend[% END %]" onclick="window.location.href='request.pl?action=toggleSuspend&reserve_id=[% reserveloo.reserve_id %]&borrowernumber=[% reserveloo.borrowernumber %]&biblionumber=[% reserveloo.biblionumber %]&date=[% reserveloo.date %]&suspend_until=' + $('#suspend_until_[% reserveloo.reserve_id %]').val()" /> > > [% IF AutoResumeSuspendedHolds %] >- <label for="suspend_until_[% reserveloo.borrowernumber %]">[% IF ( reserveloo.suspend ) %] on [% ELSE %] until [% END %]</label> >- <input name="suspend_until" id="suspend_until_[% reserveloo.borrowernumber %]" size="10" readonly="readonly" value="[% reserveloo.suspend_until | $KohaDates %]" class="datepicker" /> >- <a href='#' onclick="document.getElementById('suspend_until_[% reserveloo.borrowernumber %]').value='';">Clear Date</a> >+ <label for="suspend_until_[% reserveloo.reserve_id %]">[% IF ( reserveloo.suspend ) %] on [% ELSE %] until [% END %]</label> >+ <input name="suspend_until" id="suspend_until_[% reserveloo.reserve_id %]" size="10" readonly="readonly" value="[% reserveloo.suspend_until | $KohaDates %]" class="datepicker" /> >+ <a href='#' onclick="document.getElementById('suspend_until_[% reserveloo.reserve_id %]').value='';">Clear Date</a> > [% END %] > [% ELSE %] > <input type="hidden" name="suspend_until" value="" /> >diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt >index 8a8bd03..317441f 100644 >--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt >+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-reserve.tt >@@ -296,7 +296,11 @@ > [% IF ( bibitemloo.already_patron_possession ) %] > <div class="bibmessage">This title cannot be requested because it's already in your possession.</div> > [% ELSE %] >- <div class="bibmessage">This title cannot be requested.</div> >+ [% IF bibitemloo.max_holds %] >+ <div class="bibmessage">This title cannot be requested. You have already placed the maximum number of requests allowed for this title.</div> >+ [% ELSE %] >+ <div class="bibmessage">This title cannot be requested.</div> >+ [% END %] > [% END %] > [% END %] > [% END %] >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index e725d30..a713adb 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -37,7 +37,8 @@ use C4::Debug; > use Koha::DateUtils; > # use Data::Dumper; > >-my $MAXIMUM_NUMBER_OF_RESERVES = C4::Context->preference("maxreserves"); >+my $maxreserves = C4::Context->preference("maxreserves"); >+my $max_holds_per_record = C4::Context->preference('MaxHoldsPerRecord'); > > my $query = new CGI; > my ( $template, $borrowernumber, $cookie ) = get_template_and_user( >@@ -172,7 +173,7 @@ foreach my $biblioNumber (@biblionumbers) { > if ( $query->param('place_reserve') ) { > my $notes = $query->param('notes'); > my $reserve_cnt = 0; >- if ($MAXIMUM_NUMBER_OF_RESERVES) { >+ if ($maxreserves) { > $reserve_cnt = GetReservesFromBorrowernumber( $borrowernumber ); > } > >@@ -255,8 +256,8 @@ if ( $query->param('place_reserve') ) { > $itemNum = undef; > } > >- if ( $MAXIMUM_NUMBER_OF_RESERVES >- && $reserve_cnt >= $MAXIMUM_NUMBER_OF_RESERVES ) >+ if ( $maxreserves >+ && $reserve_cnt >= $maxreserves ) > { > $canreserve = 0; > } >@@ -317,7 +318,7 @@ if ( CheckBorrowerDebarred($borrowernumber) ) { > > my @reserves = GetReservesFromBorrowernumber( $borrowernumber ); > $template->param( RESERVES => \@reserves ); >-if ( $MAXIMUM_NUMBER_OF_RESERVES && (scalar(@reserves) >= $MAXIMUM_NUMBER_OF_RESERVES) ) { >+if ( $maxreserves && (scalar(@reserves) >= $maxreserves) ) { > $template->param( message => 1 ); > $noreserves = 1; > $template->param( too_many_reserves => scalar(@reserves)); >@@ -325,12 +326,10 @@ if ( $MAXIMUM_NUMBER_OF_RESERVES && (scalar(@reserves) >= $MAXIMUM_NUMBER_OF_RES > foreach my $res (@reserves) { > foreach my $biblionumber (@biblionumbers) { > if ( $res->{'biblionumber'} == $biblionumber && $res->{'borrowernumber'} == $borrowernumber) { >-# $template->param( message => 1 ); >-# $noreserves = 1; >-# $template->param( already_reserved => 1 ); >- $biblioDataHash{$biblionumber}->{already_reserved} = 1; >+ $biblioDataHash{$biblionumber}->{holds_count}++; > } > } >+ > } > > unless ($noreserves) { >@@ -372,7 +371,7 @@ foreach my $biblioNum (@biblionumbers) { > $biblioLoopIter{author} = $biblioData->{author}; > $biblioLoopIter{rank} = $biblioData->{rank}; > $biblioLoopIter{reservecount} = $biblioData->{reservecount}; >- $biblioLoopIter{already_reserved} = $biblioData->{already_reserved}; >+ $biblioLoopIter{holds_count} = $biblioData->{holds_count}; > > if (!$itemLevelTypes && $biblioData->{itemtype}) { > $biblioLoopIter{description} = $itemTypes->{$biblioData->{itemtype}}{description}; >@@ -499,11 +498,14 @@ foreach my $biblioNum (@biblionumbers) { > $policy_holdallowed = 0; > } > >- if (IsAvailableForItemLevelRequest($itemNum) and $policy_holdallowed and CanItemBeReserved($borrowernumber,$itemNum) and ($itemLoopIter->{already_reserved} ne 1)) { >+ if (IsAvailableForItemLevelRequest($itemNum) and $policy_holdallowed and CanItemBeReserved($borrowernumber,$itemNum) and ($itemLoopIter->{holds_count} < $max_holds_per_record )) { > $itemLoopIter->{available} = 1; > $numCopiesAvailable++; > } > >+ # Don't allow multiple item level holds on the same item if borrowers can place multiple holds on one bib >+ $itemLoopIter->{available} = 0 if ( GetReservesFromBorrowernumber( $borrowernumber, undef, undef, $itemNum ) ); >+ > # FIXME: move this to a pm > my $dbh = C4::Context->dbh; > my $sth2 = $dbh->prepare("SELECT * FROM reserves WHERE borrowernumber=? AND itemnumber=? AND found='W'"); >@@ -532,6 +534,11 @@ foreach my $biblioNum (@biblionumbers) { > $biblioLoopIter{holdable} = undef; > $anyholdable = undef; > } >+ if ($biblioLoopIter{holds_count} >= $max_holds_per_record) { >+ $biblioLoopIter{holdable} = undef; >+ $anyholdable = undef; >+ $biblioLoopIter{max_holds} = 1; >+ } > if(not CanBookBeReserved($borrowernumber,$biblioNum)){ > $biblioLoopIter{holdable} = undef; > $anyholdable = undef; >diff --git a/reserve/request.pl b/reserve/request.pl >index 5f66956..0bd639d 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -209,6 +209,7 @@ if ($borrowerslist) { > > # FIXME launch another time GetMember perhaps until > my $borrowerinfo = GetMember( borrowernumber => $borrowernumber_hold ); >+my $max_holds_per_record = C4::Context->preference('MaxHoldsPerRecord'); > > my @biblionumbers = (); > my $biblionumbers = $input->param('biblionumbers'); >@@ -254,7 +255,7 @@ foreach my $biblionumber (@biblionumbers) { > } > } > >- if ( $holds_count ) { >+ if ( $holds_count >= $max_holds_per_record ) { > $alreadyreserved = 1; > $warnings = 1; > $biblioloopiter{warn} = 1; >@@ -482,6 +483,12 @@ foreach my $biblionumber (@biblionumbers) { > $itemdata_enumchron = 1; > } > >+ # Don't allow multiple item level holds on the same item if borrowers can place multiple holds on one bib >+ if ( GetReservesFromBorrowernumber( $borrowerinfo->{borrowernumber}, undef, undef, $itemnumber) ) { >+ $item->{available} = 0; >+ $item->{override} = 0; >+ } >+ > push @{ $biblioitem->{itemloop} }, $item; > } > >-- >1.7.2.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 7710
:
8598
|
8599
|
8600
|
8601
|
8602
|
8603
|
10091
|
10098
|
10099
|
10100
|
10101
|
10102
|
10818
|
10897
|
12620
|
14083
|
14600
|
14602
|
14603
|
15252
|
15253
|
15295
|
15731
|
15732
|
17448
|
18217
|
18320
|
18321
|
19117
|
27429
|
30218
|
35397
|
35518