Bug 29355 - Pickup location list limited by RESTdefaultPageSize syspref
Summary: Pickup location list limited by RESTdefaultPageSize syspref
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 29356 29404
  Show dependency treegraph
 
Reported: 2021-10-28 17:34 UTC by Andrew Fuerste-Henry
Modified: 2021-11-05 15:36 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
21.11.00


Attachments
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic (3.18 KB, patch)
2021-10-28 18:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic (3.23 KB, patch)
2021-10-28 18:28 UTC, David Nind
Details | Diff | Splinter Review
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic (3.69 KB, patch)
2021-10-28 18:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic (3.75 KB, patch)
2021-11-01 17:26 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2021-10-28 17:34:21 UTC
To recreate:
- have multiple libraries in your system as valid pickup locations
- place a hold via the staff client, see that all your libraries appear in the pickup location dropdowns at the bib and item level
- update RESTPublicAPI, set the value to something lower than your count of pickup libraries
- place another hold, confirm your pickup location list gets cut off and only shows as many locations as RESTPublicAPI allows

We should show all valid options, even if it goes above the number set in RESTPublicAPI.
Comment 1 Tomás Cohen Arazi 2021-10-28 18:09:02 UTC
Created attachment 127071 [details] [review]
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic

This patch makes the select2 dropdowns for pickup locations not be
limited to the RESTdefaultPageSize syspref limit.

To test:
1. Have less than 20 libraries in your system as valid pickup locations
2. Place a hold via the staff client
=> SUCCESS: See that all your libraries appear in the pickup location dropdowns at the bib and item level
3. Update RESTdefaultPageSize, set the value to something lower than your count of pickup libraries
4. place another hold
=> FAIL: Your pickup location list gets cut off and only shows as many locations as RESTdefaultPageSize allows
5. Apply this patch
6. Repeat 4
=> SUCCESS: All your pickup locations show
7. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Andrew Fuerste-Henry 2021-10-28 18:27:14 UTC
This fixed the bib-level selector, but not the item-level selector. At item-level, my choices are still getting limited by RESTPublicAPI
Comment 3 David Nind 2021-10-28 18:28:56 UTC
Created attachment 127073 [details] [review]
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic

This patch makes the select2 dropdowns for pickup locations not be
limited to the RESTdefaultPageSize syspref limit.

To test:
1. Have less than 20 libraries in your system as valid pickup locations
2. Place a hold via the staff client
=> SUCCESS: See that all your libraries appear in the pickup location dropdowns at the bib and item level
3. Update RESTdefaultPageSize, set the value to something lower than your count of pickup libraries
4. place another hold
=> FAIL: Your pickup location list gets cut off and only shows as many locations as RESTdefaultPageSize allows
5. Apply this patch
6. Repeat 4
=> SUCCESS: All your pickup locations show
7. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Tomás Cohen Arazi 2021-10-28 18:30:23 UTC
Created attachment 127074 [details] [review]
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic

This patch makes the select2 dropdowns for pickup locations not be
limited to the RESTdefaultPageSize syspref limit.

To test:
1. Have less than 20 libraries in your system as valid pickup locations
2. Place a hold via the staff client
=> SUCCESS: See that all your libraries appear in the pickup location dropdowns at the bib and item level
3. Update RESTdefaultPageSize, set the value to something lower than your count of pickup libraries
4. place another hold
=> FAIL: Your pickup location list gets cut off and only shows as many locations as RESTdefaultPageSize allows
5. Apply this patch
6. Repeat 4
=> SUCCESS: All your pickup locations show
7. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Andrew Fuerste-Henry 2021-10-28 18:32:54 UTC
Confirmed Tomas's second patch also fixes the item-level selector.
Also, belatedly realized I gave the wrong syspref name, I should have said RESTdefaultPageSize. Luckily, y'all knew what I meant.
Comment 6 Martin Renvoize 2021-11-01 17:26:31 UTC
Created attachment 127187 [details] [review]
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic

This patch makes the select2 dropdowns for pickup locations not be
limited to the RESTdefaultPageSize syspref limit.

To test:
1. Have less than 20 libraries in your system as valid pickup locations
2. Place a hold via the staff client
=> SUCCESS: See that all your libraries appear in the pickup location dropdowns at the bib and item level
3. Update RESTdefaultPageSize, set the value to something lower than your count of pickup libraries
4. place another hold
=> FAIL: Your pickup location list gets cut off and only shows as many locations as RESTdefaultPageSize allows
5. Apply this patch
6. Repeat 4
=> SUCCESS: All your pickup locations show
7. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2021-11-01 17:27:10 UTC
Cool.. I wasn't aware passing -1 for page size worked to disable the limits.. nice.

Works as expected, QA scripts are happy.

Passing QA
Comment 8 Jonathan Druart 2021-11-02 15:24:35 UTC
I am not sure it makes sense, there is a filter to retrieve those not displayed. What about Koha installations with hundreds of libraries?
Comment 9 Jonathan Druart 2021-11-02 15:24:53 UTC
(will push anyway)
Comment 10 Tomás Cohen Arazi 2021-11-02 15:32:59 UTC
(In reply to Jonathan Druart from comment #8)
> I am not sure it makes sense, there is a filter to retrieve those not
> displayed. What about Koha installations with hundreds of libraries?

My feeling was the same. That changing the query to 'contains' would make their libraries show up even with the limit.
For the record: this was requested by a library with hundreds of branches, that has difficulties to find the branches.
Comment 11 Jonathan Druart 2021-11-02 15:54:44 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 12 Martin Renvoize 2021-11-02 15:59:20 UTC
Must admit I was also surprised by this one.. but it did what is says on the box as it were.

I thought Select2 supported 'infinite scrolling' using API paging.. https://select2.org/data-sources/ajax#pagination.. doesn't that work here without calling '-1'?