From 307512c1e9a9858414ee7a3401d0c3b869ac3abf Mon Sep 17 00:00:00 2001 From: James Murty Date: Tue, 1 Nov 2016 15:53:06 +1100 Subject: [PATCH] Add start/end dates to Occurrences and test timezone handling, re #4 NOTE: Only the `Timezones` tests pass after this change, further work is pending to update the rest of the system to account for the date/datetime changes to `Occurrences`. --- .../migrations/0006_auto_20161101_1419.py | 52 +++++ icekit_events/models.py | 70 +++++-- icekit_events/tests/tests.py | 186 +++++++++++++++--- 3 files changed, 259 insertions(+), 49 deletions(-) create mode 100644 icekit_events/migrations/0006_auto_20161101_1419.py diff --git a/icekit_events/migrations/0006_auto_20161101_1419.py b/icekit_events/migrations/0006_auto_20161101_1419.py new file mode 100644 index 0000000..eff242d --- /dev/null +++ b/icekit_events/migrations/0006_auto_20161101_1419.py @@ -0,0 +1,52 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import datetime +from django.utils.timezone import utc + + +class Migration(migrations.Migration): + + dependencies = [ + ('icekit_events', '0005_auto_20161024_1742'), + ] + + operations = [ + migrations.AlterModelOptions( + name='occurrence', + options={'ordering': ['start_date', 'start', 'event__title', 'pk']}, + ), + migrations.AddField( + model_name='occurrence', + name='end_date', + field=models.DateField(default=datetime.datetime(2016, 11, 1, 3, 19, 41, 815314, tzinfo=utc), db_index=True), + preserve_default=False, + ), + migrations.AddField( + model_name='occurrence', + name='original_end_date', + field=models.DateField(null=True, editable=False, blank=True), + ), + migrations.AddField( + model_name='occurrence', + name='original_start_date', + field=models.DateField(null=True, editable=False, blank=True), + ), + migrations.AddField( + model_name='occurrence', + name='start_date', + field=models.DateField(default=datetime.datetime(2016, 11, 1, 3, 19, 45, 647055, tzinfo=utc), db_index=True), + preserve_default=False, + ), + migrations.AlterField( + model_name='occurrence', + name='end', + field=models.DateTimeField(db_index=True, null=True, blank=True), + ), + migrations.AlterField( + model_name='occurrence', + name='start', + field=models.DateTimeField(db_index=True, null=True, blank=True), + ), + ] diff --git a/icekit_events/models.py b/icekit_events/models.py index 401df02..418ae46 100644 --- a/icekit_events/models.py +++ b/icekit_events/models.py @@ -708,18 +708,18 @@ def overlapping(self, start, end): :return: occurrences overlapping the given start and end datetimes, inclusive. Special logic is applied for all-day occurrences, for which the start - and end times are zeroed to find all occurrences that occur on a DATE - as opposed to within DATETIMEs. + and end times are translated into naive date values in whichever + timezone the start/end datetimes are in. """ + # Exclusive for datetime, inclusive for date. return self.filter( models.Q(is_all_day=False, start__lt=end) | models.Q(is_all_day=True, - start__lt=zero_datetime(end)) + start_date__lte=end.date()) ).filter( - # Exclusive for datetime, inclusive for date. models.Q(is_all_day=False, end__gt=start) | models.Q(is_all_day=True, - end__gte=zero_datetime(start)) + end_date__gte=start.date()) ) def start_within(self, start, end): @@ -728,14 +728,15 @@ def start_within(self, start, end): datetimes, inclusive. """ return self.filter( + # Inclusive for datetime and date. models.Q(is_all_day=False, start__gte=start) | models.Q(is_all_day=True, - start__gte=zero_datetime(start)) + start_date__gte=start.date()) ).filter( # Exclusive for datetime, inclusive for date. models.Q(is_all_day=False, start__lt=end) | models.Q(is_all_day=True, - start__lte=zero_datetime(end)) + start_date__lte=end.date()) ) def _same_day_ids(self): @@ -804,8 +805,12 @@ class Occurrence(AbstractBaseModel): EventRepeatsGenerator, blank=True, null=True) start = models.DateTimeField( - db_index=True) + blank=True, null=True, db_index=True) end = models.DateTimeField( + blank=True, null=True, db_index=True) + start_date = models.DateField( + db_index=True) + end_date = models.DateField( db_index=True) is_all_day = models.BooleanField( default=False, db_index=True) @@ -826,23 +831,27 @@ class Occurrence(AbstractBaseModel): blank=True, null=True, editable=False) original_end = models.DateTimeField( blank=True, null=True, editable=False) + original_start_date = models.DateField( + blank=True, null=True, editable=False) + original_end_date = models.DateField( + blank=True, null=True, editable=False) class Meta: - ordering = ['start', '-is_all_day', 'event', 'pk'] + ordering = ['start_date', '-is_all_day', 'start', 'event__title', 'pk'] def time_range_string(self): if self.is_all_day: if self.duration < timedelta(days=1): return u"""{0}, all day""".format( - datefilter(self.local_start, DATE_FORMAT)) + datefilter(self.start_date, DATE_FORMAT)) else: return u"""{0} - {1}, all day""".format( - datefilter(self.local_start, DATE_FORMAT), - datefilter(self.local_end, DATE_FORMAT)) + datefilter(self.start_date, DATE_FORMAT), + datefilter(self.end_date, DATE_FORMAT)) else: return u"""{0} - {1}""".format( - datefilter(self.local_start, DATETIME_FORMAT), - datefilter(self.local_end, DATETIME_FORMAT)) + datefilter(self.start, DATETIME_FORMAT), + datefilter(self.end, DATETIME_FORMAT)) def __str__(self): return u"""Occurrence of "{0}" {1}""".format( @@ -867,7 +876,10 @@ def duration(self): """ Return the duration between ``start`` and ``end`` as a timedelta. """ - return self.end - self.start + if self.is_all_day: + return self.end_date - self.start_date + timedelta(days=1) + else: + return self.end - self.start @transaction.atomic def save(self, *args, **kwargs): @@ -879,16 +891,34 @@ def save(self, *args, **kwargs): self.is_cancelled = True else: self.is_cancelled = False - # Convert datetime field values to date-compatible versions in the - # UTC timezone when we save an all-day occurrence + + # All-day events must be specified with `start_date` and `end_date` + # values, from which the `start` and `end` values are derived if self.is_all_day: - self.start = zero_datetime(self.start) - self.end = zero_datetime(self.end) - # Set original start/end times, if necessary + if self.start or self.end: + raise Exception( + "Set only `start_date` and `end_date` date fields for an" + "all-day occurrence, not `start` or `end` datetime fields") + # Timed events must be specified with `start` and `end` values, from + # which the `start_date` and `end_date` values are derived + else: + if self.start_date or self.end_date: + raise Exception( + "Set only `start_date` and `end_date` date fields for an" + "all-day occurrence, not `start` or `end` datetime fields") + self.start_date = self.start.date() + self.end_date = self.end.date() + + # Set original start/end dates and times, if necessary if not self.original_start: self.original_start = self.start if not self.original_end: self.original_end = self.end + if not self.original_start_date: + self.original_start_date = self.start_date + if not self.original_end_date: + self.original_end_Date = self.end_date + super(Occurrence, self).save(*args, **kwargs) # TODO Return __str__ as title for now, improve it later diff --git a/icekit_events/tests/tests.py b/icekit_events/tests/tests.py index 1aa73af..d245533 100644 --- a/icekit_events/tests/tests.py +++ b/icekit_events/tests/tests.py @@ -6,7 +6,7 @@ # WebTest API docs: http://webtest.readthedocs.org/en/latest/api.html from timezone import timezone as djtz # django-timezone -from datetime import datetime, timedelta, time +from datetime import date, datetime, timedelta, time import six import json @@ -1325,12 +1325,16 @@ def setUp(self): event=G(SimpleEvent, title='Los Angeles Timed'), start=djtz.datetime(2016, 10, 1, 9, 0, tzinfo='US/Pacific'), end=djtz.datetime(2016, 10, 1, 13, 0, tzinfo='US/Pacific'), + start_date=None, + end_date=None, ) self.occurrence_pacific_all_day = G( models.Occurrence, event=G(SimpleEvent, title='Los Angeles All-day'), - start=djtz.datetime(2016, 10, 1, 0, 0, tzinfo='US/Pacific'), - end=djtz.datetime(2016, 10, 1, 0, 0, tzinfo='US/Pacific'), + start_date=date(2016, 10, 1), + end_date=date(2016, 10, 1), + start=None, + end=None, is_all_day=True, ) # Occurrences based on UTC timezone @@ -1339,12 +1343,16 @@ def setUp(self): event=G(SimpleEvent, title='UTC Timed'), start=djtz.datetime(2016, 10, 1, 9, 0, tzinfo='UTC'), end=djtz.datetime(2016, 10, 1, 13, 0, tzinfo='UTC'), + start_date=None, + end_date=None, ) self.occurrence_utc_all_day = G( models.Occurrence, event=G(SimpleEvent, title='UTC All-day'), - start=djtz.datetime(2016, 10, 1, 0, 0, tzinfo='UTC'), - end=djtz.datetime(2016, 10, 1, 0, 0, tzinfo='UTC'), + start_date=date(2016, 10, 1), + end_date=date(2016, 10, 1), + start=None, + end=None, is_all_day=True, ) # Occurrences based on UK timezone @@ -1353,12 +1361,16 @@ def setUp(self): event=G(SimpleEvent, title='London Timed'), start=djtz.datetime(2016, 10, 1, 9, 0, tzinfo='Europe/London'), end=djtz.datetime(2016, 10, 1, 13, 0, tzinfo='Europe/London'), + start_date=None, + end_date=None, ) self.occurrence_uk_all_day = G( models.Occurrence, event=G(SimpleEvent, title='London All-day'), - start=djtz.datetime(2016, 10, 1, 0, 0, tzinfo='Europe/London'), - end=djtz.datetime(2016, 10, 1, 0, 0, tzinfo='Europe/London'), + start_date=date(2016, 10, 1), + end_date=date(2016, 10, 1), + start=None, + end=None, is_all_day=True, ) # Occurrences based on Australian Eastern timezone @@ -1367,15 +1379,78 @@ def setUp(self): event=G(SimpleEvent, title='Sydney Timed'), start=djtz.datetime(2016, 10, 1, 9, 0, tzinfo='Australia/Sydney'), end=djtz.datetime(2016, 10, 1, 13, 0, tzinfo='Australia/Sydney'), + start_date=None, + end_date=None, ) self.occurrence_aest_all_day = G( models.Occurrence, event=G(SimpleEvent, title='Sydney All-day'), - start=djtz.datetime(2016, 10, 1, 0, 0, tzinfo='Australia/Sydney'), - end=djtz.datetime(2016, 10, 1, 0, 0, tzinfo='Australia/Sydney'), + start_date=date(2016, 10, 1), + end_date=date(2016, 10, 1), + start=None, + end=None, is_all_day=True, ) + def test_start_and_end_times_have_expected_values_for_timed_events(self): + # Check that occurrence start times match expected value in AEST + aest_tz = djtz.pytz.timezone('Australia/Sydney') + self.assertEqual( + self.occurrence_pacific_timed.start.astimezone(aest_tz), + djtz.datetime(2016, 10, 2, 3, 0, tzinfo='Australia/Sydney'), + ) + self.assertEqual( + self.occurrence_utc_timed.start.astimezone(aest_tz), + djtz.datetime(2016, 10, 1, 19, 0, tzinfo='Australia/Sydney'), + ) + self.assertEqual( + self.occurrence_uk_timed.start.astimezone(aest_tz), + djtz.datetime(2016, 10, 1, 18, 0, tzinfo='Australia/Sydney'), + ) + self.assertEqual( + self.occurrence_aest_timed.start.astimezone(aest_tz), + djtz.datetime(2016, 10, 1, 9, 0, tzinfo='Australia/Sydney'), + ) + + def test_start_date_and_end_date_fields_are_set_for_timed_occurrences(self): + # Start and end dates of timed events set to "local" date corresponding + # to the timezone of the original datetime start/end values + self.assertEqual( + self.occurrence_pacific_timed.start_date, + date(2016, 10, 1)) + self.assertEqual( + self.occurrence_pacific_timed.end_date, + date(2016, 10, 1)) + self.assertEqual( + self.occurrence_utc_timed.start_date, + date(2016, 10, 1)) + self.assertEqual( + self.occurrence_utc_timed.end_date, + date(2016, 10, 1)) + self.assertEqual( + self.occurrence_uk_timed.start_date, + date(2016, 10, 1)) + self.assertEqual( + self.occurrence_uk_timed.end_date, + date(2016, 10, 1)) + self.assertEqual( + self.occurrence_aest_timed.start_date, + date(2016, 10, 1)) + self.assertEqual( + self.occurrence_aest_timed.end_date, + date(2016, 10, 1)) + + def test_start_and_end_fields_are_unset_for_all_day_occurrences(self): + # Start and end datetiems of all-day events set to None + self.assertIsNone(self.occurrence_pacific_all_day.start) + self.assertIsNone(self.occurrence_pacific_all_day.end) + self.assertIsNone(self.occurrence_utc_all_day.start) + self.assertIsNone(self.occurrence_utc_all_day.end) + self.assertIsNone(self.occurrence_uk_all_day.start) + self.assertIsNone(self.occurrence_uk_all_day.end) + self.assertIsNone(self.occurrence_aest_all_day.start) + self.assertIsNone(self.occurrence_aest_all_day.end) + def test_occurrence_default_ordering(self): ordered_titles = [ o.event.title for o in models.Occurrence.objects.all()] @@ -1387,15 +1462,20 @@ def test_occurrence_default_ordering(self): u'UTC All-day', # Timed occurrences are ordered after all-day occurrences, by # datetime accounting for timezone - u'Los Angeles Timed', - u'UTC Timed', - u'London Timed', u'Sydney Timed', + u'London Timed', + u'UTC Timed', + u'Los Angeles Timed', ], ordered_titles ) def test_overlapping_filter(self): + actual = [ + o.event.title for o in models.Occurrence.objects.overlapping( + djtz.datetime(2016, 10, 1, 10, 0, tzinfo='US/Pacific'), + djtz.datetime(2016, 10, 1, 11, 0, tzinfo='US/Pacific'), + )] self.assertEqual( # All-day occurrences are included in overlapping result for date [u'London All-day', @@ -1405,13 +1485,14 @@ def test_overlapping_filter(self): # Timed occurrences are included in overlapping result only when # their start & end times overlap u'Los Angeles Timed', - ], - [o.event.title for o in models.Occurrence.objects.overlapping( - djtz.datetime(2016, 10, 1, 10, 0, tzinfo='US/Pacific'), - djtz.datetime(2016, 10, 1, 11, 0, tzinfo='US/Pacific'), - )] + ], actual ) + actual = [ + o.event.title for o in models.Occurrence.objects.overlapping( + djtz.datetime(2016, 10, 1, 12, 59, tzinfo='Europe/London'), + djtz.datetime(2016, 10, 1, 9, 1, tzinfo='US/Pacific'), + )] self.assertEqual( # All-day occurrences are included in overlapping result for date [u'London All-day', @@ -1420,16 +1501,17 @@ def test_overlapping_filter(self): u'UTC All-day', # Timed occurrences are included in overlapping result only when # their start & end times overlap - u'Los Angeles Timed', - u'UTC Timed', u'London Timed', - ], - [o.event.title for o in models.Occurrence.objects.overlapping( - djtz.datetime(2016, 10, 1, 12, 59, tzinfo='US/Pacific'), - djtz.datetime(2016, 10, 1, 9, 1, tzinfo='Europe/London'), - )] + u'UTC Timed', + u'Los Angeles Timed', + ], actual ) + actual = [ + o.event.title for o in models.Occurrence.objects.overlapping( + djtz.datetime(2016, 10, 1, 13, 0, tzinfo='Australia/Sydney'), + djtz.datetime(2016, 10, 1, 9, 0, tzinfo='Europe/London'), + )] self.assertEqual( # All-day occurrences are included in overlapping result for date [u'London All-day', @@ -1438,13 +1520,59 @@ def test_overlapping_filter(self): u'UTC All-day', # Timed occurrences are included in overlapping result only when # their start & end times overlap - u'London Timed', - u'Sydney Timed', - ], - [o.event.title for o in models.Occurrence.objects.overlapping( - djtz.datetime(2016, 10, 1, 13, 0, tzinfo='Europe/London'), + # NOTE: 'Sydney Timed' not included due to exclusive start check + # NOTE: 'London Timed' not included due to exclusive end check + ], actual + ) + + def test_start_within_filter(self): + actual = [ + o.event.title for o in models.Occurrence.objects.start_within( + djtz.datetime(2016, 10, 1, 9, 0, tzinfo='US/Pacific'), + djtz.datetime(2016, 10, 1, 9, 1, tzinfo='US/Pacific'), + )] + self.assertEqual( + # All-day occurrences are included in start_within result for date + [u'London All-day', + u'Los Angeles All-day', + u'Sydney All-day', + u'UTC All-day', + # Timed occurrences + u'Los Angeles Timed', + ], actual + ) + + actual = [ + o.event.title for o in models.Occurrence.objects.start_within( + djtz.datetime(2016, 10, 1, 12, 59, tzinfo='Europe/London'), + djtz.datetime(2016, 10, 1, 9, 1, tzinfo='US/Pacific'), + )] + self.assertEqual( + # All-day occurrences are included in start_within result for date + [u'London All-day', + u'Los Angeles All-day', + u'Sydney All-day', + u'UTC All-day', + # Timed occurrences + u'Los Angeles Timed', + ], actual + ) + + actual = [ + o.event.title for o in models.Occurrence.objects.start_within( djtz.datetime(2016, 10, 1, 9, 0, tzinfo='Australia/Sydney'), + djtz.datetime(2016, 10, 1, 9, 0, tzinfo='Europe/London'), )] + self.assertEqual( + # All-day occurrences are included in start_within result for date + [u'London All-day', + u'Los Angeles All-day', + u'Sydney All-day', + u'UTC All-day', + # Timed occurrences + u'Sydney Timed' # Included due to inclusive start check + # NOTE: 'London Timed' not included due to exclusive end check + ], actual )