Bug 19978 - Fix ITEMTYPECAT feature for grouping item types for search
Summary: Fix ITEMTYPECAT feature for grouping item types for search
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Whiteboard:
Keywords:
Depends on: 17843
Blocks:
  Show dependency treegraph
 
Reported: 2018-01-16 13:23 UTC by Jonathan Druart
Modified: 2018-10-24 09:13 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19978: Fix ITEMTYPECAT behaviour (1.78 KB, patch)
2018-01-16 13:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19978: Fix ITEMTYPECAT behaviour (1.96 KB, patch)
2018-02-12 13:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19978: Fix ITEMTYPECAT behaviour (2.01 KB, patch)
2018-02-13 14:18 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 19978: Fix ITEMTYPECAT behaviour (2.08 KB, patch)
2018-02-18 11:08 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2018-01-16 13:23:56 UTC
Since commit 091d6c513bcbee224ff06477e79be48cea7fe825
    Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes
Comment 1 Jonathan Druart 2018-01-16 13:33:01 UTC
Created attachment 70551 [details] [review]
Bug 19978: Fix ITEMTYPECAT behaviour

ITEMTYPECAT permits to group and hide item types at the OPAC (see bug
10937 for a complete description).

Since commit 091d6c513bcbee224ff06477e79be48cea7fe825
    Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes
the code assume that they are item types. Before it just assigned undef
to the description.

Test plan:
Create ITEMTYPECAT authorised values
Assign an item type to this authorised value group
Search for a item using this item type at the OPAC
Without this patch applied you get:
Can't call method "translated_description" on an undefined value at
/home/vagrant/kohaclone/opac/opac-search.pl line 231.

With this patch applied the search result is displayed.

Make sure the original feature still works.
Comment 2 Claire Gravely 2018-02-12 12:13:44 UTC
Hiya,

The patch has fixed the error message.

However, in the OPAC advanced search under the Item type tab, there is a checkbox for my new ITEMTYPECAT but no description (label) is displayed. 

If I select only the new category checkbox to perform a search (no keywords etc), nothing happens and the advanced search page refreshes. If I check the new category and also another itype category (eg: music) with no keywords, then search results are displayed as expected.
Comment 3 Jonathan Druart 2018-02-12 13:54:04 UTC
Created attachment 71471 [details] [review]
Bug 19978: Fix ITEMTYPECAT behaviour

ITEMTYPECAT permits to group and hide item types at the OPAC (see bug
10937 for a complete description).

Since commit 091d6c513bcbee224ff06477e79be48cea7fe825
    Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes
the code assume that they are item types. Before it just assigned undef
to the description.

Test plan:
Create ITEMTYPECAT authorised values
Assign an item type to this authorised value group
Search for a item using this item type at the OPAC
Without this patch applied you get:
Can't call method "translated_description" on an undefined value at
/home/vagrant/kohaclone/opac/opac-search.pl line 231.

With this patch applied the search result is displayed.

Make sure the original feature still works.
Comment 4 Claire Gravely 2018-02-12 14:21:20 UTC
Retested:

I have two ITEMTYPECAT categories. Only one of them is showing a description in the OPAC Adv search. The other is still an empty checkbox. :(

Searching by selecting only one (or both together) of my ITEMTYPECAT checkboxes is still not working.  Selecting a 'Normal' itype (not in a category) performs a search.
Comment 5 Jonathan Druart 2018-02-12 18:26:30 UTC
(In reply to Claire Gravely from comment #4)
> Retested:
> 
> I have two ITEMTYPECAT categories. Only one of them is showing a description
> in the OPAC Adv search. The other is still an empty checkbox. :(

I guess you forgot to fill the OPAC description field. It should pick the staff value, but I would prefer to fix that minor bug somewhere else.

> Searching by selecting only one (or both together) of my ITEMTYPECAT
> checkboxes is still not working.  Selecting a 'Normal' itype (not in a
> category) performs a search.

This sounds like it was a bug since the beginning. Fixed.
Comment 6 Claire Gravely 2018-02-13 07:30:06 UTC
(In reply to Jonathan Druart from comment #5)
> (In reply to Claire Gravely from comment #4)
> > Retested:
> > 
> > I have two ITEMTYPECAT categories. Only one of them is showing a description
> > in the OPAC Adv search. The other is still an empty checkbox. :(
> 
> I guess you forgot to fill the OPAC description field. It should pick the
> staff value, but I would prefer to fix that minor bug somewhere else.

You are right, I had only added an OPAC description for one of the categories.
> 
> > Searching by selecting only one (or both together) of my ITEMTYPECAT
> > checkboxes is still not working.  Selecting a 'Normal' itype (not in a
> > category) performs a search.
> 
> This sounds like it was a bug since the beginning. Fixed.


When 'More options' are displayed in the adv. search the categories work correctly. It is still broken if 'More Options' hasn't been selected.
Comment 7 Jonathan Druart 2018-02-13 14:08:28 UTC
Claire, I do not recreate.
I am not willing to fix all the issues this feature has (from when it has been written) anyway :)
The goal here is to fix regressions with previous versions.
Comment 8 Claire Gravely 2018-02-13 14:16:57 UTC
(In reply to Jonathan Druart from comment #7)
> Claire, I do not recreate.
> I am not willing to fix all the issues this feature has (from when it has
> been written) anyway :)
> The goal here is to fix regressions with previous versions.

OK I'll sign off as is and see if there is a bug report for the other issue.
Comment 9 Claire Gravely 2018-02-13 14:18:21 UTC
Created attachment 71539 [details] [review]
Bug 19978: Fix ITEMTYPECAT behaviour

ITEMTYPECAT permits to group and hide item types at the OPAC (see bug
10937 for a complete description).

Since commit 091d6c513bcbee224ff06477e79be48cea7fe825
    Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes
the code assume that they are item types. Before it just assigned undef
to the description.

Test plan:
Create ITEMTYPECAT authorised values
Assign an item type to this authorised value group
Search for a item using this item type at the OPAC
Without this patch applied you get:
Can't call method "translated_description" on an undefined value at
/home/vagrant/kohaclone/opac/opac-search.pl line 231.

With this patch applied the search result is displayed.

Make sure the original feature still works.

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 10 Katrin Fischer 2018-02-18 11:08:59 UTC
Created attachment 71901 [details] [review]
Bug 19978: Fix ITEMTYPECAT behaviour

ITEMTYPECAT permits to group and hide item types at the OPAC (see bug
10937 for a complete description).

Since commit 091d6c513bcbee224ff06477e79be48cea7fe825
    Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes
the code assume that they are item types. Before it just assigned undef
to the description.

Test plan:
Create ITEMTYPECAT authorised values
Assign an item type to this authorised value group
Search for a item using this item type at the OPAC
Without this patch applied you get:
Can't call method "translated_description" on an undefined value at
/home/vagrant/kohaclone/opac/opac-search.pl line 231.

With this patch applied the search result is displayed.

Make sure the original feature still works.

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Jonathan Druart 2018-02-18 19:50:19 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 12 Nick Clemens 2018-02-26 11:39:33 UTC
Awesome work all! Pushed to stable for 17.11.03