Bugzilla – Attachment 183858 Details for
Bug 32934
SIP checkouts using "no block" flag have a calculated due rather than the specified due date
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32934: Correctly honor the no block due date for SIP checkout messages
Bug-32934-Correctly-honor-the-no-block-due-date-fo.patch (text/plain), 4.73 KB, created by
Martin Renvoize (ashimema)
on 2025-07-08 12:15:58 UTC
(
hide
)
Description:
Bug 32934: Correctly honor the no block due date for SIP checkout messages
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-07-08 12:15:58 UTC
Size:
4.73 KB
patch
obsolete
>From 6a0dd1646a8ad6cd84992e1d2f8374d9afd84de1 Mon Sep 17 00:00:00 2001 >From: Lari Strand <lari.strand@koha-suomi.fi> >Date: Fri, 4 Oct 2024 15:18:29 +0300 >Subject: [PATCH] Bug 32934: Correctly honor the no block due date for SIP > checkout messages > >**Problem:** >When using SIP checkout messages with the "no block" flag, the specified due date >was being ignored. Instead of honoring the provided due date, the system would: >1. Use the due date as the transaction timestamp (incorrect behavior) >2. Calculate a new due date based on circulation rules >3. Return an empty due date field (AH) in the SIP response > >This affected SIP2 integrations where external systems needed to specify exact >due dates for checkouts, particularly for offline circulation scenarios. > >**Root Cause:** >The `ProcessOfflineIssue` function in C4::Circulation was only returning a >success/failure message, but the SIP checkout transaction needed access to the >resulting checkout object to properly set the due date. The SIP Transaction::Checkout >module was calling `ProcessOfflineIssue` but couldn't retrieve the checkout details >to populate the transaction's due date field. > >**Solution:** >1. Modified `ProcessOfflineIssue` to return both the status message AND the checkout > object: `return ( "Success.", $checkout );` >2. Updated `ProcessOfflineOperation` to handle the new return format by capturing > only the message: `( $report ) = ProcessOfflineIssue( $operation );` >3. Modified SIP Transaction::Checkout to capture both return values and use the > checkout object to set the proper due date via `duedatefromissue` >4. Fixed the timestamp parameter to use `dt_from_string` instead of the due date > >**Test Plan:** >1. **Unit Tests:** > - Run `prove t/db_dependent/Circulation/OfflineCirculation.t` to verify > ProcessOfflineIssue returns checkout object and respects due_date parameter > - Run `prove t/db_dependent/SIP/Transaction.t` to verify SIP checkout honors > no_block_due_date parameter > >2. **Manual SIP Testing:** > - Connect to SIP server via telnet on port 6001 > - Send checkout message with no block flag and specific due date: > `111YN20250115 18000120241201 180001AP|AO|AC|AD|AB<barcode>|AA<cardnumber>|` > - Verify response contains correct due date in AH field > - Verify checkout record in database has the specified due date > >3. **Regression Testing:** > - Verify normal SIP checkouts (without no block) still work correctly > - Verify offline circulation operations continue to function > - Verify ProcessOfflineOperation still processes queued operations correctly > >Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> >--- > C4/Circulation.pm | 10 +++++----- > C4/SIP/ILS/Transaction/Checkout.pm | 6 ++++-- > 2 files changed, 9 insertions(+), 7 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index bf93c32a590..932a9cbfd5c 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -4442,7 +4442,7 @@ sub ProcessOfflineOperation { > if ( $operation->{action} eq 'return' ) { > $report = ProcessOfflineReturn($operation); > } elsif ( $operation->{action} eq 'issue' ) { >- $report = ProcessOfflineIssue($operation); >+ ( $report ) = ProcessOfflineIssue( $operation ); > } elsif ( $operation->{action} eq 'payment' ) { > $report = ProcessOfflinePayment($operation); > } >@@ -4513,15 +4513,15 @@ sub ProcessOfflineIssue { > $operation->{timestamp}, > ); > } >- AddIssue( >+ my $checkout = AddIssue( > $patron, >- $operation->{'barcode'}, >- undef, >+ $operation->{barcode}, >+ $operation->{due_date}, > undef, > $operation->{timestamp}, > undef, > ); >- return "Success."; >+ return ( "Success.", $checkout ); > } else { > return "Borrower not found."; > } >diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm >index 247ba86ca3d..4909400493d 100644 >--- a/C4/SIP/ILS/Transaction/Checkout.pm >+++ b/C4/SIP/ILS/Transaction/Checkout.pm >@@ -156,13 +156,15 @@ sub do_checkout { > > if ($no_block_due_date) { > $overridden_duedate = $no_block_due_date; >- ProcessOfflineIssue( >+ my ( $msg, $checkout ) = ProcessOfflineIssue( > { > cardnumber => $patron->cardnumber, > barcode => $barcode, >- timestamp => $no_block_due_date, >+ due_date => $no_block_due_date, >+ timestamp => dt_from_string, > } > ); >+ $self->{due} = $self->duedatefromissue( $checkout, $itemnumber ); > } else { > > # can issue >-- >2.50.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32934
:
146450
|
146467
|
162737
|
172389
|
172399
|
172566
|
172567
|
183857
|
183858
|
183869
|
183870