Attachment 40932 Details for Bug 14494 – [PASSED QA] Bug 14494: Prevent slow checkout if the patron does not have an expiry date

[patch] [PASSED QA] Bug 14494: Prevent slow checkout if the patron does not have an expiry date

PASSED-QA-Bug-14494-Prevent-slow-checkout-if-the-p.patch (text/plain), 2.84 KB, created by Kyle M Hall (khall) on 2015-07-10 15:33:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2015-07-10 15:33:11 UTC

Size: 2.84 KB

patch

obsolete

>From 268b4a95fd447282d9750b096234c7a8b297914a Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@koha-community.org>
>Date: Thu, 9 Jul 2015 09:52:28 +0100
>Subject: [PATCH] [PASSED QA] Bug 14494: Prevent slow checkout if the patron does not have an expiry date
>
>If a patron has a expiry date set to 9999-12-31 (for organizations for
>instance), the checkouts are very slow.
>
>It's caused by 2 different calls to DateTime in CanBookBeIssued:
>1/
>  DateTime->new( year => 9999, month => 12, day => 31, time_zone => C4::Context->tz );
>The time_zone should not be set (as it's done in Koha::DateUtils), set to UTC or floating tz.
>
>2/
>  DateTime->compare($today, $expiry_dt)
>The comparaison of 2 DT with 1 related to 9999 is very slow, as you can
>imagine.
>
>For 1/ we need to call Koha::DateUtils::dt_from_string (actually, we
>should never call DateTime directly).
>For 2/ we just need to test if the date is != 9999, no need to compare
>it in this case.
>
>Test plan:
>Before this patch, confirm that the checkouts are slow if the patron has a
>dateexpiry set to 9999-12-31.
>update borrowers set dateexpiry="9999-12-31" where borrowernumber=42;
>
>After this patch, you should not see any regression when checking out
>items to an expired patron and to a valid patron.
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> C4/Circulation.pm |   21 ++++++---------------
> 1 files changed, 6 insertions(+), 15 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index de4b299..aac82bc 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -767,24 +767,15 @@ sub CanBookBeIssued {
>     if ( !defined $borrower->{dateexpiry} || $borrower->{'dateexpiry'} eq '0000-00-00') {
>         $issuingimpossible{EXPIRED} = 1;
>     } else {
>-        my ($y, $m, $d) =  split /-/,$borrower->{'dateexpiry'};
>-        if ($y && $m && $d) { # are we really writing oinvalid dates to borrs
>-            my $expiry_dt = DateTime->new(
>-                year => $y,
>-                month => $m,
>-                day   => $d,
>-                time_zone => C4::Context->tz,
>-            );
>-            $expiry_dt->truncate( to => 'day');
>-            my $today = $now->clone()->truncate(to => 'day');
>-            if (DateTime->compare($today, $expiry_dt) == 1) {
>-                $issuingimpossible{EXPIRED} = 1;
>-            }
>-        } else {
>-            carp("Invalid expity date in borr");
>+        my $expiry_dt = dt_from_string( $borrower->{dateexpiry}, 'sql' );
>+        $expiry_dt->truncate( to => 'day');
>+        my $today = $now->clone()->truncate(to => 'day');
>+
>+        if ($expiry_dt->year < 9999 && DateTime->compare($today, $expiry_dt) == 1) {
>             $issuingimpossible{EXPIRED} = 1;
>         }
>     }
>+
>     #
>     # BORROWER STATUS
>     #
>-- 
>1.7.2.5

Actions: View | Diff | Splinter Review

Attachments on bug 14494: 40860 | 40873 | 40874 | 40879 | 40880 | 40881 | 40882 | 40883 | 40908 | 40909 | 40910 | 40911 | 40912 | 40932 | 40933 | 40934 | 40935