Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system
Summary: Replace existing library search groups functionality with the new hierarchica...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Kyle M Hall (khall)
QA Contact: Testopia
URL:
Keywords:
Depends on: 15707
Blocks: 20747 18403 19765 20157 20838 20875 32291
  Show dependency treegraph
 
Reported: 2016-06-14 15:05 UTC by Kyle M Hall (khall)
Modified: 2022-11-23 09:07 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 16735 - Remove modules no longer needed (8.11 KB, patch)
2016-06-23 14:18 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove tables no longer needed (2.65 KB, patch)
2016-06-23 14:18 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Clean up sample data (7.82 KB, patch)
2016-06-23 14:18 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system (40.56 KB, patch)
2016-06-23 14:18 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove modules no longer needed (8.11 KB, patch)
2016-11-02 13:41 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove tables no longer needed (2.65 KB, patch)
2016-11-02 13:41 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Clean up sample data (7.82 KB, patch)
2016-11-02 13:41 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system (38.02 KB, patch)
2016-11-02 13:41 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16735 - Remove modules no longer needed (8.18 KB, patch)
2016-12-10 08:12 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16735 - Remove tables no longer needed (2.72 KB, patch)
2016-12-10 08:12 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16735 - Clean up sample data (7.89 KB, patch)
2016-12-10 08:12 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system (38.09 KB, patch)
2016-12-10 08:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 16735 - Remove modules no longer needed (8.17 KB, patch)
2017-02-21 13:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove tables no longer needed (2.71 KB, patch)
2017-02-21 13:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Clean up sample data (7.88 KB, patch)
2017-02-21 13:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system (38.18 KB, patch)
2017-02-21 13:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Don't use objects for database update (3.25 KB, patch)
2017-02-21 13:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove use of get_categories (4.97 KB, patch)
2017-02-21 13:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Filter individual libraries from search group pulldown (1.18 KB, patch)
2017-02-21 13:39 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove modules no longer needed (8.17 KB, patch)
2017-03-01 14:08 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove tables no longer needed (2.71 KB, patch)
2017-03-01 14:08 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Clean up sample data (7.88 KB, patch)
2017-03-01 14:08 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system (38.19 KB, patch)
2017-03-01 14:08 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Don't use objects for database update (3.25 KB, patch)
2017-03-01 14:08 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove use of get_categories (4.97 KB, patch)
2017-03-01 14:08 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Filter individual libraries from search group pulldown (1.18 KB, patch)
2017-03-01 14:08 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Filter groups without immediate child libraries from group pulldown (2.26 KB, patch)
2017-03-01 14:08 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove modules no longer needed (8.17 KB, patch)
2017-08-08 16:41 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Clean up sample data (7.88 KB, patch)
2017-08-08 16:41 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system (38.32 KB, patch)
2017-08-08 16:41 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Don't use objects for database update (3.25 KB, patch)
2017-08-08 16:41 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove use of get_categories (4.98 KB, patch)
2017-08-08 16:42 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Filter individual libraries from search group pulldown (1.18 KB, patch)
2017-08-08 16:42 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Filter groups without immediate child libraries from group pulldown (2.26 KB, patch)
2017-08-08 16:42 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Use libraries in all subgroups, not just immediate children (5.87 KB, patch)
2017-08-08 16:42 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 16735 - Remove modules no longer needed (8.19 KB, patch)
2017-12-05 17:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735 - Remove tables no longer needed (2.71 KB, patch)
2017-12-05 17:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735 - Clean up sample data (5.59 KB, patch)
2017-12-05 17:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system (38.61 KB, patch)
2017-12-05 17:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735 - Don't use objects for database update (3.25 KB, patch)
2017-12-05 17:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735 - Remove use of get_categories (5.00 KB, patch)
2017-12-05 17:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735 - Filter individual libraries from search group pulldown (1.19 KB, patch)
2017-12-05 17:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735: DBIC Schema files (1.63 KB, patch)
2017-12-05 17:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735 - Use libraries in all subgroups, not just immediate children (5.90 KB, patch)
2017-12-05 17:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735: Remove modules no longer needed (8.24 KB, patch)
2017-12-11 16:11 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: Remove tables no longer needed (2.80 KB, patch)
2017-12-11 16:11 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: Clean up sample data (5.64 KB, patch)
2017-12-11 16:11 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: Migrate library search groups into the new hierarchical groups (37.65 KB, patch)
2017-12-11 16:11 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: Don't use objects for database update (3.30 KB, patch)
2017-12-11 16:11 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: Remove use of get_categories (5.05 KB, patch)
2017-12-11 16:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: Filter individual libraries from search group pulldown (1.24 KB, patch)
2017-12-11 16:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: DBIC Schema files (1.68 KB, patch)
2017-12-11 16:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: Use libraries in all subgroups, not just immediate children (5.95 KB, patch)
2017-12-11 16:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: (QA followup) POD fixes (2.06 KB, patch)
2017-12-11 16:12 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: (QA followup) Fix rebase error (1.58 KB, patch)
2017-12-11 16:29 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 16735: Remove unecessary use of Koha::LibraryCategories in onboarding (1.25 KB, patch)
2018-02-09 14:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16735: Fix POD format for Koha::Library::Groups methods (1.14 KB, patch)
2018-02-09 20:01 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2016-06-14 15:05:10 UTC
We should replace the existing library search groups with the use of the new hierarchical groups system so that old system can be removed completely.

General outline:
1) Add the option to each group for it to be a "search" group
2) Port over existing groups by creating a new group for each "categorycode" in the branchrelations table
3) Re-implement the group search using the new system, remove the old system
Comment 1 Katrin Fischer 2016-06-15 06:14:08 UTC
Could we make it so that search groups and hierarchical groups can be different? You might want to group libraries for search differently than for organisational reasons.
Comment 2 Kyle M Hall (khall) 2016-06-15 11:22:32 UTC
(In reply to Katrin Fischer from comment #1)
> Could we make it so that search groups and hierarchical groups can be
> different? You might want to group libraries for search differently than for
> organisational reasons.

First off, you can actually define multiple different hierarchies within the new editor. It allows you to create an unlimited number of root groups. This gives us much flexibility in how a system can define its' hierarchy for different features.

I think there are two ways to accomplish this:
1) Add the "show group in search" option I outlined previously
2) Allow the library to create a special root group just for searching. This group would be stored in a syspref. Each child of the root group would be a search group to be displayed. The pros of this way would be that we don't need to alter the database schema to implement it.

What do you all think?

Kyle
Comment 3 Katrin Fischer 2016-06-15 11:50:36 UTC
My thinking was to preserve a bit of the feature now, without having it interfere with concerns of transport/circulation rules etc. Say you would geographically grouped search groups... but for organisation you group schools, publics etc. - not sure if that makes sense.

Sounds like option b maybe? But asking some consortia librarians might be good :)
Comment 4 Kyle M Hall (khall) 2016-06-20 11:55:37 UTC
(In reply to Katrin Fischer from comment #3)
> My thinking was to preserve a bit of the feature now, without having it
> interfere with concerns of transport/circulation rules etc. Say you would
> geographically grouped search groups... but for organisation you group
> schools, publics etc. - not sure if that makes sense.
> 
> Sounds like option b maybe? But asking some consortia librarians might be
> good :)

I think you are correct, I think b is the way to go.

General work outline:
1) Create a new system preference "SearchGroupsGroup" or something to that effect.
   This pref will accept the name of a group to use for Search Groups. The description
   will be something like:
   "Use all direct children of the group $GroupName as search groups.
2) Make db update to detect if system is already using search groups, if so, create a new group named "Search Groups" and add each collection of libraries as a child group to the new "Search Groups" root group.
3) Add code to generate the search groups markup for forms using the new system
4) Remove the old code and table from Koha
Comment 5 Kyle M Hall (khall) 2016-06-23 14:18:00 UTC
Created attachment 52732 [details] [review]
Bug 16735 - Remove modules no longer needed
Comment 6 Kyle M Hall (khall) 2016-06-23 14:18:07 UTC
Created attachment 52733 [details] [review]
Bug 16735 - Remove tables no longer needed
Comment 7 Kyle M Hall (khall) 2016-06-23 14:18:09 UTC
Created attachment 52734 [details] [review]
Bug 16735 - Clean up sample data
Comment 8 Kyle M Hall (khall) 2016-06-23 14:18:11 UTC
Created attachment 52735 [details] [review]
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system

Test Plan:
1) Apply this patch set
2) Note your existing search groups have been ported over to the new
   __SEARCH_GROUPS__ group if you had any
3) Create the group __SEARCH_GROUPS__ if one does not already exist
4) Add some first level subgroups to this group, add libraries to those groups
5) Search the library group searching in the intranet and opac
6) Note you get the same results as pre-patch
Comment 9 Jonathan Druart 2016-10-03 12:04:21 UTC
These patches do not apply cleanly.
Comment 10 Kyle M Hall (khall) 2016-11-02 13:41:33 UTC
Created attachment 57098 [details] [review]
Bug 16735 - Remove modules no longer needed
Comment 11 Kyle M Hall (khall) 2016-11-02 13:41:45 UTC
Created attachment 57099 [details] [review]
Bug 16735 - Remove tables no longer needed
Comment 12 Kyle M Hall (khall) 2016-11-02 13:41:48 UTC
Created attachment 57100 [details] [review]
Bug 16735 - Clean up sample data
Comment 13 Kyle M Hall (khall) 2016-11-02 13:41:51 UTC
Created attachment 57101 [details] [review]
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system

Test Plan:
1) Apply this patch set
2) Note your existing search groups have been ported over to the new
   __SEARCH_GROUPS__ group if you had any
3) Create the group __SEARCH_GROUPS__ if one does not already exist
4) Add some first level subgroups to this group, add libraries to those groups
5) Search the library group searching in the intranet and opac
6) Note you get the same results as pre-patch
Comment 14 Alex Buckley 2016-12-01 00:23:12 UTC
The Library Groups webpage (http://localhost:8081/cgi-bin/koha/admin/library_groups.pl) where the user should be able to create a group (in addition to being able to see previously made groups) results in a Internal Server Error.
Comment 15 Kyle M Hall (khall) 2016-12-09 14:40:43 UTC
(In reply to Alex Buckley from comment #14)
> The Library Groups webpage
> (http://localhost:8081/cgi-bin/koha/admin/library_groups.pl) where the user
> should be able to create a group (in addition to being able to see
> previously made groups) results in a Internal Server Error.

I am unable to replicate your error on a fresh installation of kohadevbox.
Comment 16 Chris Kirby 2016-12-09 19:33:13 UTC
Patch set did not apply cleanly. "fatal: sha1 information is lacking or useless (Koha/Library/Group.pm).Repository lacks necessary blobs to fall back on 3-way merge."
Comment 17 Josef Moravec 2016-12-10 07:28:25 UTC
(In reply to Chris Kirby from comment #16)
> Patch set did not apply cleanly. "fatal: sha1 information is lacking or
> useless (Koha/Library/Group.pm).Repository lacks necessary blobs to fall
> back on 3-way merge."

On top of bug 15707 applies cleanly
Comment 18 Josef Moravec 2016-12-10 08:12:26 UTC
Created attachment 58087 [details] [review]
[SIGNED-OFF] Bug 16735 - Remove modules no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 19 Josef Moravec 2016-12-10 08:12:32 UTC
Created attachment 58088 [details] [review]
[SIGNED-OFF] Bug 16735 - Remove tables no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 20 Josef Moravec 2016-12-10 08:12:38 UTC
Created attachment 58089 [details] [review]
[SIGNED-OFF] Bug 16735 - Clean up sample data

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 21 Josef Moravec 2016-12-10 08:13:23 UTC
Created attachment 58090 [details] [review]
[SIGNED-OFF] Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system

Test Plan:
1) Apply this patch set
2) Note your existing search groups have been ported over to the new
   __SEARCH_GROUPS__ group if you had any
3) Create the group __SEARCH_GROUPS__ if one does not already exist
4) Add some first level subgroups to this group, add libraries to those groups
5) Search the library group searching in the intranet and opac
6) Note you get the same results as pre-patch

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 22 Kyle M Hall (khall) 2017-02-21 13:39:32 UTC
Created attachment 60497 [details] [review]
Bug 16735 - Remove modules no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 23 Kyle M Hall (khall) 2017-02-21 13:39:40 UTC
Created attachment 60498 [details] [review]
Bug 16735 - Remove tables no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 24 Kyle M Hall (khall) 2017-02-21 13:39:43 UTC
Created attachment 60499 [details] [review]
Bug 16735 - Clean up sample data

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 25 Kyle M Hall (khall) 2017-02-21 13:39:46 UTC
Created attachment 60500 [details] [review]
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system

Test Plan:
1) Apply this patch set
2) Note your existing search groups have been ported over to the new
   __SEARCH_GROUPS__ group if you had any
3) Create the group __SEARCH_GROUPS__ if one does not already exist
4) Add some first level subgroups to this group, add libraries to those groups
5) Search the library group searching in the intranet and opac
6) Note you get the same results as pre-patch

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 26 Kyle M Hall (khall) 2017-02-21 13:39:49 UTC
Created attachment 60501 [details] [review]
Bug 16735 - Don't use objects for database update
Comment 27 Kyle M Hall (khall) 2017-02-21 13:39:53 UTC
Created attachment 60502 [details] [review]
Bug 16735 - Remove use of get_categories

Feature using it is completely undocumented as far as my research has
shown.
Comment 28 Kyle M Hall (khall) 2017-02-21 13:39:56 UTC
Created attachment 60503 [details] [review]
Bug 16735 - Filter individual libraries from search group pulldown
Comment 29 Jonathan Druart 2017-02-22 14:51:16 UTC
Tested on top of bug 15707: 
I cannot test it, the actions from the 'Actions' button do not do anything.

You should also add the btn-default class to the buttons to make them look like other Koha buttons.
Comment 30 Kyle M Hall (khall) 2017-02-22 20:02:23 UTC
Followup for this is on bug 15707 since it is an update to the editor and not directly related to this dependent bug.

(In reply to Jonathan Druart from comment #29)
> Tested on top of bug 15707: 
> I cannot test it, the actions from the 'Actions' button do not do anything.
> 
> You should also add the btn-default class to the buttons to make them look
> like other Koha buttons.
Comment 31 Jonathan Druart 2017-02-23 17:47:15 UTC
(In reply to Jonathan Druart from comment #29)
> You should also add the btn-default class to the buttons to make them look
> like other Koha buttons.

This is still valid.
Comment 32 Kyle M Hall (khall) 2017-03-01 14:08:25 UTC
Created attachment 60772 [details] [review]
Bug 16735 - Remove modules no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 33 Kyle M Hall (khall) 2017-03-01 14:08:34 UTC
Created attachment 60773 [details] [review]
Bug 16735 - Remove tables no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 34 Kyle M Hall (khall) 2017-03-01 14:08:39 UTC
Created attachment 60774 [details] [review]
Bug 16735 - Clean up sample data

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 35 Kyle M Hall (khall) 2017-03-01 14:08:43 UTC
Created attachment 60775 [details] [review]
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system

Test Plan:
1) Apply this patch set
2) Note your existing search groups have been ported over to the new
   __SEARCH_GROUPS__ group if you had any
3) Create the group __SEARCH_GROUPS__ if one does not already exist
4) Add some first level subgroups to this group, add libraries to those groups
5) Search the library group searching in the intranet and opac
6) Note you get the same results as pre-patch

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 36 Kyle M Hall (khall) 2017-03-01 14:08:48 UTC
Created attachment 60776 [details] [review]
Bug 16735 - Don't use objects for database update
Comment 37 Kyle M Hall (khall) 2017-03-01 14:08:52 UTC
Created attachment 60777 [details] [review]
Bug 16735 - Remove use of get_categories

Feature using it is completely undocumented as far as my research has
shown.
Comment 38 Kyle M Hall (khall) 2017-03-01 14:08:55 UTC
Created attachment 60778 [details] [review]
Bug 16735 - Filter individual libraries from search group pulldown
Comment 39 Kyle M Hall (khall) 2017-03-01 14:08:59 UTC
Created attachment 60779 [details] [review]
Bug 16735 - Filter groups without immediate child libraries from group pulldown
Comment 40 Kyle M Hall (khall) 2017-03-01 14:10:20 UTC
(In reply to Jonathan Druart from comment #31)
> (In reply to Jonathan Druart from comment #29)
> > You should also add the btn-default class to the buttons to make them look
> > like other Koha buttons.
> 
> This is still valid.

Fixed!
Comment 41 Martin Renvoize (ashimema) 2017-04-13 11:30:27 UTC
The more I think about this the more I dislike the idea.

Search Groups and Functional Groups may well need to be different.

If I'm understanding it correctly, the idea of Bug 15707 is to allow for preferences (i.e. things that govern functionality including lending rules, holding rules etc etc) to be shared and inherited down a hierarchical institution structure.  As such, each entity should have one parent and only one parent.. else inheritance gets super messy.

Searches may well want to apply over completely different groups and may well want to overlap.  Think of a set of library branches spread across a large area, you may well want to have search groups to allow searching of a group of 'immediate neighbours'; as such depending on which branch you're neighbours will be different and thus one entity will likely need to be part of more than one group.
Comment 42 Kyle M Hall (khall) 2017-04-13 13:31:33 UTC
Martin, I'm not sure what you are suggesting, but I think I have the idea.

Consider this group structure:
United States
    Pennsylvania
       Crawford County
           Meadville Public Library
           Saegertown Public Library
       Erie County
           Millcreek Public Library
           Edinboro Public Library
    New York
       Niagara County
           Niagara Public Library
           Falls Public Library
       Orleans County
           Orleans Public Library
           Beaver Public Library

So, if this were the heirarchy, you'd expect the search options to include:
Pennsylvania
Crawford County
Erie County
New York
Niagara County
Orleans Count

and selecting one of those would search all the libraries that are below that group or subgroup. Is that correct? If so, I think I agree and would be happy to implement that.


(In reply to Martin Renvoize from comment #41)
> The more I think about this the more I dislike the idea.
> 
> Search Groups and Functional Groups may well need to be different.
> 
> If I'm understanding it correctly, the idea of Bug 15707 is to allow for
> preferences (i.e. things that govern functionality including lending rules,
> holding rules etc etc) to be shared and inherited down a hierarchical
> institution structure.  As such, each entity should have one parent and only
> one parent.. else inheritance gets super messy.
> 
> Searches may well want to apply over completely different groups and may
> well want to overlap.  Think of a set of library branches spread across a
> large area, you may well want to have search groups to allow searching of a
> group of 'immediate neighbours'; as such depending on which branch you're
> neighbours will be different and thus one entity will likely need to be part
> of more than one group.
Comment 43 Martin Renvoize (ashimema) 2017-04-18 16:13:30 UTC
OK.. So using your example Hierarchy.

United States
    Pennsylvania
       Crawford County
           Meadville Public Library
           Saegertown Public Library
       Erie County
           Millcreek Public Library
           Edinboro Public Library
    New York
       Niagara County
           Niagara Public Library
           Falls Public Library
       Orleans County
           Orleans Public Library
           Beaver Public Library

What if 'Meadville' and 'Orleans' are physically near each other.  You may want a 'Search Group' that allows searching just that pair.  Thus the 'Hierarchical Grouping' doesn't work for that case. I think 'Search Groups' and 'Privilege Groups' need to be different things entirely and not bound to the same fundamental structure.
Comment 44 Martin Renvoize (ashimema) 2017-04-18 16:14:44 UTC
So, what you've described in your reply is what I 'think' I understand bug 15707 to be aiming for.. or is that not the case?
Comment 45 Martin Renvoize (ashimema) 2017-04-18 16:15:16 UTC
'aiming for' in terms of 'privileges' down the tree.. not 'search groups'.
Comment 46 Kyle M Hall (khall) 2017-04-18 16:29:31 UTC
(In reply to Martin Renvoize from comment #45)
> 'aiming for' in terms of 'privileges' down the tree.. not 'search groups'.

Correct, same behavior applied to different mechanics.
Comment 47 Kyle M Hall (khall) 2017-04-18 16:31:35 UTC
You are correct, which is why you can create as many groups and hierarchies as you want! If your "political" structure differs from your "geographic" structure, you can have two complete hierarchies which you can choose from based on your needs. For searching, you could even allow both sets of hierarchies to be searchable if you so chose.

(In reply to Martin Renvoize from comment #43)
> OK.. So using your example Hierarchy.
> 
> United States
>     Pennsylvania
>        Crawford County
>            Meadville Public Library
>            Saegertown Public Library
>        Erie County
>            Millcreek Public Library
>            Edinboro Public Library
>     New York
>        Niagara County
>            Niagara Public Library
>            Falls Public Library
>        Orleans County
>            Orleans Public Library
>            Beaver Public Library
> 
> What if 'Meadville' and 'Orleans' are physically near each other.  You may
> want a 'Search Group' that allows searching just that pair.  Thus the
> 'Hierarchical Grouping' doesn't work for that case. I think 'Search Groups'
> and 'Privilege Groups' need to be different things entirely and not bound to
> the same fundamental structure.
Comment 48 Marcel de Rooy 2017-06-16 10:22:26 UTC
Depends on a report in ASSIGNED status
Comment 49 Kyle M Hall (khall) 2017-08-08 16:41:41 UTC
Created attachment 65658 [details] [review]
Bug 16735 - Remove modules no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 50 Kyle M Hall (khall) 2017-08-08 16:41:49 UTC
Created attachment 65659 [details] [review]
Bug 16735 - Clean up sample data

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 51 Kyle M Hall (khall) 2017-08-08 16:41:53 UTC
Created attachment 65660 [details] [review]
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system

Test Plan:
1) Apply this patch set
2) Note your existing search groups have been ported over to the new
   __SEARCH_GROUPS__ group if you had any
3) Create the group __SEARCH_GROUPS__ if one does not already exist
4) Add some first level subgroups to this group, add libraries to those groups
5) Search the library group searching in the intranet and opac
6) Note you get the same results as pre-patch

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Conflicts:
	catalogue/search.pl
	installer/data/mysql/updatedatabase.pl
	opac/opac-search.pl
Comment 52 Kyle M Hall (khall) 2017-08-08 16:41:58 UTC
Created attachment 65661 [details] [review]
Bug 16735 - Don't use objects for database update
Comment 53 Kyle M Hall (khall) 2017-08-08 16:42:02 UTC
Created attachment 65662 [details] [review]
Bug 16735 - Remove use of get_categories

Feature using it is completely undocumented as far as my research has
shown.
Comment 54 Kyle M Hall (khall) 2017-08-08 16:42:06 UTC
Created attachment 65663 [details] [review]
Bug 16735 - Filter individual libraries from search group pulldown
Comment 55 Kyle M Hall (khall) 2017-08-08 16:42:10 UTC
Created attachment 65664 [details] [review]
Bug 16735 - Filter groups without immediate child libraries from group pulldown
Comment 56 Kyle M Hall (khall) 2017-08-08 16:42:14 UTC
Created attachment 65665 [details] [review]
Bug 16735 - Use libraries in all subgroups, not just immediate children
Comment 57 Jonathan Druart 2017-12-05 17:13:46 UTC
Created attachment 69533 [details] [review]
Bug 16735 - Remove modules no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 58 Jonathan Druart 2017-12-05 17:13:52 UTC
Created attachment 69534 [details] [review]
Bug 16735 - Remove tables no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 59 Jonathan Druart 2017-12-05 17:13:57 UTC
Created attachment 69535 [details] [review]
Bug 16735 - Clean up sample data

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 60 Jonathan Druart 2017-12-05 17:14:04 UTC
Created attachment 69536 [details] [review]
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system

Test Plan:
1) Apply this patch set
2) Note your existing search groups have been ported over to the new
   __SEARCH_GROUPS__ group if you had any
3) Create the group __SEARCH_GROUPS__ if one does not already exist
4) Add some first level subgroups to this group, add libraries to those groups
5) Search the library group searching in the intranet and opac
6) Note you get the same results as pre-patch

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 61 Jonathan Druart 2017-12-05 17:14:09 UTC
Created attachment 69537 [details] [review]
Bug 16735 - Don't use objects for database update
Comment 62 Jonathan Druart 2017-12-05 17:14:14 UTC
Created attachment 69538 [details] [review]
Bug 16735 - Remove use of get_categories

Feature using it is completely undocumented as far as my research has
shown.
Comment 63 Jonathan Druart 2017-12-05 17:14:19 UTC
Created attachment 69539 [details] [review]
Bug 16735 - Filter individual libraries from search group pulldown
Comment 64 Jonathan Druart 2017-12-05 17:14:25 UTC
Created attachment 69540 [details] [review]
Bug 16735: DBIC Schema files
Comment 65 Jonathan Druart 2017-12-05 17:14:30 UTC
Created attachment 69541 [details] [review]
Bug 16735 - Use libraries in all subgroups, not just immediate children
Comment 66 Jonathan Druart 2017-12-05 17:35:35 UTC
Patches have been rebased.
Comment 67 Tomás Cohen Arazi (tcohen) 2017-12-11 16:11:18 UTC
Created attachment 69684 [details] [review]
Bug 16735: Remove modules no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 68 Tomás Cohen Arazi (tcohen) 2017-12-11 16:11:24 UTC
Created attachment 69685 [details] [review]
Bug 16735: Remove tables no longer needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 69 Tomás Cohen Arazi (tcohen) 2017-12-11 16:11:32 UTC
Created attachment 69686 [details] [review]
Bug 16735: Clean up sample data

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 70 Tomás Cohen Arazi (tcohen) 2017-12-11 16:11:43 UTC
Created attachment 69687 [details] [review]
Bug 16735: Migrate library search groups into the new hierarchical groups

Test Plan:
1) Apply this patch set
2) Note your existing search groups have been ported over to the new
   __SEARCH_GROUPS__ group if you had any
3) Create the group __SEARCH_GROUPS__ if one does not already exist
4) Add some first level subgroups to this group, add libraries to those groups
5) Search the library group searching in the intranet and opac
6) Note you get the same results as pre-patch

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 71 Tomás Cohen Arazi (tcohen) 2017-12-11 16:11:56 UTC
Created attachment 69688 [details] [review]
Bug 16735: Don't use objects for database update

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 72 Tomás Cohen Arazi (tcohen) 2017-12-11 16:12:05 UTC
Created attachment 69689 [details] [review]
Bug 16735: Remove use of get_categories

Feature using it is completely undocumented as far as my research has
shown.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 73 Tomás Cohen Arazi (tcohen) 2017-12-11 16:12:14 UTC
Created attachment 69690 [details] [review]
Bug 16735: Filter individual libraries from search group pulldown

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 74 Tomás Cohen Arazi (tcohen) 2017-12-11 16:12:25 UTC
Created attachment 69691 [details] [review]
Bug 16735: DBIC Schema files

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 75 Tomás Cohen Arazi (tcohen) 2017-12-11 16:12:32 UTC
Created attachment 69692 [details] [review]
Bug 16735: Use libraries in all subgroups, not just immediate children

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 76 Tomás Cohen Arazi (tcohen) 2017-12-11 16:12:38 UTC
Created attachment 69693 [details] [review]
Bug 16735: (QA followup) POD fixes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 77 Tomás Cohen Arazi (tcohen) 2017-12-11 16:29:54 UTC
Created attachment 69694 [details] [review]
Bug 16735: (QA followup) Fix rebase error

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 78 Alex Buckley 2017-12-11 18:31:45 UTC
I have tried to apply these patches as they are a dependency for the patches on bug 19765 however I have found that the patches attached to this  bug report (16735) not longer cleanly apply on an up to date branch. 

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 16735: Remove modules no longer needed
Applying: Bug 16735: Remove tables no longer needed
Applying: Bug 16735: Clean up sample data
Applying: Bug 16735: Migrate library search groups into the new hierarchical groups
fatal: sha1 information is lacking or useless (C4/Auth.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 16735: Migrate library search groups into the new hierarchical groups
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-16735-Migrate-library-search-groups-into-the-n-yOLkh7.patch
Comment 79 Tomás Cohen Arazi (tcohen) 2017-12-11 18:45:51 UTC
It applies cleanly if you have the dependencies in your branch.

This is latest master. I've applied the dependent bug (15707) manually so it is more readable:

tcohen@bibliotecas-1:koha(qa_16735)$ git reset --hard origin/master 
HEAD is now at 03dd826757 Bug 17182: (QA follow-up) Fix call to GetMarcBiblio
tcohen@bibliotecas-1:koha(qa_16735)$ git bz apply 15707
Bug 15707 - Add ability to define hierarchical groups of libraries

69573 - Bug 15707: Update DB
69574 - Bug 15707: Add Koha::Library::Group(s) modules
69575 - Bug 15707: Add library groups editor
69576 - Bug 15707: Unit Tests
69577 - Bug 15707: Fix conflict with bug 15446 (type vs _type)
69578 - Bug 15707: (QA followup) Switch to treetable which Koha already uses
69579 - Bug 15707: (QA followup) Drop table if exists
69580 - Bug 15707: (QA followup) Fix UI issues
69581 - Bug 15707: (QA followup) Remove class for treegrid
69582 - Bug 15707: (QA followup) Allow object names to be styled without impeding translation
69583 - Bug 15707: (followup) use cat-search instead of cities-search in header
69584 - Bug 15707: Switch datetimes to timestamps
69585 - Bug 15707: Add UNIQUE constraint to library groups table
69586 - Bug 15707: Updates for Bootstrap 3
69587 - Bug 15707: Display error if group title is already used
69588 - Bug 15707: DBIC Schema files
69589 - Bug 15707: (QA followup) Style buttons correctly

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 15707: Update DB
Applying: Bug 15707: Add Koha::Library::Group(s) modules
Applying: Bug 15707: Add library groups editor
Applying: Bug 15707: Unit Tests
Applying: Bug 15707: Fix conflict with bug 15446 (type vs _type)
Applying: Bug 15707: (QA followup) Switch to treetable which Koha already uses
Applying: Bug 15707: (QA followup) Drop table if exists
Applying: Bug 15707: (QA followup) Fix UI issues
Applying: Bug 15707: (QA followup) Remove class for treegrid
Applying: Bug 15707: (QA followup) Allow object names to be styled without impeding translation
Applying: Bug 15707: (followup) use cat-search instead of cities-search in header
Applying: Bug 15707: Switch datetimes to timestamps
Applying: Bug 15707: Add UNIQUE constraint to library groups table
Applying: Bug 15707: Updates for Bootstrap 3
Applying: Bug 15707: Display error if group title is already used
Applying: Bug 15707: DBIC Schema files
Applying: Bug 15707: (QA followup) Style buttons correctly
tcohen@bibliotecas-1:koha(qa_16735)$ git bz apply 16735

Bug 16735 Depends on bug 15707
Follow? [(y)es, (n)o] n
Bug 16735 - Replace existing library search groups functionality with the new hierarchical groups system

69684 - Bug 16735: Remove modules no longer needed
69685 - Bug 16735: Remove tables no longer needed
69686 - Bug 16735: Clean up sample data
69687 - Bug 16735: Migrate library search groups into the new hierarchical groups
69688 - Bug 16735: Don't use objects for database update
69689 - Bug 16735: Remove use of get_categories
69690 - Bug 16735: Filter individual libraries from search group pulldown
69691 - Bug 16735: DBIC Schema files
69692 - Bug 16735: Use libraries in all subgroups, not just immediate children
69693 - Bug 16735: (QA followup) POD fixes
69694 - Bug 16735: (QA followup) Fix rebase error

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 16735: Remove modules no longer needed
Applying: Bug 16735: Remove tables no longer needed
Applying: Bug 16735: Clean up sample data
Applying: Bug 16735: Migrate library search groups into the new hierarchical groups
Applying: Bug 16735: Don't use objects for database update
Applying: Bug 16735: Remove use of get_categories
Applying: Bug 16735: Filter individual libraries from search group pulldown
Applying: Bug 16735: DBIC Schema files
Applying: Bug 16735: Use libraries in all subgroups, not just immediate children
Applying: Bug 16735: (QA followup) POD fixes
Applying: Bug 16735: (QA followup) Fix rebase error
tcohen@bibliotecas-1:koha(qa_16735)$
Comment 80 Jonathan Druart 2018-02-02 17:29:46 UTC
Patches do not apply but rebased on https://github.com/joubu/Koha/commits/bug_18403
Comment 81 Jonathan Druart 2018-02-09 14:40:51 UTC
Created attachment 71386 [details] [review]
Bug 16735: Remove unecessary use of Koha::LibraryCategories in onboarding

This was not used, it has been copied/pasted from admin/branches.pl
Comment 82 Jonathan Druart 2018-02-09 20:01:02 UTC
Created attachment 71393 [details] [review]
Bug 16735: Fix POD format for Koha::Library::Groups methods
Comment 83 Jonathan Druart 2018-02-12 18:47:27 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 84 Nick Clemens (kidclamp) 2018-02-16 01:39:38 UTC
Awesome work all! Enhancement, not backported to stable.