http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10937

--- Comment #48 from Jonathan Druart <[email protected]> ---
Comment on attachment 34667
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34667
[Signed-off] Bug 10947 - Grouped ItemTypes - Patch should apply properly on
latest master.

Review of attachment 34667:
 --> 
(http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=10937&attachment=34667)
-----------------------------------------------------------------

Quick code review.

::: C4/Koha.pm
@@ +283,5 @@
> +The categories must be part of Authorized Values (DOCTYPECAT)
> +
> +=cut
> +
> +sub GetItemTypesCategorized {

Why don't you use C4::ItemType?

@@ +287,5 @@
> +sub GetItemTypesCategorized {
> +    my $dbh   = C4::Context->dbh;
> +    # Order is important, so that partially hidden (some items are not 
> visible in OPAC) search
> +    # categories will be visible. hideinopac=0 must be last.
> +    my $query = qq|

There are no string interpolation here, limit to q||

@@ +312,5 @@
> +
> +    my %itemtypes;
> +    while ( my $IT = $sth->fetchrow_hashref ) {
> +        $itemtypes{ $IT->{'itemtype'} } = $IT;
> +    }

useless while, have a look at selectall_hashref.

@@ +324,5 @@
> +Returns the itemtype code of all itemtypes included in a searchcategory.
> +
> +=cut
> +
> +sub GetItemTypesByCategory {

Why don't you use C4::ItemType?

@@ +334,5 @@
> +    $sth->execute($category);
> +
> +    while ( my $data = $sth->fetchrow ) {
> +        push ( @results, $data );
> +    }

useless while, have a look at selectall_arrayref

::: installer/data/mysql/kohastructure.sql
@@ +1276,4 @@
>    checkinmsg VARCHAR(255), -- message that is displayed when an item with 
> the given item type is checked in
>    checkinmsgtype CHAR(16) DEFAULT 'message' NOT NULL, -- type (CSS class) 
> for the checkinmsg, can be "alert" or "message"
>    sip_media_type VARCHAR(3) DEFAULT NULL, -- SIP2 protocol media type for 
> this itemtype
> +  hideinopac tinyint(1) NOT NULL DEFAULT 0, -- Hide the item type from the 
> search options in OPAC

There are occurrences of hideatopac in the code, maybe it's better than
hideinopac. Need to be confirmed by English native speaker.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to