diff --git a/icekit_events/models.py b/icekit_events/models.py index baa56d6..71cd3ad 100644 --- a/icekit_events/models.py +++ b/icekit_events/models.py @@ -76,13 +76,18 @@ def default_date_ends(): return default_date_starts() + appsettings.DEFAULT_DATE_ENDS_DELTA -def format_ical_dt(date_or_datetime): - """ Return datetime formatted for use in iCal """ +def format_naive_ical_dt(date_or_datetime): + """ + Return datetime formatted for use in iCal as a *naive* datetime value to + work more like people expect, e.g. creating a series of events starting + at 9am should not create some occurrences that start at 8am or 10am after + a daylight savings change. + """ dt = coerce_dt_awareness(date_or_datetime) if is_naive(dt): return dt.strftime('%Y%m%dT%H%M%S') else: - return dt.astimezone(pytz.utc).strftime('%Y%m%dT%H%M%SZ') + return dt.astimezone(get_current_timezone()).strftime('%Y%m%dT%H%M%S') def coerce_dt_awareness(date_or_datetime, tz=None): @@ -91,20 +96,36 @@ def coerce_dt_awareness(date_or_datetime, tz=None): timezone-naive `datetime` result, depending on which is appropriate for the project's settings. """ - if tz is None: - tz = get_current_timezone() if isinstance(date_or_datetime, datetime): dt = date_or_datetime else: dt = datetime.combine(date_or_datetime, datetime_time()) is_project_tz_aware = settings.USE_TZ - if is_project_tz_aware and is_naive(dt): - return make_aware(dt, tz) - elif not is_project_tz_aware and is_aware(dt): - return make_naive(dt, tz) + if is_project_tz_aware: + return coerce_aware(dt, tz) + elif not is_project_tz_aware: + return coerce_naive(dt, tz) # No changes necessary return dt + +def coerce_naive(dt, tz=None): + if is_naive(dt): + return dt + else: + if tz is None: + tz = get_current_timezone() + return make_naive(dt, tz) + + +def coerce_aware(dt, tz=None): + if is_aware(dt): + return dt + else: + if tz is None: + tz = get_current_timezone() + return make_aware(dt, tz) + # FIELDS ###################################################################### @@ -506,6 +527,10 @@ def generate(self, until=None): until = self.repeat_end else: until = timezone.now() + appsettings.REPEAT_LIMIT + # Make datetimes naive, since RRULE spec contains naive datetimes so + # our constraints must be the same + start_dt = coerce_naive(start_dt) + until = coerce_naive(until) # Determine duration to add to each start time occurrence_duration = self.duration or timedelta(days=1) # `start_dt` and `until` datetimes are exclusive for our rruleset @@ -538,13 +563,13 @@ def _build_complete_rrule(self, start_dt=None, until=None): or timezone.now() + appsettings.REPEAT_LIMIT # We assume `recurrence_rule` is always a RRULE repeat spec of the form # "FREQ=DAILY", "FREQ=WEEKLY", etc? - rrule_spec = "DTSTART:%s" % format_ical_dt(start_dt) + rrule_spec = "DTSTART:%s" % format_naive_ical_dt(start_dt) if not self.recurrence_rule: - rrule_spec += "\nRDATE:%s" % format_ical_dt(start_dt) + rrule_spec += "\nRDATE:%s" % format_naive_ical_dt(start_dt) else: rrule_spec += "\nRRULE:%s" % self.recurrence_rule # Apply this event's end repeat - rrule_spec += ";UNTIL=%s" % format_ical_dt( + rrule_spec += ";UNTIL=%s" % format_naive_ical_dt( until - timedelta(seconds=1)) return rrule_spec @@ -782,19 +807,24 @@ def is_past(self): def get_absolute_url(self): return self.event.get_absolute_url() + def get_occurrence_times_for_event(event): """ - Return a tuple with two sets containing the (start, end) datetimes of an - Event's Occurrences, or the original start datetime if an Occurrence's - start was modified by a user. + Return a tuple with two sets containing the (start, end) *naive* datetimes + of an Event's Occurrences, or the original start datetime if an + Occurrence's start was modified by a user. """ occurrences_starts = set() occurrences_ends = set() for start, original_start, end, original_end in \ event.occurrences.all().values_list('start', 'original_start', 'end', 'original_end'): - occurrences_starts.add(original_start or start) - occurrences_ends.add(original_end or end) + occurrences_starts.add( + coerce_naive(original_start or start) + ) + occurrences_ends.add( + coerce_naive(original_end or end) + ) return occurrences_starts, occurrences_ends diff --git a/icekit_events/tests/tests.py b/icekit_events/tests/tests.py index 4a06448..674ed1a 100644 --- a/icekit_events/tests/tests.py +++ b/icekit_events/tests/tests.py @@ -19,6 +19,7 @@ from django.forms.models import fields_for_model from django.test import TestCase from django.test.utils import override_settings +from django.utils.timezone import make_naive from django_dynamic_fixture import G from django_webtest import WebTest @@ -28,8 +29,6 @@ from icekit_events.models import get_occurrence_times_for_event from icekit_events.utils import timeutils -from timezone.timezone import localize, now - class TestAdmin(WebTest): @@ -727,9 +726,9 @@ def test_event_without_occurrences(self): class TestEventRepeatsGeneratorStrangeness(TestCase): def setUp(self): - self.start = datetime(2016,10,1, 9,0) - self.end = datetime(2016,10,1, 17,0) - self.repeat_end = datetime(2016,10,31, 17,0) + self.start = timezone.datetime(2016,10,1, 9,0) + self.end = timezone.datetime(2016,10,1, 17) + self.repeat_end = timezone.datetime(2016,10,31, 17) self.event = G(SimpleEvent) @@ -745,14 +744,14 @@ def test_daily_occurrences(self): occurrences = self.event.occurrences.all() self.assertEquals(occurrences.count(), 31) - st = localize(self.start).time() - et = localize(self.end).time() + st = timezone.localize(self.start).time() + et = timezone.localize(self.end).time() self.assertEquals(st, time(9,0)) self.assertEquals(et, time(17,0)) for o in occurrences: - self.assertEquals(localize(o.start).time(), st) - self.assertEquals(localize(o.end).time(), et) + self.assertEquals(timezone.localize(o.start).time(), st) + self.assertEquals(timezone.localize(o.end).time(), et) class TestEventRepeatsGeneratorModel(TestCase): @@ -764,6 +763,11 @@ def setUp(self): rounding=timeutils.ROUND_DOWN) self.end = self.start + appsettings.DEFAULT_ENDS_DELTA + self.naive_start = make_naive( + self.start, timezone.get_current_timezone()) + self.naive_end = make_naive( + self.end, timezone.get_current_timezone()) + def test_uses_recurrencerulefield(self): """ Test form field and validators. @@ -882,28 +886,28 @@ def test_limited_daily_repeating_generator(self): # Repeating generator has expected date entries in its RRULESET rruleset = generator.get_rruleset() self.assertEqual(20, rruleset.count()) - self.assertTrue(self.start in rruleset) - self.assertTrue(self.start + timedelta(days=1) in rruleset) - self.assertTrue(self.start + timedelta(days=2) in rruleset) - self.assertTrue(self.start + timedelta(days=19) in rruleset) - self.assertFalse(self.start + timedelta(days=20) in rruleset) + self.assertTrue(self.naive_start in rruleset) + self.assertTrue(self.naive_start + timedelta(days=1) in rruleset) + self.assertTrue(self.naive_start + timedelta(days=2) in rruleset) + self.assertTrue(self.naive_start + timedelta(days=19) in rruleset) + self.assertFalse(self.naive_start + timedelta(days=20) in rruleset) # Repeating generator generates expected start/end times start_and_end_times_list = list(generator.generate()) self.assertEqual(20, len(start_and_end_times_list)) self.assertEqual( - (self.start, self.end), + (self.naive_start, self.naive_end), start_and_end_times_list[0]) self.assertEqual( - (self.start + timedelta(days=1), self.end + timedelta(days=1)), + (self.naive_start + timedelta(days=1), self.naive_end + timedelta(days=1)), start_and_end_times_list[1]) self.assertEqual( - (self.start + timedelta(days=2), self.end + timedelta(days=2)), + (self.naive_start + timedelta(days=2), self.naive_end + timedelta(days=2)), start_and_end_times_list[2]) self.assertEqual( - (self.start + timedelta(days=19), self.end + timedelta(days=19)), + (self.naive_start + timedelta(days=19), self.naive_end + timedelta(days=19)), start_and_end_times_list[19]) self.assertFalse( - (self.start + timedelta(days=20), self.end + timedelta(days=20)) + (self.naive_start + timedelta(days=20), self.naive_end + timedelta(days=20)) in start_and_end_times_list) def test_unlimited_daily_repeating_generator(self): @@ -915,33 +919,33 @@ def test_unlimited_daily_repeating_generator(self): ) # Repeating generator has expected date entries in its RRULESET rruleset = generator.get_rruleset() - self.assertTrue(self.start in rruleset) - self.assertTrue(self.start + timedelta(days=1) in rruleset) - self.assertTrue(self.start + timedelta(days=2) in rruleset) + self.assertTrue(self.naive_start in rruleset) + self.assertTrue(self.naive_start + timedelta(days=1) in rruleset) + self.assertTrue(self.naive_start + timedelta(days=2) in rruleset) # Default ``appsettings.REPEAT_LIMIT`` is 13 weeks - self.assertTrue(self.start + timedelta(days=7 * 13) in rruleset) - self.assertFalse(self.start + timedelta(days=7 * 13 + 1) in rruleset) + self.assertTrue(self.naive_start + timedelta(days=7 * 13) in rruleset) + self.assertFalse(self.naive_start + timedelta(days=7 * 13 + 1) in rruleset) # Repeating generator generates expected start/end times start_and_end_times = generator.generate() self.assertEqual( - (self.start, self.end), + (self.naive_start, self.naive_end), next(start_and_end_times)) self.assertEqual( - (self.start + timedelta(days=1), self.end + timedelta(days=1)), + (self.naive_start + timedelta(days=1), self.naive_end + timedelta(days=1)), next(start_and_end_times)) self.assertEqual( - (self.start + timedelta(days=2), self.end + timedelta(days=2)), + (self.naive_start + timedelta(days=2), self.naive_end + timedelta(days=2)), next(start_and_end_times)) for i in range(16): next(start_and_end_times) self.assertEqual( - (self.start + timedelta(days=19), self.end + timedelta(days=19)), + (self.naive_start + timedelta(days=19), self.naive_end + timedelta(days=19)), next(start_and_end_times)) # Default ``appsettings.REPEAT_LIMIT`` is 13 weeks for i in range(13 * 7 - 20): next(start_and_end_times) self.assertEqual( - (self.start + timedelta(days=91), self.end + timedelta(days=91)), + (self.naive_start + timedelta(days=91), self.naive_end + timedelta(days=91)), next(start_and_end_times)) @@ -975,11 +979,15 @@ def test_initial_event_occurrences_automatically_created(self): first_occurrence = event.occurrences.all()[0] for days_hence in range(20): start = first_occurrence.start + timedelta(days=days_hence) - self.assertTrue(start in occurrence_starts, - "Missing start time %d days hence" % days_hence) + self.assertTrue( + make_naive(start, timezone.get_current_timezone()) + in occurrence_starts, + "Missing start time %d days hence" % days_hence) end = first_occurrence.end + timedelta(days=days_hence) - self.assertTrue(end in occurrence_ends, - "Missing end time %d days hence" % days_hence) + self.assertTrue( + make_naive(end, timezone.get_current_timezone()) + in occurrence_ends, + "Missing end time %d days hence" % days_hence) # Confirm Event correctly returns first & last occurrences self.assertEqual( self.start, event.get_occurrences_range()[0].start) @@ -1036,11 +1044,18 @@ def test_unlimited_daily_repeating_occurrences(self): event.occurrences.all()[19].start) # Default repeat limit prevents far-future occurrences but we can # override that if we want - event.extend_occurrences(until=self.end + timedelta(days=999)) + event.extend_occurrences( + until=self.end + timedelta(days=999, seconds=1)) self.assertEqual(1000, event.occurrences.all().count()) - self.assertEqual( - self.start + timedelta(days=999), - event.occurrences.all()[999].start) + # We need to be careful comparing timese because our test start time + # is affected by daylight savings changes whereas the occurrence start + # time will remain at 9am (for example) regardless of daylight saving + # changes. + delta = \ + self.start + timedelta(days=999) \ + - event.occurrences.all()[999].start + self.assertTrue( + delta <= timedelta(hours=1) or delta <= timedelta(hours=-1)) def test_add_arbitrary_occurrence_to_nonrepeating_event(self): event = G(SimpleEvent) @@ -1060,8 +1075,12 @@ def test_add_arbitrary_occurrence_to_nonrepeating_event(self): self.assertTrue(added_occurrence.is_user_modified) def test_add_arbitrary_occurrences_to_repeating_event(self): - arbitrary_dt1 = self.start + timedelta(days=3, hours=-2) - arbitrary_dt2 = self.start + timedelta(days=7, hours=5) + arbitrary_dt1 = make_naive( + self.start + timedelta(days=3, hours=-2), + timezone.get_current_timezone()) + arbitrary_dt2 = make_naive( + self.start + timedelta(days=7, hours=5), + timezone.get_current_timezone()) event = G(SimpleEvent) generator = G( models.EventRepeatsGenerator, @@ -1123,8 +1142,12 @@ def test_cancel_arbitrary_occurrence_from_repeating_event(self): occurrence_to_cancel_1 = event.occurrences.all()[3] occurrence_to_cancel_2 = event.occurrences.all()[5] occurrence_starts, __ = get_occurrence_times_for_event(event) - self.assertTrue(occurrence_to_cancel_1.start in occurrence_starts) - self.assertTrue(occurrence_to_cancel_2.start in occurrence_starts) + self.assertTrue( + make_naive(occurrence_to_cancel_1.start, timezone.get_current_timezone()) + in occurrence_starts) + self.assertTrue( + make_naive(occurrence_to_cancel_2.start, timezone.get_current_timezone()) + in occurrence_starts) # Cancel occurrences event.cancel_occurrence(occurrence_to_cancel_1) event.cancel_occurrence( @@ -1228,5 +1251,5 @@ def test_round_datetime(self): ) for dt1, dt2, precision, rounding in data: self.assertEqual( - timeutils.ROUND_datetime(datetime(*dt1), precision, rounding), + timeutils.round_datetime(datetime(*dt1), precision, rounding), datetime(*dt2))