Avoid timezone confusion

Patchwork saves patches, comments etc with UTC timezone and reports
this time when opening the patch details. However, internally generated
processes such as events are reported with the instance's local time.
There's nothing wrong with that and making PW timezone-aware would add
useless complexity, but in a world-wide collaboration a lot of confusion
may arise as the timezone is not reported at all. Instance's local time
might be very different from the local time of CI integrating with PW,
which is different from the local time of person dealing with it etc.

Use UTC everywhere by default instead of UTC for sumbissions and local
timezone for internally generated events (which is not reported).

Signed-off-by: Veronika Kabatova <vkabatov@redhat.com>
[dja:
 - squash 2 patches: https://patchwork.ozlabs.org/patch/876744/
                     https://patchwork.ozlabs.org/patch/877815/
 - minor changes to both patches - rejig order of migrations and
   adjust wording: "happened sooner" -> "happened earlier"]
Tested-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Daniel Axtens <dja@axtens.net>

diff --git a/docs/api/rest.rst b/docs/api/rest.rst
index 3d7292ef577306b4f4843bd43666ce97e4dcf928..d526b270567c8543e577d56a888d6b3993a03f9a 100644 (file)
--- a/docs/api/rest.rst
+++ b/docs/api/rest.rst
@@ -107,7 +107,8 @@ Schema
 ------
 
 Responses are returned as JSON. Blank fields are returned as ``null``, rather
-than being omitted. Timestamps use the ISO 8601 format::
+than being omitted. Timestamps use the ISO 8601 format, times are by default
+in UTC::
 
     YYYY-MM-DDTHH:MM:SSZ
 

diff --git a/patchwork/migrations/0023_timezone_unify.py b/patchwork/migrations/0023_timezone_unify.py
new file mode 100644 (file)
index 0000000..8a84ab8
--- /dev/null
+++ b/patchwork/migrations/0023_timezone_unify.py
@@ -0,0 +1,46 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.8 on 2018-02-22 23:11
+from __future__ import unicode_literals
+
+import datetime
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('patchwork', '0022_add_subject_match_to_project'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='check',
+            name='date',
+            field=models.DateTimeField(default=datetime.datetime.utcnow),
+        ),
+        migrations.AlterField(
+            model_name='comment',
+            name='date',
+            field=models.DateTimeField(default=datetime.datetime.utcnow),
+        ),
+        migrations.AlterField(
+            model_name='emailconfirmation',
+            name='date',
+            field=models.DateTimeField(default=datetime.datetime.utcnow),
+        ),
+        migrations.AlterField(
+            model_name='event',
+            name='date',
+            field=models.DateTimeField(default=datetime.datetime.utcnow, help_text=b'The time this event was created.'),
+        ),
+        migrations.AlterField(
+            model_name='patchchangenotification',
+            name='last_modified',
+            field=models.DateTimeField(default=datetime.datetime.utcnow),
+        ),
+        migrations.AlterField(
+            model_name='submission',
+            name='date',
+            field=models.DateTimeField(default=datetime.datetime.utcnow),
+        ),
+    ]

diff --git a/patchwork/models.py b/patchwork/models.py
index 581dbf7ab7224a1cf56d852ce46a6d0d103cbad0..b2491752f04ac1573a8338a45076db4a5d7c3e7b 100644 (file)
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -316,7 +316,7 @@ class EmailMixin(models.Model):
     # email metadata
 
     msgid = models.CharField(max_length=255)
-    date = models.DateTimeField(default=datetime.datetime.now)
+    date = models.DateTimeField(default=datetime.datetime.utcnow)
     headers = models.TextField(blank=True)
 
     # content
@@ -836,7 +836,7 @@ class Check(models.Model):
 
     patch = models.ForeignKey(Patch, on_delete=models.CASCADE)
     user = models.ForeignKey(User, on_delete=models.CASCADE)
-    date = models.DateTimeField(default=datetime.datetime.now)
+    date = models.DateTimeField(default=datetime.datetime.utcnow)
 
     state = models.SmallIntegerField(
         choices=STATE_CHOICES, default=STATE_PENDING,
@@ -908,7 +908,7 @@ class Event(models.Model):
         db_index=True,
         help_text='The category of the event.')
     date = models.DateTimeField(
-        default=datetime.datetime.now,
+        default=datetime.datetime.utcnow,
         help_text='The time this event was created.')
 
     # event object
@@ -973,7 +973,7 @@ class EmailConfirmation(models.Model):
     email = models.CharField(max_length=200)
     user = models.ForeignKey(User, null=True, on_delete=models.CASCADE)
     key = HashField()
-    date = models.DateTimeField(default=datetime.datetime.now)
+    date = models.DateTimeField(default=datetime.datetime.utcnow)
     active = models.BooleanField(default=True)
 
     def deactivate(self):
@@ -981,7 +981,7 @@ class EmailConfirmation(models.Model):
         self.save()
 
     def is_valid(self):
-        return self.date + self.validity > datetime.datetime.now()
+        return self.date + self.validity > datetime.datetime.utcnow()
 
     def save(self, *args, **kwargs):
         limit = 1 << 32
@@ -1007,5 +1007,5 @@ class EmailOptout(models.Model):
 class PatchChangeNotification(models.Model):
     patch = models.OneToOneField(Patch, primary_key=True,
                                  on_delete=models.CASCADE)
-    last_modified = models.DateTimeField(default=datetime.datetime.now)
+    last_modified = models.DateTimeField(default=datetime.datetime.utcnow)
     orig_state = models.ForeignKey(State, on_delete=models.CASCADE)

diff --git a/patchwork/notifications.py b/patchwork/notifications.py
index 88e96628e294482f59f294289182a97b93b8fc16..a5f642352c1dc15e2230de37dca277d1bea79e40 100644 (file)
--- a/patchwork/notifications.py
+++ b/patchwork/notifications.py
@@ -35,7 +35,7 @@ from patchwork.models import PatchChangeNotification
 
 
 def send_notifications():
-    date_limit = datetime.datetime.now() - datetime.timedelta(
+    date_limit = datetime.datetime.utcnow() - datetime.timedelta(
         minutes=settings.NOTIFICATION_DELAY_MINUTES)
 
     # We delay sending notifications to a user if they have other
@@ -104,7 +104,7 @@ def expire_notifications():
     Users whose registration confirmation has expired are removed.
     """
     # expire any invalid confirmations
-    q = (Q(date__lt=datetime.datetime.now() - EmailConfirmation.validity) |
+    q = (Q(date__lt=datetime.datetime.utcnow() - EmailConfirmation.validity) |
          Q(active=False))
     EmailConfirmation.objects.filter(q).delete()
 

diff --git a/patchwork/signals.py b/patchwork/signals.py
index e5e7370f527752dec2a0a276724fd4a2eaf732dd..f7b4f547067a8c98ed3bee73ce1137748134e4d2 100644 (file)
--- a/patchwork/signals.py
+++ b/patchwork/signals.py
@@ -65,7 +65,7 @@ def patch_change_callback(sender, instance, raw, **kwargs):
         notification.delete()
         return
 
-    notification.last_modified = dt.now()
+    notification.last_modified = dt.utcnow()
     notification.save()
 
 

diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
index 6ed20c348486f152b4c1e6136bb4614401008e17..e817713f7079c50233a384aacee371a102865c57 100644 (file)
--- a/patchwork/templates/patchwork/submission.html
+++ b/patchwork/templates/patchwork/submission.html
@@ -255,7 +255,7 @@ function toggle_div(link_id, headers_id)
 <div class="comment">
 <div class="meta">
  <span>{{ submission.submitter|personify:project }}</span>
- <span class="pull-right">{{ submission.date }}</span>
+ <span class="pull-right">{{ submission.date }} UTC</span>
 </div>
 <pre class="content">
 {{ submission|commentsyntax }}
@@ -271,7 +271,7 @@ function toggle_div(link_id, headers_id)
 <div class="comment">
 <div class="meta">
  <span>{{ item.submitter|personify:project }}</span>
- <span class="pull-right">{{ item.date }} | <a href="{% url 'comment-redirect' comment_id=item.id %}"
+ <span class="pull-right">{{ item.date }} UTC | <a href="{% url 'comment-redirect' comment_id=item.id %}"
    >#{{ forloop.counter }}</a></span>
 </div>
 <pre class="content">

diff --git a/patchwork/tests/test_checks.py b/patchwork/tests/test_checks.py
index c0487f345b768b8b453ad216d5fabfffafdf4f6b..797390c8582033e70f29b6fce71d2888f5ece7b8 100644 (file)
--- a/patchwork/tests/test_checks.py
+++ b/patchwork/tests/test_checks.py
@@ -86,12 +86,12 @@ class PatchChecksTest(TransactionTestCase):
         self.assertChecksEqual(self.patch, [check_a, check_b])
 
     def test_checks__duplicate_checks(self):
-        self._create_check(date=(dt.now() - timedelta(days=1)))
+        self._create_check(date=(dt.utcnow() - timedelta(days=1)))
         check = self._create_check()
         # this isn't a realistic scenario (dates shouldn't be set by user so
         # they will always increment), but it's useful to verify the removal
         # of older duplicates by the function
-        self._create_check(date=(dt.now() - timedelta(days=2)))
+        self._create_check(date=(dt.utcnow() - timedelta(days=2)))
         self.assertChecksEqual(self.patch, [check])
 
     def test_checks__nultiple_users(self):
@@ -107,7 +107,7 @@ class PatchChecksTest(TransactionTestCase):
         self.assertCheckCountEqual(self.patch, 1, {Check.STATE_SUCCESS: 1})
 
     def test_check_count__multiple_checks(self):
-        self._create_check(date=(dt.now() - timedelta(days=1)))
+        self._create_check(date=(dt.utcnow() - timedelta(days=1)))
         self._create_check(context='new/test1')
         self.assertCheckCountEqual(self.patch, 2, {Check.STATE_SUCCESS: 2})
 
@@ -117,14 +117,14 @@ class PatchChecksTest(TransactionTestCase):
         self.assertCheckCountEqual(self.patch, 2, {Check.STATE_SUCCESS: 2})
 
     def test_check_count__duplicate_check_same_state(self):
-        self._create_check(date=(dt.now() - timedelta(days=1)))
+        self._create_check(date=(dt.utcnow() - timedelta(days=1)))
         self.assertCheckCountEqual(self.patch, 1, {Check.STATE_SUCCESS: 1})
 
         self._create_check()
         self.assertCheckCountEqual(self.patch, 2, {Check.STATE_SUCCESS: 1})
 
     def test_check_count__duplicate_check_new_state(self):
-        self._create_check(date=(dt.now() - timedelta(days=1)))
+        self._create_check(date=(dt.utcnow() - timedelta(days=1)))
         self.assertCheckCountEqual(self.patch, 1, {Check.STATE_SUCCESS: 1})
 
         self._create_check(state=Check.STATE_FAIL)

diff --git a/patchwork/tests/test_expiry.py b/patchwork/tests/test_expiry.py
index 054d1569fa0561cf63195e405fe79b26e7a29161..ce308bc1cc714e9b5b2a13777b0f8f1e655e069e 100644 (file)
--- a/patchwork/tests/test_expiry.py
+++ b/patchwork/tests/test_expiry.py
@@ -46,7 +46,7 @@ class TestRegistrationExpiry(TestCase):
         return (user, conf)
 
     def test_old_registration_expiry(self):
-        date = ((datetime.datetime.now() - EmailConfirmation.validity) -
+        date = ((datetime.datetime.utcnow() - EmailConfirmation.validity) -
                 datetime.timedelta(hours=1))
         user, conf = self.register(date)
 
@@ -57,7 +57,7 @@ class TestRegistrationExpiry(TestCase):
             EmailConfirmation.objects.filter(pk=conf.pk).exists())
 
     def test_recent_registration_expiry(self):
-        date = ((datetime.datetime.now() - EmailConfirmation.validity) +
+        date = ((datetime.datetime.utcnow() - EmailConfirmation.validity) +
                 datetime.timedelta(hours=1))
         user, conf = self.register(date)
 
@@ -68,7 +68,7 @@ class TestRegistrationExpiry(TestCase):
             EmailConfirmation.objects.filter(pk=conf.pk).exists())
 
     def test_inactive_registration_expiry(self):
-        user, conf = self.register(datetime.datetime.now())
+        user, conf = self.register(datetime.datetime.utcnow())
 
         # confirm registration
         conf.user.is_active = True
@@ -87,7 +87,7 @@ class TestRegistrationExpiry(TestCase):
         submitter = patch.submitter
 
         # ... then starts registration...
-        date = ((datetime.datetime.now() - EmailConfirmation.validity) -
+        date = ((datetime.datetime.utcnow() - EmailConfirmation.validity) -
                 datetime.timedelta(hours=1))
         user = create_user(link_person=False, email=submitter.email)
         user.is_active = False

diff --git a/patchwork/tests/test_notifications.py b/patchwork/tests/test_notifications.py
index 6cd3200733b32a4a165be5e79cc5eddc247f6c44..6d902f89426c80472e6593580fad8b09885ee99a 100644 (file)
--- a/patchwork/tests/test_notifications.py
+++ b/patchwork/tests/test_notifications.py
@@ -120,7 +120,7 @@ class PatchNotificationEmailTest(TestCase):
         self.project = create_project(send_notifications=True)
 
     def _expire_notifications(self, **kwargs):
-        timestamp = datetime.datetime.now() - \
+        timestamp = datetime.datetime.utcnow() - \
             datetime.timedelta(minutes=settings.NOTIFICATION_DELAY_MINUTES + 1)
 
         qs = PatchChangeNotification.objects.all()

diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index fcd6762b5033065549722a37b12f86ea02f4ac3b..f0a71fbe5d9de43c5a9d10e35ab36d4c6a03f6f0 100644 (file)
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -215,7 +215,7 @@ def create_check(**kwargs):
     values = {
         'patch': create_patch() if 'patch' not in kwargs else None,
         'user': create_user() if 'user' not in kwargs else None,
-        'date': dt.now(),
+        'date': dt.utcnow(),
         'state': Check.STATE_SUCCESS,
         'target_url': 'http://example.com/',
         'description': '',
@@ -230,7 +230,7 @@ def create_series(**kwargs):
     """Create 'Series' object."""
     values = {
         'project': create_project() if 'project' not in kwargs else None,
-        'date': dt.now(),
+        'date': dt.utcnow(),
         'submitter': create_person() if 'submitter' not in kwargs else None,
         'total': 1,
     }
@@ -276,7 +276,7 @@ def _create_submissions(create_func, count=1, **kwargs):
         'submitter': create_person() if 'submitter' not in kwargs else None,
     }
     values.update(kwargs)
-    date = dt.now()
+    date = dt.utcnow()
 
     objects = []
     for i in range(0, count):

diff --git a/releasenotes/notes/unify-timezones-0f7022f0c2a371be.yaml b/releasenotes/notes/unify-timezones-0f7022f0c2a371be.yaml
new file mode 100644 (file)
index 0000000..5ea1f18
--- /dev/null
+++ b/releasenotes/notes/unify-timezones-0f7022f0c2a371be.yaml
@@ -0,0 +1,7 @@
+---
+other:
+  - |
+    Unify timezones used -- use UTC for both email submissions and internal
+    events. Please note that this change doesn't modify already existing data
+    so in case the instance's timezone is UTC+XX, events will appear out of
+    order (as if they happened earlier) for XX hours in the events API feed.