Bug 14702: [QA Follow-up] More readable variable names, less queries

The names are much better now :)
Combined the queries for items and issues.
Only check the number of holds when needed.

Test plan:
Verify the changes here by running the unit test again.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index 639692b..dab2b48 100644 (file)
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -672,18 +672,30 @@ sub GetReserveFee {
     my $borquery = qq{
 SELECT reservefee FROM borrowers LEFT JOIN categories ON borrowers.categorycode = categories.categorycode WHERE borrowernumber = ?
     };
+    my $issue_qry = qq{
+SELECT COUNT(*) FROM items
+LEFT JOIN issues USING (itemnumber)
+WHERE items.biblionumber=? AND issues.issue_id IS NULL
+    };
+    my $holds_qry = qq{
+SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>?
+    };
 
     my $dbh = C4::Context->dbh;
     my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) );
     if( $fee && $fee > 0 ) {
         # This is a reconstruction of the old code:
-        # Calculate number of items, items issued and holds
-        my ( $cnt1 ) = $dbh->selectrow_array( "SELECT COUNT(*) FROM items WHERE biblionumber=?", undef, ( $biblionumber ) );
-        my ( $cnt2 ) = $dbh->selectrow_array( "SELECT COUNT(*) FROM issues LEFT JOIN items USING (itemnumber) WHERE biblionumber=?", undef, ( $biblionumber ));
-        my ( $cnt3 ) = $dbh->selectrow_array( "SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>?", undef, ( $biblionumber, $borrowernumber ) );
+        # Compare number of items with items issued, and optionally check holds
         # If not all items are issued and there are no holds: charge no fee
         # NOTE: Lost, damaged, not-for-loan, etc. are just ignored here
-        $fee = 0 if $cnt1 - $cnt2 > 0 && $cnt3 == 0;
+        my ( $notissued, $reserved );
+        ( $notissued ) = $dbh->selectrow_array( $issue_qry, undef,
+            ( $biblionumber ) );
+        if( $notissued ) {
+            ( $reserved ) = $dbh->selectrow_array( $holds_qry, undef,
+                ( $biblionumber, $borrowernumber ) );
+            $fee = 0 if $reserved == 0;
+        }
     }
     return $fee;
 }