Bug 29136 - Patron search on request.pl has performance and display issues
Summary: Patron search on request.pl has performance and display issues
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 28782
Blocks: 30093
  Show dependency treegraph
 
Reported: 2021-09-29 17:55 UTC by Nick Clemens
Modified: 2022-12-12 21:24 UTC (History)
7 users (show)

See Also:
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


Attachments
Bug 29136: Ajaxify the patron search when placing a hold (15.41 KB, patch)
2022-02-01 11:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29136: Don't redirect if the whole set is > 1 (1.30 KB, patch)
2022-02-07 16:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29136: Ajaxify the patron search when placing a hold (15.45 KB, patch)
2022-02-07 17:42 UTC, David Nind
Details | Diff | Splinter Review
Bug 29136: Don't redirect if the whole set is > 1 (1.35 KB, patch)
2022-02-07 17:42 UTC, David Nind
Details | Diff | Splinter Review
Bug 29136: Ajaxify the patron search when placing a hold (15.51 KB, patch)
2022-02-18 16:38 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29136: Don't redirect if the whole set is > 1 (1.41 KB, patch)
2022-02-18 16:38 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29136: (QA follow-up) missing filters (2.46 KB, patch)
2022-02-18 16:38 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29136: Ajaxify the patron search when placing a hold (15.60 KB, patch)
2022-03-03 19:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29136: Don't redirect if the whole set is > 1 (1.41 KB, patch)
2022-03-03 19:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29136: (QA follow-up) missing filters (2.46 KB, patch)
2022-03-03 19:33 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.