Bugzilla – Attachment 46463 Details for
Bug 15534
Add the ability to prevent a patron from placing a hold on a record with available items
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15534 - Add the ability to prevent a patron from placing a hold on a record with available items
Bug-15534---Add-the-ability-to-prevent-a-patron-fr.patch (text/plain), 18.22 KB, created by
Kyle M Hall (khall)
on 2016-01-09 11:25:07 UTC
(
hide
)
Description:
Bug 15534 - Add the ability to prevent a patron from placing a hold on a record with available items
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2016-01-09 11:25:07 UTC
Size:
18.22 KB
patch
obsolete
>From 841a40e6618bd2345bb23a5e4f7624ab3a9df236 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Fri, 1 Jan 2016 11:33:49 +0000 >Subject: [PATCH] Bug 15534 - Add the ability to prevent a patron from placing > a hold on a record with available items > >Some libraries would like to prevent patrons from placing holds on >records/items where there are other items available for the patron to >check out. > >Test Plan: >1) Apply this patch >2) Run updatedatabase.pl >3) Browse to the circulation rules >4) Note the new rule "Allow hold if others available" >5) Set the rule to "No" >6) Find a patron/branch/itemtype applicable to this rule >7) Ensure at least one item on the record is available for the > patron to check out >8) Attempt to place a hold for the patron >9) Note you cannot place the hold >10) Check the available item out to another patron >11) Note you can now place a hold for the first patron >--- > C4/Members.pm | 7 +- > C4/Reserves.pm | 71 +++++++----- > Koha/Schema/Result/Issuingrule.pm | 12 +- > admin/smart-rules.pl | 2 + > .../mysql/atomicupdate/on_shelf_non_available.sql | 1 + > installer/data/mysql/kohastructure.sql | 1 + > .../prog/en/modules/admin/smart-rules.tt | 9 ++ > t/db_dependent/Holds/AllowHoldIfItemsAvailable.t | 123 +++++++++++++++++++++ > 8 files changed, 198 insertions(+), 28 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/on_shelf_non_available.sql > create mode 100755 t/db_dependent/Holds/AllowHoldIfItemsAvailable.t > >diff --git a/C4/Members.pm b/C4/Members.pm >index 4ba1f19..d12aa03 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -452,7 +452,12 @@ sub GetMember { > #FIXME interface to this routine now allows generation of a result set > #so whole array should be returned but bowhere in the current code expects this > if (@{$data} ) { >- return $data->[0]; >+ my $patron = $data->[0]; >+ >+ my $flags = patronflags( $patron ); >+ $patron->{flags} = $flags; >+ >+ return $patron; > } > > return; >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 4d20c98..2b4edad 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -29,6 +29,7 @@ use C4::Members; > use C4::Items; > use C4::Circulation; > use C4::Accounts; >+use C4::Circulation qw(CanBookBeIssued); > > # for _koha_notify_reserve > use C4::Members::Messaging; >@@ -41,6 +42,7 @@ use Koha::Calendar; > use Koha::Database; > use Koha::Hold; > use Koha::Holds; >+use Koha::Items; > > use List::MoreUtils qw( firstidx any ); > use Carp; >@@ -421,19 +423,20 @@ See CanItemBeReserved() for possible return values. > > =cut > >-sub CanBookBeReserved{ >- my ($borrowernumber, $biblionumber) = @_; >+sub CanBookBeReserved { >+ my ( $borrowernumber, $biblionumber ) = @_; > > my $items = GetItemnumbersForBiblio($biblionumber); >+ > #get items linked via host records > my @hostitems = get_hostitemnumbers_of($biblionumber); >- if (@hostitems){ >- push (@$items,@hostitems); >+ if (@hostitems) { >+ push( @$items, @hostitems ); > } > > my $canReserve; >- foreach my $item (@$items) { >- $canReserve = CanItemBeReserved( $borrowernumber, $item ); >+ foreach my $itemnumber (@$items) { >+ $canReserve = CanItemBeReserved( $borrowernumber, $itemnumber ); > return 'OK' if $canReserve eq 'OK'; > } > return $canReserve; >@@ -459,7 +462,7 @@ sub CanItemBeReserved{ > my $dbh = C4::Context->dbh; > my $ruleitemtype; # itemtype of the matching issuing rule > my $allowedreserves = 0; >- >+ > # we retrieve borrowers and items informations # > # item->{itype} will come for biblioitems if necessery > my $item = GetItem($itemnumber); >@@ -476,7 +479,7 @@ sub CanItemBeReserved{ > my $controlbranch = C4::Context->preference('ReservesControlBranch'); > > # we retrieve user rights on this itemtype and branchcode >- my $sth = $dbh->prepare("SELECT categorycode, itemtype, branchcode, reservesallowed >+ my $sth = $dbh->prepare("SELECT categorycode, itemtype, branchcode, reservesallowed, allow_hold_if_items_available > FROM issuingrules > WHERE (categorycode in (?,'*') ) > AND (itemtype IN (?,'*')) >@@ -495,8 +498,8 @@ sub CanItemBeReserved{ > LEFT JOIN borrowers USING (borrowernumber) > WHERE borrowernumber = ? > "; >- >- >+ >+ > my $branchcode = ""; > my $branchfield = "reserves.branchcode"; > >@@ -507,20 +510,24 @@ sub CanItemBeReserved{ > $branchfield = "borrowers.branchcode"; > $branchcode = $borrower->{branchcode}; > } >- >- # we retrieve rights >+ >+ my $allow_hold_if_items_available; >+ >+ # we retrieve rights > $sth->execute($borrower->{'categorycode'}, $item->{'itype'}, $branchcode); >- if(my $rights = $sth->fetchrow_hashref()){ >- $ruleitemtype = $rights->{itemtype}; >- $allowedreserves = $rights->{reservesallowed}; >- }else{ >+ if ( my $rights = $sth->fetchrow_hashref() ) { >+ $ruleitemtype = $rights->{itemtype}; >+ $allowedreserves = $rights->{reservesallowed}; >+ $allow_hold_if_items_available = $rights->{allow_hold_if_items_available}; >+ } >+ else { > $ruleitemtype = '*'; > } > > # we retrieve count > > $querycount .= "AND $branchfield = ?"; >- >+ > # If using item-level itypes, fall back to the record > # level itemtype if the hold has no associated item > $querycount .= >@@ -530,7 +537,7 @@ sub CanItemBeReserved{ > if ( $ruleitemtype ne "*" ); > > my $sthcount = $dbh->prepare($querycount); >- >+ > if($ruleitemtype eq "*"){ > $sthcount->execute($borrowernumber, $branchcode); > }else{ >@@ -572,6 +579,20 @@ sub CanItemBeReserved{ > } > } > >+ unless ( $allow_hold_if_items_available ) { >+ my $item = Koha::Items->find($itemnumber); >+ my @items = Koha::Items->search({ biblionumber => $item->biblionumber }); >+ foreach $item (@items) { >+ my ( $issuingimpossible, $needsconfirmation ) = C4::Circulation::CanBookBeIssued( >+ $borrower, $item->barcode, my $duedate, >+ my $inprocess, >+ my $ignore_reserves = 1 >+ ); >+ >+ return 'itemsAvailable' unless ( keys %$issuingimpossible || keys %$needsconfirmation ); >+ } >+ } >+ > return 'OK'; > } > >@@ -744,8 +765,8 @@ sub GetReservesToBranch { > my $dbh = C4::Context->dbh; > my $sth = $dbh->prepare( > "SELECT reserve_id,borrowernumber,reservedate,itemnumber,timestamp >- FROM reserves >- WHERE priority='0' >+ FROM reserves >+ WHERE priority='0' > AND branchcode=?" > ); > $sth->execute( $frombranch ); >@@ -770,7 +791,7 @@ sub GetReservesForBranch { > > my $query = " > SELECT reserve_id,borrowernumber,reservedate,itemnumber,waitingdate >- FROM reserves >+ FROM reserves > WHERE priority='0' > AND found='W' > "; >@@ -1389,7 +1410,7 @@ sub ModReserveMinusPriority { > my $dbh = C4::Context->dbh; > my $query = " > UPDATE reserves >- SET priority = 0 , itemnumber = ? >+ SET priority = 0 , itemnumber = ? > WHERE reserve_id = ? > "; > my $sth_upd = $dbh->prepare($query); >@@ -1604,7 +1625,7 @@ sub ToggleLowestPriority { > > my $sth = $dbh->prepare( "UPDATE reserves SET lowestPriority = NOT lowestPriority WHERE reserve_id = ?"); > $sth->execute( $reserve_id ); >- >+ > _FixPriority({ reserve_id => $reserve_id, rank => '999999' }); > } > >@@ -1814,7 +1835,7 @@ sub _FixPriority { > $priority[$j]->{'reserve_id'} > ); > } >- >+ > $sth = $dbh->prepare( "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 ORDER BY priority" ); > $sth->execute(); > >@@ -2052,7 +2073,7 @@ sub _koha_notify_reserve { > if (! $notification_sent) { > &$send_notification('print', 'HOLD'); > } >- >+ > } > > =head2 _ShiftPriorityByDateAndPriority >diff --git a/Koha/Schema/Result/Issuingrule.pm b/Koha/Schema/Result/Issuingrule.pm >index f782456..1f43373 100644 >--- a/Koha/Schema/Result/Issuingrule.pm >+++ b/Koha/Schema/Result/Issuingrule.pm >@@ -191,6 +191,12 @@ __PACKAGE__->table("issuingrules"); > is_nullable: 0 > size: 1 > >+=head2 allow_hold_if_items_available >+ >+ data_type: 'tinyint' >+ default_value: 1 >+ is_nullable: 0 >+ > =cut > > __PACKAGE__->add_columns( >@@ -257,6 +263,8 @@ __PACKAGE__->add_columns( > { data_type => "tinyint", default_value => 0, is_nullable => 0 }, > "opacitemholds", > { data_type => "char", default_value => "N", is_nullable => 0, size => 1 }, >+ "allow_hold_if_items_available", >+ { data_type => "tinyint", default_value => 1, is_nullable => 0 }, > ); > > =head1 PRIMARY KEY >@@ -276,8 +284,8 @@ __PACKAGE__->add_columns( > __PACKAGE__->set_primary_key("branchcode", "categorycode", "itemtype"); > > >-# Created by DBIx::Class::Schema::Loader v0.07042 @ 2015-12-31 15:26:16 >-# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:K/8SKpDjba5CM4+WPZtWPw >+# Created by DBIx::Class::Schema::Loader v0.07042 @ 2016-01-02 12:22:52 >+# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:P6v2zqse5sAEUI5qHdFOHQ > > > # You can replace this text with custom content, and it will be preserved on regeneration >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index 8e17802..f21968d 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -144,6 +144,7 @@ elsif ($op eq 'add') { > my $hardduedatecompare = $input->param('hardduedatecompare'); > my $rentaldiscount = $input->param('rentaldiscount'); > my $opacitemholds = $input->param('opacitemholds') || 0; >+ my $allow_hold_if_items_available = $input->param('allow_hold_if_items_available') || 0; > my $overduefinescap = $input->param('overduefinescap') || undef; > my $cap_fine_to_replacement_price = $input->param('cap_fine_to_replacement_price') eq 'on'; > $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price"; >@@ -173,6 +174,7 @@ elsif ($op eq 'add') { > onshelfholds => $onshelfholds, > opacitemholds => $opacitemholds, > overduefinescap => $overduefinescap, >+ allow_hold_if_items_available => $allow_hold_if_items_available, > cap_fine_to_replacement_price => $cap_fine_to_replacement_price, > }; > >diff --git a/installer/data/mysql/atomicupdate/on_shelf_non_available.sql b/installer/data/mysql/atomicupdate/on_shelf_non_available.sql >new file mode 100644 >index 0000000..015a214 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/on_shelf_non_available.sql >@@ -0,0 +1 @@ >+ALTER TABLE issuingrules ADD COLUMN allow_hold_if_items_available TINYINT(1) NOT NULL DEFAULT 1 AFTER opacitemholds; >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index bc3c3b1..ec33194 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -1198,6 +1198,7 @@ CREATE TABLE `issuingrules` ( -- circulation and fine rules > cap_fine_to_replacement_price BOOLEAN NOT NULL DEFAULT '0', -- cap the fine based on item's replacement price > onshelfholds tinyint(1) NOT NULL default 0, -- allow holds for items that are on shelf > opacitemholds char(1) NOT NULL default 'N', -- allow opac users to place specific items on hold >+ allow_hold_if_items_available TINYINT(1) NOT NULL DEFAULT 1, -- allow holds or not if other items available > PRIMARY KEY (`branchcode`,`categorycode`,`itemtype`), > KEY `categorycode` (`categorycode`), > KEY `itemtype` (`itemtype`) >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >index 52f749c..3b6ac51 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt >@@ -172,6 +172,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > <th>Holds allowed (count)</th> > <th>On shelf holds allowed</th> > <th>Item level holds</th> >+ <th>Allow hold if others available</th> > <th>Rental discount (%)</th> > <th colspan="2"> </th> > </tr> >@@ -250,6 +251,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > <td>[% rule.reservesallowed %]</td> > <td>[% IF rule.onshelfholds %]Yes[% ELSE %]No[% END %]</td> > <td>[% IF rule.opacitemholds == 'F'%]Force[% ELSIF rule.opacitemholds == 'Y'%]Allow[% ELSE %]Don't allow[% END %]</td> >+ <td>[% IF rule.allow_hold_if_items_available %]Yes[% ELSE %]No[% END %]</td> > <td>[% rule.rentaldiscount %]</td> > <td><a href="#" class="editrule">Edit</a></td> > <td> >@@ -328,6 +330,12 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > <option value="F">Force</option> > </select> > </td> >+ <td> >+ <select name="allow_hold_if_items_available" id="allow_hold_if_items_available"> >+ <option value="0">No</option> >+ <option value="1" selected="selected">Yes</option> >+ </select> >+ </td> > <td><input type="text" name="rentaldiscount" id="rentaldiscount" size="2" /></td> > <td colspan="2"> > <input type="hidden" name="branch" value="[% current_branch %]"/> >@@ -359,6 +367,7 @@ for="tobranch"><strong>Clone these rules to:</strong></label> <input type="hidde > <th>Holds allowed (count)</th> > <th>On shelf holds allowed</th> > <th>Item level holds</th> >+ <th>Allow hold if others available</th> > <th>Rental discount (%)</th> > <th colspan="2"> </th> > </tr> >diff --git a/t/db_dependent/Holds/AllowHoldIfItemsAvailable.t b/t/db_dependent/Holds/AllowHoldIfItemsAvailable.t >new file mode 100755 >index 0000000..caaa46a >--- /dev/null >+++ b/t/db_dependent/Holds/AllowHoldIfItemsAvailable.t >@@ -0,0 +1,123 @@ >+#!/usr/bin/perl >+ >+use Modern::Perl; >+ >+use C4::Context; >+use C4::Items; >+use C4::Circulation; >+use Koha::IssuingRule; >+ >+use Test::More tests => 4; >+ >+use t::lib::TestBuilder; >+ >+BEGIN { >+ use FindBin; >+ use lib $FindBin::Bin; >+ use_ok('C4::Reserves'); >+} >+ >+my $schema = Koha::Database->schema; >+$schema->storage->txn_begin; >+my $dbh = C4::Context->dbh; >+ >+my $builder = t::lib::TestBuilder->new; >+ >+my $library1 = $builder->build({ >+ source => 'Branch', >+}); >+ >+# Now, set a userenv >+C4::Context->_new_userenv('xxx'); >+C4::Context->set_userenv(0,0,0,'firstname','surname', $library1->{branchcode}, 'Midway Public Library', '', '', ''); >+ >+my $bib_title = "Test Title"; >+ >+my $borrower1 = $builder->build({ >+ source => 'Borrower', >+ value => { >+ categorycode => 'S', >+ branchcode => $library1->{branchcode}, >+ dateexpiry => '3000-01-01', >+ } >+}); >+ >+my $borrower2 = $builder->build({ >+ source => 'Borrower', >+ value => { >+ categorycode => 'S', >+ branchcode => $library1->{branchcode}, >+ dateexpiry => '3000-01-01', >+ } >+}); >+ >+# Test hold_fulfillment_policy >+my ( $itemtype ) = @{ $dbh->selectrow_arrayref("SELECT itemtype FROM itemtypes LIMIT 1") }; >+my $borrowernumber1 = $borrower1->{borrowernumber}; >+my $borrowernumber2 = $borrower2->{borrowernumber}; >+my $library_A = $library1->{branchcode}; >+ >+$dbh->do("INSERT INTO biblio (frameworkcode, author, title, datecreated) VALUES ('', 'Koha test', '$bib_title', '2011-02-01')"); >+ >+my $biblionumber = $dbh->selectrow_array("SELECT biblionumber FROM biblio WHERE title = '$bib_title'") >+ or BAIL_OUT("Cannot find newly created biblio record"); >+ >+$dbh->do("INSERT INTO biblioitems (biblionumber, marcxml, itemtype) VALUES ($biblionumber, '', '$itemtype')"); >+ >+my $biblioitemnumber = >+ $dbh->selectrow_array("SELECT biblioitemnumber FROM biblioitems WHERE biblionumber = $biblionumber") >+ or BAIL_OUT("Cannot find newly created biblioitems record"); >+ >+$dbh->do(" >+ INSERT INTO items (barcode, biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype) >+ VALUES ('AllowHoldIf1', $biblionumber, $biblioitemnumber, '$library_A', '$library_A', 0, 0, 0, 0, NULL, '$itemtype') >+"); >+ >+my $itemnumber1 = >+ $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber") >+ or BAIL_OUT("Cannot find newly created item"); >+ >+my $item1 = GetItem( $itemnumber1 ); >+ >+$dbh->do(" >+ INSERT INTO items (barcode, biblionumber, biblioitemnumber, homebranch, holdingbranch, notforloan, damaged, itemlost, withdrawn, onloan, itype) >+ VALUES ('AllowHoldIf2', $biblionumber, $biblioitemnumber, '$library_A', '$library_A', 0, 0, 0, 0, NULL, '$itemtype') >+"); >+ >+my $itemnumber2 = >+ $dbh->selectrow_array("SELECT itemnumber FROM items WHERE biblionumber = $biblionumber ORDER BY itemnumber DESC") >+ or BAIL_OUT("Cannot find newly created item"); >+ >+my $item2 = GetItem( $itemnumber2 ); >+ >+$dbh->do("DELETE FROM issuingrules"); >+my $rule = Koha::IssuingRule->new( >+ { >+ categorycode => '*', >+ itemtype => '*', >+ branchcode => '*', >+ maxissueqty => 99, >+ issuelength => 7, >+ lengthunit => 8, >+ reservesallowed => 99, >+ onshelfholds => 1, >+ allow_hold_if_items_available => 0, >+ } >+); >+$rule->store(); >+ >+my $can = CanItemBeReserved( $borrowernumber1, $itemnumber1); >+is( $can, 'itemsAvailable', "Item can be held" ); >+ >+AddIssue( $borrower2, $item1->{barcode} ); >+ >+$can = CanItemBeReserved( $borrowernumber1, $itemnumber1); >+is( $can, 'itemsAvailable', "Item can be held" ); >+ >+AddIssue( $borrower2, $item2->{barcode} ); >+ >+$can = CanItemBeReserved( $borrowernumber1, $itemnumber1); >+is( $can, 'OK', "Item can be held" ); >+ >+# Cleanup >+$schema->storage->txn_rollback; >-- >2.1.4
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 15534
:
46463
|
48039
|
48764
|
49223
|
49406
|
50326
|
50389
|
50390
|
50802
|
50804
|
50805
|
50806
|
50807
|
50808
|
50809
|
50813
|
50824
|
50825
|
50826
|
50827