From 45e6afc06e9f75ab66270e7d2db4ae607a7f1758 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 10 Mar 2022 14:08:42 +0000 Subject: [PATCH] Bug 30269: Use objects directly on request.pl This patch removes our loop over the hold objects, and simply passes them directly to the template Template is adjusted to use ojbects and accessor method Related objects are prefetched to avoid more DB calls when display Use of 'iswaiting' and related methods are removed - it is faster to use the accessor and test To test: 1 - Place many holds on a record 2 - Set soem waiting, some in transit 3 - View the holds page and note loading time 4 - Apply patch 5 - Reload 6 - Page appears the same 7 - Load time is faster (more with more holds) --- Koha/Schema/Result/Reserve.pm | 24 ++++++++ .../prog/en/includes/holds_table.inc | 54 +++++++++--------- reserve/request.pl | 55 +------------------ 3 files changed, 54 insertions(+), 79 deletions(-) diff --git a/Koha/Schema/Result/Reserve.pm b/Koha/Schema/Result/Reserve.pm index 04b81a1960..ea127b5c08 100644 --- a/Koha/Schema/Result/Reserve.pm +++ b/Koha/Schema/Result/Reserve.pm @@ -465,6 +465,30 @@ __PACKAGE__->belongs_to( { is_deferrable => 1, on_delete => "CASCADE", on_update => "CASCADE" }, ); +__PACKAGE__->belongs_to( + "library", + "Koha::Schema::Result::Branch", + { branchcode => "branchcode" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "CASCADE", + on_update => "CASCADE", + }, +); + +__PACKAGE__->belongs_to( + "desk", + "Koha::Schema::Result::Desk", + { desk_id => "desk_id" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "SET NULL", + on_update => "CASCADE", + }, +); + __PACKAGE__->add_columns( '+item_level_hold' => { is_boolean => 1 }, '+lowestPriority' => { is_boolean => 1 }, diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc index b3951572ca..f498d75835 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -68,9 +68,9 @@ + [% ELSE %] - [% hold.date | $KohaDates %] + [% hold.reservedate | $KohaDates %] [% END %] @@ -119,26 +119,26 @@ [%- IF ( hold.found ) -%] - - [%- IF ( hold.atdestination ) -%] - Item waiting at [% hold.wbrname | html %][% IF hold.desk_name %], [% hold.desk_name | html %],[% END %] since [% hold.waiting_date | $KohaDates %] - [%- ELSIF (hold.intransit) -%] - Item being transferred to [% hold.wbrname | html %] - [%- ELSIF (hold.inprocessing) -%] - Item being processed at [% hold.wbrname | html %] + + [%- IF ( hold.found == 'W' && hold.item.holdingbranch == hold.branchcode ) -%] + Item waiting at [% hold.branch.branchname | html %][% IF hold.desk %], [% hold.desk.desk_name | html %],[% END %] since [% hold.waitingdate | $KohaDates %] + [%- ELSIF (hold.found == 'T') -%] + Item being transferred to [% hold.branch.branchname | html %] + [%- ELSIF (hold.found == 'P') -%] + Item being processed at [% hold.branch.branchname | html %] [%- ELSE -%] - Hold expected at [% hold.wbrname | html %], please checkin to verify status + Hold expected at [% hold.branch.branchname | html %], please checkin to verify status [%- END -%] [%- ELSE -%] [%- IF Koha.Preference('IndependentBranches') && Branches.all().size == 1 -%] - [% Branches.GetName(hold.branchcode) | html %] + [% hold.branch.branchname | html %] [%- ELSE -%] @@ -148,8 +148,8 @@ [%- IF ( hold.found ) -%] - [%- IF ( hold.barcodenumber ) -%] - [%- hold.barcodenumber | html -%] + [%- IF ( hold.item.barcode ) -%] + [%- hold.item.barcode | html -%] [%- ELSE -%] No barcode @@ -160,8 +160,8 @@ Only item - [%- IF ( hold.barcodenumber ) -%] - [%- hold.barcodenumber | html -%] + [%- IF ( hold.item.barcode ) -%] + [%- hold.item.barcode | html -%] [%- ELSE -%] No barcode @@ -185,7 +185,7 @@ [%- IF ( CAN_user_reserveforothers_modify_holds_priority ) -%] [%- UNLESS hold.found -%] - + [%- IF ( hold.lowestPriority ) -%] Unset lowest priority [%- ELSE -%] @@ -198,7 +198,7 @@ [%- END -%] [%- END -%] - + Cancel @@ -222,10 +222,10 @@ [%- END -%] [%- END # IF SuspendHoldsIntranet -%] [%- IF ( hold.found ) -%] - + [%- END -%] - [% IF ( hold.intransit || hold.atdestination ) %] + [% IF ( hold.found == 'T' || ( hold.found == 'W' && hold.item.holdingbranch == hold.branchcode ) ) %] [% ELSE %] diff --git a/reserve/request.pl b/reserve/request.pl index 15067821cc..56601ae6cb 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -584,58 +584,9 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) $template->param( always_show_holds => $always_show_holds ); my $show_holds_now = $input->param('show_holds_now'); unless( (defined $always_show_holds && $always_show_holds eq 'DONT') && !$show_holds_now ){ - my @reserves = Koha::Holds->search( { biblionumber => $biblionumber }, { order_by => 'priority' } )->as_list; - foreach my $res ( - sort { - my $a_found = $a->found() || ''; - my $b_found = $a->found() || ''; - $a_found cmp $b_found; - } @reserves - ) - { - my %reserve; - if ( $res->is_found() ) { - $reserve{'holdingbranch'} = $res->item()->holdingbranch(); - $reserve{'biblionumber'} = $res->item()->biblionumber(); - $reserve{'barcodenumber'} = $res->item()->barcode(); - $reserve{'wbrcode'} = $res->branchcode(); - $reserve{'itemnumber'} = $res->itemnumber(); - $reserve{'wbrname'} = $res->branch()->branchname(); - $reserve{'atdestination'} = $res->is_at_destination(); - $reserve{'desk_name'} = ( $res->desk() ) ? $res->desk()->desk_name() : '' ; - $reserve{'found'} = $res->is_found(); - $reserve{'inprocessing'} = $res->is_in_processing(); - $reserve{'intransit'} = $res->is_in_transit(); - } - elsif ( $res->priority() > 0 ) { - if ( my $item = $res->item() ) { - $reserve{'itemnumber'} = $item->id(); - $reserve{'barcodenumber'} = $item->barcode(); - $reserve{'item_level_hold'} = 1; - } - } - - $reserve{'expirationdate'} = $res->expirationdate; - $reserve{'date'} = $res->reservedate; - $reserve{'borrowernumber'} = $res->borrowernumber(); - $reserve{'biblionumber'} = $res->biblionumber(); - $reserve{'patron'} = $res->borrower; - $reserve{'notes'} = $res->reservenotes(); - $reserve{'waiting_date'} = $res->waitingdate(); - $reserve{'ccode'} = $res->item() ? $res->item()->ccode() : undef; - $reserve{'barcode'} = $res->item() ? $res->item()->barcode() : undef; - $reserve{'priority'} = $res->priority(); - $reserve{'lowestPriority'} = $res->lowestPriority(); - $reserve{'suspend'} = $res->suspend(); - $reserve{'suspend_until'} = $res->suspend_until(); - $reserve{'reserve_id'} = $res->reserve_id(); - $reserve{itemtype} = $res->itemtype(); - $reserve{branchcode} = $res->branchcode(); - $reserve{non_priority} = $res->non_priority(); - $reserve{object} = $res; - - push( @reserveloop, \%reserve ); - } + @reserveloop = Koha::Holds->search( { 'me.biblionumber' => $biblionumber }, { + prefetch => ['item','patron','library','desk'], + order_by => [ {-desc => 'found'},'priority'] } )->as_list; } # get the time for the form name... -- 2.30.2