Bug 30337

Summary: Holds to pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended
Product: Koha Reporter: Kyle M Hall <kyle>
Component: CirculationAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: minor    
Priority: P5 - low CC: arthur.suzuki, caroline.cyr-la-rose, gmcharlt, kyle.m.hall, lucas, martin.renvoize, tomascohen, victor
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.01, 21.11.07
Attachments: Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended

Description Kyle M Hall 2022-03-22 18:14:16 UTC
Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended, and shows the wrong number of "patrons with holds" which presumably tells the librarian how many copies to pull from the shelves.

Test Plan:
1) Create a record with one or more holdable items
2) Place two holds on the record
3) Note they show in the holds to pull report
4) Suspend the priority 2 hold
5) Note the report continues showing that 2 patrons have holds
6) Resume the priority 2 hold
7) Suspend the priority 1 hold
8) Note the hold disappears from the holds to pull report
9) Apply this patch
10) Restart all the things!
11) Reload the holds to pull report
12) The report should show one hold that needs an item pulled to fill it!
Comment 1 Kyle M Hall 2022-03-22 18:16:00 UTC
Created attachment 132032 [details] [review]
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended

Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended,
and shows the wrong number of "patrons with holds" which presumably tells the
librarian how many copies to pull from the shelves.

Test Plan:
1) Create a record with one or more holdable items
2) Place two holds on the record
3) Note they show in the holds to pull report
4) Suspend the priority 2 hold
5) Note the report continues showing that 2 patrons have holds
6) Resume the priority 2 hold
7) Suspend the priority 1 hold
8) Note the hold disappears from the holds to pull report
9) Apply this patch
10) Restart all the things!
11) Reload the holds to pull report
12) The report should show one hold that needs an item pulled to fill it!
Comment 2 Kyle M Hall 2022-03-22 18:19:54 UTC
Created attachment 132033 [details] [review]
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended

Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended,
and shows the wrong number of "patrons with holds" which presumably tells the
librarian how many copies to pull from the shelves.

Test Plan:
1) Create a record with one or more holdable items
2) Place two holds on the record
3) Note they show in the holds to pull report
4) Suspend the priority 2 hold
5) Note the report continues showing that 2 patrons have holds
6) Resume the priority 2 hold
7) Suspend the priority 1 hold
8) Note the hold disappears from the holds to pull report
9) Apply this patch
10) Restart all the things!
11) Reload the holds to pull report
12) The report should show one hold that needs an item pulled to fill it!
Comment 3 David Nind 2022-05-04 03:44:10 UTC
Created attachment 134568 [details] [review]
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended

Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended,
and shows the wrong number of "patrons with holds" which presumably tells the
librarian how many copies to pull from the shelves.

Test Plan:
1) Create a record with one or more holdable items
2) Place two holds on the record
3) Note they show in the holds to pull report
4) Suspend the priority 2 hold
5) Note the report continues showing that 2 patrons have holds
6) Resume the priority 2 hold
7) Suspend the priority 1 hold
8) Note the hold disappears from the holds to pull report
9) Apply this patch
10) Restart all the things!
11) Reload the holds to pull report
12) The report should show one hold that needs an item pulled to fill it!

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Caroline Cyr La Rose 2022-05-13 13:38:07 UTC
Created attachment 134972 [details] [review]
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended

Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended,
and shows the wrong number of "patrons with holds" which presumably tells the
librarian how many copies to pull from the shelves.

Test Plan:
1) Create a record with one or more holdable items
2) Place two holds on the record
3) Note they show in the holds to pull report
4) Suspend the priority 2 hold
5) Note the report continues showing that 2 patrons have holds
6) Resume the priority 2 hold
7) Suspend the priority 1 hold
8) Note the hold disappears from the holds to pull report
9) Apply this patch
10) Restart all the things!
11) Reload the holds to pull report
12) The report should show one hold that needs an item pulled to fill it!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 5 Caroline Cyr La Rose 2022-05-13 13:39:02 UTC
BTW, this is a problem all the way back to 21.05
Comment 6 Martin Renvoize 2022-05-19 16:26:59 UTC
Created attachment 135213 [details] [review]
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended

Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended,
and shows the wrong number of "patrons with holds" which presumably tells the
librarian how many copies to pull from the shelves.

Test Plan:
1) Create a record with one or more holdable items
2) Place two holds on the record
3) Note they show in the holds to pull report
4) Suspend the priority 2 hold
5) Note the report continues showing that 2 patrons have holds
6) Resume the priority 2 hold
7) Suspend the priority 1 hold
8) Note the hold disappears from the holds to pull report
9) Apply this patch
10) Restart all the things!
11) Reload the holds to pull report
12) The report should show one hold that needs an item pulled to fill it!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2022-05-19 16:28:39 UTC
Works as expected and resolves a regression.  QA script is happy.

I'd love to have seen some work to move some of the queries here out of the controller and up into the Koha::Holds class (so we could also have unit tests added)..

But as a fix, this does the job and is pragmatic so I can't really fail it ;P

Passing
Comment 8 Tomás Cohen Arazi 2022-06-02 13:20:46 UTC
(In reply to Martin Renvoize from comment #7)
> Works as expected and resolves a regression.  QA script is happy.
> 
> I'd love to have seen some work to move some of the queries here out of the
> controller and up into the Koha::Holds class (so we could also have unit
> tests added)..

Was going to post the same. Let's follow-up and build the query in Koha::Holds->filter_by_something or the like.
Comment 9 Tomás Cohen Arazi 2022-06-02 13:23:36 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 10 Lucas Gass 2022-06-10 16:31:55 UTC
Pushed to 22.05.x for 22.05.01
Comment 11 Arthur Suzuki 2022-06-21 13:32:04 UTC
thx, pushed to 21.11.x for 21.11.07
Comment 12 Victor Grousset/tuxayo 2022-06-25 23:56:45 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.
Comment 13 Caroline Cyr La Rose 2023-05-05 18:38:35 UTC
This seems like a bug fix, nothing to add to the manual.