From 20e0b073a37728dab87cc5b0a0a8c31d75d264bb Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Mon, 13 Jul 2015 21:55:55 +0200
Subject: [PATCH] Bug 14526: MoveReserve should look at future holds too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

At checkout a hold for the same borrower is considered to be filled.
It is consistent to do the same for holds of the same borrower within
[ConfirmFutureHolds] days (if non-zero).

This goal is achieved by adjusting the CheckReserves call within
MoveReserve, adding the lookahead parameter.
I used this occasion to revisit other calls of CheckReserves:
- transferbook: no need to add lookahead; a future hold should not block
  a transfer;
- CanBookBeIssued: no lookahead; future hold does not block an issue;
- CanBookBeRenewed: idem.
- GetOtherReserves (only used in circ/returns): this call might be a
  candidate for lookahead too, but I leave that for another report. It is
  in the context of checkin and transfer, not checkout.

Test plan:
[1] Set ConfirmFutureHolds to zero days. (You may also need to enable
    AllowHoldDateInFuture.)
[2] Place a hold with borrower A on biblio X for tomorrow. Also place a hold
    with borrower B on X for today. (Use biblio level holds.)
[3] Check out item Y of X to borrower A. Ignore the warning for borrower B
    and do not cancel the hold of B (so: confirm checkout).
    Verify that X has still two holds.
[4] Check in Y (without confirming a hold).
[5] Enable ConfirmFutureHolds, say 2 days.
[6] Check out Y to A again. Ignore the warning for B (no cancel). Verify that
    X now only has one hold for borrower B (the hold for A was filled).

Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 C4/Reserves.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/C4/Reserves.pm b/C4/Reserves.pm
index a300a4a..1f4f31a 100644
--- a/C4/Reserves.pm
+++ b/C4/Reserves.pm
@@ -2151,7 +2151,8 @@ If $cancelreserve boolean is set to true, it will remove existing reserve
 sub MoveReserve {
     my ( $itemnumber, $borrowernumber, $cancelreserve ) = @_;
 
-    my ( $restype, $res, $all_reserves ) = CheckReserves( $itemnumber );
+    my $lookahead= C4::Context->preference('ConfirmFutureHolds'); #number of days to look for future holds
+    my ( $restype, $res, $all_reserves ) = CheckReserves( $itemnumber, undef, $lookahead );
     return unless $res;
 
     my $biblionumber     =  $res->{biblionumber};
-- 
2.1.0