Bugzilla – Attachment 132245 Details for
Bug 30269
Use objects directly to display holds on request.pl
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 30269: Use objects directly on request.pl
Bug-30269-Use-objects-directly-on-requestpl.patch (text/plain), 20.16 KB, created by
Nick Clemens (kidclamp)
on 2022-03-25 13:01:16 UTC
(
hide
)
Description:
Bug 30269: Use objects directly on request.pl
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2022-03-25 13:01:16 UTC
Size:
20.16 KB
patch
obsolete
>From 45e6afc06e9f75ab66270e7d2db4ae607a7f1758 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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 @@ > <input type="hidden" name="rank-request" class="rank-request" value="[% hold.priority | html %]" data-hold-id="[% hold.reserve_id | html %]"> > <select name="rank-request" class="rank-request" disabled="disabled" data-hold-id="[% hold.reserve_id | html %]"> > [% IF ( hold.found ) %] >- [% IF ( hold.intransit ) %] >+ [% IF ( hold.found == 'T' ) %] > <option value="T" selected="selected">In transit</option> >- [% ELSIF (hold.inprocessing) %] >+ [% ELSIF ( hold.found == 'P' ) %] > <option value="P" selected="selected">In processing</option> > [% ELSE %] > <option value="W" selected="selected">Waiting</option> >@@ -86,16 +86,16 @@ > [%- SET prev_priority = loop.prev.priority -%] > [%- SET next_priority = loop.next.priority -%] > <td style="white-space:nowrap;"> >- <a title="Move hold up" href="request.pl?action=move&where=up&first_priority=[% first_priority | html %]&last_priority=[% last_priority | html %]&prev_priority=[% prev_priority | html %]&next_priority=[% next_priority | html %]&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.date | html %]"> >+ <a title="Move hold up" href="request.pl?action=move&where=up&first_priority=[% first_priority | html %]&last_priority=[% last_priority | html %]&prev_priority=[% prev_priority | html %]&next_priority=[% next_priority | html %]&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.reservedate | html %]"> > <img src="[% interface | html %]/[% theme | html %]/img/go-up.png" alt="Go up" /> > </a> >- <a title="Move hold to top" href="request.pl?action=move&where=top&first_priority=[% first_priority | html %]&last_priority=[% last_priority | html %]&prev_priority=[% prev_priority | html %]&next_priority=[% next_priority | html %]&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.date | html %]"> >+ <a title="Move hold to top" href="request.pl?action=move&where=top&first_priority=[% first_priority | html %]&last_priority=[% last_priority | html %]&prev_priority=[% prev_priority | html %]&next_priority=[% next_priority | html %]&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.reservedate | html %]"> > <img src="[% interface | html %]/[% theme | html %]/img/go-top.png" alt="Go top" /> > </a> >- <a title="Move hold to bottom" href="request.pl?action=move&where=bottom&first_priority=[% first_priority | html %]&last_priority=[% last_priority | html %]&prev_priority=[% prev_priority | html %]&next_priority=[% next_priority | html %]&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.date | html %]"> >+ <a title="Move hold to bottom" href="request.pl?action=move&where=bottom&first_priority=[% first_priority | html %]&last_priority=[% last_priority | html %]&prev_priority=[% prev_priority | html %]&next_priority=[% next_priority | html %]&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.reservedate | html %]"> > <img src="[% interface | html %]/[% theme | html %]/img/go-bottom.png" alt="Go bottom" /> > </a> >- <a title="Move hold down" href="request.pl?action=move&where=down&first_priority=[% first_priority | html %]&last_priority=[% last_priority | html %]&prev_priority=[% prev_priority | html %]&next_priority=[% next_priority | html %]&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.date | html %]"> >+ <a title="Move hold down" href="request.pl?action=move&where=down&first_priority=[% first_priority | html %]&last_priority=[% last_priority | html %]&prev_priority=[% prev_priority | html %]&next_priority=[% next_priority | html %]&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.reservedate | html %]"> > <img src="[% interface | html %]/[% theme | html %]/img/go-down.png" alt="Go down" /> > </a> > </td> >@@ -106,12 +106,12 @@ > <td> > [% INCLUDE 'patron-title.inc' patron=hold.patron hide_patron_infos_if_needed=1 %] > </td> >- <td>[% hold.notes | html | html_line_break %]</td> >+ <td>[% hold.reservenotes | html | html_line_break %]</td> > <td> > [% IF Koha.Preference('AllowHoldDateInFuture') %] >- <input type="text" class="flatpickr" value="[% hold.date | $KohaDates %]" required="required" size="10" name="reservedate" /> >+ <input type="text" class="flatpickr" value="[% hold.reservedate | $KohaDates %]" required="required" size="10" name="reservedate" /> > [% ELSE %] >- [% hold.date | $KohaDates %] >+ [% hold.reservedate | $KohaDates %] > [% END %] > </td> > <td> >@@ -119,26 +119,26 @@ > </td> > <td> > [%- IF ( hold.found ) -%] >- <input type="hidden" name="pickup" value="[% hold.wbrcode | html %]" /> >- [%- IF ( hold.atdestination ) -%] >- Item waiting at <strong> [% hold.wbrname | html %]</strong>[% IF hold.desk_name %], [% hold.desk_name | html %],[% END %] since [% hold.waiting_date | $KohaDates %] >- [%- ELSIF (hold.intransit) -%] >- Item being transferred to <strong> [% hold.wbrname | html %]</strong> >- [%- ELSIF (hold.inprocessing) -%] >- Item being processed at <strong> [% hold.wbrname | html %]</strong> >+ <input type="hidden" name="pickup" value="[% hold.branchcode | html %]" /> >+ [%- IF ( hold.found == 'W' && hold.item.holdingbranch == hold.branchcode ) -%] >+ Item waiting at <strong> [% hold.branch.branchname | html %]</strong>[% IF hold.desk %], [% hold.desk.desk_name | html %],[% END %] since [% hold.waitingdate | $KohaDates %] >+ [%- ELSIF (hold.found == 'T') -%] >+ Item being transferred to <strong> [% hold.branch.branchname | html %]</strong> >+ [%- ELSIF (hold.found == 'P') -%] >+ Item being processed at <strong> [% hold.branch.branchname | html %]</strong> > [%- ELSE -%] >- Hold expected at <strong>[% hold.wbrname | html %]</strong>, please checkin to verify status >+ Hold expected at <strong>[% hold.branch.branchname | html %]</strong>, please checkin to verify status > [%- END -%] > [%- ELSE -%] > [%- IF Koha.Preference('IndependentBranches') && Branches.all().size == 1 -%] >- [% Branches.GetName(hold.branchcode) | html %] <input type="hidden" name="pickup" value="[% hold.branchcode | html %]" /> >+ [% hold.branch.branchname | html %] <input type="hidden" name="pickup" value="[% hold.branchcode | html %]" /> > [%- ELSE -%] > <select class="pickup_location_dropdown" > data-selected="[% hold.branchcode | html %]" > data-hold-id="[% hold.reserve_id | html %]" > data-pickup-location-source="hold" > name="pickup"> >- <option selected="selected" value="[% hold.branchcode | html %]">[% Branches.GetName(hold.branchcode) | html %]</option> >+ <option selected="selected" value="[% hold.branchcode | html %]">[% hold.branch.branchname | html %]</option> > <option value="" disabled="disabled" class="loading">Loading...</option> > </select> > <img class="loading_[% hold.reserve_id | html %]" src="[% interface | html %]/[% theme | html %]/img/spinner-small.gif" alt="" style="display:none;"/> >@@ -148,8 +148,8 @@ > <td> > [%- IF ( hold.found ) -%] > <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% hold.biblionumber | uri %]&itemnumber=[% hold.itemnumber | uri %]#item[% hold.itemnumber | uri %]"> >- [%- IF ( hold.barcodenumber ) -%] >- [%- hold.barcodenumber | html -%] >+ [%- IF ( hold.item.barcode ) -%] >+ [%- hold.item.barcode | html -%] > <input type="hidden" name="itemnumber" value="[% hold.itemnumber | html %]" /> > [%- ELSE -%] > No barcode >@@ -160,8 +160,8 @@ > <em> > Only item > <a href="/cgi-bin/koha/catalogue/moredetail.pl?biblionumber=[% hold.biblionumber | uri %]&itemnumber=[% hold.itemnumber | uri %]#item[% hold.itemnumber | uri %]"> >- [%- IF ( hold.barcodenumber ) -%] >- [%- hold.barcodenumber | html -%] >+ [%- IF ( hold.item.barcode ) -%] >+ [%- hold.item.barcode | html -%] > <input type="hidden" name="itemnumber" value="[% hold.itemnumber | html %]" /> > [%- ELSE -%] > No barcode >@@ -185,7 +185,7 @@ > [%- IF ( CAN_user_reserveforothers_modify_holds_priority ) -%] > [%- UNLESS hold.found -%] > <td> >- <a title="Toggle lowest priority" href="request.pl?action=setLowestPriority&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.date | html %]"> >+ <a title="Toggle lowest priority" href="request.pl?action=setLowestPriority&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.reservedate | html %]"> > [%- IF ( hold.lowestPriority ) -%] > <img src="[% interface | html %]/[% theme | html %]/img/go-bottom.png" alt="Unset lowest priority" /> > [%- ELSE -%] >@@ -198,7 +198,7 @@ > [%- END -%] > [%- END -%] > <td> >- <a class="btn btn-default btn-xs cancel-hold" data-borrowernumber="[% hold.borrowernumber | html %]" data-biblionumber="[% hold.biblionumber | html %]" data-id="[% hold.reserve_id | html %]" href="request.pl?action=cancel&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.date | html %]"> >+ <a class="btn btn-default btn-xs cancel-hold" data-borrowernumber="[% hold.borrowernumber | html %]" data-biblionumber="[% hold.biblionumber | html %]" data-id="[% hold.reserve_id | html %]" href="request.pl?action=cancel&borrowernumber=[% hold.borrowernumber | html %]&biblionumber=[% hold.biblionumber | html %]&reserve_id=[% hold.reserve_id | html %]&date=[% hold.reservedate | html %]"> > <i class="fa fa-trash" aria-hidden="true"></i> Cancel > </a> > </td> >@@ -222,10 +222,10 @@ > [%- END -%] > [%- END # IF SuspendHoldsIntranet -%] > [%- IF ( hold.found ) -%] >- <input type="button" id="revert_hold_[% hold.reserve_id | html %]" value="[% IF hold.intransit %]Revert transit status[% ELSE %]Revert waiting status[% END %]" onclick="window.location.href='request.pl?action=move&where=down&first_priority=[% first_priority | uri %]&last_priority=[% last_priority | uri %]&prev_priority=0&next_priority=1&borrowernumber=[% hold.borrowernumber | uri %]&biblionumber=[% hold.biblionumber | uri %]&itemnumber=[% hold.itemnumber | uri %]&reserve_id=[% hold.reserve_id | uri %]&date=[% hold.date | uri %]'"> >+ <input type="button" id="revert_hold_[% hold.reserve_id | html %]" value="[% IF hold.found == 'T' %]Revert transit status[% ELSE %]Revert waiting status[% END %]" onclick="window.location.href='request.pl?action=move&where=down&first_priority=[% first_priority | uri %]&last_priority=[% last_priority | uri %]&prev_priority=0&next_priority=1&borrowernumber=[% hold.borrowernumber | uri %]&biblionumber=[% hold.biblionumber | uri %]&itemnumber=[% hold.itemnumber | uri %]&reserve_id=[% hold.reserve_id | uri %]&date=[% hold.reservedate | uri %]'"> > [%- END -%] > </td> >- [% IF ( hold.intransit || hold.atdestination ) %] >+ [% IF ( hold.found == 'T' || ( hold.found == 'W' && hold.item.holdingbranch == hold.branchcode ) ) %] > <td><input class="printholdslip" type="button" name="printholdslip" value="Print slip" data-reserve_id=[% hold.reserve_id | html %]></td> > [% ELSE %] > <td></td> >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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=132245">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 30269
:
131558
|
131559
|
132245
|
134548
|
134552
|
136794
|
137725
|
137726