Bug 29355

Summary: Pickup location list limited by RESTdefaultPageSize syspref
Product: Koha Reporter: Andrew Fuerste-Henry <andrewfh>
Component: Hold requestsAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: gmcharlt, jonathan.druart, kyle, martin.renvoize, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29397
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00
Bug Depends on:    
Bug Blocks: 29356, 29404    
Attachments: Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic
Bug 29355: Make pickup location dropdowns RESTdefaultPageSize agnostic

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'?