From 77cc2ebee8fb2ac1800dcfc1bf5c7991e5ab6115 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 1 May 2019 10:57:01 +0000 Subject: [PATCH] Bug 21946: (follow-up) Address QA concerns 1 - Correct hashref for searching 2 - Correct undef for all (this only simplifies later logic so didn't affect outcomes) 3 - remove duplicated query 4 - Use get_column instead of loops 5 - Fix typo 6 - Whitespace fixes removed from earlier patches Signed-off-by: Christopher Brannon --- C4/Circulation.pm | 32 ++++++++-------------- .../prog/en/modules/admin/itemtypes.tt | 2 +- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index b499a10aee..ab52ea6e91 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -388,7 +388,7 @@ sub TooMany { my ($type_object, $parent_type, $parent_maxissueqty_rule); $type_object = Koha::ItemTypes->find( $type ); $parent_type = $type_object->parent_type if $type_object; - my $child_types = Koha::ItemTypes->search( parent_type => $type ); + my $child_types = Koha::ItemTypes->search({ parent_type => $type }); # Find any children if we are a parent_type; # given branch, patron category, and item type, determine @@ -403,7 +403,7 @@ sub TooMany { } ) if $parent_type; # If the parent rule is for default type we discount it - $parent_maxissueqty_rule = undef if $parent_maxissueqty_rule && $parent_maxissueqty_rule->itemtype eq '*'; + $parent_maxissueqty_rule = undef if $parent_maxissueqty_rule && !defined $parent_maxissueqty_rule->itemtype ; my $maxissueqty_rule = Koha::CirculationRules->get_effective_rule( { @@ -449,21 +449,15 @@ sub TooMany { # matching rule has the default item type, so count only # those existing loans that don't fall under a more # specific rule + my $issuing_itemtypes_query = " SELECT itemtype FROM issuingrules + WHERE branchcode = ? + AND (categorycode = ? OR categorycode = ?) + AND itemtype <> '*' "; if (C4::Context->preference('item-level_itypes')) { - $count_query .= " WHERE items.itype NOT IN ( - SELECT itemtype FROM issuingrules - WHERE branchcode = ? - AND (categorycode = ? OR categorycode = ?) - AND itemtype <> '*' - ) "; + $count_query .= " WHERE items.itype NOT IN ( $issuing_itemtypes_query )"; } else { $count_query .= " JOIN biblioitems USING (biblionumber) - WHERE biblioitems.itemtype NOT IN ( - SELECT itemtype FROM issuingrules - WHERE branchcode = ? - AND (categorycode = ? OR categorycode = ?) - AND itemtype <> '*' - ) "; + WHERE biblioitems.itemtype NOT IN ( $issuing_itemtypes_query )"; } push @bind_params, $maxissueqty_rule->branchcode; push @bind_params, $maxissueqty_rule->categorycode; @@ -473,16 +467,12 @@ sub TooMany { if ( $parent_maxissueqty_rule ) { # if we have a parent item type then we count loans of the # specific item type or its siblings or parent - push @types, $parent_type; my $children = Koha::ItemTypes->search({ parent_type => $parent_type }); - while ( my $child = $children->next ){ - push @types, $child->itemtype; - } + @types = $children->get_column('itemtype'); + push @types, $parent_type; } elsif ( $child_types ) { # If we are a parent type, we need to count all child types and our own type - while ( my $child = $child_types->next ){ - push @types, $child->itemtype; - } + @types = $child_types->get_column('itemtype'); push @types, $type; # And don't forget to count our own types } else { push @types, $type; } # Otherwise only count the specific itemtype my $types_param = ( '?,' ) x @types; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt index 9e9a97b9ef..0d5732d54c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt @@ -96,7 +96,7 @@ Item types administration [% IF itemtype %]

Modify item type

[% ELSE %] - ]

Add item type

+

Add item type

[% END %]
-- 2.11.0