Scenario: * The library has a 4wk loan item type and a 1hr loan item type. * The library would like 1hr loan item types to be issued and returned on a day marked as closed in the circ calendar, but only allow 4wk loan types to be returned on an open day. Currently, if a 1hr loan is issued on a closed day, the due date is pushed back to the next open day, overriding the duration of the loan. If the usedaysmode syspref is set to "Circulation rules only" due dates for 1hr loan items are calculated at 1hr later as defined in the circ rules, however 4wk loan due dates can potentially be set to closed days.
Created attachment 99758 [details] [review] Bug 24159: Move useDaysMode pref to circulation rules Moving the useDaysMode system preference to a circulation rule will add much more flexibility in the calculation of the due date. The initial request was to make hourly loan returned on closed when (when checked out on the same close day). To do so we do not want to take into account the calendar. However the calendar need to be taken into account for other loan item types. Other scenarios are possible, for instance depending on the branch. This patchset will add a new "Days mode" column (next to "Loan period") to the circulation rules page, with the different values of the "useDaysMode" system preference + a "default" value, to default to the system preference value. Test plan: - Define a long loan item type (like 10 days) that will use the calendar (or default to the pref value, if the pref is not set to "ignore the calendar") - and a hourly loan (like 2 hours) that will ignore the calendar - Create items with those item types - Mark today as a closed day - Check the items out => The hourly loan is due the same day => The other loan is due on an open day QA note: There is the need to force the "days_mode" option when Koha::Calendar is initiated for the due date calculation. To make sure devs will not forget it, the methods that need have it defined will throw an exception. Sponsored-by: Institute of Technology Carlow
Created attachment 99759 [details] [review] Bug 24159: Throw an exception if days_mode option is not given when needed Sponsored-by: Institute of Technology Carlow
Created attachment 99760 [details] [review] Bug 24159: Add UI changes Sponsored-by: Institute of Technology Carlow
Created attachment 99761 [details] [review] Bug 24159: Adjust tests Sponsored-by: Institute of Technology Carlow
Created attachment 99762 [details] [review] Bug 24159: Add new tests for get_useDaysMode_effective_value Sponsored-by: Institute of Technology Carlow
Created attachment 100608 [details] [review] Bug 24159: Move useDaysMode pref to circulation rules Moving the useDaysMode system preference to a circulation rule will add much more flexibility in the calculation of the due date. The initial request was to make hourly loan returned on closed when (when checked out on the same close day). To do so we do not want to take into account the calendar. However the calendar need to be taken into account for other loan item types. Other scenarios are possible, for instance depending on the branch. This patchset will add a new "Days mode" column (next to "Loan period") to the circulation rules page, with the different values of the "useDaysMode" system preference + a "default" value, to default to the system preference value. Test plan: - Define a long loan item type (like 10 days) that will use the calendar (or default to the pref value, if the pref is not set to "ignore the calendar") - and a hourly loan (like 2 hours) that will ignore the calendar - Create items with those item types - Mark today as a closed day - Check the items out => The hourly loan is due the same day => The other loan is due on an open day QA note: There is the need to force the "days_mode" option when Koha::Calendar is initiated for the due date calculation. To make sure devs will not forget it, the methods that need have it defined will throw an exception. Sponsored-by: Institute of Technology Carlow
Created attachment 100609 [details] [review] Bug 24159: Throw an exception if days_mode option is not given when needed Sponsored-by: Institute of Technology Carlow
Created attachment 100610 [details] [review] Bug 24159: Add UI changes Sponsored-by: Institute of Technology Carlow
Created attachment 100611 [details] [review] Bug 24159: Adjust tests Sponsored-by: Institute of Technology Carlow
Created attachment 100612 [details] [review] Bug 24159: Add new tests for get_useDaysMode_effective_value Sponsored-by: Institute of Technology Carlow
Created attachment 100613 [details] [review] Bug 24159: Set days_mode according to circ rules in 3 other places There are 3 other occurrences where the new circ rule can be used: * C4::Circulation::checkHighHolds * Koha::Hold->set_waiting * misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl Test plan: * checkHighHolds Enable decreaseLoanHighHolds and fill decreaseLoanHighHoldsDuration Setup things to hit a "high demand" alert with a shortened due date Check an item out => The due date must be recalculated depending on the circ rule useDaysMode. * set_waiting Set ExcludeHolidaysFromMaxPickUpDelay to "1" (note that there is currently a bug in the description of the syspref, see bug 22381 comment 19) Mark a hold waiting The expiration date should have been set depending on the value of the circ rule. * TalkingTech cronjob Cannot test this
Created attachment 100614 [details] [review] Bug 24159: Use days_mode circ rule to calculate the dropbox date Calculate the drop box date depending on the circ rule. Note that only the branchcode is taken into account here. Is that correct? Test plan: Use the drop box date feature and confirm that it takes into account the calendar, depending on the circ rule.
Created attachment 100615 [details] [review] Bug 24159: Fix one occurrence in updatedatabase.pl Here we just want to get the value from the syspref, the circ rule does not exist yet.
Created attachment 101053 [details] Test
Created attachment 101054 [details] Test
Hello Jonathan I'm coordinating testing with Simon Perry, and he raised the following issue: "I’ve hit problems with this bug. I created a test bibliographic record with two items records attached (one as an hourly loan and one as a long loan – see attachment for details), set yesterday as a closed day in the calendar and issued both items to a patron. I hadn’t changed the UseDaysMode syspref, so I wanted to check the items in, change the syspref and then reissue. I’ve found that I can’t check either item in and am getting a string of error messages when I try to check in (attached). Note that I deleted the closed day entry in the calendar for yesterday to see if it would make a difference to checkin – it didn’t work. Do you want to investigate or create a new sandbox so that I can retest?"
Hi Paul, Can you confirm you were testing with all the patches applied? Mark reported me this error by email, but that should have been fixed by the patches I attached here on March 12th.
Hello Jonathan I touched base with Simon and he says the best thing would be if PTFS can re-send the link to the sandbox for this bug and he can retest. Can you organise that please?
Created attachment 101556 [details] Test 24/3/20 Attached is testing completed today. I tested all 4 options available in the useDaysMode syspref. Only for the Ignore and Use calendar to push due date to next pen matching weekday.... syspref settings did the checkout due date/hour remain on the days of checkout. This wouldn't work for our live system settings as noted. Regards, Simon
Hello Simon, This development moves the system preference to the circulation rules, for more granularity. On the circulation rules you should see a new column "Days mode" with 5 different options. In your case, I think what you need is: Set the syspref to "Push the due date to next open day" Define a circulation rule for Hourly loan and select "Ignore the Calendar" in the "Days mode" column. Note that there is a small cosmetic bug on the circulation table, the header of the table says "Unit" instead of "Days mode". The footer is correct but there is a shift in the header.
Created attachment 101649 [details] [review] Bug 24159: Fix shift in the header
Created attachment 101698 [details] Test 25/3/20 Hi Jonathan, thanks for clarification. I just retested with today set as closed day in Calendar, Days mode in hourly circulation rule set to 'Calendar' (the dropdown menu for the Days Mode only says 'Calendar' not 'Ignore the Calendar' btw) and useDaysMode syspref set to 'Use the Calendar to push the due date to next due day.' When I checked out an hourly loan item, the due date was moved on to tomorrow, not retained as today's. Please see screenshot attachment for details. Do you want me to test with other options set for useDaysMode etc.? Simon
Simon, I think I understand where the confusion comes from. The values in the circulation rules have the following meaning: Days: Ignore the calendar Calendar: Use the calendar to skip days the library is closed Datedue: Use the calendar to push the due date to the next open day Dayweek: Use the calendar to push the due date to the next open matching weekday for weekly loan periods, or the next open day otherwise They are the "internal values" of the syspref. It's not ideal but we cannot use the long descriptions in the dropdown list in the circ rules table, that will be too wide. It seems that it is not enough but to avoid the confusion there is a tooltip that is displayed (over the cursor on the cell): https://snipboard.io/We6flu.jpg it cannot be seen on the screenshot, but the cursor is on "Days". If you have anything else to suggest to make it less confusing, let me know!
Thanks for clarification Jonathan, from your comments it looks like I should be selecting the 'Days' (ignore calendar) for the hourly loan. Am I right in assuming that the DaysMode column in the Circulation rules table will override what is set in the useDaysMode syspref? And therefore if the Days Mode column in the Circ. rules table is set to 'Default' it will use whatever value is set for the useDaysMode syspref? Tooltip is useful and I guess the other thing is to ensure it is well documented in the Koha circulation help. I'll retest and feedback results. Regards, Simon
(In reply to Simon Perry from comment #24) > Thanks for clarification Jonathan, from your comments it looks like I should > be selecting the 'Days' (ignore calendar) for the hourly loan. Am I right in > assuming that the DaysMode column in the Circulation rules table will > override what is set in the useDaysMode syspref? And therefore if the Days > Mode column in the Circ. rules table is set to 'Default' it will use > whatever value is set for the useDaysMode syspref? Yes, that's exactly how it is supposed to work!
Created attachment 102009 [details] testing 270320 Hi Jonathan, I've now tested with UsedaysMode set to 'Push due date to next open day' and Days Mode set to 'Days' for the hourly loan rule. Checkout for an hourly loan item type with today set as a library closed day correctly retained today as the due date with an hour's loan allowed. See screenshots of test. I'd like to do some more testing with different useDaysMode syspref values before signing off. Unfortunately the sandbox has got very slow for some reason this afternoon (in fact I cannot connect to it right now). Maybe it could do with a refresh? I'll check again on Monday. Regards, Simon
Created attachment 102148 [details] Bug 24159 test 31 March Hi Jonathan, Just started testing again. I had the sandbox set to the following configuration: (1) useDaysMode syspref – Use Calendar to push due date to the next open day Today – closed day in calendar Circulation rules hourly loan Days Mode column – days (ignore calendar) Result Hourly loan – correct loan due date same 1hr loan Long loan -correct due date 6/4/20 the hourly loan checked out correctly retaining today's date and advancing due time by 1 hour from checkout, but when I tried to check the hourly loan and the long loan I got a checkin error again. See attachment for details. Not sure if this is just sandbox playing up or something else but thought you should know. Regards, Simon
Thanks for reporting that Simon, that seems a sandbox-related bug. I will ask Martin.
(In reply to Jonathan Druart from comment #28) > Thanks for reporting that Simon, that seems a sandbox-related bug. I will > ask Martin. PTFS-E Sandboxes updated to use a newer MariaDB version with the auto-increment fix.
Created attachment 102303 [details] Sandbox error Hi, I've just tried to complete testing but am getting an error when I try to add a new bib record or item for testing. See attached for details. Thanks, Simon
Hello Simon, We have been fighting with an annoying bugs the last 2 days. The error you saw is not related to these patches, but bug 25040 (but you do not need to care about, it's technical). We fixed it in the sandbox manually. I do not know if you have been told to "refresh the schema" before trying the sandbox, but you should not until bug 25040 is pushed to master. Basically the sandbox is ready for testing, biblio and item creation is ok now.
Hi, I lost contact with http://staff-bug24159.sandboxes.ptfs-europe.co.uk/ on Friday afternoon. I still can't connect today. I just want to run a couple more tests before signing off so could access be restored please. Thanks, Simon
Created attachment 102554 [details] Final patch testing Hi, I've completed final testing (details attached for information). I tried a few different settings in the Circulation Days Mode column just to check that other options worked as I would expect. Now signing off bug. Regards, Simon
Created attachment 102555 [details] [review] Bug 24159: Move useDaysMode pref to circulation rules Moving the useDaysMode system preference to a circulation rule will add much more flexibility in the calculation of the due date. The initial request was to make hourly loan returned on closed when (when checked out on the same close day). To do so we do not want to take into account the calendar. However the calendar need to be taken into account for other loan item types. Other scenarios are possible, for instance depending on the branch. This patchset will add a new "Days mode" column (next to "Loan period") to the circulation rules page, with the different values of the "useDaysMode" system preference + a "default" value, to default to the system preference value. Test plan: - Define a long loan item type (like 10 days) that will use the calendar (or default to the pref value, if the pref is not set to "ignore the calendar") - and a hourly loan (like 2 hours) that will ignore the calendar - Create items with those item types - Mark today as a closed day - Check the items out => The hourly loan is due the same day => The other loan is due on an open day QA note: There is the need to force the "days_mode" option when Koha::Calendar is initiated for the due date calculation. To make sure devs will not forget it, the methods that need have it defined will throw an exception. Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 102556 [details] [review] Bug 24159: Throw an exception if days_mode option is not given when needed Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 102557 [details] [review] Bug 24159: Add UI changes Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 102558 [details] [review] Bug 24159: Adjust tests Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 102559 [details] [review] Bug 24159: Add new tests for get_useDaysMode_effective_value Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 102560 [details] [review] Bug 24159: Set days_mode according to circ rules in 3 other places There are 3 other occurrences where the new circ rule can be used: * C4::Circulation::checkHighHolds * Koha::Hold->set_waiting * misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl Test plan: * checkHighHolds Enable decreaseLoanHighHolds and fill decreaseLoanHighHoldsDuration Setup things to hit a "high demand" alert with a shortened due date Check an item out => The due date must be recalculated depending on the circ rule useDaysMode. * set_waiting Set ExcludeHolidaysFromMaxPickUpDelay to "1" (note that there is currently a bug in the description of the syspref, see bug 22381 comment 19) Mark a hold waiting The expiration date should have been set depending on the value of the circ rule. * TalkingTech cronjob Cannot test this Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 102561 [details] [review] Bug 24159: Use days_mode circ rule to calculate the dropbox date Calculate the drop box date depending on the circ rule. Note that only the branchcode is taken into account here. Is that correct? Test plan: Use the drop box date feature and confirm that it takes into account the calendar, depending on the circ rule. Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 102562 [details] [review] Bug 24159: Fix one occurrence in updatedatabase.pl Here we just want to get the value from the syspref, the circ rule does not exist yet. Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 102563 [details] [review] Bug 24159: Fix shift in the header Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Thanks for the test session, Simon!
This no longer applies - can you please rebase? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 24159: Move useDaysMode pref to circulation rules Applying: Bug 24159: Throw an exception if days_mode option is not given when needed Applying: Bug 24159: Add UI changes Applying: Bug 24159: Adjust tests Applying: Bug 24159: Add new tests for get_useDaysMode_effective_value Applying: Bug 24159: Set days_mode according to circ rules in 3 other places error: sha1 information is lacking or useless (C4/Circulation.pm). error: could not build fake ancestor
Created attachment 104197 [details] [review] Bug 24159: Set days_mode according to circ rules in 3 other places There are 3 other occurrences where the new circ rule can be used: * C4::Circulation::checkHighHolds * Koha::Hold->set_waiting * misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl Test plan: * checkHighHolds Enable decreaseLoanHighHolds and fill decreaseLoanHighHoldsDuration Setup things to hit a "high demand" alert with a shortened due date Check an item out => The due date must be recalculated depending on the circ rule useDaysMode. * set_waiting Set ExcludeHolidaysFromMaxPickUpDelay to "1" (note that there is currently a bug in the description of the syspref, see bug 22381 comment 19) Mark a hold waiting The expiration date should have been set depending on the value of the circ rule. * TalkingTech cronjob Cannot test this
Created attachment 104198 [details] [review] Bug 24159: Use days_mode circ rule to calculate the dropbox date Calculate the drop box date depending on the circ rule. Note that only the branchcode is taken into account here. Is that correct? Test plan: Use the drop box date feature and confirm that it takes into account the calendar, depending on the circ rule.
Created attachment 104199 [details] [review] Bug 24159: Fix one occurrence in updatedatabase.pl Here we just want to get the value from the syspref, the circ rule does not exist yet.
Created attachment 104200 [details] [review] Bug 24159: Fix shift in the header
Created attachment 104201 [details] [review] Bug 24159: Set days_mode according to circ rules in 3 other places There are 3 other occurrences where the new circ rule can be used: * C4::Circulation::checkHighHolds * Koha::Hold->set_waiting * misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl Test plan: * checkHighHolds Enable decreaseLoanHighHolds and fill decreaseLoanHighHoldsDuration Setup things to hit a "high demand" alert with a shortened due date Check an item out => The due date must be recalculated depending on the circ rule useDaysMode. * set_waiting Set ExcludeHolidaysFromMaxPickUpDelay to "1" (note that there is currently a bug in the description of the syspref, see bug 22381 comment 19) Mark a hold waiting The expiration date should have been set depending on the value of the circ rule. * TalkingTech cronjob Cannot test this Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 104202 [details] [review] Bug 24159: Use days_mode circ rule to calculate the dropbox date Calculate the drop box date depending on the circ rule. Note that only the branchcode is taken into account here. Is that correct? Test plan: Use the drop box date feature and confirm that it takes into account the calendar, depending on the circ rule. Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 104203 [details] [review] Bug 24159: Fix one occurrence in updatedatabase.pl Here we just want to get the value from the syspref, the circ rule does not exist yet. Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 104204 [details] [review] Bug 24159: Fix shift in the header Signed-off-by: Simon Perry <simon.perry@itcarlow.ie>
Created attachment 106007 [details] [review] Bug 24159: Move useDaysMode pref to circulation rules Moving the useDaysMode system preference to a circulation rule will add much more flexibility in the calculation of the due date. The initial request was to make hourly loan returned on closed when (when checked out on the same close day). To do so we do not want to take into account the calendar. However the calendar need to be taken into account for other loan item types. Other scenarios are possible, for instance depending on the branch. This patchset will add a new "Days mode" column (next to "Loan period") to the circulation rules page, with the different values of the "useDaysMode" system preference + a "default" value, to default to the system preference value. Test plan: - Define a long loan item type (like 10 days) that will use the calendar (or default to the pref value, if the pref is not set to "ignore the calendar") - and a hourly loan (like 2 hours) that will ignore the calendar - Create items with those item types - Mark today as a closed day - Check the items out => The hourly loan is due the same day => The other loan is due on an open day QA note: There is the need to force the "days_mode" option when Koha::Calendar is initiated for the due date calculation. To make sure devs will not forget it, the methods that need have it defined will throw an exception. Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106008 [details] [review] Bug 24159: Throw an exception if days_mode option is not given when needed Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106013 [details] [review] Bug 24159: Move useDaysMode pref to circulation rules Moving the useDaysMode system preference to a circulation rule will add much more flexibility in the calculation of the due date. The initial request was to make hourly loan returned on closed when (when checked out on the same close day). To do so we do not want to take into account the calendar. However the calendar need to be taken into account for other loan item types. Other scenarios are possible, for instance depending on the branch. This patchset will add a new "Days mode" column (next to "Loan period") to the circulation rules page, with the different values of the "useDaysMode" system preference + a "default" value, to default to the system preference value. Test plan: - Define a long loan item type (like 10 days) that will use the calendar (or default to the pref value, if the pref is not set to "ignore the calendar") - and a hourly loan (like 2 hours) that will ignore the calendar - Create items with those item types - Mark today as a closed day - Check the items out => The hourly loan is due the same day => The other loan is due on an open day QA note: There is the need to force the "days_mode" option when Koha::Calendar is initiated for the due date calculation. To make sure devs will not forget it, the methods that need have it defined will throw an exception. Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106014 [details] [review] Bug 24159: Throw an exception if days_mode option is not given when needed Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106015 [details] [review] Bug 24159: Add UI changes Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106016 [details] [review] Bug 24159: Adjust tests Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106017 [details] [review] Bug 24159: Add new tests for get_useDaysMode_effective_value Sponsored-by: Institute of Technology Carlow Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106018 [details] [review] Bug 24159: Set days_mode according to circ rules in 3 other places There are 3 other occurrences where the new circ rule can be used: * C4::Circulation::checkHighHolds * Koha::Hold->set_waiting * misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl Test plan: * checkHighHolds Enable decreaseLoanHighHolds and fill decreaseLoanHighHoldsDuration Setup things to hit a "high demand" alert with a shortened due date Check an item out => The due date must be recalculated depending on the circ rule useDaysMode. * set_waiting Set ExcludeHolidaysFromMaxPickUpDelay to "1" (note that there is currently a bug in the description of the syspref, see bug 22381 comment 19) Mark a hold waiting The expiration date should have been set depending on the value of the circ rule. * TalkingTech cronjob Cannot test this Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106019 [details] [review] Bug 24159: Use days_mode circ rule to calculate the dropbox date Calculate the drop box date depending on the circ rule. Note that only the branchcode is taken into account here. Is that correct? Test plan: Use the drop box date feature and confirm that it takes into account the calendar, depending on the circ rule. Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106020 [details] [review] Bug 24159: Fix one occurrence in updatedatabase.pl Here we just want to get the value from the syspref, the circ rule does not exist yet. Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106021 [details] [review] Bug 24159: Fix shift in the header Signed-off-by: Simon Perry <simon.perry@itcarlow.ie> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 106022 [details] [review] Bug 24159: (QA follow-up) Make terminology consistent This patch changes the new circulation rule that's introduced from useDaysMode to daysmode to improve consistency with other rule names. We also update the accessors and code using them to reflect the new term. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This works well and passes the qa script. I have added a trivial followup to update the new rule name and introduced accessor to more closely match the existing rules and methods for consistency. I would love to see some future work, and with any luck I can contribute it, to further 'simplify' the Koha::CirculationRules module to make it more self-consistent but that work doesn't need to happen here. Passing QA
Hi, would you agree that this is not a bug? It appears to add new configuration options in the circulation rules to allow covering more uses cases - this is valid but I'd see it more as an enhancement. I'd also love to see a more descriptive bug title that tells us more about what this patchset does. I really like the default value mentioned in the patch description that allows fallback to the pref - I was worried a bit about having to set yet another value!
Suggestion for the bug title: "Move useDaysMode preference for calculating due dates to the circulation rules"
(In reply to Katrin Fischer from comment #67) > Suggestion for the bug title: > > "Move useDaysMode preference for calculating due dates to the circulation > rules" I agree it's an enhancement as it stands... as for the title how about: "Allow daysMode to be set at the circulation rules level" I think that still conveys the right message but also hints that the original system preference is intact?
I sneaked in a few more words :)
Pushed to master for 20.11, thanks to everybody involved!
Small (below trivial :P) postfix: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25896
not backporting enhancement to 20.05.x series