Bug 34147

Still valid in master? I don't recreate.

If you do can you provide what you have in the console and eventually the Koha log if you got a 500?

Is this a duplicate of bug 34145?

I was able to replicate this in a sandbox with these steps:

1. Make sure you are logged in as Centerville
2. Do a patron search for 'Ac' -- there are two patrons: Henry (Staff) and Edna (Patron)
2. Go to Administration > Patron Categories
3. Edit the Patron (PT) category, set the Library limitations to Fairfield only
4. Do the patron search for 'Ac' again - it hangs up with a Processing box

This happens when logged in with the superlibrarian account, and when logged in with an account with view_borrower_infos_from_any_library enabled.

The console shows this error:

Uncaught TypeError: Cannot read properties of undefined (reading 'description')
    at render (member.pl?quicksearch=1&searchmember=Ac&searchfieldstype=standard&branchcode_filter=&categorycode_filter=:3141:90)
    at datatables.min_24.0600014.js:41:12136
    at e.fnGetData (datatables.min_24.0600014.js:41:15516)
    at S (datatables.min_24.0600014.js:41:18049)
    at St (datatables.min_24.0600014.js:41:20531)
    at y (datatables.min_24.0600014.js:41:22994)
    at It (datatables.min_24.0600014.js:41:28350)
    at datatables.min_24.0600014.js:41:26897
    at Object.e [as success] (datatables.min_24.0600014.js:41:25877)
    at c (jquery-3.6.0.min_24.0600014.js:2:28327)

*** Bug 37379 has been marked as a duplicate of this bug. ***

Created attachment 169073 [details] [review]
Bug 34147: Prevent crashing of patron search

This is just working around the problem, I think.
It fixes the crashing issue but the limited patron's category is no longer listed on the table.
I dont fully understand the 'library limitation' functionality, what is the expected functionality here when following the test plan?
Is 'Acosta, Edna' expected to not show up? Since its PT category?

> Is 'Acosta, Edna' expected to not show up? Since its PT category?

I think yes. If there is a library limitation and your are logged into library not included in that limitation you should see no search results for the given patron category.

Historically patron category library limitations only restrict the creation of patrons to libraries for which the limits are set and do not affect patron search results.

In the example, I definitely do still want to see Edna show up. As a consortium with a shared patron database that limits different categories for different branches, we need to be able to see all possible patrons when searching to avoid duplicates, and so patrons from one library can be found when they walk into another library.

The patch seems to work well other than what Pedro mentioned. When following the steps to replicate, the table cell in the "Category" column for Edna Acosta is blank where it should say "Patron".

Our consortia does not share a patron database.  We use the patron category branch limitation to force libraries to only select patron categories that they use regularly.  This isn't the tool we use to prevent one branch from seeing another branches patrons.  For that we use a permission called "view_borrower_infos_from_any_libraries" and when that is not selected it, it prevents one branch from seeing another branches patrons, even if the share a patron category.  This being set is also causing a similar failing in searching as described in bug 37378 - Bob Bennhoff

Bug 37368 has a note that makes me think it might be related - even a fix maybe?

This patch adds the missing library embed from the patron search forms.
This embed is required for the case where a patron is missing both first
and last name or is from a different library group and thus falls into
the 'Patron from library X' block.

Created attachment 169417 [details] [review]
Bug 34147: Load all categories for displaying patron results

Currently we are only loading unlimited categories into the patron search, so when we return
patrons from a category limited to another library we crash because we don't have the descriptions
loaded.

This patch removes the limit from 'categories_map' which is only used for displaying patrons. As patron
visibility is a feature of permissions and groups, I don't think we should enforce if based on category
limitations.

NOTE: the categories for searching are still limited in the left hand filters - but not in the dropdown if you click the options button in the top search form. This is beyond the scope of this bug.

To test:
1. Make sure you are logged in as Centerville
2. Do a patron search for 'Ac' -- there are two patrons: Henry (Staff) and Edna (Patron)
2. Go to Administration > Patron Categories
3. Edit the Patron (PT) category, set the Library limitations to Fairfield only
4. Do the patron search for 'Ac' again - it hangs up with a Processing box
5. Apply patch
6. Reload the page (clear cache) and confirm the results now load

(In reply to Katrin Fischer from comment #9)
> Bug 37368 has a note that makes me think it might be related - even a fix
> maybe?
> 
> This patch adds the missing library embed from the patron search forms.
> This embed is required for the case where a patron is missing both first
> and last name or is from a different library group and thus falls into
> the 'Patron from library X' block.

Unfortunately no, this bug is related to the fact that we are not returning the descriptions of limited categories, but returning those patrons. I think allowing all categories to be viewed is correct. As Jason points out, the limits are to restrict the categories used when adding patrons, not to enforce visibility of patrons

Created attachment 169421 [details] [review]
Bug 34147: Load all categories for displaying patron results

Currently we are only loading unlimited categories into the patron search, so when we return
patrons from a category limited to another library we crash because we don't have the descriptions
loaded.

This patch removes the limit from 'categories_map' which is only used for displaying patrons. As patron
visibility is a feature of permissions and groups, I don't think we should enforce if based on category
limitations.

NOTE: the categories for searching are still limited in the left hand filters - but not in the dropdown if you click the options button in the top search form. This is beyond the scope of this bug.

To test:
1. Make sure you are logged in as Centerville
2. Do a patron search for 'Ac' -- there are two patrons: Henry (Staff) and Edna (Patron)
2. Go to Administration > Patron Categories
3. Edit the Patron (PT) category, set the Library limitations to Fairfield only
4. Do the patron search for 'Ac' again - it hangs up with a Processing box
5. Apply patch
6. Reload the page (clear cache) and confirm the results now load

Signed-off-by: Jason Robb <jrobb@sekls.org>

Created attachment 169559 [details] [review]
Bug 34147: Load all categories for displaying patron results

Currently we are only loading unlimited categories into the patron search, so when we return
patrons from a category limited to another library we crash because we don't have the descriptions
loaded.

This patch removes the limit from 'categories_map' which is only used for displaying patrons. As patron
visibility is a feature of permissions and groups, I don't think we should enforce if based on category
limitations.

NOTE: the categories for searching are still limited in the left hand filters - but not in the dropdown if you click the options button in the top search form. This is beyond the scope of this bug.

To test:
1. Make sure you are logged in as Centerville
2. Do a patron search for 'Ac' -- there are two patrons: Henry (Staff) and Edna (Patron)
2. Go to Administration > Patron Categories
3. Edit the Patron (PT) category, set the Library limitations to Fairfield only
4. Do the patron search for 'Ac' again - it hangs up with a Processing box
5. Apply patch
6. Reload the page (clear cache) and confirm the results now load

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>

Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant

Backported to 24.05.x for upcoming 24,05.04

Please provide tests when you are modifying the patrons search code. We have a selenium tests for that (t/db_dependent/selenium/patrons_search.t).

> As patron visibility is a feature of permissions and groups, I don't think we should enforce if based on category limitations.

> NOTE: the categories for searching are still limited in the left hand filters - but not in the dropdown if you click the options button in the top search form. This is beyond the scope of this bug.

Then what? We need to follow-up here.

Is there a consensus on the desired behaviour? IIRC the original goal of this "library limitation" at category level was to completely restrict the access. Since we now have the library groups, does it mean we should get rid of it?

(In reply to Jonathan Druart from comment #17)
> > As patron visibility is a feature of permissions and groups, I don't think we should enforce if based on category limitations.
> 
> > NOTE: the categories for searching are still limited in the left hand filters - but not in the dropdown if you click the options button in the top search form. This is beyond the scope of this bug.
> 
> Then what? We need to follow-up here.
> 
> Is there a consensus on the desired behaviour? IIRC the original goal of
> this "library limitation" at category level was to completely restrict the
> access. Since we now have the library groups, does it mean we should get rid
> of it?

I am not sure if the assumption is correct about the original goal. I think it's more: you cannot "use" the category as a restricted library. This means you cannot add patrons with this category for your library. The whole purpose of the feature is not always clear in all areas.

But we already had the issue where libraries can see and edit patrons of other libraries and had to work around that to avoid unintentional data loss on editing these patrons. 

So for the "Add" buttons we should definitely enforce library limitations.
For the pull down in search I think it's also helpful, but more for usability - the library restrictions can be used to "shorten" the pull downs to what you actually use.
But if a datatable has patrons I can see with library categories I can't use, I think having them in the filter would not hurt?