From 0509d427b9ef7358cc7d044f19c4be2039ccfced Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Fri, 10 Mar 2017 10:59:41 -0300
Subject: [PATCH] Bug 17560: Improve strenght of hold existence test

This patch is a QA follow-up to fix several issues:
- 1 call to GetReserveFee was wrong in ModReserveFill
- Update DB entry was wrong and insufficient
- Add robustness to the tests in sco-main

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 C4/Reserves.pm                                |  2 +-
 .../data/mysql/atomicupdate/bug_XXXXX.sql     |  3 ++-
 opac/sco/sco-main.pl                          | 25 ++++++++++++++++---
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index fff932cbf7..1598eb6581 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -1184,7 +1184,7 @@ sub ModReserveFill {
     $hold->delete();
 
     if ( C4::Context->preference('HoldFeeMode') eq 'any_time_is_collected' ) {
-        my $reserve_fee = GetReserveFee( $hold->borrowernumber );
+        my $reserve_fee = GetReserveFee( $hold->borrowernumber, $hold->biblionumber );
         ChargeReserveFee( $hold->borrowernumber, $reserve_fee, $hold->biblio->title );
     }
 
diff --git a/installer/data/mysql/atomicupdate/bug_XXXXX.sql b/installer/data/mysql/atomicupdate/bug_XXXXX.sql
index f7526e14d0..96c142f592 100644
--- a/installer/data/mysql/atomicupdate/bug_XXXXX.sql
+++ b/installer/data/mysql/atomicupdate/bug_XXXXX.sql
@@ -1 +1,2 @@
-UPDATE systempreferences SET options="any_time_is_placed|not_always|any_time_is_collected" WHERE value="HoldFeeMode";
+UPDATE systempreferences SET options="any_time_is_placed|not_always|any_time_is_collected" WHERE variable="HoldFeeMode";
+UPDATE systempreferences SET value="any_time_is_placed" WHERE variable="HoldFeeMode" AND value="always";
diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl
index e4c77e030f..b313df9aee 100755
--- a/opac/sco/sco-main.pl
+++ b/opac/sco/sco-main.pl
@@ -200,12 +200,22 @@ elsif ( $op eq "checkout" ) {
         }
     } else {
         if ( $confirmed || $issuenoconfirm ) {    # we'll want to call getpatroninfo again to get updated issues.
-            my $hold_existed;
+            my ( $hold_existed, $item );
             if ( C4::Context->preference('HoldFeeMode') eq 'any_time_is_collected' ) {
                 # There is no easy way to know if the patron has been charged for this item.
                 # So we check if a hold existed for this item before the check in
-                my $item = Koha::Items->find({ barcode => $barcode });
-                $hold_existed = Koha::Holds->search({ -or => { 'biblionumber' => $item->biblionumber, 'itemnumber' => $item->itemnumber}})->count;
+                $item = Koha::Items->find({ barcode => $barcode });
+                $hold_existed = Koha::Holds->search(
+                    {
+                        -and => {
+                            borrowernumber => $borrower->{borrowernumber},
+                            -or            => {
+                                biblionumber => $item->biblionumber,
+                                itemnumber   => $item->itemnumber
+                            }
+                        }
+                    }
+                )->count;
             }
             AddIssue( $borrower, $barcode );
 
@@ -214,7 +224,14 @@ elsif ( $op eq "checkout" ) {
                 $template->param(
                     # If the hold existed before the check in, let's confirm that the charge line exists
                     # Note that this should not be needed but since we do not have proper exception handling here we do it this way
-                    patron_has_hold_fee => Koha::Account::Lines->search({ borrowernumber => $borrower->{borrowernumber}, accounttype => 'Res', date => $dtf->format_date( dt_from_string ) })->count,
+                    patron_has_hold_fee => Koha::Account::Lines->search(
+                        {
+                            borrowernumber => $borrower->{borrowernumber},
+                            accounttype    => 'Res',
+                            description    => 'Reserve Charge - ' . $item->biblio->title,
+                            date           => $dtf->format_date(dt_from_string)
+                        }
+                      )->count,
                 );
             }
         } else {