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

@@ -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,7 +90,6 @@ class View(BrowserView):
 
                 new_in_reply_to = None
                 if should_migrate:
-
                     # create a reply object
                     comment = CommentFactory()
                     comment.title = reply.Title()
@@ -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)

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, '<p>My Text</p>\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
         #