Bug 37966 - When overriding a hold to renew a book the due date becomes "now" if not specified
Summary: When overriding a hold to renew a book the due date becomes "now" if not spec...
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Michael Hafen
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-09-19 18:05 UTC by Michael Hafen
Modified: 2025-04-03 09:39 UTC (History)
6 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 37966: When Renewing a book on hold, don't require the due date (2.35 KB, patch)
2024-09-19 18:33 UTC, Michael Hafen
Details | Diff | Splinter Review
When Renewing a book on hold, don't require the due date (2.35 KB, patch)
2025-03-19 17:09 UTC, Michael Hafen
Details | Diff | Splinter Review
Bug 37966: When Renewing a book on hold, don't require the due date (2.37 KB, patch)
2025-04-03 09:36 UTC, Marion Durand
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Hafen 2024-09-19 18:05:27 UTC
In circ/renew.pl, when trying to renew a book, if the book is on hold the override dialog is presented.  If the 'renewonholdduedate' field is left empty, then the due date ends up being "now".
Comment 1 Michael Hafen 2024-09-19 18:20:51 UTC
I just read the description for the RenewalOnHoldOverride sys pref ("Allow staff to renew items that are on hold by manually specifying a due date."), and it seems this is more expected behavior than bug.
Still I think an improvement is needed.  First off, the date due field in that dialog didn't seem required to me.
I see three options:  Change the sys pref description and the code, calculate the new due date and use that as a default, or just change the style of the due date field to indicate it is required.
I'm leaning toward changing the sys pref and the code, but I'm open to other opinions.
Comment 2 Michael Hafen 2024-09-19 18:33:10 UTC
Created attachment 171790 [details] [review]
Bug 37966: When Renewing a book on hold, don't require the  due date

When renewing a book on hold with the RenewalOnHoldOverride sys pref set
to "Allow", if the due date is not specified then it becomes "now".  By
checking the input is set rather than just defined the due date is set as
expected based on circulation rules.
Also change the description of the RenewalOnHoldOverride sys pref as the
due date would no longer be required to be entered with this change.

Test plan:
1. make sure RenewalOnHoldOverride is set to 'Allow'
2. check out a book, then place a title-level hold on it
3. renew the book, see the hold override dialog
4. leave the due date empty and hit the override button
5. observe the new due date is "now"
6. apply patch and restart plack
7. renew the book again, overriding the hold, and see the expected due date
   as per circulation rules.
Comment 3 Shi Yao Wang 2024-09-26 18:22:40 UTC
It seems to work fine as step 7 indicates without the patch?
Comment 4 Michael Hafen 2024-10-02 21:11:22 UTC
On my dev install of the main branch on my workstation, which I pulled Sept. 30th, it behaves as the plan outlines, with the new due date on renewal, after overriding the hold, being the date and time that the book was renewed rather than the expected 3 weeks later (in my case) when the new due date field in the hold override dialog is left empty.
I don't know why it didn't behave as expected for you.
Comment 5 Laurence Rault 2025-03-19 13:58:56 UTC
Patch does not apply on sandbox : 
CONFLICT (content): Merge conflict in C4/Circulation.pm
Comment 6 Michael Hafen 2025-03-19 17:09:54 UTC
Created attachment 179490 [details] [review]
When Renewing a book on hold, don't require the  due date

[Rebased 2025-03-19]
When renewing a book on hold with the RenewalOnHoldOverride sys pref set
to "Allow", if the due date is not specified then it becomes "now".  By
checking the input is set rather than just defined the due date is set as
expected based on circulation rules.
Also change the description of the RenewalOnHoldOverride sys pref as the
due date would no longer be required to be entered with this change.

Test plan:
1. make sure RenewalOnHoldOverride is set to 'Allow'
2. check out a book, then place a title-level hold on it
3. renew the book, see the hold override dialog
4. leave the due date empty and hit the override button
5. observe the new due date is "now"
6. apply patch and restart plack
7. renew the book again, overriding the hold, and see the expected due date
   as per circulation rules.
Comment 7 Baptiste Wojtkowski (bwoj) 2025-03-20 14:59:33 UTC
Could not reproduce the issue here. The date correctly updates for me on step 4
Comment 8 Michael Hafen 2025-03-20 17:53:36 UTC
Maybe I didn't describe step 4 well enough, or maybe there's something in my preferences, but I just tried this on master and the due date went from 4/3/2025 to 3/20/2025 (today) when using the Renew page after overriding the hold when the 'Override due date' field was left empty.
Looking through the preferences, the only one I see, besides the AllowRenewal*Override ones that might affect this is 'RenewalPeriodBase' which I have set to 'the current date'.
Comment 9 Marion Durand 2025-04-03 09:36:52 UTC
Created attachment 180478 [details] [review]
Bug 37966: When Renewing a book on hold, don't require the due date

When renewing a book on hold with the RenewalOnHoldOverride sys pref set
to "Allow", if the due date is not specified then it becomes "now".  By
checking the input is set rather than just defined the due date is set as
expected based on circulation rules.
Also change the description of the RenewalOnHoldOverride sys pref as the
due date would no longer be required to be entered with this change.

Test plan:
1. make sure RenewalOnHoldOverride is set to 'Allow'
2. check out a book, then place a title-level hold on it
3. renew the book, see the hold override dialog
4. leave the due date empty and hit the override button
5. observe the new due date is "now"
6. apply patch and restart plack
7. renew the book again, overriding the hold, and see the expected due date
   as per circulation rules.

Signed-off-by: Marion Durand <marion.durand@biblibre.com>