Bug 30337 - Holds to pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended
Summary: Holds to pull ( pendingreserves.pl ) ignores holds if priority 1 hold is susp...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Kyle M Hall (khall)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-22 18:14 UTC by Kyle M Hall (khall)
Modified: 2023-12-28 20:43 UTC (History)
8 users (show)

See Also:
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
Circulation function:


Attachments
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended (2.24 KB, patch)
2022-03-22 18:16 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended (2.57 KB, patch)
2022-03-22 18:19 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended (2.62 KB, patch)
2022-05-04 03:44 UTC, David Nind
Details | Diff | Splinter Review
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended (2.69 KB, patch)
2022-05-13 13:38 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 30337: Holds to Pull ( pendingreserves.pl ) ignores holds if priority 1 hold is suspended (2.75 KB, patch)
2022-05-19 16:26 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 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 (khall) 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 (khall) 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 (ashimema) 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 (ashimema) 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 (tcohen) 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 (tcohen) 2022-06-02 13:23:36 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 10 Lucas Gass (lukeg) 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.