From 1a05874a92d5edd75f15a4997259e51aa46fa37b 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 2c1fd418dd..1225da8787 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 c89a89ee90..b1283287d2 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