We've got four very similar system preferences with very different wording, which fosters confusion. ExcludeHolidaysFromMaxPickUpDelay is worded is "(Allow/Don't Allow) Closed days to be taken into account in reserves max pickup delay." OverdueNoticeCalendar is worded as "(Use Calendar/Ignore Calendar) when working out the period for overdue notices." useDaysMode is "Calculate the due date using (the calendar to skip days the library is closed/the calendar to push the due date to the next open day/circulation rules only)" finesCalendar is "Calculate fines based on days overdue (directly/not including days the library is closed." Standardizing wording across these four would make them all clearer as a group. If nothing else, I'd love to see the word "calendar" included somewhere in ExcludeHolidaysFromMaxPickUpDelay so it comes up in an easy search with the others.
Furthermore "closed days to be taken into account" on ExcludeHolidaysFromMaxPickUpDelay is very much open to two contradictory readings. Does taking closed days "into account" mean we are *counting* those days? Or does it mean we *are* using the calendar and therefore *not counting* those days?
Hi Andrew, do you want to make a suggestion for rewording? I could write up a patch from that or maybe you would want to do it? I'd read it like that: Allow closed days to be taken into account = we are not counting them, using the calendar, counting days the library is open) Don't allow closed days to be taken into account = we are counting them, ignoring the calendar, only counting 'days')
I'd go with: useDaysMode: (Use the calendar to skip days the library is closed / Use the calendar to push the due date to the next open day / Ignore the calendar) when calculating the date due. OverdueNoticeCalendar: (Use the calendar/Ignore the calendar) when calculating the period for overdue notices. finesCalendar: (Use the calendar/Ignore the calendar) when calculating the period for fines. ExcludeHolidaysFromMaxPickUpDelay: (Use the calendar/Ignore the calendar) when calculating the period for reserves max pickup delay. This makes all four follow the general syntax that's currently used for OverdueNoticeCalendar, which is really clear.
Created attachment 94791 [details] [review] Bug 22381: Change wording on Calendar-related system preferences TEST PLAN: 1. Apply Patch 2. Look at the 4 sysprefs (useDaysMode, OverdueNoticeCalendar, finesCalendar, ExcludeHolidaysFromMaxPickUpDelay) 3. Make sure the wording and options all make sense 4. Make sure the options are properly ignoring or using the calendar.
Created attachment 94925 [details] [review] Bug 22381: Change wording on Calendar-related system preferences Rebased this patch so it will apply with bz15260, test plan remains the same TEST PLAN: 1. Apply Patch 2. Look at the 4 sysprefs (useDaysMode, OverdueNoticeCalendar, finesCalendar, ExcludeHolidaysFromMaxPickUpDelay) 3. Make sure the wording and options all make sense 4. Make sure the options are properly ignoring or using the calendar.
Created attachment 95127 [details] [review] Bug 22381: Change wording on Calendar-related system preferences Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Conflict with bug 13958, please rebase.
Created attachment 95241 [details] [review] Bug 22381: Change wording on Calendar-related system preferences TEST PLAN: 1. Apply Patch 2. Look at the 4 sysprefs (useDaysMode, OverdueNoticeCalendar, finesCalendar, ExcludeHolidaysFromMaxPickUpDelay) 3. Make sure the wording and options all make sense 4. Make sure the options are properly ignoring or using the calendar.
I think you need to update the recently added SuspensionsCalendar as well 844 - Calculate suspension expiration based on days overdue 845 - pref: SuspensionsCalendar 846 type: choice 847 choices: 848 ignoreCalendar: directly. 849 noSuspensionsWhenClosed: not including days the library is closed.
Good catch! Thanks, Jonathan! So, following the wording used above, that's be: (Use the calendar/Ignore the calendar) when calculating the period for suspension expiration.
Created attachment 96002 [details] [review] Bug 22381: Change wording on Calendar-related system TEST PLAN: 1. Apply Patch 2. Look at the 5 sysprefs (useDaysMode, OverdueNoticeCalendar, finesCalendar, ExcludeHolidaysFromMaxPickUpDelay, SuspensionsCalendar) 3. Make sure the wording and options all make sense 4. Make sure the options are properly ignoring or using the calendar.
Created attachment 96262 [details] [review] Bug 22381: Change wording on Calendar-related system TEST PLAN: 1. Apply Patch 2. Look at the 5 sysprefs (useDaysMode, OverdueNoticeCalendar, finesCalendar, ExcludeHolidaysFromMaxPickUpDelay, SuspensionsCalendar) 3. Make sure the wording and options all make sense 4. Make sure the options are properly ignoring or using the calendar. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Fixed double 'when' on line 740 No errors
Created attachment 96316 [details] [review] Bug 22381: Change wording on Calendar-related system TEST PLAN: 1. Apply Patch 2. Look at the 5 sysprefs (useDaysMode, OverdueNoticeCalendar, finesCalendar, ExcludeHolidaysFromMaxPickUpDelay, SuspensionsCalendar) 3. Make sure the wording and options all make sense 4. Make sure the options are properly ignoring or using the calendar. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Fixed double 'when' on line 740 No errors Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We have a quick long option now, may be problematic for translated interface (?) https://snipboard.io/4cwWXl.jpg Passing QA anyway, not blocker (hopefully)
Nice work everyone! Pushed to master for 20.05
Pushed to 19.11.x for 19.11.02
backported to 19.05.x for 19.05.07
Oops, I was too hasty with my backport. This bug has a dependency that is not in 19.05.x, reverting this. No backported to 19.05.x
I think there is something wrong: About the pref ExcludeHolidaysFromMaxPickUpDelay, we switch "yes" (value=1) from "Allow Closed days to be taken into account in reserves max pickup delay" to "Ignore the calendar when calculating the period for reserves max pickup delay."
Just testing this and Jonathan is correct. To verify in master: - mark your library closed today and tomorrow - set ReservesMaxPickUpDelay to 1 day - set ExcludeHolidaysFromMaxPickUpDelay to "Ignore the calendar" - place hold, capture hold - hold expiration date is set to the day after tomorrow (skips closed days) - Set ExcludeHolidaysFromMaxPickUpDelay to "Use the calendar" - Place hold, capture hold - hold expiration date is set to tomorrow (doesn't skip closed days) This is 100% my error, thanks for catching it.
Andrew, did you open a bug report?
I have now. Bug 24858.