Bug 37966

Summary: When overriding a hold to renew a book the due date becomes "now" of not specified
Product: Koha Reporter: Michael Hafen <michael.hafen>
Component: CirculationAssignee: Michael Hafen <michael.hafen>
Status: Needs Signoff --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, kyle.m.hall, shi-yao.wang
Version: Main   
Hardware: All   
OS: All   
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

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.