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

--- Comment #60 from Katrin Fischer <[email protected]> ---
Comment on attachment 36856
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36856
[PASSED QA] Bug 10937 - Option to hide and group itemtypes from advanced search

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

I talked to Tomas about this a while ago and finally got around to put my
concerns in writing. Leaving to Tomas for final decision as this is PQA right
now.

::: C4/Koha.pm
@@ +33,2 @@
>  use DBI qw(:sql_types);
> + use Data::Dumper;

This here doesn't look quite right, first you have: 
use autouse 'Data::cselectall_arrayref' => qw(Dumper);
then an additional
use Data::Dumper;
Can you explain?

@@ +279,5 @@
> +    $categories = GetItemTypesCategorized();
> +
> +Returns a hashref containing search categories.
> +A search category will be put in the hash if at least one of its itemtypes 
> is visible in OPAC.
> +The categories must be part of Authorized Values (DOCTYPECAT)

I am not happy with the name: DOCTYPECAT. If this is to help group itemtypes,
why not name it accordingly? ITYPECAT or ITEMTYPECAT? Or a bit different:
ITYPEGROUP?

::: installer/data/mysql/kohastructure.sql
@@ +1276,5 @@
>    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
> +  searchcategory varchar(20) default NULL, -- Group this item type with 
> others with the same value on OPAC search options

I think this is used to store the authorised value code picked from the
authorized value DOCTYPECAT category. If I am not wrong, they should match in
length to avoid problems:
authorised_values.authorised_value is varchar(80)

::: installer/data/mysql/updatedatabase.pl
@@ +9838,5 @@
> +    $dbh->do(q{
> +        ALTER TABLE itemtypes
> +            ADD hideinopac TINYINT(1) NOT NULL DEFAULT 0
> +              AFTER sip_media_type,
> +            ADD searchcategory VARCHAR(20) DEFAULT NULL

See above!

::: t/db_dependent/Koha.t
@@ +7,4 @@
>  use warnings;
>  use C4::Context;
>  use Koha::DateUtils qw(dt_from_string);
> +use Data::Dumper;

I am not sure if this is needed here.

-- 
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