From dcc714cc8285953cce48da69d8558583b9c1ac0e Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Wed, 8 May 2019 09:30:54 +0100
Subject: [PATCH] fix the timestamp changing between when a comment updates

---
 opentech/apply/activity/messaging.py          |  2 ++
 .../0022_add_versioning_to_comments.py        |  5 ++++
 opentech/apply/activity/models.py             |  2 +-
 opentech/apply/activity/tests/factories.py    |  2 ++
 opentech/apply/activity/views.py              |  2 ++
 opentech/apply/determinations/views.py        |  3 ++
 opentech/apply/funds/serializers.py           |  3 +-
 opentech/apply/funds/tests/test_api_views.py  | 29 +++++++++++++++++--
 8 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py
index d3c24d144..e5e77f64e 100644
--- a/opentech/apply/activity/messaging.py
+++ b/opentech/apply/activity/messaging.py
@@ -7,6 +7,7 @@ from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth import get_user_model
 from django.template.loader import render_to_string
+from django.utils import timezone
 
 from .models import INTERNAL, PUBLIC
 from .options import MESSAGES
@@ -332,6 +333,7 @@ class ActivityAdapter(AdapterBase):
         Activity.actions.create(
             user=user,
             submission=submission,
+            timestamp=timezone.now(),
             message=message,
             visibility=visibility,
             related_object=related_object,
diff --git a/opentech/apply/activity/migrations/0022_add_versioning_to_comments.py b/opentech/apply/activity/migrations/0022_add_versioning_to_comments.py
index e883730f1..10034b8d2 100644
--- a/opentech/apply/activity/migrations/0022_add_versioning_to_comments.py
+++ b/opentech/apply/activity/migrations/0022_add_versioning_to_comments.py
@@ -11,6 +11,11 @@ class Migration(migrations.Migration):
     ]
 
     operations = [
+        migrations.AlterField(
+            model_name='activity',
+            name='timestamp',
+            field=models.DateTimeField(),
+        ),
         migrations.AddField(
             model_name='activity',
             name='current',
diff --git a/opentech/apply/activity/models.py b/opentech/apply/activity/models.py
index fcd0c488f..089686efe 100644
--- a/opentech/apply/activity/models.py
+++ b/opentech/apply/activity/models.py
@@ -82,7 +82,7 @@ class ActionManager(ActivityBaseManager):
 
 
 class Activity(models.Model):
-    timestamp = models.DateTimeField(auto_now_add=True)
+    timestamp = models.DateTimeField()
     type = models.CharField(choices=ACTIVITY_TYPES.items(), max_length=30)
     user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.PROTECT)
     submission = models.ForeignKey('funds.ApplicationSubmission', related_name='activities', on_delete=models.CASCADE)
diff --git a/opentech/apply/activity/tests/factories.py b/opentech/apply/activity/tests/factories.py
index 30c09ab31..fb312efc0 100644
--- a/opentech/apply/activity/tests/factories.py
+++ b/opentech/apply/activity/tests/factories.py
@@ -1,6 +1,7 @@
 import uuid
 
 import factory
+from django.utils import timezone
 
 from opentech.apply.activity.models import Activity, Event, INTERNAL, Message, MESSAGES, REVIEWER
 from opentech.apply.funds.tests.factories import ApplicationSubmissionFactory
@@ -18,6 +19,7 @@ class CommentFactory(factory.DjangoModelFactory):
     submission = factory.SubFactory(ApplicationSubmissionFactory)
     user = factory.SubFactory(UserFactory)
     message = factory.Faker('sentence')
+    timestamp = factory.LazyFunction(timezone.now)
 
     @classmethod
     def _get_manager(cls, model_class):
diff --git a/opentech/apply/activity/views.py b/opentech/apply/activity/views.py
index 07ccbd32c..cba63a229 100644
--- a/opentech/apply/activity/views.py
+++ b/opentech/apply/activity/views.py
@@ -1,4 +1,5 @@
 from django.views.generic import CreateView
+from django.utils import timezone
 
 from opentech.apply.utils.views import DelegatedViewMixin
 
@@ -57,6 +58,7 @@ class CommentFormView(DelegatedViewMixin, CreateView):
         form.instance.user = self.request.user
         form.instance.submission = self.kwargs['submission']
         form.instance.type = COMMENT
+        form.instance.timestamp = timezone.now()
         response = super().form_valid(form)
         messenger(
             MESSAGES.COMMENT,
diff --git a/opentech/apply/determinations/views.py b/opentech/apply/determinations/views.py
index 0adae9a6f..a5e65f32d 100644
--- a/opentech/apply/determinations/views.py
+++ b/opentech/apply/determinations/views.py
@@ -6,6 +6,7 @@ from django.core.exceptions import PermissionDenied
 from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.urls import reverse_lazy
+from django.utils import timezone
 from django.utils.decorators import method_decorator
 from django.utils.translation import ugettext_lazy as _
 from django.views.generic import DetailView, CreateView
@@ -135,6 +136,7 @@ class BatchDeterminationCreateView(CreateView):
                     # We keep a record of the message sent to the user in the comment
                     Activity.comments.create(
                         message=determination.stripped_message,
+                        timestamp=timezone.now(),
                         user=self.request.user,
                         submission=submission,
                         related_object=determination,
@@ -252,6 +254,7 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView):
                 # We keep a record of the message sent to the user in the comment
                 Activity.comments.create(
                     message=self.object.stripped_message,
+                    timestamp=timezone.now(),
                     user=self.request.user,
                     submission=self.submission,
                     related_object=self.object,
diff --git a/opentech/apply/funds/serializers.py b/opentech/apply/funds/serializers.py
index 4d1ca335e..e88252e2e 100644
--- a/opentech/apply/funds/serializers.py
+++ b/opentech/apply/funds/serializers.py
@@ -226,10 +226,11 @@ class CommentCreateSerializer(serializers.ModelSerializer):
     class Meta:
         model = Activity
         fields = ('id', 'timestamp', 'user', 'message', 'visibility', 'edited')
+        read_only_fields = ('timestamp', 'edited',)
 
 
 class CommentEditSerializer(CommentCreateSerializer):
     user = serializers.StringRelatedField()
 
     class Meta(CommentCreateSerializer.Meta):
-        read_only_fields = ('visibility', 'edited',)
+        read_only_fields = ('timestamp', 'visibility', 'edited',)
diff --git a/opentech/apply/funds/tests/test_api_views.py b/opentech/apply/funds/tests/test_api_views.py
index 68251a09e..df9cb2aab 100644
--- a/opentech/apply/funds/tests/test_api_views.py
+++ b/opentech/apply/funds/tests/test_api_views.py
@@ -1,5 +1,6 @@
 from django.test import TestCase, override_settings
 from django.urls import reverse_lazy
+from django.utils import timezone
 
 from opentech.apply.activity.models import Activity, PUBLIC, PRIVATE
 from opentech.apply.activity.tests.factories import CommentFactory
@@ -30,16 +31,23 @@ class TestCommentEdit(TestCase):
 
         response = self.post_to_edit(comment.pk, new_message)
 
-        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.status_code, 200, response.json())
         self.assertEqual(Activity.objects.count(), 2)
 
         comment.refresh_from_db()
+
+        # Match the behaviour of DRF
+        time = comment.timestamp.astimezone(timezone.get_current_timezone()).isoformat()
+        if time.endswith('+00:00'):
+            time = time[:-6] + 'Z'
+
+        self.assertEqual(time, response.json()['timestamp'])
         self.assertFalse(comment.current)
         self.assertEqual(response.json()['message'], new_message)
 
     def test_incorrect_id_denied(self):
         response = self.post_to_edit(10000)
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 403, response.json())
 
     def test_does_nothing_if_same_message(self):
         user = UserFactory()
@@ -63,5 +71,20 @@ class TestCommentEdit(TestCase):
             },
         )
 
-        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.status_code, 200, response.json())
         self.assertEqual(response.json()['visibility'], PRIVATE)
+
+    def test_out_of_order_does_nothing(self):
+        user = UserFactory()
+        comment = CommentFactory(user=user)
+        self.client.force_login(user)
+
+        new_message = 'hi there'
+        newer_message = 'hello there'
+
+        response_one = self.post_to_edit(comment.pk, new_message)
+        response_two = self.post_to_edit(comment.pk, newer_message)
+
+        self.assertEqual(response_one.status_code, 200, response_one.json())
+        self.assertEqual(response_two.status_code, 404, response_two.json())
+        self.assertEqual(Activity.objects.count(), 2)
-- 
GitLab