Bug 38486 - No block checkouts are still blocked by fines, checkouts, and blocked item types
Summary: No block checkouts are still blocked by fines, checkouts, and blocked item types
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Kyle M Hall (khall)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-11-19 15:44 UTC by Kyle M Hall (khall)
Modified: 2025-03-31 08:31 UTC (History)
7 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes SIP so that it allows noblock checkouts, regardless of normal patron checkout blocks. Explanation: The purpose of no block checkouts in SIP is to indicate that the SIP machine has made an offline ("store and forward") transaction. The patron already has the item. As such, the item must be checked out to the patron or the library risks losing the item due to lack of tracking. As such, no block checkouts should not be blocked under any circumstances.
Version(s) released in:
25.05.00,24.11.02,24.05.08,23.11.13
Circulation function:


Attachments
Bug 38486: Fix unit tests (2.78 KB, patch)
2024-11-19 16:16 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 38486: For SIP allow noblock checkouts reguardless of normal patron checkout blocks (3.41 KB, patch)
2024-11-19 16:16 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 38486: Fix unit tests (2.78 KB, patch)
2024-12-06 15:20 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 38486: For SIP allow noblock checkouts reguardless of normal patron checkout blocks (3.40 KB, patch)
2024-12-06 15:20 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 38486: Fix unit tests (2.81 KB, patch)
2024-12-06 22:06 UTC, David Nind
Details | Diff | Splinter Review
Bug 38486: For SIP allow noblock checkouts reguardless of normal patron checkout blocks (3.43 KB, patch)
2024-12-06 22:06 UTC, David Nind
Details | Diff | Splinter Review
Bug 38486: Fix unit tests (2.88 KB, patch)
2024-12-09 14:44 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38486: For SIP allow noblock checkouts reguardless of normal patron checkout blocks (3.49 KB, patch)
2024-12-09 14:45 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 38486: [24.05.x] Fix unit tests (2.90 KB, patch)
2025-03-20 12:01 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 38486: [24.05.x] For SIP allow noblock checkouts reguardless of normal patron checkout blocks (3.22 KB, patch)
2025-03-20 12:01 UTC, Kyle M Hall (khall)
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) 2024-11-19 15:44:45 UTC
The purpose of no block checkouts in SIP is to indicate that the SIP machine has made an offline ( "store and forward" ) transaction. The patron already has the item. As such, the item must be checked out to the patron or the library risks losing the item do to lack of tracking. As such, no block checkouts should not be blocked under any circumstances.
Comment 1 Kyle M Hall (khall) 2024-11-19 16:16:42 UTC
Created attachment 174795 [details] [review]
Bug 38486: Fix unit tests
Comment 2 Kyle M Hall (khall) 2024-11-19 16:16:48 UTC
Created attachment 174796 [details] [review]
Bug 38486: For SIP allow noblock checkouts reguardless of normal patron checkout blocks

The purpose of no block checkouts in SIP is to indicate that the SIP machine has made an offline ( "store and forward" ) transaction.
The patron already has the item. As such, the item must be checked out to the patron or the library risks losing the item do to lack of tracking.
As such, no block checkouts should not be blocked under any circumstances.

The sub C4::SIP::ILS::Transaction::Checkout::do_checkout already honors this, but it is wrapped by
C4::SIP::ILS::checkout which has additional checks that do not.

Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Transaction.t will fail
3) Apply the second patch
4) prove t/db_dependent/SIP/Transaction.t will pass!
Comment 3 Kyle M Hall (khall) 2024-12-06 15:20:31 UTC
Created attachment 175263 [details] [review]
Bug 38486: Fix unit tests
Comment 4 Kyle M Hall (khall) 2024-12-06 15:20:38 UTC
Created attachment 175264 [details] [review]
Bug 38486: For SIP allow noblock checkouts reguardless of normal patron checkout blocks

The purpose of no block checkouts in SIP is to indicate that the SIP machine has made an offline ( "store and forward" ) transaction.
The patron already has the item. As such, the item must be checked out to the patron or the library risks losing the item do to lack of tracking.
As such, no block checkouts should not be blocked under any circumstances.

The sub C4::SIP::ILS::Transaction::Checkout::do_checkout already honors this, but it is wrapped by
C4::SIP::ILS::checkout which has additional checks that do not.

Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Transaction.t will fail
3) Apply the second patch
4) prove t/db_dependent/SIP/Transaction.t will pass!
Comment 5 David Nind 2024-12-06 22:06:43 UTC
Created attachment 175276 [details] [review]
Bug 38486: Fix unit tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2024-12-06 22:06:46 UTC
Created attachment 175277 [details] [review]
Bug 38486: For SIP allow noblock checkouts reguardless of normal patron checkout blocks

The purpose of no block checkouts in SIP is to indicate that the SIP machine has made an offline ( "store and forward" ) transaction.
The patron already has the item. As such, the item must be checked out to the patron or the library risks losing the item do to lack of tracking.
As such, no block checkouts should not be blocked under any circumstances.

The sub C4::SIP::ILS::Transaction::Checkout::do_checkout already honors this, but it is wrapped by
C4::SIP::ILS::checkout which has additional checks that do not.

Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Transaction.t will fail
3) Apply the second patch
4) prove t/db_dependent/SIP/Transaction.t will pass!

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 Martin Renvoize (ashimema) 2024-12-09 14:44:59 UTC
Created attachment 175312 [details] [review]
Bug 38486: Fix unit tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize (ashimema) 2024-12-09 14:45:01 UTC
Created attachment 175313 [details] [review]
Bug 38486: For SIP allow noblock checkouts reguardless of normal patron checkout blocks

The purpose of no block checkouts in SIP is to indicate that the SIP machine has made an offline ( "store and forward" ) transaction.
The patron already has the item. As such, the item must be checked out to the patron or the library risks losing the item do to lack of tracking.
As such, no block checkouts should not be blocked under any circumstances.

The sub C4::SIP::ILS::Transaction::Checkout::do_checkout already honors this, but it is wrapped by
C4::SIP::ILS::checkout which has additional checks that do not.

Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Transaction.t will fail
3) Apply the second patch
4) prove t/db_dependent/SIP/Transaction.t will pass!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize (ashimema) 2024-12-09 15:57:14 UTC
No regressions found.. sip is always fun :S 

Passing QA
Comment 10 Katrin Fischer 2024-12-23 08:38:17 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 11 Lucas Gass (lukeg) 2025-01-16 17:01:14 UTC
Can this be backported to 24.11?
Comment 12 Paul Derscheid 2025-02-03 19:09:21 UTC
Nice work everyone!

Pushed to 24.11.x for 24.11.01
Comment 13 Alex Buckley 2025-03-19 21:29:35 UTC
Hi Kyle,

We are testing this patchset for 24.05.x backport.

When we cherry-pick the first patch from 24.11.x onto 24.05.x it applies cleanly. But the second patch conflicts, see below:

---

kohadev-koha@kohadevbox:koha(24.05.x)$ git cherry-pick 34df58a5b98f8f36e8820666834a76a0bf7167d2
Auto-merging C4/SIP/ILS.pm
CONFLICT (content): Merge conflict in C4/SIP/ILS.pm
error: could not apply 34df58a5b98... Bug 38486: For SIP allow noblock checkouts reguardless of normal patron checkout blocks
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
kohadev-koha@kohadevbox:koha(24.05.x|CHERRY-PICKING)$ git status
On branch 24.05.x
You are currently cherry-picking commit 34df58a5b98.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   C4/SIP/ILS.pm

no changes added to commit (use "git add" and/or "git commit -a")
(failed reverse-i-search)`sed': re^Ct_all
kohadev-koha@kohadevbox:koha(24.05.x|CHERRY-PICKING)$

---

Could you please attach a 24.05.x rebase for the second patch to this bug report?

Thanks in advance,
Alex
Comment 14 Kyle M Hall (khall) 2025-03-20 12:01:50 UTC
Created attachment 179514 [details] [review]
Bug 38486: [24.05.x] Fix unit tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Kyle M Hall (khall) 2025-03-20 12:01:57 UTC
Created attachment 179515 [details] [review]
Bug 38486: [24.05.x] For SIP allow noblock checkouts reguardless of normal patron checkout blocks

The purpose of no block checkouts in SIP is to indicate that the SIP machine has made an offline ( "store and forward" ) transaction.
The patron already has the item. As such, the item must be checked out to the patron or the library risks losing the item do to lack of tracking.
As such, no block checkouts should not be blocked under any circumstances.

The sub C4::SIP::ILS::Transaction::Checkout::do_checkout already honors this, but it is wrapped by
C4::SIP::ILS::checkout which has additional checks that do not.

Test Plan:
1) Apply the unit test patch
2) prove t/db_dependent/SIP/Transaction.t will fail
3) Apply the second patch
4) prove t/db_dependent/SIP/Transaction.t will pass!

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Kyle M Hall (khall) 2025-03-20 12:03:55 UTC
> Could you please attach a 24.05.x rebase for the second patch to this bug
> report?

Here you go!
Comment 17 Alex Buckley 2025-03-20 20:29:55 UTC
(In reply to Kyle M Hall (khall) from comment #16)
> > Could you please attach a 24.05.x rebase for the second patch to this bug
> > report?
> 
> Here you go!

Thank you very much!
Comment 18 Alex Buckley 2025-03-20 20:30:39 UTC
Nice work everyone!

Pushed to 24.05.x for 24.05.08
Comment 19 Fridolin Somers 2025-03-24 10:02:36 UTC
Pushed to 23.11.x for 23.11.13
Comment 20 Jesse Maseto 2025-03-31 08:31:35 UTC
Patch doesn't apply to 22.11.x. Many merge conflicts.