diff --git a/news/204.bugfix b/news/204.bugfix new file mode 100644 index 0000000..c6f022d --- /dev/null +++ b/news/204.bugfix @@ -0,0 +1 @@ + Set timezones for creation and modification dates of comments [instification] \ No newline at end of file diff --git a/plone/app/discussion/browser/comments.py b/plone/app/discussion/browser/comments.py index 387cead..02ca122 100644 --- a/plone/app/discussion/browser/comments.py +++ b/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.modification_date = datetime.utcnow() + comment.creation_date = localized_now() + comment.modification_date = localized_now() # Get author name and email comment.author_name, comment.author_email = self.get_author(data) diff --git a/plone/app/discussion/catalog.py b/plone/app/discussion/catalog.py index 6413d11..5acb28b 100644 --- a/plone/app/discussion/catalog.py +++ b/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( - 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", - ) + return DT( object.creation_date ) @indexer(IComment) def created(object): # the catalog index needs Zope DateTime instead of Python datetime - return DateTime( - 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", - ) + return DT(object.creation_date) @indexer(IComment) def modified(object): # the catalog index needs Zope DateTime instead of Python datetime - return DateTime( - 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", - ) + return DT(object.modification_date) # Override the conversation indexers for comments diff --git a/plone/app/discussion/comment.py b/plone/app/discussion/comment.py index 438e726..0186a6e 100644 --- a/plone/app/discussion/comment.py +++ b/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 diff --git a/plone/app/discussion/profiles/default/metadata.xml b/plone/app/discussion/profiles/default/metadata.xml index 141a440..3b4e730 100644 --- a/plone/app/discussion/profiles/default/metadata.xml +++ b/plone/app/discussion/profiles/default/metadata.xml @@ -1,5 +1,5 @@ - 2000 + 2001 profile-plone.resource:default profile-plone.app.registry:default diff --git a/plone/app/discussion/tests/test_catalog.py b/plone/app/discussion/tests/test_catalog.py index 395f6c2..03af8e9 100644 --- a/plone/app/discussion/tests/test_catalog.py +++ b/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.modification_date = datetime(2006, 9, 17, 14, 18, 12) + comment1.creation_date = \ + 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.modification_date = datetime(2009, 9, 17, 14, 18, 12) + comment2.creation_date = \ + 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 diff --git a/plone/app/discussion/tests/test_comment.py b/plone/app/discussion/tests/test_comment.py index a0a0669..f9a9ac0 100644 --- a/plone/app/discussion/tests/test_comment.py +++ b/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 diff --git a/plone/app/discussion/tests/test_conversation.py b/plone/app/discussion/tests/test_conversation.py index febce28..b14888e 100644 --- a/plone/app/discussion/tests/test_conversation.py +++ b/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): diff --git a/plone/app/discussion/tests/test_indexers.py b/plone/app/discussion/tests/test_indexers.py index b1b30f1..a6f577f 100644 --- a/plone/app/discussion/tests/test_indexers.py +++ b/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.modification_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).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.modification_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).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): diff --git a/plone/app/discussion/upgrades.py b/plone/app/discussion/upgrades.py index 40cdc8f..fbad45c 100644 --- a/plone/app/discussion/upgrades.py +++ b/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)) diff --git a/plone/app/discussion/upgrades.zcml b/plone/app/discussion/upgrades.zcml index 20b288e..7f8ffe7 100644 --- a/plone/app/discussion/upgrades.zcml +++ b/plone/app/discussion/upgrades.zcml @@ -88,4 +88,15 @@ handler=".upgrades.upgrade_comment_workflows" /> + + + + diff --git a/setup.py b/setup.py index 31aa776..e5dfd16 100644 --- a/setup.py +++ b/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",