Attachment 138913 Details for Bug 31321 – Bug 31321: Remove GetItemsInfo from catalogue/detail.pl

[patch] Bug 31321: Remove GetItemsInfo from catalogue/detail.pl

Bug-31321-Remove-GetItemsInfo-from-cataloguedetail.patch (text/plain), 21.59 KB, created by Owen Leonard on 2022-08-09 13:30:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2022-08-09 13:30:24 UTC

Size: 21.59 KB

patch

obsolete

>From 0efe587307e5a061169f713f72f9451d9cf96df3 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Fri, 3 Dec 2021 16:43:06 +0100
>Subject: [PATCH] Bug 31321: Remove GetItemsInfo from catalogue/detail.pl
>
>Bug 27272 is going to remove C4::Items::GetItemsInfo in favour of Koha::Items->search_ordered.
>
>Here we are going to deal with catalogue/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>
>---
> catalogue/detail.pl | 160 +++++++++---------
> .../prog/en/modules/catalogue/detail.tt | 58 ++++---
> 2 files changed, 112 insertions(+), 106 deletions(-)
>
>diff --git a/catalogue/detail.pl b/catalogue/detail.pl
>index 29d816aa93..3afa521ac7 100755
>--- a/catalogue/detail.pl
>+++ b/catalogue/detail.pl
>@@ -33,7 +33,7 @@ use C4::Koha qw(
> use C4::Serials qw( CountSubscriptionFromBiblionumber SearchSubscriptions GetLatestSerials );
> use C4::Output qw( output_html_with_http_headers );
> use C4::Biblio qw( GetBiblioData GetFrameworkCode );
>-use C4::Items qw( GetAnalyticsCount GetHostItemsInfo GetItemsInfo );
>+use C4::Items qw( GetAnalyticsCount );
> use C4::Circulation qw( GetTransfers );
> use C4::Reserves;
> use C4::Serials qw( CountSubscriptionFromBiblionumber SearchSubscriptions GetLatestSerials );
>@@ -60,6 +60,7 @@ use Koha::Plugins;
> use Koha::Recalls;
> use Koha::SearchEngine::Search;
> use Koha::SearchEngine::QueryBuilder;
>+use Koha::Serial::Items;
> 
> my $query = CGI->new();
> 
>@@ -189,24 +190,24 @@ $template->param(
> content_identifier_exists => $content_identifier_exists,
> );
> 
>-my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search->unblessed } };
>-
>-my $dbh = C4::Context->dbh;
>-
>-my @all_items = GetItemsInfo( $biblionumber );
>+my $itemtypes = { map { $_->itemtype => $_ } @{ Koha::ItemTypes->search_with_localization->as_list } };
>+my $all_items = $biblio->items->search_ordered;
> my @items;
> my $patron = Koha::Patrons->find( $borrowernumber );
>-for my $itm (@all_items) {
>- push @items, $itm unless ( $itm->{itemlost} && $patron->category->hidelostitems && !$showallitems);
>+while ( my $item = $all_items->next ) {
>+ push @items, $item
>+ unless $item->itemlost
>+ && $patron->category->hidelostitems
>+ && !$showallitems;
> }
> 
> # flag indicating existence of at least one item linked via a host record
> my $hostrecords;
> # adding items linked via host biblios
>-my @hostitems = GetHostItemsInfo($marc_record);
>-if (@hostitems){
>- $hostrecords =1;
>- push (@items,@hostitems);
>+my $hostitems = $biblio->host_items;
>+if ( $hostitems->count ) {
>+ $hostrecords = 1;
>+ push @items, $hostitems->as_list;
> }
> 
> my $dat = &GetBiblioData($biblionumber);
>@@ -317,12 +318,12 @@ if ( C4::Context->preference('suggestion') ) {
> }
> 
> if ( defined $dat->{'itemtype'} ) {
>- $dat->{imageurl} = getitemtypeimagelocation( 'intranet', $itemtypes->{ $dat->{itemtype} }{imageurl} );
>+ $dat->{imageurl} = getitemtypeimagelocation( 'intranet', $itemtypes->{ $dat->{itemtype} }->imageurl );
> }
> 
>-$dat->{'count'} = scalar @all_items + @hostitems;
>-$dat->{'showncount'} = scalar @items + @hostitems;
>-$dat->{'hiddencount'} = scalar @all_items + @hostitems - scalar @items;
>+$dat->{'count'} = $all_items->count + $hostitems->count;
>+$dat->{'showncount'} = scalar @items + $hostitems->count;
>+$dat->{'hiddencount'} = $all_items->count + $hostitems->count - scalar @items;
> 
> my $shelflocations =
> { map { $_->{authorised_value} => $_->{lib} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => $fw, kohafield => 'items.location' } ) };
>@@ -364,61 +365,65 @@ if ($currentbranch and C4::Context->preference('SeparateHoldings')) {
> }
> my $separatebranch = C4::Context->preference('SeparateHoldingsBranch') || 'homebranch';
> my ( $itemloop_has_images, $otheritemloop_has_images );
>-foreach my $item (@items) {
>- my $itembranchcode = $item->{$separatebranch};
> 
>- $item->{imageurl} = defined $item->{itype} ? getitemtypeimagelocation('intranet', $itemtypes->{ $item->{itype} }{imageurl})
>- : '';
>+foreach my $item (@items) {
>+ my $itembranchcode = $item->$separatebranch;
> 
>- $item->{datedue} = format_sqldatetime($item->{datedue});
>+ my $item_info = $item->unblessed;
>+ $item_info->{itemtype} = $itemtypes->{$item->effective_itemtype};
> 
> #get shelf location and collection code description if they are authorised value.
> # same thing for copy number
>- my $shelfcode = $item->{'location'};
>- $item->{'location'} = $shelflocations->{$shelfcode} if ( defined( $shelfcode ) && defined($shelflocations) && exists( $shelflocations->{$shelfcode} ) );
>- my $ccode = $item->{'ccode'};
>- $item->{'ccode'} = $collections->{$ccode} if ( defined( $ccode ) && defined($collections) && exists( $collections->{$ccode} ) );
>+ my $shelfcode = $item->location;
>+ $item_info->{'location'} = $shelflocations->{$shelfcode} if ( defined( $shelfcode ) && defined($shelflocations) && exists( $shelflocations->{$shelfcode} ) );
>+ my $ccode = $item->ccode;
>+ $item_info->{'ccode'} = $collections->{$ccode} if ( defined( $ccode ) && defined($collections) && exists( $collections->{$ccode} ) );
> my $copynumber = $item->{'copynumber'};
>- $item->{'copynumber'} = $copynumbers->{$copynumber} if ( defined($copynumber) && defined($copynumbers) && exists( $copynumbers->{$copynumber} ) );
>- foreach (qw(ccode enumchron copynumber stocknumber itemnotes itemnotes_nonpublic uri publisheddate)) { # Warning when removing GetItemsInfo - publisheddate (at least) is not part of the items table
>- $itemfields{$_} = 1 if ( $item->{$_} );
>+ $item_info->{'copynumber'} = $copynumbers->{$copynumber} if ( defined($copynumber) && defined($copynumbers) && exists( $copynumbers->{$copynumber} ) );
>+ foreach (qw(ccode enumchron copynumber stocknumber itemnotes itemnotes_nonpublic uri )) {
>+ $itemfields{$_} = 1 if $item->$_;
>+ }
>+
>+ # 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;
>+ $itemfields{publisheddate} = 1;
> }
> 
>+ $item_info->{object} = $item;
>+
> # checking for holds
>- my $item_object = Koha::Items->find( $item->{itemnumber} );
>- $item->{object} = $item_object;
>- my $holds = $item_object->current_holds;
>+ my $holds = $item->current_holds;
> if ( my $first_hold = $holds->next ) {
>- $item->{first_hold} = $first_hold;
>+ $item_info->{first_hold} = $first_hold;
> }
> 
>- if ( my $checkout = $item_object->checkout ) {
>- $item->{CheckedOutFor} = $checkout->patron;
>- }
>+ $item_info->{checkout} = $item->checkout;
> 
> # Check the transit status
>- my ( $transfertwhen, $transfertfrom, $transfertto ) = GetTransfers($item->{itemnumber});
>- if ( defined( $transfertwhen ) && ( $transfertwhen ne '' ) ) {
>- $item->{transfertwhen} = $transfertwhen;
>- $item->{transfertfrom} = $transfertfrom;
>- $item->{transfertto} = $transfertto;
>- $item->{nocancel} = 1;
>+ my $transfer = $item->get_transfer;
>+ if ( $transfer ) {
>+ $item_info->{transfertwhen} = $transfer->datesent;
>+ $item_info->{transfertfrom} = $transfer->frombranch;
>+ $item_info->{transfertto} = $transfer->tobranch;
>+ $item_info->{nocancel} = 1;
> }
> 
> foreach my $f (qw( itemnotes )) {
>- if ($item->{$f}) {
>- $item->{$f} =~ s|\n| |g;
>+ if ($item_info->{$f}) {
>+ $item_info->{$f} =~ s|\n| |g;
> $itemfields{$f} = 1;
> }
> }
> 
> #item has a host number if its biblio number does not match the current bib
> 
>- if ($item->{biblionumber} ne $biblionumber){
>- $item->{hostbiblionumber} = $item->{biblionumber};
>- $item->{hosttitle} = GetBiblioData($item->{biblionumber})->{title};
>+ if ($item->biblionumber ne $biblionumber){
>+ $item_info->{hostbiblionumber} = $item->biblionumber;
>+ $item_info->{hosttitle} = $item->biblio->title;
> }
>-	
>+
> 
> if ( $analyze ) {
> # count if item is used in analytical bibliorecords
>@@ -426,65 +431,62 @@ foreach my $item (@items) {
> my $countanalytics = GetAnalyticsCount( $item->{itemnumber} );
> if ($countanalytics > 0){
> $analytics_flag=1;
>- $item->{countanalytics} = $countanalytics;
>+ $item_info->{countanalytics} = $countanalytics;
> }
> }
> 
>- if (defined($item->{'materials'}) && $item->{'materials'} =~ /\S/){
>+ if (defined($item->materials) && $item->materials =~ /\S/){
> $materials_flag = 1;
>- if (defined $materials_map{ $item->{materials} }) {
>- $item->{materials} = $materials_map{ $item->{materials} };
>+ if (defined $materials_map{ $item->materials }) {
>+ $item_info->{materials} = $materials_map{ $item->materials };
> }
> }
> 
> if ( C4::Context->preference('UseCourseReserves') ) {
>- $item->{'course_reserves'} = GetItemCourseReservesInfo( itemnumber => $item->{'itemnumber'} );
>- }
>-
>- if ( C4::Context->preference('IndependentBranches') ) {
>- my $userenv = C4::Context->userenv();
>- if ( not C4::Context->IsSuperLibrarian()
>- and $userenv->{branch} ne $item->{homebranch} ) {
>- $item->{cannot_be_edited} = 1;
>- }
>+ $item_info->{'course_reserves'} = GetItemCourseReservesInfo( itemnumber => $item->itemnumber );
> }
> 
> if ( C4::Context->preference("LocalCoverImages") == 1 ) {
>- $item->{cover_images} = $item_object->cover_images;
>+ $item_info->{cover_images} = $item->cover_images;
> }
> 
> if ( C4::Context->preference('UseRecalls') ) {
>- my $recall = Koha::Recalls->find({ item_id => $item->{itemnumber}, completed => 0 });
>- if ( defined $recall ) {
>- $item->{recalled} = 1;
>- $item->{recall} = $recall;
>+ $item_info->{recall} = $item->recall;
>+ }
>+
>+ if ( C4::Context->preference('IndependentBranches') ) {
>+ my $userenv = C4::Context->userenv();
>+ if ( not C4::Context->IsSuperLibrarian()
>+ and $userenv->{branch} ne $item->homebranch ) {
>+ $item_info->{cannot_be_edited} = 1;
>+ $item_info->{not_same_branch} = 1;
> }
> }
> 
>- if ( $item_object->is_bundle ) {
>- $item->{bundled} =
>- $item_object->bundle_items->search( { itemlost => { '!=' => 0 } } )
>+ if ( $item->is_bundle ) {
>+ $item_info->{bundled} =
>+ $item->bundle_items->search( { itemlost => { '!=' => 0 } } )
> ->count;
>- $item->{bundled_lost} =
>- $item_object->bundle_items->search( { itemlost => 0 } )->count;
>- $item->{is_bundle} = 1;
>+ $item_info->{bundled_lost} =
>+ $item->bundle_items->search( { itemlost => 0 } )->count;
>+ $item_info->{is_bundle} = 1;
> }
> 
>- if ($item_object->in_bundle) {
>- $item->{bundle_host} = $item_object->bundle_host;
>+ if ($item->in_bundle) {
>+ $item_info->{bundle_host} = $item->bundle_host;
> }
> 
> if ($currentbranch and C4::Context->preference('SeparateHoldings')) {
> if ($itembranchcode and $itembranchcode eq $currentbranch) {
>- push @itemloop, $item;
>- $itemloop_has_images++ if $item_object->cover_images->count;
>+ push @itemloop, $item_info;
>+ $itemloop_has_images++ if $item->cover_images->count;
> } else {
>- push @otheritemloop, $item;
>- $otheritemloop_has_images++ if $item_object->cover_images->count;
>+ push @otheritemloop, $item_info;
>+ $otheritemloop_has_images++ if $item->cover_images->count;
> }
> } else {
>- push @itemloop, $item;
>- $itemloop_has_images++ if $item_object->cover_images->count;
>+ push @itemloop, $item_info;
>+ $itemloop_has_images++ if $item->cover_images->count;
> }
> }
> 
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
>index 0fbbb9442b..f6951219d4 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
>@@ -397,13 +397,14 @@
> 
> [% IF ( item_level_itypes ) %]
> <td class="itype">
>- [% IF !noItemTypeImages && item.imageurl %]
>- <img src="[% item.imageurl | html %]" alt="[% item.translated_description | html %]" title="[% item.translated_description | html %]" />
>+ [% SET itemtype = item.itemtype %]
>+ [% IF !noItemTypeImages && itemtype.image_location %]
>+ <img src="[% itemtype.image_location | html %]" alt="[% itemtype.translated_description | html %]" title="[% itemtype.translated_description | html %]" />
> [% END %]
>- [% item.translated_description | html %]
>+ [% itemtype.translated_description | html %]
> </td>
> [% END %]
>- <td class="location">[% UNLESS ( singlebranchmode ) %][% Branches.GetName( item.branchcode ) | html %] [% END %]</td>
>+ <td class="location">[% UNLESS ( singlebranchmode ) %][% Branches.GetName( item.holdingbranch ) | html %] [% END %]</td>
> <td class="homebranch">
> [% Branches.GetName(item.homebranch) | html %]
> 
>@@ -426,25 +427,26 @@ Note that permanent location is a code, and location may be an authval.
> [% IF Koha.Preference('EnableItemGroups') %]<td class="item_group">[% item.object.item_group.description | html %]</td>[% END %]
> <td class="itemcallnumber">[% IF ( item.itemcallnumber ) %] [% item.itemcallnumber | html %][% END %]</td>
> [% IF ( volinfo ) %]
>+ [% SET serial = item.itemserial.serial %]
> [% IF itemdata_publisheddate #If there is at least one published date, use it for sorting %]
>- <td class="enumchron" data-order="[% item.publisheddate | html %]">
>+ <td class="enumchron" data-order="[% serial.publisheddate | html %]">
> [% ELSE %]
> <td class="enumchron">
> [% END %]
> [% IF ( itemdata_enumchron ) %]
>- [% IF item.enumchron && item.serialseq %]
>+ [% IF item.enumchron && serial.serialseq %]
> [% item.enumchron | html %]
>- [% IF ( item.serialseq && item.enumchron!=item.serialseq ) %]
>+ [% IF ( item.serialseq && item.enumchron != serial.serialseq ) %]
> -- 
>- [% item.serialseq | html %]
>+ [% serial.serialseq | html %]
> [% END %]
> [% ELSIF item.enumchron %]
> [% item.enumchron | html %]
> [% ELSIF item.serialseq %]
>- [% item.serialseq | html %]
>+ [% serial.serialseq | html %]
> [% END %]
>- [% IF ( item.publisheddate ) %]
>- ([% item.publisheddate | $KohaDates %])
>+ [% IF serial.publisheddate %]
>+ ([% serial.publisheddate | $KohaDates %])
> [% END %]
> [% END %]
> 
>@@ -452,21 +454,21 @@ Note that permanent location is a code, and location may be an authval.
> [% END %]
> <td class="status">
> 
>- [% IF item.CheckedOutFor %]
>- [% IF item.onsite_checkout %]
>+ [% IF item.checkout %]
>+ [% IF item.checkout.onsite_checkout %]
> Currently in local use
> [% ELSE %]
> Checked out
> [% END %]
>- [% UNLESS ( item.NOTSAMEBRANCH ) %]
>- [% IF item.onsite_checkout %]
>+ [% UNLESS ( item.not_same_branch) %]
>+ [% IF item.checkout.onsite_checkout %]
> by
> [% ELSE %]
> to
> [% END %]
>- [% INCLUDE 'patron-title.inc' patron=item.CheckedOutFor hide_patron_infos_if_needed=1 %]
>+ [% INCLUDE 'patron-title.inc' patron=item.checkout.patron hide_patron_infos_if_needed=1 %]
> [% END %]
>- : due [% item.datedue | html %]
>+ : due [% item.checkout.date_due | $KohaDates as_due_date => 1 %]
> 
> [% ELSIF ( item.transfertwhen ) %]
> In transit from [% Branches.GetName( item.transfertfrom ) | html %] to [% Branches.GetName( item.transfertto ) | html %] since [% item.transfertwhen | $KohaDates %]
>@@ -508,10 +510,11 @@ Note that permanent location is a code, and location may be an authval.
> [% END %]
> [% END %]
> 
>- [% IF ( item.itemnotforloan || item.notforloan_per_itemtype ) %]
>+ [% IF ( item.notforloan || item.itemtype.notforloan ) %]
> Not for loan
>- [% IF ( item.notforloanvalue ) %]
>- ([% item.notforloanvalue | html %])
>+ [% SET not_for_loan_description = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.notforloan', authorised_value => item.notforloan ) %]
>+ [% IF not_for_loan_description %]
>+ ([% not_for_loan_description | html %])
> [% END %]
> 
> [% END %]
>@@ -529,21 +532,22 @@ Note that permanent location is a code, and location may be an authval.
> [% END %]
> [% END %]
> 
>- [% IF item.recalled %]
>- [% IF item.recall.waiting_date %]
>- Waiting at [% Branches.GetName( item.recall.pickup_library_id ) | html %] since [% item.recall.waiting_date | $KohaDates %]
>+ [% SET recall = item.recall %]
>+ [% IF recall %]
>+ [% IF recall.waiting_date %]
>+ Waiting at [% Branches.GetName( recall.pickup_library_id ) | html %] since [% recall.waiting_date | $KohaDates %]
> [% ELSE %]
>- [% patron_link = BLOCK %]<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% item.recall.patron_id | uri %]">[% item.recall.patron.firstname | html %] [% item.recall.patron.surname | html %] ([% item.recall.patron.cardnumber | html %])</a>[% END %]
>- Item recalled by [% patron_link| $raw %] on [% item.recall.created_date | $KohaDates %]
>+ [% patron_link = BLOCK %]<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% recall.patron_id | uri %]">[% recall.patron.firstname | html %] [% recall.patron.surname | html %] ([% recall.patron.cardnumber | html %])</a>[% END %]
>+ recalled by [% patron_link| $raw %] on [% recall.created_date | $KohaDates %]
> [% END %]
> [% END %]
> 
>- [% UNLESS ( item.itemnotforloan || item.notforloan_per_itemtype || item.onloan || item.itemlost || item.withdrawn || item.damaged || item.transfertwhen || hold || item.recalled ) %]
>+ [% UNLESS ( item.itemnotforloan || item.notforloan_per_itemtype || item.onloan || item.itemlost || item.withdrawn || item.damaged || item.transfertwhen || hold || ( Koha.Preference('UseRecalls') && recall ) ) %]
> Available
> [% END %]
> 
> [% IF ( item.restricted ) %]
>- ([% item.restrictedvalue | html %])
>+ ([% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.restricted', authorised_value => item.restricted ) | html %])
> [% END %]
> 
> [% IF ( item.bundle_host ) %]
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 31321: 138831 | 138913 | 139115 | 139116 | 139523