Proper timezone handling for schedules #369
Conversation
jrauh01
force-pushed
the
fix/schedule-timezone-handling
branch
from
efafe1d to
1b6de94
Compare
jrauh01
force-pushed
the
fix/round-times-to-1130-pm
branch
from
229f6a8 to
edfee49
Compare
jrauh01
force-pushed
the
fix/schedule-timezone-handling
branch
4 times, most recently
from
18670a3 to
bb75f8d
Compare
jrauh01
force-pushed
the
fix/schedule-timezone-handling
branch
2 times, most recently
from
8e83df5 to
eaecd7b
Compare
|
Some timezones from Please make sure the daemon also supports these time zones, or filter out unsupported ones to avoid runtime errors. |
There was a problem hiding this comment.
I noticed that there is another big flaw with how the display timezone is currently implemented. Since it's persisted to a URL parameter, all links, form actions and redirects must be able to declare it. Though, not all of them do this right now, e.g. the refresh button of the container. I think this should be done differently and in a way that is even beneficial for the user.
URL parameter handling aside, the only relevant parts are \Icinga\Module\Notifications\Controllers\ScheduleController::createTimezonePicker() and \Icinga\Module\Notifications\Widget\Detail\ScheduleDetail\Controls::getStartDate(). Let the controller do all the work instead. It should use a dedicated session namespace which tz to use as display tz. This way it's also persisted across the entire user session. Submitting the picker writes to the namespace, the schedule detail should require the start date in its constructor, so that the schedule's controls don't provide a start date at all anymore.
| $dt->setTime($hour, $minute); | ||
| $options[$dt->format('H:i')] = $formatter->format($dt); | ||
|
|
||
| if ($this->displayTimezone !== $this->scheduleTimezone) { |
There was a problem hiding this comment.
I personally struggled at first to know what the difference between those two times are. I mean, which tz which one uses. I think, users will struggle as well. Since the options here are used to define the first handoff, please show it in the display tz in addition if it differs. We have plenty of space there and can show a non-ambiguous hint. The dropdowns then only show times without any tz offset.
| $this->addHtml(new HtmlElement( | ||
| 'p', | ||
| null, | ||
| new FormattedString($this->translate('The schedule\'s timezone is %s'), [ |
There was a problem hiding this comment.
I believe this should explain what this means. i.e., that all form options assume this tz.
| $handoff = DateTime::createFromFormat( | ||
| 'Y-m-d H:i', | ||
| $rotation->first_handoff . ' ' . $time, | ||
| new DateTimeZone($this->displayTimezone) |
There was a problem hiding this comment.
Shouldn't this use the schedule tz? (In turn, the until_time below as well?)
| ? ['Next Day' => $nextDayTimeOptions] | ||
| : ['Today' => $timeOptions, 'Next Day' => $nextDayTimeOptions] |
There was a problem hiding this comment.
The optgroup labels must be translatable
There was a problem hiding this comment.
And, just noticed, Today -> Same day
Add dropdown menu to choose schedule timezone.
A dropdown menu to pick a timezone.
To change the timezone to display the schedule in. Per default the timezone the schedule is created in is used.
`ScheduleDateTimeFactory` is a factory class to create DateTime objects in a specific timezone that is set in a static attribute.
Use the chosen value from the timezone picker control to set the timezone attribute in `ScheduleDateTimeFactory`.
Makes use of `ScheduleDateTimeFactory`.
A Widget that represents a warning if the display timezone differs from the schedule timezone. It's used for the forms to add and edit rotations.
...if the display timezone differs from the schedule timezone.
Use the schedule timezone to configure rotations. All times entered are handled in the schedule timezone.
This is because we want to remove the parentheses for future changes where those should be used to display a time in another timezone.
In the dropdown menu in the rotation config form show times in the display timezone in parentheses next to the normal time (schedule timezone).
... if the display timezone differs.
... to prevent the schedule view mode switcher resetting the timezone picker.
... by `ScheduleTimezoneStorage`. This just stores the display and schedule timezone, instead of creating `DateTime` objects.
... instead using schedule timezone.
Some timezones from `DateTimeZone::listIdentifiers()` are not compatible with the `IntlDateFormatter`. So now we get the timezones from `IntlTimeZone::createEnumeration()`. This provides timezones of **type 2 and 3**. Type 2 timezones cause problems, so we want to filter them out. To make sure we only get timezones from **type 3**, we check if they have a location. (Only type 3 timezones have a location).
Show the first handoff in the display timezone if it differs to the schedule timezone.
In the dropdown menu in the rotation config form from now on don't show times in the display timezone in parentheses next to the normal time (schedule timezone).
Instead of using an url param that has to be preserved by every link, form action or redirect, we now use the 'notifications' session namespace to store the display timezone for the current user. The start day for the timeline now comes from the controller, no longer from the schedule detail controls.
If the entries shift to another weekday in the display timezone the flyouts have to point this out. For that we shift the days array for **partial** mode rotations or the start and end days for **multi** mode rotations.
jrauh01
force-pushed
the
fix/schedule-timezone-handling
branch
from
8fd5baf to
f701588
Compare
This PR introduces timezone support for schedules. It relies on the
schedule.timezonecolumn in the database added in Icinga/icinga-notifications#344.For the following explanation, it is important to understand the difference between schedule and display timezone.
Schedule List
The timeline part has been removed completely, so that we don't have to create a header for each schedule which wouldn't be very well readable.
Schedule Creation
The form to create a new schedule now has a dropdown menu to select the timezone the schedule will be created for. Once created, the timezone cannot be changed via the web UI, because the dropdown won't be displayed for the
editAction(). This is ensured with a bool$showTimezoneDropin theScheduleFormwhich defaults tofalse. The dropdown's default is the same timezone as in the the user's preferences. The value of the dropdown will be stored in theschedule.timezonecolumn.ScheduleDateTimeFactory
The
ScheduleDateTimeFactoryis a util factory class which stores a display timezone statically and createsDateTimeobjects from it.Schedule Detail View
The
ScheduleDateTimeFactoryis used to create theDateTimeobjects that represents the timeline start and the clock that indicates the current time in the timezone the user wants to display the schedule in. Therotation.actual_handoffandtimeperiod_entry.start_timeare stored as unix timestamps, so the entries are displayed correctly in the timeline grid. Additionally all date formatters are constructed with the right timezone, so all titles are displayed correctly.Currently the user changes the display timezone via a dropdown control called
TimezonePicker. This dropdown is temporarily added for development purposes and won't be in the final version.Rotation Config Form
All configurations in the form are done in the schedule timezone, no matter what display timezone is chosen. The
RotationConfigFormis now constructed with the display timezone. If the display timezone differs following will happen:The dropdowns to select times did show 'Next Day' hints earlier. This logic has been replaced by an own option group for next day elements.
Requires Icinga/icinga-notifications#344
Requires Icinga/ipl-scheduler#55
Fixes #273