Attachment 137972 Details for Bug 27272 – Bug 27272: Remove GetItemsInfo from opac-detail

[patch] Bug 27272: Remove GetItemsInfo from opac-detail

Bug-27272-Remove-GetItemsInfo-from-opac-detail.patch (text/plain), 20.31 KB, created by Jonathan Druart on 2022-07-21 14:04:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2022-07-21 14:04:28 UTC

Size: 20.31 KB

patch

obsolete

>From 56f24a6ff4de8022bb6e34f2aec37bd070ad29b5 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 27272: Remove GetItemsInfo from opac-detail
>
>---
> .../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 7c30dafda96..673bc22c0de 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
>@@ -1226,16 +1226,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 %]">
>- [% 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 %]">
>+ [% 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 %]
>- [% ITEM_RESULT.branchname | html %]
>+ [% ITEM_RESULT.holding_branch.branchname | html %]
> [% END %]
> 
> [% IF ( Koha.Preference('OpacLocationOnDetail') == 'holding' || Koha.Preference('OpacLocationOnDetail') == 'both' ) %]
>@@ -1250,16 +1250,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 %]">
>- [% Branches.GetName( ITEM_RESULT.homebranch ) | html %]
>+ [% IF ( ITEM_RESULT.holding_branch.opac_info ) %] [%# FIXME Shouldn't this be home_branch instead of holding_branc? %]
>+ <a href="[% ITEM_RESULT.home_branch.branchurl | url %]" class="library_info" data-branchcode="[% ITEM_RESULT.home_branch.branchchode | html %]">
>+ [% 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 %]
>- [% Branches.GetName( ITEM_RESULT.homebranch ) | html %]
>+ [% ITEM_RESULT.home_branch.branchname | html %]
> [% END %]
> 
> [% IF ( Koha.Preference('OpacLocationOnDetail') == 'home' || Koha.Preference('OpacLocationOnDetail') == 'both' ) %]
>@@ -1295,20 +1295,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 %]
> [% ITEM_RESULT.enumchron | html %]
>- [% IF ( ITEM_RESULT.serialseq && ITEM_RESULT.enumchron!=ITEM_RESULT.serialseq ) %]
>+ [% IF ( serial.serialseq && ITEM_RESULT.enumchron!=serial.serialseq ) %]
> -- 
>- [% ITEM_RESULT.serialseq | html %]
>+ [% serial.serialseq | html %]
> [% END %]
> [% ELSIF ITEM_RESULT.enumchron %]
> [% ITEM_RESULT.enumchron | html %]
>- [% ELSIF ITEM_RESULT.serialseq %]
>- [% ITEM_RESULT.serialseq | html %]
>+ [% ELSIF serial.serialseq %]
>+ [% serial.serialseq | html %]
> [% END %]
>- [% IF ( ITEM_RESULT.publisheddate ) %]
>- ([% ITEM_RESULT.publisheddate | $KohaDates %])
>+ [% IF ( serial.publisheddate ) %]
>+ ([% serial.publisheddate | $KohaDates %])
> [% END %]
> </td>
> [% END %]
>@@ -1343,7 +1344,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 6bf5f6dc2c5..51feb0da95a 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;
> 
>@@ -104,12 +104,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 );
>@@ -119,6 +113,12 @@ unless ( $biblio && $record ) {
> exit;
> }
> 
>+my $items = $biblio->items;
>+if ($specific_item) {
>+ $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
>@@ -126,9 +126,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 } );
> }
> }
> 
>@@ -494,44 +493,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(
>+ {
>+ 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(
>@@ -590,8 +563,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);
>@@ -702,85 +674,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 ) {
>@@ -827,7 +837,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") || ' ';
>@@ -1114,7 +1124,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');
>@@ -1242,12 +1252,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.25.1

Actions: View | Diff | Splinter Review

Attachments on bug 27272: 128230 | 128231 | 135640 | 135641 | 135642 | 135643 | 135644 | 135645 | 135646 | 135647 | 135648 | 135649 | 135650 | 135651 | 135652 | 135653 | 135654 | 135655 | 135656 | 135657 | 135658 | 135659 | 137966 | 137967 | 137968 | 137969 | 137970 | 137971 | 137972 | 137973 | 137974 | 137975 | 137976 | 137977 | 137978 | 137979 | 137980 | 137981 | 137982 | 138109 | 138296 | 138297 | 138298 | 138303 | 138304 | 138305 | 138306 | 138307 | 138308 | 138437 | 138438 | 138439 | 138440 | 138441 | 138442 | 138553 | 138554 | 138555 | 138556 | 138557 | 138558 | 138559 | 138560 | 138561 | 138562 | 138563 | 138564 | 138565 | 138566 | 138567 | 138568 | 138569 | 138570 | 138571 | 138572 | 138573 | 138574 | 138575 | 138614 | 138615 | 138616 | 138617 | 138618 | 138619 | 138620 | 138621 | 138622 | 138623 | 138624 | 138625 | 138626 | 138627 | 138628 | 138629 | 138630 | 138631 | 138632 | 138633 | 138634 | 138635 | 138636 | 138714 | 138715 | 138716 | 138717 | 138718 | 138719 | 138720 | 138721 | 138722 | 138723 | 138724 | 138725 | 138726 | 138727 | 138728 | 138729 | 138730 | 138731 | 138732 | 138733 | 138734 | 138735 | 138736 | 138773 | 138774 | 138775 | 138776 | 138777 | 138778 | 138779 | 138780 | 138781 | 138782 | 138783 | 138784 | 138785 | 138786 | 138787 | 138788 | 138789 | 138790 | 138791 | 138792 | 138793 | 138794 | 138832 | 139928