From 53c68b937949e3b88e8a30c3669aa5c44255feb0 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 10 May 2021 16:30:31 +0100 Subject: [PATCH] Bug 27600: Remove password check from `add_hold` As suggested in the bugzilla comments, the add_hold method also doesn't require the password checking code. Test plan 1/ Run t/db_dependent/SIP/ILS.t and watch it pass Signed-off-by: Nick Clemens Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall (cherry picked from commit bc72a6d508f1ce93ba190153966c1c9667414e48) Signed-off-by: Fridolin Somers --- C4/SIP/ILS.pm | 9 +++---- t/db_dependent/SIP/ILS.t | 53 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm index fdc493e708..112eb201c3 100644 --- a/C4/SIP/ILS.pm +++ b/C4/SIP/ILS.pm @@ -296,11 +296,10 @@ sub add_hold { my $trans = C4::SIP::ILS::Transaction::Hold->new(); - $patron = C4::SIP::ILS::Patron->new( $patron_id); - if (!$patron - || (defined($patron_pwd) && !$patron->check_password($patron_pwd))) { - $trans->screen_msg("Invalid Patron."); - return $trans; + $patron = C4::SIP::ILS::Patron->new( $patron_id ); + if ( !$patron ) { + $trans->screen_msg("Invalid patron barcode."); + return $trans; } unless ($item = C4::SIP::ILS::Item->new($item_id || $title_id)) { diff --git a/t/db_dependent/SIP/ILS.t b/t/db_dependent/SIP/ILS.t index bd8d670a08..ebfdeea9c6 100755 --- a/t/db_dependent/SIP/ILS.t +++ b/t/db_dependent/SIP/ILS.t @@ -20,7 +20,7 @@ use Modern::Perl; -use Test::More tests => 12; +use Test::More tests => 13; use t::lib::TestBuilder; use t::lib::Mocks; @@ -71,6 +71,57 @@ is( $ils->test_cardnumber_compare( 'A1234', 'a1234' ), is( $ils->test_cardnumber_compare( 'A1234', 'b1234' ), q{}, 'borrower bc test identifies difference' ); +subtest add_hold => sub { + plan tests => 4; + + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + branchcode => $library->branchcode, + } + } + ); + t::lib::Mocks::mock_userenv( + { branchcode => $library->branchcode, flags => 1 } ); + + my $item = $builder->build_sample_item( + { + library => $library->branchcode, + } + ); + + Koha::CirculationRules->set_rules( + { + categorycode => $patron->categorycode, + branchcode => $library->branchcode, + itemtype => $item->effective_itemtype, + rules => { + onshelfholds => 1, + reservesallowed => 3, + holds_per_record => 3, + issuelength => 5, + lengthunit => 'days', + } + } + ); + + my $ils = C4::SIP::ILS->new( { id => $library->branchcode } ); + + # Send empty AD segments (i.e. empty string for patron_pwd) + my $transaction = $ils->add_hold( $patron->cardnumber, "", $item->barcode, undef ); + isnt( + $transaction->{screen_msg}, + 'Invalid patron password.', + "Empty password succeeds" + ); + ok( $transaction->{ok}, "Transaction returned success"); + is( $item->biblio->holds->count(), 1, "Hold was placed on bib"); + # FIXME: Should we not allow for item-level holds when we're passed an item barcode... + is( $item->holds->count(),0,"Hold was placed at bib level"); +}; + subtest cancel_hold => sub { plan tests => 6; -- 2.39.5