Bugzilla – Attachment 138916 Details for
Bug 31314
Remove GetItemsInfo from opac-reserve.pl
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31314: Remove GetHostItemsInfo and GetItemsInfo from opac-reserve
Bug-31314-Remove-GetHostItemsInfo-and-GetItemsInfo.patch (text/plain), 21.18 KB, created by
Owen Leonard
on 2022-08-09 14:19:54 UTC
(
hide
)
Description:
Bug 31314: Remove GetHostItemsInfo and GetItemsInfo from opac-reserve
Filename:
MIME Type:
Creator:
Owen Leonard
Created:
2022-08-09 14:19:54 UTC
Size:
21.18 KB
patch
obsolete
>From c7d81e2097b3f8fffbbcd0fc6a24766dbabae449 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 3 Jun 2022 07:44:46 +0200 >Subject: [PATCH] Bug 31314: Remove GetHostItemsInfo and GetItemsInfo from > opac-reserve > >Bug 27272 is going to remove C4::Items::GetItemsInfo in favour of Koha::Items->search_ordered. > >Here we are going to deal with opac-reserve > >Test plan: >List items on the modified view and confirm that all the info is >displayed correctly > >Signed-off-by: Owen Leonard <oleonard@myacpl.org> >--- > .../bootstrap/en/modules/opac-reserve.tt | 26 +-- > opac/opac-reserve.pl | 160 ++++++------------ > 2 files changed, 61 insertions(+), 125 deletions(-) > >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >index 8bb46e8ff7..1d1d55fe4d 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >@@ -370,7 +370,7 @@ > [% SET unholdable_items = 0 %] > [% FOREACH itemLoo IN bibitemloo.itemLoop %] > [% IF ( itemLoo.available ) %] >- [% IF ( itemLoo.onloan ) %] >+ [% IF ( itemLoo.checkout ) %] > <tr class="holdable onloan"> > [% ELSE %] > <tr class="holdable"> >@@ -379,7 +379,7 @@ > <input type="radio" class="checkitem checkitem_[% bibitemloo.biblionumber | html %]" name="checkitem_[% bibitemloo.biblionumber | html %]" value="[% itemLoo.itemnumber | html %]" /> > [% ELSE %] > [% SET unholdable_items = 1 %] >- [% IF ( itemLoo.onloan ) %] >+ [% IF ( itemLoo.checkout ) %] > <tr class="unholdable onloan" style="display:none;"> > [% ELSE %] > <tr class="unholdable" style="display:none;"> >@@ -401,19 +401,19 @@ > <img src="[% itemLoo.imageurl | html %]" alt="" /> > [% END %] > [% END %] >- [% itemLoo.translated_description | html %] >+ [% itemLoo.itemtype.translated_description | html %] > </td> > [% END %] > > <td class="barcode">[% itemLoo.barcode | html %]</td> > [% UNLESS ( singleBranchMode ) %] >- <td class="homebranch">[% Branches.GetName( itemLoo.homeBranchName ) | html %]</td> >- <td class="holdingbranch">[% Branches.GetName( itemLoo.holdingBranchName ) | html %]</td> >+ <td class="homebranch">[% Branches.GetName( itemLoo.homebranch) | html %]</td> >+ <td class="holdingbranch">[% Branches.GetName( itemLoo.holdingbranch ) | html %]</td> > [% END %] > [% IF ( itemdata_ccode ) %] > <td class="ccode"> [% IF ( itemLoo.ccode ) %][% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode', authorised_value => itemLoo.ccode, opac => 1 ) | html %][% END %]</td> > [% END %] >- <td class="call_no">[% itemLoo.callNumber | html %]</td> >+ <td class="call_no">[% itemLoo.itemcallnumber | html %]</td> > [% IF ( itemdata_enumchron ) %] > <td class="vol_info">[% itemLoo.enumchron | html %]</td> > [% END %] >@@ -421,21 +421,21 @@ > [% itemLoo.itemnotes | html %] > </td> > <td class="information"> >- [% IF ( itemLoo.dateDue ) %] >- <span class="checkedout">Due [% itemLoo.dateDue | $KohaDates as_due_date => 1 %]</span> >+ [% IF ( itemLoo.checkout.date_due) %] >+ <span class="checkedout">Due [% itemLoo.checkout.date_due| $KohaDates as_due_date => 1 %]</span> > [% ELSIF ( itemLoo.transfertwhen ) %] >- <span class="intransit">In transit from [% Branches.GetName( itemLoo.transfertfrom ) | html %] to [% Branches.GetName( itemLoo.transfertto ) | html %] since [% itemLoo.transfertwhen | html %]</span> >+ <span class="intransit">In transit from [% Branches.GetName( itemLoo.transfertfrom ) | html %] to [% Branches.GetName( itemLoo.transfertto ) | html %] since [% itemLoo.transfertwhen | $KohaDates %]</span> > [% END %] > >- [% IF ( itemLoo.message ) %] >+ [% IF ( itemLoo.itemlost == 1 || itemLoo.itemlost == 2 ) %] [%# FIXME Why only for 1 or 2? Shouldn't we test for withdrawn as well? %] > <span class="lost">Unavailable (lost or missing)</span> > [% END %] > > [% IF ( itemLoo.notforloan ) %] >- <span class="notforloan">Not for loan ([% itemLoo.notforloanvalue | html %])</span> >+ <span class="notforloan">Not for loan ([% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.notforloan', authorised_value => itemLoo.notforloan ) %] %])</span> > [% END %] > >- [% IF ( itemLoo.reservedate ) %] >+ [% IF ( itemLoo.first_hold ) %] > <span class="waiting"> > [% IF ( itemLoo.waitingdate ) %] > <span>Waiting for patron at [% Branches.GetName( itemLoo.ExpectedAtLibrary ) | html %] since [% itemLoo.waitingdate | $KohaDates %]</span> >@@ -447,7 +447,7 @@ > </span> > [% ELSE %] > <span class="notonhold">Not on hold</span> >- [% END # / IF ( itemLoo.reservedate )%] >+ [% END # / IF ( itemLoo.first_hold )%] > </td> > </tr> > [% END # / FOREACH itemLoo IN bibitemloo.itemLoop%] >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index a4cacfe50f..6cdbbbd3df 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -26,7 +26,6 @@ use C4::Koha qw( getitemtypeimagelocation getitemtypeimagesrc ); > use C4::Circulation qw( GetBranchItemRule GetTransfers ); > use C4::Reserves qw( CanItemBeReserved CanBookBeReserved AddReserve GetReservesControlBranch ItemsAnyAvailableAndNotRestricted IsAvailableForItemLevelRequest ); > use C4::Biblio qw( GetBiblioData GetFrameworkCode ); >-use C4::Items qw( GetHostItemsInfo GetItemsInfo ); > use C4::Output qw( output_html_with_http_headers ); > use C4::Context; > use C4::Members; >@@ -142,30 +141,32 @@ $template->param( branch => $branch ); > # > > my %biblioDataHash; # Hash of biblionumber to biblio/biblioitems record. >-my %itemInfoHash; # Hash of itemnumber to item info. > foreach my $biblioNumber (@biblionumbers) { > > my $biblioData = GetBiblioData($biblioNumber); > $biblioDataHash{$biblioNumber} = $biblioData; > >- my @itemInfos = GetItemsInfo($biblioNumber); >- > my $biblio = Koha::Biblios->find( $biblioNumber ); > next unless $biblio; > > my $marcrecord = $biblio->metadata->record; > >- # flag indicating existence of at least one item linked via a host record >- # adding items linked via host biblios >- my @hostitemInfos = GetHostItemsInfo($marcrecord); >- if (@hostitemInfos){ >- push (@itemInfos,@hostitemInfos); >- } >+ my $items = >+ $biblio->items->filter_by_visible_in_opac( { patron => $patron } ); >+ my $host_items = >+ $biblio->host_items->filter_by_visible_in_opac( { patron => $patron } ); >+ $items = $biblio->items->search_ordered( >+ { >+ itemnumber => { >+ -in => [ >+ $items->get_column('itemnumber'), >+ $host_items->get_column('itemnumber') >+ ] >+ } >+ } >+ ); > >- $biblioData->{itemInfos} = \@itemInfos; >- foreach my $itemInfo (@itemInfos) { >- $itemInfoHash{$itemInfo->{itemnumber}} = $itemInfo; >- } >+ $biblioData->{items} = [$items->as_list]; # FIXME Potentially a lot in memory here! > > # Compute the priority rank. > $biblioData->{object} = $biblio; >@@ -237,6 +238,7 @@ if ( $query->param('place_reserve') ) { > $branch = $patron->branchcode; > } > >+ # FIXME We shouldn't need to fetch the item here > my $item = $itemNum ? Koha::Items->find( $itemNum ) : undef; > # When choosing a specific item, the default pickup library should be dictated by the default hold policy > if ( ! C4::Context->preference("OPACAllowUserToChooseBranch") && $item ) { >@@ -411,6 +413,7 @@ my $itemLevelTypes = C4::Context->preference('item-level_itypes'); > my $pickup_locations = Koha::Libraries->search({ pickup_location => 1 }); > $template->param('item_level_itypes' => $itemLevelTypes); > >+my $patron_unblessed = $patron->unblessed; > foreach my $biblioNum (@biblionumbers) { > > # Init the bib item with the choices for branch pickup >@@ -448,25 +451,7 @@ foreach my $biblioNum (@biblionumbers) { > $biblioLoopIter{imageurl} = getitemtypeimagesrc() . "/". $itemtypes->{$biblioData->{itemtype}}{imageurl}; > } > >- foreach my $itemInfo (@{$biblioData->{itemInfos}}) { >- if ($itemLevelTypes && $itemInfo->{itype}) { >- $itemInfo->{translated_description} = $itemtypes->{$itemInfo->{itype}}{translated_description}; >- $itemInfo->{imageurl} = getitemtypeimagesrc() . "/". $itemtypes->{$itemInfo->{itype}}{imageurl}; >- } >- >- if (!$itemInfo->{'notforloan'} && !($itemInfo->{'itemnotforloan'} > 0)) { >- $biblioLoopIter{forloan} = 1; >- } >- } >- >- my @notforloan_avs = Koha::AuthorisedValues->search_by_koha_field({ kohafield => 'items.notforloan', frameworkcode => $frameworkcode })->as_list; >- my $notforloan_label_of = { map { $_->authorised_value => $_->opac_description } @notforloan_avs }; >- >- my $visible_items = { map { $_->itemnumber => $_ } $biblio->items->filter_by_visible_in_opac( { patron => $patron } )->as_list }; > >- # Only keep the items that are visible in the opac (i.e. those in %visible_items) >- # FIXME: We should get rid of itemInfos altogether and use $visible_items >- $biblioData->{itemInfos} = [ grep { $visible_items->{ $_->{itemnumber} } } @{ $biblioData->{itemInfos} } ]; > > $biblioLoopIter{itemLoop} = []; > my $numCopiesAvailable = 0; >@@ -477,91 +462,44 @@ foreach my $biblioNum (@biblionumbers) { > # (before this loop was inside that sub loop so it was O(n^2) ) > my $items_any_available; > $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $biblioNum, patron => $patron }) if $patron; >- foreach my $itemInfo (@{$biblioData->{itemInfos}}) { >- my $itemNum = $itemInfo->{itemnumber}; >- my $item = $visible_items->{$itemNum}; >- my $itemLoopIter = {}; >- >- $itemLoopIter->{itemnumber} = $itemNum; >- $itemLoopIter->{barcode} = $itemInfo->{barcode}; >- $itemLoopIter->{homeBranchName} = $itemInfo->{homebranch}; >- $itemLoopIter->{callNumber} = $itemInfo->{itemcallnumber}; >- $itemLoopIter->{enumchron} = $itemInfo->{enumchron}; >- $itemLoopIter->{ccode} = $itemInfo->{ccode}; >- $itemLoopIter->{copynumber} = $itemInfo->{copynumber}; >- $itemLoopIter->{itemnotes} = $itemInfo->{itemnotes}; >- if ($itemLevelTypes) { >- $itemLoopIter->{translated_description} = $itemInfo->{translated_description}; >- $itemLoopIter->{itype} = $itemInfo->{itype}; >- $itemLoopIter->{imageurl} = $itemInfo->{imageurl}; >- } >- >- # If the holdingbranch is different than the homebranch, we show the >- # holdingbranch of the document too. >- if ( $itemInfo->{homebranch} ne $itemInfo->{holdingbranch} ) { >- $itemLoopIter->{holdingBranchName} = $itemInfo->{holdingbranch}; >- } >+ foreach my $item (@{$biblioData->{items}}) { > >- # If the item is currently on loan, we display its return date and >- # change the background color. >- my $issue = Koha::Checkouts->find( { itemnumber => $itemNum } ); >- if ( $issue ) { >- $itemLoopIter->{dateDue} = $issue->date_due; >- $itemLoopIter->{onloan} = 'onloan'; >+ my $item_info = $item->unblessed; >+ $item_info->{holding_branch} = $item->holding_branch; >+ $item_info->{home_branch} = $item->home_branch; >+ if ($itemLevelTypes) { >+ my $itemtype = $item->itemtype; >+ $item_info->{'imageurl'} = getitemtypeimagelocation( 'opac', >+ $itemtypes->{ $itemtype->itemtype }->{'imageurl'} ); >+ $item_info->{'translated_description'} = >+ $itemtypes->{ $itemtype->itemtype }->{translated_description}; > } > >- # checking reserve >+ # checking for holds > my $holds = $item->current_holds; >- > if ( my $first_hold = $holds->next ) { >- $itemLoopIter->{backgroundcolor} = 'reserved'; >- $itemLoopIter->{reservedate} = $first_hold->reservedate; >- $itemLoopIter->{ExpectedAtLibrary} = $first_hold->branchcode; >- $itemLoopIter->{waitingdate} = $first_hold->waitingdate; >+ $item_info->{first_hold} = $first_hold; > } > >- $itemLoopIter->{notforloan} = $itemInfo->{notforloan}; >- $itemLoopIter->{itemnotforloan} = $itemInfo->{itemnotforloan}; >- >- # Management of the notforloan document >- if ( $itemLoopIter->{notforloan} || $itemLoopIter->{itemnotforloan}) { >- $itemLoopIter->{backgroundcolor} = 'other'; >- $itemLoopIter->{notforloanvalue} = >- $notforloan_label_of->{ $itemLoopIter->{notforloan} }; >- } >- >- # Management of lost or long overdue items >- if ( $itemInfo->{itemlost} ) { >- >- # FIXME localized strings should never be in Perl code >- $itemLoopIter->{message} = >- $itemInfo->{itemlost} == 1 ? "(lost)" >- : $itemInfo->{itemlost} == 2 ? "(long overdue)" >- : ""; >- $itemInfo->{backgroundcolor} = 'other'; >- } >+ $item_info->{checkout} = $item->checkout; > > # Check of the transferred documents > my ( $transfertwhen, $transfertfrom, $transfertto ) = >- GetTransfers($itemNum); >+ GetTransfers($item->itemnumber); > if ( $transfertwhen && ($transfertwhen ne '') ) { >- $itemLoopIter->{transfertwhen} = output_pref({ dt => dt_from_string($transfertwhen), dateonly => 1 }); >- $itemLoopIter->{transfertfrom} = $transfertfrom; >- $itemLoopIter->{transfertto} = $transfertto; >- $itemLoopIter->{nocancel} = 1; >+ $item_info->{transfertwhen} = $transfertwhen; >+ $item_info->{transfertfrom} = $transfertfrom; >+ $item_info->{transfertto} = $transfertto; >+ $item_info->{nocancel} = 1; > } > > # if the items belongs to a host record, show link to host record >- if ( $itemInfo->{biblionumber} ne $biblioNum ) { >- $itemLoopIter->{hostbiblionumber} = $itemInfo->{biblionumber}; >- $itemLoopIter->{hosttitle} = Koha::Biblios->find( $itemInfo->{biblionumber} )->title; >+ if ( $item_info->{biblionumber} ne $biblioNum ) { >+ $item_info->{hostbiblionumber} = $item->biblionumber; >+ $item_info->{hosttitle} = Koha::Biblios->find( $item_info->{biblionumber} )->title; > } > >- # If there is no loan, return and transfer, we show a checkbox. >- $itemLoopIter->{notforloan} = $itemLoopIter->{notforloan} || 0; >- >- my $patron_unblessed = $patron->unblessed; >- my $branch = GetReservesControlBranch( $itemInfo, $patron_unblessed ); >+ my $branch = GetReservesControlBranch( $item_info, $patron_unblessed ); > > # items_any_available defined outside of the current loop, > # so we avoiding loop inside IsAvailableForItemLevelRequest: >@@ -572,33 +510,31 @@ foreach my $biblioNum (@biblionumbers) { > if ($policy_holdallowed) { > my $opac_hold_policy = Koha::CirculationRules->get_opacitemholds_policy( { item => $item, patron => $patron } ); > if ( $opac_hold_policy ne 'N' ) { # If Y or F >- $itemLoopIter->{available} = 1; >+ $item_info->{available} = 1; > $numCopiesOPACAvailable++; > $biblioLoopIter{force_hold} = 1 if $opac_hold_policy eq 'F'; > } > $numCopiesAvailable++; > > unless ( $can_place_hold_if_available_at_pickup ) { >- my $items_in_this_library = Koha::Items->search({ biblionumber => $itemInfo->{biblionumber}, holdingbranch => $itemInfo->{holdingbranch} }); >+ my $items_in_this_library = Koha::Items->search({ biblionumber => $item->biblionumber, holdingbranch => $item->holdingbranch }); > my $nb_of_items_issued = $items_in_this_library->search({ 'issue.itemnumber' => { not => undef }}, { join => 'issue' })->count; > if ( $items_in_this_library->count > $nb_of_items_issued ) { >- push @not_available_at, $itemInfo->{holdingbranch}; >+ push @not_available_at, $item->holdingbranch; > } > } > } > >- $itemLoopIter->{imageurl} = getitemtypeimagelocation( 'opac', $itemtypes->{ $itemInfo->{itype} }{imageurl} ); >- >- # Show serial enumeration when needed >- if ($itemLoopIter->{enumchron}) { >+ # Show serial enumeration when needed >+ if ($item_info->{enumchron}) { > $itemdata_enumchron = 1; > } >- # Show collection when needed >- if ($itemLoopIter->{ccode}) { >+ # Show collection when needed >+ if ($item_info->{ccode}) { > $itemdata_ccode = 1; > } > >- push @{$biblioLoopIter{itemLoop}}, $itemLoopIter; >+ push @{$biblioLoopIter{itemLoop}}, $item_info; > } > $template->param( > itemdata_enumchron => $itemdata_enumchron, >@@ -635,7 +571,7 @@ foreach my $biblioNum (@biblionumbers) { > > if ( $biblioLoopIter{holdable} and C4::Context->preference('AllowHoldItemTypeSelection') ) { > # build the allowed item types loop >- my $rs = $biblio->items->search( >+ my $rs = $biblio->items->search_ordered( > undef, > { select => [ { distinct => 'itype' } ], > as => 'item_type' >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 31314
:
138823
|
138882
|
138916
|
139066
|
139067
|
139068