Attachment 125060 Details for Bug 29043 – Bug 29043: Don't load items unless a patron has been chosen on request.pl

[patch] Bug 29043: Don't load items unless a patron has been chosen on request.pl

Bug-29043-Dont-load-items-unless-a-patron-has-been.patch (text/plain), 11.17 KB, created by Andrew Fuerste-Henry on 2021-09-20 12:51:47 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Fuerste-Henry

Created: 2021-09-20 12:51:47 UTC

Size: 11.17 KB

patch

obsolete

>From 8825ae5414f383da0e0e99a883672a6eeb422fed Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Thu, 16 Sep 2021 18:50:27 +0000
>Subject: [PATCH] Bug 29043: Don't load items unless a patron has been chosen
> on request.pl
>
>This page wraps the item processing into an 'if( $patron )' conditional
>to boost performance before a patron has been located.
>
>To test:
> 1 - Browse to a bib in the staff client
> 2 - Click on the holds tab
> 3 - Note loading time
> 4 - Add 500 items to bib
> 5 - Note loading time - it takes longer
> 6 - Note items are not displayed
> 7 - Apply patch
> 8 - Reload page - is much faster
> 9 - Find/choose a patron
>10 - Confirm items are loaded correctly
>
>Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
>---
> reserve/request.pl | 168 ++++++++++++++++++++++-----------------------
> 1 file changed, 84 insertions(+), 84 deletions(-)
>
>diff --git a/reserve/request.pl b/reserve/request.pl
>index 6ab8ba7ac5..94715aa975 100755
>--- a/reserve/request.pl
>+++ b/reserve/request.pl
>@@ -464,106 +464,105 @@ foreach my $biblionumber (@biblionumbers) {
>         # it's complicated logic to analyse.
>         # (before this loop was inside that sub loop so it was O(n^2) )
>         my $items_any_available;
>-        $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblioitem->{biblionumber}, patron => $patron })
>-            if $patron;
>-
>-        foreach my $itemnumber ( @{ $itemnumbers_of_biblioitem{$biblioitemnumber} } )    {
>-            my $item = $iteminfos_of->{$itemnumber};
>-            my $do_check;
>-            if ( $patron ) {
>-                $do_check = $patron->do_check_for_previous_checkout($item) if $wants_check;
>-                if ( $do_check && $wants_check ) {
>-                    $item->{checked_previously} = $do_check;
>-                    if ( $multi_hold ) {
>-                        $biblioloopiter{checked_previously} = $do_check;
>-                    } else {
>-                        $template->param( checked_previously => $do_check );
>+        if ( $patron ) {
>+            $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblioitem->{biblionumber}, patron => $patron });
>+
>+            foreach my $itemnumber ( @{ $itemnumbers_of_biblioitem{$biblioitemnumber} } )    {
>+                my $item = $iteminfos_of->{$itemnumber};
>+                my $do_check;
>+                if ( $patron ) {
>+                    $do_check = $patron->do_check_for_previous_checkout($item) if $wants_check;
>+                    if ( $do_check && $wants_check ) {
>+                        $item->{checked_previously} = $do_check;
>+                        if ( $multi_hold ) {
>+                            $biblioloopiter{checked_previously} = $do_check;
>+                        } else {
>+                            $template->param( checked_previously => $do_check );
>+                        }
>                     }
>                 }
>-            }
>-            $item->{force_hold_level} = $force_hold_level;
>+                $item->{force_hold_level} = $force_hold_level;
> 
>-            unless (C4::Context->preference('item-level_itypes')) {
>-                $item->{itype} = $biblioitem->{itemtype};
>-            }
>+                unless (C4::Context->preference('item-level_itypes')) {
>+                    $item->{itype} = $biblioitem->{itemtype};
>+                }
> 
>-            $item->{itypename} = $itemtypes->{ $item->{itype} }{description};
>-            $item->{imageurl} = getitemtypeimagelocation( 'intranet', $itemtypes->{ $item->{itype} }{imageurl} );
>-            $item->{homebranch} = $item->{homebranch};
>+                $item->{itypename} = $itemtypes->{ $item->{itype} }{description};
>+                $item->{imageurl} = getitemtypeimagelocation( 'intranet', $itemtypes->{ $item->{itype} }{imageurl} );
>+                $item->{homebranch} = $item->{homebranch};
> 
>-            # if the holdingbranch is different than the homebranch, we show the
>-            # holdingbranch of the document too
>-            if ( $item->{homebranch} ne $item->{holdingbranch} ) {
>-                $item->{holdingbranch} = $item->{holdingbranch};
>-            }
>+                # if the holdingbranch is different than the homebranch, we show the
>+                # holdingbranch of the document too
>+                if ( $item->{homebranch} ne $item->{holdingbranch} ) {
>+                    $item->{holdingbranch} = $item->{holdingbranch};
>+                }
> 
>-            if($item->{biblionumber} ne $biblionumber){
>-                $item->{hostitemsflag} = 1;
>-                $item->{hosttitle} = Koha::Biblios->find( $item->{biblionumber} )->title;
>-            }
>+                if($item->{biblionumber} ne $biblionumber){
>+                    $item->{hostitemsflag} = 1;
>+                    $item->{hosttitle} = Koha::Biblios->find( $item->{biblionumber} )->title;
>+                }
> 
>-            # if the item is currently on loan, we display its return date and
>-            # change the background color
>-            my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } );
>-            if ( $issue ) {
>-                $item->{date_due} = $issue->date_due;
>-                $item->{backgroundcolor} = 'onloan';
>-            }
>+                # if the item is currently on loan, we display its return date and
>+                # change the background color
>+                my $issue = Koha::Checkouts->find( { itemnumber => $itemnumber } );
>+                if ( $issue ) {
>+                    $item->{date_due} = $issue->date_due;
>+                    $item->{backgroundcolor} = 'onloan';
>+                }
> 
>-            # checking reserve
>-            my $item_object = Koha::Items->find( $itemnumber );
>-            my $holds = $item_object->current_holds;
>-            if ( my $first_hold = $holds->next ) {
>-                my $p = Koha::Patrons->find( $first_hold->borrowernumber );
>-
>-                $item->{backgroundcolor} = 'reserved';
>-                $item->{reservedate}     = output_pref({ dt => dt_from_string( $first_hold->reservedate ), dateonly => 1 }); # FIXME Should be formatted in the template
>-                $item->{ReservedFor}     = $p;
>-                $item->{ExpectedAtLibrary}     = $first_hold->branchcode;
>-                $item->{waitingdate} = $first_hold->waitingdate;
>-            }
>+                # checking reserve
>+                my $item_object = Koha::Items->find( $itemnumber );
>+                my $holds = $item_object->current_holds;
>+                if ( my $first_hold = $holds->next ) {
>+                    my $p = Koha::Patrons->find( $first_hold->borrowernumber );
>+
>+                    $item->{backgroundcolor} = 'reserved';
>+                    $item->{reservedate}     = output_pref({ dt => dt_from_string( $first_hold->reservedate ), dateonly => 1 }); # FIXME Should be formatted in the template
>+                    $item->{ReservedFor}     = $p;
>+                    $item->{ExpectedAtLibrary}     = $first_hold->branchcode;
>+                    $item->{waitingdate} = $first_hold->waitingdate;
>+                }
> 
>-            # Management of the notforloan document
>-            if ( $item->{notforloan} ) {
>-                $item->{backgroundcolor} = 'other';
>-            }
>+                # Management of the notforloan document
>+                if ( $item->{notforloan} ) {
>+                    $item->{backgroundcolor} = 'other';
>+                }
> 
>-            # Management of lost or long overdue items
>-            if ( $item->{itemlost} ) {
>-                $item->{backgroundcolor} = 'other';
>-                if ($logged_in_patron->category->hidelostitems && !$showallitems) {
>-                    $item->{hide} = 1;
>-                    $hiddencount++;
>+                # Management of lost or long overdue items
>+                if ( $item->{itemlost} ) {
>+                    $item->{backgroundcolor} = 'other';
>+                    if ($logged_in_patron->category->hidelostitems && !$showallitems) {
>+                        $item->{hide} = 1;
>+                        $hiddencount++;
>+                    }
>                 }
>-            }
> 
>-            # Check the transit status
>-            my ( $transfertwhen, $transfertfrom, $transfertto ) =
>-              GetTransfers($itemnumber);
>+                # Check the transit status
>+                my ( $transfertwhen, $transfertfrom, $transfertto ) =
>+                  GetTransfers($itemnumber);
> 
>-            if ( defined $transfertwhen && $transfertwhen ne '' ) {
>-                $item->{transfertwhen} = output_pref({ dt => dt_from_string( $transfertwhen ), dateonly => 1 });
>-                $item->{transfertfrom} = $transfertfrom;
>-                $item->{transfertto} = $transfertto;
>-                $item->{nocancel} = 1;
>-            }
>+                if ( defined $transfertwhen && $transfertwhen ne '' ) {
>+                    $item->{transfertwhen} = output_pref({ dt => dt_from_string( $transfertwhen ), dateonly => 1 });
>+                    $item->{transfertfrom} = $transfertfrom;
>+                    $item->{transfertto} = $transfertto;
>+                    $item->{nocancel} = 1;
>+                }
> 
>-            # If there is no loan, return and transfer, we show a checkbox.
>-            $item->{notforloan} ||= 0;
>-
>-            # if independent branches is on we need to check if the person can reserve
>-            # for branches they arent logged in to
>-            if ( C4::Context->preference("IndependentBranches") ) {
>-                if (! C4::Context->preference("canreservefromotherbranches")){
>-                    # can't reserve items so need to check if item homebranch and userenv branch match if not we can't reserve
>-                    my $userenv = C4::Context->userenv;
>-                    unless ( C4::Context->IsSuperLibrarian ) {
>-                        $item->{cantreserve} = 1 if ( $item->{homebranch} ne $userenv->{branch} );
>+                # If there is no loan, return and transfer, we show a checkbox.
>+                $item->{notforloan} ||= 0;
>+
>+                # if independent branches is on we need to check if the person can reserve
>+                # for branches they arent logged in to
>+                if ( C4::Context->preference("IndependentBranches") ) {
>+                    if (! C4::Context->preference("canreservefromotherbranches")){
>+                        # can't reserve items so need to check if item homebranch and userenv branch match if not we can't reserve
>+                        my $userenv = C4::Context->userenv;
>+                        unless ( C4::Context->IsSuperLibrarian ) {
>+                            $item->{cantreserve} = 1 if ( $item->{homebranch} ne $userenv->{branch} );
>+                        }
>                     }
>                 }
>-            }
> 
>-            if ( $patron ) {
>                 my $patron_unblessed = $patron->unblessed;
>                 my $branch = C4::Circulation::_GetCircControlBranch($item, $patron_unblessed);
> 
>@@ -623,9 +622,10 @@ foreach my $biblionumber (@biblionumbers) {
>                 if ($item->{ccode}) {
>                     $itemdata_ccode = 1;
>                 }
>-            }
> 
>             push @{ $biblioitem->{itemloop} }, $item;
>+
>+            }
>         }
> 
>         # While we can't override an alreay held item, we should be able to override the others
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 29043: 124941 | 125060 | 127731 | 127732 | 128254 | 128309 | 128336 | 128337 | 128338 | 128339 | 128340 | 128421 | 128422 | 128423 | 128474 | 128475 | 128476 | 128971 | 128972 | 128973 | 129200 | 129498 | 129530 | 129531 | 129820 | 129821 | 129915 | 129916 | 129949 | 129950