Merge pull request #204 from plone/set_timezone

Set timezone for creation and modification dates of comments

news/204.bugfix

@@ -0,0 +1 @@
+ Set timezones for creation and modification dates of comments [instification]

plone/app/discussion/browser/comments.py

@@ -1,7 +1,6 @@
 from AccessControl import getSecurityManager
 from AccessControl import Unauthorized
 from Acquisition import aq_inner
-from datetime import datetime
 from DateTime import DateTime
 from plone.app.discussion import _
 from plone.app.discussion.browser.validator import CaptchaValidator
@@ -10,6 +9,7 @@ from plone.app.discussion.interfaces import IComment
 from plone.app.discussion.interfaces import IConversation
 from plone.app.discussion.interfaces import IDiscussionSettings
 from plone.app.discussion.interfaces import IReplies
+from plone.app.event.base import localized_now
 from plone.app.layout.viewlets.common import ViewletBase
 from plone.base.utils import safe_text
 from plone.registry.interfaces import IRegistry
@@ -192,8 +192,8 @@ class CommentForm(extensible.ExtensibleForm, form.Form):
             setattr(comment, attribute, data[attribute])
 
         # Set dates
-        comment.creation_date = datetime.utcnow()
+        comment.creation_date = localized_now()
-        comment.modification_date = datetime.utcnow()
+        comment.modification_date = localized_now()
 
         # Get author name and email
         comment.author_name, comment.author_email = self.get_author(data)

plone/app/discussion/catalog.py

@@ -6,6 +6,7 @@ Also provide event handlers to actually catalog the comments.
 from DateTime import DateTime
 from plone.app.discussion.interfaces import IComment
 from plone.app.discussion.interfaces import IConversation
+from plone.app.event.base import DT
 from plone.base.utils import safe_text
 from plone.indexer import indexer
 from plone.uuid.interfaces import IUUID
@@ -102,43 +103,19 @@ def in_response_to(object):
 @indexer(IComment)
 def effective(object):
     # the catalog index needs Zope DateTime instead of Python datetime
-    return DateTime(
+    return DT( object.creation_date )
-        object.creation_date.year,
-        object.creation_date.month,
-        object.creation_date.day,
-        object.creation_date.hour,
-        object.creation_date.minute,
-        object.creation_date.second,
-        "GMT",
-    )
 
 
 @indexer(IComment)
 def created(object):
     # the catalog index needs Zope DateTime instead of Python datetime
-    return DateTime(
+    return DT(object.creation_date)
-        object.creation_date.year,
-        object.creation_date.month,
-        object.creation_date.day,
-        object.creation_date.hour,
-        object.creation_date.minute,
-        object.creation_date.second,
-        "GMT",
-    )
 
 
 @indexer(IComment)
 def modified(object):
     # the catalog index needs Zope DateTime instead of Python datetime
-    return DateTime(
+    return DT(object.modification_date)
-        object.modification_date.year,
-        object.modification_date.month,
-        object.modification_date.day,
-        object.modification_date.hour,
-        object.modification_date.minute,
-        object.modification_date.second,
-        "GMT",
-    )
 
 
 # Override the conversation indexers for comments

plone/app/discussion/comment.py

@@ -5,10 +5,10 @@ from AccessControl.SecurityManagement import getSecurityManager
 from Acquisition import aq_base
 from Acquisition import aq_parent
 from Acquisition import Implicit
-from datetime import datetime
 from OFS.owner import Owned
 from OFS.role import RoleManager
 from OFS.Traversable import Traversable
+from datetime import timezone
 from persistent import Persistent
 from plone.app.discussion import _
 from plone.app.discussion.events import CommentAddedEvent
@@ -20,6 +20,7 @@ from plone.app.discussion.events import ReplyRemovedEvent
 from plone.app.discussion.interfaces import IComment
 from plone.app.discussion.interfaces import IConversation
 from plone.app.discussion.interfaces import IDiscussionSettings
+from plone.app.event.base import localized_now
 from plone.base.interfaces.controlpanel import IMailSchema
 from plone.base.utils import safe_text
 from plone.registry.interfaces import IRegistry
@@ -119,7 +120,7 @@ class Comment(
     # IConversation.addComment().
 
     def __init__(self):
-        self.creation_date = self.modification_date = datetime.utcnow()
+        self.creation_date = self.modification_date = localized_now()
         self.mime_type = "text/plain"
 
         user = getSecurityManager().getUser()
@@ -133,6 +134,17 @@ class Comment(
                 user.getId(): ["Owner"],
             }
 
+    def __getattribute__(self, attr):
+        # In older versions of the add-on dates were set timezone naive.
+        # In tz aware versions, the value is stored as self._creation_date
+        if attr in ["creation_date", "modification_date"]:
+            old_date = super(Comment, self).__getattribute__(attr)
+            if old_date.tzinfo is None:
+                # Naive dates were always stored utc
+                return old_date.replace(tzinfo=timezone.utc)
+            return old_date
+        return super().__getattribute__(attr)
+
     @property
     def __name__(self):
         return self.comment_id and str(self.comment_id) or None

plone/app/discussion/profiles/default/metadata.xml

@@ -1,5 +1,5 @@
 <metadata>
- <version>2000</version>
+ <version>2001</version>
  <dependencies>
   <dependency>profile-plone.resource:default</dependency>
   <dependency>profile-plone.app.registry:default</dependency>

plone/app/discussion/tests/test_catalog.py

@@ -1,6 +1,7 @@
 """Test the plone.app.discussion catalog indexes
 """
 from datetime import datetime
+from datetime import timezone
 from plone.app.discussion.interfaces import IConversation
 from plone.app.discussion.testing import (  # noqa
     PLONE_APP_DISCUSSION_INTEGRATION_TESTING,
@@ -67,8 +68,10 @@ class ConversationCatalogTest(unittest.TestCase):
         comment1.text = "Comment text"
         comment1.creator = "jim"
         comment1.author_username = "Jim"
-        comment1.creation_date = datetime(2006, 9, 17, 14, 18, 12)
+        comment1.creation_date = \
-        comment1.modification_date = datetime(2006, 9, 17, 14, 18, 12)
+            datetime(2006, 9, 17, 14, 18, 12).astimezone(timezone.utc)
+        comment1.modification_date = \
+            datetime(2006, 9, 17, 14, 18, 12).astimezone(timezone.utc)
 
         new_comment1_id = conversation.addComment(comment1)
         self.comment_id = new_comment1_id
@@ -115,15 +118,17 @@ class ConversationCatalogTest(unittest.TestCase):
         self.assertTrue("last_comment_date" in self.doc1_brain)
         self.assertEqual(
             self.doc1_brain.last_comment_date,
-            datetime(2006, 9, 17, 14, 18, 12),
+            datetime(2006, 9, 17, 14, 18, 12).astimezone(timezone.utc),
         )
 
         # Add another comment and check if last comment date is updated.
         comment2 = createObject("plone.Comment")
         comment2.title = "Comment 2"
         comment2.text = "Comment text"
-        comment2.creation_date = datetime(2009, 9, 17, 14, 18, 12)
+        comment2.creation_date = \
-        comment2.modification_date = datetime(2009, 9, 17, 14, 18, 12)
+            datetime(2009, 9, 17, 14, 18, 12).astimezone(timezone.utc)
+        comment2.modification_date = \
+            datetime(2009, 9, 17, 14, 18, 12).astimezone(timezone.utc)
         new_comment2_id = self.conversation.addComment(comment2)
 
         comment2 = self.portal.doc1.restrictedTraverse(
@@ -141,7 +146,7 @@ class ConversationCatalogTest(unittest.TestCase):
         doc1_brain = brains[0]
         self.assertEqual(
             doc1_brain.last_comment_date,
-            datetime(2009, 9, 17, 14, 18, 12),
+            datetime(2009, 9, 17, 14, 18, 12).astimezone(timezone.utc),
         )
 
         # Remove the comment again
@@ -158,7 +163,7 @@ class ConversationCatalogTest(unittest.TestCase):
         doc1_brain = brains[0]
         self.assertEqual(
             doc1_brain.last_comment_date,
-            datetime(2006, 9, 17, 14, 18, 12),
+            datetime(2006, 9, 17, 14, 18, 12).astimezone(timezone.utc),
         )
 
         # remove all comments

plone/app/discussion/tests/test_comment.py

@@ -63,7 +63,7 @@ class CommentTest(unittest.TestCase):
                 "get hidden by that"
             )
         comment1 = createObject("plone.Comment")
-        local_utc = datetime.datetime.utcnow()
+        local_utc = datetime.datetime.now().astimezone(datetime.timezone.utc)
         for date in (comment1.creation_date, comment1.modification_date):
             difference = abs(date - local_utc)
             difference = difference.seconds

plone/app/discussion/tests/test_conversation.py

@@ -8,6 +8,9 @@ from Acquisition import aq_base
 from Acquisition import aq_parent
 from datetime import datetime
 from datetime import timedelta
+from datetime import timezone
+from dateutil import tz
+from plone.app.event.base import default_timezone
 from plone.app.testing import setRoles
 from plone.app.testing import TEST_USER_ID
 from plone.app.vocabularies.types import BAD_TYPES
@@ -17,6 +20,7 @@ from Products.CMFCore.utils import getToolByName
 from zope import interface
 from zope.annotation.interfaces import IAnnotations
 from zope.component import createObject
+from zope.component import getUtility
 from zope.component import queryUtility
 
 import unittest
@@ -31,6 +35,11 @@ class ConversationTest(unittest.TestCase):
         setRoles(self.portal, TEST_USER_ID, ["Manager"])
         interface.alsoProvides(self.portal.REQUEST, IDiscussionLayer)
 
+        # Set the portal timezone to something non-utc
+        reg_key = "plone.portal_timezone"
+        registry = getUtility(IRegistry)
+        registry[reg_key] = "America/Los_Angeles"
+
         self.typetool = self.portal.portal_types
         self.portal_discussion = getToolByName(
             self.portal,
@@ -70,9 +79,43 @@ class ConversationTest(unittest.TestCase):
         self.assertEqual(len(tuple(conversation.getThreads())), 1)
         self.assertEqual(conversation.total_comments(), 1)
         self.assertTrue(
-            conversation.last_comment_date - datetime.utcnow() < timedelta(seconds=1),
+            datetime.now().astimezone(tz.gettz(default_timezone()))
+            - conversation.last_comment_date
+            >= timedelta(seconds=0)
+            <= timedelta(seconds=1),
         )
 
+    def test_timezone_naive_comment(self):
+        # Create a conversation. In this case we doesn't assign it to an
+        # object, as we just want to check the Conversation object API.
+        conversation = IConversation(self.portal.doc1)
+
+        # Add a comment. Note: in real life, we always create comments via the
+        # factory to allow different factories to be swapped in
+        comment = createObject("plone.Comment")
+        comment.text = "Comment text"
+
+        conversation.addComment(comment)
+
+        # Check that comments have the correct portal timezones
+        self.assertTrue(comment.creation_date.tzinfo, tz.gettz("America/Los_Angeles"))
+        self.assertTrue(comment.modification_date.tzinfo, tz.gettz("America/Los_Angeles"))
+
+        # Remove the timezone from the comment dates
+        comment.creation_date = datetime.utcnow()
+        comment.modification_date = datetime.utcnow()
+
+        # Check that the timezone naive date is converted to UTC
+        # See https://github.com/plone/plone.app.discussion/pull/204
+        self.assertTrue(
+            datetime.utcnow().replace(tzinfo=timezone.utc)
+            - conversation.last_comment_date
+            >= timedelta(seconds=0)
+            <= timedelta(seconds=1),
+        )
+        self.assertTrue(comment.creation_date.tzinfo, timezone.utc)
+        self.assertTrue(comment.modification_date.tzinfo, timezone.utc)
+
     def test_private_comment(self):
         conversation = IConversation(self.portal.doc1)
 
@@ -488,27 +531,35 @@ class ConversationTest(unittest.TestCase):
         # swapped in
         comment1 = createObject("plone.Comment")
         comment1.text = "Comment text"
-        comment1.creation_date = datetime.utcnow() - timedelta(4)
+        comment1.creation_date = datetime.now().astimezone(
+            tz.gettz(default_timezone())
+        ) - timedelta(4)
         conversation.addComment(comment1)
 
         comment2 = createObject("plone.Comment")
         comment2.text = "Comment text"
-        comment2.creation_date = datetime.utcnow() - timedelta(2)
+        comment2.creation_date = datetime.now().astimezone(
+            tz.gettz(default_timezone())
+        ) - timedelta(2)
         new_comment2_id = conversation.addComment(comment2)
 
         comment3 = createObject("plone.Comment")
         comment3.text = "Comment text"
-        comment3.creation_date = datetime.utcnow() - timedelta(1)
+        comment3.creation_date = datetime.now().astimezone(
+            tz.gettz(default_timezone())
+        ) - timedelta(1)
         new_comment3_id = conversation.addComment(comment3)
 
         # check if the latest comment is exactly one day old
         self.assertTrue(
             conversation.last_comment_date
-            < datetime.utcnow() - timedelta(hours=23, minutes=59, seconds=59),
+            < datetime.now().astimezone(tz.gettz(default_timezone()))
+            - timedelta(hours=23, minutes=59, seconds=59),
         )
         self.assertTrue(
             conversation.last_comment_date
-            > datetime.utcnow() - timedelta(days=1, seconds=1),
+            > datetime.now().astimezone(tz.gettz(default_timezone()))
+            - timedelta(days=1, seconds=1),
         )
 
         # remove the latest comment
@@ -518,11 +569,13 @@ class ConversationTest(unittest.TestCase):
         # the latest comment should be exactly two days old
         self.assertTrue(
             conversation.last_comment_date
-            < datetime.utcnow() - timedelta(days=1, hours=23, minutes=59, seconds=59),
+            < datetime.now().astimezone(tz.gettz(default_timezone()))
+            - timedelta(days=1, hours=23, minutes=59, seconds=59),
         )
         self.assertTrue(
             conversation.last_comment_date
-            > datetime.utcnow() - timedelta(days=2, seconds=1),
+            > datetime.now().astimezone(tz.gettz(default_timezone()))
+            - timedelta(days=2, seconds=1),
         )
 
         # remove the latest comment again
@@ -532,11 +585,13 @@ class ConversationTest(unittest.TestCase):
         # the latest comment should be exactly four days old
         self.assertTrue(
             conversation.last_comment_date
-            < datetime.utcnow() - timedelta(days=3, hours=23, minutes=59, seconds=59),
+            < datetime.now().astimezone(tz.gettz(default_timezone()))
+            - timedelta(days=3, hours=23, minutes=59, seconds=59),
         )
         self.assertTrue(
             conversation.last_comment_date
-            > datetime.utcnow() - timedelta(days=4, seconds=2),
+            > datetime.now().astimezone(tz.gettz(default_timezone()))
+            - timedelta(days=4, seconds=2),
         )
 
     def test_get_comments_full(self):

plone/app/discussion/tests/test_indexers.py

@@ -4,11 +4,15 @@ from .. import catalog
 from ..interfaces import IConversation
 from ..testing import PLONE_APP_DISCUSSION_INTEGRATION_TESTING  # noqa
 from datetime import datetime
+from dateutil import tz
 from DateTime import DateTime
+from plone.app.event.base import default_timezone
 from plone.app.testing import setRoles
 from plone.app.testing import TEST_USER_ID
 from plone.indexer.delegate import DelegatingIndexerFactory
+from plone.registry.interfaces import IRegistry
 from zope.component import createObject
+from zope.component import getUtility
 
 import unittest
 
@@ -36,6 +40,12 @@ class ConversationIndexersTest(unittest.TestCase):
         workflow = self.portal.portal_workflow
         workflow.doActionFor(self.portal.doc1, "publish")
 
+        # Change the timezone to PDT to test timezones properly
+        reg_key = "plone.portal_timezone"
+        registry = getUtility(IRegistry)
+        registry[reg_key] = "America/Los_Angeles"
+        self.portal_timezone = tz.gettz(default_timezone())
+
         # Create a conversation.
         conversation = IConversation(self.portal.doc1)
 
@@ -43,6 +53,8 @@ class ConversationIndexersTest(unittest.TestCase):
         comment1.text = "Comment Text"
         comment1.creator = "jim"
         comment1.author_username = "Jim"
+        # Purposefully exclude timezone to test the conversation getter
+        # (see plone.app.discussion.comment.Comment object)
         comment1.creation_date = datetime(2006, 9, 17, 14, 18, 12)
         comment1.modification_date = datetime(2006, 9, 17, 14, 18, 12)
         self.new_id1 = conversation.addComment(comment1)
@@ -51,16 +63,16 @@ class ConversationIndexersTest(unittest.TestCase):
         comment2.text = "Comment Text"
         comment2.creator = "emma"
         comment2.author_username = "Emma"
-        comment2.creation_date = datetime(2007, 12, 13, 4, 18, 12)
+        comment2.creation_date = datetime(2007, 12, 13, 4, 18, 12).astimezone(self.portal_timezone)
-        comment2.modification_date = datetime(2007, 12, 13, 4, 18, 12)
+        comment2.modification_date = datetime(2007, 12, 13, 4, 18, 12).astimezone(self.portal_timezone)
         self.new_id2 = conversation.addComment(comment2)
 
         comment3 = createObject("plone.Comment")
         comment3.text = "Comment Text"
         comment3.creator = "lukas"
         comment3.author_username = "Lukas"
-        comment3.creation_date = datetime(2009, 4, 12, 11, 12, 12)
+        comment3.creation_date = datetime(2009, 4, 12, 11, 12, 12).astimezone(self.portal_timezone)
-        comment3.modification_date = datetime(2009, 4, 12, 11, 12, 12)
+        comment3.modification_date = datetime(2009, 4, 12, 11, 12, 12).astimezone(self.portal_timezone)
         self.new_id3 = conversation.addComment(comment3)
 
         self.conversation = conversation
@@ -88,12 +100,12 @@ class ConversationIndexersTest(unittest.TestCase):
         )
         self.assertEqual(
             catalog.last_comment_date(self.portal.doc1)(),
-            datetime(2009, 4, 12, 11, 12, 12),
+            datetime(2009, 4, 12, 11, 12, 12).astimezone(self.portal_timezone),
         )
         del self.conversation[self.new_id3]
         self.assertEqual(
             catalog.last_comment_date(self.portal.doc1)(),
-            datetime(2007, 12, 13, 4, 18, 12),
+            datetime(2007, 12, 13, 4, 18, 12).astimezone(self.portal_timezone),
         )
         del self.conversation[self.new_id2]
         del self.conversation[self.new_id1]
@@ -122,12 +134,21 @@ class CommentIndexersTest(unittest.TestCase):
         # Add a comment. Note: in real life, we always create comments via the
         # factory to allow different factories to be swapped in
 
+        # Set the portal timezone to something non-utc
+        reg_key = "plone.portal_timezone"
+        registry = getUtility(IRegistry)
+        registry[reg_key] = "America/Los_Angeles"
+
         comment = createObject("plone.Comment")
         comment.text = "Lorem ipsum dolor sit amet."
         comment.creator = "jim"
         comment.author_name = "Jim"
-        comment.creation_date = datetime(2006, 9, 17, 14, 18, 12)
+
-        comment.modification_date = datetime(2008, 3, 12, 7, 32, 52)
+        # Create date in PDT (ie daylight savings)
+        comment.creation_date = datetime(2006, 9, 17, 14, 18, 12).replace(tzinfo=tz.gettz("America/Los_Angeles"))
+
+        # Create date in PST (ie not daylight savings)
+        comment.modification_date = datetime(2008, 2, 12, 7, 32, 52).replace(tzinfo=tz.gettz("America/Los_Angeles"))
 
         self.comment_id = conversation.addComment(comment)
         self.comment = comment.__of__(conversation)
@@ -161,15 +182,15 @@ class CommentIndexersTest(unittest.TestCase):
         # Test if created, modified, effective etc. are set correctly
         self.assertEqual(
             catalog.created(self.comment)(),
-            DateTime(2006, 9, 17, 14, 18, 12, "GMT"),
+            DateTime(2006, 9, 17, 14, 18, 12, "America/Los_Angeles"),
         )
         self.assertEqual(
             catalog.effective(self.comment)(),
-            DateTime(2006, 9, 17, 14, 18, 12, "GMT"),
+            DateTime(2006, 9, 17, 14, 18, 12, "America/Los_Angeles"),
         )
         self.assertEqual(
             catalog.modified(self.comment)(),
-            DateTime(2008, 3, 12, 7, 32, 52, "GMT"),
+            DateTime(2008, 2, 12, 7, 32, 52, "America/Los_Angeles"),
         )
 
     def test_searchable_text(self):

plone/app/discussion/upgrades.py

@@ -2,6 +2,8 @@ from plone.app.discussion.interfaces import IDiscussionSettings
 from plone.registry.interfaces import IRegistry
 from Products.CMFCore.utils import getToolByName
 from zope.component import getUtility
+from plone import api
+from datetime import timezone
 
 import logging
 
@@ -77,3 +79,24 @@ def add_js_to_plone_legacy(context):
 def extend_review_workflow(context):
     """Apply changes made to review workflow."""
     upgrade_comment_workflows_retain_current_workflow(context)
+
+
+def set_timezone_on_dates(context):
+    """Ensure timezone data is stored against all creation/modified dates"""
+    pc = api.portal.get_tool('portal_catalog')
+    creations = 0
+    modifieds = 0
+    logger.info('Setting timezone information on comment dates')
+    comments = pc.search({'Type': 'Comment'})
+    for cbrain in comments:
+        comment = cbrain.getObject()
+        if not comment.creation_date.tzinfo:
+            creations += 1
+            comment.creation_date = \
+                comment.creation_date.astimezone(timezone.utc)
+        if not comment.modification_date.tzinfo:
+            modifieds += 1
+            comment.modification_date = \
+                comment.modification_date.astimezone(timezone.utc)
+    logger.info('Updated %i creation dates and %i modification dates' %
+                (creations, modifieds))

plone/app/discussion/upgrades.zcml

@@ -88,4 +88,15 @@
         handler=".upgrades.upgrade_comment_workflows" />
   </genericsetup:upgradeSteps>
 
+  <genericsetup:upgradeSteps
+    source="2000"
+    destination="2001"
+    profile="plone.app.discussion:default">
+    <genericsetup:upgradeStep
+        title="Set timezone on comment dates"
+        description="Ensure the timezone is set for comment dates"
+        handler=".upgrades.set_timezone_on_dates"
+        />
+  </genericsetup:upgradeSteps>
+
 </configure>

setup.py

@@ -2,7 +2,7 @@ from setuptools import find_packages
 from setuptools import setup
 
 
-version = "4.0.0b4.dev0"
+version = "4.0.0b4.dev1"
 
 install_requires = [
     "setuptools",