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

@@ -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 );
     }
 

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";

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 });
+                $item = Koha::Items->find({ barcode => $barcode });
-                $hold_existed = Koha::Holds->search({ -or => { 'biblionumber' => $item->biblionumber, 'itemnumber' => $item->itemnumber}})->count;
+                $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 {