Bugzilla – Attachment 191259 Details for
Bug 37966
When overriding a hold to renew a book the due date becomes "now" if not specified
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37966: Fix renewal on hold override in circulation.pl
650b1a2.patch (text/plain), 5.83 KB, created by
Martin Renvoize (ashimema)
on 2026-01-12 14:07:27 UTC
(
hide
)
Description:
Bug 37966: Fix renewal on hold override in circulation.pl
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-01-12 14:07:27 UTC
Size:
5.83 KB
patch
obsolete
>From 650b1a260a725bb1b27ec6757a0bf3efa6987b7d Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Mon, 12 Jan 2026 14:03:37 +0000 >Subject: [PATCH] Bug 37966: Fix renewal on hold override in circulation.pl > >This extends the fix from Bug 37966 to circ/circulation.pl. The original >bug fix addressed circ/renew.pl, where renewing a book on hold with an >empty due date would result in the due date being set to "now" (today). > >The same issue occurs in circ/circulation.pl when: >1. Attempting to check out an item that is already checked out to the > same patron (which triggers a renewal) >2. The item has a hold on it, triggering the hold override dialog >3. The librarian overrides the hold without specifying a due date > >In this scenario, the renewal would incorrectly set the due date to >today instead of calculating it based on circulation rules. > >CHANGES: > >1. circ/circulation.pl (lines 550-558) > - Added logic to handle the `renewonholdduedate` parameter when > both RENEW_ISSUE and RESERVED conditions are present > - Follows the same pattern as circ/renew.pl > - If `renewonholdduedate` is empty, passes empty string to AddIssue/ > AddRenewal, allowing proper date calculation > >2. koha-tmpl/.../circulation.tt (lines 351-356) > - Added "Renewal due date" input field in the hold override dialog > - Only appears when both RESERVED and RENEW_ISSUE are true > - Optional field - if left empty, due date is calculated per > circulation rules > >TEST PLAN: > >1. Enable the RenewalOnHoldOverride system preference (set to "Allow") > >2. Set up a patron, item, and circulation rules: > - Create a patron (or use existing) > - Create an item (or use existing) > - Ensure circulation rules have a renewal period (e.g., 14 days) > >3. Create a hold scenario: > - Check out the item to the patron > - Place a title-level hold on the same item (you may need a second > patron or use the same patron depending on your holds rules) > >4. Test renewal via circulation.pl: > - Go to Circulation (Home > Circulation) > - Scan the patron's barcode > - Scan the item's barcode (the one that's already checked out with > a hold) > - Observe the hold override dialog appears > >5. Test WITHOUT this patch: > - Leave the "Renewal due date" field empty > - Click "Yes, renew" > - BUG: The due date is set to today (same as check-in date) > >6. Test WITH this patch: > - Leave the "Renewal due date" field empty > - Click "Yes, renew" > - SUCCESS: The due date is calculated based on circulation rules > (e.g., 14 days from now if that's your renewal period) > >7. Test optional due date override: > - Repeat the renewal process > - This time, enter a specific date in "Renewal due date" field > - Click "Yes, renew" > - SUCCESS: The due date is set to your specified date > >8. Test SpecifyDueDate preference interaction: > - If SpecifyDueDate is enabled and a due date is set via that > interface, it should take precedence over renewonholdduedate > >TECHNICAL NOTES: > >The fix ensures that when `renewonholdduedate` is empty (empty string), >it's passed through to AddIssue which then calls AddRenewal. The >improved empty string handling in C4::Circulation (from the follow-up >commit) ensures empty strings are not parsed as "now" but instead >trigger proper date calculation based on circulation rules. > >This matches the behavior fixed in circ/renew.pl and provides a >consistent user experience across both circulation interfaces. >--- > circ/circulation.pl | 11 +++++++++++ > .../intranet-tmpl/prog/en/modules/circ/circulation.tt | 6 ++++++ > 2 files changed, 17 insertions(+) > >diff --git a/circ/circulation.pl b/circ/circulation.pl >index dddd2a1f5f9..ad770f2a8f2 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -547,6 +547,17 @@ if ( @$barcodes && $op eq 'cud-checkout' ) { > if ( my $booked = $needsconfirmation->{BOOKED_EARLY} // $alerts->{BOOKED} ) { > $datedue = $booked->end_date; > } >+ >+ # If renewing an item on hold, use renewonholdduedate (similar to renew.pl) >+ # This handles the same issue as Bug 37966 for circ/renew.pl >+ if ( $needsconfirmation->{RENEW_ISSUE} && $needsconfirmation->{RESERVED} ) { >+ >+ # Use duedatespec if SpecifyDueDate is enabled and duedatespec is provided >+ # Otherwise use renewonholdduedate (may be empty string, letting AddRenewal calculate) >+ if ( !( $duedatespec_allow && $duedatespec ) ) { >+ $datedue = $query->param('renewonholdduedate'); >+ } >+ } > $needsconfirmation->{'DEBT'} = $needsconfirmationDEBT if ($debt_confirmed); > my $issue = AddIssue( > $patron, $barcode, $datedue, >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt >index 672d27da176..39430abe791 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt >@@ -348,6 +348,12 @@ > <input type="checkbox" id="cancelreserve" name="cancelreserve" value="cancel" /> > <label for="cancelreserve">Cancel hold</label> > </p> >+ [% IF ( RENEW_ISSUE ) %] >+ <p> >+ <label for="renewonholdduedate">Renewal due date:</label> >+ <input type="text" size="20" id="renewonholdduedate" name="renewonholdduedate" value="" class="flatpickr" data-flatpickr-enable-time="true" /> >+ </p> >+ [% END %] > [% END %] > > [% IF ( RESERVE_WAITING ) %] >-- >2.52.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 37966
:
171790
|
179490
|
180478
|
188524
|
190590
|
190591
|
191256
|
191257
|
191258
| 191259