From ac840a846011f8f6f24686ef68f73e7b59f9704e Mon Sep 17 00:00:00 2001 From: Alex Arnaud Date: Wed, 10 Jan 2018 15:31:54 +0000 Subject: [PATCH] Bug 19945: ILSDI - Return the reason a reserve is impossible Currently, the ILDSI services HoldTitle and HoldItem always return a "NotHoldable" code is the reserve is impossible. We need to know why Test plan: - Apply this patch - Place a hold on a non reservable title using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=1&bib_id=1&request_location=) - you should get the reason instead of NotHoldable, - Place a hold on a non reservable item using ILS-DI web service (http://koha-opac.example.org/cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=1&bib_id=1&item_id=1) - you should get the reason instead of NotHoldable, Signed-off-by: Mark Tompsett Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall Signed-off-by: Nick Clemens --- C4/ILSDI/Services.pm | 10 ++-- t/db_dependent/ILSDI_Services.t | 91 ++++++++++++++++++++++++++------- 2 files changed, 79 insertions(+), 22 deletions(-) diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 689973ea3e..4b03330428 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -663,7 +663,8 @@ sub HoldTitle { my $title = $biblio ? $biblio->title : ''; # Check if the biblio can be reserved - return { code => 'NotHoldable' } unless CanBookBeReserved( $borrowernumber, $biblionumber )->{status} eq 'OK'; + my $code = CanBookBeReserved( $borrowernumber, $biblionumber )->{status}; + return { code => $code } unless ( $code eq 'OK' ); my $branch; @@ -739,9 +740,10 @@ sub HoldItem { return { code => 'RecordNotFound' } if $$item{biblionumber} ne $biblio->biblionumber; # Check for item disponibility - my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber ); - my $canbookbereserved = C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber ); - return { code => 'NotHoldable' } unless $canbookbereserved->{status} eq 'OK' and $canitembereserved->{status} eq 'OK'; + my $canitembereserved = C4::Reserves::CanItemBeReserved( $borrowernumber, $itemnumber )->{status}; + my $canbookbereserved = C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber )->{status}; + return { code => $canitembereserved } unless $canitembereserved eq 'OK'; + return { code => $canbookbereserved } unless $canbookbereserved eq 'OK'; # Pickup branch management my $branch; diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index 232a54a699..6891660d31 100644 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -297,41 +297,96 @@ subtest 'LookupPatron test' => sub { $schema->storage->txn_rollback; }; -# This is a stub, as it merely is for triggering the GetMarcBiblio call. -subtest 'GetRecords' => sub { +subtest 'Holds test' => sub { - plan tests => 1; + plan tests => 3; $schema->storage->txn_begin; + t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 ); + + my $patron = $builder->build({ + source => 'Borrower', + }); + my $biblio = $builder->build({ source => 'Biblio', - value => { - title => 'Title 1', }, + }); + + my $biblioitems = $builder->build({ + source => 'Biblioitem', + value => { + biblionumber => $biblio->{biblionumber}, + } }); my $item = $builder->build({ source => 'Item', - value => { + value => { biblionumber => $biblio->{biblionumber}, - }, + damaged => 1 + } }); - my $biblioitem = $builder->build({ - source => 'Biblioitem', - value => { + my $query = new CGI; + $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'bib_id', $biblio->{biblionumber}); + + my $reply = C4::ILSDI::Services::HoldTitle( $query ); + is( $reply->{code}, 'damaged', "Item damaged" ); + + my $item_o = Koha::Items->find($item->{itemnumber}); + $item_o->damaged(0)->store; + + my $hold = $builder->build({ + source => 'Reserve', + value => { + borrowernumber => $patron->{borrowernumber}, biblionumber => $biblio->{biblionumber}, - itemnumber => $item->{itemnumber}, - }, + itemnumber => $item->{itemnumber} + } + }); + + $reply = C4::ILSDI::Services::HoldTitle( $query ); + is( $reply->{code}, 'itemAlreadyOnHold', "Item already on hold" ); + + my $biblio2 = $builder->build({ + source => 'Biblio', + }); + + my $biblioitems2 = $builder->build({ + source => 'Biblioitem', + value => { + biblionumber => $biblio2->{biblionumber}, + } }); - my $query = CGI->new({ - 'schema' => 'MARCXML', - 'id' => [ $biblio->{biblionumber} ] + my $item2 = $builder->build({ + source => 'Item', + value => { + biblionumber => $biblio2->{biblionumber}, + damaged => 0 + } + }); + + t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' ); + my $issuingrule = $builder->build({ + source => 'Issuingrule', + value => { + categorycode => $patron->{categorycode}, + itemtype => $item2->{itype}, + branchcode => $patron->{branchcode}, + reservesallowed => 0, + } }); - my $result = C4::ILSDI::Services::GetRecords($query); - ok($result,'There is a result'); + $query = new CGI; + $query->param( 'patron_id', $patron->{borrowernumber}); + $query->param( 'bib_id', $biblio2->{biblionumber}); + $query->param( 'item_id', $item2->{itemnumber}); + + $reply = C4::ILSDI::Services::HoldItem( $query ); + is( $reply->{code}, 'tooManyReserves', "Too many reserves" ); $schema->storage->txn_rollback; -} +}; -- 2.39.5