Bugzilla – Attachment 38884 Details for
Bug 14155
Placing holds via ILSDI bypass some checks
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Add maxreserves and alreadyreserved checks when placing hold via ILSDI
0001-Add-maxreserves-and-alreadyreserved-checks-when-plac.patch (text/plain), 9.38 KB, created by
Alex Arnaud
on 2015-05-06 13:03:04 UTC
(
hide
)
Description:
Add maxreserves and alreadyreserved checks when placing hold via ILSDI
Filename:
MIME Type:
Creator:
Alex Arnaud
Created:
2015-05-06 13:03:04 UTC
Size:
9.38 KB
patch
obsolete
>From 1519942e2680d9e0b1047fa63cfccdccc6d1a121 Mon Sep 17 00:00:00 2001 >From: Alex Arnaud <alex.arnaud@biblibre.com> >Date: Wed, 6 May 2015 14:58:20 +0200 >Subject: [PATCH] Add maxreserves and alreadyreserved checks when placing hold > via ILSDI > >--- > C4/ILSDI/Services.pm | 12 +++++-- > C4/Reserves.pm | 51 +++++++++++++++++++++++++- > reserve/request.pl | 19 +++------- > t/db_dependent/ILSDI_Services.t | 75 ++++++++++++++++++++++++++++++++++++++- > 4 files changed, 139 insertions(+), 18 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 5874260..90292c4 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -119,11 +119,14 @@ BEGIN { > > &CheckReserves > &CanBookBeReserved >- &CanItemBeReserved >+ &CanItemBeReserved > &CanReserveBeCanceledFromOpac > &CancelReserve > &CancelExpiredReserves > >+ &AlreadyReserved >+ &ReachedMaxReserves >+ > &AutoUnsuspendReserves > > &IsAvailableForItemLevelRequest >@@ -1093,6 +1096,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 c30fb21..bb505f3 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.7.10.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 14155
:
38884
|
40170
|
40174
|
40175
|
40176
|
40208
|
40210
|
40239
|
40240
|
40241
|
44047
|
44048
|
44049
|
44050
|
44064