Bug 24410 - Multi holds broken
Summary: Multi holds broken
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
: 24571 24587 (view as bug list)
Depends on: 19618
Blocks: 15261 24599
  Show dependency treegraph
 
Reported: 2020-01-13 13:29 UTC by Jonathan Druart
Modified: 2020-03-09 17:51 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.04


Attachments
Bug 24410: Fix multi holds (1.75 KB, patch)
2020-01-13 13:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24410: Fix multi holds (1.80 KB, patch)
2020-02-05 13:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 24410: Fix multi holds (1.86 KB, patch)
2020-02-05 18:04 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 24410: Fix multi holds (1.92 KB, patch)
2020-02-06 08:52 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 Jonathan Druart 2020-01-13 13:29:30 UTC
Bug 19618 replaces the call to placerequest.pl with a REST API /holds
That is wrong as it does not support multi holds.
Comment 1 Jonathan Druart 2020-01-13 13:31:40 UTC
Created attachment 97310 [details] [review]
Bug 24410: Fix multi holds

Bug 19618 replaces the call to placerequest.pl with a REST API /holds
That is wrong as it does not support multi holds.

Test plan:
Confirm that prior to this patch you are not able to place a hold on
several records at once, and that this patch fixes it
Comment 2 Tomás Cohen Arazi 2020-01-13 14:06:30 UTC
I tested the multi_hold flow and it calls request.pl or modrequest.pl (when you update the multi hold).

I belive we are safe without this patch.
Comment 3 Jonathan Druart 2020-01-13 16:14:58 UTC
This seems to work ok. Assigning to Tomas for the next steps (ie. more work is needed if placerequest can be removed).
Comment 4 Jonathan Druart 2020-02-04 11:26:38 UTC
*** Bug 24571 has been marked as a duplicate of this bug. ***
Comment 5 Jonathan Druart 2020-02-04 11:27:43 UTC
From Nick on bug 24571

"""
In the staff client do a search that returns 20+  results that can be held

Select all

Place hold

Confirm

The api submits ~8 requests, then the page moves to the hold results, but not alll records are held


While this does work in Chrome, there is no feedback as to what is happening - the submit button is not disabled. Testing with 200 holds there is a lot of waiting with no feedback
"""
Comment 6 Agustín Moyano 2020-02-04 23:28:40 UTC
(In reply to Jonathan Druart from comment #5)
> From Nick on bug 24571
> 
> """
> In the staff client do a search that returns 20+  results that can be held
> 
> Select all
> 
> Place hold
> 
> Confirm
> 
> The api submits ~8 requests, then the page moves to the hold results, but
> not alll records are held
> 
> 
> While this does work in Chrome, there is no feedback as to what is happening
> - the submit button is not disabled. Testing with 200 holds there is a lot
> of waiting with no feedback
> """

I'm thinking.. is there any RFC for bulk processing endpoints?.. 

maybe we can create an endpoint that does multiholds.

What do You think Jonathan?
Comment 7 Jonathan Druart 2020-02-05 08:20:58 UTC
*** Bug 24587 has been marked as a duplicate of this bug. ***
Comment 8 Jonathan Druart 2020-02-05 08:22:56 UTC
Agustin, as this affect stable and that the bug is now confirmed I think we should move this patch forward ASAP then discuss later how we can use the endpoint for multi holds.
Comment 9 Julian Maurice 2020-02-05 13:23:18 UTC
Created attachment 98479 [details] [review]
Bug 24410: Fix multi holds

Bug 19618 replaces the call to placerequest.pl with a REST API /holds
That is wrong as it does not support multi holds.

Test plan:
Confirm that prior to this patch you are not able to place a hold on
several records at once, and that this patch fixes it

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 10 Agustín Moyano 2020-02-05 18:04:32 UTC
Created attachment 98508 [details] [review]
Bug 24410: Fix multi holds

Bug 19618 replaces the call to placerequest.pl with a REST API /holds
That is wrong as it does not support multi holds.

Test plan:
Confirm that prior to this patch you are not able to place a hold on
several records at once, and that this patch fixes it

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Comment 11 Martin Renvoize 2020-02-06 08:52:41 UTC
Created attachment 98517 [details] [review]
Bug 24410: Fix multi holds

Bug 19618 replaces the call to placerequest.pl with a REST API /holds
That is wrong as it does not support multi holds.

Test plan:
Confirm that prior to this patch you are not able to place a hold on
several records at once, and that this patch fixes it

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2020-02-06 08:53:41 UTC
This is a simple partial revert to restore functionality.. Passing QA.

Lets handle the final removal of those controllers and switch to REST apis as a separate issue.
Comment 13 Martin Renvoize 2020-02-06 12:37:55 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 14 Joy Nelson 2020-03-05 00:20:35 UTC
Pushed to 19.11.x for 19.11.04
Comment 15 Lucas Gass 2020-03-09 17:51:24 UTC
missing dependencies for 19.05.x, no backport