From bb0f6c348fd4997b8901f564787631664d823c63 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 7 Aug 2020 12:01:10 +0200 Subject: [PATCH] Bug 26139: Centralize code for "Place hold" button (detail) There is a "norequest" boolean passed to the include cat-toolbar.inc, to display or not the "Pace hold" button. This flag was not calculated in some place (ISBDdetail and moredetail for instance) Here we centralize the code to make it more robust and less regression prone. Note that the same problem appears at the OPAC (opac-MARCdetail is always display the button). That will have to be fixed separately Test plan: Create biblio A with 0 item, B and C with 1 item and D with 2 items item for B is not for loan, C is for loan and D has 1 of each Go to the bibliographic detail page of each record and confirm that the "Place hold" button appears appropriately. Test the different tabs of the catalogue module QA note: This patch is only centralizing the existing code, but it is still too naive. To manage the visibility of this button we certainly want to copy what is done in C4::Search::searchResults: # can place a hold on a item if # not lost nor withdrawn # not damaged unless AllowHoldsOnDamagedItems is true # item is either for loan or on order (notforloan < 0) $can_place_holds = 1 if ( !$item->{itemlost} && !$item->{withdrawn} && ( !$item->{damaged} || C4::Context->preference('AllowHoldsOnDamagedItems') ) && ( !$item->{notforloan} || $item->{notforloan} < 0 ) ); Signed-off-by: Didier Gautheron --- Koha/Items.pm | 13 +++++++++++++ catalogue/MARCdetail.pl | 4 ---- catalogue/detail.pl | 5 ----- catalogue/imageviewer.pl | 10 ---------- .../prog/en/includes/cat-toolbar.inc | 5 ++++- t/db_dependent/Koha/Items.t | 15 ++++++++++++++- 6 files changed, 31 insertions(+), 21 deletions(-) diff --git a/Koha/Items.pm b/Koha/Items.pm index d38c5475cf..057a361f90 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -37,6 +37,19 @@ Koha::Items - Koha Item object set class =cut +=head3 filter_by_for_loan + +$items->filter_by_not_for_loan; + +Return the items of the set that are not for loan + +=cut + +sub filter_by_for_loan { + my ($self) = @_; + return $self->search( { notforloan => [ 0, undef ] } ); +} + =head3 type =cut diff --git a/catalogue/MARCdetail.pl b/catalogue/MARCdetail.pl index 44bc8d0801..d4f71a5029 100755 --- a/catalogue/MARCdetail.pl +++ b/catalogue/MARCdetail.pl @@ -267,7 +267,6 @@ my %witness ; #---- stores the list of subfields used at least once, with the "meaning" of the code my @item_subfield_codes; my @item_loop; -my $norequests = 1; foreach my $field (@fields) { next if ( $field->tag() < 10 ); @@ -295,8 +294,6 @@ foreach my $field (@fields) { $item->{ $subf[$i][0] } .= GetAuthorisedValueDesc( $field->tag(), $subf[$i][0], $subf[$i][1], '', $tagslib) || $subf[$i][1]; } - $norequests = 0 if $tagslib->{ $field->tag() }->{ $subf[$i][0] }->{kohafield} eq 'items.notforloan' and $subf[$i][1] == 0; - my $kohafield = $tagslib->{ $field->tag() }->{ $subf[$i][0] }->{kohafield}; $item->{ $subf[$i][0] } = output_pref( { str => $item->{ $subf[$i][0] }, dateonly => 1 } ) if grep { $kohafield eq $_ } @@ -330,7 +327,6 @@ if ($subscriptionscount) { } $template->param ( - norequests => $norequests, item_loop => \@item_loop, item_header_loop => \@item_header_loop, item_subfield_codes => \@item_subfield_codes, diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 4a2aa3afa7..5011c347ea 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -270,7 +270,6 @@ my $collections = my $copynumbers = { map { $_->{authorised_value} => $_->{lib} } Koha::AuthorisedValues->get_descriptions_by_koha_field( { frameworkcode => $fw, kohafield => 'items.copynumber' } ) }; my (@itemloop, @otheritemloop, %itemfields); -my $norequests = 1; my $mss = Koha::MarcSubfieldStructures->search({ frameworkcode => $fw, kohafield => 'items.itemlost', authorised_value => [ -and => {'!=' => undef }, {'!=' => ''}] }); if ( $mss->count ) { @@ -306,9 +305,6 @@ my $separatebranch = C4::Context->preference('SeparateHoldingsBranch') || 'homeb foreach my $item (@items) { my $itembranchcode = $item->{$separatebranch}; - # can place holds defaults to yes - $norequests = 0 unless ( ( $item->{'notforloan'} > 0 ) || ( $item->{'itemnotforloan'} > 0 ) ); - $item->{imageurl} = defined $item->{itype} ? getitemtypeimagelocation('intranet', $itemtypes->{ $item->{itype} }{imageurl}) : ''; @@ -409,7 +405,6 @@ if (scalar(@itemloop) == 0 || scalar(@otheritemloop) == 0) { } } -$template->param( norequests => $norequests ); $template->param( MARCNOTES => $marcnotesarray, itemdata_ccode => $itemfields{ccode}, diff --git a/catalogue/imageviewer.pl b/catalogue/imageviewer.pl index 67ded03c46..325ff6d283 100755 --- a/catalogue/imageviewer.pl +++ b/catalogue/imageviewer.pl @@ -48,15 +48,6 @@ my $biblio = Koha::Biblios->find( $biblionumber ); my $itemcount = $biblio ? $biblio->items->count : 0; my @items = GetItemsInfo($biblionumber); -my $norequests = 1; -foreach my $item (@items) { - - # can place holds defaults to yes - $norequests = 0 - unless ( ( $item->{'notforloan_per_itemtype'} > 0 ) - || ( $item->{'itemnotforloan'} > 0 ) ); -} - if ( $query->cookie("holdfor") ) { my $holdfor_patron = Koha::Patrons->find( $query->cookie("holdfor") ); $template->param( @@ -83,7 +74,6 @@ if ( C4::Context->preference("LocalCoverImages") ) { } $template->{VARS}->{'count'} = $itemcount; $template->{VARS}->{'biblionumber'} = $biblionumber; -$template->{VARS}->{'norequests'} = $norequests; $template->param(C4::Search::enabled_staff_search_views); $template->{VARS}->{'biblio'} = $biblio; diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc index e6259f991c..4d07b26ce6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc @@ -1,3 +1,4 @@ +[% USE Context %] [% INCLUDE 'blocking_errors.inc' %]
@@ -116,7 +117,9 @@ CAN_user_serials_create_subscription ) %] [% IF ( CAN_user_reserveforothers ) %] - [% UNLESS ( norequests ) %] + [%# biblio.items.filter_by_for_loan.count %] + [% SET items = biblio.items %] + [% IF Context.Scalar(Context.Scalar(items, "filter_by_for_loan"), "count") %] [% IF ( holdfor ) %]