From 650b1a260a725bb1b27ec6757a0bf3efa6987b7d Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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 @@

+ [% IF ( RENEW_ISSUE ) %] +

+ + +

+ [% END %] [% END %] [% IF ( RESERVE_WAITING ) %] -- 2.52.0