Refactor: move NewsController#add_comment to CommentsController#create

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4170 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb
new file mode 100644 (file)
index 0000000..c0d524a
--- /dev/null
+++ b/app/controllers/comments_controller.rb
@@ -0,0 +1,30 @@
+class CommentsController < ApplicationController
+  default_search_scope :news
+  model_object News
+  before_filter :find_model_object
+  before_filter :find_project_from_association
+  before_filter :authorize
+
+  verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed }
+  def create
+    @comment = Comment.new(params[:comment])
+    @comment.author = User.current
+    if @news.comments << @comment
+      flash[:notice] = l(:label_comment_added)
+    end
+    
+    redirect_to :controller => 'news', :action => 'show', :id => @news
+  end
+
+  private
+
+  # ApplicationController's find_model_object sets it based on the controller
+  # name so it needs to be overriden and set to @news instead
+  def find_model_object
+    super
+    @news = @object
+    @comment = nil
+    @news
+  end
+  
+end

diff --git a/app/controllers/news_controller.rb b/app/controllers/news_controller.rb
index 5a1d36e0f9fe20debcba8522b179a4f3b621c008..d15ae3291c285d6285e7adc7d2b9447e66c13b12 100644 (file)
--- a/app/controllers/news_controller.rb
+++ b/app/controllers/news_controller.rb
@@ -73,18 +73,6 @@ class NewsController < ApplicationController
     end
   end
 
-  def add_comment
-    @comment = Comment.new(params[:comment])
-    @comment.author = User.current
-    if @news.comments << @comment
-      flash[:notice] = l(:label_comment_added)
-      redirect_to :action => 'show', :id => @news
-    else
-      show
-      render :action => 'show'
-    end
-  end
-
   def destroy_comment
     @news.comments.find(params[:comment_id]).destroy
     redirect_to :action => 'show', :id => @news

diff --git a/app/views/news/show.rhtml b/app/views/news/show.rhtml
index 851ef3f758625f9cae28cceb4775dda992c83dab..9b8f2510e67b7d0f9eb2b582517dc54885104154 100644 (file)
--- a/app/views/news/show.rhtml
+++ b/app/views/news/show.rhtml
@@ -47,9 +47,9 @@
 <% end if @comments.any? %>
 </div>
 
-<% if authorize_for 'news', 'add_comment' %>
+<% if authorize_for 'comments', 'create' %>
 <p><%= toggle_link l(:label_comment_add), "add_comment_form", :focus => "comment_comments" %></p>
-<% form_tag({:action => 'add_comment', :id => @news}, :id => "add_comment_form", :style => "display:none;") do %>
+<% form_tag({:controller => 'comments', :action => 'create', :id => @news}, :id => "add_comment_form", :style => "display:none;") do %>
 <div class="box">
     <%= text_area 'comment', 'comments', :cols => 80, :rows => 15, :class => 'wiki-edit' %>
     <%= wikitoolbar_for 'comment_comments' %>

diff --git a/config/routes.rb b/config/routes.rb
index 2950cba2180ab4f0a84538a0df27f045f087c248..6c765dab15dcaddd2ccaa3c1ed2310ff06d409aa 100644 (file)
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -152,6 +152,8 @@ ActionController::Routing::Routes.draw do |map|
       news_actions.connect 'news/:id/destroy', :action => 'destroy'
     end
     news_routes.connect 'news/:id/edit', :action => 'update', :conditions => {:method => :put}
+
+    news_routes.connect 'news/:id/comments', :controller => 'comments', :action => 'create', :conditions => {:method => :post}
   end
   
   map.connect 'projects/:id/members/new', :controller => 'members', :action => 'new'

diff --git a/lib/redmine.rb b/lib/redmine.rb
index a7417df880cd42035982a5c4f604bc5123ec7921..198d37304bbe7551989e2df5a70adb00268353ce 100644 (file)
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -93,7 +93,7 @@ Redmine::AccessControl.map do |map|
   map.project_module :news do |map|
     map.permission :manage_news, {:news => [:new, :create, :edit, :update, :destroy, :destroy_comment]}, :require => :member
     map.permission :view_news, {:news => [:index, :show]}, :public => true
-    map.permission :comment_news, {:news => :add_comment}
+    map.permission :comment_news, {:comments => :create}
   end
 
   map.project_module :documents do |map|

diff --git a/test/functional/comments_controller_test.rb b/test/functional/comments_controller_test.rb
new file mode 100644 (file)
index 0000000..1a9d628
--- /dev/null
+++ b/test/functional/comments_controller_test.rb
@@ -0,0 +1,46 @@
+# redMine - project management software
+# Copyright (C) 2006-2007  Jean-Philippe Lang
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+
+require File.dirname(__FILE__) + '/../test_helper'
+
+class CommentsControllerTest < ActionController::TestCase
+  fixtures :projects, :users, :roles, :members, :member_roles, :enabled_modules, :news, :comments
+  
+  def setup
+    User.current = nil
+  end
+  
+  def test_add_comment
+    @request.session[:user_id] = 2
+    post :create, :id => 1, :comment => { :comments => 'This is a test comment' }
+    assert_redirected_to 'news/1'
+    
+    comment = News.find(1).comments.find(:first, :order => 'created_on DESC')
+    assert_not_nil comment
+    assert_equal 'This is a test comment', comment.comments
+    assert_equal User.find(2), comment.author
+  end
+  
+  def test_empty_comment_should_not_be_added
+    @request.session[:user_id] = 2
+    assert_no_difference 'Comment.count' do
+      post :create, :id => 1, :comment => { :comments => '' }
+      assert_response :redirect
+      assert_redirected_to 'news/1'
+    end
+  end
+end

diff --git a/test/functional/news_controller_test.rb b/test/functional/news_controller_test.rb
index 5f00fd914d6edf28d6bc227c9350d72ae7930a3e..ddc8b0c2c738d1061ae16c3aaea04b3e0f9068f5 100644 (file)
--- a/test/functional/news_controller_test.rb
+++ b/test/functional/news_controller_test.rb
@@ -111,26 +111,6 @@ class NewsControllerTest < ActionController::TestCase
                               :content => /1 error/
   end
   
-  def test_add_comment
-    @request.session[:user_id] = 2
-    post :add_comment, :id => 1, :comment => { :comments => 'This is a NewsControllerTest comment' }
-    assert_redirected_to 'news/1'
-    
-    comment = News.find(1).comments.find(:first, :order => 'created_on DESC')
-    assert_not_nil comment
-    assert_equal 'This is a NewsControllerTest comment', comment.comments
-    assert_equal User.find(2), comment.author
-  end
-  
-  def test_empty_comment_should_not_be_added
-    @request.session[:user_id] = 2
-    assert_no_difference 'Comment.count' do
-      post :add_comment, :id => 1, :comment => { :comments => '' }
-      assert_response :success
-      assert_template 'show'
-    end
-  end
-  
   def test_destroy_comment
     comments_count = News.find(1).comments.size
     @request.session[:user_id] = 2

diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb
index 0daaad6dc10847d7dd18fdcf549d91561b6f28fb..4ce8fe4a7ebc83e202427a06817e18ae0b54063f 100644 (file)
--- a/test/integration/routing_test.rb
+++ b/test/integration/routing_test.rb
@@ -160,7 +160,8 @@ class RoutingTest < ActionController::IntegrationTest
 
     should_route :post, "/projects/567/news", :controller => 'news', :action => 'create', :project_id => '567'
     should_route :post, "/news/567/destroy", :controller => 'news', :action => 'destroy', :id => '567'
-
+    should_route :post, "/news/567/comments", :controller => 'comments', :action => 'create', :id => '567'
+    
     should_route :put, "/news/567/edit", :controller => 'news', :action => 'update', :id => '567'
   end