[Koha-bugs] [Bug 9011] Add the ability to store the last patron to return an item

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9011

Kyle M Hall <kyle at bywatersolutions.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Failed QA                   |Signed Off

--- Comment #115 from Kyle M Hall <kyle at bywatersolutions.com> ---
(In reply to Jonathan Druart from comment #114)
> Comment on attachment 39815 [details] [review]
> Bug 9011 [QA Followup]
> 
> Review of attachment 39815 [details] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Circulation.pm
> @@ +3198,5 @@
> >      }
> >  
> >      if ( C4::Context->preference('StoreLastBorrower') ) {
> > +        # Do not delete the newest old_issue for any itemnumber
> > +        $query .= " AND issue_id NOT IN ( SELECT issue_id FROM ( SELECT * FROM old_issues ORDER BY issue_id DESC ) AS oi GROUP BY itemnumber ) ";
> 
> I think that the NOT IN clause should be avoid here.
> It can cause the query to be very slow.
> See bug 13740.

As far as I can tell there is no way around this. I tried to use a left join
but you cannot use a group by in an update statement.

Here is my proof of concept diff that doesn't work, for posterity:

diff --git a/C4/Circulation.pm b/C4/Circulation.pm
index b174a12..bc3e28e 100644
--- a/C4/Circulation.pm
+++ b/C4/Circulation.pm
@@ -3183,11 +3183,15 @@ sub AnonymiseIssueHistory {
     my $date           = shift;
     my $borrowernumber = shift;
     my $dbh            = C4::Context->dbh;
-    my $query          = "
-        UPDATE old_issues
-        SET    borrowernumber = ?
-        WHERE  returndate < ?
-          AND borrowernumber IS NOT NULL
+
+    my $StoreLastBorrower = C4::Context->preference('StoreLastBorrower');
+
+    my $query = "UPDATE old_issues";
+    $query .= " LEFT JOIN old_issues last_checkout_for_item USING ( itemnumber
)" if $StoreLastBorrower;
+    $query .= "
+        SET    old_issues.borrowernumber = ?
+        WHERE  old_issues.returndate < ?
+          AND  old_issues.borrowernumber IS NOT NULL
     ";

     # The default of 0 does not work due to foreign key constraints
@@ -3196,16 +3200,18 @@ sub AnonymiseIssueHistory {
     my @bind_params = ($anonymouspatron, $date);

     if (defined $borrowernumber) {
-       $query .= " AND borrowernumber = ?";
+       $query .= " AND old_issues.borrowernumber = ?";
        push @bind_params, $borrowernumber;
     } else {
        $query .= " AND (SELECT privacy FROM borrowers WHERE
borrowers.borrowernumber=old_issues.borrowernumber) <> 0";
     }

-    if ( C4::Context->preference('StoreLastBorrower') ) {
-        # Do not delete the newest old_issue for any itemnumber
-        $query .= " AND issue_id NOT IN ( SELECT issue_id FROM ( SELECT * FROM
old_issues ORDER BY issue_id DESC ) AS oi GROUP BY itemnumber ) ";
-    }
+    # Do not delete the newest old_issue for any itemnumber
+    $query .= "
+        AND old_issues.issue_id <> last_checkout_for_item.issue_id
+        GROUP BY last_checkout_for_item.itemnumber
+        ORDER BY last_checkout_for_item.issue_id DESC
+    " if $StoreLastBorrower;

     my $sth = $dbh->prepare($query);
     $sth->execute(@bind_params);

-- 
You are receiving this mail because:
You are watching all bug changes.