Bug 29136

Summary: Patron search on request.pl has performance and display issues
Product: Koha Reporter: Nick Clemens <nick>
Component: Hold requestsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: andrewfh, david, gmcharlt, jonathan.druart, kyle, martin.renvoize, oleonard
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15812
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes the performance and display of patron search results when placing a hold from a record details page using the staff interface. Patron results are now paginated and all the results are now available - previously all results were listed on one page, which could cause performance issues.
Version(s) released in:
22.05.00
Bug Depends on: 28782    
Bug Blocks: 30093    
Attachments: Bug 29136: Ajaxify the patron search when placing a hold
Bug 29136: Don't redirect if the whole set is > 1
Bug 29136: Ajaxify the patron search when placing a hold
Bug 29136: Don't redirect if the whole set is > 1
Bug 29136: Ajaxify the patron search when placing a hold
Bug 29136: Don't redirect if the whole set is > 1
Bug 29136: (QA follow-up) missing filters
Bug 29136: Ajaxify the patron search when placing a hold
Bug 29136: Don't redirect if the whole set is > 1
Bug 29136: (QA follow-up) missing filters

Description Nick Clemens 2021-09-29 17:55:43 UTC
Similar to bug 15812, the patron search on request.pl retrieves all patrons from a search without pagination, so broad searches can have poor performance.

Additionally, the datatables call on that page doesn't paginate, so all but the first 20 results are unavailable
Comment 1 Jonathan Druart 2021-11-04 13:46:25 UTC
If anybody is interested in this, please have a look at bug 15812 comment 37.
IMO we shouldn't follow what has been done on bug 15812.
Comment 2 Jonathan Druart 2022-02-01 11:19:34 UTC
Created attachment 130039 [details] [review]
Bug 29136: Ajaxify the patron search when placing a hold
Comment 3 Jonathan Druart 2022-02-01 11:19:58 UTC
Nick, would that work for you?
Comment 4 David Nind 2022-02-05 12:42:13 UTC
Tested suing KTD.

Noted this issue when testing:

. Generate two or more pages of results - use a or e as the patron search term.
. Clicking on 3 or Last results in:
  . koha_sip (koha_sip) patron being selected as the patron, and 
  . being returned to the area to finish placing a hold
. With only two pages of results (search for i) this doesn't occur.

Test plan:
1. Search for a record in the staff interface.
2. Go the detail page for an item in the search results.
3. Click 'Place hold'.
4. In the patron search box enter an a or e.
5. Note that the results are unpaginated and all results that match are displayed on one page.
6. Apply the patch and repeat steps 1 to 5.
7. Note that the results are now paginated using the standard DataTables layout and features.
8. Check that everything works as expected: pagination, including Next, last, previous, each number; select columns; change number of results; export.
9. Sign off :D!
Comment 5 Jonathan Druart 2022-02-07 16:28:25 UTC
Created attachment 130239 [details] [review]
Bug 29136: Don't redirect if the whole set is > 1

To prevent a redirect when we click on a page with only 1 result
Comment 6 Jonathan Druart 2022-02-07 16:28:38 UTC
Good catch, David. Thanks!
Comment 7 David Nind 2022-02-07 17:42:20 UTC
Created attachment 130245 [details] [review]
Bug 29136: Ajaxify the patron search when placing a hold

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2022-02-07 17:42:25 UTC
Created attachment 130246 [details] [review]
Bug 29136: Don't redirect if the whole set is > 1

To prevent a redirect when we click on a page with only 1 result

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 Nick Clemens 2022-02-18 16:38:10 UTC
Created attachment 130874 [details] [review]
Bug 29136: Ajaxify the patron search when placing a hold

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Nick Clemens 2022-02-18 16:38:13 UTC
Created attachment 130875 [details] [review]
Bug 29136: Don't redirect if the whole set is > 1

To prevent a redirect when we click on a page with only 1 result

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens 2022-02-18 16:38:17 UTC
Created attachment 130876 [details] [review]
Bug 29136: (QA follow-up) missing filters

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Jonathan Druart 2022-03-03 19:33:17 UTC
Created attachment 131355 [details] [review]
Bug 29136: Ajaxify the patron search when placing a hold

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Jonathan Druart 2022-03-03 19:33:22 UTC
Created attachment 131356 [details] [review]
Bug 29136: Don't redirect if the whole set is > 1

To prevent a redirect when we click on a page with only 1 result

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Jonathan Druart 2022-03-03 19:33:26 UTC
Created attachment 131357 [details] [review]
Bug 29136: (QA follow-up) missing filters

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 15 Fridolin Somers 2022-03-09 09:07:07 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 16 Kyle M Hall 2022-03-11 11:44:53 UTC
Jonathan, if you think this should be backported, can you please rebase to 21.11.x? If it's not needed, go head and set the bug to RESOLVED/FIXED. Thanks!
Comment 17 Jonathan Druart 2022-03-17 08:27:43 UTC
(In reply to Kyle M Hall from comment #16)
> Jonathan, if you think this should be backported, can you please rebase to
> 21.11.x? If it's not needed, go head and set the bug to RESOLVED/FIXED.
> Thanks!

I don't think it should be backported, it's based on other bugs that are not in 21.11.