From d650f8c2a6c5ee594a1c03fcab41c34a1cbce5fd 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 Signed-off-by: David Nind Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- catalogue/detail.pl | 64 ++------------ .../prog/en/modules/catalogue/detail.tt | 84 +++++++++---------- 2 files changed, 45 insertions(+), 103 deletions(-) diff --git a/catalogue/detail.pl b/catalogue/detail.pl index d99ecbdc56..225c992d27 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,9 @@ if ( $hostitems->count ) { } my $dat = &GetBiblioData($biblionumber); +$dat->{'count'} = $biblio->items->count + $hostitems->count; +$dat->{'showncount'} = scalar @items; +$dat->{'hiddencount'} = $dat->{'count'} - $dat->{'showncount'}; #is biblio a collection and are bundles enabled my $leader = $marc_record->leader(); @@ -320,16 +318,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 +359,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->$_; } @@ -399,21 +379,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){ @@ -445,23 +410,6 @@ foreach my $item (@items) { $item_info->{can_be_edited} = $patron->can_edit_items_from( $item->homebranch ); - 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 1befd20d7a..af95d0acad 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -386,7 +386,7 @@
- [% FOREACH image IN item.cover_images %] + [% FOREACH image IN item.object.cover_images %]
Local cover image @@ -411,22 +411,23 @@ [% Branches.GetName(item.homebranch) | html %] - + [%# 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 %] - [% IF item.location AND item.location != permloc_authval AND item.location != item.permanent_location %] - ([% item.location | html %]) + [% SET item_location = 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 %] + [% item_location | html %] [% END %] - [% IF ( itemdata_ccode ) %][% item.ccode | 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 ) %] @@ -457,62 +458,53 @@ Note that permanent location is a code, and location may be an authval. [% END %] - [% IF item.checkout %] - [% IF item.checkout.onsite_checkout %] + [% IF item.object.checkout %] + [% IF item.object.checkout.onsite_checkout %] Currently in local use [% ELSE %] Checked out [% END %] - [% UNLESS ( item.not_same_branch) %] - [% IF item.checkout.onsite_checkout %] + [% IF item.can_be_edited %] + [% IF item.object.checkout.onsite_checkout %] by [% ELSE %] to [% END %] - [% INCLUDE 'patron-title.inc' patron=item.checkout.patron hide_patron_infos_if_needed=1 %] + [% INCLUDE 'patron-title.inc' patron=item.object.checkout.patron hide_patron_infos_if_needed=1 %] [% END %] - : due [% item.checkout.date_due | $KohaDates as_due_date => 1 %] + : 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 %] + [% 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( item.transfer.frombranch ) | html %] to [% Branches.GetName( item.transfer.tobranch ) | html %] since [% item.transfer.daterequested | $KohaDates %] + 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 %] - [% FOREACH itemlostloo IN itemlostloop %] - [% IF itemlostloo.authorised_value == item.itemlost %] - [% itemlostloo.lib | html %] - [% END %] - [% END %] + [% 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 %] + [% 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 %] + [% SET damaged_description = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.damaged', authorised_value => item.damaged ) %] + [% IF damaged_description %] + [% damaged_description | html %] [% ELSE %] Damaged [% END %] @@ -540,13 +532,15 @@ Note that permanent location is a code, and location may be an authval. [% 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 %] + [% 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 %] @@ -583,7 +577,7 @@ Note that permanent location is a code, and location may be an authval. [% END %] [% END %] [% IF ( itemdata_copynumber ) %] - [% item.copynumber | html %] + [% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.copynumber', authorised_value => item.copynumber ) | html %] [% END %] [% IF ( itemdata_stocknumber ) %] [% item.stocknumber | html %] @@ -592,7 +586,7 @@ Note that permanent location is a code, and location may be an authval. [% item.materials | html %] [% END %] [% IF ( itemdata_itemnotes ) %] -
[% item.itemnotes | $raw %]
+
[% item.object.itemnotes.replace('\n','
') | $raw %]
[% END %] [% IF itemdata_nonpublicnotes %] [% item.itemnotes_nonpublic | html %]