Bugzilla – Attachment 139119 Details for
Bug 31313
Remove GetItemsInfo from opac-detail
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31313: Remove GetItemsInfo from opac-detail
Bug-31313-Remove-GetItemsInfo-from-opac-detail.patch (text/plain), 20.70 KB, created by
Nick Clemens (kidclamp)
on 2022-08-15 11:48:00 UTC
(
hide
)
Description:
Bug 31313: Remove GetItemsInfo from opac-detail
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2022-08-15 11:48:00 UTC
Size:
20.70 KB
patch
obsolete
>From 3fc09cfaca4d6f507076a48e7aebc89e71531a06 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 3 Jun 2022 06:50:44 +0200 >Subject: [PATCH] Bug 31313: Remove GetItemsInfo from opac-detail > >Bug 27272 is going to remove C4::Items::GetItemsInfo in favour of Koha::Items->search_ordered. > >Here we are going to deal with opac-detail > >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> > >Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > .../bootstrap/en/modules/opac-detail.tt | 48 ++-- > opac/opac-detail.pl | 246 +++++++++--------- > 2 files changed, 152 insertions(+), 142 deletions(-) > >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt >index b3607465d8..d7efd5e415 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt >@@ -1246,16 +1246,16 @@ > <link property="itemOffered" href="#record" /> > <link property="businessFunction" href="http://purl.org/goodrelations/v1#LeaseOut"> > >- [% IF ( ITEM_RESULT.holding_branch_opac_info ) %] >- <a href="[% ITEM_RESULT.branchurl | url %]" class="library_info" data-branchcode="[% ITEM_RESULT.branchcode | html %]"> >- <i class="fa fa-info-circle" aria-hidden="true"></i> [% ITEM_RESULT.branchname | html %] >+ [% IF ( ITEM_RESULT.holding_branch.opac_info ) %] >+ <a href="[% ITEM_RESULT.holding_branch.branchurl | url %]" class="library_info" data-branchcode="[% ITEM_RESULT.holding_branch.branchcode | html %]"> >+ <i class="fa fa-info-circle" aria-hidden="true"></i> [% ITEM_RESULT.holding_branch.branchname | html %] > </a> >- [% ELSIF ( ITEM_RESULT.branchurl ) %] >+ [% ELSIF ( ITEM_RESULT.holding_branch.branchurl ) %] > <a href="[% ITEM_RESULT.branchurl | url %]"> >- [% ITEM_RESULT.branchname | html %] >+ [% ITEM_RESULT.holding_branch.branchname | html %] > </a> > [% ELSE %] >- <span>[% ITEM_RESULT.branchname | html %]</span> >+ <span>[% ITEM_RESULT.holding_branch.branchname | html %]</span> > [% END %] > > [% IF ( Koha.Preference('OpacLocationOnDetail') == 'holding' || Koha.Preference('OpacLocationOnDetail') == 'both' ) %] >@@ -1270,16 +1270,16 @@ > <link property="itemOffered" href="#record" /> > <link property="businessFunction" href="http://purl.org/goodrelations/v1#LeaseOut"> > >- [% IF ( ITEM_RESULT.holding_branch_opac_info ) %] >- <a href="[% Branches.GetURL( ITEM_RESULT.homebranch ) | url %]" class="library_info" data-branchcode="[% ITEM_RESULT.homebranch | html %]"> >- <i class="fa fa-info-circle" aria-hidden="true"></i> [% Branches.GetName( ITEM_RESULT.homebranch ) | html %] >+ [% IF ( ITEM_RESULT.holding_branch.opac_info ) %] [%# FIXME Shouldn't this be home_branch instead of holding_branch? %] >+ <a href="[% ITEM_RESULT.home_branch.branchurl | url %]" class="library_info" data-branchcode="[% ITEM_RESULT.home_branch.branchchode | html %]"> >+ <i class="fa fa-info-circle" aria-hidden="true"></i> [% ITEM_RESULT.home_branch.branchname | html %] > </a> >- [% ELSIF ( Branches.GetURL( ITEM_RESULT.homebranch ) ) %] >- <a href="[% Branches.GetURL( ITEM_RESULT.homebranch ) | url %]"> >- [% Branches.GetName( ITEM_RESULT.homebranch ) | html %] >+ [% ELSIF ( ITEM_RESULT.home_branch.branchurl ) %] >+ <a href="[% ITEM_RESULT.home_branch.branchurl | url %]"> >+ [% ITEM_RESULT.home_branch.branchname | html %] > </a> > [% ELSE %] >- <span>[% Branches.GetName( ITEM_RESULT.homebranch ) | html %]</span> >+ <span>[% ITEM_RESULT.home_branch.branchname | html %]</span> > [% END %] > > [% IF ( Koha.Preference('OpacLocationOnDetail') == 'home' || Koha.Preference('OpacLocationOnDetail') == 'both' ) %] >@@ -1315,20 +1315,21 @@ > [% END %] > > [% IF ( itemdata_enumchron ) %] >+ [% SET serial = ITEM_RESULT.serial %] > <td class="vol_info"> >- [% IF ITEM_RESULT.enumchron && ITEM_RESULT.serialseq %] >+ [% IF ITEM_RESULT.enumchron && serial.serialseq %] > <span class="enum">[% ITEM_RESULT.enumchron | html %]</span> >- [% IF ( ITEM_RESULT.serialseq && ITEM_RESULT.enumchron!=ITEM_RESULT.serialseq ) %] >+ [% IF ( serial.serialseq && ITEM_RESULT.enumchron!=serial.serialseq ) %] > <span class="sep"> -- </span> >- <span class="serialseq">[% ITEM_RESULT.serialseq | html %]</span> >+ <span class="serialseq">[% serial.serialseq | html %]</span> > [% END %] > [% ELSIF ITEM_RESULT.enumchron %] > <span class="enum">[% ITEM_RESULT.enumchron | html %]</span> >- [% ELSIF ITEM_RESULT.serialseq %] >- <span class="serialseq">[% ITEM_RESULT.serialseq | html %]</span> >+ [% ELSIF serial.serialseq %] >+ <span class="serialseq">[% serial.serialseq | html %]</span> > [% END %] >- [% IF ( ITEM_RESULT.publisheddate ) %] >- <span class="pubdate">([% ITEM_RESULT.publisheddate | $KohaDates %])</span> >+ [% IF ( serial.publisheddate ) %] >+ <span class="pubdate">([% serial.publisheddate | $KohaDates %])</span> > [% END %] > </td> > [% END %] >@@ -1363,7 +1364,12 @@ > [% IF ( itemdata_copynumber ) %]<td class="copynumber">[% ITEM_RESULT.copynumber | html %]</td>[% END %] > <td class="status">[% INCLUDE 'item-status-schema-org.inc' item = ITEM_RESULT %][% INCLUDE 'item-status.inc' item = ITEM_RESULT %]</td> > [% IF ( itemdata_itemnotes ) %]<td class="notes" property="description">[% ITEM_RESULT.itemnotes | $raw %]</td>[% END %] >- <td class="date_due" data-order="[% ITEM_RESULT.datedue | html %]">[% ITEM_RESULT.datedue | $KohaDates as_due_date => 1 %]</td> >+ [% IF ITEM_RESULT.checkout %] >+ <td class="date_due" data-order="[% ITEM_RESULT.checkout.date_due | html %]">[% ITEM_RESULT.checkout.date_due | $KohaDates as_due_date => 1 %]</td> >+ [% ELSE %] >+ <td class="date_due"></td> >+ [% END %] >+ > <td class="barcode" property="serialNumber">[% ITEM_RESULT.barcode | html %]</td> > [% IF holds_count.defined || show_priority %] > <td class="holds_count"> >diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl >index 06d7044942..c883c4d035 100755 >--- a/opac/opac-detail.pl >+++ b/opac/opac-detail.pl >@@ -45,7 +45,6 @@ use C4::Biblio qw( > GetMarcSubjects > GetMarcUrls > ); >-use C4::Items qw( GetItemsInfo ); > use C4::Circulation qw( GetTransfers ); > use C4::Tags qw( get_tags ); > use C4::XISBN qw( get_xisbns ); >@@ -82,6 +81,7 @@ use Koha::Plugins; > use Koha::Ratings; > use Koha::Recalls; > use Koha::Reviews; >+use Koha::Serial::Items; > use Koha::SearchEngine::Search; > use Koha::SearchEngine::QueryBuilder; > use Koha::Util::MARC; >@@ -105,12 +105,6 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( > } > ); > >-my @all_items = GetItemsInfo($biblionumber); >-if( $specific_item ) { >- @all_items = grep { $_->{itemnumber} == $query->param('itemnumber') } @all_items; >- $template->param( specific_item => 1 ); >-} >-my @items_to_show; > my $patron = Koha::Patrons->find( $borrowernumber ); > > my $biblio = Koha::Biblios->find( $biblionumber ); >@@ -120,6 +114,12 @@ unless ( $biblio && $record ) { > exit; > } > >+my $items = $biblio->items->search_ordered; >+if ($specific_item) { >+ $items = $items->search( { itemnumber => scalar $query->param('itemnumber') } ); >+ $template->param( specific_item => 1 ); >+} >+ > unless ( $patron and $patron->category->override_hidden_items ) { > # only skip this check if there's a logged in user > # and its category overrides OpacHiddenItems >@@ -127,9 +127,8 @@ unless ( $patron and $patron->category->override_hidden_items ) { > print $query->redirect('/cgi-bin/koha/errors/404.pl'); # escape early > exit; > } >- if ( scalar @all_items >= 1 ) { >- @items_to_show = Koha::Items->search( { itemnumber => [ map { $_->{itemnumber} } @all_items ] } ) >- ->filter_by_visible_in_opac( { patron => $patron } )->as_list; >+ if ( $items->count >= 1 ) { >+ $items = $items->filter_by_visible_in_opac( { patron => $patron } ); > } > } > >@@ -495,44 +494,18 @@ $template->param( > OPACShowCheckoutName => C4::Context->preference("OPACShowCheckoutName"), > ); > >-if ( C4::Context->preference('EasyAnalyticalRecords') ) { >- # adding items linked via host biblios >- my $analyticfield = '773'; >- if ($marcflavour eq 'MARC21'){ >- $analyticfield = '773'; >- } elsif ($marcflavour eq 'UNIMARC') { >- $analyticfield = '461'; >- } >- foreach my $hostfield ( $record->field($analyticfield)) { >- my $hostbiblionumber = $hostfield->subfield("0"); >- my $linkeditemnumber = $hostfield->subfield("9"); >- my @hostitemInfos = GetItemsInfo($hostbiblionumber); >- foreach my $hostitemInfo (@hostitemInfos){ >- if ($hostitemInfo->{itemnumber} eq $linkeditemnumber){ >- push(@all_items, $hostitemInfo); >- } >- } >- } >-} >+my $host_items = $biblio->host_items->filter_by_visible_in_opac({ patron => $patron }); > >-my @items; >- >-# Are there items to hide? >-# Hide items >-if ( @items_to_show != @all_items ) { >- for my $itm (@all_items) { >- next unless any { $itm->{itemnumber} eq $_ } @items_to_show; >- if ( C4::Context->preference('hidelostitems') ) { >- push @items, $itm unless $itm->{itemlost}; >- } >- else { >- push @items, $itm; >+$items = $biblio->items->search_ordered( >+ { >+ 'me.itemnumber' => { >+ -in => [ >+ $items->get_column('itemnumber'), >+ $host_items->get_column('itemnumber') >+ ] > } > } >-} else { >- # Or not >- @items = @all_items; >-} >+); > > my $dat = &GetBiblioData($biblionumber); > my $HideMARC = $record_processor->filters->[0]->should_hide_marc( >@@ -591,8 +564,7 @@ foreach my $subscription (@subscriptions) { > push @subs, \%cell; > } > >-$dat->{'count'} = scalar(@items); >- >+$dat->{'count'} = $items->count; > > my (%item_reserves, %priority); > my ($show_holds_count, $show_priority); >@@ -703,85 +675,123 @@ if ( C4::Context->preference('OPACAcquisitionDetails' ) ) { > > my $allow_onshelf_holds; > my ( $itemloop_has_images, $otheritemloop_has_images ); >-if ( not $viewallitems and @items > $max_items_to_display ) { >+if ( not $viewallitems and $items->count > $max_items_to_display ) { > $template->param( > too_many_items => 1, >- items_count => scalar( @items ), >+ items_count => $items->count, > ); >-} else { >- for my $itm (@items) { >- my $item = Koha::Items->find( $itm->{itemnumber} ); >- $itm->{holds_count} = $item_reserves{ $itm->{itemnumber} }; >- $itm->{priority} = $priority{ $itm->{itemnumber} }; >- >- $allow_onshelf_holds = Koha::CirculationRules->get_onshelfholds_policy( { item => $item, patron => $patron } ) >- unless $allow_onshelf_holds; >- >- # get collection code description, too >- my $ccode = $itm->{'ccode'}; >- $itm->{'ccode'} = $collections->{$ccode} if defined($ccode) && $collections && exists( $collections->{$ccode} ); >- my $copynumber = $itm->{'copynumber'}; >- $itm->{'copynumber'} = $copynumbers->{$copynumber} if ( defined($copynumbers) && defined($copynumber) && exists( $copynumbers->{$copynumber} ) ); >- if ( defined $itm->{'location'} ) { >- $itm->{'location_description'} = $shelflocations->{ $itm->{'location'} }; >- } >- if (exists $itm->{itype} && defined($itm->{itype}) && exists $itemtypes->{ $itm->{itype} }) { >- $itm->{'imageurl'} = getitemtypeimagelocation( 'opac', $itemtypes->{ $itm->{itype} }->{'imageurl'} ); >- $itm->{'description'} = $itemtypes->{ $itm->{itype} }->{translated_description}; >- } >- foreach (qw(ccode materials enumchron copynumber itemnotes location_description uri)) { >- $itemfields{$_} = 1 if ($itm->{$_}); >- } >+} >+else { >+ while ( my $item = $items->next ) { >+ my $item_info = $item->unblessed; >+ $item->{holds_count} = $item_reserves{ $item->itemnumber }; >+ $item->{priority} = $priority{ $item->itemnumber }; >+ >+ $allow_onshelf_holds = Koha::CirculationRules->get_onshelfholds_policy( >+ { item => $item, patron => $patron } ) >+ unless $allow_onshelf_holds; >+ >+ # get collection code description, too >+ my $ccode = $item->ccode; >+ $item_info->{'ccode'} = $collections->{$ccode} >+ if defined($ccode) >+ && $collections >+ && exists( $collections->{$ccode} ); >+ >+ my $copynumber = $item->copynumber; >+ $item_info->{copynumber} = $copynumbers->{$copynumber} >+ if ( defined($copynumbers) >+ && defined($copynumber) >+ && exists( $copynumbers->{$copynumber} ) ); >+ >+ if ( defined $item->location ) { >+ $item_info->{'location_description'} = >+ $shelflocations->{ $item->location }; >+ } > >- my $reserve_status = C4::Reserves::GetReserveStatus($itm->{itemnumber}); >- if ( $reserve_status eq "Waiting" ) { $itm->{'waiting'} = 1; } >- if ( $reserve_status eq "Reserved" ) { $itm->{'onhold'} = 1; } >+ my $itemtype = $item->itemtype; >+ $item_info->{'imageurl'} = getitemtypeimagelocation( 'opac', >+ $itemtypes->{ $itemtype->itemtype }->{'imageurl'} ); >+ $item_info->{'description'} = >+ $itemtypes->{ $itemtype->itemtype }->{translated_description}; > >- if ( C4::Context->preference('UseRecalls') ) { >- my $pending_recall_count = Koha::Recalls->search( >- { >- item_id => $itm->{itemnumber}, >- status => 'waiting', >- } >- )->count; >- if ( $pending_recall_count ) { $itm->{has_pending_recall} = 1; } >- } >+ foreach my $field ( >+ qw(ccode materials enumchron copynumber itemnotes location_description uri) >+ ) >+ { >+ $itemfields{$field} = 1 if $item_info->{$field}; >+ } > >- my ( $transfertwhen, $transfertfrom, $transfertto ) = GetTransfers($itm->{itemnumber}); >- if ( defined( $transfertwhen ) && $transfertwhen ne '' ) { >- $itm->{transfertwhen} = $transfertwhen; >- $itm->{transfertfrom} = $transfertfrom; >- $itm->{transfertto} = $transfertto; >- } >+ # FIXME The following must be Koha::Item->serial >+ my $serial_item = Koha::Serial::Items->find($item->itemnumber); >+ if ( $serial_item ) { >+ $item_info->{serial} = $serial_item; >+ } > >- if ( C4::Context->preference('OPACAcquisitionDetails') ) { >- $itm->{on_order} = 1 >- if grep { $_ eq $itm->{itemnumber} } @itemnumbers_on_order; >- } >+ $item_info->{checkout} = $item->checkout; > >- if ( C4::Context->preference("OPACLocalCoverImages") == 1 ) { >- $itm->{cover_images} = $item->cover_images; >- } >+ my $reserve_status = >+ C4::Reserves::GetReserveStatus( $item->itemnumber ); >+ if ( $reserve_status eq "Waiting" ) { $item_info->{'waiting'} = 1; } >+ if ( $reserve_status eq "Reserved" ) { $item_info->{'onhold'} = 1; } > >- if ( $item->in_bundle ) { >- my $host = $item->bundle_host; >- $itm->{bundle_host} = $host; >- } >+ if ( C4::Context->preference('UseRecalls') ) { >+ my $pending_recall_count = Koha::Recalls->search( >+ { >+ item_id => $item->itemnumber, >+ status => 'waiting', >+ } >+ )->count; >+ if ( $pending_recall_count ) { $item_info->has_pending_recall = 1; } >+ } >+ >+ my ( $transfertwhen, $transfertfrom, $transfertto ) = >+ GetTransfers( $item->itemnumber ); >+ if ( defined($transfertwhen) && $transfertwhen ne '' ) { >+ $item_info->{transfertwhen} = $transfertwhen; >+ $item_info->{transfertfrom} = $transfertfrom; >+ $item_info->{transfertto} = $transfertto; >+ } >+ >+ if ( C4::Context->preference('OPACAcquisitionDetails') ) { >+ $item_info->{on_order} = 1 >+ if grep { $_ eq $item->itemnumber } @itemnumbers_on_order; >+ } >+ >+ if ( C4::Context->preference("OPACLocalCoverImages") == 1 ) { >+ $item_info->{cover_images} = $item->cover_images; >+ } >+ >+ >+ if ( $item->in_bundle ) { >+ $item_info->{bundle_host} = $item->bundle_host; >+ } >+ >+ if ( C4::Context->preference('UseCourseReserves') ) { >+ $item_info->{course_reserves} = GetItemCourseReservesInfo( itemnumber => $item->itemnumber ); >+ } >+ >+ $item_info->{holding_branch} = $item->holding_branch; >+ $item_info->{home_branch} = $item->home_branch; > >- my $itembranch = $itm->{$separatebranch}; >- if ($currentbranch and C4::Context->preference('OpacSeparateHoldings')) { >- if ($itembranch and $itembranch eq $currentbranch) { >- push @itemloop, $itm; >+ my $itembranch = $item->$separatebranch; >+ if ( $currentbranch >+ and C4::Context->preference('OpacSeparateHoldings') ) >+ { >+ if ( $itembranch and $itembranch eq $currentbranch ) { >+ push @itemloop, $item_info; >+ $itemloop_has_images++ if $item->cover_images->count; >+ } >+ else { >+ push @otheritemloop, $item_info; >+ $otheritemloop_has_images++ if $item->cover_images->count; >+ } >+ } >+ else { >+ push @itemloop, $item_info; > $itemloop_has_images++ if $item->cover_images->count; >- } else { >- push @otheritemloop, $itm; >- $otheritemloop_has_images++ if $item->cover_images->count; > } >- } else { >- push @itemloop, $itm; >- $itemloop_has_images++ if $item->cover_images->count; > } >- } > } > > if( $allow_onshelf_holds || CountItemsIssued($biblionumber) || $biblio->has_items_waiting_or_intransit ) { >@@ -828,7 +838,7 @@ my $norequests = ! $biblio->items->filter_by_for_hold->count; > OpacStarRatings => C4::Context->preference("OpacStarRatings"), > ); > >-if (C4::Context->preference("AlternateHoldingsField") && scalar @items == 0) { >+if (C4::Context->preference("AlternateHoldingsField") && $items->count == 0) { > my $fieldspec = C4::Context->preference("AlternateHoldingsField"); > my $subfields = substr $fieldspec, 3; > my $holdingsep = C4::Context->preference("AlternateHoldingsSeparator") || ' '; >@@ -1115,7 +1125,7 @@ if (C4::Context->preference("OPACShelfBrowser")) { > ); > > # in which tab shelf browser should open ? >- if (grep { $starting_itemnumber == $_->{itemnumber} } @itemloop) { >+ if (grep { $starting_itemnumber == $_->itemnumber } @itemloop) { > $template->param(shelfbrowser_tab => 'holdings'); > } else { > $template->param(shelfbrowser_tab => 'otherholdings'); >@@ -1245,12 +1255,6 @@ if (C4::Context->preference('OpacHighlightedWords')) { > } > $template->{VARS}->{'trackclicks'} = C4::Context->preference('TrackClicks'); > >-if ( C4::Context->preference('UseCourseReserves') ) { >- foreach my $i ( @items ) { >- $i->{'course_reserves'} = GetItemCourseReservesInfo( itemnumber => $i->{'itemnumber'} ); >- } >-} >- > $template->param( > 'OpacLocationBranchToDisplay' => C4::Context->preference('OpacLocationBranchToDisplay'), > ); >-- >2.30.2
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 31313
:
138821
|
138822
|
138847
|
138848
| 139119 |
139120
|
139121
|
139122
|
139193
|
139239
|
139693
|
139694
|
139695
|
139761
|
139762
|
139763