Bug 20838

Summary: Search by group of libraries is broken
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: SearchingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: ata.rehman, fridolin.somers, m.de.rooy, martin.renvoize, nick
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20875
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 16735    
Bug Blocks: 20875    
Attachments: Bug 20838: (bug 16735 follow-up) Fix search by group of libraries
Bug 20838: Make Koha::Library::Group->all_libraries return a list of Koha::Libraries
Bug 20838: (bug 16735 follow-up) Fix search by group of libraries
Bug 20838: Make Koha::Library::Group->all_libraries return a list of Koha::Libraries
Bug 20838: (bug 16735 follow-up) Fix search by group of libraries
Bug 20838: Make Koha::Library::Group->all_libraries return a list of Koha::Libraries
Bug 20838: Add 2 small tests to check ref

Description Jonathan Druart 2018-05-30 17:40:18 UTC
In the last patches of bug 16735, we completely broke the feature!

The limit is using library_groups.id instead of branches.branchcode.
Comment 1 Jonathan Druart 2018-05-30 17:43:43 UTC
Created attachment 75686 [details] [review]
Bug 20838: (bug 16735 follow-up) Fix search by group of libraries

In the last patches of bug 16735, we completely broke the feature!

The limit is using library_groups.id instead of branches.branchcode.

Test plan:
Create a group of library with the search feature
Search (OPAC and staff interfaces) using this limit

=> Without this patch you will see that the generated search query does
not contain branchcodes
=> With this patch applied you will see the branchcodes
Comment 2 Jonathan Druart 2018-05-30 17:54:16 UTC
Created attachment 75687 [details] [review]
Bug 20838: Make Koha::Library::Group->all_libraries return a list of Koha::Libraries

Instead of a list of Koha::Library::Group
Ideally it should be a Koh::Libraries iterator
Comment 3 Kyle M Hall 2018-05-31 12:15:42 UTC
Created attachment 75700 [details] [review]
Bug 20838: (bug 16735 follow-up) Fix search by group of libraries

In the last patches of bug 16735, we completely broke the feature!

The limit is using library_groups.id instead of branches.branchcode.

Test plan:
Create a group of library with the search feature
Search (OPAC and staff interfaces) using this limit

=> Without this patch you will see that the generated search query does
not contain branchcodes
=> With this patch applied you will see the branchcodes

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Kyle M Hall 2018-05-31 12:15:49 UTC
Created attachment 75701 [details] [review]
Bug 20838: Make Koha::Library::Group->all_libraries return a list of Koha::Libraries

Instead of a list of Koha::Library::Group
Ideally it should be a Koh::Libraries iterator

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Marcel de Rooy 2018-06-01 09:28:22 UTC
QA: Looking here
Comment 6 Marcel de Rooy 2018-06-01 09:43:43 UTC
Created attachment 75731 [details] [review]
Bug 20838: (bug 16735 follow-up) Fix search by group of libraries

In the last patches of bug 16735, we completely broke the feature!

The limit is using library_groups.id instead of branches.branchcode.

Test plan:
Create a group of library with the search feature
Search (OPAC and staff interfaces) using this limit

=> Without this patch you will see that the generated search query does
not contain branchcodes
=> With this patch applied you will see the branchcodes

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2018-06-01 09:43:48 UTC
Created attachment 75732 [details] [review]
Bug 20838: Make Koha::Library::Group->all_libraries return a list of Koha::Libraries

Instead of a list of Koha::Library::Group
Ideally it should be a Koh::Libraries iterator

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2018-06-01 09:44:13 UTC
Test coverage of all_libraries is rather limited..
Comment 9 Jonathan Druart 2018-06-01 14:40:16 UTC
Created attachment 75739 [details] [review]
Bug 20838: Add 2 small tests to check ref
Comment 10 Nick Clemens 2018-06-04 13:55:14 UTC
I am having issues testing this - when I enable "OpacAddMastheadLibraryPulldown" I don't have a list of libraries and my group doesn't show any values
Comment 11 Jonathan Druart 2018-06-04 14:10:23 UTC
(In reply to Nick Clemens from comment #10)
> I am having issues testing this - when I enable
> "OpacAddMastheadLibraryPulldown" I don't have a list of libraries and my
> group doesn't show any values

Right, it's another bug. I am going to deal with it on a separate bug report.
To test this one you need to use the advanced search form.
Comment 12 Nick Clemens 2018-06-04 17:50:14 UTC
Pushed to master for 18.11, awesome work everybody!
Comment 13 Martin Renvoize 2018-06-05 11:24:14 UTC
This patch has been pushed to 18.05.x and will be in 18.05.01
Comment 14 Fridolin Somers 2018-06-07 07:22:04 UTC
Depends on Bug 16735 not in 17.11.x