From bc72a6d508f1ce93ba190153966c1c9667414e48 Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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 <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 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