Our use case are item types used for ILL - it's possible to hide the records and the items, but the advanced search form is a bit trickier. It would be nice if we could specify which itemtypes are offered as search options in the OPAC advanced search form.
Two new columns will be added in the item type. 1) Display in OPAC Default : Yes Set to NO to hide and itemtype in the opac's advanced search. 2) Search category The search category allows library to group some « similar » itemtypes in a single checkbox in the opac. The categories are defined in an Authorized value. For Example, itemtypes DVD, VHS could use the Video category. Searching for Video in the OPAC would returns the DVD and VHS items
Created attachment 22007 [details] [review] Bug 10937 - Options to group or hide items in adv. search This allows to group certain item types in a category, to be displayed (and searched) as such in OPAC's advanced search. For example, you can group Reserve 2h and Reserve 12h into a Reserve category. The 2 and 12h types won't appear anymore. This also allows to simply prevent an item type from displaying as a search option. Modified: C4/Koha.pm GetItemTypesCategorized admin/itemtypes.pl koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt opac/opac-search.pl installer/data/mysql/kohastructure.sql Added two columns, hideinopac and searchcategory installer/data/mysql/updatedatabase.pl Testing: 0) run updatedatabase.pl to add the two columns to itemtypes 1) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the category dropdown 2) Click button "New authorized value for DOCTYPECAT" 3) Enter HARDWARE as Atuhorize value, ANything as Description and Hardware as OPAC description then click SAVE. 4) In Intranet/Koha Amind/Item types, select (at least) two different items and proceed with the following steps a) Click action/Edit on the right column b) Third row is the search category list box, select Hardware c) click Save changes at the bottom 5) In Intranet/Koha Admin/Item types (again), select a different item type and click the checkbox "Hide in OPAC" 6) Go to OPAC/Adv search. 7) Validate that all items modified above do not appear in Item type list 8) Validate that new item type Hardware does appear instead. 9) Select item Hardware, start Search. 10) Validate returned items are the of the two types that were grouped into the Hardware category in step 4.
I'm glad that someone has worked on this issue as I was just talking about it a few hours ago! However: 1) There is an issue with your code in regards to "Search Category". If an item type has a "search category" of none, it won't appear on the Advanced Search page. That is...if "searchcategory" is null in the database and "hideinopac" is 0, the item type will appear. However, if "searchcategory" is blank and "hideinopac" is 0, the item type will be hidden. "searchcategory" will be blank any time an item type is saved and "None" is selected as the Search Category. Definitely not a good thing! There are a few other things that you will also want to consider in revising your patch: 2) Whitespace changes should be made in a separate patch so it's easier to separate code differences and format differences (like in kohastructure.sql). 3) You have a typo on the Add/Edit item type page (http://devonestaff/cgi-bin/koha/admin/itemtypes.pl?op=add_form). It says "Advanded Search" rather than "Advanced Search". -- You might want to also consider the following: 4) Your test plan assumes that there is an authorized value category of "DOCTYPECAT". You should mention that most users will probably have to create this authorized value category. Also, your test plan says to choose "Hardware" in the staff client, but the authorized value will actually be "Anything" according to your plan. Of course, it should be the only search category available (other than none), but the test plan is a rather dense block of text, so I found it difficult to tell exactly what I was supposed to see. It might be useful to add some whitespace between the test plan steps to make it easier to read. -- I'll be happy to look again once you have an updated patch! Thanks, Philippe.
Created attachment 22276 [details] [review] Sponsor: Vanier college Following David Cook's comments, I made the following modifications: - C4/Koha.pm : include the non-NULL but empty category in the selection - admin/authorised_values.pl : Added the DOCTYPECAT as default. Seemed the cleanest solution. - installer/data/mysql/kohastructure.sql : cleaned the unwelcomed whitespaces. - koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt : fixed typo and added instruction message - koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt : minimized the diff by redoing the code. New testing instructions. SETUP 0) run updatedatabase.pl to add the two columns to itemtypes 1) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the category dropdown b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware NORMAL USAGE 2) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 3) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. FINAL TESTING 4) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. e) Validate returned items are the of the two types that were grouped into the Hardware category in step 4.
The patch is additive. The first one is required. I rebased before doing it, so all should be good. Feedbacks are welcomed.
Applying: Sponsor: Vanier college /home/koha/kohaclone/.git/rebase-apply/patch:24: tab in indent. LEFT JOIN authorised_values ON searchcategory = authorised_value WHERE searchcategory > '' /home/koha/kohaclone/.git/rebase-apply/patch:62: tab in indent. quantity SMALLINT(6) default NULL, -- suggested quantity to be purchased /home/koha/kohaclone/.git/rebase-apply/patch:63: tab in indent. currency VARCHAR(3) default NULL, -- suggested currency for the suggested price /home/koha/kohaclone/.git/rebase-apply/patch:64: tab in indent. price DECIMAL(28,6) default NULL, -- suggested price /home/koha/kohaclone/.git/rebase-apply/patch:65: tab in indent. total DECIMAL(28,6) default NULL, -- suggested total cost (price*quantity updated for currency) fatal: sha1 information is lacking or useless (C4/Koha.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Sponsor: Vanier college
Hello Could you write a followup for the new default bootstrap theme? Mathieu
Patches apply for me, for the second patch I think the subject should be a sponsor line :) (http://wiki.koha-community.org/wiki/Commit_messages#Sponsor_line)
Blou, could you take a look and provide a follow-up for the bootstrap theme? It's the default theme now and we shouldn't add a new feature that doesn't work with it. Also, both patches seem almost identical - I am obsoleting the older patch.
Created attachment 24596 [details] [review] Conflicts: installer/data/mysql/updatedatabase.pl
Created attachment 24597 [details] [review] Bug 10937 - Option to hide and group itemtypes from advanced search Sponsor: Vanier college Added bootstrap theme same instructions. SETUP 0) run updatedatabase.pl to add the two columns to itemtypes 1) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the category dropdown b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware NORMAL USAGE 2) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 3) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. FINAL TESTING 4) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. e) Validate returned items are the of the two types that were grouped into the Hardware category in step 4.
Comment on attachment 24597 [details] [review] Bug 10937 - Option to hide and group itemtypes from advanced search Review of attachment 24597 [details] [review]: ----------------------------------------------------------------- ::: C4/Koha.pm @@ +41,4 @@ > &slashifyDate > &subfield_is_koha_internal_p > &GetPrinters &GetPrinter > + &GetItemTypes &GetItemTypesCategorized &GetItemTypesByCategory &getitemtypeinfo No problem, but I would have put the new functions on a different line, so as to reduce the chance of messing up the old ones. @@ +270,4 @@ > } > } > > +=head2 GetItemTypes GetItemTypesCategorized. Oops! ::: admin/itemtypes.pl @@ +168,4 @@ > $input->param('summary'), > $input->param('checkinmsg'), > $input->param('checkinmsgtype'), > + ( $input->param('hideinopac') ? 1 : 0 ), This one is in parenthesis? @@ +193,4 @@ > $input->param('summary'), > $input->param('checkinmsg'), > $input->param('checkinmsgtype'), > + $input->param('hideinopac') ? 1 : 0, This one is not? It might be best to avoid the inline expr?true:false in this list. ::: installer/data/mysql/kohastructure.sql @@ +1241,5 @@ > summary text, -- information from the summary field, may include HTML > 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" > + hideinopac tinyint(1) NOT NULL DEFAULT 0, -- Hide the item type from the search options in OPAC > + searchcategory varchar(15) default NULL, -- Group this item type with others with the same value on OPAC search options How did you decide on varchar(15)? ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-advsearch.tt @@ +146,3 @@ > [% itemtypeloo.description %]</label></div> > [% IF ( loop.last ) %]</div>[% ELSE %][% UNLESS ( loop.count % 4 ) %]</div><div class="row-fluid">[% END %][% END %] > + [% END %] This END isn't even the area you were adding your IF around. Why change the indentation? Indentation issues should perhaps be a separate patch.
Does not apply to master. Please rebase.
Created attachment 24778 [details] [review] Bug 10937 - Option to hide and group itemtypes from advanced search Fix and rebase following mtompset's comments Bug 10937 - Option to hide and group itemtypes from advanced search Fixed and rebased following mtompset's comments. - Kept inline expr, same as "model" 5 lines earlier. - No indentation change. END matching (now cleaner) introductory IF - changed to varchar(20) to match authorised_value's type.
Applies fine. mtompset@ubuntu:~/kohaclone$ koha-qa.pl -v 2 -c 1 testing 1 commit(s) (applied to bab96c3 'Bug 11571: fix breakage of -f option ') FAIL C4/Koha.pm OK pod FAIL forbidden patterns forbidden pattern: tab char (line 288) forbidden pattern: tab char (line 45) OK valid OK critic ... FAIL opac/opac-search.pl OK pod FAIL forbidden patterns forbidden pattern: tab char (line 411) forbidden pattern: tab char (line 228) OK valid OK critic Please correct your tab issues.
Created attachment 24779 [details] [review] Fix tab issues
Comment on attachment 24778 [details] [review] Bug 10937 - Option to hide and group itemtypes from advanced search Review of attachment 24778 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/updatedatabase.pl @@ +7950,5 @@ > + $dbh->do(q{ > + ALTER TABLE itemtypes > + ADD hideinopac TINYINT(1) NOT NULL DEFAULT 0, > + ADD searchcategory VARCHAR(20) DEFAULT NULL; > + }); You forgot to print out a meaningful message and call SetVersion, like the other ones. :)
CURRENT TEST PLAN ------------------ 0) Back up database, so you can reset and retest easily. ;) 1) Run Koha QA tool. 2) Apply the patch SETUP 3) run ./installer/data/mysql/updatedatabase.pl to add the two columns to itemtypes -- Does a meaningful message get printed? Are the columns added? "DESCRIBE itemtypes;" should list hideinopac and searchcategory. 4) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the 'Show category:' dropdown b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware NORMAL USAGE 2) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 3) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. FINAL TESTING 4) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. e) Validate returned items are the of the two types that were grouped into the Hardware category in step 4. More revisions to follow once I actually start testing this fully. :)
Created attachment 24782 [details] [review] (Re?)added update message and set version to 3.15.XXX
Okay... that fresh install works. the update works. the editing the itemtype stuff all seems to work. But I can't get it to search properly. ARTCL Article AUD Audio CD BK Books CF Computer Files CR Continuing Resources EBK Ebooks, Online Resources IM In house Material MP Maps MF Microfiche MX Mixed Materials MU Music REF Reference RPR Reprint, VFile TH Thesis, Dissertation VCD Video CD VM Visual Materials These are my item types. I took the first 8 and did a pretty 0-7 binary combination with HARDWARE, Hide in OPAC, and Not for Loan. I then went to the OPAC interface, clicked on Advanced Search and tried to make sense of the Item type box. 10 things plus hardware were displayed. 16-4 (hidden in OPAC) = 12, 12-2 (not hidden in OPAC, but HARDWARE) = 10. plus hardware. Display looks good. I checked hardware, and then clicked the Search button at the bottom of the page. I got a bunch of results, but this is where it doesn't make sense. My first result was a journal (Continuing Resources), which I had marked as no category, not for loan, hide in OPAC. The fact that the search returned a no category, and not the hardware category makes it seem broken. Here is the URL (the name doesn't exist): https://demo.library.kohasystem.ca/cgi-bin/koha/opac-search.pl?idx=kw&q=&idx=kw&q=&idx=kw&q=&limit-yr=&searchcat=HARDWARE&limit=&limit=&multibranchlimit=&sort_by=relevance&do=Search Hmmm... I changed the sort by drop down to title A-Z and the first one was BK, so that was expected. The result count is strangely high too, but perhaps I am mistaken in that account. I'll check manually later. Until then, I'm leaving this as Failed QA, because the search by relevance gave me a bad result, and I don't know if it is this patch, or just the by relevance search in general.
Results hard to reproduce, I'll wait on your further testing results. If you get "unwanted results", like the "no category" you listed below, could you examine the search string in the header, something like Results of search with limit(s): 'mc-itype,phr:CDR or mc-itype,phr:VG or mc-itype,phr:VT' That is what is generated when HARDWARE is exploded in my latest test with URL: cgi-bin/koha/opac-search.pl?idx=kw&q=&idx=kw&q=&idx=kw&q=&do=Search&searchcat=HARDWARE&limit-yr=&limit=&limit=&sort_by=relevance Thanks
Created attachment 28571 [details] [review] Fix and rebase following mtompset's comments
Created attachment 28572 [details] [review] Fix tab issues
Created attachment 28573 [details] [review] (Re?)added update message and set version to 3.15.XXX
Created attachment 28575 [details] [review] Bug 10937 - Option to hide and group itemtypes from advanced search Rebased, squashed, and fixed whitespace. CURRENT TEST PLAN ------------------ 0) Back up database, so you can reset and retest easily. ;) 1) Run Koha QA tool. 2) Apply the patch SETUP 3) run ./installer/data/mysql/updatedatabase.pl to add the two columns to itemtypes -- Does a meaningful message get printed? Are the columns added? "DESCRIBE itemtypes;" should list hideinopac and searchcategory. 4) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the 'Show category:' dropdown b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware NORMAL USAGE 2) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 3) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. FINAL TESTING 4) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. e) Validate returned items are the of the two types that were grouped into the Hardware category in step 4. More revisions to follow once I actually start testing this fully. :)
Created attachment 28576 [details] [review] Bug 10937 - Option to hide and group itemtypes from advanced search Rebased, squashed, and fixed whitespace. Added AFTER to updatedatabase.pl, so order will match kohastructure.sql Fixed GetItemTypesCategorized, so that partially hidden search categories still appear in the item type list. HOWEVER, THIS CREATES A PROBLEM. SEARCHING ON A CATEGORY MAY RETURN THINGS WHICH ARE NOT SUPPOSED TO BE SHOWN. SEE FURTHER COMMENTS. CURRENT TEST PLAN ------------------ 0) Back up database, so you can reset and retest easily. ;) 1) Run Koha QA tool. 2) Apply the patch SETUP 3) run ./installer/data/mysql/updatedatabase.pl to add the two columns to itemtypes -- Does a meaningful message get printed? Are the columns added? "DESCRIBE itemtypes;" should list hideinopac and searchcategory. 4) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the 'Show category:' dropdown b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware NORMAL USAGE 2) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 3) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. FINAL TESTING 4) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. e) Validate returned items are the of the two types that were grouped into the Hardware category in step 4. Current status: In Discussion
I think there is some inclusion/exclusion logic going awry here. If we don't show an item in the item types available in the advanced search, we likely do not want the user to actually get results with those in them. However, the results will still return them. If I hide everything except one item type, then if I don't select that item type, then everything is returned. If I do select just that item type, then the results will only be limited in that item type. Is this really what is desired?
Also, rather than just dump this to failed qa, because there is a lack of C4/Koha test for the new routines, I'd like to figure out this discussion first.
Okay, if OpacHiddenItems is set in a corresponding manner to the item types being marked as hidden in OPAC, then I think this should suffice. That is assuming that you only want the individual items not listed if they are grouped together. However, this is lacking testing for the C4/Koha routines that were added. Marking as Failed QA.
Created attachment 28579 [details] [review] Bug 10937 - Option to hide and group itemtypes from advanced search It's possible to hide the records and the items, but the advanced search form is a bit trickier. It would be nice if we could specify which itemtypes are offered as search options in the OPAC advanced search form. To solve this, two new columns will be added in the item type. 1) Display in OPAC Default : Yes Set to NO to hide and itemtype in the opac's advanced search. 2) Search category The search category allows library to group some similar itemtypes in a single checkbox in the opac. The categories are defined in an Authorized value. For Example, itemtypes DVD, VHS could use the Video category. Searching for Video in the OPAC would returns the DVD and VHS items. Initial coding by philippe.blouin@inlibro.com with mtompset@hotmail.com providing extra testing, patching, squashing, and rebasing. CURRENT TEST PLAN ------------------ 0) Back up database, so you can reset and retest easily. ;) 1) Run Koha QA tool. 2) Apply the patch SETUP 3) run ./installer/data/mysql/updatedatabase.pl to add the two columns to itemtypes -- Does a meaningful message get printed? Are the columns added? "DESCRIBE itemtypes;" should list hideinopac and searchcategory. 4) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the 'Show category:' dropdown i) If you do not have a DOCTYPECAT category, create one. b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware NORMAL USAGE 2) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 3) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. FINAL TESTING 4) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. e) Validate returned items are the of the two types that were grouped into the Hardware category in step 4. Sponsored-by: Vanier college
Created attachment 28624 [details] [review] Bug 10937 - Add unit tests. Modified t/db_dependant/Koha.t to test the two new functions from C4/Koha.pm.
Created attachment 28626 [details] [review] Bug 10937 - Option to hide and group itemtypes from advanced search It's possible to hide the records and the items, but the advanced search form is a bit trickier. It would be nice if we could specify which itemtypes are offered as search options in the OPAC advanced search form. To solve this, two new columns will be added in the item type. 1) Display in OPAC Default : Yes Set to NO to hide and itemtype in the opac's advanced search. 2) Search category The search category allows library to group some similar itemtypes in a single checkbox in the opac. The categories are defined in an Authorized value. For Example, itemtypes DVD, VHS could use the Video category. Searching for Video in the OPAC would returns the DVD and VHS items. As C4/Koha was modified, a set of subtests were added to confirm the changes in t/db_dependent/Koha.t. The two new routines add a grouping functionality into the OPAC Advanced Search Item type display area. Initial coding by philippe.blouin@inlibro.com with mtompset@hotmail.com providing extra testing, patching, squashing, and rebasing. CURRENT TEST PLAN ------------------ 0) Back up database, so you can reset and retest easily. ;) 1) Apply the patch 2) Run Koha QA tool. 3) prove -v t/db_dependent/Koha.t -- all tests should pass. 4) run ./installer/data/mysql/updatedatabase.pl to add the two columns to itemtypes -- Does a meaningful message get printed? Are the columns added? "DESCRIBE itemtypes;" should list hideinopac and searchcategory. 5) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the 'Show category:' dropdown i) If you do not have a DOCTYPECAT category, create one. b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware 6) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 7) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. 8) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. e) Validate returned items are the of the two types that were grouped into the Hardware category in step 4. Sponsored-by: Vanier college
Comment on attachment 28626 [details] [review] Bug 10937 - Option to hide and group itemtypes from advanced search Review of attachment 28626 [details] [review]: ----------------------------------------------------------------- ::: C4/Koha.pm @@ +278,5 @@ > > +=head2 GetItemTypesCategorized > + > +Returns item types but grouped by category if available. > +The categories must be part of Authorized Values (DOCTYPECAT) The perldoc on these new functions is lacking. @@ +320,5 @@ > +=head2 GetItemTypesByCategory > + > + $category = category filter > + > +Returns the itemtypes that are grouped into the category. The perldoc is valid, but horrible. ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt @@ +127,5 @@ > + [% END %] > + </select> > + (Options are defined as the authorized values for the DOCTYPECAT category) > + </li> > + [% ELSE %] Can you explain why there is an IF/ELSE case? ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-advsearch.tt @@ +141,5 @@ > <legend>Limit to any of the following:</legend> > <div class="row-fluid"> > [% FOREACH itemtypeloo IN advsearchloo.code_loop %] > + [% IF ((!itemtypeloo.searchcategory) AND (itemtypeloo.cat == 0)) OR (itemtypeloo.cat == 1) %] > + <div class="span3"><input type="checkbox" id="[% itemtypeloo.ccl FILTER remove(',') %]-[% itemtypeloo.number %]" name="[% IF ( itemtypeloo.cat == 1 ) %]searchcat[% ELSE %]limit[% END %]" value="[% IF ( itemtypeloo.cat == 1 ) %][% itemtypeloo.code %][% ELSE %]mc-[% itemtypeloo.ccl %]:[% itemtypeloo.code %][% END %]"/><label for="[% itemtypeloo.ccl FILTER remove(',') %]-[% itemtypeloo.number %]">[% UNLESS ( noItemTypeImages ) %][% IF ( itemtypeloo.imageurl ) %]<img border="0" src="[% itemtypeloo.imageurl %]" alt="[% itemtypeloo.description %]" />[% END %] [% END %] This be some ugly code. Improve readability, please. ::: koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt @@ +187,5 @@ > <table> > <tr> > [% FOREACH itemtypeloo IN advsearchloo.code_loop %] > + [% IF ((!itemtypeloo.searchcategory) AND (itemtypeloo.cat == 0)) OR (itemtypeloo.cat == 1) %] > + <td><input type="checkbox" id="[% itemtypeloo.ccl FILTER remove(',') %]-[% itemtypeloo.number %]" name="[% IF ( itemtypeloo.cat == 1 ) %]searchcat[% ELSE %]limit[% END %]" value="[% IF ( itemtypeloo.cat == 1 ) %][% itemtypeloo.code %][% ELSE %]mc-[% itemtypeloo.ccl %]:[% itemtypeloo.code %][% END %]"/><label for="[% itemtypeloo.ccl FILTER remove(',') %]-[% itemtypeloo.number %]">[% UNLESS ( noItemTypeImages ) %][% IF ( itemtypeloo.imageurl ) %]<img border="0" src="[% itemtypeloo.imageurl %]" alt="[% itemtypeloo.description %]" />[% END %] [% END %] You should looking into Tool Template variables. This code is ugly to read. http://www.template-toolkit.org/docs/manual/Variables.html
Created attachment 29383 [details] [review] Bug 10947 - Follow-up to: 'Review of attachment 28626 [details] [review]' C4/Koha.pm: Perldoc has been updated to better describe subroutine behaviour and usage. itemtypes.tt: The 'IF/ELSE' was used to distiguish between the 'ADD' and 'MODIFY' screens. It has been removed and the same 'select' is now used for both cases. opac-advsearch.tt (both files): I've reformatted the code to improve readability. CURRENT TEST PLAN ------------------ 0) Back up database, so you can reset and retest easily. ;) 1) Apply the patch 2) Run Koha QA tool. 3) prove -v t/db_dependent/Koha.t -- all tests should pass. 4) run ./installer/data/mysql/updatedatabase.pl to add the two columns to itemtypes -- Does a meaningful message get printed? Are the columns added? "DESCRIBE itemtypes;" should list hideinopac and searchcategory. 5) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the 'Show category:' dropdown i) If you do not have a DOCTYPECAT category, create one. b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware 6) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 7) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. 8) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. e) Validate returned items are the of the two types that were grouped into the Hardware category in step 4. Sponsored-by: Vanier college
Sorry, doesn't apply cleanly. Please rebase.
Created attachment 30308 [details] [review] Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. Other patches are now obsolete, this is a squashed patch. Merge conflicts were resolved. Sponsored-by: Vanier college
Created attachment 31896 [details] [review] Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. Other patches are now obsolete, this is a squashed patch. Merge conflicts were resolved. Sponsored-by: Vanier college
The above rebase was I, just the db update conflict to allow the sandbox to work ;)
Comment on attachment 31896 [details] [review] Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. Review of attachment 31896 [details] [review]: ----------------------------------------------------------------- ::: t/db_dependent/Koha.t @@ +7,4 @@ > use warnings; > use C4::Context; > use Koha::DateUtils qw(dt_from_string); > +use Data::Dumper; Oops! Forgot to remove it?
There's a problem when applying this patch on a sandbox : The sandbox you've requested is not ready. Some problems occurred applying patches from bug 10937: <h1>Something went wrong !</h1>Applying: Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging C4/Koha.pm Auto-merging installer/data/mysql/kohastructure.sql Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-advsearch.tt CONFLICT (delete/modify): koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt deleted in HEAD and modified in Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master.. Version Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. of koha-tmpl/opac-tmpl/prog/en/modules/opac-advsearch.tt left in tree. Auto-merging opac/opac-search.pl Failed to merge in the changes. Patch failed at 0001 Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master.
Created attachment 34419 [details] [review] Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. Other patches are now obsolete, this is a squashed patch. Merge conflicts were resolved. Sponsored-by: Vanier college
Sorry, merge conflicts (again) with current master (3.19.00.002): Applying: Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging opac/opac-search.pl CONFLICT (content): Merge conflict in opac/opac-search.pl Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Failed to merge in the changes. Oh, the patch title reads: Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. ...but we are on Bug 10937?
Created attachment 34635 [details] [review] Bug 10937 - Grouped ItemTypes - Patch should apply properly on latest master. Other patches are now obsolete, this is a squashed patch. Merge conflicts were resolved. Sponsored-by: Vanier college
Created attachment 34667 [details] [review] [Signed-off] Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. Other patches are now obsolete, this is a squashed patch. Merge conflicts were resolved. Sponsored-by: Vanier college http://bugs.koha-community.org/show_bug.cgi?id=10937 Followed test plan (comment #34), works as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Sorry, but modifying the table structure requires a patch to Schema files. http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7567#c58 That was my first experience with them. Hope this helps.
I am not sure if the change to the schema files is mandatory yet, but if you do it, please put it in a separate patch to avoid conflicts. http://wiki.koha-community.org/wiki/Database_updates
This patch needs a little bit more work to be readable by QAers. There is not decent commit message, please read http://wiki.koha-community.org/wiki/Commit_messages Marked as Failed QA.
Comment on attachment 34667 [details] [review] [Signed-off] Bug 10947 - Grouped ItemTypes - Patch should apply properly on latest master. Review of attachment 34667 [details] [review]: ----------------------------------------------------------------- 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.
(In reply to Jonathan Druart from comment #48) > There are occurrences of hideatopac in the code, maybe it's better than > hideinopac. Need to be confirmed by English native speaker. I can see how both might be used. The idea being that something is hidden at display time by OPAC vs. OPAC is being used an something is hidden in the output. Personally, hide in OPAC is clearer to Canadian, Anglophone me.
Created attachment 35452 [details] [review] Bug 10937 - Grouped ItemTypes - Patch should apply properly on latest master. This allows to group certain item types in a category, to be displayed (and searched) as such in OPAC's advanced search. For example, you can group Reserve 2h and Reserve 12h into a Reserve category. The 2 and 12h types won't appear anymore. This also allows to simply prevent an item type from displaying as a search option. TEST PLAN ------------------ 0) Back up database, so you can reset and retest easily. ;) 1) Apply the patch 2) Run Koha QA tool. 3) prove -v t/db_dependent/Koha.t -- all tests should pass. 4) run ./installer/data/mysql/updatedatabase.pl to add the two columns to itemtypes -- Does a meaningful message get printed? Are the columns added? "DESCRIBE itemtypes;" should list hideinopac and searchcategory. 5) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the 'Show category:' dropdown i) If you do not have a DOCTYPECAT category, create one. b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware 6) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 7) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. 8) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. ) Validate returned items are the of the two types that were grouped into the Hardware category in step 4. Sponsored-by: Vanier college
Created attachment 36191 [details] [review] Bug 10937 - Grouped ItemTypes - Patch should apply properly on latest master. Easy conflict on updatedatabase.pl fixed.
Created attachment 36240 [details] [review] Bug 10937 - Grouped ItemTypes - Patch should apply properly on latest master. This allows to group certain item types in a category, to be displayed (and searched) as such in OPAC's advanced search. For example, you can group Reserve 2h and Reserve 12h into a Reserve category. The 2 and 12h types won't appear anymore. This also allows to simply prevent an item type from displaying as a search option. TEST PLAN ------------------ 0) Back up database, so you can reset and retest easily. ;) 1) Apply the patch 2) Run Koha QA tool. 3) prove -v t/db_dependent/Koha.t -- all tests should pass. 4) run ./installer/data/mysql/updatedatabase.pl to add the two columns to itemtypes -- Does a meaningful message get printed? Are the columns added? "DESCRIBE itemtypes;" should list hideinopac and searchcategory. 5) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the 'Show category:' dropdown i) If you do not have a DOCTYPECAT category, create one. b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware 6) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 7) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. 8) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. ) Validate returned items are the of the two types that were grouped into the Hardware category in step 4. Sponsored-by: Vanier college Signed-off-by: Nick Clemens<nick@quecheelibrary.org>
I like this idea, but have a a small readability issue with the Hide In OPAC description on the Modify Item type page. It says: (if checked, items of this type will be hidden as filters in OPAC's advanced search) I think more on the lines of: If checked, the option to filter by items of this type will be hidden in OPAC advanced search (item of this type will not explicitly be hidden by this preference)
FAIL C4/Koha.pm OK critic OK forbidden patterns OK pod FAIL valid FAIL admin/authorised_values.pl OK critic OK forbidden patterns OK pod FAIL valid FAIL admin/itemtypes.pl OK critic OK forbidden patterns OK pod FAIL valid OK installer/data/mysql/updatedatabase.pl OK critic OK forbidden patterns OK pod OK valid FAIL opac/opac-search.pl OK critic OK forbidden patterns OK pod FAIL valid FAIL t/db_dependent/Koha.t OK critic OK forbidden patterns OK pod FAIL valid
I got that when I ran QA : OK C4/Koha.pm OK pod OK valid OK critic OK forbidden patterns OK admin/authorised_values.pl OK pod OK valid OK critic OK forbidden patterns OK admin/itemtypes.pl OK pod OK forbidden patterns OK valid OK critic OK installer/data/mysql/updatedatabase.pl OK forbidden patterns OK valid OK critic OK pod OK opac/opac-search.pl OK critic OK valid OK forbidden patterns OK pod OK t/db_dependent/Koha.t OK valid OK critic OK forbidden patterns OK pod OK koha-tmpl/intranet-tmpl/prog/en/modules/admin/itemtypes.tt OK valid_template OK forbidden patterns OK tt_valid OK koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-advsearch.tt OK tt_valid OK forbidden patterns OK valid_template Can anyone else run QA?
Looks good to me too. Kyle could you run perl -wc Koha.pm and tell us what is the error/warning? It's certainly related to the perl version (5.20 here).
It might be a good idea to QA bug 13787 at the same time as this bug.
(In reply to Jonathan Druart from comment #56) > Looks good to me too. > Kyle could you run perl -wc Koha.pm and tell us what is the error/warning? > It's certainly related to the perl version (5.20 here). Agreed, I'm running perl v5.10.1
Created attachment 36856 [details] [review] [PASSED QA] Bug 10937 - Option to hide and group itemtypes from advanced search This allows to group certain item types in a category, to be displayed (and searched) as such in OPAC's advanced search. For example, you can group Reserve 2h and Reserve 12h into a Reserve category. The 2 and 12h types won't appear anymore. This also allows to simply prevent an item type from displaying as a search option. TEST PLAN ------------------ 0) Back up database, so you can reset and retest easily. ;) 1) Apply the patch 2) Run Koha QA tool. 3) prove -v t/db_dependent/Koha.t -- all tests should pass. 4) run ./installer/data/mysql/updatedatabase.pl to add the two columns to itemtypes -- Does a meaningful message get printed? Are the columns added? "DESCRIBE itemtypes;" should list hideinopac and searchcategory. 5) You need to add a category to group your item types: a) In Intranet/Koha Admin/Authorized values, select DOCTYPECAT in the 'Show category:' dropdown i) If you do not have a DOCTYPECAT category, create one. b) Click button "New authorized value for DOCTYPECAT" c) Enter Authorized value: HARDWARE Description : Hardware Description (OPAC): Hardware 6) Group your items under that new category a) In Intranet/Koha Admin/Item types, choose (at least) two item types and for each: - Click action/Edit on the right column - Third row (below Description) is the Search category list box, select Hardware - click Save changes at the bottom 7) Select at least one item to be hidden in the OPAC search a) In Intranet/Koha Admin/Item types (again), choose a different item type: - Click action/Edit - Click the checkbox "Hide in OPAC" below the list of icons. 8) Go test your modifications a) Go to OPAC/Adv search. b) Validate that all items modified above (hidden or grouped) do not appear in Item type list c) Validate that new item type Hardware does appear instead. d) Select item Hardware, start Search. ) Validate returned items are the of the two types that were grouped into the Hardware category in step 4. Sponsored-by: Vanier college Signed-off-by: Nick <nick@quecheelibrary.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment on attachment 36856 [details] [review] [PASSED QA] Bug 10937 - Option to hide and group itemtypes from advanced search Review of attachment 36856 [details] [review]: ----------------------------------------------------------------- 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.
All those comments are perfectly valid. I'll correct them all (the use Dumper are most probably something forgotten by the dev, no excuse). Regarding the name, which one would you prefer "ITYPECAT or ITEMTYPECAT? Or a bit different: ITYPEGROUP" If no response, I'll go with ITYPEGROUP (of course, I'll do a patch's patch. I'm not messing with a passed qa)
Created attachment 40884 [details] [review] Bug 10937 - cleanup and rename DOCTYPECAT to ITEMTYPECAT - "Item" and not "Document" is the word used throughout Koha. - Moved the updates to an atomic update - changed searchcategory from varchar(20) to (80) - cleanup
Since the word "category" was used throughout the patch, I kept it instead of using "group". updatedatabase.pl doesn't apply anymore with the first patch, but since it's PassedQA, I'm not touching it. The second patch puts it all in atomicupdate, to help in case it all falls apart and someone recreate a new squashed patch from 0.
Created attachment 42323 [details] [review] Bug 10937 - Fix Seach Type in Advanced Search In order to see the checkboxes of the Collection and Shelving location of the advanced search, the if changed so that only the Itemtypes will go through the previous if
Patches pushed to master. Thanks Ahmed and Philippe!
Adding a see also to bug 18400. The functionality around ITEMTYPECAT looks pretty weird to me btw.