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: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords: rel_22_05_candidate
Depends on:
Blocks:
 
Reported: 2022-03-22 18:14 UTC by Kyle M Hall
Modified: 2022-06-25 23:56 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


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
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
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
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 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.