Bug 28726

Summary: Add sort1 and sort2 to patron card creator patron search
Product: Koha Reporter: Michael Hafen <michael.hafen>
Component: Label/patron card printingAssignee: Michael Hafen <michael.hafen>
Status: In Discussion --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: e.betemps, jonathan.druart+koha, lucas
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Proposed fix
Bug 28726 - Add sort1 and sort2 to patron card creator - patron search.
Bug 28726 - Add sort1 and sort2 to patron card creator - patron search.
Bug 28726: Add sort1 and sort2 to patron card creator - patron search.
Bug 28726: Add sort1 and sort2 to patron card creator - patron search.
Bug 28726: Add sort1 and sort2 to patron card creator - patron search.

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 sandboxes@biblibre.com 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).