Attachment 148431 Details for Bug 33167 – Bug 33167: Cleanup staff detail page

[patch] Bug 33167: Cleanup staff detail page

Bug-33167-Cleanup-staff-detail-page.patch (text/plain), 17.76 KB, created by Nick Clemens (kidclamp) on 2023-03-20 18:44:04 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2023-03-20 18:44:04 UTC

Size: 17.76 KB

patch

obsolete

>From 1d33972efe2e73d3a3877f31a5581bbf1800dd08 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>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
>---
> catalogue/detail.pl | 66 ++-------------
> .../prog/en/modules/catalogue/detail.tt | 80 +++++++++----------
> 2 files changed, 45 insertions(+), 101 deletions(-)
>
>diff --git a/catalogue/detail.pl b/catalogue/detail.pl
>index 48bd799879..cad270d1c8 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_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 707a5d7cf3..4d368faceb 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
>@@ -391,7 +391,7 @@
> <td class="cover">
> <div class="bookcoverimg">
> <div class="cover-slider">
>- [% FOREACH image IN item.cover_images %]
>+ [% FOREACH image IN item.object.cover_images %]
> <div class="cover-image local-coverimg">
> <a href="/cgi-bin/koha/catalogue/image.pl?itemnumber=[% image.itemnumber | uri %]&amp;imagenumber=[% image.imagenumber | uri %]" title="Local cover image">
> <img src="/cgi-bin/koha/catalogue/image.pl?thumbnail=1&amp;imagenumber=[% image.imagenumber | uri %]" alt="Local cover image" data-link="/cgi-bin/koha/catalogue/imageviewer.pl?itemnumber=[% item.itemnumber | uri %]&amp;imagenumber=[% image.imagenumber | uri %]" />
>@@ -416,22 +416,23 @@
> <td class="homebranch">
> [% 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_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 %]
>+ [% item_location | html %]
> [% END %]
> 
> </td>
>- [% IF ( itemdata_ccode ) %]<td>[% item.ccode | html %]</td>[% END %]
>+ [% IF ( itemdata_ccode ) %]<td>[% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode', authorised_value => item.ccode ) | html %]</td>[% END %]
> [% 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 ) %]
>@@ -462,62 +463,53 @@ Note that permanent location is a code, and location may be an authval.
> [% END %]
> <td class="status">
> 
>- [% IF item.checkout %]
>+ [% IF item.object.checkout %]
> [% IF item.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 %]
> [% 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 %]
>@@ -545,13 +537,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 %]<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 %]
>+ [% 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 %]<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 %]
> [% END %]
> 
>@@ -588,7 +582,7 @@ Note that permanent location is a code, and location may be an authval.
> [% END %]
> [% END %]
> [% IF ( itemdata_copynumber ) %]
>- <td class="copynumber">[% item.copynumber | html %]</td>
>+ <td class="copynumber">[% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.copynumber', authorised_value => item.copynumber ) | html %]</td>
> [% END %]
> [% IF ( itemdata_stocknumber ) %]
> <td class="stocknumber">[% item.stocknumber | html %]</td>
>@@ -597,7 +591,7 @@ Note that permanent location is a code, and location may be an authval.
> <td class="materials"> [% item.materials | html %] </td>
> [% END %]
> [% IF ( itemdata_itemnotes ) %]
>- <td><div class="itemnotes">[% item.itemnotes | $raw %]</div></td>
>+ <td><div class="itemnotes">[% item.object.itemnotes.replace('\n',' ') | $raw %]</div></td>
> [% END %]
> [% IF itemdata_nonpublicnotes %]
> <td class="nonpublicnote">[% item.itemnotes_nonpublic | html %]</td>
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 33167: 148029 | 148082 | 148083 | 148084 | 148085 | 148431 | 148432 | 148433 | 148434 | 149677 | 149704 | 150075 | 150076