Bug 26963 deals with improving the efficiency of the pickup_locations routine. Currently we are loading these dropdowns for every hold on a bib. It would save loading time to only load the dropdown if a library attempts to edit the pickup location
Created attachment 113395 [details] [review] Bug 26988: Add API route to fetch hold pickup locations and use it in the holds table To test: 1 - Place a number of holds on a record 2 - Have different pickup locations for the holds 3 - Have some libraries that are not pickup locations 4 - Load the holds tab for the record and note libraries not pickup locations are not in dropdowns 5 - Apply patch and restart all things 6 - Reload the holds table 7 - Click on a dropdown, note the spinner, should load successfully 8 - Confirm the dropdown matches the options before the patch 9 - Confirm updating the hold location works
Created attachment 113418 [details] [review] Bug 26988: Add API route to fetch hold pickup locations and use it in the holds table To test: 1 - Place a number of holds on a record 2 - Have different pickup locations for the holds 3 - Have some libraries that are not pickup locations 4 - Load the holds tab for the record and note libraries not pickup locations are not in dropdowns 5 - Apply patch and restart all things 6 - Reload the holds table 7 - Click on a dropdown, note the spinner, should load successfully 8 - Confirm the dropdown matches the options before the patch 9 - Confirm updating the hold location works Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Created attachment 113426 [details] [review] Bug 26988: Add API route to fetch hold pickup locations and use it in the holds table To test: 1 - Place a number of holds on a record 2 - Have different pickup locations for the holds 3 - Have some libraries that are not pickup locations 4 - Load the holds tab for the record and note libraries not pickup locations are not in dropdowns 5 - Apply patch and restart all things 6 - Reload the holds table 7 - Click on a dropdown, note the spinner, should load successfully 8 - Confirm the dropdown matches the options before the patch 9 - Confirm updating the hold location works Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Created attachment 113427 [details] [review] Bug 26988: Make ->patron prefetchable Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113428 [details] [review] Bug 26988: (QA follow-up) Make the API return proper library objects Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113456 [details] [review] Bug 26988: Add API route to fetch hold pickup locations and use it in the holds table To test: 1 - Place a number of holds on a record 2 - Have different pickup locations for the holds 3 - Have some libraries that are not pickup locations 4 - Load the holds tab for the record and note libraries not pickup locations are not in dropdowns 5 - Apply patch and restart all things 6 - Reload the holds table 7 - Click on a dropdown, note the spinner, should load successfully 8 - Confirm the dropdown matches the options before the patch 9 - Confirm updating the hold location works Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Created attachment 113457 [details] [review] Bug 26988: Make ->patron prefetchable Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113458 [details] [review] Bug 26988: (QA follow-up) Make the API return proper library objects Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113459 [details] [review] Bug 26988: (follow-up) Adjust code to use api field names
Created attachment 113460 [details] [review] Bug 26988: (follow-up) Fix filter and encode api values and escape rendered values This patch makes the QA tools happy and follows correct procedures for passing data to and rendering data from the api
This is working in Chrome but not on Firefox. We are trying to fix it!
Created attachment 113502 [details] [review] Bug 26988: (follow-up) Change event to focus Click event on select element if Firefox does not work as expected, so this patch changes the event to focus for it to work seamlessly in both browsers. https://bugs.koha-community.org/show_bug.cgi?id=26899
Created attachment 113533 [details] [review] Bug 26988: Add API route to fetch hold pickup locations and use it in the holds table To test: 1 - Place a number of holds on a record 2 - Have different pickup locations for the holds 3 - Have some libraries that are not pickup locations 4 - Load the holds tab for the record and note libraries not pickup locations are not in dropdowns 5 - Apply patch and restart all things 6 - Reload the holds table 7 - Click on a dropdown, note the spinner, should load successfully 8 - Confirm the dropdown matches the options before the patch 9 - Confirm updating the hold location works Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113534 [details] [review] Bug 26988: Make ->patron prefetchable Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113535 [details] [review] Bug 26988: (QA follow-up) Make the API return proper library objects Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113536 [details] [review] Bug 26988: (follow-up) Adjust code to use api field names Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113537 [details] [review] Bug 26988: (follow-up) Fix filter and encode api values and escape rendered values This patch makes the QA tools happy and follows correct procedures for passing data to and rendering data from the api Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113538 [details] [review] Bug 26988: (follow-up) Change event to focus Click event on select element if Firefox does not work as expected, so this patch changes the event to focus for it to work seamlessly in both browsers. https://bugs.koha-community.org/show_bug.cgi?id=26899 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113590 [details] [review] Bug 26988: No need to check if hold is linked to a patron Holds are deleted along with patrons.
Created attachment 113601 [details] [review] Bug 26988: Adjust for bug 26963
Pushed to master for 20.11, thanks to everybody involved!
Created attachment 113669 [details] [review] Bug 26988: (QA follow-up) Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Dear RM, we forgot the unit tests for the route. The latest patch adds them.
(In reply to Tomás Cohen Arazi from comment #22) > Created attachment 113669 [details] [review] [review] > Bug 26988: (QA follow-up) Unit tests > > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Patch pushed to master.
Backported to 20.05.x for 20.05.07
(In reply to Andrew Fuerste-Henry from comment #25) > Backported to 20.05.x for 20.05.07 There is a test failing on 20.05.x, Tomas can you have a look please? t/db_dependent/api/v1/holds.t .. 9/10 # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/holds.t line 768. # Structures begin differing at: # $got = HASH(0x5630131b9158) # $expected = ARRAY(0x5630134b3bf8)
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.
(In reply to Jonathan Druart from comment #26) > (In reply to Andrew Fuerste-Henry from comment #25) > > Backported to 20.05.x for 20.05.07 > > There is a test failing on 20.05.x, Tomas can you have a look please? > > t/db_dependent/api/v1/holds.t .. 9/10 > # Failed test 'exact match for JSON Pointer ""' > # at t/db_dependent/api/v1/holds.t line 768. > # Structures begin differing at: > # $got = HASH(0x5630131b9158) > # $expected = ARRAY(0x5630134b3bf8) Is this still the case, Andrew?
I am not getting that test failure on 20.05.x: root@kohadevbox:koha(rmain2005)$ prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. ok All tests successful. Files=1, Tests=12, 11 wallclock secs ( 0.03 usr 0.01 sys + 7.19 cusr 1.00 csys = 8.23 CPU) Result: PASS