Bug 26896

Summary: SIP option holds_block_checkin does not actually block checkin of items on hold
Product: Koha Reporter: Kyle M Hall <kyle>
Component: SIP2Assignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: major    
Priority: P5 - low CC: colin.campbell, jonathan.druart, lucas, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00
Bug Depends on: 25541    
Bug Blocks:    
Attachments: Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds
Bug 26896: Enable SIP option holds_block_checkin to actually block checkin of items with holds

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