Skip to content

Commit

Permalink
Refactor: split NewsController#new into #new and #create methods.
Browse files Browse the repository at this point in the history
git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4163 e93f8b46-1217-0410-a6f0-8f06a7374b81
  • Loading branch information
edavis10 committed Sep 20, 2010
1 parent 1a4f5e8 commit 3bc29e2
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 12 deletions.
12 changes: 9 additions & 3 deletions app/controllers/news_controller.rb
Expand Up @@ -18,9 +18,9 @@
class NewsController < ApplicationController
default_search_scope :news
model_object News
before_filter :find_model_object, :except => [:new, :index, :preview]
before_filter :find_project_from_association, :except => [:new, :index, :preview]
before_filter :find_project, :only => [:new, :preview]
before_filter :find_model_object, :except => [:new, :create, :index, :preview]
before_filter :find_project_from_association, :except => [:new, :create, :index, :preview]
before_filter :find_project, :only => [:new, :create, :preview]
before_filter :authorize, :except => [:index, :preview]
before_filter :find_optional_project, :only => :index
accept_key_auth :index
Expand All @@ -46,11 +46,17 @@ def show

def new
@news = News.new(:project => @project, :author => User.current)
end

def create
@news = News.new(:project => @project, :author => User.current)
if request.post?
@news.attributes = params[:news]
if @news.save
flash[:notice] = l(:notice_successful_create)
redirect_to :controller => 'news', :action => 'index', :project_id => @project
else
render :action => 'new'
end
end

This comment has been minimized.

Copy link
@andreacampi

andreacampi Sep 21, 2010

Doesn't this miss a 'render' if the !post case?

Or probably better, similarly as other controllers:

if request.post? && @news.update_attributes(params[:news])

This comment has been minimized.

Copy link
@edavis10

edavis10 Sep 21, 2010

Author Owner

Yes, if a request comes in that isn't a POST it will be missed. I'm about to refactor the routes for this controller so only POST requests are sent to this action. Then (or a little later) I'll remove the if request.post? completely.

This comment has been minimized.

Copy link
@andreacampi

andreacampi Sep 21, 2010

Fair enough.

end
Expand Down
2 changes: 1 addition & 1 deletion app/views/news/index.rhtml
Expand Up @@ -7,7 +7,7 @@

<div id="add-news" style="display:none;">
<h2><%=l(:label_news_new)%></h2>
<% labelled_tabular_form_for :news, @news, :url => { :controller => 'news', :action => 'new', :project_id => @project },
<% labelled_tabular_form_for :news, @news, :url => { :controller => 'news', :action => 'create', :project_id => @project },
:html => { :id => 'news-form' } do |f| %>
<%= render :partial => 'news/form', :locals => { :f => f } %>
<%= submit_tag l(:button_create) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/news/new.rhtml
@@ -1,6 +1,6 @@
<h2><%=l(:label_news_new)%></h2>

<% labelled_tabular_form_for :news, @news, :url => { :controller => 'news', :action => 'new', :project_id => @project },
<% labelled_tabular_form_for :news, @news, :url => { :controller => 'news', :action => 'create', :project_id => @project },
:html => { :id => 'news-form' } do |f| %>
<%= render :partial => 'news/form', :locals => { :f => f } %>
<%= submit_tag l(:button_create) %>
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Expand Up @@ -148,7 +148,7 @@
news_views.connect 'news/:id/edit', :action => 'edit'
end
news_routes.with_options do |news_actions|
news_actions.connect 'projects/:project_id/news', :action => 'new'
news_actions.connect 'projects/:project_id/news', :action => 'create', :conditions => {:method => :post}
news_actions.connect 'news/:id/edit', :action => 'edit'
news_actions.connect 'news/:id/destroy', :action => 'destroy'
end
Expand Down
2 changes: 1 addition & 1 deletion lib/redmine.rb
Expand Up @@ -91,7 +91,7 @@
end

map.project_module :news do |map|
map.permission :manage_news, {:news => [:new, :edit, :destroy, :destroy_comment]}, :require => :member
map.permission :manage_news, {:news => [:new, :create, :edit, :destroy, :destroy_comment]}, :require => :member
map.permission :view_news, {:news => [:index, :show]}, :public => true
map.permission :comment_news, {:news => :add_comment}
end
Expand Down
8 changes: 4 additions & 4 deletions test/functional/news_controller_test.rb
Expand Up @@ -65,12 +65,12 @@ def test_get_new
assert_template 'new'
end

def test_post_new
def test_post_create
ActionMailer::Base.deliveries.clear
Setting.notified_events << 'news_added'

@request.session[:user_id] = 2
post :new, :project_id => 1, :news => { :title => 'NewsControllerTest',
post :create, :project_id => 1, :news => { :title => 'NewsControllerTest',
:description => 'This is the description',
:summary => '' }
assert_redirected_to 'projects/ecookbook/news'
Expand Down Expand Up @@ -98,9 +98,9 @@ def test_post_edit
assert_equal 'Description changed by test_post_edit', news.description
end

def test_post_new_with_validation_failure
def test_post_create_with_validation_failure
@request.session[:user_id] = 2
post :new, :project_id => 1, :news => { :title => '',
post :create, :project_id => 1, :news => { :title => '',
:description => 'This is the description',
:summary => '' }
assert_response :success
Expand Down
2 changes: 1 addition & 1 deletion test/integration/routing_test.rb
Expand Up @@ -157,7 +157,7 @@ class RoutingTest < ActionController::IntegrationTest
should_route :get, "/projects/567/news/new", :controller => 'news', :action => 'new', :project_id => '567'
should_route :get, "/news/234", :controller => 'news', :action => 'show', :id => '234'

should_route :post, "/projects/567/news/new", :controller => 'news', :action => 'new', :project_id => '567'
should_route :post, "/projects/567/news", :controller => 'news', :action => 'create', :project_id => '567'
should_route :post, "/news/567/edit", :controller => 'news', :action => 'edit', :id => '567'
should_route :post, "/news/567/destroy", :controller => 'news', :action => 'destroy', :id => '567'
end
Expand Down

0 comments on commit 3bc29e2

Please sign in to comment.