Attachment 93094 Details for Bug 21946 – Bug 21946: (follow-up) Address QA concerns

[patch] Bug 21946: (follow-up) Address QA concerns

Bug-21946-follow-up-Address-QA-concerns.patch (text/plain), 5.19 KB, created by ByWater Sandboxes on 2019-09-21 16:35:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: ByWater Sandboxes

Created: 2019-09-21 16:35:56 UTC

Size: 5.19 KB

patch

obsolete

>From 77cc2ebee8fb2ac1800dcfc1bf5c7991e5ab6115 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>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 <cbrannon@cdalibrary.org>
>---
> 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 %]
>         <h3>Modify item type</h3>
>     [% ELSE %]
>-        ]<h3>Add item type</h3>
>+        <h3>Add item type</h3>
>     [% END %]
>     <form action="/cgi-bin/koha/admin/itemtypes.pl" name="Aform" method="post" id="itemtypeentry">
>         <input type="hidden" name="op" value="add_validate" />
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 21946: 87713 | 87714 | 87715 | 87716 | 87717 | 87718 | 87719 | 88392 | 88574 | 88575 | 88576 | 88577 | 88578 | 88579 | 88580 | 88581 | 88926 | 88927 | 88928 | 88929 | 88930 | 88931 | 88932 | 88933 | 88934 | 89167 | 89168 | 89169 | 89170 | 89171 | 89172 | 89173 | 89174 | 89175 | 89176 | 93075 | 93076 | 93077 | 93078 | 93079 | 93080 | 93081 | 93082 | 93083 | 93084 | 93086 | 93087 | 93088 | 93089 | 93090 | 93091 | 93092 | 93093 | 93094 | 93095 | 93096 | 93097 | 93098 | 93099 | 93100 | 93101 | 93102 | 93103 | 93104 | 93105 | 96977 | 96978 | 96979 | 96980 | 96981 | 96982 | 96983 | 96984 | 96985 | 96986 | 96987 | 99419 | 99420 | 99421 | 99422 | 99423 | 99424 | 107035 | 107036 | 107037 | 107038 | 107039 | 107040 | 107041 | 107278 | 107279 | 107280 | 107281 | 107282 | 107283 | 107284 | 107467 | 107489 | 107686 | 107687 | 107688 | 107689 | 107690 | 107691 | 107692 | 107693 | 107694 | 107701 | 107733