Attachment 76222 Details for Bug 15261 – Bug 15261 - Check reserves while renewing an issue

[patch] Bug 15261 - Check reserves while renewing an issue

Bug-15261---Check-reserves-while-renewing-an-issue.patch (text/plain), 6.49 KB, created by Alex Arnaud on 2018-06-21 07:48:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Arnaud

Created: 2018-06-21 07:48:56 UTC

Size: 6.49 KB

patch

obsolete

>From d8e7fe8c87bf648a26f09d5bf4bc1307bb0a1aec Mon Sep 17 00:00:00 2001
>From: Alex Arnaud <alex.arnaud@biblibre.com>
>Date: Fri, 15 Sep 2017 12:21:01 +0000
>Subject: [PATCH] Bug 15261 - Check reserves while renewing an issue
>
>Test plan:
>
>  - Create an issue (i.e from 15/09/2017 to 04/10/2017),
>  - place a reserve on the same item from 05/10/2017 to 30/10/2017),
>  - enable PreventCheckoutOnSameReservePeriod,
>  - Check that you are not able to renew the issue.
>
>Rebased (2018-03-15)
>Rebased-by: Alex Arnaud <alex.arnaud@biblibre.com>
>---
> C4/Circulation.pm            | 24 +++++++++++++++--
> svc/renew                    |  2 +-
> t/db_dependent/Circulation.t | 63 +++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 85 insertions(+), 4 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index f841538..b2cb422 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2606,12 +2606,12 @@ 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;
> 
>-    my $item      = GetItem($itemnumber)      or return ( 0, 'no_item' );
>+    my $item = Koha::Items->find( $itemnumber ) or return ( 0, 'no_item' );;
>     my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } ) or return ( 0, 'no_checkout' );
>     return ( 0, 'onsite_checkout' ) if $issue->onsite_checkout;
> 
>@@ -2684,6 +2684,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, $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/svc/renew b/svc/renew
>index 1aa9a28..2a03e1f 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 ( $data->{renew_okay} ) {
>     $date_due = AddRenewal( $borrowernumber, $itemnumber, $branchcode, $date_due );
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index 9f0791a..b26a4a7 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 118;
>+use Test::More tests => 120;
> 
> use DateTime;
> use POSIX qw( floor );
>@@ -231,6 +231,7 @@ C4::Context->dbh->do("DELETE FROM accountlines");
>     my $title = 'Silence in the library';
>     my ($biblionumber, $biblioitemnumber) = add_biblio($title, 'Moffat, Steven');
> 
>+
>     my $barcode = 'R00000342';
>     my $branch = $library2->{branchcode};
> 
>@@ -337,6 +338,7 @@ C4::Context->dbh->do("DELETE FROM accountlines");
>     my $borrowing_borrowernumber = Koha::Checkouts->find( { itemnumber => $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, $itemnumber, 1);
>     is( $renewokay, 1, 'Can renew, no holds for this title or item');
> 
>@@ -1162,6 +1164,65 @@ C4::Context->dbh->do("DELETE FROM accountlines");
> }
> 
> {
>+    # 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' });
> 
>     my ($biblionumber, $biblioitemnumber) = add_biblio();
>-- 
>2.7.4

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