From add2f752b9717c07cc950d315fe093dc3c6e9536 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) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Joonas Kylmälä Signed-off-by: Jonathan Druart --- 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 1078dc2f5a..491c15e982 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 ec349d4d1d..f865ddb28d 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 0fe29a301b..7f4b710844 100755 --- a/catalogue/imageviewer.pl +++ b/catalogue/imageviewer.pl @@ -47,15 +47,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( @@ -82,7 +73,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 ) %]
Print
[% 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 ) %]