From 485571325f349d666c0c022d9c8226d8ee217d96 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Tue, 27 Nov 2012 21:32:07 -0800 Subject: [PATCH 1/4] fix migration of author_name and author_username --- plone/app/discussion/browser/migration.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plone/app/discussion/browser/migration.py b/plone/app/discussion/browser/migration.py index 7c55491..2762f2a 100644 --- a/plone/app/discussion/browser/migration.py +++ b/plone/app/discussion/browser/migration.py @@ -70,6 +70,8 @@ class View(BrowserView): workflow = context.portal_workflow oldchain = workflow.getChainForPortalType('Discussion Item') new_workflow = workflow.comment_review_workflow + mt = getToolByName(self.context, 'portal_membership') + if type(oldchain) == TupleType and len(oldchain) > 0: oldchain = oldchain[0] @@ -96,6 +98,9 @@ class View(BrowserView): comment.text = reply.cooked_text comment.mime_type = 'text/html' comment.creator = reply.Creator() + comment.author_username = reply.getProperty('author_username',reply.Creator()) + member = mt.getMemberById(comment.author_username) + comment.author_name = member.getProperty('fullname',None) email = reply.getProperty('email', None) if email: From ff1f65a75ea72227a11884847fbb304d48c1dca2 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Thu, 13 Dec 2012 21:14:40 -0800 Subject: [PATCH 2/4] simplify some of the logic and get some tests in place to cover them. TDB: test against actual site to ensure the changes to the logic don't break a real environment, but still work with the tests.. --- plone/app/discussion/browser/migration.py | 21 ++++--- plone/app/discussion/tests/test_migration.py | 63 ++++++++++++++++++++ 2 files changed, 77 insertions(+), 7 deletions(-) diff --git a/plone/app/discussion/browser/migration.py b/plone/app/discussion/browser/migration.py index 2762f2a..30d285b 100644 --- a/plone/app/discussion/browser/migration.py +++ b/plone/app/discussion/browser/migration.py @@ -72,7 +72,6 @@ class View(BrowserView): new_workflow = workflow.comment_review_workflow mt = getToolByName(self.context, 'portal_membership') - if type(oldchain) == TupleType and len(oldchain) > 0: oldchain = oldchain[0] @@ -91,8 +90,7 @@ class View(BrowserView): new_in_reply_to = None if should_migrate: - - # create a reply object + # create a reply object comment = CommentFactory() comment.title = reply.Title() comment.text = reply.cooked_text @@ -100,11 +98,20 @@ class View(BrowserView): comment.creator = reply.Creator() comment.author_username = reply.getProperty('author_username',reply.Creator()) member = mt.getMemberById(comment.author_username) - comment.author_name = member.getProperty('fullname',None) + if member: +# comment.author_name = member.get('fullname') + comment.author_name = member.fullname +# import pdb; pdb.set_trace() - email = reply.getProperty('email', None) - if email: - comment.author_email = email +# .get is overloaded into zope somewhere.. so it'snot getting reply.email +## email = reply.get('email') +## if email: +## comment.author_email = email + + try: + comment.author_email = reply.email + except AttributeError: + comment.author_email = None comment.creation_date = DT2dt(reply.creation_date) comment.modification_date = DT2dt(reply.modification_date) diff --git a/plone/app/discussion/tests/test_migration.py b/plone/app/discussion/tests/test_migration.py index 3d86bb0..d4ac2fd 100644 --- a/plone/app/discussion/tests/test_migration.py +++ b/plone/app/discussion/tests/test_migration.py @@ -95,6 +95,69 @@ class MigrationTest(unittest.TestCase): ], list(conversation.getThreads())) self.assertFalse(self.doc.talkback) + + def test_migrate_comment_with_creator(self): + + # Create a user Jimmy Jones so comments creator migration can work? + acl_users = getToolByName(self.portal, 'acl_users') + acl_users.userFolderAddUser('Jim', 'secret', ['Member'], []) + mt = getToolByName(self.portal, 'portal_membership') + member = mt.getMemberById('Jim') + member.fullname = 'Jimmy Jones' + + + # Create a comment + talkback = self.discussion.getDiscussionFor(self.doc) + self.doc.talkback.createReply('My Title', 'My Text', Creator='Jim') + reply = talkback.getReplies()[0] + reply.setReplyTo(self.doc) + reply.creation_date = DateTime(2003, 3, 11, 9, 28, 6, 'GMT') + reply.modification_date = DateTime(2009, 7, 12, 19, 38, 7, 'GMT') + reply.author_username = 'Jim' + reply.email = 'jimmy@jones.xyz' + + self._publish(reply) + self.assertEqual(reply.Title(), 'My Title') + self.assertEqual(reply.EditableBody(), 'My Text') + self.assertTrue('Jim' in reply.listCreators()) + self.assertEqual(talkback.replyCount(self.doc), 1) + self.assertEqual(reply.inReplyTo(), self.doc) + self.assertEqual(reply.author_username,'Jim') + self.assertEqual(reply.email,'jimmy@jones.xyz') + + # Call migration script + self.view() + + # Make sure a conversation has been created + self.assertTrue('plone.app.discussion:conversation' in + IAnnotations(self.doc)) + conversation = IConversation(self.doc) + + # Check migration + self.assertEqual(conversation.total_comments, 1) + self.assertTrue(conversation.getComments().next()) + comment1 = conversation.values()[0] + self.assertTrue(IComment.providedBy(comment1)) + self.assertEqual(comment1.Title(), 'My Title') + self.assertEqual(comment1.text, '

My Text

\n') + self.assertEqual(comment1.mime_type, 'text/html') + self.assertEqual(comment1.Creator(), 'Jim') + self.assertEqual(comment1.creation_date, + datetime(2003, 3, 11, 9, 28, 6)) + self.assertEqual(comment1.modification_date, + datetime(2009, 7, 12, 19, 38, 7)) + self.assertEqual([ + {'comment': comment1, 'depth': 0, 'id': long(comment1.id)} + ], list(conversation.getThreads())) + self.assertFalse(self.doc.talkback) + + # Though this should be Jimmy, but looks like getProperty won't pick up 'author_username' + # (reply.author_username is not None), so it's propagating Creator()..? + self.assertEqual(comment1.author_username,'Jim') + + self.assertEqual(comment1.author_name,'Jimmy Jones') + self.assertEqual(comment1.author_email,'jimmy@jones.xyz') + def test_migrate_nested_comments(self): # Create some nested comments and migrate them # From 372b573cb12f6c2ee7036facf38cc074f266f6a2 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Sat, 15 Dec 2012 16:45:56 -0800 Subject: [PATCH 3/4] move creation of test user to test setup, rather than just in the new test_migrate_comment_with_creator() method --- plone/app/discussion/tests/test_migration.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/plone/app/discussion/tests/test_migration.py b/plone/app/discussion/tests/test_migration.py index d4ac2fd..98a12d5 100644 --- a/plone/app/discussion/tests/test_migration.py +++ b/plone/app/discussion/tests/test_migration.py @@ -50,6 +50,13 @@ class MigrationTest(unittest.TestCase): self.workflowTool.setChainForPortalTypes(('Discussion Item',), 'comment_review_workflow') + # Create a user Jimmy Jones so comments creator migration can work? + acl_users = getToolByName(self.portal, 'acl_users') + acl_users.userFolderAddUser('Jim', 'secret', ['Member'], []) + mt = getToolByName(self.portal, 'portal_membership') + member = mt.getMemberById('Jim') + member.fullname = 'Jimmy Jones' + self.doc = self.portal.doc def test_migrate_comment(self): @@ -98,12 +105,6 @@ class MigrationTest(unittest.TestCase): def test_migrate_comment_with_creator(self): - # Create a user Jimmy Jones so comments creator migration can work? - acl_users = getToolByName(self.portal, 'acl_users') - acl_users.userFolderAddUser('Jim', 'secret', ['Member'], []) - mt = getToolByName(self.portal, 'portal_membership') - member = mt.getMemberById('Jim') - member.fullname = 'Jimmy Jones' # Create a comment From 68d195375ef37c6c275cb9e99f9a9f231bf7c4e1 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Sat, 15 Dec 2012 16:47:01 -0800 Subject: [PATCH 4/4] use member.getProperty('fullname') if member.fullname is an empty string. Not sure in what case this would happen, but it did happen in my site migration --- plone/app/discussion/browser/migration.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/plone/app/discussion/browser/migration.py b/plone/app/discussion/browser/migration.py index 30d285b..fa806d0 100644 --- a/plone/app/discussion/browser/migration.py +++ b/plone/app/discussion/browser/migration.py @@ -96,18 +96,20 @@ class View(BrowserView): comment.text = reply.cooked_text comment.mime_type = 'text/html' comment.creator = reply.Creator() - comment.author_username = reply.getProperty('author_username',reply.Creator()) + + try: + comment.author_username = reply.author_username + except AttributeError: + comment.author_username = reply.Creator() + member = mt.getMemberById(comment.author_username) if member: -# comment.author_name = member.get('fullname') comment.author_name = member.fullname -# import pdb; pdb.set_trace() - -# .get is overloaded into zope somewhere.. so it'snot getting reply.email -## email = reply.get('email') -## if email: -## comment.author_email = email + if not comment.author_name: + # In migrated site member.fullname ='' while member.getProperty('fullname') has the correct value + comment.author_name = member.getProperty('fullname') + try: comment.author_email = reply.email except AttributeError: