Bugzilla – Attachment 108159 Details for
Bug 26139
'Place hold' button isn't hidden in all detail views if there are no items available for loan
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26139: Centralize code for "Place hold" button (detail)
Bug-26139-Centralize-code-for-Place-hold-button-de.patch (text/plain), 8.08 KB, created by
Jonathan Druart
on 2020-08-13 06:35:11 UTC
(
hide
)
Description:
Bug 26139: Centralize code for "Place hold" button (detail)
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-08-13 06:35:11 UTC
Size:
8.08 KB
patch
obsolete
>From 3abc9b18697ea81183a6de3844c375a7fb3f6f65 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >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 ) > ); >--- > 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 37968dd02f..1a74f9a762 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' %] > <div id="toolbar" class="btn-toolbar"> > >@@ -116,7 +117,9 @@ CAN_user_serials_create_subscription ) %] > <div class="btn-group"><a id="printbiblio" class="btn btn-default"><i class="fa fa-print"></i> Print</a></div> > > [% 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 ) %] > <div class="btn-group"> > <button class="btn btn-default dropdown-toggle" data-toggle="dropdown"> >diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t >index d04559a16d..aa5a9d0737 100644 >--- a/t/db_dependent/Koha/Items.t >+++ b/t/db_dependent/Koha/Items.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > >-use Test::More tests => 11; >+use Test::More tests => 12; > use Test::Exception; > > use C4::Circulation; >@@ -196,6 +196,19 @@ subtest 'can_be_transferred' => sub { > 'We get the same result also if we pass the from-library parameter.'); > }; > >+subtest 'filter_by_for_loan' => sub { >+ plan tests => 3; >+ >+ my $biblio = $builder->build_sample_biblio; >+ is( $biblio->items->filter_by_for_loan->count, 0, 'no item yet' ); >+ $builder->build_sample_item( { biblionumber => $biblio->biblionumber, notforloan => 1 } ); >+ is( $biblio->items->filter_by_for_loan->count, 0, 'no item for loan' ); >+ $builder->build_sample_item( { biblionumber => $biblio->biblionumber, notforloan => 0 } ); >+ is( $biblio->items->filter_by_for_loan->count, 1, '1 item for loan' ); >+ >+ $biblio->delete; >+}; >+ > $retrieved_item_1->delete; > is( Koha::Items->search->count, $nb_of_items + 1, 'Delete should have deleted the item' ); > >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 26139
:
107949
|
108159
|
108379
|
108965
|
108966
|
109057