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".
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.
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.
It seems to work fine as step 7 indicates without the patch?
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.