Fixing crash on visualizing forum with comments from unauthenticated users

(ActionItem1822)

Fixing crash on visualizing forum with comments from unauthenticated users
(ActionItem1822)
Joenio Costa · Daniela Feitosa
1 parent cc6d42c3
Showing 11 changed files with 104 additions and 27 deletions Show diff stats
app/helpers/forum_helper.rb
app/models/article.rb
app/models/comment.rb
features/forum.feature
features/step_definitions/custom_webrat_steps.rb
features/step_definitions/noosfero_steps.rb
test/test_helper.rb
test/unit/article_block_test.rb
test/unit/article_test.rb
test/unit/comment_test.rb
test/unit/forum_helper_test.rb
@@ -40,8 +40,12 @@ module ForumHelper
   end
   def last_topic_update(article)
-    last_update_at, last_update_by = (article.comments.count.zero? ? [article.updated_at, article.author] : [article.comments.last.created_at, article.comments.last.author])
-    time_ago_as_sentence(last_update_at) + ' ' + _('ago') + ' ' + _('by') + ' ' + link_to(last_update_by.name, last_update_by.url)
+    info = article.info_from_last_update
+    if info[:author_url]
+      time_ago_as_sentence(info[:date]) + ' ' + _('ago') + ' ' + _('by') + ' ' + link_to(info[:author_name], info[:author_url])
+    else
+      time_ago_as_sentence(info[:date]) + ' ' + _('ago') + ' ' + _('by') + ' ' + info[:author_name]
+    end
   end
 end
@@ -232,6 +232,15 @@ class Article &lt; ActiveRecord::Base
     self.parent and self.parent.blog?
   end
+  def info_from_last_update
+    last_comment = comments.last
+    if last_comment
+      {:date => last_comment.created_at, :author_name => last_comment.author_name, :author_url => last_comment.author_url}
+    else
+      {:date => updated_at, :author_name => author.name, :author_url => author.url}
+    end
+  end
+
   def url
     @url ||= self.profile.url.merge(:page => path.split('/'))
   end
@@ -43,6 +43,10 @@ class Comment &lt; ActiveRecord::Base
     author ? author.url : email
   end
+  def author_url
+    author ? author.url : nil
+  end
+
   def url
     article.view_url.merge(:anchor => anchor)
   end
@@ -67,3 +67,29 @@ Feature: forum
     And I go to /joaosilva/forum-one
     When I follow "Configure forum"
     Then I should be on edit "Forum One" by joaosilva
+
+  Scenario: last topic update by unautenticated user should not link
+    Given the following forums
+       | owner     | name |
+       | joaosilva | Forum |
+    And the following articles
+       | owner     | name | parent |
+       | joaosilva | Post one | Forum |
+    And the following comments
+       | article | name | email | title | body |
+       | Post one | Joao | joao@example.com | Hi all | Hi all |
+   When I go to /joaosilva/forum
+   Then I should not see "Joao" link
+
+  Scenario: last topic update by autenticated user should link to profile url
+    Given the following forums
+       | owner     | name |
+       | joaosilva | Forum |
+    And the following articles
+       | owner     | name | parent |
+       | joaosilva | Post one | Forum |
+    And the following comments
+       | article | author | title | body |
+       | Post one | joaosilva | Hi all | Hi all |
+   When I go to /joaosilva/forum
+   Then I should see "Joao" linking to "http:///joaosilva"
@@ -6,6 +6,11 @@ When /^I should not see &quot;([^\&quot;]+)&quot; link$/ do |text|
   response.should_not have_selector("a:contains('#{text}')")
 end
+When /^I should see "([^\"]+)" linking to "([^\"]+)"$/ do |text, href|
+  response.should have_selector("a:contains('#{text}')")
+  response.should have_selector("a[href='#{href}']")
+end
+
 Then /^I should be exactly on (.+)$/ do |page_name|
   URI.parse(current_url).request_uri.should == path_to(page_name)
 end
@@ -52,9 +52,14 @@ Given /^the following (articles|events|blogs|folders|forums|galleries)$/ do |con
   }[content] || raise("Don't know how to build %s" % content)
   table.hashes.map{|item| item.dup}.each do |item|
     owner_identifier = item.delete("owner")
+    parent = item.delete("parent")
     owner = Profile[owner_identifier]
     home = item.delete("homepage")
-    result = klass.create!(item.merge(:profile => owner))
+    result = klass.new(item.merge(:profile => owner))
+    if parent
+      result.parent = Article.find_by_name(parent)
+    end
+    result.save!
     if home
       owner.home_page = result
       owner.save!
@@ -318,8 +323,11 @@ Given /^the following comments?$/ do |table|
   table.hashes.each do |item|
     data = item.dup
     article = Article.find_by_name(data.delete("article"))
-    author = Profile[data.delete("author")]
-    comment = article.comments.build(:author => author, :title => data.delete("title"), :body => data.delete("body"))
+    author = data.delete("author")
+    comment = article.comments.build(data)
+    if author
+      comment.author = Profile[author]
+    end
     comment.save!
   end
 end
@@ -193,7 +193,8 @@ module NoosferoTestHelper
   end
   def content_tag(tag, content, options = {})
-    "<#{tag}>#{content}</#{tag}>"
+    tag_attr = options.blank? ? '' : ' ' + options.collect{ |o| "#{o[0]}=\"#{o[1]}\"" }.join(' ')
+    "<#{tag}#{tag_attr}>#{content}</#{tag}>"
   end
   def submit_tag(content, options = {})
@@ -228,6 +229,8 @@ module NoosferoTestHelper
     icon
   end
+  def will_paginate(arg1, arg2)
+  end
 end
 class ActionController::IntegrationTest
@@ -89,7 +89,7 @@ class ArticleBlockTest &lt; Test::Unit::TestCase
     block.expects(:title).returns('')
     block.stubs(:article).returns(article)
-    assert_equal "<h3><span></span></h3>Article content", instance_eval(&block.content)
+    assert_equal "<h3 class=\"block-title empty\"><span></span></h3>Article content", instance_eval(&block.content)
   end
   should "display title if defined" do
@@ -99,7 +99,7 @@ class ArticleBlockTest &lt; Test::Unit::TestCase
     block.expects(:title).returns('Article title')
     block.stubs(:article).returns(article)
-    assert_equal "<h3><span>Article title</span></h3>Article content", instance_eval(&block.content)
+    assert_equal "<h3 class=\"block-title\"><span>Article title</span></h3>Article content", instance_eval(&block.content)
   end
   should 'display image if article is an image' do
@@ -1423,4 +1423,21 @@ class ArticleTest &lt; Test::Unit::TestCase
     assert_equal 'some name', a.author_name
   end
+  should 'retrieve latest info from topic when has no comments' do
+    forum = fast_create(Forum, :name => 'Forum test', :profile_id => profile.id)
+    post = fast_create(TextileArticle, :name => 'First post', :profile_id => profile.id, :parent_id => forum.id, :updated_at => Time.now)
+    assert_equal post.updated_at, post.info_from_last_update[:date]
+    assert_equal profile.name, post.info_from_last_update[:author_name]
+    assert_equal profile.url, post.info_from_last_update[:author_url]
+  end
+
+  should 'retrieve latest info from comment when has comments' do
+    forum = fast_create(Forum, :name => 'Forum test', :profile_id => profile.id)
+    post = fast_create(TextileArticle, :name => 'First post', :profile_id => profile.id, :parent_id => forum.id, :updated_at => Time.now)
+    post.comments << Comment.new(:name => 'Guest', :email => 'guest@example.com', :title => 'test comment', :body => 'hello!')
+    assert_equal post.comments.last.created_at, post.info_from_last_update[:date]
+    assert_equal "Guest", post.info_from_last_update[:author_name]
+    assert_nil post.info_from_last_update[:author_url]
+  end
+
 end
@@ -317,4 +317,14 @@ class CommentTest &lt; Test::Unit::TestCase
     assert result[1].replies.empty?
   end
+  should 'provide author url for authenticated user' do
+    author = Person.new
+    author.expects(:url).returns('http://blabla.net/author')
+    assert_equal  'http://blabla.net/author', Comment.new(:author => author).author_url
+  end
+
+  should 'not provide author url for unauthenticated user' do
+    assert_nil Comment.new(:email => 'my@email.com').author_url
+  end
+
 end
@@ -9,7 +9,6 @@ class ForumHelperTest &lt; Test::Unit::TestCase
   include ApplicationHelper
   def setup
-    stubs(:show_date).returns('')
     @environment = Environment.default
     @profile = create_user('forum_helper_test').person
     @forum = fast_create(Forum, :profile_id => profile.id, :name => 'Forum test')
@@ -39,9 +38,9 @@ class ForumHelperTest &lt; Test::Unit::TestCase
   should 'return post update if it has no comments' do
     author = create_user('forum test author').person
     some_post = TextileArticle.create!(:name => 'First post', :profile => profile, :parent => forum, :published => true)
-    some_post.expects(:author).returns(author)
+    some_post.expects(:author).returns(author).times(2)
     assert some_post.comments.empty?
-    assert_equal "#{some_post.updated_at.to_s} ago by #{author.name}", last_topic_update(some_post)
+    assert_match /#{some_post.updated_at.to_s} ago by <a href='[^']+'>forum test author<\/a>/, last_topic_update(some_post)
   end
   should 'return last comment date if it has comments' do
@@ -51,27 +50,19 @@ class ForumHelperTest &lt; Test::Unit::TestCase
     some_post.comments << Comment.new(:title => 'test', :body => 'test', :author => a2)
     c = Comment.last
     assert_equal 2, some_post.comments.count
-    assert_equal "#{c.created_at.to_s} ago by #{a2.name}", last_topic_update(some_post)
+    assert_match /#{c.created_at.to_s} ago by <a href='[^']+'>a2<\/a>/, last_topic_update(some_post)
   end
-  protected
-
-  def will_paginate(arg1, arg2)
-  end
-
-  def link_to(content, url)
-    content
+  should "return last comment author's name from unauthenticated user" do
+    some_post = TextileArticle.create!(:name => 'First post', :profile => profile, :parent => forum, :published => true)
+    some_post.comments << Comment.new(:name => 'John', :email => 'lenon@example.com', :title => 'test', :body => 'test')
+    c = Comment.last
+    assert_match /#{c.created_at.to_s} ago by John/m, last_topic_update(some_post)
   end
-  def tag(tag, args = {})
-    attrs = args.map{|k,v| "#{k}='#{v}'"}.join(' ')
-    "<#{tag} #{attrs} />"
-  end
+  protected
-  def content_tag(tag, content, options = {})
-    tag_attr = options.blank? ? "" : options.collect{ |o| "#{o[0]}=\"#{o[1]}\"" }.join(' ')
-    "<#{tag}#{tag_attr}>#{content}</#{tag}>"
-  end
+  include NoosferoTestHelper
   def time_ago_as_sentence(t = Time.now)
     t.to_s
	@@ -232,6 +232,15 @@ class Article < ActiveRecord::Base		@@ -232,6 +232,15 @@ class Article < ActiveRecord::Base
232	self.parent and self.parent.blog?	232	self.parent and self.parent.blog?
233	end	233	end
234		234
		235	+ def info_from_last_update
		236	+ last_comment = comments.last
		237	+ if last_comment
		238	+ {:date => last_comment.created_at, :author_name => last_comment.author_name, :author_url => last_comment.author_url}
		239	+ else
		240	+ {:date => updated_at, :author_name => author.name, :author_url => author.url}
		241	+ end
		242	+ end
		243	+
235	def url	244	def url
236	@url \|\|= self.profile.url.merge(:page => path.split('/'))	245	@url \|\|= self.profile.url.merge(:page => path.split('/'))
237	end	246	end
	@@ -67,3 +67,29 @@ Feature: forum		@@ -67,3 +67,29 @@ Feature: forum
67	And I go to /joaosilva/forum-one	67	And I go to /joaosilva/forum-one
68	When I follow "Configure forum"	68	When I follow "Configure forum"
69	Then I should be on edit "Forum One" by joaosilva	69	Then I should be on edit "Forum One" by joaosilva
		70	+
		71	+ Scenario: last topic update by unautenticated user should not link
		72	+ Given the following forums
		73	+ \| owner \| name \|
		74	+ \| joaosilva \| Forum \|
		75	+ And the following articles
		76	+ \| owner \| name \| parent \|
		77	+ \| joaosilva \| Post one \| Forum \|
		78	+ And the following comments
		79	+ \| article \| name \| email \| title \| body \|
		80	+ \| Post one \| Joao \| joao@example.com \| Hi all \| Hi all \|
		81	+ When I go to /joaosilva/forum
		82	+ Then I should not see "Joao" link
		83	+
		84	+ Scenario: last topic update by autenticated user should link to profile url
		85	+ Given the following forums
		86	+ \| owner \| name \|
		87	+ \| joaosilva \| Forum \|
		88	+ And the following articles
		89	+ \| owner \| name \| parent \|
		90	+ \| joaosilva \| Post one \| Forum \|
		91	+ And the following comments
		92	+ \| article \| author \| title \| body \|
		93	+ \| Post one \| joaosilva \| Hi all \| Hi all \|
		94	+ When I go to /joaosilva/forum
		95	+ Then I should see "Joao" linking to "http:///joaosilva"
	@@ -89,7 +89,7 @@ class ArticleBlockTest < Test::Unit::TestCase		@@ -89,7 +89,7 @@ class ArticleBlockTest < Test::Unit::TestCase
89	block.expects(:title).returns('')	89	block.expects(:title).returns('')
90	block.stubs(:article).returns(article)	90	block.stubs(:article).returns(article)
91		91
92	- assert_equal "<h3><span></span></h3>Article content", instance_eval(&block.content)	92	+ assert_equal "<h3 class=\"block-title empty\"><span></span></h3>Article content", instance_eval(&block.content)
93	end	93	end
94		94
95	should "display title if defined" do	95	should "display title if defined" do
	@@ -99,7 +99,7 @@ class ArticleBlockTest < Test::Unit::TestCase		@@ -99,7 +99,7 @@ class ArticleBlockTest < Test::Unit::TestCase
99	block.expects(:title).returns('Article title')	99	block.expects(:title).returns('Article title')
100	block.stubs(:article).returns(article)	100	block.stubs(:article).returns(article)
101		101
102	- assert_equal "<h3><span>Article title</span></h3>Article content", instance_eval(&block.content)	102	+ assert_equal "<h3 class=\"block-title\"><span>Article title</span></h3>Article content", instance_eval(&block.content)
103	end	103	end
104		104
105	should 'display image if article is an image' do	105	should 'display image if article is an image' do
	@@ -1423,4 +1423,21 @@ class ArticleTest < Test::Unit::TestCase		@@ -1423,4 +1423,21 @@ class ArticleTest < Test::Unit::TestCase
1423	assert_equal 'some name', a.author_name	1423	assert_equal 'some name', a.author_name
1424	end	1424	end
1425		1425
		1426	+ should 'retrieve latest info from topic when has no comments' do
		1427	+ forum = fast_create(Forum, :name => 'Forum test', :profile_id => profile.id)
		1428	+ post = fast_create(TextileArticle, :name => 'First post', :profile_id => profile.id, :parent_id => forum.id, :updated_at => Time.now)
		1429	+ assert_equal post.updated_at, post.info_from_last_update[:date]
		1430	+ assert_equal profile.name, post.info_from_last_update[:author_name]
		1431	+ assert_equal profile.url, post.info_from_last_update[:author_url]
		1432	+ end
		1433	+
		1434	+ should 'retrieve latest info from comment when has comments' do
		1435	+ forum = fast_create(Forum, :name => 'Forum test', :profile_id => profile.id)
		1436	+ post = fast_create(TextileArticle, :name => 'First post', :profile_id => profile.id, :parent_id => forum.id, :updated_at => Time.now)
		1437	+ post.comments << Comment.new(:name => 'Guest', :email => 'guest@example.com', :title => 'test comment', :body => 'hello!')
		1438	+ assert_equal post.comments.last.created_at, post.info_from_last_update[:date]
		1439	+ assert_equal "Guest", post.info_from_last_update[:author_name]
		1440	+ assert_nil post.info_from_last_update[:author_url]
		1441	+ end
		1442	+
1426	end	1443	end
	@@ -317,4 +317,14 @@ class CommentTest < Test::Unit::TestCase		@@ -317,4 +317,14 @@ class CommentTest < Test::Unit::TestCase
317	assert result[1].replies.empty?	317	assert result[1].replies.empty?
318	end	318	end
319		319
		320	+ should 'provide author url for authenticated user' do
		321	+ author = Person.new
		322	+ author.expects(:url).returns('http://blabla.net/author')
		323	+ assert_equal 'http://blabla.net/author', Comment.new(:author => author).author_url
		324	+ end
		325	+
		326	+ should 'not provide author url for unauthenticated user' do
		327	+ assert_nil Comment.new(:email => 'my@email.com').author_url
		328	+ end
		329	+
320	end	330	end