Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 29094: Adding hold via SIP should check if patron can hold item first

Browse source 
When placing holds via SIP2, there is no holdability check. This seems very incorrect.

Test Plan:
1) Apply this patch
2) prove -r t/db_dependent/SIP

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(cherry picked from commit 7b21fb020b)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Kyle Hall 2021-09-23 08:57:30 -04:00 committed by Lucas Gass
parent 9456af97be
commit 63bd50e721
3 changed files with 89 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
C4/SIP/ILS/Transaction/Hold.pm
View file

@ -7,7 +7,7 @@ use Modern::Perl;
use C4::SIP::ILS::Transaction;
use C4::Reserves qw( CalculatePriority AddReserve ModReserve );
use C4::Reserves qw( CalculatePriority AddReserve ModReserve CanItemBeReserved );
use Koha::Holds;
use Koha::Patrons;
use Koha::Items;
@ -61,18 +61,23 @@ sub do_hold {
return $self;
}
my $priority = C4::Reserves::CalculatePriority($item->biblionumber);
AddReserve(
{
priority => $priority,
branchcode => $branch,
borrowernumber => $patron->borrowernumber,
biblionumber => $item->biblionumber
}
);
my $canReserve = CanItemBeReserved($patron, $item, $branch);
if ($canReserve->{status} eq 'OK') {
my $priority = C4::Reserves::CalculatePriority($item->biblionumber);
AddReserve(
{
priority => $priority,
branchcode => $branch,
borrowernumber => $patron->borrowernumber,
biblionumber => $item->biblionumber
}
);
$self->ok(1);
} else {
$self->ok(0);
}
# unfortunately no meaningful return value
$self->ok(1);
return $self;
}

9
t/db_dependent/SIP/ILS.t
View file

@ -74,7 +74,14 @@ is( $ils->test_cardnumber_compare( 'A1234', 'b1234' ),
subtest add_hold => sub {
plan tests => 4;
my $library = $builder->build_object( { class => 'Koha::Libraries' } );
my $library = $builder->build_object(
{
class => 'Koha::Libraries',
value => {
pickup_location => 1
}
}
);
my $patron = $builder->build_object(
{
class => 'Koha::Patrons',

66
t/db_dependent/SIP/Transaction.t
View file

@ -4,7 +4,7 @@
# Current state is very rudimentary. Please help to extend it!
use Modern::Perl;
use Test::More tests => 14;
use Test::More tests => 15;
use Koha::Database;
use t::lib::TestBuilder;
@ -212,7 +212,14 @@ subtest cancel_hold => sub {
subtest do_hold => sub {
plan tests => 8;
my $library = $builder->build_object( { class => 'Koha::Libraries' } );
my $library = $builder->build_object(
{
class => 'Koha::Libraries',
value => {
pickup_location => 1
}
}
);
my $patron_1 = $builder->build_object(
{
class => 'Koha::Patrons',
@ -271,6 +278,61 @@ subtest do_hold => sub {
is( $THE_hold->branchcode, $patron_2->branchcode, 'Hold placed from SIP should have the branchcode set' );
};
subtest "Placing holds via SIP check CanItemBeReserved" => sub {
plan tests => 4;
my $library = $builder->build_object(
{
class => 'Koha::Libraries',
value => {
pickup_location => 0
}
}
);
my $patron_1 = $builder->build_object(
{
class => 'Koha::Patrons',
value => {
branchcode => $library->branchcode,
}
}
);
my $patron_2 = $builder->build_object(
{
class => 'Koha::Patrons',
value => {
branchcode => $library->branchcode,
categorycode => $patron_1->categorycode,
}
}
);
t::lib::Mocks::mock_userenv(
{ branchcode => $library->branchcode, flags => 1 } );
my $item = $builder->build_sample_item(
{
library => $library->branchcode,
}
);
my $sip_patron = C4::SIP::ILS::Patron->new( $patron_2->cardnumber );
my $sip_item = C4::SIP::ILS::Item->new( $item->barcode );
my $transaction = C4::SIP::ILS::Transaction::Hold->new();
is(
ref $transaction,
"C4::SIP::ILS::Transaction::Hold",
"New transaction created"
);
is( $transaction->patron($sip_patron),
$sip_patron, "Patron assigned to transaction" );
is( $transaction->item($sip_item),
$sip_item, "Item assigned to transaction" );
my $hold = $transaction->do_hold();
is( $transaction->ok, 0, "Hold was not allowed" );
};
subtest do_checkin => sub {
plan tests => 12;