zetkin / app.zetkin.org

Current-generation ("Gen 3") web interface for the Zetkin Platform.
https://app.dev.zetkin.org
23 stars 52 forks source link

🎪Time Zones Mega Issue🎪, or maybe just archive includes future events #2248

Open WULCAN opened 2 days ago

WULCAN commented 2 days ago

Sorry, I have no screenshots for you today, but please enjoy several code snippets instead.

Description

When we create an event by dragging a box in the Week mode calendar, a startMinutes and endMinutes are created from top and bottom of the box in relation to top and bottom of day, assuming every day has exactly 24 hours.

    function handleMouseDown(ev: MouseEvent) {
      ...
      startRatio = snapToGrid((ev.clientY - rect.top) / rect.height);
    function handleMouseMove(ev: MouseEvent) {
      ...
        const dragRatio = (ev.clientY - rect.top) / rect.height;
        height = snapToGrid(dragRatio - startRatio);
    function handleMouseUp() {
      ...
        const startMinutes = Math.round(startRatio * (24 * 60));
        const endMinutes = startMinutes + Math.round(height * 24 * 60);

These minutes since the start of day are converted to hours since start of day and minutes since start of hour:

        onCreateRef.current(
          [Math.floor(startMinutes / 60), startMinutes % 60],
          [Math.floor(endMinutes / 60), endMinutes % 60]
        );

and then converted to dates with Date.UTC which treats date and time components passed as UTC.

                onCreate={(startTime, endTime) => {
                  const startDate = new Date(
                    Date.UTC(
                      date.getFullYear(),
                      date.getMonth(),
                      date.getDate(),
                      startTime[0],
                      startTime[1]
                    )
                  );

                  const endDate = new Date(
                    Date.UTC(
                      date.getFullYear(),
                      date.getMonth(),
                      date.getDate(),
                      endTime[0] >= 24 ? 23 : endTime[0],
                      endTime[0] >= 24 ? 59 : endTime[1]
                    )
                  );

Then they are finally converted to strings with toISOString before being passed of for storage via the API. The function toISOString produces a a simplified format based on ISO 8601, always in UTC and properly denoted as such with a suffix Z.

                        <MenuItem
                          onClick={async () => {
                            setCreating(true);
                            setGhostAnchorEl(null);
                            await createEvent({
                              activity_id: null,
                              campaign_id: campId,
                              end_time: pendingEvent[1].toISOString(),
                              location_id: null,
                              start_time: pendingEvent[0].toISOString(),
                              title: null,
                            });

Taking the API at face value, the calendar is a UTC calendar but reasonably, almost every user will treat is a calendar in local time. The UI does not show that these are actually UTC times. Since #2192 we even have a 'Winter time` label which cements this a local time calendar. The UTC time zone in the API must be seen as an internal technical quirk.

As long as the frontend continues to ignore the time zone from the API, and instead treats the UTC date and time components as if they were local time components, this works pretty well.

There are unfortunately exceptions and the first I found is in src/features/campaigns hooks/useActivityArchive.ts where visibleUntil is compared with a the current local time.

  const now = new Date();
  const nowDate = new Date(now.getFullYear(), now.getMonth(), now.getDate());
  const filtered = activities.filter(
    (activity) =>
      activity.visibleFrom &&
      activity.visibleUntil &&
      activity.visibleUntil < nowDate
  );

visibleUntil is derived from end_time:

          visibleUntil: getUTCDateWithoutTime(event.end_time || null),

but with time truncated:

  const dateFromNaive = new Date(naiveDateString);
  const utcTime = Date.UTC(
    dateFromNaive.getFullYear(),
    dateFromNaive.getMonth(),
    dateFromNaive.getDate()
  );

Consider an American event ending on a Tuesday. Our calendar will record it in the API as ending during the UTC Tuesday. If an organizer does useActivityArchive on that Tuesday, nowDate will be set to the local midnight. The event's visibleUntil will be set to UTC midnight, which is before local midnight. Thus the event will be accepted by the filter and included in the archive, even if the event has not ended yet.

Steps to reproduce

  1. Change your desktop's time zone to an American time zone.
  2. Open Zetkin, select an organization.
  3. Open the Calendar tab and select Week mode.
  4. Create an event that ends later today and publish it.
  5. Go back to your organization and open its Archive tab.
  6. Include Events in the filter.

Expected Behaviour

It is expected that your event is not displayed, as it has not ended yet and as it ends today.

Actual Behaviour

Your new event is included in the archived events, even though it has not ended yet.

But wait

There's very likely more. Searching for actions you will find a other integrations with events stored in the API and you can check how the frontend handles these local times stored as UTC. Just stumbling around, with an LA time zone configured, I think I found at least two other bugs which I will investigate later.

I'm not sure if I want to pile on more findings and make this a Mega issue, or if I should just continue spawning more small issues as I go.

richardolsson commented 2 days ago

It's very cool that you are investigating this and doing all of this research of how things currently are working with regards of timezones. I think maybe you are fighting an uphill battle because there is so much history here, and so much current context (other web apps, the backend, the database, etc) but I'd love to chat with you about it.

The issue of timezone support (or lack thereof) in Zetkin is huge. It's not just a web app issue, but also requires big changes to how the backend works. To tackle it requires proper understanding of the entire system, so I believe it's difficult to track it in an issue like this. It's one of the things that we plan to address with our v2 of the Zetkin API which is currently coming out of a pre-production phase.

Let's talk about it at the next hackathon and we can discuss an action plan, and what role you'd like to play in that.