Bug 22381 - Wording on Calendar-related system preferences not standardized
Summary: Wording on Calendar-related system preferences not standardized
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on: 15260
Blocks: 24858
  Show dependency treegraph
 
Reported: 2019-02-20 22:48 UTC by Andrew Fuerste-Henry
Modified: 2021-06-14 21:28 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.02


Attachments
Bug 22381: Change wording on Calendar-related system preferences (3.26 KB, patch)
2019-10-25 16:06 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22381: Change wording on Calendar-related system preferences (3.10 KB, patch)
2019-10-31 18:11 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22381: Change wording on Calendar-related system preferences (3.15 KB, patch)
2019-11-06 21:23 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22381: Change wording on Calendar-related system preferences (3.13 KB, patch)
2019-11-08 20:26 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22381: Change wording on Calendar-related system (3.55 KB, patch)
2019-12-04 23:52 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22381: Change wording on Calendar-related system (3.91 KB, patch)
2019-12-13 15:20 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 22381: Change wording on Calendar-related system (3.99 KB, patch)
2019-12-16 11:30 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2019-02-20 22:48:23 UTC
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.
Comment 1 Andrew Fuerste-Henry 2019-03-04 23:15:08 UTC
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?
Comment 2 Katrin Fischer 2019-08-18 09:16:01 UTC
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')
Comment 3 Andrew Fuerste-Henry 2019-08-19 12:36:41 UTC
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.
Comment 4 Lucas Gass 2019-10-25 16:06:21 UTC
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.
Comment 5 Lucas Gass 2019-10-31 18:11:11 UTC
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.
Comment 6 ByWater Sandboxes 2019-11-06 21:23:58 UTC
Created attachment 95127 [details] [review]
Bug 22381: Change wording on Calendar-related system preferences

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 7 Jonathan Druart 2019-11-07 11:35:41 UTC
Conflict with bug 13958, please rebase.
Comment 8 Lucas Gass 2019-11-08 20:26:34 UTC
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.
Comment 9 Jonathan Druart 2019-11-19 08:13:56 UTC
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.
Comment 10 Andrew Fuerste-Henry 2019-11-19 14:12:39 UTC
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.
Comment 11 Lucas Gass 2019-12-04 23:52:13 UTC
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.
Comment 12 Bernardo Gonzalez Kriegel 2019-12-13 15:20:04 UTC
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
Comment 13 Jonathan Druart 2019-12-16 11:30:09 UTC
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>
Comment 14 Jonathan Druart 2019-12-16 11:31:37 UTC
We have a quick long option now, may be problematic for translated interface (?)

https://snipboard.io/4cwWXl.jpg

Passing QA anyway, not blocker (hopefully)
Comment 15 Martin Renvoize 2019-12-16 21:08:02 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 16 Joy Nelson 2020-01-03 18:03:57 UTC
Pushed to 19.11.x for 19.11.02
Comment 17 Lucas Gass 2020-01-07 22:51:23 UTC
backported to 19.05.x for 19.05.07
Comment 18 Lucas Gass 2020-01-07 22:59:29 UTC
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
Comment 19 Jonathan Druart 2020-03-12 07:44:08 UTC
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."
Comment 20 Andrew Fuerste-Henry 2020-03-12 12:13:53 UTC
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.
Comment 21 Jonathan Druart 2020-03-12 13:15:47 UTC
Andrew, did you open a bug report?
Comment 22 Andrew Fuerste-Henry 2020-03-12 13:50:00 UTC
I have now. Bug 24858.