From 2667ac2fad2669231fb7ba35b4c11fbd9ab03d8e Mon Sep 17 00:00:00 2001 From: Alex Arnaud Date: Wed, 6 May 2015 14:58:20 +0200 Subject: [PATCH] Add maxreserves and alreadyreserved checks when placing hold via ILSDI http://bugs.koha-community.org/show_bug.cgi?id=14155 Signed-off-by: Indranil Das Gupta (L2C2 Technologies) --- C4/ILSDI/Services.pm | 12 +++++-- C4/Reserves.pm | 49 +++++++++++++++++++++++++++ reserve/request.pl | 19 +++-------- t/db_dependent/ILSDI_Services.t | 75 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 138 insertions(+), 17 deletions(-) diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 5d3f882..413b316 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -26,7 +26,7 @@ use C4::Circulation; use C4::Branch; use C4::Accounts; use C4::Biblio; -use C4::Reserves qw(AddReserve GetReservesFromBiblionumber GetReservesFromBorrowernumber CanBookBeReserved CanItemBeReserved IsAvailableForItemLevelRequest); +use C4::Reserves qw(AddReserve GetReservesFromBiblionumber GetReservesFromBorrowernumber CanBookBeReserved CanItemBeReserved IsAvailableForItemLevelRequest AlreadyReserved ReachedMaxReserves); use C4::Context; use C4::AuthoritiesMarc; use XML::Simple; @@ -605,9 +605,13 @@ sub HoldTitle { my $biblionumber = $cgi->param('bib_id'); my $biblio = GetBiblio( $biblionumber ); return { code => 'RecordNotFound' } unless $$biblio{biblionumber}; - + my $title = $$biblio{title}; + return { code => 'NotHoldable' } if ReachedMaxReserves($borrowernumber); + + return { code => 'NotHoldable' } if AlreadyReserved($biblionumber, $borrowernumber); + # Check if the biblio can be reserved return { code => 'NotHoldable' } unless CanBookBeReserved( $borrowernumber, $biblionumber ) eq 'OK'; @@ -684,6 +688,10 @@ sub HoldItem { # If the biblio does not match the item, return an error code return { code => 'RecordNotFound' } if $$item{biblionumber} ne $$biblio{biblionumber}; + return { code => 'NotHoldable' } if ReachedMaxReserves($borrowernumber); + + return { code => 'NotHoldable' } if AlreadyReserved($biblionumber, $borrowernumber); + # Check for item disponibility my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber ); my $canbookbereserved = C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber ); diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 9c760b6..c2dc369 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -124,6 +124,9 @@ BEGIN { &CancelReserve &CancelExpiredReserves + &AlreadyReserved + &ReachedMaxReserves + &AutoUnsuspendReserves &IsAvailableForItemLevelRequest @@ -1109,6 +1112,52 @@ sub CancelExpiredReserves { } +=head2 AlreadyReserved + + $alreadyreserved = AlreadyReserved($biblionumber, $borrowernumber); + + return 1 if borrower already reserved this biblio. + +=cut + +sub AlreadyReserved { + my ($biblionumber, $borrowernumber) = @_; + + return 0 unless $biblionumber && $borrowernumber; + + my $reserves = GetReservesFromBiblionumber({ biblionumber => $biblionumber, all_dates => 1 }); + + return 0 unless scalar( @$reserves ); + + foreach my $res (@$reserves) { + return 1 if $borrowernumber eq $res->{borrowernumber}; + } + + return 0; +} + +=head2 ReachedMaxReserves + + $hasmaxreserves = ReachedMaxReserves($borrowernumber); + + return 1 if borrower reached maxreserves. + +=cut + +sub ReachedMaxReserves { + my $borrowernumber = shift; + + my $number_reserves = GetReserveCount( $borrowernumber ); + + return 0 unless C4::Context->preference('maxreserves'); + + if ( $number_reserves >= C4::Context->preference('maxreserves') ) { + return 1; + } + + return 0; +} + =head2 AutoUnsuspendReserves AutoUnsuspendReserves(); diff --git a/reserve/request.pl b/reserve/request.pl index 62f82a8..9faae06 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -135,11 +135,7 @@ if ($borrowernumber_hold && !$action) { # we check the reserves of the borrower, and if he can reserv a document # FIXME At this time we have a simple count of reservs, but, later, we could improve the infos "title" ... - - my $number_reserves = - GetReserveCount( $borrowerinfo->{'borrowernumber'} ); - - if ( C4::Context->preference('maxreserves') && ($number_reserves >= C4::Context->preference('maxreserves')) ) { + if ( ReachedMaxReserves($borrowerinfo->{'borrowernumber'}) ) { $warnings = 1; $maxreserves = 1; } @@ -225,23 +221,18 @@ foreach my $biblionumber (@biblionumbers) { my $reserves = GetReservesFromBiblionumber({ biblionumber => $biblionumber, all_dates => 1 }); my $count = scalar( @$reserves ); my $totalcount = $count; - my $holds_count = 0; my $alreadyreserved = 0; foreach my $res (@$reserves) { if ( defined $res->{found} ) { # found can be 'W' or 'T' $count--; } - - if ( defined $borrowerinfo && defined($borrowerinfo->{borrowernumber}) && ($borrowerinfo->{borrowernumber} eq $res->{borrowernumber}) ) { - $holds_count++; - } } - if ( $holds_count ) { - $alreadyreserved = 1; - $biblioloopiter{warn} = 1; - $biblioloopiter{alreadyres} = 1; + if ( AlreadyReserved($biblionumber, $borrowerinfo->{borrowernumber}) ) { + $alreadyreserved = 1; + $biblioloopiter{warn} = 1; + $biblioloopiter{alreadyres} = 1; } $template->param( diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index 9aee73e..473c826 100644 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -4,9 +4,13 @@ use Modern::Perl; use C4::Members qw/AddMember GetMember GetBorrowercategory/; use C4::Branch; +use C4::Biblio; +use C4::Items; +use C4::Reserves; use CGI qw ( -utf8 ); +use t::lib::Mocks; -use Test::More tests => 15; +use Test::More tests => 23; BEGIN { use_ok('C4::ILSDI::Services'); @@ -18,6 +22,9 @@ my $dbh = C4::Context->dbh; $dbh->{AutoCommit} = 0; $dbh->{RaiseError} = 1; +my $maxallowedreserves = 3; +t::lib::Mocks::mock_preference('maxreserves', $maxallowedreserves); + # Create patron my %data = ( firstname => 'my firstname', @@ -29,6 +36,16 @@ my %data = ( password => 'ilsdi-password', ); +my %data2 = ( + firstname => 'Joe', + surname => 'John', + categorycode => 'UT', + branchcode => 'UT', + cardnumber => '99999', + userid => 'jjohn', + password => 'ilsdi-password', +); + # Crate patron category unless ( GetBorrowercategory('UT') ) { $dbh->do("INSERT INTO categories @@ -44,8 +61,28 @@ unless ( GetBranchDetail('UT') ) { my $borrowernumber = AddMember(%data); +my $borrowernumber2 = AddMember(%data2); my $borrower = GetMember( borrowernumber => $borrowernumber ); +# Create biblio and item +my %item_branch_infos = ( + homebranch => 'UT', + holdingbranch => 'UT', +); + +my $record = MARC::Record->new; +my $title = MARC::Field->new('200','','','a' => 'Hello world.'); +$record->append_fields($title); + +my ($biblionumber) = AddBiblio($record, ''); +my ($biblionumber2) = AddBiblio(MARC::Record->new, ''); +my ($biblionumber3) = AddBiblio(MARC::Record->new, ''); +my ($biblionumber4) = AddBiblio(MARC::Record->new, ''); +my $itemnumber = AddItem({ barcode => '0101', %item_branch_infos }, $biblionumber); +my $itemnumber2 = AddItem({ barcode => '0201', %item_branch_infos }, $biblionumber2); +my $itemnumber3 = AddItem({ barcode => '0301', %item_branch_infos }, $biblionumber3); +my $itemnumber4 = AddItem({ barcode => '0401', %item_branch_infos }, $biblionumber4); + { # AuthenticatePatron test my $query = new CGI; @@ -90,3 +127,39 @@ my $borrower = GetMember( borrowernumber => $borrowernumber ); } +{ # Add reserves tests + + my $query = new CGI; + $query->param('patron_id',$borrowernumber); + $query->param('bib_id', $biblionumber); + $query->param('request_location','127.0.0.1'); + my $reply = C4::ILSDI::Services::HoldTitle($query); + is($reply->{'title'}, 'Hello world.'); + is($reply->{'pickup_location'}, 'Unit test library', 'Hold title'); + + $reply = C4::ILSDI::Services::HoldTitle($query); + is($reply->{'code'}, 'NotHoldable', 'Hold same title again - ILSDI'); + + $query->param('bib_id', $biblionumber2); + $reply = C4::ILSDI::Services::HoldTitle($query); + is($reply->{'pickup_location'}, 'Unit test library', 'Hold a second title - ILSDI'); + + $query->param('bib_id', $biblionumber3); + $reply = C4::ILSDI::Services::HoldTitle($query); + is($reply->{'pickup_location'}, 'Unit test library', 'Hold a third title - ILSDI'); + + $query->param('bib_id', $biblionumber4); + $reply = C4::ILSDI::Services::HoldTitle($query); + is($reply->{'code'}, 'NotHoldable', 'Hold a fourth title - ILSDI'); + + $query->param('bib_id', $biblionumber4); + $query->param('item_id', $itemnumber4); + $reply = C4::ILSDI::Services::HoldItem($query); + is($reply->{'code'}, 'NotHoldable', 'Hold a fourth item - ILSDI'); + + $query->param('bib_id', $biblionumber); + $query->param('patron_id',$borrowernumber2); + $query->param('item_id', $itemnumber); + $reply = C4::ILSDI::Services::HoldItem($query); + is($reply->{'pickup_location'}, 'Unit test library', 'Joe John hold an item - ILSDI'); +} -- 1.9.1