Attachment 120830 Details for Bug 27600 – Bug 27600: Remove password check from `add_hold`

[patch] Bug 27600: Remove password check from `add_hold`

Bug-27600-Remove-password-check-from-addhold.patch (text/plain), 3.45 KB, created by Nick Clemens (kidclamp) on 2021-05-11 10:15:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2021-05-11 10:15:16 UTC

Size: 3.45 KB

patch

obsolete

>From cc082b15557b609be06dc007b11eedeb8cfce8ea 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>
>---
> 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.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 27600: 116243 | 116245 | 116246 | 120266 | 120267 | 120796 | 120797 | 120798 | 120799 | 120829 | 120830