From b08f3784e44428421a47e395dcfefc36702ad87a Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 9 Mar 2023 20:37:28 +0000 Subject: [PATCH] Bug 33167: Cleanup staff detail page This patch begins reduing some of the extra things we are doing in detail.pl that could easily be handled in the templates - fetching authorised values and branches etc It also removes a loop to find items that should be hidden, and instead uses a searh parameter The template changes either use item object rather than passed variables, or utilize plugins to fetch authorised valued Template also includes addition of chomp to the template directives, this reduces the size of the items table by several megabytes on records with many items WIP --- catalogue/detail.pl | 66 +--- .../prog/en/modules/catalogue/detail.tt | 348 +++++++++--------- 2 files changed, 179 insertions(+), 235 deletions(-) diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 905120ec9d..c5d6f98227 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -190,15 +190,10 @@ $template->param( ); my $itemtypes = { map { $_->itemtype => $_ } @{ Koha::ItemTypes->search_with_localization->as_list } }; -my $all_items = $biblio->items->search_ordered; -my @items; +my $params; my $patron = Koha::Patrons->find( $borrowernumber ); -while ( my $item = $all_items->next ) { - push @items, $item - unless $item->itemlost - && $patron->category->hidelostitems - && !$showallitems; -} +$params->{ itemlost } = 0 if $patron->category->hidelostitems && !$showallitems; +my @items = $biblio->items->search_ordered( $params )->as_list; # flag indicating existence of at least one item linked via a host record my $hostrecords; @@ -210,6 +205,11 @@ if ( $hostitems->count ) { } my $dat = &GetBiblioData($biblionumber); +$dat->{'count'} = $biblio->items->count + $hostitems->count; +$dat->{'showncount'} = scalar @items + $hostitems->count; +$dat->{'hiddencount'} = $dat->{'count'} - $dat->{'showncount'}; + + #is biblio a collection and are bundles enabled my $leader = $marc_record->leader(); @@ -320,16 +320,6 @@ if ( defined $dat->{'itemtype'} ) { $dat->{imageurl} = getitemtypeimagelocation( 'intranet', $itemtypes->{ $dat->{itemtype} }->imageurl ); } -$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' } ) }; -my $collections = - { map { $_->{authorised_value} => $_->{lib} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => $fw, kohafield => 'items.ccode' } ) }; -my $copynumbers = - { map { $_->{authorised_value} => $_->{lib} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => $fw, kohafield => 'items.copynumber' } ) }; my (@itemloop, @otheritemloop, %itemfields); my $mss = Koha::MarcSubfieldStructures->search({ frameworkcode => $fw, kohafield => 'items.itemlost', authorised_value => [ -and => {'!=' => undef }, {'!=' => ''}] }); @@ -371,14 +361,6 @@ foreach my $item (@items) { 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_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_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->$_; } @@ -398,21 +380,6 @@ foreach my $item (@items) { $item_info->{first_hold} = $first_hold; } - $item_info->{checkout} = $item->checkout; - - # Check the transit status - my $transfer = $item->get_transfer; - if ( $transfer ) { - $item_info->{transfer} = $transfer; - } - - foreach my $f (qw( itemnotes )) { - 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){ @@ -444,23 +411,6 @@ foreach my $item (@items) { $item_info->{can_be_edited} = $patron->can_edit_item( $item ); - if ( C4::Context->preference("LocalCoverImages") == 1 ) { - $item_info->{cover_images} = $item->cover_images; - } - - if ( C4::Context->preference('UseRecalls') ) { - $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->is_bundle ) { $item_info->{bundled} = $item->bundle_items->search( { itemlost => { '!=' => 0 } } ) 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 707a5d7cf3..ddfcf79259 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -380,192 +380,186 @@ [% FOREACH item IN items %] - [% IF (StaffDetailItemSelection) %] + [%- IF (StaffDetailItemSelection) %] - [% IF item.can_be_edited %] + [%- IF item.can_be_edited %] - [% END %] + [%- END %] - [% END %] - [% IF Koha.Preference('LocalCoverImages') && ( tab == 'holdings' && itemloop_has_images || tab == 'otherholdings' && otheritemloop_has_images ) %] + [%- END %] + [%- IF Koha.Preference('LocalCoverImages') && ( tab == 'holdings' && itemloop_has_images || tab == 'otherholdings' && otheritemloop_has_images ) %]
- [% FOREACH image IN item.cover_images %] + [%- FOREACH image IN item.object.cover_images %]
Local cover image
- [% END %] + [%- END %]
- [% END %] + [%- END %] - [% IF ( item_level_itypes ) %] + [%- IF ( item_level_itypes ) %] - [% SET itemtype = item.itemtype %] - [% IF !noItemTypeImages && itemtype.image_location('intranet') %] + [%- SET itemtype = item.itemtype %] + [%- IF !noItemTypeImages && itemtype.image_location('intranet') %] [% itemtype.translated_description | html %] - [% END %] + [%- END %] [% itemtype.translated_description | html %] - [% END %] + [%- END %] [% UNLESS ( singlebranchmode ) %][% Branches.GetName( item.holdingbranch ) | html %] [% END %] [% Branches.GetName(item.homebranch) | html %] - - [% IF item.permanent_location %] - [% SET permloc_authval = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.permanent_location ) %] - [% permloc_authval | html %] - [% IF item.location AND item.location != permloc_authval AND item.location != item.permanent_location %] - ([% item.location | html %]) - [% END %] - [% ELSE %] - [% item.location | html %] - [% END %] + [%# If permanent location is defined, show description or code and + display current location in parentheses. If not, display current location. + Note that permanent location is a code, and location may be an authval. + %] + [%- IF item.permanent_location %] + [%- SET permloc_authval = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.permanent_location ) %] + [%- permloc_authval | html %] + [%- SET item_locatoin = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.location', authorised_value => item.location ) %] + [%- IF item_location AND item_location != permloc_authval AND item.location != item.permanent_location %] + ([% item_location | html %]) + [%- END %] + [%- ELSE %] + [%- item_location | html %] + [%- END %] - [% IF ( itemdata_ccode ) %][% item.ccode | html %][% END %] - [% IF Koha.Preference('EnableItemGroups') %][% item.object.item_group.description | html %][% END %] + [%- IF ( itemdata_ccode ) %][% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode', authorised_value => item.ccode ) | html %][% END %] + [%- IF Koha.Preference('EnableItemGroups') %][% item.object.item_group.description | html %][% END %] [% IF ( item.itemcallnumber ) %] [% item.itemcallnumber | html %][% END %] - [% IF ( volinfo ) %] - [% SET serial = item.itemserial.serial %] - [% IF itemdata_publisheddate #If there is at least one published date, use it for sorting %] + [%- IF ( volinfo ) %] + [%- SET serial = item.itemserial.serial %] + [%- IF itemdata_publisheddate #If there is at least one published date, use it for sorting %] - [% ELSE %] + [%- ELSE %] - [% END %] - [% IF ( itemdata_enumchron ) %] - [% IF item.enumchron && serial.serialseq %] + [%- END %] + [%- IF ( itemdata_enumchron ) %] + [%- IF item.enumchron && serial.serialseq %] [% item.enumchron | html %] - [% IF ( item.serialseq && item.enumchron != serial.serialseq ) %] + [%- IF ( item.serialseq && item.enumchron != serial.serialseq ) %] -- [% serial.serialseq | html %] - [% END %] - [% ELSIF item.enumchron %] + [%- END %] + [%- ELSIF item.enumchron %] [% item.enumchron | html %] - [% ELSIF item.serialseq %] + [%- ELSIF item.serialseq %] [% serial.serialseq | html %] - [% END %] - [% IF serial.publisheddate %] + [%- END %] + [%- IF serial.publisheddate %] ([% serial.publisheddate | $KohaDates %]) - [% END %] - [% END %] + [%- END %] + [%- END %] - [% END %] + [%- END %] - [% IF item.checkout %] - [% IF item.checkout.onsite_checkout %] + [%- IF item.object.checkout %] + [%- IF item.checkout.onsite_checkout %] Currently in local use - [% ELSE %] + [%- ELSE %] Checked out - [% END %] - [% UNLESS ( item.not_same_branch) %] - [% IF item.checkout.onsite_checkout %] + [%- END %] + [%- IF item.can_be_edited %] + [%- IF item.object.checkout.onsite_checkout %] by - [% ELSE %] + [%- ELSE %] to - [% END %] + [%- END %] [% INCLUDE 'patron-title.inc' patron=item.checkout.patron hide_patron_infos_if_needed=1 %] - [% END %] - : due [% item.checkout.date_due | $KohaDates as_due_date => 1 %] + [%- END %] + : due [% item.object.checkout.date_due | $KohaDates as_due_date => 1 %] - [% ELSIF ( item.transfer ) %] - [% IF (item.transfer.datesent) %] - In transit from [% Branches.GetName( item.transfer.frombranch ) | html %] to [% Branches.GetName( item.transfer.tobranch ) | html %] since [% item.transfer.datesent | $KohaDates %] - [% ELSE %] - Transit pending from [% Branches.GetName( item.transfer.frombranch ) | html %] to [% Branches.GetName( item.transfer.tobranch ) | html %] since [% item.transfer.daterequested | $KohaDates %] - - [% END %] - [% END %] - - [% IF ( item.itemlost ) %] - [% IF itemlostloop %] - [% FOREACH itemlostloo IN itemlostloop %] - [% IF itemlostloo.authorised_value == item.itemlost %] - [% itemlostloo.lib | html %] - [% END %] - [% END %] - [% ELSE %] + [%- ELSIF ( transfer = item.object.get_transfer ) %] + [%- IF (transfer.datesent) %] + In transit from [% Branches.GetName( transfer.frombranch ) | html %] to [% Branches.GetName( transfer.tobranch ) | html %] since [% transfer.datesent | $KohaDates %] + [%- ELSE %] + Transit pending from [% Branches.GetName( transfer.frombranch ) | html %] to [% Branches.GetName( transfer.tobranch ) | html %] since [% item.transfer.daterequested | $KohaDates %] + + [%- END %] + [%- END -%] + + [%- IF ( item.itemlost ) %] + [%- SET itemlost_description = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.itemlost', authorised_value => item.itemlost ) %] + [%- IF itemlostloop %] + [% itemlost_description | html %] + [%- ELSE %] Unavailable (lost or missing) - [% END %] - [% END %] - - [% IF ( item.withdrawn ) %] - [% IF itemwithdrawnloop %] - [% FOREACH itemwithdrawnloo IN itemwithdrawnloop %] - [% IF itemwithdrawnloo.authorised_value == item.withdrawn %] - [% itemwithdrawnloo.lib | html %] - [% END %] - [% END %] - [% ELSE %] + [%- END %] + [%- END -%] + + [%- IF ( item.withdrawn ) %] + [%- SET withdrawn_description = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.withdrawn', authorised_value => item.withdrawn ) %] + [%- IF withdrawn_description %] + [% withdrawn_description | html %] + [%- ELSE %] Withdrawn - [% END %] - [% END %] - - [% IF ( item.damaged ) %] - [% IF itemdamagedloop %] - [% FOREACH itemdamagedloo IN itemdamagedloop %] - [% IF itemdamagedloo.authorised_value == item.damaged %] - [% itemdamagedloo.lib | html %] - [% END %] - [% END %] - [% ELSE %] + [%- END %] + [%- END -%] + + [%- IF ( item.damaged ) %] + [%- SET damaged_description = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.damaged', authorised_value => item.damaged ) %] + [%- IF damaged_description %] + [% damaged_description | html %] + [%- ELSE %] Damaged - [% END %] - [% END %] + [%- END %] + [%- END -%] - [% IF ( item.notforloan || item.itemtype.notforloan ) %] + [%- IF ( item.notforloan || item.itemtype.notforloan ) %] Not for loan - [% SET not_for_loan_description = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.notforloan', authorised_value => item.notforloan ) %] - [% IF not_for_loan_description %] + [%- 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 %] - [% END %] + [%- END -%] - [% SET hold = item.first_hold %] - [% IF hold %] - [% IF hold.waitingdate %] + [%- SET hold = item.first_hold %] + [%- IF hold %] + [%- IF hold.waitingdate %] Waiting at [% Branches.GetName( hold.branchcode ) | html %][% IF ( hold.desk_id ) %], [% hold.desk.desk_name | html %][% END %] since [% hold.waitingdate | $KohaDates %]. - [% IF canreservefromotherbranches AND ( hold.waitingdate OR hold.priority == 1 ) %] + [%- IF canreservefromotherbranches AND ( hold.waitingdate OR hold.priority == 1 ) %] Hold for: - [% INCLUDE 'patron-title.inc' patron=hold.borrower hide_patron_infos_if_needed=1 %] - [% END %] - [% ELSE %] + [%- INCLUDE 'patron-title.inc' patron=hold.borrower hide_patron_infos_if_needed=1 %] + [%- END %] + [%- ELSE %] There is an item level hold on this item (priority = [% hold.priority | html %]). - [% END %] - [% END %] - - [% 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 %][% recall.patron.firstname | html %] [% recall.patron.surname | html %] ([% recall.patron.cardnumber | html %])[% END %] - recalled by [% patron_link| $raw %] on [% recall.created_date | $KohaDates %] - [% END %] - [% END %] - - [% UNLESS ( item.notforloan || item.itemtype.notforloan || item.onloan || item.itemlost || item.withdrawn || item.damaged || item.transfer || hold || ( Koha.Preference('UseRecalls') && recall ) ) %] + [%- END %] + [%- END -%] + + [% IF Koha.Preference('UseRecalls') %] + [%- SET recall = item.object.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 %][% recall.patron.firstname | html %] [% recall.patron.surname | html %] ([% recall.patron.cardnumber | html %])[% END %] + recalled by [% patron_link| $raw %] on [% recall.created_date | $KohaDates %] + [%- END %] + [%- END -%] + [%- END -%] + + [%- UNLESS ( item.notforloan || item.itemtype.notforloan || item.onloan || item.itemlost || item.withdrawn || item.damaged || item.transfer || hold || ( Koha.Preference('UseRecalls') && recall ) ) %] Available - [% END %] + [%- END -%] - [% IF ( item.restricted ) %] + [%- IF ( item.restricted ) %] ([% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.restricted', authorised_value => item.restricted ) | html %]) - [% END %] + [%- END -%] - [% IF ( item.bundle_host ) %] + [%- IF ( item.bundle_host ) %] In bundle: [% INCLUDE 'biblio-title.inc' biblio = item.bundle_host.biblio link = 1 %] - [% END %] + [%- END -%] [% item.datelastseen | $KohaDates %] @@ -574,92 +568,92 @@ Note that permanent location is a code, and location may be an authval. [% item.dateaccessioned | $KohaDates %] [% item.datelastborrowed | $KohaDates %] [% item.barcode | html %] - [% IF ( itemdata_uri ) %] - [% IF item.uri.split(' \| ').size > 1 %] + [%- IF ( itemdata_uri ) %] + [%- IF item.uri.split(' \| ').size > 1 %] - [% FOREACH uri IN item.uri.split(' \| ') %][% uri | html %]
[% END %] + [%- FOREACH uri IN item.uri.split(' \| ') %][% uri | html %]
[% END %] - [% ELSE %] + [%- ELSE %] - [% IF item.uri %] + [%- IF item.uri %] [% IF Koha.Preference('URLLinkText') %][% Koha.Preference('URLLinkText') | html %][% ELSE %]Link to resource[% END %] - [% END %] + [%- END %] - [% END %] - [% END %] - [% IF ( itemdata_copynumber ) %] - [% item.copynumber | html %] - [% END %] - [% IF ( itemdata_stocknumber ) %] + [%- END %] + [%- END %] + [%- IF ( itemdata_copynumber ) %] + [% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.copynumber', authorised_value => item.copynumber ) | html %] + [%- END %] + [%- IF ( itemdata_stocknumber ) %] [% item.stocknumber | html %] - [% END %] - [% IF materials %] + [%- END %] + [%- IF materials %] [% item.materials | html %] - [% END %] - [% IF ( itemdata_itemnotes ) %] -
[% item.itemnotes | $raw %]
- [% END %] - [% IF itemdata_nonpublicnotes %] + [%- END %] + [%- IF ( itemdata_itemnotes ) %] +
[% item.object.itemnotes.replace('\n','
') | $raw %]
+ [%- END %] + [%- IF itemdata_nonpublicnotes %] [% item.itemnotes_nonpublic | html %] - [% END %] - [% IF ( hostrecords ) %] + [%- END %] + [%- IF ( hostrecords ) %] [% IF ( item.hostbiblionumber) %][% item.hosttitle | html %][% END %] - [% END %] - [% IF ( analyze ) %] + [%- END %] + [%- IF ( analyze ) %] - [% IF ( item.countanalytics ) %] + [%- IF ( item.countanalytics ) %] [% item.countanalytics | html %] analytics - [% END %] + [%- END %] - [% END %] - [% IF ( analyze ) %] + [%- END %] + [%- IF ( analyze ) %] Create analytics - [% END %] + [%- END -%] - [% IF ShowCourseReserves %] + [%- IF ShowCourseReserves %] - [% IF item.course_reserves %] - [% FOREACH r IN item.course_reserves %] - [% IF r.course.enabled == 'yes' %] + [%- IF item.course_reserves %] + [%- FOREACH r IN item.course_reserves %] + [%- IF r.course.enabled == 'yes' %]

- [% r.course.course_name | html %] + [%- r.course.course_name | html %] - [% IF r.course.section %] [% r.course.section | html %] [% END %] - [% IF r.course.term %] [% AuthorisedValues.GetByCode( 'TERM', r.course.term ) | html %] [% END %] + [%- IF r.course.section %] [% r.course.section | html %] [% END %] + [%- IF r.course.term %] [% AuthorisedValues.GetByCode( 'TERM', r.course.term ) | html %] [% END %]

- [% END %] - [% END %] - [% END %] + [%- END %] + [%- END %] + [%- END %] - [% END %] + [%- END -%] - [% IF ( SpineLabelShowPrintOnBibDetails ) %] + [%- IF ( SpineLabelShowPrintOnBibDetails ) %] Print label - [% END %] + [%- END -%] - [% IF CAN_user_editcatalogue_edit_items %] + [%- IF CAN_user_editcatalogue_edit_items %] - [% IF item.can_be_edited %] - [% IF Koha.Preference('LocalCoverImages') OR Koha.Preference('OPACLocalCoverImages') %] + [%- IF item.can_be_edited %] + [%- IF Koha.Preference('LocalCoverImages') OR Koha.Preference('OPACLocalCoverImages') %]
Edit
- [% ELSE %] + [%- ELSE %] Edit - [% END %] - [% END %] - [% IF bundlesEnabled %] + [%- END %] + [%- END %] + [%- IF bundlesEnabled %] - [% END %] + [%- END %] - [% END %] + [%- END %] - [% END %] + [%- END %] -- 2.30.2