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.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
parent
14de13d3ad
commit
d463fce02d
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 ) %]
|
||||
|
|
|
|||
Loading…
Reference in New Issue