Summary: | Dropdown selector when changing pickup library should not depend on RESTdefaultPageSize | ||
---|---|---|---|
Product: | Koha | Reporter: | Laura Escamilla <Laura.escamilla> |
Component: | Circulation | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | Pushed to stable --- | QA Contact: | Pedro Amorim <pedro.amorim> |
Severity: | normal | ||
Priority: | P5 - low | CC: | david, emmi.takkinen, gmcharlt, jonathan.druart, kyle.m.hall, lucas, martin.renvoize, matt.blenkinsop, mikko.liimatainen, pedro.amorim, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This fixes the dropdown list for holds pickup locations in the staff interface - it now shows the complete list of libraries. Previously, in some circumstances, it was not showing the complete list of pickup locations (for example, with RESTdefaultPageSize = 5 and AllowHoldPolicyOverride = Allow, it would only show the final page of libraries instead of the full list of libraries).
|
Version(s) released in: |
25.05.00,24.11.04
|
Circulation function: | Holds | ||
Attachments: |
Buggy dropdown selector
Bug 34068: Fix pagination for hold pickup locations Bug 34068: Fix pagination for hold pickup locations Bug 34068: Fix pagination for hold pickup locations Bug 34068: Add tests Bug 34068: Fix tests Bug 34068: Mock RESTdefaultPageSize Bug 34068: Fix pagination for hold pickup locations Bug 34068: Add tests Bug 34068: Fix tests Bug 34068: Mock RESTdefaultPageSize Bug 34068: Fix pagination for hold pickup locations Bug 34068: Add tests Bug 34068: Fix tests Bug 34068: Mock RESTdefaultPageSize |
Had the same issue with 60 pickup branches where the last 20 libraries kept repeating. Turns out this was caused by RESTdefaultPageSize being set to 20. Increasing the setting to 80 solved the problem. Mikko's answer solved the issue. Thanks! It does solve it, but I think the dependency is not good - we should keep this open and look into changing the behaviour. +1 Created attachment 171964 [details] [review] Bug 34068: Fix pagination for hold pickup locations This patch addresses an issue where pagination filters are set incorrectly for the pickup locations endpoint. Only the final page is returned rather than the expected full list of results Test plan: 1) In KTD, set the syspref RESTdefaultPageSize to 5 2) Set AllowHoldPolicyOverride to Allow 3) Create a hold on an item for a patron 4) Navigate to the patron's page and click on the holds tab 5) Click the dropdown in pickup locations 6) There will be two results, repeated three times 7) Apply patch and restart_all 8) Repeat step 5, there should now be a full list of 12 libraries Created attachment 171969 [details] [review] Bug 34068: Fix pagination for hold pickup locations This patch addresses an issue where pagination filters are set incorrectly for the pickup locations endpoint. Only the final page is returned rather than the expected full list of results Test plan: 1) In KTD, set the syspref RESTdefaultPageSize to 5 2) Set AllowHoldPolicyOverride to Allow 3) Create a hold on an item for a patron 4) Navigate to the patron's page and click on the holds tab 5) Click the dropdown in pickup locations 6) There will be two results, repeated three times 7) Apply patch and restart_all 8) Repeat step 5, there should now be a full list of 12 libraries Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com> Created attachment 171970 [details] [review] Bug 34068: Fix pagination for hold pickup locations This patch addresses an issue where pagination filters are set incorrectly for the pickup locations endpoint. Only the final page is returned rather than the expected full list of results Test plan: 1) In KTD, set the syspref RESTdefaultPageSize to 5 2) Set AllowHoldPolicyOverride to Allow 3) Create a hold on an item for a patron 4) Navigate to the patron's page and click on the holds tab 5) Click the dropdown in pickup locations 6) There will be two results, repeated three times 7) Apply patch and restart_all 8) Repeat step 5, there should now be a full list of 12 libraries Co-authored-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com> Created attachment 174071 [details] [review] Bug 34068: Add tests Created attachment 174072 [details] [review] Bug 34068: Fix tests Several wrong things in the previous attempt: * as_list returns an array * as_list returns objects, ->{library_id} does not exist * as_list does not returns api object, you need to call ->branchcode to get the id * $pickup_locations defined for both situations was confusing Please retest with and without AllowHoldPolicyOverride. Tests are failing here for me now, not always, but it seems that if I set RESTdefaultPageSize to something other than 20 they fail: # Failed test 'exact match for JSON Pointer ""' # at /kohadevbox/koha/t/db_dependent/api/v1/holds.t line 908. # Structures begin differing at: # $got->[5] = Does not exist # $expected->[5] = HASH(0x5ed8c2627ed8) ok 13 - Patron passed correctly ok 14 - GET //tomasito:thePassword123@/api/v1/holds/157/pickup_locations?_order_by=marc_org_code&_page=2 not ok 15 - exact match for JSON Pointer "" # Failed test 'exact match for JSON Pointer ""' # at /kohadevbox/koha/t/db_dependent/api/v1/holds.t line 912. # Structures begin differing at: # $got->[0]{address3} = 'gftd3B_' # $expected->[0]{address3} = 'YtBCoGC' # Looks like you failed 2 tests of 15. Created attachment 174106 [details] [review] Bug 34068: Mock RESTdefaultPageSize Created attachment 176110 [details] [review] Bug 34068: Fix pagination for hold pickup locations This patch addresses an issue where pagination filters are set incorrectly for the pickup locations endpoint. Only the final page is returned rather than the expected full list of results Test plan: 1) In KTD, set the syspref RESTdefaultPageSize to 5 2) Set AllowHoldPolicyOverride to Allow 3) Create a hold on an item for a patron 4) Navigate to the patron's page and click on the holds tab 5) Click the dropdown in pickup locations 6) There will be two results, repeated three times 7) Apply patch and restart_all 8) Repeat step 5, there should now be a full list of 12 libraries Co-authored-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Created attachment 176111 [details] [review] Bug 34068: Add tests Signed-off-by: David Nind <david@davidnind.com> Created attachment 176112 [details] [review] Bug 34068: Fix tests Several wrong things in the previous attempt: * as_list returns an array * as_list returns objects, ->{library_id} does not exist * as_list does not returns api object, you need to call ->branchcode to get the id * $pickup_locations defined for both situations was confusing Signed-off-by: David Nind <david@davidnind.com> Created attachment 176113 [details] [review] Bug 34068: Mock RESTdefaultPageSize Signed-off-by: David Nind <david@davidnind.com> I've changed this to a normal bug, instead of an enhancement. I'm not sure who the assignee should be 8-), so haven't updated that. The tests pass: prove t/db_dependent/api/v1/holds.t Created attachment 176331 [details] [review] Bug 34068: Fix pagination for hold pickup locations This patch addresses an issue where pagination filters are set incorrectly for the pickup locations endpoint. Only the final page is returned rather than the expected full list of results Test plan: 1) In KTD, set the syspref RESTdefaultPageSize to 5 2) Set AllowHoldPolicyOverride to Allow 3) Create a hold on an item for a patron 4) Navigate to the patron's page and click on the holds tab 5) Click the dropdown in pickup locations 6) There will be two results, repeated three times 7) Apply patch and restart_all 8) Repeat step 5, there should now be a full list of 12 libraries Co-authored-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 176332 [details] [review] Bug 34068: Add tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 176333 [details] [review] Bug 34068: Fix tests Several wrong things in the previous attempt: * as_list returns an array * as_list returns objects, ->{library_id} does not exist * as_list does not returns api object, you need to call ->branchcode to get the id * $pickup_locations defined for both situations was confusing Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Created attachment 176334 [details] [review] Bug 34068: Mock RESTdefaultPageSize Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Thanks for improving upon the initial patch, Joubu, and all the additional work here. Pushed for 25.05! Well done everyone, thank you! Nice work everyone! Pushed to 24.11.x for 24.11.04 |
Created attachment 152486 [details] Buggy dropdown selector To recreate: 1. Have a library that has more than 30 pickup branches. 2. Place a hold on an item. 3. Go to the patron account and try to change the pickup location. The dropdown selector will keep repeating the last 10 libraries.