Bug 28726 - Add sort1 and sort2 to patron card creator patron search
Summary: Add sort1 and sort2 to patron card creator patron search
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Michael Hafen
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 34094 34209 34233 34592
  Show dependency treegraph
 
Reported: 2021-07-20 18:17 UTC by Michael Hafen
Modified: 2023-09-13 20:38 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08


Attachments
Proposed fix (9.88 KB, patch)
2021-07-20 18:17 UTC, Michael Hafen
Details | Diff | Splinter Review
Bug 28726 - Add sort1 and sort2 to patron card creator - patron search. (9.89 KB, patch)
2022-01-07 20:32 UTC, The Minh Luong
Details | Diff | Splinter Review
Bug 28726 - Add sort1 and sort2 to patron card creator - patron search. (11.96 KB, patch)
2022-04-26 23:30 UTC, Michael Hafen
Details | Diff | Splinter Review
Bug 28726: Add sort1 and sort2 to patron card creator - patron search. (802 bytes, patch)
2022-07-18 22:56 UTC, Michael Hafen
Details | Diff | Splinter Review
Bug 28726: Add sort1 and sort2 to patron card creator - patron search. (11.18 KB, patch)
2022-08-03 22:11 UTC, Michael Hafen
Details | Diff | Splinter Review
Bug 28726: Add sort1 and sort2 to patron card creator - patron search. (11.28 KB, patch)
2022-08-05 07:28 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 28726: Add sort1 and sort2 to patron card creator - patron search. (12.14 KB, patch)
2022-10-05 23:20 UTC, Michael Hafen
Details | Diff | Splinter Review
Bug 28726: Add sort1 and sort2 to patron card creator - patron search. (12.17 KB, patch)
2022-11-10 13:17 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 28726: Add sort1 and sort2 to patron card creator - patron search. (10.11 KB, patch)
2023-02-28 00:05 UTC, Michael Hafen
Details | Diff | Splinter Review
Bug 28726: Add sort1 and sort2 to patron card creator - patron search. (10.12 KB, patch)
2023-03-21 17:30 UTC, solene.ngamga
Details | Diff | Splinter Review
Bug 28726: Add sort1 and sort2 to patron card creator - patron search. (10.22 KB, patch)
2023-05-04 11:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Hafen 2021-07-20 18:17:08 UTC

    
Comment 1 Michael Hafen 2021-07-20 18:17:55 UTC
Created attachment 122991 [details] [review]
Proposed fix
Comment 2 Lucas Gass 2021-07-31 20:39:34 UTC
Hey Micheal,

Can you provide a test plan for testing this patch?
Comment 3 Michael Hafen 2021-08-02 23:47:25 UTC
Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator -> New -> card batch)
2. click the Add patron(s) button.  Observe that Category and Library are the only options.
3. Close search for patron window.
4. Apply patch.
5. click the Add patron(s) button.  Observe that you can now search for patrons by their sort1 and sort2 values.
6. perform a search and observe the 'Select all | Clear all | Add selected patrons' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card batch's Add by borrowernumber(s) text input field.
9. close the search for patron window.
Comment 4 The Minh Luong 2022-01-07 20:32:27 UTC
Created attachment 129203 [details] [review]
Bug 28726 - Add sort1 and sort2 to patron card creator - patron search.

Also adds a select all / clear all and checkboxes.

Signed-off-by: The Minh Luong <the-minh.luong@inlibro.com>
Comment 5 Michael Hafen 2022-04-26 23:30:38 UTC
Created attachment 134003 [details] [review]
Bug 28726 - Add sort1 and sort2 to patron card creator - patron search.

Add sort1 and sort2 filters, checkboxes and select/clear all links, and a add selected button to the patron search (for the patron card creator).

Rebase after change to how the patron search works.

Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator -> New -> card batch)
2. click the Add patron(s) button.  Observe that Category and Library are the only options.
3. Close search for patron window.
4. Apply patch.
5. click the Add patron(s) button.  Observe that you can now search for patrons by their sort1 and sort2 values.
6. perform a search and observe the 'Select all | Clear all | Add selected patrons' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card batch's Add by borrowernumber(s) text input field.
9. close the search for patron window.
Comment 6 Katrin Fischer 2022-07-17 10:32:22 UTC
Hi Michaeal, could you please rebase against the master branch?

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 28726: Add sort1 and sort2 to patron card creator - patron search.
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc).
error: could not build fake ancestor
Patch failed at 0001 Bug 28726: Add sort1 and sort2 to patron card creator - patron search.
hint: Use 'git am --show-current-patch=diff' to see the failed 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-28726---Add-sort1-and-sort2-to-patron-card-cre-61BF84.patch
Comment 7 Michael Hafen 2022-07-18 22:56:49 UTC
Created attachment 137854 [details] [review]
Bug 28726: Add sort1 and sort2 to patron card creator -  patron search.

[ Rebased 2022-07-18 ]
Comment 8 Katrin Fischer 2022-07-24 21:01:23 UTC
Please don't forget to switch back the status when you rebase :)
Comment 9 Emmanuel Bétemps 2022-07-28 12:17:47 UTC
I got this message after applying the patch (which is not displayed in the KohaGitLog)
(I test using a sandbox)

-------------------------------------------
Bug 28726 - Add sort1 and sort2 to patron card creator patron search

137854 - Bug 28726: Add sort1 and sort2 to patron card creator -  patron search.

Apply? [(y)es, (n)o, (i)nteractive] Traceback (most recent call last):
  File "/usr/bin/git-bz", line 2716, in <module>
    applied = do_apply(bug_ref)
  File "/usr/bin/git-bz", line 1827, in do_apply
    users = bug.server.get_xmlrpc_proxy().User.get({ 'names': [patch.attacher] })['users']
  File "/usr/lib/python2.7/xmlrpclib.py", line 1243, in __call__
    return self.__send(self.__name, args)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1602, in __request
    verbose=self.__verbose
  File "/usr/lib/python2.7/xmlrpclib.py", line 1283, in request
    return self.single_request(host, handler, request_body, verbose)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1331, in single_request
    response.msg,
xmlrpclib.ProtocolError: <ProtocolError for bugs.koha-community.org/xmlrpc.cgi: 404 Not Found>
Comment 10 Michael Hafen 2022-07-28 14:58:29 UTC
(In reply to Emmanuel Bétemps from comment #9)
> I got this message after applying the patch (which is not displayed in the
> KohaGitLog)
> (I test using a sandbox)
> 
> -------------------------------------------
> Bug 28726 - Add sort1 and sort2 to patron card creator patron search
> 
> 137854 - Bug 28726: Add sort1 and sort2 to patron card creator -  patron
> search.
> 
> Apply? [(y)es, (n)o, (i)nteractive] Traceback (most recent call last):
>   File "/usr/bin/git-bz", line 2716, in <module>
>     applied = do_apply(bug_ref)
>   File "/usr/bin/git-bz", line 1827, in do_apply
>     users = bug.server.get_xmlrpc_proxy().User.get({ 'names':
> [patch.attacher] })['users']
>   File "/usr/lib/python2.7/xmlrpclib.py", line 1243, in __call__
>     return self.__send(self.__name, args)
>   File "/usr/lib/python2.7/xmlrpclib.py", line 1602, in __request
>     verbose=self.__verbose
>   File "/usr/lib/python2.7/xmlrpclib.py", line 1283, in request
>     return self.single_request(host, handler, request_body, verbose)
>   File "/usr/lib/python2.7/xmlrpclib.py", line 1331, in single_request
>     response.msg,
> xmlrpclib.ProtocolError: <ProtocolError for
> bugs.koha-community.org/xmlrpc.cgi: 404 Not Found>

To be that looks like a problem with the koha-community.org git server.
Comment 11 Katrin Fischer 2022-07-29 07:33:47 UTC
Hi Emmanuel, what kind of environment are you using for testing?
Comment 12 Katrin Fischer 2022-07-29 07:55:12 UTC
Ok, I have the same error. Chris said it could be related to git-bz not working with ipv6.
Comment 13 Katrin Fischer 2022-07-29 08:10:12 UTC
(In reply to Katrin Fischer from comment #12)
> Ok, I have the same error. Chris said it could be related to git-bz not
> working with ipv6.

Ok, we did some more testing and git-bz works on other bugs. I am not sure what's causing the problem here, but it appears to be bug specific and so probably not network related.
Comment 14 Katrin Fischer 2022-07-29 08:11:41 UTC
Michael, please check the attache file, this doesn't appear to be a correct patch file, but only the commit message.
Comment 15 Michael Hafen 2022-08-03 22:11:34 UTC
Created attachment 138552 [details] [review]
Bug 28726: Add sort1 and sort2 to patron card creator -  patron search.

Re-upload (2022-08-03).  Don't know how the previous one was uploaded as just the commit message.

Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator ->
 New -> card batch)
2. click the Add patron(s) button.  Observe that Category and Library are the 
 only options.
3. Close search for patron window.
4. Apply patch.
5. click the Add patron(s) button.  Observe that you can now search for patrons
 by their sort1 and sort2 values.
6. perform a search and observe the 'Select all | Clear all | 
 Add selected patrons' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card
 batch's Add by borrowernumber(s) text input field.
9. close the search for patron window.
Comment 16 Biblibre Sandboxes 2022-08-05 07:28:23 UTC
Created attachment 138653 [details] [review]
Bug 28726: Add sort1 and sort2 to patron card creator - patron search.

Also adds a select all / clear all and checkboxes.

Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator -> New -> card batch)
2. click the Add patron(s) button.  Observe that Category and Library are the only options.
3. Close search for patron window.
4. Apply patch.
5. click the Add patron(s) button.  Observe that you can now search for patrons by their sort1 and sort2 values.
6. perform a search and observe the 'Select all | Clear all | Add selected patrons' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card batch's Add by borrowernumber(s) text input field.
9. close the search for patron window.

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Comment 17 Jonathan Druart 2022-08-09 09:24:43 UTC
+    @sort1 = sort {$a cmp $b} uniq( Koha::Patrons->search($sort_filter)->get_column('sort1') );

This is terrible if you have lot of patrons. You should at least do the unique (distinct) at the DBMS level.

What about the following suggestion: you could display sort1 and sort2 authorised values in the dropdown list, or simply display a search input.
You could also add it to the table's headers (filter at the top of the column).
Comment 18 Katrin Fischer 2022-08-09 09:35:02 UTC
(In reply to Jonathan Druart from comment #17)
> +    @sort1 = sort {$a cmp $b} uniq(
> Koha::Patrons->search($sort_filter)->get_column('sort1') );
> 
> This is terrible if you have lot of patrons. You should at least do the
> unique (distinct) at the DBMS level.
> 
> What about the following suggestion: you could display sort1 and sort2
> authorised values in the dropdown list, or simply display a search input.
> You could also add it to the table's headers (filter at the top of the
> column).

sort1 can be free text or an AV. The AV category always exists, but maybe we could check for existing values. If values exist: show a drop down with the values. If there are no entries for the AV, use a free text input field. That would match the behavior on the patron edit form.
Comment 19 Jonathan Druart 2022-08-09 09:58:15 UTC
Thanks Katrin, that's exactly what I wanted to say but some parts got lost when I rephrased the comment apparently.
Comment 20 Katrin Fischer 2022-08-12 14:36:06 UTC
Hi Michaeal,

could you take a look at changing the form as suggested in comment#19?

The patron edit form could be a good starting place for this.

I realize there is another place in Koha at least where it's done like you do here - the statistics wizard for patrons - but I believe it would be worth changing and changing it there too.
I can easily see a library using the field for individual notes.
Comment 21 Katrin Fischer 2022-08-12 14:38:31 UTC
To explain a little more: even with just the sample data the pull down is a little hard to handle. If we get over 100 entries there, it will be much harder.
Comment 22 Michael Hafen 2022-08-12 15:11:55 UTC
I'll take a look, but it will mean a big change to how I'm handling patrons, as my use case is a little different.
I'm in a school district, and all our patron info comes in from LDAP.  We aren't using the Authorized Values categories for the sort fields.  I wasn't even planning on trying to use them since our servers handle building those drop down pretty well as is.  We also have Independant Branches on, so the list gets filtered by branchcode, so I'd also have to see if the Authorized Values categories can do that too.
We have 72079 patrons with 2975 unique sort2 values, but each library has no more than 200 values in the sort2, and it's usually closer to or less than 100 values. Even as the SuperLibrarian, when I load the patron card create patron search window it doesn't take more than a second on my servers, so I've been fine with it that way.
I can see that this isn't optimal, but the alternative is creating a cron script to scrape LDAP to pull all the possible values for those two attributes to sync the Authorized Values categories.

Anyway, I'll have to look at the Authorized Values stuff to see if it can meet our use case.  And I don't mind changing the patch to use that instead, and then changing it again in my local fork to scrape the database instead.
Comment 23 Katrin Fischer 2022-08-12 15:21:22 UTC
I cannot tell about performance, but UX wise, maybe switching to a Select2 style pull down could be a compromise (like the ones used in the edit item form).
Comment 24 Michael Hafen 2022-10-05 23:20:12 UTC
Created attachment 141405 [details] [review]
Bug 28726: Add sort1 and sort2 to patron card creator -  patron search.

[ 2022-10-05 updated for recommendations: AV and select2 ]

Bug 28726: Add sort1 and sort2 to patron card creator - patron search.

Also adds a select all / clear all and checkboxes.

Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator ->
 New -> card batch)
2. click the Add patron(s) button.  Observe that Category and Library are the on
ly options.
3. Close search for patron window.
4. Apply patch.
5. click the Add patron(s) button.  Observe that you can now search for patrons 
by their sort1 and sort2 values.
6. perform a search and observe the 'Select all | Clear all | Add selected patro
ns' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card b
atch's Add by borrowernumber(s) text input field.
9. close the search for patron window.
Comment 25 ByWater Sandboxes 2022-11-10 13:17:42 UTC
Created attachment 143646 [details] [review]
Bug 28726: Add sort1 and sort2 to patron card creator - patron search.

Also adds a select all / clear all and checkboxes.

Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator -> New -> card batch)
2. click the Add patron(s) button.  Observe that Category and Library are the only options.
3. Close search for patron window.
4. Apply patch.
5. click the Add patron(s) button.  Observe that you can now search for patrons by their sort1 and sort2 values.
6. perform a search and observe the 'Select all | Clear all | Add selected patrons' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card batch's Add by borrowernumber(s) text input field.
9. close the search for patron window.

Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Comment 26 Kyle M Hall 2023-02-24 17:43:10 UTC
I don't think we should be pre-populating a select pulldown from sort1/2. These fields could contain unique information, and a library could have tens or hundreds of thousands of patrons. In that case bad things (tm) would happen. I think it would be best of those fields were free text fields.
Comment 27 Katrin Fischer 2023-02-24 18:35:04 UTC
(In reply to Kyle M Hall from comment #26)
> I don't think we should be pre-populating a select pulldown from sort1/2.
> These fields could contain unique information, and a library could have tens
> or hundreds of thousands of patrons. In that case bad things (tm) would
> happen. I think it would be best of those fields were free text fields.

Just to clarify, if AV values exist, would you be ok with populating from the config (authorised values) instead of the data? If the category has no entries, we do freetext, otherwise a pull down.

That matches the behviour in other spots, like the patron edit/display forms and the statistical wizard for patrons.
Comment 28 Michael Hafen 2023-02-24 19:13:40 UTC
The patch right now is trying the Authorized Values first, and if those are empty, then it hits the database.

Kyle, are you suggesting an auto-complete style text input?  I could do that instead, though I'd have to figure out how.  That would still mean a lot of database queries though.

Maybe a (not unique) index on each of those two columns? (assuming we don't already have one.)

I don't think we can't avoid querying the database at this point.  As long as sort1 and sort2 are free entry fields on the member entry page, we can't avoid querying the database to collect those values.  And we need those values in order to use them as a filter when searching members.
Comment 29 Katrin Fischer 2023-02-24 20:14:09 UTC
I think maybe it will be easier to keep it simple and have a text field or a pull down from AV depending on config first. As Kyle said sort fields can have unique values, so you will have an endless pull down with one entry per patron, this is not useful. 

Also: pull down and free text will match how this field is used in all other places in Koha (= existing pattern).

Further improvements could then be made on a follow-up bug.
Comment 30 Michael Hafen 2023-02-24 22:13:53 UTC
Making it a free text field doesn't work for me.  In my opinion having a free text input doesn't work well for a filter field on a search page; especially in my case where the librarians don't control that field, it's controlled by LDAP.

Katrin mentioned the Patron Statistics Wizard.  Looking at that, what's done for the sort2 column is to pull distinct values limited to 200.  That would work for me, that should be well under what my librarians would encounter in that field.

Would doing that instead be acceptable?
Comment 31 Katrin Fischer 2023-02-24 22:18:40 UTC
(In reply to Michael Hafen from comment #30)
> Making it a free text field doesn't work for me.  In my opinion having a
> free text input doesn't work well for a filter field on a search page;
> especially in my case where the librarians don't control that field, it's
> controlled by LDAP.
> 
> Katrin mentioned the Patron Statistics Wizard.  Looking at that, what's done
> for the sort2 column is to pull distinct values limited to 200.  That would
> work for me, that should be well under what my librarians would encounter in
> that field.
> 
> Would doing that instead be acceptable?

It won't work for any library that stored an identifier or unique value in there that has more than 200 patrons. They would see an incomplete pull down. Also: a 200 entry pull down is not really nice to use?

Actually I was not aware the wizard did that :( we should fix it as well. I'll claim the patron form then as the example and the batch patron modification templates (sorry).

Why will a free text not work? If they have no idea what they are looking for, will also not be very helpful? I think it's a specific use case we are dealing with here that I'd like to understand better.
Comment 32 Michael Hafen 2023-02-24 22:29:44 UTC
The patron entry form and the batch patron mod form are both for setting the value, which is a different circumstance from using a value as a filter in a patron search.
For those entry forms having either the AV drop down or a free text input makes sense.
For a patron search it's easier to use a drop down, where the librarian probably want's an exact match to a value used in the database.

In the case of my libraries most of our patron information is controlled by LDAP, and the librarians aren't the ones entering the information in the LDAP Directory.  So they have an idea what to look for, but stuff like capitalization, or whether first name or last name comes first, isn't known.  Or remembering 40 to 90 values is a burden I would spare them from.

Does that explain my use case?
Comment 33 Katrin Fischer 2023-02-24 22:32:01 UTC
We'll need something that works universally. The auto-complete seemed like a good idea, but I'd like Kyle and others to comment on performance concerns.

Koha's tables are case insensitive in the database - Koha doesn't care about it in searches that are SQL based.
Comment 34 Michael Hafen 2023-02-28 00:05:18 UTC
Created attachment 147477 [details] [review]
Bug 28726: Add sort1 and sort2 to patron card creator -  patron search.

[rebased 2023-02-27]
[changed to pass QA - use av-build-dropbox.inc]

Also adds a select all / clear all and checkboxes.

Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator -> New -> card batch)
2. click the Add patron(s) button.  Observe that Category and Library are the only options.
3. Close search for patron window.
4. Apply patch.
5. click the Add patron(s) button.  Observe that you can now search for patrons by their sort1 and sort2 values.
6. perform a search and observe the 'Select all | Clear all | Add selected patrons' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card batch's Add by borrowernumber(s) text input field.
9. close the search for patron window.
Comment 35 Michael Hafen 2023-02-28 00:07:38 UTC
I didn't even realize that the sql search would be case-insensitive.  Now that you mention it, of course it is.

Anyway, here's a patch that just pulls from AV Bsort1 and Bsort2, using av-build-dropbox.inc, so it's either a select or an input.  I can do otherwise on my local fork if it upsets my librarians (which I expect it will).
Comment 36 solene.ngamga 2023-03-21 17:30:09 UTC
Created attachment 148489 [details] [review]
Bug 28726: Add sort1 and sort2 to patron card creator - patron search.

Also adds a select all / clear all and checkboxes.

Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator -> New -> card batch)
2. click the Add patron(s) button.  Observe that Category and Library are the only options.
3. Close search for patron window.
4. Apply patch.
5. click the Add patron(s) button.  Observe that you can now search for patrons by their sort1 and sort2 values.
6. perform a search and observe the 'Select all | Clear all | Add selected patrons' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card batch's Add by borrowernumber(s) text input field.
9. close the search for patron window.

Signed-off-by: Solene Ngamga <solene.ngamga@inLibro.com>
Comment 37 Marcel de Rooy 2023-04-28 06:55:57 UTC
Looking here
Comment 38 Marcel de Rooy 2023-04-28 07:01:33 UTC
If I am adding borrowers to the patron card batch, I am still seeing the box "There are no patrons in this batch" and I have no possibility to go further? Could be that I am missing config data for this module still, but this form should be more helpful. Probably out of scope, but it limits testing of this change.
Comment 39 Michael Hafen 2023-05-01 17:54:04 UTC
Let me try to amend the test plan with more detail.  I was just able to apply this patch to a current master and get through the test plan successfully, but there are details missing from the test plan that could cause confusion if one isn't familiar with the module.

Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator -> New -> card batch)
2. click the Add patron(s) button.  This will open the 'Patron search' window.  Observe that Category and Library are the only options.
3. Close 'Patron search' window.
4. Apply patch.  Restart Plack, etc.
5. click the Add patron(s) button.  This will reopen the 'Patron search' window.  Observe that you can now search for patrons by their sort1 and sort2 values.
6. perform a search using a sort1 or sort2 value that will have multiple results.  Observe the 'Select all | Clear all | Add selected patrons' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card batch's Add by borrowernumber(s) text input field in the main window.
9. close the 'Patron search' window.  Click the Add patron(s) button again to add the patrons to the batch.
Comment 40 Marcel de Rooy 2023-05-04 11:45:30 UTC
(In reply to Michael Hafen from comment #39)

Thanks.

> 8. use the Add selected patrons button to add the selected patrons to the
> card batch's Add by borrowernumber(s) text input field in the main window.

This is the confusing part. Since I clicked on that button before selecting the patrons, it is quite strange that you should click it again to kind of confirm the action you did.
It is outside the scope of this report, but the operation is not intuitive.
Comment 41 Marcel de Rooy 2023-05-04 11:50:06 UTC
Created attachment 150650 [details] [review]
Bug 28726: Add sort1 and sort2 to patron card creator - patron search.

Also adds a select all / clear all and checkboxes.

Test plan:

1. start a new batch in the patron card creator (Tools -> Patron card creator -> New -> card batch)
2. click the Add patron(s) button.  Observe that Category and Library are the only options.
3. Close search for patron window.
4. Apply patch.
5. click the Add patron(s) button.  Observe that you can now search for patrons by their sort1 and sort2 values.
6. perform a search and observe the 'Select all | Clear all | Add selected patrons' links and button.
7. use the Select all link to select all the patrons found by the search.
8. use the Add selected patrons button to add the selected patrons to the card batch's Add by borrowernumber(s) text input field.
9. close the search for patron window.

Signed-off-by: Solene Ngamga <solene.ngamga@inLibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 42 Tomás Cohen Arazi 2023-06-23 13:01:32 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 43 Jonathan Druart 2023-07-04 08:16:00 UTC
This patch is actually adding Sort1 and Sort2 to all patrons searches.
Comment 44 Jonathan Druart 2023-07-04 08:19:05 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2023-07-04 08:21:23 UTC
<div class="searchheader fh-fixedHeader" id="searchheader" style="">

This empty div is visible when I search for patrons in the popup: https://snipboard.io/2zBDoA.jpg
Comment 46 Michael Hafen 2023-07-05 17:21:21 UTC
I'm guessing you are seeing that on the patron search popup from a page other then the patron card creator.  I'll open another bug with a follow up patch to move the whole div into the checkbox column condition.
Comment 47 Martin Renvoize 2023-07-17 13:58:55 UTC
This has wider reaching impact than the title suggests.. and I'm not sure we want to force exposure of these fields for all cases.. as such I'm opting not to backport this one right now (though I am backporting most of the other patron search improvements as they go a long way towards fixing the bugs in patron search people have been highlighting when switching to 22.11.x)
Comment 48 Martin Renvoize 2023-07-17 15:04:05 UTC
Needed for simple backport of 34094 and the tree above it.
Comment 49 Matt Blenkinsop 2023-07-18 13:11:54 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x