Bug 26896 - SIP option holds_block_checkin does not actually block checkin of items on hold
Summary: SIP option holds_block_checkin does not actually block checkin of items on hold
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 25541
Blocks:
  Show dependency treegraph
 
Reported: 2020-11-02 14:50 UTC by Kyle M Hall
Modified: 2020-11-16 21:01 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds (3.86 KB, patch)
2020-11-02 14:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds (3.89 KB, patch)
2020-11-02 19:23 UTC, David Nind
Details | Diff | Splinter Review
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds (3.96 KB, patch)
2020-11-03 08:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds (4.01 KB, patch)
2020-11-03 08:56 UTC, Matthias Meusburger
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 2020-11-02 14:50:25 UTC
Thanks to matts for alerting me to this major issue with holds_block_checkin!

It appears that at some point, the actual blocking of checkins on items with holds got lost from the patch set. This slipped by because the sip server still outputs everything correctly, but the unit tests don't actually test to see if the checkout still exists afterward!
Comment 1 Kyle M Hall 2020-11-02 14:52:34 UTC
Created attachment 112843 [details] [review]
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds

It appears that at some point, the actual blocking of checkins on items
with holds got lost from the patch set. This slipped by because the sip
server still outputs everything correctly, but the unit tests don't
actually test to see if the checkout still exists afterward!

Test plan:
1) Apply this patch
2) Prove t/db_dependent/SIP/Message.t
Comment 2 David Nind 2020-11-02 19:23:50 UTC
Created attachment 112863 [details] [review]
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds

It appears that at some point, the actual blocking of checkins on items
with holds got lost from the patch set. This slipped by because the sip
server still outputs everything correctly, but the unit tests don't
actually test to see if the checkout still exists afterward!

Test plan:
1) Apply this patch
2) Prove t/db_dependent/SIP/Message.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Martin Renvoize 2020-11-03 08:48:04 UTC
Yikes.. I think you and I were working in this area around the same time Kyle.. bet it was a mix of our bugs that caused the issue.

Looks good now, just testing for QA
Comment 4 Martin Renvoize 2020-11-03 08:49:19 UTC
Created attachment 112897 [details] [review]
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds

It appears that at some point, the actual blocking of checkins on items
with holds got lost from the patch set. This slipped by because the sip
server still outputs everything correctly, but the unit tests don't
actually test to see if the checkout still exists afterward!

Test plan:
1) Apply this patch
2) Prove t/db_dependent/SIP/Message.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2020-11-03 08:49:43 UTC
Tests now cover fully and pass.

Passing QA
Comment 6 Matthias Meusburger 2020-11-03 08:56:53 UTC
Created attachment 112900 [details] [review]
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds

It appears that at some point, the actual blocking of checkins on items
with holds got lost from the patch set. This slipped by because the sip
server still outputs everything correctly, but the unit tests don't
actually test to see if the checkout still exists afterward!

Test plan:
1) Apply this patch
2) Prove t/db_dependent/SIP/Message.t

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Comment 7 Jonathan Druart 2020-11-03 11:03:09 UTC
Lowering the severity, it's not in stable yet.
Comment 8 Jonathan Druart 2020-11-04 12:04:45 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 9 Lucas Gass 2020-11-16 21:01:02 UTC
this patchset does not apply cleanly to 20.05.x, no backport

please rebase if needed