refactor: refactor timestamp handling so tests in meeting app pass (ietf-tools#4371)

* refactor: replace datetime.now with timezone.now

* refactor: migrate model fields to use timezone.now as default

* refactor: replace datetime.today with timezone.now

datetime.datetime.today() is equivalent to datetime.datetime.now(); both
return a naive datetime with the current local time.

* refactor: rephrase datetime.now(tz) as timezone.now().astimezone(tz)

This is effectively the same, but is less likely to encourage accidental
use of naive datetimes.

* refactor: revert datetime.today() change to old migrations

* refactor: change a missed datetime.now to timezone.now

* chore: renumber timezone_now migration

* chore: add migration to change timestamps to UTC

* refactor: move tz instantiation/caching from TimeSlot to Meeting

* fix: assume utc if meeting.time_zone is blank

* chore: make datetime.combine() calls tz aware in the meeting app

* ci: correctly use meeting.tz in TimeSlotFactory

* chore: compute TimeSlot utc / local times assuming tz-aware times

* chore: use tzaware math for agenda editor timeslot layout

* chore: fill in Meeting.time_zone where it is blank

Nearly all interim meetings on or before 2016-07-01 have blank
time_zone values. This migration fills these in with PST8PDT.

* chore: disallow blank Meeting.time_zone value

* refactor: no need to handle blank time_zone case in TZ migration

* refactor: remove now-unnecessary checks that meeting has time_zone

* chore: fix timezone handling in agenda.ics and Meeting.updated()

* chore: fix tz handling in interim_request_details, exercise in tests

* chore: fix timezone handling for test_interim_send_announcement

* chore: fix timezone handling in agenda_json()

* chore: fix timezone handling in old agenda

* chore: fix timezone handling for EditTimeslotsTests

* refactor: refactor a few fixes for more consistent timezone handling

* chore: add timezone info to timestamps in fixtures

* chore: remove naive datetime warnings found in meetings.tests_views

* chore: fix a few more test failures in meetings.tests_views

All tests in meetings.tests_views now passing

* chore: remove unused import

* chore: fix timezone handling in test_schedule_generator.py

* chore: fix timezone handling affecting meeting.tests_js

* chore: fix timeslot test bug when local date != UTC date

* test: fix a few failing tests, all meetings tests now pass

(for me, anyway)

* chore: renumber migrations

* chore: update timestamp conversion migration

The django-celery-beat package introduces tables with timestamp
columns. These columns are stored in CELERY_TIMEZONE. Because we run with
this set to UTC, the migration ignores these columns.

* chore: fix pytz-related change in migration

* chore: remove duplicate migrations

* chore: remove CELERY_BEAT_TZ_AWARE setting now that USE_TZ is True

* test: avoid failure in test with bogus timezone

Author: jennifer-richards

ietf/group/models.py

@@ -2,7 +2,6 @@
 # -*- coding: utf-8 -*-
 
 
-import datetime
 import email.utils
 import jsonfield
 import os
@@ -181,11 +180,15 @@ def liaison_approvers(self):
         return self.role_set.none()
 
     def status_for_meeting(self,meeting):
-        end_date = meeting.end_date()+datetime.timedelta(days=1)
         previous_meeting = meeting.previous_meeting()
-        status_events = self.groupevent_set.filter(type='status_update',time__lte=end_date).order_by('-time')
+        status_events = self.groupevent_set.filter(
+            type='status_update',
+            time__lt=meeting.end_datetime(),
+        ).order_by('-time')
         if previous_meeting:
-            status_events = status_events.filter(time__gte=previous_meeting.end_date()+datetime.timedelta(days=1))
+            status_events = status_events.filter(
+                time__gte=previous_meeting.end_datetime()
+            )
         return status_events.first()
 
     def get_description(self):

ietf/meeting/factories.py

@@ -187,7 +187,9 @@ def location(obj, create, extracted, **kwargs): # pylint: disable=no-self-argume
 
     @factory.lazy_attribute
     def time(self):
-        return datetime.datetime.combine(self.meeting.date,datetime.time(11,0))
+        return self.meeting.tz().localize(
+            datetime.datetime.combine(self.meeting.date, datetime.time(11, 0))
+        )
 
     @factory.lazy_attribute
     def duration(self):

ietf/meeting/fixtures/proceedings_templates.json

@@ -32,7 +32,7 @@
       "comments": "",
       "list_subscribe": "",
       "state": "active",
-      "time": "2012-02-26T00:21:36",
+      "time": "2012-02-26T00:21:36Z",
       "unused_tags": [],
       "list_archive": "",
       "type": "ietf",

ietf/meeting/helpers.py

@@ -118,7 +118,10 @@ def preprocess_assignments_for_agenda(assignments_queryset, meeting, extra_prefe
     # assignments = list(assignments_queryset) # make sure we're set in stone
     assignments = assignments_queryset
 
-    meeting_time = datetime.datetime.combine(meeting.date, datetime.time())
+    # meeting_time is meeting-local midnight at the start of the meeting date
+    meeting_time = meeting.tz().localize(
+        datetime.datetime.combine(meeting.date, datetime.time())
+    )
 
     # replace groups with historic counterparts
     groups = [ ]
@@ -1149,26 +1152,30 @@ def sessions_post_cancel(request, sessions):
 
 
 def update_interim_session_assignment(form):
-    """Helper function to create / update timeslot assigned to interim session"""
-    time = datetime.datetime.combine(
-        form.cleaned_data['date'],
-        form.cleaned_data['time'])
+    """Helper function to create / update timeslot assigned to interim session
+
+    form is an InterimSessionModelForm
+    """
     session = form.instance
+    meeting = session.meeting
+    time = meeting.tz().localize(
+        datetime.datetime.combine(form.cleaned_data['date'], form.cleaned_data['time'])
+    )
     if session.official_timeslotassignment():
         slot = session.official_timeslotassignment().timeslot
         slot.time = time
         slot.duration = session.requested_duration
         slot.save()
     else:
         slot = TimeSlot.objects.create(
-            meeting=session.meeting,
+            meeting=meeting,
             type_id='regular',
             duration=session.requested_duration,
             time=time)
         SchedTimeSessAssignment.objects.create(
             timeslot=slot,
             session=session,
-            schedule=session.meeting.schedule)
+            schedule=meeting.schedule)
 
 def populate_important_dates(meeting):
     assert ImportantDate.objects.filter(meeting=meeting).exists() is False

ietf/meeting/management/commands/create_dummy_meeting.py

@@ -48,7 +48,7 @@
 import datetime
 import pytz
 
-from django.core.management.base import BaseCommand
+from django.core.management.base import BaseCommand, CommandError
 from django.db import transaction
 from django.db.models import Q
 
@@ -75,10 +75,12 @@ def add_arguments(self, parser):
 
     def _meeting_datetime(self, day, *time_args):
         """Generate a datetime on a meeting day"""
-        return datetime.datetime.combine(
-            self.start_date,
-            datetime.time(*time_args)
-        ) + datetime.timedelta(days=day)
+        return self.meeting_tz.localize(
+            datetime.datetime.combine(
+                self.start_date,
+                datetime.time(*time_args)
+            ) + datetime.timedelta(days=day)
+        )
 
     def handle(self, *args, **options):
         if socket.gethostname().split('.')[0] in ['core3', 'ietfa', 'ietfb', 'ietfc', ]:
@@ -87,17 +89,19 @@ def handle(self, *args, **options):
         opt_delete = options.get('delete', False)
         opt_use_old_conflicts = options.get('old_conflicts', False)
         self.start_date = options['start_date']
-        meeting_tz = options['tz']
-        if not opt_delete and (meeting_tz not in pytz.common_timezones):
-            self.stderr.write("Warning: {} is not a recognized time zone.".format(meeting_tz))
-
+        meeting_tzname = options['tz']
         if opt_delete:
             if Meeting.objects.filter(number='999').exists():
                 Meeting.objects.filter(number='999').delete()
                 self.stdout.write("Deleted dummy meeting IETF 999 and its related objects.")
             else:
                 self.stderr.write("Dummy meeting IETF 999 does not exist; nothing to do.\n")
         else:
+            try:
+                self.meeting_tz = pytz.timezone(meeting_tzname)
+            except pytz.UnknownTimeZoneError:
+                raise CommandError("{} is not a recognized time zone.".format(meeting_tzname))
+
             if Meeting.objects.filter(number='999').exists():
                 self.stderr.write("Dummy meeting IETF 999 already exists; nothing to do.\n")
             else:
@@ -111,7 +115,7 @@ def handle(self, *args, **options):
                     type_id='IETF',
                     date=self._meeting_datetime(0).date(),
                     days=7,
-                    time_zone=meeting_tz,
+                    time_zone=meeting_tzname,
                 )
 
                 # Set enabled constraints

ietf/meeting/models.py

@@ -138,6 +138,11 @@ def get_meeting_date (self,offset):
     def end_date(self):
         return self.get_meeting_date(self.days-1)
 
+    def end_datetime(self):
+        """Datetime of the first instant _after_ the meeting's last day"""
+        return self.tz().localize(
+            datetime.datetime.combine(self.get_meeting_date(self.days), datetime.time())
+        )
     def get_00_cutoff(self):
         start_date = datetime.datetime(year=self.date.year, month=self.date.month, day=self.date.day, tzinfo=pytz.utc)
         importantdate = self.importantdate_set.filter(name_id='idcutoff').first()
@@ -322,23 +327,23 @@ def build_timeslices(self):
         for ts in self.timeslot_set.all():
             if ts.location_id is None:
                 continue
-            ymd = ts.time.date()
+            ymd = ts.local_start_time().date()
             if ymd not in time_slices:
                 time_slices[ymd] = []
                 slots[ymd] = []
                 days.append(ymd)
 
             if ymd in time_slices:
                 # only keep unique entries
-                if [ts.time, ts.time + ts.duration, ts.duration.seconds] not in time_slices[ymd]:
-                    time_slices[ymd].append([ts.time, ts.time + ts.duration, ts.duration.seconds])
+                if [ts.local_start_time(), ts.local_end_time(), ts.duration.seconds] not in time_slices[ymd]:
+                    time_slices[ymd].append([ts.local_start_time(), ts.local_end_time(), ts.duration.seconds])
                     slots[ymd].append(ts)
 
         days.sort()
         for ymd in time_slices:
             # Make sure these sort the same way
             time_slices[ymd].sort()
-            slots[ymd].sort(key=lambda x: (x.time, x.duration))
+            slots[ymd].sort(key=lambda x: (x.local_start_time(), x.duration))
         return days,time_slices,slots
 
     # this functions makes a list of timeslices and rooms, and
@@ -354,6 +359,11 @@ def build_timeslices(self):
 #                    SchedTimeSessAssignment.objects.create(schedule = sched,
 #                                                    timeslot = ts)
 
+    def tz(self):
+        if not hasattr(self, '_cached_tz'):
+            self._cached_tz = pytz.timezone(self.time_zone)
+        return self._cached_tz
+
     def vtimezone(self):
         try:
             tzfn = os.path.join(settings.TZDATA_ICS_PATH, self.time_zone + ".ics")
@@ -374,16 +384,14 @@ def set_official_schedule(self, schedule):
             self.save()
 
     def updated(self):
-        min_time = datetime.datetime(1970, 1, 1, 0, 0, 0) # should be Meeting.modified, but we don't have that
+        # should be Meeting.modified, but we don't have that
+        min_time = pytz.utc.localize(datetime.datetime(1970, 1, 1, 0, 0, 0))
         timeslots_updated = self.timeslot_set.aggregate(Max('modified'))["modified__max"] or min_time
         sessions_updated = self.session_set.aggregate(Max('modified'))["modified__max"] or min_time
         assignments_updated = min_time
         if self.schedule:
             assignments_updated = SchedTimeSessAssignment.objects.filter(schedule__in=[self.schedule, self.schedule.base if self.schedule else None]).aggregate(Max('modified'))["modified__max"] or min_time
-        ts = max(timeslots_updated, sessions_updated, assignments_updated)
-        tz = pytz.timezone(settings.PRODUCTION_TIMEZONE)
-        ts = tz.localize(ts)
-        return ts
+        return max(timeslots_updated, sessions_updated, assignments_updated)
 
     @memoize
     def previous_meeting(self):
@@ -604,29 +612,22 @@ def get_html_location(self):
         return self._cached_html_location
 
     def tz(self):
-        if not hasattr(self, '_cached_tz'):
-            self._cached_tz = pytz.timezone(self.meeting.time_zone)
-        return self._cached_tz
+        return self.meeting.tz()
 
     def tzname(self):
         return self.tz().tzname(self.time)
 
     def utc_start_time(self):
-        local_start_time = self.tz().localize(self.time)
-        return local_start_time.astimezone(pytz.utc)
+        return self.time.astimezone(pytz.utc)  # USE_TZ is True, so time is aware
 
     def utc_end_time(self):
-        utc_start = self.utc_start_time()
-        # Add duration after converting start time, otherwise errors creep in around DST change
-        return None if utc_start is None else utc_start + self.duration
+        return self.time.astimezone(pytz.utc) + self.duration  # USE_TZ is True, so time is aware
 
     def local_start_time(self):
-        return self.tz().localize(self.time)
+        return self.time.astimezone(self.tz())
 
     def local_end_time(self):
-        local_start = self.local_start_time()
-        # Add duration after converting start time, otherwise errors creep in around DST change
-        return None if local_start is None else local_start + self.duration
+        return (self.time.astimezone(pytz.utc) + self.duration).astimezone(self.tz())
 
     @property
     def js_identifier(self):

ietf/meeting/test_data.py

@@ -21,10 +21,12 @@
 from ietf.person.models import Person
 from ietf.utils.test_data import make_test_data
 
-def make_interim_meeting(group,date,status='sched'):
+def make_interim_meeting(group,date,status='sched',tz='UTC'):
     system_person = Person.objects.get(name="(System)")
-    time = datetime.datetime.combine(date, datetime.time(9))
-    meeting = create_interim_meeting(group=group,date=date)
+    meeting = create_interim_meeting(group=group,date=date,timezone=tz)
+    time = meeting.tz().localize(
+        datetime.datetime.combine(date, datetime.time(9))
+    )
     session = SessionFactory(meeting=meeting, group=group,
                              attendees=10,
                              requested_duration=datetime.timedelta(minutes=20),
@@ -102,24 +104,37 @@ def make_meeting_test_data(meeting=None, create_interims=False):
 
     # slots
     session_date = meeting.date + datetime.timedelta(days=1)
+    tz = meeting.tz()
     slot1 = TimeSlot.objects.create(meeting=meeting, type_id='regular', location=room,
                                     duration=datetime.timedelta(minutes=60),
-                                    time=datetime.datetime.combine(session_date, datetime.time(9, 30)))
+                                    time=tz.localize(
+                                        datetime.datetime.combine(session_date, datetime.time(9, 30))
+                                    ))
     slot2 = TimeSlot.objects.create(meeting=meeting, type_id='regular', location=room,
                                     duration=datetime.timedelta(minutes=60),
-                                    time=datetime.datetime.combine(session_date, datetime.time(10, 50)))
+                                    time=tz.localize(
+                                        datetime.datetime.combine(session_date, datetime.time(10, 50))
+                                    ))
     breakfast_slot = TimeSlot.objects.create(meeting=meeting, type_id="lead", location=breakfast_room,
                                     duration=datetime.timedelta(minutes=90),
-                                    time=datetime.datetime.combine(session_date, datetime.time(7,0)))
+                                    time=tz.localize(
+                                        datetime.datetime.combine(session_date, datetime.time(7,0))
+                                    ))
     reg_slot = TimeSlot.objects.create(meeting=meeting, type_id="reg", location=reg_room,
                                        duration=datetime.timedelta(minutes=480),
-                                       time=datetime.datetime.combine(session_date, datetime.time(9,0)))
+                                       time=tz.localize(
+                                           datetime.datetime.combine(session_date, datetime.time(9,0))
+                                       ))
     break_slot = TimeSlot.objects.create(meeting=meeting, type_id="break", location=break_room,
                                          duration=datetime.timedelta(minutes=90),
-                                         time=datetime.datetime.combine(session_date, datetime.time(7,0)))
+                                         time=tz.localize(
+                                             datetime.datetime.combine(session_date, datetime.time(7,0))
+                                         ))
     plenary_slot = TimeSlot.objects.create(meeting=meeting, type_id="plenary", location=room,
                                          duration=datetime.timedelta(minutes=60),
-                                         time=datetime.datetime.combine(session_date, datetime.time(11,0)))
+                                         time=tz.localize(
+                                             datetime.datetime.combine(session_date, datetime.time(11,0))
+                                         ))
     # mars WG
     mars = Group.objects.get(acronym='mars')
     mars_session = SessionFactory(meeting=meeting, group=mars,
@@ -213,7 +228,7 @@ def make_meeting_test_data(meeting=None, create_interims=False):
 
     return meeting
 
-def make_interim_test_data():
+def make_interim_test_data(meeting_tz='UTC'):
     date = datetime.date.today() + datetime.timedelta(days=365)
     date2 = datetime.date.today() + datetime.timedelta(days=1000)
     PersonFactory(user__username='plain')
@@ -225,10 +240,10 @@ def make_interim_test_data():
     RoleFactory(group=mars,person__user__username='marschairman',name_id='chair')
     RoleFactory(group=ames,person__user__username='ameschairman',name_id='chair')
 
-    make_interim_meeting(group=mars,date=date,status='sched')
-    make_interim_meeting(group=mars,date=date2,status='apprw')
-    make_interim_meeting(group=ames,date=date,status='canceled')
-    make_interim_meeting(group=ames,date=date2,status='apprw')
+    make_interim_meeting(group=mars,date=date,status='sched',tz=meeting_tz)
+    make_interim_meeting(group=mars,date=date2,status='apprw',tz=meeting_tz)
+    make_interim_meeting(group=ames,date=date,status='canceled',tz=meeting_tz)
+    make_interim_meeting(group=ames,date=date2,status='apprw',tz=meeting_tz)
 
     return