[Koha-bugs] [Bug 21946] Group circulation by item type

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21946

Jonathan Druart <jonathan.druart at bugs.koha-community.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jonathan.druart at bugs.koha-c
                   |                            |ommunity.org

--- Comment #29 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
Quick code review:

1.
+    my $child_types = Koha::ItemTypes->search( parent_type => $type );

search take a hashef!

2. 
+    $parent_maxissueqty_rule = undef if $parent_maxissueqty_rule &&
$parent_maxissueqty_rule->itemtype eq '*';

Is that possible? IIRC we had NULL in DB to tell "all", not "*"

3. 
+        if (C4::Context->preference('item-level_itypes')) {
+            $count_query .= q|SELECT COALESCE( SUM( IF(items.itype = '| .$type
. q|',1,0) ), 0) as type_total, COUNT(*) AS total,
COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts|;
+        } else{
+            $count_query .= q|SELECT COALESCE(SUM( IF(biblioitems.itype = '|
.$type . q|',1,0) ), 0) as type_total, COUNT(*) AS total,
COALESCE(SUM(onsite_checkout), 0) AS onsite_checkouts|
+        }

Please do not repeat, the only change is items vs biblioitems, so put it into a
var then generate the query.

4.
+                while ( my $child = $children->next ){
+                    push @types, $child->itemtype;
+                }

You need $children->get_column

5.
-        <h3>Add item type</h3>
+        ]<h3>Add item type</h3>

6. The indentation fix should not be here, it will add unnecessary conflict
with patches already in the queue (bug 15522 and deps for instance!).
smart-rules.tt particularly

7. I am really not sure the comparisons with '*' in smart-rules.tt are correct.

-- 
You are receiving this mail because:
You are watching all bug changes.