From f67c7cde6d2eee9955a451fa40096da9ba65bfc3 Mon Sep 17 00:00:00 2001 From: Timo Stollenwerk Date: Wed, 29 Sep 2010 07:56:36 +0000 Subject: [PATCH] Remove title field from the comment form and replace it with an auto-generated title ("John Doe on Welcome to Plone"). This also fixes http://dev.plone.org/plone/ticket/11098 svn path=/plone.app.discussion/trunk/; revision=40431 --- CHANGES.txt | 8 +++ plone/app/discussion/browser/comments.py | 21 ++++-- plone/app/discussion/catalog.py | 4 +- plone/app/discussion/comment.py | 31 +++++++-- plone/app/discussion/tests/test_catalog.py | 32 ++++----- plone/app/discussion/tests/test_comment.py | 21 +++--- .../discussion/tests/test_comments_viewlet.py | 68 ++----------------- plone/app/discussion/tests/test_indexers.py | 20 +++--- plone/app/discussion/tests/test_migration.py | 7 +- .../tests/test_regression_pickle.py | 1 - plone/app/discussion/tests/test_tool.py | 8 ++- 11 files changed, 100 insertions(+), 121 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 848ec6f..2a097c6 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -4,6 +4,14 @@ Changelog 1.0b8 (unreleased) ------------------ +- Remove title field from the comment form and replace it with an auto-generated + title ("John Doe on Welcome to Plone"). + [timo] + +- Fix http://dev.plone.org/plone/ticket/11098: "Comment byline shows login + name, not full name" + [kiorky] + - Make sure the __parent__ pointer (the conversation) of a comment is not acquisition wrapped in conversation.addComment. This fixes http://dev.plone.org/plone/ticket/11157. diff --git a/plone/app/discussion/browser/comments.py b/plone/app/discussion/browser/comments.py index 518e0c7..2df2b66 100644 --- a/plone/app/discussion/browser/comments.py +++ b/plone/app/discussion/browser/comments.py @@ -43,6 +43,7 @@ try: except ImportError: # pragma: no cover HAS_WRAPPED_FORM = False + class CommentForm(extensible.ExtensibleForm, form.Form): ignoreContext = True # don't use context to get widget data @@ -55,7 +56,8 @@ class CommentForm(extensible.ExtensibleForm, form.Form): 'creator', 'creation_date', 'modification_date', - 'author_username') + 'author_username', + 'title') def updateFields(self): super(CommentForm, self).updateFields() @@ -160,17 +162,24 @@ class CommentForm(extensible.ExtensibleForm, form.Form): if portal_membership.isAnonymousUser() and \ settings.anonymous_comments: - comment.creator = None + # Anonymous Users + comment.creator = author_name comment.author_name = author_name comment.author_email = author_email #comment.author_notification = author_notification comment.creation_date = comment.modification_date = datetime.utcnow() elif not portal_membership.isAnonymousUser(): + # Member member = portal_membership.getAuthenticatedMember() - comment.creator = member.id - comment.author_username = member.getUserName() - comment.author_name = member.getProperty('fullname') - comment.author_email = member.getProperty('email') + username = member.getUserName() + email = member.getProperty('email') + fullname = member.getProperty('fullname') + if not fullname or fullname == '': + fullname = member.getUserName() + comment.creator = fullname + comment.author_username = username + comment.author_name = fullname + comment.author_email = email #comment.author_notification = comment.author_notification comment.creation_date = comment.modification_date = datetime.utcnow() else: diff --git a/plone/app/discussion/catalog.py b/plone/app/discussion/catalog.py index 4eb9a4a..cd708d1 100644 --- a/plone/app/discussion/catalog.py +++ b/plone/app/discussion/catalog.py @@ -63,7 +63,7 @@ def commentators(object): @indexer(IComment) def title(object): - return object.title + return object.Title() @indexer(IComment) def creator(object): @@ -79,7 +79,7 @@ def description(object): @indexer(IComment) def searchable_text(object): - return object.title, object.text + return object.text @indexer(IComment) def in_response_to(object): diff --git a/plone/app/discussion/comment.py b/plone/app/discussion/comment.py index 7b54fe1..5cacdc2 100644 --- a/plone/app/discussion/comment.py +++ b/plone/app/discussion/comment.py @@ -11,7 +11,7 @@ from zope.i18n import translate from zope.i18nmessageid import Message from zope.interface import implements -from Acquisition import aq_parent, Implicit +from Acquisition import aq_parent, aq_base, Implicit from string import Template @@ -46,6 +46,9 @@ except: from OFS.Traversable import Traversable as WorkflowAware PLONE_4 = False +COMMENT_TITLE = _(u"comment_title", + default=u"${creator} on ${content}") + MAIL_NOTIFICATION_MESSAGE = _(u"mail_notification_message", default=u"A comment with the title '${title}' " "has been posted here: ${link}") @@ -103,26 +106,40 @@ class Comment(CatalogAware, WorkflowAware, DynamicType, Traversable, return self.comment_id and str(self.comment_id) or None def getId(self): - """The id of the comment, as a string + """The id of the comment, as a string. """ return self.id def getText(self): - '''the text''' + """The body text of a comment. + """ return self.text def Title(self): - """The title of the comment + """The title of the comment. """ - return self.title + if not self.creator: + anonymous = _(u"label_anonymous", + default=u"Anonymous") + + # Fetch the content object (the parent of the comment is the + # conversation, the parent of the conversation is the content object). + content = aq_base(self.__parent__.__parent__) + + title = translate( + Message(COMMENT_TITLE, + mapping={'creator': self.creator, + 'content': content.Title()})) + return title + def Creator(self): - """The name of the person who wrote the comment + """The name of the person who wrote the comment. """ return self.creator def Type(self): - """The Discussion Item content type + """The Discussion Item content type. """ return self.fti_title diff --git a/plone/app/discussion/tests/test_catalog.py b/plone/app/discussion/tests/test_catalog.py index 7ef57fe..23c199e 100644 --- a/plone/app/discussion/tests/test_catalog.py +++ b/plone/app/discussion/tests/test_catalog.py @@ -44,8 +44,9 @@ class ConversationCatalogTest(PloneTestCase): def afterSetUp(self): # First we need to create some content. self.loginAsPortalOwner() - typetool = self.portal.portal_types - typetool.constructContent('Document', self.portal, 'doc1') + self.portal.invokeFactory(id='doc1', + Title='Document 1', + type_name='Document') self.catalog = getToolByName(self.portal, 'portal_catalog') @@ -204,35 +205,33 @@ class CommentCatalogTest(PloneTestCase): layer = DiscussionLayer def afterSetUp(self): - # First we need to create some content. + """Create a document with a comment. + """ self.loginAsPortalOwner() - self.typetool = typetool = self.portal.portal_types - typetool.constructContent('Document', self.portal, 'doc1') - + self.portal.invokeFactory(id='doc1', + title='Document 1', + type_name='Document') self.catalog = getToolByName(self.portal, 'portal_catalog') conversation = IConversation(self.portal.doc1) - self.conversation = conversation comment1 = createObject('plone.Comment') - comment1.title = 'Comment 1' comment1.text = 'Comment text' comment1.creator = 'Jim' - new_comment1_id = conversation.addComment(comment1) self.comment_id = new_comment1_id + # Comment brain self.comment = self.portal.doc1.restrictedTraverse( '++conversation++default/%s' % new_comment1_id) - brains = self.catalog.searchResults( path = {'query' : '/'.join(self.comment.getPhysicalPath()) }) self.comment_brain = brains[0] def test_title(self): - self.assertEquals(self.comment_brain.Title, 'Comment 1') + self.assertEquals(self.comment_brain.Title, 'Jim on Document 1') def test_type(self): self.assertEquals(self.comment_brain.portal_type, 'Discussion Item') @@ -246,9 +245,10 @@ class CommentCatalogTest(PloneTestCase): self.assertEquals(self.comment_brain.Creator, 'Jim') def test_in_response_to(self): - # make sure in_response_to returns the title or id of the content - # object the comment was added to - self.assertEquals(self.comment_brain.in_response_to, 'doc1') + """Make sure in_response_to returns the title or id of the content + object the comment was added to. + """ + self.assertEquals(self.comment_brain.in_response_to, 'Document 1') def test_add_comment(self): self.failUnless(self.comment_brain) @@ -275,7 +275,7 @@ class CommentCatalogTest(PloneTestCase): brains = self.catalog.searchResults(portal_type = 'Discussion Item') self.failUnless(brains) comment_brain = brains[0] - self.assertEquals(comment_brain.Title, 'Comment 1') + self.assertEquals(comment_brain.Title, u'Jim on Document 1') def test_clear_and_rebuild_catalog_for_nested_comments(self): @@ -334,7 +334,7 @@ class CommentCatalogTest(PloneTestCase): self.assertEquals(len(brains), 6) def test_collection(self): - self.typetool.constructContent('Topic', self.portal, 'topic') + self.portal.invokeFactory(id='topic', type_name='Topic') topic = self.portal.topic crit = topic.addCriterion('Type', 'ATSimpleStringCriterion') crit.setValue('Comment') diff --git a/plone/app/discussion/tests/test_comment.py b/plone/app/discussion/tests/test_comment.py index 89b1a44..bc1a49d 100644 --- a/plone/app/discussion/tests/test_comment.py +++ b/plone/app/discussion/tests/test_comment.py @@ -25,10 +25,12 @@ class CommentTest(PloneTestCase): layer = DiscussionLayer def afterSetUp(self): - # First we need to create some content. + """Create a document. + """ self.loginAsPortalOwner() - typetool = self.portal.portal_types - typetool.constructContent('Document', self.portal, 'doc1') + self.portal.invokeFactory(id='doc1', + title='Document 1', + type_name='Document') def test_factory(self): comment1 = createObject('plone.Comment') @@ -58,10 +60,12 @@ class CommentTest(PloneTestCase): self.assertEquals(u'123', comment1.__name__) def test_title(self): + conversation = IConversation(self.portal.doc1) comment1 = createObject('plone.Comment') - comment1.title = "New title" - self.assertEquals("New title", comment1.Title()) - + comment1.creator = "Jim Fulton" + conversation.addComment(comment1) + self.assertEquals("Jim Fulton on Document 1", comment1.Title()) + def test_creator(self): comment1 = createObject('plone.Comment') comment1.creator = "Jim" @@ -166,8 +170,9 @@ class RepliesTest(PloneTestCase): def afterSetUp(self): # First we need to create some content. self.loginAsPortalOwner() - typetool = self.portal.portal_types - typetool.constructContent('Document', self.portal, 'doc1') + self.portal.invokeFactory(id='doc1', + title='Document 1', + type_name='Document') def test_add_comment(self): # Add comments to a CommentReplies adapter diff --git a/plone/app/discussion/tests/test_comments_viewlet.py b/plone/app/discussion/tests/test_comments_viewlet.py index f9dd4f8..4788001 100644 --- a/plone/app/discussion/tests/test_comments_viewlet.py +++ b/plone/app/discussion/tests/test_comments_viewlet.py @@ -72,19 +72,8 @@ class TestCommentForm(PloneTestCase): factory=CommentForm, name=u"comment-form") - # The form should return errors if the two required fields are empty - request = make_request(form={}) - - commentForm = getMultiAdapter((self.context, request), - name=u"comment-form") - commentForm.update() - data, errors = commentForm.extractData() - - self.assertEquals(len(errors), 2) - self.failIf(commentForm.handleComment(commentForm, "foo")) - # The form should return an error if the comment text field is empty - request = make_request(form={'form.widgets.text': u'foo'}) + request = make_request(form={}) commentForm = getMultiAdapter((self.context, request), name=u"comment-form") @@ -93,11 +82,10 @@ class TestCommentForm(PloneTestCase): self.assertEquals(len(errors), 1) self.failIf(commentForm.handleComment(commentForm, "foo")) - - # The form is submitted successfully, if all required fields are + + # The form is submitted successfully, if the required text field is # filled out - request = make_request(form={'form.widgets.title': u'foo', - 'form.widgets.text': u'bar'}) + request = make_request(form={'form.widgets.text': u'bar'}) commentForm = getMultiAdapter((self.context, request), name=u"comment-form") @@ -177,53 +165,7 @@ class TestCommentForm(PloneTestCase): commentForm, "foo") - -class TestCommentsViewletIntegration(FunctionalTestCase): - - layer = DiscussionLayer - - def testCommentsViewlet(self): - # Fetch testbrowser - browser = Browser() - portal_url = self.portal.absolute_url() - browser.handleErrors = False - - # Login - browser.open(portal_url + '/login_form') - browser.getControl(name='__ac_name').value = portal_owner - browser.getControl(name='__ac_password').value = default_password - browser.getControl(name='submit').click() - - # Create page with comments allowed - browser.open(portal_url) - browser.getLink(id='document').click() - browser.getControl(name='title').value = "Doc1" - browser.getControl(name='allowDiscussion:boolean').value = True - browser.getControl(name='form.button.save').click() - - # Check that the form has been properly submitted - self.assertEquals(browser.url, 'http://nohost/plone/doc1') - - # Check that the old comments viewlet does not show up - self.failIf('discussion_reply_form' in browser.contents) - - # Check that the comment form/viewlet shows up - self.failUnless('formfield-form-widgets-in_reply_to' in - browser.contents) - self.failUnless('formfield-form-widgets-title' in browser.contents) - self.failUnless('formfield-form-widgets-text' in browser.contents) - - # Submit the comment form - browser.getControl(name='form.widgets.title').value = "My Comment" - browser.getControl(name='form.widgets.text').value = "Lorem ipsum" - submit = browser.getControl(name='form.buttons.comment') - submit.click() - - # Check that the comment has been posted - self.failUnless("My Comment" in browser.contents) - self.failUnless("Lorem ipsum" in browser.contents) - - + class TestCommentsViewlet(PloneTestCase): layer = DiscussionLayer diff --git a/plone/app/discussion/tests/test_indexers.py b/plone/app/discussion/tests/test_indexers.py index 956d3f3..0f8b9c5 100644 --- a/plone/app/discussion/tests/test_indexers.py +++ b/plone/app/discussion/tests/test_indexers.py @@ -37,14 +37,14 @@ class ConversationIndexersTest(PloneTestCase): def afterSetUp(self): # First we need to create some content. self.loginAsPortalOwner() - typetool = self.portal.portal_types - typetool.constructContent('Document', self.portal, 'doc1') + self.portal.invokeFactory(id='doc1', + title='Document 1', + type_name='Document') # Create a conversation. conversation = IConversation(self.portal.doc1) comment1 = createObject('plone.Comment') - comment1.title = 'Comment 1' comment1.text = 'Comment Text' comment1.creator = "Jim" comment1.author_username = "Jim" @@ -53,7 +53,6 @@ class ConversationIndexersTest(PloneTestCase): self.new_id1 = conversation.addComment(comment1) comment2 = createObject('plone.Comment') - comment2.title = 'Comment 2' comment2.text = 'Comment Text' comment2.creator = "Emma" comment2.author_username = "Emma" @@ -62,7 +61,6 @@ class ConversationIndexersTest(PloneTestCase): self.new_id2 = conversation.addComment(comment2) comment3 = createObject('plone.Comment') - comment3.title = 'Comment 3' comment3.text = 'Comment Text' comment3.creator = "Lukas" comment3.author_username = "Lukas" @@ -108,8 +106,9 @@ class CommentIndexersTest(PloneTestCase): def afterSetUp(self): # First we need to create some content. self.loginAsPortalOwner() - typetool = self.portal.portal_types - typetool.constructContent('Document', self.portal, 'doc1') + self.portal.invokeFactory(id='doc1', + title='Document 1', + type_name='Document') # Create a conversation. In this case we doesn't assign it to an # object, as we just want to check the Conversation object API. @@ -119,7 +118,6 @@ class CommentIndexersTest(PloneTestCase): # factory to allow different factories to be swapped in comment = createObject('plone.Comment') - comment.title = 'Comment 1' comment.text = 'Lorem ipsum dolor sit amet.' comment.creator = "Jim" comment.creation_date = datetime(2006, 9, 17, 14, 18, 12) @@ -130,7 +128,7 @@ class CommentIndexersTest(PloneTestCase): self.conversation = conversation def test_title(self): - self.assertEquals(catalog.title(self.comment)(), 'Comment 1') + self.assertEquals(catalog.title(self.comment)(), 'Jim on Document 1') self.assert_(isinstance(catalog.title, DelegatingIndexerFactory)) def test_description(self): @@ -159,7 +157,7 @@ class CommentIndexersTest(PloneTestCase): def test_searchable_text(self): # Test if searchable text is a concatenation of title and comment text self.assertEquals(catalog.searchable_text(self.comment)(), - ('Comment 1', 'Lorem ipsum dolor sit amet.')) + ('Lorem ipsum dolor sit amet.')) self.assert_(isinstance(catalog.searchable_text, DelegatingIndexerFactory)) @@ -169,8 +167,6 @@ class CommentIndexersTest(PloneTestCase): def test_in_response_to(self): # make sure in_response_to returns the title or id of the content # object the comment was added to - self.assertEquals(catalog.in_response_to(self.comment)(), 'doc1') - self.portal.doc1.title = 'Document 1' self.assertEquals(catalog.in_response_to(self.comment)(), 'Document 1') def test_suite(): diff --git a/plone/app/discussion/tests/test_migration.py b/plone/app/discussion/tests/test_migration.py index fb43244..db9e953 100644 --- a/plone/app/discussion/tests/test_migration.py +++ b/plone/app/discussion/tests/test_migration.py @@ -21,8 +21,9 @@ class MigrationTest(PloneTestCase): def afterSetUp(self): self.loginAsPortalOwner() - typetool = self.portal.portal_types - typetool.constructContent('Document', self.portal, 'doc') + self.portal.invokeFactory(id='doc', + title='Document 1', + type_name='Document') # Create a document self.discussion = getToolByName(self.portal, 'portal_discussion', None) self.discussion.overrideDiscussionFor(self.portal.doc, 1) @@ -67,7 +68,7 @@ class MigrationTest(PloneTestCase): self.failUnless(conversation.getComments().next()) comment1 = conversation.values()[0] self.assert_(IComment.providedBy(comment1)) - self.assertEquals(comment1.Title(), 'My Title') + self.assertEquals(comment1.Title(), 'Jim on Document 1') self.assertEquals(comment1.text, 'My Text') self.assertEquals(comment1.Creator(), 'Jim') self.assertEquals(comment1.creation_date, diff --git a/plone/app/discussion/tests/test_regression_pickle.py b/plone/app/discussion/tests/test_regression_pickle.py index 35188b3..00f09db 100644 --- a/plone/app/discussion/tests/test_regression_pickle.py +++ b/plone/app/discussion/tests/test_regression_pickle.py @@ -74,7 +74,6 @@ class TestPostCommentsRegression(FunctionalTestCase): browser.getControl(name='__ac_password').value = password browser.getControl(name='submit').click() browser.open(url) - browser.getControl(name='form.widgets.title').value = "%s My Comment" % poster browser.getControl(name='form.widgets.text').value = "%s Lorem ipsum" % poster submit = browser.getControl(name='form.buttons.comment') submit.click() diff --git a/plone/app/discussion/tests/test_tool.py b/plone/app/discussion/tests/test_tool.py index 2c96c8d..7737854 100644 --- a/plone/app/discussion/tests/test_tool.py +++ b/plone/app/discussion/tests/test_tool.py @@ -14,8 +14,9 @@ class ToolTest(PloneTestCase): def afterSetUp(self): # First we need to create some content. self.loginAsPortalOwner() - typetool = self.portal.portal_types - typetool.constructContent('Document', self.portal, 'doc1') + self.portal.invokeFactory(id='doc1', + title='Document 1', + type_name='Document') def test_tool_indexing(self): # Create a conversation. In this case we doesn't assign it to an @@ -25,6 +26,7 @@ class ToolTest(PloneTestCase): # Add a comment. comment = createObject('plone.Comment') comment.title = 'Comment 1' + comment.creator = 'Jim' comment.text = 'Comment text' conversation.addComment(comment) @@ -34,7 +36,7 @@ class ToolTest(PloneTestCase): comment = list(tool.searchResults()) self.assert_(len(comment) == 1, "There is only one comment, but we got" " %s results in the search" % len(comment)) - self.assertEquals(comment[0].Title, 'Comment 1') + self.assertEquals(comment[0].Title, 'Jim on Document 1') def test_unindexing(self): pass