From 85cfe5d546d11c7807dddc78d28174a42e182b05 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@helsinki.fi>
Date: Fri, 19 Feb 2021 09:49:51 +0200
Subject: [PATCH] Bug 27058: Remove confusing and unnecessary $biblionumber1
 variable
Content-Type: text/plain; charset=utf-8

The code is much more understandable now because with $biblio2 and
$biblionumber1 variables use to point to the same biblio one might
have thought $biblionumber1 points to $biblio1 which in this case is
not true. Let's just drop the extra variable because the object
notation of accessing is just as simple.

To test:
 1) prove t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t => passes

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
index eb7284bf84..13600eb871 100755
--- a/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
+++ b/t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
@@ -326,16 +326,15 @@ subtest 'Check holds availability with different item types' => sub {
     # (Bug 24683):
 
     my $biblio2       = $builder->build_sample_biblio( { itemtype => $itemtype } );
-    my $biblionumber1 = $biblio2->biblionumber;
     my $item4         = $builder->build_sample_item(
-        {   biblionumber  => $biblionumber1,
+        {   biblionumber  => $biblio2->biblionumber,
             itype         => $itemtype,
             homebranch    => $library_A,
             holdingbranch => $library_A
         }
     );
     my $item5 = $builder->build_sample_item(
-        {   biblionumber  => $biblionumber1,
+        {   biblionumber  => $biblio2->biblionumber,
             itype         => $itemtype2,
             homebranch    => $library_A,
             holdingbranch => $library_A
@@ -371,7 +370,7 @@ subtest 'Check holds availability with different item types' => sub {
         }
     );
 
-    $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber1, patron => $patron1 } );
+    $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblio2->biblionumber, patron => $patron1 } );
     is( $is, 1, "Items availability: 2 items, one allowed by smart rule but not checked out, another one not allowed to be held by smart rule" );
 
     $is = IsAvailableForItemLevelRequest( $item4, $patron1 );
@@ -382,7 +381,7 @@ subtest 'Check holds availability with different item types' => sub {
 
     AddIssue( $patron2->unblessed, $item4->barcode );
 
-    $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber1, patron => $patron1 } );
+    $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblio2->biblionumber, patron => $patron1 } );
     is( $is, 0, "Items availability: 2 items, one allowed by smart rule and checked out, another one not allowed to be held by smart rule" );
 
     $is = IsAvailableForItemLevelRequest( $item4, $patron1 );
@@ -397,9 +396,8 @@ subtest 'Check item checkout availability with ordered item' => sub {
     plan tests => 1;
 
     my $biblio2       = $builder->build_sample_biblio( { itemtype => $itemtype } );
-    my $biblionumber1 = $biblio2->biblionumber;
     my $item1 = $builder->build_sample_item(
-        {   biblionumber  => $biblionumber1,
+        {   biblionumber  => $biblio2->biblionumber,
             itype         => $itemtype2,
             homebranch    => $library_A,
             holdingbranch => $library_A,
@@ -422,7 +420,7 @@ subtest 'Check item checkout availability with ordered item' => sub {
         }
     );
 
-    $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblionumber1, patron => $patron1 } );
+    $is = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblio2->biblionumber, patron => $patron1 } );
     is( $is, 0, "Ordered item cannot be checked out" );
 };
 
-- 
2.11.0