jro_apps/redmine Commit - r3574:2c898d8a817e

Refactor: Split IssuesController#new to #new and #create to match REST pattern....

Eric Davis -

r3574:2c898d8a817e

parent child

Context file:

r3574:2c898d8a817e

Collapse all files

app/controllers/issues_controller.rb +49 -28

@@ -21,7 +21,7 class IssuesController < ApplicationController
21		21
22	before_filter :find_issue, :only => [:show, :edit, :update, :reply]	22	before_filter :find_issue, :only => [:show, :edit, :update, :reply]
23	before_filter :find_issues, :only => [:bulk_edit, :move, :destroy]	23	before_filter :find_issues, :only => [:bulk_edit, :move, :destroy]
24	before_filter :find_project, :only => [:new, :update_form, :preview, :auto_complete]	24	before_filter :find_project, :only => [:new, :create, :update_form, :preview, :auto_complete]
25	before_filter :authorize, :except => [:index, :changes, :gantt, :calendar, :preview, :context_menu]	25	before_filter :authorize, :except => [:index, :changes, :gantt, :calendar, :preview, :context_menu]
26	before_filter :find_optional_project, :only => [:index, :changes, :gantt, :calendar]	26	before_filter :find_optional_project, :only => [:index, :changes, :gantt, :calendar]
27	accept_key_auth :index, :show, :changes	27	accept_key_auth :index, :show, :changes
@@ -51,6 +51,7 class IssuesController < ApplicationController
51	:only => :destroy,	51	:only => :destroy,
52	:render => { :nothing => true, :status => :method_not_allowed }	52	:render => { :nothing => true, :status => :method_not_allowed }
53		53
		54	verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed }
54	verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed }	55	verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed }
55		56
56	def index	57	def index
@@ -145,35 +146,55 class IssuesController < ApplicationController
145	@issue.watcher_user_ids = params[:issue]['watcher_user_ids'] if User.current.allowed_to?(:add_issue_watchers, @project)	146	@issue.watcher_user_ids = params[:issue]['watcher_user_ids'] if User.current.allowed_to?(:add_issue_watchers, @project)
146	end	147	end
147	@issue.author = User.current	148	@issue.author = User.current
148		149	@issue.start_date \|\|= Date.today
149	if request.get? \|\| request.xhr?	150	@priorities = IssuePriority.all
150	@issue.start_date \|\|= Date.today	151	@allowed_statuses = @issue.new_statuses_allowed_to(User.current, true)
151	else	152	render :action => 'new', :layout => !request.xhr?
152	call_hook(:controller_issues_new_before_save, { :params => params, :issue => @issue })	153	end
153	if @issue.save	154
154	attachments = Attachment.attach_files(@issue, params[:attachments])	155	def create
155	render_attachment_warning_if_needed(@issue)	156	@issue = Issue.new
156	flash[:notice] = l(:notice_successful_create)	157	@issue.copy_from(params[:copy_from]) if params[:copy_from]
157	call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue})	158	@issue.project = @project
158	respond_to do \|format\|	159	# Tracker must be set before custom field values
159	format.html {	160	@issue.tracker \|\|= @project.trackers.find((params[:issue] && params[:issue][:tracker_id]) \|\| params[:tracker_id] \|\| :first)
160	redirect_to(params[:continue] ? { :action => 'new', :issue => {:tracker_id => @issue.tracker,	161	if @issue.tracker.nil?
161	:parent_issue_id => @issue.parent_issue_id}.reject {\|k,v\| v.nil?} } :	162	render_error l(:error_no_tracker_in_project)
162	{ :action => 'show', :id => @issue })	163	return
163	}	164	end
164	format.xml { render :action => 'show', :status => :created, :location => url_for(:controller => 'issues', :action => 'show', :id => @issue) }	165	if @issue.status.nil?
165	end	166	render_error l(:error_no_default_issue_status)
166	return	167	return
167	else	168	end
168	respond_to do \|format\|	169	if params[:issue].is_a?(Hash)
169	format.html { }	170	@issue.safe_attributes = params[:issue]
170	format.xml { render(:xml => @issue.errors, :status => :unprocessable_entity); return }	171	@issue.watcher_user_ids = params[:issue]['watcher_user_ids'] if User.current.allowed_to?(:add_issue_watchers, @project)
171	end	172	end
172	end	173	@issue.author = User.current
173	end	174
174	@priorities = IssuePriority.all	175	@priorities = IssuePriority.all
175	@allowed_statuses = @issue.new_statuses_allowed_to(User.current, true)	176	@allowed_statuses = @issue.new_statuses_allowed_to(User.current, true)
176	render :layout => !request.xhr?	177
		178	call_hook(:controller_issues_new_before_save, { :params => params, :issue => @issue })
		179	if @issue.save
		180	attachments = Attachment.attach_files(@issue, params[:attachments])
		181	render_attachment_warning_if_needed(@issue)
		182	flash[:notice] = l(:notice_successful_create)
		183	call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue})
		184	respond_to do \|format\|
		185	format.html {
		186	redirect_to(params[:continue] ? { :action => 'new', :issue => {:tracker_id => @issue.tracker, :parent_issue_id => @issue.parent_issue_id}.reject {\|k,v\| v.nil?} } :
		187	{ :action => 'show', :id => @issue })
		188	}
		189	format.xml { render :action => 'show', :status => :created, :location => url_for(:controller => 'issues', :action => 'show', :id => @issue) }
		190	end
		191	return
		192	else
		193	respond_to do \|format\|
		194	format.html { render :action => 'new' }
		195	format.xml { render(:xml => @issue.errors, :status => :unprocessable_entity); return }
		196	end
		197	end
177	end	198	end
178		199
179	# Attributes that can be updated on workflow transition (without :edit permission)	200	# Attributes that can be updated on workflow transition (without :edit permission)

app/views/issues/new.rhtml +1 -1

             <h2><%=l(:label_issue_new)%></h2>
-            <% labelled_tabular_form_for :issue, @issue,
+            <% labelled_tabular_form_for :issue, @issue, :url => {:controller => 'issues', :action => 'create', :project_id => @project},
                                          :html => {:multipart => true, :id => 'issue-form'} do |f| %>
                 <%= error_messages_for 'issue' %>
                 <div class="box">

config/routes.rb +2 -2

                 end
                 issues_routes.with_options :conditions => {:method => :post} do |issues_actions|
                   issues_actions.connect 'issues', :action => 'index'
-                  issues_actions.connect 'projects/:project_id/issues', :action => 'new'
+                  issues_actions.connect 'projects/:project_id/issues', :action => 'create'
                   issues_actions.connect 'issues/:id/quoted', :action => 'reply', :id => /\d+/
                   issues_actions.connect 'issues/:id/:action', :action => /edit|move|destroy/, :id => /\d+/
-                  issues_actions.connect 'issues.:format', :action => 'new', :format => /xml/
+                  issues_actions.connect 'issues.:format', :action => 'create', :format => /xml/
                 end
                 issues_routes.with_options :conditions => {:method => :put} do |issues_actions|
                   issues_actions.connect 'issues/:id/edit', :action => 'update', :id => /\d+/

lib/redmine.rb +1 -1

                                               :versions => [:show, :status_by],
                                               :queries => :index,
                                               :reports => [:issue_report, :issue_report_details]}
-                map.permission :add_issues, {:issues => [:new, :update_form]}
+                map.permission :add_issues, {:issues => [:new, :create, :update_form]}
                 map.permission :edit_issues, {:issues => [:edit, :update, :reply, :bulk_edit, :update_form]}
                 map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]}
                 map.permission :manage_subtasks, {}

test/functional/issues_controller_test.rb +20 -20

                 assert_equal 'This is the test_new issue', issue.subject
               end
-              def test_post_new
+              def test_post_create
                 @request.session[:user_id] = 2
                 assert_difference 'Issue.count' do
-                  post :new, :project_id => 1,
+                  post :create, :project_id => 1,
                              :issue => {:tracker_id => 3,
                                         :status_id => 2,
                                         :subject => 'This is the test_new issue',
                 assert_equal 'Value for field 2', v.value
               end
-              def test_post_new_and_continue
+              def test_post_create_and_continue
                 @request.session[:user_id] = 2
-                post :new, :project_id => 1,
+                post :create, :project_id => 1,
                            :issue => {:tracker_id => 3,
                                       :subject => 'This is first issue',
                                       :priority_id => 5},
                 assert_redirected_to :controller => 'issues', :action => 'new', :issue => {:tracker_id => 3}
               end
-              def test_post_new_without_custom_fields_param
+              def test_post_create_without_custom_fields_param
                 @request.session[:user_id] = 2
                 assert_difference 'Issue.count' do
-                  post :new, :project_id => 1,
+                  post :create, :project_id => 1,
                              :issue => {:tracker_id => 1,
                                         :subject => 'This is the test_new issue',
                                         :description => 'This is the description',
                 assert_redirected_to :controller => 'issues', :action => 'show', :id => Issue.last.id
               end
-              def test_post_new_with_required_custom_field_and_without_custom_fields_param
+              def test_post_create_with_required_custom_field_and_without_custom_fields_param
                 field = IssueCustomField.find_by_name('Database')
                 field.update_attribute(:is_required, true)
                 @request.session[:user_id] = 2
-                post :new, :project_id => 1,
+                post :create, :project_id => 1,
                            :issue => {:tracker_id => 1,
                                       :subject => 'This is the test_new issue',
                                       :description => 'This is the description',
                 assert_equal I18n.translate('activerecord.errors.messages.invalid'), issue.errors.on(:custom_values)
               end
-              def test_post_new_with_watchers
+              def test_post_create_with_watchers
                 @request.session[:user_id] = 2
                 ActionMailer::Base.deliveries.clear
                 assert_difference 'Watcher.count', 2 do
-                  post :new, :project_id => 1,
+                  post :create, :project_id => 1,
                              :issue => {:tracker_id => 1,
                                         :subject => 'This is a new issue with watchers',
                                         :description => 'This is the description',
                 assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail)
               end
-              def test_post_new_subissue
+              def test_post_create_subissue
                 @request.session[:user_id] = 2
                 assert_difference 'Issue.count' do
-                  post :new, :project_id => 1,
+                  post :create, :project_id => 1,
                              :issue => {:tracker_id => 1,
                                         :subject => 'This is a child issue',
                                         :parent_issue_id => 2}
                 assert_equal Issue.find(2), issue.parent
               end
-              def test_post_new_should_send_a_notification
+              def test_post_create_should_send_a_notification
                 ActionMailer::Base.deliveries.clear
                 @request.session[:user_id] = 2
                 assert_difference 'Issue.count' do
-                  post :new, :project_id => 1,
+                  post :create, :project_id => 1,
                              :issue => {:tracker_id => 3,
                                         :subject => 'This is the test_new issue',
                                         :description => 'This is the description',
                 assert_equal 1, ActionMailer::Base.deliveries.size
               end
-              def test_post_should_preserve_fields_values_on_validation_failure
+              def test_post_create_should_preserve_fields_values_on_validation_failure
                 @request.session[:user_id] = 2
-                post :new, :project_id => 1,
+                post :create, :project_id => 1,
                            :issue => {:tracker_id => 1,
                                       # empty subject
                                       :subject => '',
                                                     :value => 'Value for field 2'}
               end
-              def test_post_new_should_ignore_non_safe_attributes
+              def test_post_create_should_ignore_non_safe_attributes
                 @request.session[:user_id] = 2
                 assert_nothing_raised do
-                  post :new, :project_id => 1, :issue => { :tracker => "A param can not be a Tracker" }
+                  post :create, :project_id => 1, :issue => { :tracker => "A param can not be a Tracker" }
                 end
               end
                   should "accept default status" do
                     assert_difference 'Issue.count' do
-                      post :new, :project_id => 1,
+                      post :create, :project_id => 1,
                                  :issue => {:tracker_id => 1,
                                             :subject => 'This is an issue',
                                             :status_id => 1}
                   should "ignore unauthorized status" do
                     assert_difference 'Issue.count' do
-                      post :new, :project_id => 1,
+                      post :create, :project_id => 1,
                                  :issue => {:tracker_id => 1,
                                             :subject => 'This is an issue',
                                             :status_id => 3}

test/integration/routing_test.rb +2 -1

                 should_route :get, "/issues/64.xml", :controller => 'issues', :action => 'show', :id => '64', :format => 'xml'
                 should_route :get, "/projects/23/issues/new", :controller => 'issues', :action => 'new', :project_id => '23'
-                should_route :post, "/issues.xml", :controller => 'issues', :action => 'new', :format => 'xml'
+                should_route :post, "/projects/23/issues", :controller => 'issues', :action => 'create', :project_id => '23'
+                should_route :post, "/issues.xml", :controller => 'issues', :action => 'create', :format => 'xml'
                 should_route :get, "/issues/64/edit", :controller => 'issues', :action => 'edit', :id => '64'
                 # TODO: Should use PUT

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages