[Bug 15812] New: Checkout search with too many results (single character search) causes poor performance or timeout
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Bug ID: 15812 Summary: Checkout search with too many results (single character search) causes poor performance or timeout Change sponsored?: --- Product: Koha Version: 3.22 Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com On a system with many patrons a checkout search for a single character (2,X,J) can cause a slow search or timeout from the large number of results returned. Patron search has no problem with the same searches -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal Priority|P5 - low |P3 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Heather Braum <hbraum@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hbraum@nekls.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |abl@biblos.pk.edu.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |enhancement Version|3.22 |master CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Hi Nick, It's because this patron list contains all patrons matching the search. It'd be quite easy to make it use DT server-side processing, but it's more a new enhancement than a bug. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Jesse Weaver <jweaver@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jweaver@bywatersolutions.co | |m Severity|enhancement |normal --- Comment #2 from Jesse Weaver <jweaver@bywatersolutions.com> --- Marking this as a bug due to its potential to crash servers. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 George Williams (NEKLS) <george@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |george@nekls.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 George Williams (NEKLS) <george@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|gwilliams@nekls.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125224 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125224&action=edit Bug 15812: Use patron search in place of circ search The 'Checkout' search hijacks some of the DataTables searching code used for 'Search patrons' Rather than try to implement the search again on another page, we can simply send the user to the patron search if the cardnumber is not found Additionally, this patch adds a 'Check out' button to the patron search results to allow going to checkotus directly To test: 1 - Apply patch 2 - Perform a 'Checkout' search from the header 3 - Note that: For a cardnumber, you are redirected directly to checkouts page for the borrower For a search with one result, you are redirected directly to the checkout page for the borrower For a search with many results, you are redirected to the patron search results and there is a 'Checkout' button under the cardnumber 4 - Confirm circulation page works as expected (i.e. checkout to a patron) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Note: while changes to circulation.tt appear large, it is mostly a whitespace change to fix indentation after removal of conditional on 'borrowers' -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125224|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 125234 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125234&action=edit Bug 15812: Use patron search in place of circ search The 'Checkout' search hijacks some of the DataTables searching code used for 'Search patrons' Rather than try to implement the search again on another page, we can simply send the user to the patron search if the cardnumber is not found Additionally, this patch adds a 'Check out' button to the patron search results to allow going to checkotus directly To test: 1 - Apply patch 2 - Perform a 'Checkout' search from the header 3 - Note that: For a cardnumber, you are redirected directly to checkouts page for the borrower For a search with one result, you are redirected directly to the checkout page for the borrower For a search with many results, you are redirected to the patron search results and there is a 'Checkout' button under the cardnumber 4 - Confirm circulation page works as expected (i.e. checkout to a patron) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29104 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #6 from Fridolin Somers <fridolin.somers@biblibre.com> --- Nice change, I test it.
For a search with many results, you are redirected to the patron search results and there is a 'Checkout' button under the cardnumber I see this also when searching via 'Seach patrons' form. And looks like permission check is needed there right ?
In my opinion this button should be with 'Edit' in actions column. When searching for patron one can assume cardnumber is unknown so button being next to it is not really useful. Note than cardnumber column may be hidden. Patron may be identified by name so when clicking on it we may go to circulation page. Best regards, -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 sandboxes@biblibre.com <sandboxes@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125234|0 |1 is obsolete| | --- Comment #7 from sandboxes@biblibre.com <sandboxes@biblibre.com> --- Created attachment 125413 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125413&action=edit Bug 15812: Use patron search in place of circ search The 'Checkout' search hijacks some of the DataTables searching code used for 'Search patrons' Rather than try to implement the search again on another page, we can simply send the user to the patron search if the cardnumber is not found Additionally, this patch adds a 'Check out' button to the patron search results to allow going to checkotus directly To test: 1 - Apply patch 2 - Perform a 'Checkout' search from the header 3 - Note that: For a cardnumber, you are redirected directly to checkouts page for the borrower For a search with one result, you are redirected directly to the checkout page for the borrower For a search with many results, you are redirected to the patron search results and there is a 'Checkout' button under the cardnumber 4 - Confirm circulation page works as expected (i.e. checkout to a patron) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: George Williams <george@nekls.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 George Williams (NEKLS) <george@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #8 from George Williams (NEKLS) <george@nekls.org> --- Tested on the biblibre sandboxes and the test plan goes as expected. Nice fix. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29134 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Signed Off |Patch doesn't apply --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Sorry guys, patch doesn't apply right now.. could we get a rebase please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125413|0 |1 is obsolete| | --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125489 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125489&action=edit Bug 15812: Use patron search in place of circ search The 'Checkout' search hijacks some of the DataTables searching code used for 'Search patrons' Rather than try to implement the search again on another page, we can simply send the user to the patron search if the cardnumber is not found Additionally, this patch adds a 'Check out' button to the patron search results to allow going to checkotus directly To test: 1 - Apply patch 2 - Perform a 'Checkout' search from the header 3 - Note that: For a cardnumber, you are redirected directly to checkouts page for the borrower For a search with one result, you are redirected directly to the checkout page for the borrower For a search with many results, you are redirected to the patron search results and there is a 'Checkout' button under the cardnumber 4 - Confirm circulation page works as expected (i.e. checkout to a patron) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: George Williams <george@nekls.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125490 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125490&action=edit Bug 15812: (follow-up) Add permission check for 'Check out' button -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #12 from Fridolin Somers <fridolin.somers@biblibre.com> --- Super, permission is OK now. (In reply to Fridolin Somers from comment #6)
Note than cardnumber column may be hidden.
Isn't that a problem ? Also for consistency, everywhere in tables buttons are in actions column. But it is minor. It is a great change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This is a lovely improvement... it simplifies and reduces code and adds direct access to checkouts from patron results page. I'd love to see a follow-on bug to do the same for holds searching and then we can ditch some more code :). However, I do think Fridolin raises a good point about the 'cardnumber' field being one you can hide which could cause confusion. The button styling isn't complete and I agree it's a little jarring to see it outside of the 'actions' field. I'd be tempted move the button to the actions column and perhaps update the cardnumber text to be a link directly to checkout (with a tooltip preferably). If you think that would fulfil all your customers requirements, I'd be happy to make that change in a follow-up Nick? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125489|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125580 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125580&action=edit Bug 15812: Use patron search in place of circ search The 'Checkout' search hijacks some of the DataTables searching code used for 'Search patrons' Rather than try to implement the search again on another page, we can simply send the user to the patron search if the cardnumber is not found Additionally, this patch adds a 'Check out' button to the patron search results to allow going to checkotus directly To test: 1 - Apply patch 2 - Perform a 'Checkout' search from the header 3 - Note that: For a cardnumber, you are redirected directly to checkouts page for the borrower For a search with one result, you are redirected directly to the checkout page for the borrower For a search with many results, you are redirected to the patron search results and there is a 'Checkout' button under the cardnumber 4 - Confirm circulation page works as expected (i.e. checkout to a patron) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: George Williams <george@nekls.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125490|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125582 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125582&action=edit Bug 15812: (follow-up) Add permission check for 'Check out' button Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125583 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125583&action=edit Bug 15812: (follow-up) Tweaks to patron results display Move the button into the actions column and make 'cardnumber' a link to checkout, with a tooltip. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Code speaks.. let me know what you think of that final followup. Otherwise, I'm happy to PQA :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- Whaou for me it is awesome. Thanks a lot Martin ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125583|0 |1 is obsolete| | --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125597 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125597&action=edit Bug 15812: (follow-up) Tweaks to patron results display Move the button into the actions column and make 'cardnumber' a link to checkout, with a tooltip. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Last little tweak made the action button prettier ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #21 from Owen Leonard <oleonard@myacpl.org> --- I like the change to the buttons but I'm sorry to see Bug 28081 undone. I'd like to argue that the link is redundant since the button has been added. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Good catch Owen.. I wasn't aware of the label stuff.. I thought it had been added here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125604 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125604&action=edit Bug 15812: (follow-up) Enable click row to select This patch adds handling to allow clicking anywhere in the table row to select/deselect the patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This patch replaces the release notes| |special case patron results | |page from circulation | |searches and instead | |redirects to the standard | |patron search results page. | | | |To enable quick onward | |navigation to checkout, we | |add a link to the | |cardnumber field and a | |button to the actions | |column. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125621 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125621&action=edit Bug 15812: (follow-up) Enable click cell to select This patch adds handling to allow clicking anywhere in the table cell to select/deselect the patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125621|0 |1 is obsolete| | --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125622 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125622&action=edit Bug 15812: (follow-up) Enable click cell to select This patch adds handling to allow clicking anywhere in the table cell to select/deselect the patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125604|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, I think we're all done now. I restored the label, but made it a visually hidden one inside the check column for accessibility and I made the whole table cell clickable for the checkbox to restore the larger click handling area. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125622|0 |1 is obsolete| | --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 125623 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125623&action=edit Bug 15812: (follow-up) Enable click cell to select This patch adds handling to allow clicking anywhere in the table cell to select/deselect the patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125623|0 |1 is obsolete| | --- Comment #28 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 125624 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125624&action=edit Bug 15812: (follow-up) Enable click cell to select This patch adds handling to allow clicking anywhere in the table cell to select/deselect the patron Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joonas.kylmala@iki.fi --- Comment #29 from Joonas Kylmälä <joonas.kylmala@iki.fi> --- (In reply to Martin Renvoize from comment #26)
OK, I think we're all done now.
I restored the label, but made it a visually hidden one inside the check column for accessibility and I made the whole table cell clickable for the checkbox to restore the larger click handling area.
Hmm, I'm not able to do this still even with this patch (firefox esr on debian 11). Could you double-check it works and if it does could you explain me how to test more specifically. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Trivial patch |Medium patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29193 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125580|0 |1 is obsolete| | Attachment #125582|0 |1 is obsolete| | Attachment #125597|0 |1 is obsolete| | Attachment #125624|0 |1 is obsolete| | --- Comment #30 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125957 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125957&action=edit Bug 15812: Use patron search in place of circ search The 'Checkout' search hijacks some of the DataTables searching code used for 'Search patrons' Rather than try to implement the search again on another page, we can simply send the user to the patron search if the cardnumber is not found Additionally, this patch adds a 'Check out' button to the patron search results to allow going to checkotus directly To test: 1 - Apply patch 2 - Perform a 'Checkout' search from the header 3 - Note that: For a cardnumber, you are redirected directly to checkouts page for the borrower For a search with one result, you are redirected directly to the checkout page for the borrower For a search with many results, you are redirected to the patron search results and there is a 'Checkout' button under the cardnumber 4 - Confirm circulation page works as expected (i.e. checkout to a patron) Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: George Williams <george@nekls.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #31 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125958 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125958&action=edit Bug 15812: (follow-up) Add permission check for 'Check out' button Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #32 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125959 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125959&action=edit Bug 15812: (follow-up) Tweaks to patron results display Move the button into the actions column and make 'cardnumber' a link to checkout, with a tooltip. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #33 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125960 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125960&action=edit Bug 15812: (follow-up) Enable click cell to select This patch adds handling to allow clicking anywhere in the table cell to select/deselect the patron Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #34 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Joonas Kylmälä from comment #29)
(In reply to Martin Renvoize from comment #26)
OK, I think we're all done now.
I restored the label, but made it a visually hidden one inside the check column for accessibility and I made the whole table cell clickable for the checkbox to restore the larger click handling area.
Hmm, I'm not able to do this still even with this patch (firefox esr on debian 11). Could you double-check it works and if it does could you explain me how to test more specifically.
Works for me in Firefox - I misread on my first test and thought it was supposed to be the whole row, but it is just clicking inside the first cell, but outside the checkbox will still check the box Tested and signed off Martin's two attachments, so moving to PQA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #35 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Is there a good reason (apart from laziness) to redirect to member.pl from circulation.pl? 229 print $query->redirect( "/cgi-bin/koha/members/member.pl?quicksearch=1&circsearch=1&searchmember=" . $findborrower ); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Passed QA --- Comment #36 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #35)
Is there a good reason (apart from laziness) to redirect to member.pl from circulation.pl?
229 print $query->redirect( "/cgi-bin/koha/members/member.pl?quicksearch=1&circsearch=1&searchmember=" . $findborrower );
The idea was to avoid a redirect when a single patron was searched for using a cardnumber. If we don't need a search we can proceed with circulation directly, if we need to search, then we redirect to search -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #37 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I won't block it more, but I don't think it's the best implementation. I do like the "Check out" on the patron list. However I don't understand why you didn't go the usual way, which is using common/patron_search.tt and svc/members/search. It was a bit more code but we would have avoided the double redirect when a single patron matches the search (and it's not a cardnumber). Which is certainly the most common use case (after using the cardnumber). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #38 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29261 --- Comment #39 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- In members/tables/members_results.tt: title='View' This is bad, it breaks translated template. Strings must be surrounded by double quote. However here we will have to escape it with \ and the string won't be picked by the translate script. See bug 29261. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29261 [Bug 29261] Translation script breaks members/tables/members_results.tt -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #40 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #39)
In members/tables/members_results.tt:
title='View'
This is bad, it breaks translated template. Strings must be surrounded by double quote. However here we will have to escape it with \ and the string won't be picked by the translate script.
See bug 29261.
Damn, that's my mistake in a follow-up.. apologies. Hmm, not sure what the resolution is. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #41 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #39)
However here we will have to escape it with \ and the string won't be picked by the translate script.
This is not correct, it actually works. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #42 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #37)
I won't block it more, but I don't think it's the best implementation. I do like the "Check out" on the patron list. However I don't understand why you didn't go the usual way, which is using common/patron_search.tt and svc/members/search.
It was a bit more code but we would have avoided the double redirect when a single patron matches the search (and it's not a cardnumber). Which is certainly the most common use case (after using the cardnumber).
Nick? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29136 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #43 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #42)
(In reply to Jonathan Druart from comment #37)
I won't block it more, but I don't think it's the best implementation. I do like the "Check out" on the patron list. However I don't understand why you didn't go the usual way, which is using common/patron_search.tt and svc/members/search.
It was a bit more code but we would have avoided the double redirect when a single patron matches the search (and it's not a cardnumber). Which is certainly the most common use case (after using the cardnumber).
Nick?
I am not against this idea - this seemed a straightforward path and eliminated a problem so I went this way. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #44 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- circ-patron-search-results.inc is now only used in one last place and I think it could be deprecated in favour of the approach here.. I'll post a bug for that. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #45 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #39)
In members/tables/members_results.tt:
title='View'
This is bad, it breaks translated template. Strings must be surrounded by double quote. However here we will have to escape it with \ and the string won't be picked by the translate script.
See bug 29261.
Same for title='Checkout'! will fix on bug 29261 as well -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to master |RESOLVED CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #46 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Comment on attachment 125960 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125960 Bug 15812: (follow-up) Enable click cell to select Review of attachment 125960: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=15812&attachment=125960) ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/modules/members/tables/members_results.tt @@ +13,4 @@
{ [% IF CAN_user_borrowers_edit_borrowers || CAN_user_tools_manage_patron_lists %] "dt_borrowernumber": + "<label for='check[% data.borrowernumber | html %]' class='content_hidden'>Select patron</label><input type='checkbox' id='check[% data.borrowernumber | html %]' class='selection' name='borrowernumber' value='[% data.borrowernumber | html %]' />",
This is a bit weird, we are adding a new string "Select patron" that will never be displayed (but is translatable however). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 --- Comment #47 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- It's for screen readers I believe... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29411 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29411 [Bug 29411] Single result for checkout search by name should redirect to check out tab -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29430 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29430 [Bug 29430] Table cell click doesn't activate buttons in patron search -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29463 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29463 [Bug 29463] Umlauts in search field get changed into Replacement character -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30230 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30230 [Bug 30230] Search for patrons in checkout should not require edit_borrowers permission -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #48 from David Cook <dcook@prosentient.com.au> --- FYI this has caused a regression as noted in bug 30230 but it doesn't seem easily resolvable especially since this change has been around too long to easily revert... -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org