Attachment 94871 Details for Bug 15261 – Bug 15261: Check if checkout/reserve request overlaps existing reserves

[patch] Bug 15261: Check if checkout/reserve request overlaps existing reserves

Bug-15261-Check-if-checkoutreserve-request-overlap.patch (text/plain), 36.69 KB, created by Julian Maurice on 2019-10-30 12:15:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Julian Maurice

Created: 2019-10-30 12:15:52 UTC

Size: 36.69 KB

patch

obsolete

>From 8c87c68968cce71cf7d86fddfa07a0727fe75b83 Mon Sep 17 00:00:00 2001
>From: Alex Arnaud <alex.arnaud@biblibre.com>
>Date: Thu, 26 Nov 2015 11:00:22 +0100
>Subject: [PATCH] Bug 15261: Check if checkout/reserve request overlaps
> existing reserves
>
>When checking out or placing hold, we should check if an existing
>reserve whose period overlap exists.
>
>- A user places a hold from opac whose requested period overlaps with an
>  existing reserve period => prevent reserve
>- A librarian places a hold from staff whose requested period overlaps
>  with an existing reserve period => Warn librarian (Ask for
>  confirmation)
>- A librarian makes a checkout from staff whose requested period
>  overlaps with an existing reserve period => Warn librarian (Ask for
>  confirmation)
>
>Test plan:
>
>Enable syspref: AllowHoldDateInFuture OPACAllowHoldDateInFuture
>  PreventChechoutOnSameReservePeriod and PreventReservesOnSamePeriod
>
>1 (staff side):
>- Place a hold on title (which has only one items) level with start date
>  and expiration date.
>- Place another hold (also title level) with period overlaping this
>  reserve.
>- Check you are warned about an existing reserve
>
>2 (staff side):
>- Place a hold on title (which has more than one items) level with start
>  date and expiration date.
>- Place another hold (also title level) with period overlaping this
>  reserve.
>- Check you are NOT warned about an existing reserve. Because it remains
>  at least one item not reserved.
>
>3 (staff side):
>- Place a hold on item level with start date and expiration date.
>- Place another hold on item level with period overlaping this reserve.
>- Check you are warned about an existing reserve.
>
>4 (opac side):
>- Do the same than for staff side. Instead of a warn, reserve is
>  prevented.
>
>5:
>- Place a hold on title (which has only one items) level with start date
>  and expiration date.
>- Try to checkout the unique item from this title with period overlaping
>  the reserve period.
>- Check you are warned about an existing reserve
>
>6:
>- Place a hold on title (which has more than one items) level with start
>  date and expiration date.
>- Checkout an item from this title with period overlaping the reserve
>  period.
>- Check you are NOT warned about an existing reserve.
>
>7:
>- Place a hold on item level with start date and expiration date.
>- Checkout this item period overlaping the reserve period.
>- Check you are warned about an existing reserve
>
>8:
>- Create a checkout (i.e from ${TODAY} to ${TODAY + 10 days}),
>- Place a hold on the same item from ${TODAY + 11 days} to ${TODAY + 30
>days}),
>- Check that you cannot renew the checkout.
>---
> C4/Circulation.pm                             |  45 +++++++-
> C4/Reserves.pm                                |  48 ++++++++
> Koha/DateUtils.pm                             |  44 ++++++-
> circ/circulation.pl                           |   4 +
> ...entchechoutonsamereserveperiod_syspref.sql |   1 +
> ...dd_preventreservesonsameperiod_syspref.sql |   1 +
> installer/data/mysql/sysprefs.sql             |   2 +
> .../admin/preferences/circulation.pref        |  12 ++
> .../prog/en/modules/circ/circulation.tt       |   2 +
> .../prog/en/modules/reserve/placerequest.tt   |  88 ++++++++++++++
> opac/opac-reserve.pl                          |   9 ++
> reserve/placerequest.pl                       |  48 +++++++-
> svc/renew                                     |   2 +-
> t/db_dependent/Circulation.t                  |  62 +++++++++-
> t/db_dependent/Circulation/CanBookBeIssued.t  | 106 +++++++++++++++++
> t/db_dependent/Reserves/ReserveDate.t         | 108 ++++++++++++++++++
> 16 files changed, 575 insertions(+), 7 deletions(-)
> create mode 100644 installer/data/mysql/atomicupdate/bug_15261-add_preventchechoutonsamereserveperiod_syspref.sql
> create mode 100644 installer/data/mysql/atomicupdate/bug_15261-add_preventreservesonsameperiod_syspref.sql
> create mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/reserve/placerequest.tt
> create mode 100644 t/db_dependent/Circulation/CanBookBeIssued.t
> create mode 100644 t/db_dependent/Reserves/ReserveDate.t
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 5880abe691..2c53e10892 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -1036,6 +1036,7 @@ sub CanBookBeIssued {
>                     $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
>                     $needsconfirmation{'resbranchcode'} = $res->{branchcode};
>                     $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'};
>+                    $needsconfirmation{resreserveid} = $res->{reserve_id};
>                 }
>                 elsif ( $restype eq "Reserved" ) {
>                     # The item is on reserve for someone else.
>@@ -1046,9 +1047,31 @@ sub CanBookBeIssued {
>                     $needsconfirmation{'resborrowernumber'} = $patron->borrowernumber;
>                     $needsconfirmation{'resbranchcode'} = $patron->branchcode;
>                     $needsconfirmation{'resreservedate'} = $res->{reservedate};
>+                    $needsconfirmation{resreserveid} = $res->{reserve_id};
>                 }
>             }
>         }
>+
>+        my $now = dt_from_string();
>+        my $preventCheckoutOnSameReservePeriod =
>+            C4::Context->preference("PreventCheckoutOnSameReservePeriod");
>+        my $reserves_on_same_period =
>+            ReservesOnSamePeriod($item_object->biblionumber, $item_object->itemnumber, $now->ymd, $duedate->ymd);
>+        if ($preventCheckoutOnSameReservePeriod && $reserves_on_same_period) {
>+            my $reserve = $reserves_on_same_period->[0];
>+            my $patron = Koha::Patrons->find( $reserve->{borrowernumber} );
>+            my $branchname = Koha::Libraries->find($reserve->{branchcode})->branchname;
>+
>+            $needsconfirmation{RESERVED} = 1;
>+            $needsconfirmation{resfirstname} = $patron->firstname;
>+            $needsconfirmation{ressurname} = $patron->surname;
>+            $needsconfirmation{rescardnumber} = $patron->cardnumber;
>+            $needsconfirmation{resborrowernumber} = $patron->borrowernumber;
>+            $needsconfirmation{resbranchname} = $branchname;
>+            $needsconfirmation{resreservedate} = $reserve->{reservedate};
>+            $needsconfirmation{resreserveid} = $reserve->{reserve_id};
>+        }
>+
>     }
> 
>     ## CHECK AGE RESTRICTION
>@@ -2641,7 +2664,7 @@ already renewed the loan. $error will contain the reason the renewal can not pro
> =cut
> 
> sub CanBookBeRenewed {
>-    my ( $borrowernumber, $itemnumber, $override_limit ) = @_;
>+    my ( $borrowernumber, $itemnumber, $override_limit, $date_due ) = @_;
> 
>     my $dbh    = C4::Context->dbh;
>     my $renews = 1;
>@@ -2718,6 +2741,26 @@ sub CanBookBeRenewed {
>             }
>         }
>     }
>+
>+    unless ($date_due) {
>+        my $itemtype = $item->effective_itemtype;
>+        my $patron_unblessed = $patron->unblessed;
>+        $date_due = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ?
>+                                        dt_from_string( $issue->date_due, 'sql' ) :
>+                                        DateTime->now( time_zone => C4::Context->tz());
>+        $date_due =  CalcDateDue($date_due, $itemtype, _GetCircControlBranch($item->unblessed(), $patron_unblessed), $patron_unblessed, 'is a renewal');
>+    }
>+
>+    my $now = dt_from_string();
>+    my $biblionumber = $item->biblionumber;
>+    my $preventCheckoutOnSameReservePeriod =
>+        C4::Context->preference("PreventCheckoutOnSameReservePeriod");
>+    my $reserves_on_same_period =
>+        ReservesOnSamePeriod($biblionumber, $itemnumber, $now->ymd, $date_due->ymd);
>+    if ($preventCheckoutOnSameReservePeriod && $reserves_on_same_period) {
>+        $resfound = 1;
>+    }
>+
>     return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
> 
>     return ( 1, undef ) if $override_limit;
>diff --git a/C4/Reserves.pm b/C4/Reserves.pm
>index cf6ee50169..df997b1b36 100644
>--- a/C4/Reserves.pm
>+++ b/C4/Reserves.pm
>@@ -133,6 +133,7 @@ BEGIN {
>         &SuspendAll
> 
>         &GetReservesControlBranch
>+        &ReservesOnSamePeriod
> 
>         IsItemOnHoldAndFound
> 
>@@ -2180,6 +2181,53 @@ sub GetHoldRule {
>     return $sth->fetchrow_hashref();
> }
> 
>+=head2 ReservesOnSamePeriod
>+
>+    my $reserve = ReservesOnSamePeriod( $biblionumber, $itemnumber, $resdate, $expdate);
>+
>+    Return the reserve that match the period ($resdate => $expdate),
>+    undef if no reserve match.
>+
>+=cut
>+
>+sub ReservesOnSamePeriod {
>+    my ($biblionumber, $itemnumber, $resdate, $expdate) = @_;
>+
>+    unless ($resdate && $expdate) {
>+        return;
>+    }
>+
>+    my @reserves = Koha::Holds->search({ biblionumber => $biblionumber });
>+
>+    $resdate = output_pref({ str => $resdate, dateonly => 1, dateformat => 'iso' });
>+    $expdate = output_pref({ str => $expdate, dateonly => 1, dateformat => 'iso' });
>+
>+    my @reserves_overlaps;
>+    foreach my $reserve ( @reserves ) {
>+
>+        unless ($reserve->reservedate && $reserve->expirationdate) {
>+            next;
>+        }
>+
>+        if (date_ranges_overlap($resdate, $expdate,
>+                                $reserve->reservedate,
>+                                $reserve->expirationdate)) {
>+
>+            # If reserve is item level and the requested periods overlap.
>+            if ($itemnumber && $reserve->itemnumber == $itemnumber ) {
>+                return [$reserve->unblessed];
>+            }
>+            push @reserves_overlaps, $reserve->unblessed;
>+        }
>+    }
>+
>+    if ( @reserves_overlaps >= Koha::Items->search({ biblionumber => $biblionumber })->count() ) {
>+        return \@reserves_overlaps;
>+    }
>+
>+    return;
>+}
>+
> =head1 AUTHOR
> 
> Koha Development Team <http://koha-community.org/>
>diff --git a/Koha/DateUtils.pm b/Koha/DateUtils.pm
>index 459a5a89d7..04dd01fc77 100644
>--- a/Koha/DateUtils.pm
>+++ b/Koha/DateUtils.pm
>@@ -24,7 +24,7 @@ use Koha::Exceptions;
> use base 'Exporter';
> 
> our @EXPORT = (
>-    qw( dt_from_string output_pref format_sqldatetime )
>+    qw( dt_from_string output_pref format_sqldatetime date_ranges_overlap )
> );
> 
> =head1 DateUtils
>@@ -322,4 +322,46 @@ sub format_sqldatetime {
>     return q{};
> }
> 
>+=head2 date_ranges_overlap
>+
>+    $bool = date_ranges_overlap($start1, $end1, $start2, $end2);
>+
>+    Tells if first range ($start1 => $end1) overlaps
>+    the second one ($start2 => $end2)
>+
>+=cut
>+
>+sub date_ranges_overlap {
>+    my ($start1, $end1, $start2, $end2) = @_;
>+
>+    $start1 = dt_from_string( $start1, 'iso' );
>+    $end1 = dt_from_string( $end1, 'iso' );
>+    $start2 = dt_from_string( $start2, 'iso' );
>+    $end2 = dt_from_string( $end2, 'iso' );
>+
>+    if (
>+        # Start of range 2 is in the range 1.
>+        (
>+            DateTime->compare($start2, $start1) >= 0
>+            && DateTime->compare($start2, $end1) <= 0
>+        )
>+        ||
>+        # End of range 2 is in the range 1.
>+        (
>+            DateTime->compare($end2, $start1) >= 0
>+            && DateTime->compare($end2, $end1) <= 0
>+        )
>+        ||
>+        # Range 2 start before and end after range 1.
>+        (
>+            DateTime->compare($start2, $start1) < 0
>+            && DateTime->compare($end2, $end1) > 0
>+        )
>+    ) {
>+        return 1;
>+    }
>+
>+    return;
>+}
>+
> 1;
>diff --git a/circ/circulation.pl b/circ/circulation.pl
>index 60e5455279..79520144a8 100755
>--- a/circ/circulation.pl
>+++ b/circ/circulation.pl
>@@ -433,6 +433,10 @@ if (@$barcodes) {
>         }
>         unless($confirm_required) {
>             my $switch_onsite_checkout = exists $messages->{ONSITE_CHECKOUT_WILL_BE_SWITCHED};
>+            if ( $cancelreserve eq 'cancel' ) {
>+                CancelReserve({ reserve_id => $query->param('reserveid') });
>+            }
>+            $cancelreserve = $cancelreserve eq 'revert' ? 'revert' : undef;
>             my $issue = AddIssue( $patron->unblessed, $barcode, $datedue, $cancelreserve, undef, undef, { onsite_checkout => $onsite_checkout, auto_renew => $session->param('auto_renew'), switch_onsite_checkout => $switch_onsite_checkout, } );
>             $template_params->{issue} = $issue;
>             $session->clear('auto_renew');
>diff --git a/installer/data/mysql/atomicupdate/bug_15261-add_preventchechoutonsamereserveperiod_syspref.sql b/installer/data/mysql/atomicupdate/bug_15261-add_preventchechoutonsamereserveperiod_syspref.sql
>new file mode 100644
>index 0000000000..9c0ccac080
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_15261-add_preventchechoutonsamereserveperiod_syspref.sql
>@@ -0,0 +1 @@
>+INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES ('PreventCheckoutOnSameReservePeriod', '0', 'Prevent to checkout a document if a reserve on same period exists', NULL, 'YesNo');
>diff --git a/installer/data/mysql/atomicupdate/bug_15261-add_preventreservesonsameperiod_syspref.sql b/installer/data/mysql/atomicupdate/bug_15261-add_preventreservesonsameperiod_syspref.sql
>new file mode 100644
>index 0000000000..9d83db2eb0
>--- /dev/null
>+++ b/installer/data/mysql/atomicupdate/bug_15261-add_preventreservesonsameperiod_syspref.sql
>@@ -0,0 +1 @@
>+INSERT IGNORE INTO systempreferences (variable,value,explanation,options,type) VALUES ('PreventReservesOnSamePeriod', '0', 'Prevent to hold a document if a reserve on same period exists', NULL, 'YesNo');
>diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
>index 11aa53d1dc..bf2ac0e71b 100644
>--- a/installer/data/mysql/sysprefs.sql
>+++ b/installer/data/mysql/sysprefs.sql
>@@ -491,6 +491,8 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
> ('PayPalUser',  '', NULL ,  'Your PayPal API username ( email address )',  'Free'),
> ('PrefillItem','0','','When a new item is added, should it be prefilled with last created item values?','YesNo'),
> ('PreserveSerialNotes','1','','When a new "Expected" issue is generated, should it be prefilled with last created issue notes?','YesNo'),
>+('PreventCheckoutOnSameReservePeriod','0','','Prevent to checkout a document if a reserve on same period exists','YesNo'),
>+('PreventReservesOnSamePeriod','0','','Prevent to hold a document if a reserve on same period exists','YesNo'),
> ('previousIssuesDefaultSortOrder','asc','asc|desc','Specify the sort order of Previous Issues on the circulation page','Choice'),
> ('printcirculationslips','1','','If ON, enable printing circulation receipts','YesNo'),
> ('PrintNoticesMaxLines','0','','If greater than 0, sets the maximum number of lines an overdue notice will print. If the number of items is greater than this number, the notice will end with a warning asking the borrower to check their online account for a full list of overdue items.','Integer'),
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>index 3c24d51273..1fac8b4eba 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
>@@ -496,6 +496,12 @@ Circulation:
>             - "<br />ccode: [NEWFIC,NULL,DVD]"
>             - "<br />itype: [NEWBK,\"\"]"
>             - "<br /> Note: the word 'NULL' can be used to block renewal on undefined fields, while an empty string \"\" will block on an empty (but defined) field."
>+        -
>+            - pref: PreventCheckoutOnSameReservePeriod
>+              choices:
>+                  yes: Do
>+                  no: "Don't"
>+            - If yes, checkouts periods can't overlap with a reserve period.
>     Checkin Policy:
>         -
>             - pref: HoldsAutoFill
>@@ -805,6 +811,12 @@ Circulation:
>             - pref: UpdateItemWhenLostFromHoldList
>               type: textarea
>               syntax: text/x-yaml
>+        -
>+            - pref: PreventReservesOnSamePeriod
>+              choices:
>+                  yes: Do
>+                  no: "Don't"
>+            - If yes, Reserves periods for the same document can't overlap.
>     Interlibrary Loans:
>         -
>             - pref: ILLModule
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>index 86803440d4..0acb426cf2 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
>@@ -213,6 +213,7 @@
>                                     [% IF ( RESERVED ) %]
>                                         <p>
>                                             <input type="checkbox" id="cancelreserve" name="cancelreserve" value="cancel" />
>+                                            <input type="hidden" name="reserveid" value="[% resreserveid | html %]" />
>                                             <label for="cancelreserve">Cancel hold</label>
>                                         </p>
>                                     [% END %]
>@@ -221,6 +222,7 @@
>                                         <p>
>                                             <label for="cancelreserve">Cancel hold</label>
>                                             <input type="radio" value="cancel" name="cancelreserve" id="cancelreserve" /><br />
>+                                            <input type="hidden" name="reserveid" value="[% resreserveid | html %]" />
>                                             <label for="revertreserve">Revert waiting status</label>
>                                             <input type="radio" value="revert" name="cancelreserve" id="revertreserve" checked="checked"/>
>                                         </p>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/placerequest.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/placerequest.tt
>new file mode 100644
>index 0000000000..8342d99e15
>--- /dev/null
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/placerequest.tt
>@@ -0,0 +1,88 @@
>+[% USE JSON.Escape %]
>+[% USE raw %]
>+[% INCLUDE 'doc-head-open.inc' %]
>+    <title>Koha &rsaquo; Circulation &rsaquo; Holds &rsaquo; Confirm holds</title>
>+[% INCLUDE 'doc-head-close.inc' %]
>+</head>
>+<body id="circ_placerequest" class="catalog">
>+[% INCLUDE 'header.inc' %]
>+[% INCLUDE 'circ-search.inc' %]
>+
>+<div id="breadcrumbs">
>+  <a href="/cgi-bin/koha/mainpage.pl">Home</a>
>+  &rsaquo;
>+  <a href="/cgi-bin/koha/catalogue/search.pl">Catalog</a>
>+  &rsaquo;
>+  Confirm holds
>+</div>
>+
>+<div id="doc3" class="yui-t2">
>+
>+  <div id="bd">
>+    <div id="yui-main">
>+      <div class="yui-b">
>+        <div class="modal fade" tabindex="-1" role="dialog">
>+          <div class="modal-dialog" role="document">
>+            <div class="modal-content">
>+              <div class="modal-header">
>+                <h4 class="modal-title">Confirm holds</h4>
>+              </div>
>+              <div class="modal-body">
>+                <p>
>+                  Some of the reserves you are trying to do overlaps with existing reserves.
>+                  Please tick the box confirming the reservation then click on Confirm if you want to proceed.
>+                </p>
>+
>+                <form method="post" id="confirm-holds">
>+                  <input type="hidden" name="borrowernumber" value="[% borrowernumber | html %]">
>+                  [% IF multi_hold %]
>+                    <input type="hidden" name="biblionumbers" value="[% biblionumbers | html %]">
>+                    <input type="hidden" name="multi_hold" value="1">
>+                  [% ELSE %]
>+                    <input type="hidden" name="biblionumber" value="[% biblionumber | html %]">
>+                  [% END %]
>+                  <input type="hidden" name="reserve_date" value="[% reserve_date | html %]">
>+                  <input type="hidden" name="expiration_date" value="[% expiration_date | html %]">
>+                  <input type="hidden" name="pickup" value="[% pickup | html %]">
>+                  <input type="hidden" name="notes" value="[% notes | html %]">
>+                  <input type="hidden" name="confirm" value="1">
>+                  <input type="hidden" name="type" value="str8">
>+
>+                  [% FOREACH biblionumber IN overlap_reserves.keys %]
>+                    [% input_id = "confirm_$biblionumber" %]
>+                    <div>
>+                      <input type="hidden" name="rank_[% biblionumber | html %]" value="[% overlap_reserves.$biblionumber.rank | html %]">
>+                      [% IF (overlap_reserves.$biblionumber.checkitem) %]
>+                        <input type="hidden" name="checkitem" value="[% overlap_reserves.$biblionumber.checkitem | html %]">
>+                      [% END %]
>+                      <input type="checkbox" name="confirm_biblionumbers" id="[% input_id | html %]"
>+                             value="[% biblionumber | html %]">
>+                      <label for="[% input_id | html %]">Confirm hold for [% overlap_reserves.$biblionumber.title | html %]</label>
>+                    </div>
>+                  [% END %]
>+                </form>
>+              </div>
>+              <div class="modal-footer">
>+                <button type="button" class="btn btn-default deny" data-dismiss="modal"><i class="fa fa-times"></i> Cancel</button>
>+                <button type="submit" class="btn btn-default approve" form="confirm-holds"><i class="fa fa-check"></i> Confirm</button>
>+              </div>
>+            </div>
>+          </div>
>+        </div>
>+      </div>
>+    </div>
>+
>+  </div>
>+  <script>
>+    $(document).ready(function () {
>+      var modal = $('.modal').modal();
>+      modal.on('hide.bs.modal', function (e) {
>+        [% IF multi_hold %]
>+          window.location = '/cgi-bin/koha/reserve/request.pl?biblionumbers=' + [% biblionumbers.json | $raw %] + '&multi_hold=1' + '&borrowernumber=' + [% borrowernumber.json | $raw %];
>+        [% ELSE %]
>+          window.location = '/cgi-bin/koha/reserve/request.pl?biblionumber=' + [% biblionumber.json | $raw %] + '&borrowernumber=' + [% borrowernumber.json | $raw %]
>+        [% END %]
>+      })
>+    });
>+  </script>
>+[% INCLUDE 'intranet-bottom.inc' %]
>diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
>index 56c5bc2695..15629426da 100755
>--- a/opac/opac-reserve.pl
>+++ b/opac/opac-reserve.pl
>@@ -282,6 +282,15 @@ if ( $query->param('place_reserve') ) {
>             # Inserts a null into the 'itemnumber' field of 'reserves' table.
>             $itemNum = undef;
>         }
>+
>+        if ($canreserve) {
>+            if (C4::Context->preference("PreventReservesOnSamePeriod") &&
>+                ReservesOnSamePeriod($biblioNum, $itemNum, $startdate, $expiration_date)) {
>+                $canreserve = 0;
>+                $failed_holds++;
>+            }
>+        }
>+
>         my $notes = $query->param('notes_'.$biblioNum)||'';
> 
>         if (   $maxreserves
>diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl
>index 89c591d55b..a3c160d956 100755
>--- a/reserve/placerequest.pl
>+++ b/reserve/placerequest.pl
>@@ -23,6 +23,8 @@
> 
> use Modern::Perl;
> 
>+use List::MoreUtils qw( none );
>+
> use CGI qw ( -utf8 );
> use C4::Biblio;
> use C4::Items;
>@@ -30,17 +32,22 @@ use C4::Output;
> use C4::Reserves;
> use C4::Circulation;
> use C4::Members;
>-use C4::Auth qw/checkauth/;
>+use C4::Auth;
> 
> use Koha::Items;
> use Koha::Patrons;
> 
> my $input = CGI->new();
> 
>-checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet');
>+my ($template, $loggedinuser, $cookie) = get_template_and_user({
>+    template_name   => 'reserve/placerequest.tt',
>+    query           => $input,
>+    type            => 'intranet',
>+    authnotrequired => 0,
>+    flagsrequired   => { reserveforothers => 'place_holds' },
>+});
> 
> my @bibitems       = $input->multi_param('biblioitem');
>-my @reqbib         = $input->multi_param('reqbib');
> my $biblionumber   = $input->param('biblionumber');
> my $borrowernumber = $input->param('borrowernumber');
> my $notes          = $input->param('notes');
>@@ -52,6 +59,8 @@ my $title          = $input->param('title');
> my $checkitem      = $input->param('checkitem');
> my $expirationdate = $input->param('expiration_date');
> my $itemtype       = $input->param('itemtype') || undef;
>+my $confirm        = $input->param('confirm');
>+my @confirm_biblionumbers = $input->multi_param('confirm_biblionumbers');
> 
> my $borrower = Koha::Patrons->find( $borrowernumber );
> $borrower = $borrower->unblessed if $borrower;
>@@ -73,8 +82,41 @@ foreach my $bibnum (@biblionumbers) {
> my $found;
> 
> if ( $type eq 'str8' && $borrower ) {
>+    if (C4::Context->preference('PreventReservesOnSamePeriod') && !$confirm) {
>+        my %overlap_reserves;
>+        foreach my $biblionumber ( keys %bibinfos ) {
>+            my $overlapping_reserves = ReservesOnSamePeriod($biblionumber, $checkitem, $startdate, $expirationdate);
>+            if ($overlapping_reserves && 0 < scalar @$overlapping_reserves) {
>+                $overlap_reserves{$biblionumber} = {
>+                    title => $multi_hold ? $bibinfos{$biblionumber}->{title} : $title,
>+                    checkitem => $checkitem,
>+                    rank => $multi_hold ? $bibinfos{$biblionumber}->{rank} : $rank[0],
>+                };
>+            }
>+        }
>+
>+        if (scalar keys %overlap_reserves) {
>+            $template->param(
>+                borrowernumber => $borrowernumber,
>+                biblionumbers => $biblionumbers,
>+                biblionumber => $biblionumber,
>+                overlap_reserves => \%overlap_reserves,
>+                reserve_date => $startdate,
>+                expiration_date => $expirationdate,
>+                notes => $notes,
>+                rank_request => \@rank,
>+                pickup => $branch,
>+                multi_hold => $multi_hold,
>+            );
>+
>+            output_html_with_http_headers $input, $cookie, $template->output;
>+            exit;
>+        }
>+    }
> 
>     foreach my $biblionumber ( keys %bibinfos ) {
>+        next if ($confirm && none { $_ eq $biblionumber } @confirm_biblionumbers);
>+
>         my $count = @bibitems;
>         @bibitems = sort @bibitems;
>         my $i2 = 1;
>diff --git a/svc/renew b/svc/renew
>index 29e51d8cf7..f94ef28e9a 100755
>--- a/svc/renew
>+++ b/svc/renew
>@@ -57,7 +57,7 @@ $data->{borrowernumber} = $borrowernumber;
> $data->{branchcode} = $branchcode;
> 
> ( $data->{renew_okay}, $data->{error} ) =
>-  CanBookBeRenewed( $borrowernumber, $itemnumber, $override_limit );
>+  CanBookBeRenewed( $borrowernumber, $itemnumber, $override_limit, $date_due );
> 
> # If we're allowing reserved items to be renewed...
> if ( $data->{error} && $data->{error} eq 'on_reserve' && C4::Context->preference('AllowRenewalOnHoldOverride')) {
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index e323508469..5f81e2d8a2 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -18,7 +18,7 @@
> use Modern::Perl;
> use utf8;
> 
>-use Test::More tests => 45;
>+use Test::More tests => 47;
> use Test::MockModule;
> 
> use Data::Dumper;
>@@ -382,6 +382,7 @@ subtest "CanBookBeRenewed tests" => sub {
>     my $borrowing_borrowernumber = Koha::Checkouts->find( { itemnumber => $item_1->itemnumber } )->borrowernumber;
>     is ($borrowing_borrowernumber, $renewing_borrowernumber, "Item checked out to $renewing_borrower->{firstname} $renewing_borrower->{surname}");
> 
>+    t::lib::Mocks::mock_preference('CircControl', 'PatronLibrary');
>     my ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1);
>     is( $renewokay, 1, 'Can renew, no holds for this title or item');
> 
>@@ -1250,6 +1251,65 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
>     is( $error, 'onsite_checkout', 'A correct error code should be returned by CanBookBeRenewed for on-site checkout' );
> }
> 
>+{
>+    # Don't allow renewing on reserve period with PreventCheckoutOnSameReservePeriod enabled
>+    t::lib::Mocks::mock_preference( 'PreventCheckoutOnSameReservePeriod', 1 );
>+
>+    my $start_issue_dt = DateTime->now();
>+    $start_issue_dt->subtract( days => 15);
>+    my $due_date = $start_issue_dt->clone;
>+    $due_date->add( days => 17 );
>+
>+    my $biblio = $builder->build({
>+        source => 'Biblio',
>+    });
>+    my $biblionumber = $biblio->{biblionumber};
>+
>+    my $item = $builder->build({
>+        source => 'Item',
>+        value => {
>+            biblionumber => $biblionumber
>+        }
>+    });
>+    my $itemnumber = $item->{itemnumber};
>+
>+    my $issue = $builder->build({
>+        source => 'Issue',
>+        value => {
>+            itemnumber => $itemnumber,
>+            biblionumber => $biblionumber,
>+            issuedate => $start_issue_dt->ymd,
>+            date_due => $due_date->ymd,
>+            onsite_checkout => 0
>+        }
>+    });
>+
>+    my $reservedate = $due_date->clone;
>+    $reservedate->add( days => 5 );
>+    my $expirationdate = $reservedate->clone;
>+    $expirationdate->add( days => 15 );
>+    $builder->build({
>+        source => 'Reserve',
>+        value => {
>+            biblionumber => $biblionumber,
>+            itemnumber => $itemnumber,
>+            reservedate => $reservedate->ymd,
>+            expirationdate => $expirationdate->ymd
>+        }
>+    });
>+
>+    my $requested_date_due = $due_date->clone;
>+    $requested_date_due->add( days => 4 );
>+    my ( $renewed, $error ) = CanBookBeRenewed( $issue->{borrowernumber}, $itemnumber, 1, $requested_date_due );
>+    is( $renewed, 1, 'CanBookBeRenewed should allow to renew if no reserve date overlap' );
>+
>+    $requested_date_due->add( days => 2 );
>+    ( $renewed, $error ) = CanBookBeRenewed( $issue->{borrowernumber}, $itemnumber, 1, $requested_date_due );
>+    is( $renewed, 0, 'CanBookBeRenewed should not allow to renew if reserve date overlap' );
>+
>+    t::lib::Mocks::mock_preference( 'PreventCheckoutOnSameReservePeriod', 0 );
>+}
>+
> {
>     my $library = $builder->build({ source => 'Branch' });
> 
>diff --git a/t/db_dependent/Circulation/CanBookBeIssued.t b/t/db_dependent/Circulation/CanBookBeIssued.t
>new file mode 100644
>index 0000000000..fad19eea13
>--- /dev/null
>+++ b/t/db_dependent/Circulation/CanBookBeIssued.t
>@@ -0,0 +1,106 @@
>+#!/usr/bin/env perl
>+
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+use Modern::Perl;
>+
>+use Test::More tests => 1;
>+use C4::Members;
>+use C4::Reserves;
>+use C4::Circulation;
>+use Koha::DateUtils;
>+
>+use t::lib::TestBuilder;
>+
>+my $schema  = Koha::Database->new->schema;
>+$schema->storage->txn_begin;
>+
>+my $builder = t::lib::TestBuilder->new();
>+
>+subtest 'Tests for CanBookBeIssued with overlap reserves' => sub {
>+    plan tests => 6;
>+
>+    my $categorycode = $builder->build({ source => 'Category' })->{ categorycode };
>+    my $branch = $builder->build({ source => 'Branch' });
>+    my $branchcode = $branch->{branchcode};
>+
>+    my $borrower = $builder->build_object({
>+        class => 'Koha::Patrons',
>+        value => {
>+            branchcode   => $branchcode,
>+            categorycode => $categorycode,
>+        }
>+    });
>+    my $borrowernumber = $borrower->borrowernumber;
>+
>+    my $biblio = $builder->build({source => 'Biblio'});
>+    my $biblioitem = $builder->build({
>+        source => 'Biblioitem',
>+        value => {
>+            biblionumber => $biblio->{biblionumber},
>+        },
>+    });
>+    my $item = $builder->build({
>+        source => 'Item',
>+        value => {
>+            biblionumber => $biblio->{biblionumber},
>+            biblioitemnumber => $biblioitem->{biblioitemnumber},
>+            withdrawn => 0,
>+            itemlost => 0,
>+            notforloan => 0,
>+        },
>+    });
>+
>+
>+    my $startdate = dt_from_string();
>+    $startdate->add_duration(DateTime::Duration->new(days => 4));
>+    my $expdate = $startdate->clone();
>+    $expdate->add_duration(DateTime::Duration->new(days => 10));
>+
>+    my $reserveid = AddReserve($branchcode, $borrowernumber,
>+        $item->{biblionumber}, undef,  1, $startdate->ymd(), $expdate->ymd,
>+        undef, undef, undef, undef);
>+
>+    my $non_overlap_duedate = dt_from_string();
>+    $non_overlap_duedate->add_duration(DateTime::Duration->new(days => 2));
>+    my ($error, $question, $alerts ) =
>+        CanBookBeIssued($borrower, $item->{barcode}, $non_overlap_duedate, 1, 0);
>+
>+    is_deeply($error, {}, "");
>+    is_deeply($question, {}, "");
>+    is_deeply($alerts, {}, "");
>+
>+    my $overlap_duedate = dt_from_string();
>+    $overlap_duedate->add_duration(DateTime::Duration->new(days => 5));
>+    ($error, $question, $alerts ) =
>+        CanBookBeIssued($borrower, $item->{barcode}, $overlap_duedate, 1, 0);
>+
>+    is_deeply($error, {}, "");
>+    my $expected = {
>+        RESERVED => 1,
>+        resfirstname => $borrower->firstname,
>+        ressurname => $borrower->surname,
>+        rescardnumber => $borrower->cardnumber,
>+        resborrowernumber => $borrower->borrowernumber,
>+        resbranchname => $branch->{branchname},
>+        resreservedate => $startdate->ymd,
>+        resreserveid => $reserveid,
>+    };
>+    is_deeply($question, $expected, "");
>+    is_deeply($alerts, {}, "");
>+};
>+
>+$schema->storage->txn_rollback;
>diff --git a/t/db_dependent/Reserves/ReserveDate.t b/t/db_dependent/Reserves/ReserveDate.t
>new file mode 100644
>index 0000000000..4d7f8afc4a
>--- /dev/null
>+++ b/t/db_dependent/Reserves/ReserveDate.t
>@@ -0,0 +1,108 @@
>+#!/usr/bin/perl
>+
>+# This file is part of Koha.
>+#
>+# Koha is free software; you can redistribute it and/or modify it
>+# under the terms of the GNU General Public License as published by
>+# the Free Software Foundation; either version 3 of the License, or
>+# (at your option) any later version.
>+#
>+# Koha is distributed in the hope that it will be useful, but
>+# WITHOUT ANY WARRANTY; without even the implied warranty of
>+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>+# GNU General Public License for more details.
>+#
>+# You should have received a copy of the GNU General Public License
>+# along with Koha; if not, see <http://www.gnu.org/licenses>.
>+
>+use Modern::Perl;
>+
>+use Test::More tests => 6;
>+use Test::Warn;
>+
>+use MARC::Record;
>+use DateTime::Duration;
>+
>+use C4::Biblio;
>+use C4::Items;
>+use C4::Members;
>+use C4::Circulation;
>+use Koha::Holds;
>+use t::lib::TestBuilder;
>+
>+use Koha::DateUtils;
>+
>+
>+use_ok('C4::Reserves');
>+
>+my $dbh = C4::Context->dbh;
>+
>+# Start transaction
>+$dbh->{AutoCommit} = 0;
>+$dbh->{RaiseError} = 1;
>+
>+my $builder = t::lib::TestBuilder->new();
>+my $categorycode = $builder->build({ source => 'Category' })->{ categorycode };
>+my $branchcode = $builder->build({ source => 'Branch' })->{ branchcode };
>+
>+my $borrower = $builder->build({
>+    source => 'Borrower',
>+    value => {
>+        branchcode   => $branchcode,
>+        categorycode => $categorycode,
>+    }
>+});
>+
>+my $borrower2 = $builder->build({
>+    source => 'Borrower',
>+    value => {
>+        branchcode   => $branchcode,
>+        categorycode => $categorycode,
>+    }
>+});
>+
>+my $borrowernumber = $borrower->{borrowernumber};
>+my $borrowernumber2 = $borrower2->{borrowernumber};
>+
>+# Create a helper biblio
>+my $biblio = MARC::Record->new();
>+my $title = 'Alone in the Dark';
>+my $author = 'Karen Rose';
>+if( C4::Context->preference('marcflavour') eq 'UNIMARC' ) {
>+    $biblio->append_fields(
>+        MARC::Field->new('600', '', '1', a => $author),
>+        MARC::Field->new('200', '', '', a => $title),
>+    );
>+}
>+else {
>+    $biblio->append_fields(
>+        MARC::Field->new('100', '', '', a => $author),
>+        MARC::Field->new('245', '', '', a => $title),
>+    );
>+}
>+my ($bibnum, $bibitemnum);
>+($bibnum, $title, $bibitemnum) = AddBiblio($biblio, '');
>+
>+my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branchcode, holdingbranch => $branchcode, barcode => '333' } , $bibnum);
>+
>+C4::Context->set_preference('AllowHoldDateInFuture', 1);
>+
>+AddReserve($branchcode, $borrowernumber, $bibnum,
>+           $bibitemnum,  1, '2015-11-01', '2015-11-20', undef,
>+           undef, undef, undef);
>+
>+is(ReservesOnSamePeriod($bibnum, undef, '2015-11-25', '2015-11-30'), undef, "Period doesn't overlaps");
>+
>+ok(ReservesOnSamePeriod($bibnum, undef, '2015-11-02', '2015-11-10'), "Period overlaps");
>+
>+my ($item_bibnum2, $item_bibitemnum2, $itemnumber2) = AddItem({ homebranch => $branchcode, holdingbranch => $branchcode, barcode => '444' } , $bibnum);
>+is(ReservesOnSamePeriod($bibnum, undef, '2015-11-02', '2015-11-10'), undef, "Period overlaps but there is 2 items");
>+
>+AddReserve($branchcode, $borrowernumber2, $bibnum,
>+           $bibitemnum,  1, '2016-02-01', '2016-02-10', undef,
>+           undef, $itemnumber, undef);
>+is(ReservesOnSamePeriod($bibnum, $itemnumber, '02/12/2015', '10/12/2015'), undef, "Period on item does not overlap (with metric date format)");
>+
>+my $reserve = ReservesOnSamePeriod($bibnum, $itemnumber, '2016-01-31', '2016-02-05');
>+is($reserve->[0]->{itemnumber}, $itemnumber, 'Period on item overlaps');
>+$dbh->rollback;
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 15261: 45216 | 45833 | 45841 | 46432 | 46433 | 46441 | 46442 | 49001 | 49002 | 49003 | 52704 | 59643 | 61534 | 61539 | 61544 | 67088 | 67094 | 67143 | 68660 | 68661 | 68662 | 68663 | 68664 | 68852 | 68853 | 72956 | 72957 | 76221 | 76222 | 88292 | 88293 | 88297 | 88298 | 92376 | 92377 | 92584 | 92585 | 92664 | 93136 | 94871 | 98383 | 98424 | 98672 | 106944 | 106949 | 120822 | 123405 | 125793 | 125794 | 125795 | 125796 | 131569 | 131570 | 131571 | 131572 | 131573