From 4f39323d829e4f49ddda5e9e863e1ebb65f9f1e2 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..76a84cdb0c 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( + "branch", + "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..dbda7ece69 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.waiting_date | $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 19c5c13d0a..dea6a35b6f 100755 --- a/reserve/request.pl +++ b/reserve/request.pl @@ -598,58 +598,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','branch','desk'], + order_by => [ {-desc => 'found'},'priority'] } )->as_list; } # get the time for the form name... -- 2.30.2