Commit d3d39461e6c60e3137f90ea9381371323ee76d29
Committed by
Antonio Terceiro
1 parent
66975041
Exists in
master
and in
29 other branches
More anti-spam measures
(ActionItem1361)
Showing
8 changed files
with
107 additions
and
81 deletions
Show diff stats
app/controllers/public/contact_controller.rb
@@ -7,7 +7,7 @@ class ContactController < PublicController | @@ -7,7 +7,7 @@ class ContactController < PublicController | ||
7 | inverse_captcha :field => 'e_mail' | 7 | inverse_captcha :field => 'e_mail' |
8 | def new | 8 | def new |
9 | @contact | 9 | @contact |
10 | - if request.post? && params[self.icaptcha_field].blank? | 10 | + if request.post? && params[self.icaptcha_field].blank? && params[:confirm] == 'true' |
11 | @contact = user.build_contact(profile, params[:contact]) | 11 | @contact = user.build_contact(profile, params[:contact]) |
12 | @contact.city = (!params[:city].blank? && City.exists?(params[:city])) ? City.find(params[:city]).name : nil | 12 | @contact.city = (!params[:city].blank? && City.exists?(params[:city])) ? City.find(params[:city]).name : nil |
13 | @contact.state = (!params[:state].blank? && State.exists?(params[:state])) ? State.find(params[:state]).name : nil | 13 | @contact.state = (!params[:state].blank? && State.exists?(params[:state])) ? State.find(params[:state]).name : nil |
app/controllers/public/content_viewer_controller.rb
@@ -77,7 +77,7 @@ class ContentViewerController < ApplicationController | @@ -77,7 +77,7 @@ class ContentViewerController < ApplicationController | ||
77 | 77 | ||
78 | @form_div = params[:form] | 78 | @form_div = params[:form] |
79 | 79 | ||
80 | - if request.post? && params[:comment] && params[self.icaptcha_field].blank? && @page.accept_comments? | 80 | + if request.post? && params[:comment] && params[self.icaptcha_field].blank? && params[:confirm] == 'true' && @page.accept_comments? |
81 | add_comment | 81 | add_comment |
82 | end | 82 | end |
83 | 83 |
app/views/contact/new.rhtml
@@ -5,6 +5,7 @@ | @@ -5,6 +5,7 @@ | ||
5 | 5 | ||
6 | <% labelled_form_for :contact, @contact do |f| %> | 6 | <% labelled_form_for :contact, @contact do |f| %> |
7 | <%= icaptcha_field() %> | 7 | <%= icaptcha_field() %> |
8 | + <%= hidden_field_tag(:confirm, 'false') %> | ||
8 | 9 | ||
9 | <%= required_fields_message %> | 10 | <%= required_fields_message %> |
10 | 11 | ||
@@ -15,6 +16,5 @@ | @@ -15,6 +16,5 @@ | ||
15 | <%= required f.text_area(:message, :rows => 10, :cols => 60) %> | 16 | <%= required f.text_area(:message, :rows => 10, :cols => 60) %> |
16 | <%= labelled_form_field check_box(:contact, :receive_a_copy) + _('I want to receive a copy of the message in my e-mail.'), '' %> | 17 | <%= labelled_form_field check_box(:contact, :receive_a_copy) + _('I want to receive a copy of the message in my e-mail.'), '' %> |
17 | 18 | ||
18 | - <%= submit_button(:send, _('Send')) %> | ||
19 | - | 19 | + <%= submit_button(:send, _('Send'), :onclick => "$('confirm').value = 'true'") %> |
20 | <% end %> | 20 | <% end %> |
app/views/content_viewer/_comment_form.rhtml
@@ -17,8 +17,9 @@ | @@ -17,8 +17,9 @@ | ||
17 | 17 | ||
18 | <h4><%= content_tag('a', '', :name => 'comment_form') + _('Post a comment') %></h4> | 18 | <h4><%= content_tag('a', '', :name => 'comment_form') + _('Post a comment') %></h4> |
19 | 19 | ||
20 | -<% form_tag( @page.view_url, { :id => comment_form_id } ) do %> | 20 | +<% form_tag( url_for(@page.view_url.merge({:only_path => true})), { :id => comment_form_id } ) do %> |
21 | <%= icaptcha_field() %> | 21 | <%= icaptcha_field() %> |
22 | + <%= hidden_field_tag(:confirm, 'false') %> | ||
22 | 23 | ||
23 | <%= required_fields_message %> | 24 | <%= required_fields_message %> |
24 | 25 | ||
@@ -36,7 +37,7 @@ | @@ -36,7 +37,7 @@ | ||
36 | <%= required labelled_form_field(_('Title'), text_field(:comment, :title)) %> | 37 | <%= required labelled_form_field(_('Title'), text_field(:comment, :title)) %> |
37 | <%= required labelled_form_field(_('Enter your comment'), text_area(:comment, :body, :rows => 5)) %> | 38 | <%= required labelled_form_field(_('Enter your comment'), text_area(:comment, :body, :rows => 5)) %> |
38 | <% button_bar do %> | 39 | <% button_bar do %> |
39 | - <%= submit_button('add', _('Post comment')) %> | 40 | + <%= submit_button('add', _('Post comment'), :onclick => "$('confirm').value = 'true'") %> |
40 | <% end %> | 41 | <% end %> |
41 | <% end %> | 42 | <% end %> |
42 | 43 |
@@ -0,0 +1,61 @@ | @@ -0,0 +1,61 @@ | ||
1 | +Feature: comment | ||
2 | + As a visitor | ||
3 | + I want to post comments | ||
4 | + | ||
5 | + Background: | ||
6 | + Given the following users | ||
7 | + | login | | ||
8 | + | booking | | ||
9 | + And the following articles | ||
10 | + | owner | name | | ||
11 | + | booking | article to comment | | ||
12 | + | ||
13 | + Scenario: not post a comment without javascript | ||
14 | + Given I am on /booking/article-to-comment | ||
15 | + And I fill in "Name" with "Joey Ramone" | ||
16 | + And I fill in "e-Mail" with "joey@ramones.com" | ||
17 | + And I fill in "Title" with "Hey ho, let's go!" | ||
18 | + And I fill in "Enter your comment" with "Hey ho, let's go!" | ||
19 | + When I press "Post comment" | ||
20 | + Then I should not see "Hey ho, let's go" | ||
21 | + | ||
22 | + @selenium | ||
23 | + Scenario: post a comment while not authenticated | ||
24 | + Given I am on /booking/article-to-comment | ||
25 | + And I fill in "Name" with "Joey Ramone" | ||
26 | + And I fill in "e-Mail" with "joey@ramones.com" | ||
27 | + And I fill in "Title" with "Hey ho, let's go!" | ||
28 | + And I fill in "Enter your comment" with "Hey ho, let's go!" | ||
29 | + When I press "Post comment" | ||
30 | + Then I should see "Hey ho, let's go" | ||
31 | + | ||
32 | + @selenium | ||
33 | + Scenario: post comment while authenticated | ||
34 | + Given I am logged in as "booking" | ||
35 | + And I am on /booking/article-to-comment | ||
36 | + And I fill in "Title" with "Hey ho, let's go!" | ||
37 | + And I fill in "Enter your comment" with "Hey ho, let's go!" | ||
38 | + When I press "Post comment" | ||
39 | + Then I should see "Hey ho, let's go" | ||
40 | + | ||
41 | + @selenium | ||
42 | + Scenario: redirect to right place after comment a picture | ||
43 | + Given I am logged in as "booking" | ||
44 | + And the following files | ||
45 | + | owner | file | mime | | ||
46 | + | booking | rails.png | image/png | | ||
47 | + And I am on /booking/rails.png?view=true | ||
48 | + And I fill in "Title" with "Hey ho, let's go!" | ||
49 | + And I fill in "Enter your comment" with "Hey ho, let's go!" | ||
50 | + When I press "Post comment" | ||
51 | + And I wait 2 seconds | ||
52 | + Then I should be exactly on /booking/rails.png?view=true | ||
53 | + | ||
54 | + @selenium | ||
55 | + Scenario: show error messages when make a blank comment | ||
56 | + Given I am logged in as "booking" | ||
57 | + And I am on /booking/article-to-comment | ||
58 | + When I press "Post comment" | ||
59 | + And I wait 2 seconds | ||
60 | + Then I should see "Title can't be blank" | ||
61 | + And I should see "Body can't be blank" |
features/step_definitions/custom_webrat_steps.rb
@@ -10,3 +10,6 @@ When /^I wait (\d+) seconds$/ do |seconds| | @@ -10,3 +10,6 @@ When /^I wait (\d+) seconds$/ do |seconds| | ||
10 | sleep seconds.to_i | 10 | sleep seconds.to_i |
11 | end | 11 | end |
12 | 12 | ||
13 | +Then /^I should be exactly on (.+)$/ do |page_name| | ||
14 | + URI.parse(current_url).request_uri.should == path_to(page_name) | ||
15 | +end |
test/functional/contact_controller_test.rb
@@ -40,12 +40,6 @@ class ContactControllerTest < Test::Unit::TestCase | @@ -40,12 +40,6 @@ class ContactControllerTest < Test::Unit::TestCase | ||
40 | assert_tag :tag => 'textarea', :attributes => { :name => 'contact[message]' } | 40 | assert_tag :tag => 'textarea', :attributes => { :name => 'contact[message]' } |
41 | end | 41 | end |
42 | 42 | ||
43 | - should 'redirect back to contact page after send contact' do | ||
44 | - post :new, :profile => enterprise.identifier, :contact => {:subject => 'Hi', :message => 'Hi, all'} | ||
45 | - assert_response :redirect | ||
46 | - assert_redirected_to :action => 'new' | ||
47 | - end | ||
48 | - | ||
49 | should 'have logged user email' do | 43 | should 'have logged user email' do |
50 | get :new, :profile => enterprise.identifier | 44 | get :new, :profile => enterprise.identifier |
51 | assert_equal profile.email, assigns(:contact).email | 45 | assert_equal profile.email, assigns(:contact).email |
@@ -56,27 +50,11 @@ class ContactControllerTest < Test::Unit::TestCase | @@ -56,27 +50,11 @@ class ContactControllerTest < Test::Unit::TestCase | ||
56 | assert_equal profile.name, assigns(:contact).name | 50 | assert_equal profile.name, assigns(:contact).name |
57 | end | 51 | end |
58 | 52 | ||
59 | - should 'define city and state' do | ||
60 | - City.stubs(:exists?).returns(true) | ||
61 | - City.stubs(:find).returns(City.new(:name => 'Camaçari')) | ||
62 | - State.stubs(:exists?).returns(true) | ||
63 | - State.stubs(:find).returns(State.new(:name => 'Bahia')) | ||
64 | - post :new, :profile => enterprise.identifier, :contact => {:subject => 'Hi', :message => 'Hi, all'}, :state => '1', :city => '1' | ||
65 | - assert_equal 'Camaçari', assigns(:contact).city | ||
66 | - assert_equal 'Bahia', assigns(:contact).state | ||
67 | - end | ||
68 | - | ||
69 | should 'display checkbox for receive copy of email' do | 53 | should 'display checkbox for receive copy of email' do |
70 | get :new, :profile => enterprise.identifier | 54 | get :new, :profile => enterprise.identifier |
71 | assert_tag :tag => 'input', :attributes => {:name => 'contact[receive_a_copy]'} | 55 | assert_tag :tag => 'input', :attributes => {:name => 'contact[receive_a_copy]'} |
72 | end | 56 | end |
73 | 57 | ||
74 | - should 'deliver contact if subject and message are filled' do | ||
75 | - post :new, :profile => enterprise.identifier, :contact => {:subject => 'Hi', :message => 'Hi, all'} | ||
76 | - assert_response :redirect | ||
77 | - assert_redirected_to :action => 'new' | ||
78 | - end | ||
79 | - | ||
80 | should 'not throws exception when city and state is blank' do | 58 | should 'not throws exception when city and state is blank' do |
81 | State.expects(:exists?).with('').never | 59 | State.expects(:exists?).with('').never |
82 | City.expects(:exists?).with('').never | 60 | City.expects(:exists?).with('').never |
@@ -95,13 +73,6 @@ class ContactControllerTest < Test::Unit::TestCase | @@ -95,13 +73,6 @@ class ContactControllerTest < Test::Unit::TestCase | ||
95 | assert_no_tag :tag => 'select', :attributes => {:name => 'state'} | 73 | assert_no_tag :tag => 'select', :attributes => {:name => 'state'} |
96 | end | 74 | end |
97 | 75 | ||
98 | - should 'be able to post contact while inverse captcha field filled' do | ||
99 | - post :new, :profile => enterprise.identifier, :contact => {:subject => 'Hi', :message => 'Hi, all', :state => '', :city => ''} | ||
100 | - | ||
101 | - assert_response :redirect | ||
102 | - assert_redirected_to :action => 'new' | ||
103 | - end | ||
104 | - | ||
105 | should 'not be able to post contact while inverse captcha field filled' do | 76 | should 'not be able to post contact while inverse captcha field filled' do |
106 | post :new, :profile => enterprise.identifier, @controller.icaptcha_field => 'filled', :contact => {:subject => 'Hi', :message => 'Hi, all', :state => '', :city => ''} | 77 | post :new, :profile => enterprise.identifier, @controller.icaptcha_field => 'filled', :contact => {:subject => 'Hi', :message => 'Hi, all', :state => '', :city => ''} |
107 | 78 | ||
@@ -121,4 +92,32 @@ class ContactControllerTest < Test::Unit::TestCase | @@ -121,4 +92,32 @@ class ContactControllerTest < Test::Unit::TestCase | ||
121 | assert_equal Person['contact_test_user'], assigns(:contact).sender | 92 | assert_equal Person['contact_test_user'], assigns(:contact).sender |
122 | end | 93 | end |
123 | 94 | ||
95 | + should 'send contact while inverse captcha field not filled' do | ||
96 | + post :new, :profile => enterprise.identifier, :contact => {:subject => 'Hi', :message => 'Hi, all', :state => '', :city => ''}, :confirm => 'true' | ||
97 | + assert_response :redirect | ||
98 | + assert_redirected_to :action => 'new' | ||
99 | + end | ||
100 | + | ||
101 | + should 'deliver contact if subject and message are filled' do | ||
102 | + post :new, :profile => enterprise.identifier, :contact => {:subject => 'Hi', :message => 'Hi, all'}, :confirm => 'true' | ||
103 | + assert_response :redirect | ||
104 | + assert_redirected_to :action => 'new' | ||
105 | + end | ||
106 | + | ||
107 | + should 'redirect back to contact page after send contact' do | ||
108 | + post :new, :profile => enterprise.identifier, :contact => {:subject => 'Hi', :message => 'Hi, all'}, :confirm => 'true' | ||
109 | + assert_response :redirect | ||
110 | + assert_redirected_to :action => 'new' | ||
111 | + end | ||
112 | + | ||
113 | + should 'define city and state for contact object' do | ||
114 | + City.stubs(:exists?).returns(true) | ||
115 | + City.stubs(:find).returns(City.new(:name => 'Camaçari')) | ||
116 | + State.stubs(:exists?).returns(true) | ||
117 | + State.stubs(:find).returns(State.new(:name => 'Bahia')) | ||
118 | + post :new, :profile => enterprise.identifier, :contact => {:subject => 'Hi', :message => 'Hi, all'}, :state => '1', :city => '1', :confirm => 'true' | ||
119 | + assert_equal 'Camaçari', assigns(:contact).city | ||
120 | + assert_equal 'Bahia', assigns(:contact).state | ||
121 | + end | ||
122 | + | ||
124 | end | 123 | end |
test/functional/content_viewer_controller_test.rb
@@ -64,29 +64,6 @@ class ContentViewerControllerTest < Test::Unit::TestCase | @@ -64,29 +64,6 @@ class ContentViewerControllerTest < Test::Unit::TestCase | ||
64 | assert_response :missing | 64 | assert_response :missing |
65 | end | 65 | end |
66 | 66 | ||
67 | - def test_should_be_able_to_post_comment_while_authenticated | ||
68 | - profile = create_user('popstar').person | ||
69 | - page = profile.articles.build(:name => 'myarticle', :body => 'the body of the text') | ||
70 | - page.save! | ||
71 | - profile.home_page = page; profile.save! | ||
72 | - | ||
73 | - assert_difference Comment, :count do | ||
74 | - login_as('ze') | ||
75 | - post :view_page, :profile => 'popstar', :page => [ 'myarticle' ], :comment => { :title => 'crap!', :body => 'I think that this article is crap' } | ||
76 | - end | ||
77 | - end | ||
78 | - | ||
79 | - def test_should_be_able_to_post_comment_while_not_authenticated | ||
80 | - profile = create_user('popstar').person | ||
81 | - page = profile.articles.build(:name => 'myarticle', :body => 'the body of the text') | ||
82 | - page.save! | ||
83 | - profile.home_page = page; profile.save! | ||
84 | - | ||
85 | - assert_difference Comment, :count do | ||
86 | - post :view_page, :profile => 'popstar', :page => [ 'myarticle' ], :comment => { :title => 'crap!', :body => 'I think that this article is crap', :name => 'Anonymous coward', :email => 'coward@anonymous.com' } | ||
87 | - end | ||
88 | - end | ||
89 | - | ||
90 | should 'produce a download-like when article is not text/html' do | 67 | should 'produce a download-like when article is not text/html' do |
91 | 68 | ||
92 | # for example, RSS feeds | 69 | # for example, RSS feeds |
@@ -243,20 +220,6 @@ class ContentViewerControllerTest < Test::Unit::TestCase | @@ -243,20 +220,6 @@ class ContentViewerControllerTest < Test::Unit::TestCase | ||
243 | assert_tag :tag => 'input', :attributes => { :type => 'text', :name => @controller.icaptcha_field } | 220 | assert_tag :tag => 'input', :attributes => { :type => 'text', :name => @controller.icaptcha_field } |
244 | end | 221 | end |
245 | 222 | ||
246 | - should 'show error messages when make a blank comment' do | ||
247 | - login_as @profile.identifier | ||
248 | - page = profile.articles.create!(:name => 'myarticle', :body => 'the body of the text') | ||
249 | - post :view_page, :profile => @profile.identifier, :page => [ 'myarticle' ], :comment => { :title => '', :body => '' } | ||
250 | - assert_tag :tag => 'div', :attributes => { :class => 'errorExplanation', :id => 'errorExplanation' } | ||
251 | - end | ||
252 | - | ||
253 | - should 'show comment form opened on error' do | ||
254 | - login_as @profile.identifier | ||
255 | - page = profile.articles.create!(:name => 'myarticle', :body => 'the body of the text') | ||
256 | - post :view_page, :profile => @profile.identifier, :page => [ 'myarticle' ], :comment => { :title => '', :body => '' } | ||
257 | - assert_tag :tag => 'div', :attributes => { :class => 'post_comment_box opened' } | ||
258 | - end | ||
259 | - | ||
260 | should 'filter html content from body' do | 223 | should 'filter html content from body' do |
261 | login_as @profile.identifier | 224 | login_as @profile.identifier |
262 | page = profile.articles.create!(:name => 'myarticle', :body => 'the body of the text') | 225 | page = profile.articles.create!(:name => 'myarticle', :body => 'the body of the text') |
@@ -279,7 +242,7 @@ class ContentViewerControllerTest < Test::Unit::TestCase | @@ -279,7 +242,7 @@ class ContentViewerControllerTest < Test::Unit::TestCase | ||
279 | 242 | ||
280 | get :view_page, :profile => profile.identifier, :page => [ 'myarticle' ] | 243 | get :view_page, :profile => profile.identifier, :page => [ 'myarticle' ] |
281 | 244 | ||
282 | - assert_tag :tag => 'form', :attributes => { :id => /^comment_form/, :action => 'http://www.mysite.com/person/article' } | 245 | + assert_tag :tag => 'form', :attributes => { :id => /^comment_form/, :action => '/person/article' } |
283 | end | 246 | end |
284 | 247 | ||
285 | should "display current article's tags" do | 248 | should "display current article's tags" do |
@@ -778,14 +741,6 @@ class ContentViewerControllerTest < Test::Unit::TestCase | @@ -778,14 +741,6 @@ class ContentViewerControllerTest < Test::Unit::TestCase | ||
778 | assert_tag :tag => 'a', :content => 'Upload files', :attributes => {:href => /parent_id=#{folder.id}/} | 741 | assert_tag :tag => 'a', :content => 'Upload files', :attributes => {:href => /parent_id=#{folder.id}/} |
779 | end | 742 | end |
780 | 743 | ||
781 | - should 'have a link to properly post a comment' do | ||
782 | - login_as(profile.identifier) | ||
783 | - file = UploadedFile.create!(:profile => profile, :uploaded_data => fixture_file_upload('/files/rails.png', 'image/png')) | ||
784 | - get :view_page, :profile => profile.identifier, :page => file.explode_path, :view => true | ||
785 | - | ||
786 | - assert_tag :tag => 'input', :attributes => {:type => 'submit', :value => 'Post comment'}, :ancestor => {:tag => 'form', :attributes => {:action => /#{file.slug}.*view=true/}} | ||
787 | - end | ||
788 | - | ||
789 | should 'post comment in a image' do | 744 | should 'post comment in a image' do |
790 | login_as(profile.identifier) | 745 | login_as(profile.identifier) |
791 | image = UploadedFile.create!(:profile => profile, :uploaded_data => fixture_file_upload('/files/rails.png', 'image/png')) | 746 | image = UploadedFile.create!(:profile => profile, :uploaded_data => fixture_file_upload('/files/rails.png', 'image/png')) |
@@ -905,4 +860,11 @@ class ContentViewerControllerTest < Test::Unit::TestCase | @@ -905,4 +860,11 @@ class ContentViewerControllerTest < Test::Unit::TestCase | ||
905 | assert_tag :tag => 'span', :content => '(removed user)', :attributes => {:class => 'comment-info'} | 860 | assert_tag :tag => 'span', :content => '(removed user)', :attributes => {:class => 'comment-info'} |
906 | end | 861 | end |
907 | 862 | ||
863 | + should 'show comment form opened on error' do | ||
864 | + login_as @profile.identifier | ||
865 | + page = profile.articles.create!(:name => 'myarticle', :body => 'the body of the text') | ||
866 | + post :view_page, :profile => @profile.identifier, :page => [ 'myarticle' ], :comment => { :title => '', :body => '' }, :confirm => 'true' | ||
867 | + assert_tag :tag => 'div', :attributes => { :class => 'post_comment_box opened' } | ||
868 | + end | ||
869 | + | ||
908 | end | 870 | end |