Refactor: Split IssuesController#new to #new and #create to match REST pattern.
git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3688 e93f8b46-1217-0410-a6f0-8f06a7374b81
This commit is contained in:
parent
9b595c689d
commit
2c898d8a81
|
@ -21,7 +21,7 @@ class IssuesController < ApplicationController
|
||||||
|
|
||||||
before_filter :find_issue, :only => [:show, :edit, :update, :reply]
|
before_filter :find_issue, :only => [:show, :edit, :update, :reply]
|
||||||
before_filter :find_issues, :only => [:bulk_edit, :move, :destroy]
|
before_filter :find_issues, :only => [:bulk_edit, :move, :destroy]
|
||||||
before_filter :find_project, :only => [:new, :update_form, :preview, :auto_complete]
|
before_filter :find_project, :only => [:new, :create, :update_form, :preview, :auto_complete]
|
||||||
before_filter :authorize, :except => [:index, :changes, :gantt, :calendar, :preview, :context_menu]
|
before_filter :authorize, :except => [:index, :changes, :gantt, :calendar, :preview, :context_menu]
|
||||||
before_filter :find_optional_project, :only => [:index, :changes, :gantt, :calendar]
|
before_filter :find_optional_project, :only => [:index, :changes, :gantt, :calendar]
|
||||||
accept_key_auth :index, :show, :changes
|
accept_key_auth :index, :show, :changes
|
||||||
|
@ -51,6 +51,7 @@ class IssuesController < ApplicationController
|
||||||
:only => :destroy,
|
:only => :destroy,
|
||||||
:render => { :nothing => true, :status => :method_not_allowed }
|
:render => { :nothing => true, :status => :method_not_allowed }
|
||||||
|
|
||||||
|
verify :method => :post, :only => :create, :render => {:nothing => true, :status => :method_not_allowed }
|
||||||
verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed }
|
verify :method => :put, :only => :update, :render => {:nothing => true, :status => :method_not_allowed }
|
||||||
|
|
||||||
def index
|
def index
|
||||||
|
@ -145,35 +146,55 @@ class IssuesController < ApplicationController
|
||||||
@issue.watcher_user_ids = params[:issue]['watcher_user_ids'] if User.current.allowed_to?(:add_issue_watchers, @project)
|
@issue.watcher_user_ids = params[:issue]['watcher_user_ids'] if User.current.allowed_to?(:add_issue_watchers, @project)
|
||||||
end
|
end
|
||||||
@issue.author = User.current
|
@issue.author = User.current
|
||||||
|
@issue.start_date ||= Date.today
|
||||||
if request.get? || request.xhr?
|
|
||||||
@issue.start_date ||= Date.today
|
|
||||||
else
|
|
||||||
call_hook(:controller_issues_new_before_save, { :params => params, :issue => @issue })
|
|
||||||
if @issue.save
|
|
||||||
attachments = Attachment.attach_files(@issue, params[:attachments])
|
|
||||||
render_attachment_warning_if_needed(@issue)
|
|
||||||
flash[:notice] = l(:notice_successful_create)
|
|
||||||
call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue})
|
|
||||||
respond_to do |format|
|
|
||||||
format.html {
|
|
||||||
redirect_to(params[:continue] ? { :action => 'new', :issue => {:tracker_id => @issue.tracker,
|
|
||||||
:parent_issue_id => @issue.parent_issue_id}.reject {|k,v| v.nil?} } :
|
|
||||||
{ :action => 'show', :id => @issue })
|
|
||||||
}
|
|
||||||
format.xml { render :action => 'show', :status => :created, :location => url_for(:controller => 'issues', :action => 'show', :id => @issue) }
|
|
||||||
end
|
|
||||||
return
|
|
||||||
else
|
|
||||||
respond_to do |format|
|
|
||||||
format.html { }
|
|
||||||
format.xml { render(:xml => @issue.errors, :status => :unprocessable_entity); return }
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
@priorities = IssuePriority.all
|
@priorities = IssuePriority.all
|
||||||
@allowed_statuses = @issue.new_statuses_allowed_to(User.current, true)
|
@allowed_statuses = @issue.new_statuses_allowed_to(User.current, true)
|
||||||
render :layout => !request.xhr?
|
render :action => 'new', :layout => !request.xhr?
|
||||||
|
end
|
||||||
|
|
||||||
|
def create
|
||||||
|
@issue = Issue.new
|
||||||
|
@issue.copy_from(params[:copy_from]) if params[:copy_from]
|
||||||
|
@issue.project = @project
|
||||||
|
# Tracker must be set before custom field values
|
||||||
|
@issue.tracker ||= @project.trackers.find((params[:issue] && params[:issue][:tracker_id]) || params[:tracker_id] || :first)
|
||||||
|
if @issue.tracker.nil?
|
||||||
|
render_error l(:error_no_tracker_in_project)
|
||||||
|
return
|
||||||
|
end
|
||||||
|
if @issue.status.nil?
|
||||||
|
render_error l(:error_no_default_issue_status)
|
||||||
|
return
|
||||||
|
end
|
||||||
|
if params[:issue].is_a?(Hash)
|
||||||
|
@issue.safe_attributes = params[:issue]
|
||||||
|
@issue.watcher_user_ids = params[:issue]['watcher_user_ids'] if User.current.allowed_to?(:add_issue_watchers, @project)
|
||||||
|
end
|
||||||
|
@issue.author = User.current
|
||||||
|
|
||||||
|
@priorities = IssuePriority.all
|
||||||
|
@allowed_statuses = @issue.new_statuses_allowed_to(User.current, true)
|
||||||
|
|
||||||
|
call_hook(:controller_issues_new_before_save, { :params => params, :issue => @issue })
|
||||||
|
if @issue.save
|
||||||
|
attachments = Attachment.attach_files(@issue, params[:attachments])
|
||||||
|
render_attachment_warning_if_needed(@issue)
|
||||||
|
flash[:notice] = l(:notice_successful_create)
|
||||||
|
call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue})
|
||||||
|
respond_to do |format|
|
||||||
|
format.html {
|
||||||
|
redirect_to(params[:continue] ? { :action => 'new', :issue => {:tracker_id => @issue.tracker, :parent_issue_id => @issue.parent_issue_id}.reject {|k,v| v.nil?} } :
|
||||||
|
{ :action => 'show', :id => @issue })
|
||||||
|
}
|
||||||
|
format.xml { render :action => 'show', :status => :created, :location => url_for(:controller => 'issues', :action => 'show', :id => @issue) }
|
||||||
|
end
|
||||||
|
return
|
||||||
|
else
|
||||||
|
respond_to do |format|
|
||||||
|
format.html { render :action => 'new' }
|
||||||
|
format.xml { render(:xml => @issue.errors, :status => :unprocessable_entity); return }
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
# Attributes that can be updated on workflow transition (without :edit permission)
|
# Attributes that can be updated on workflow transition (without :edit permission)
|
||||||
|
|
|
@ -1,6 +1,6 @@
|
||||||
<h2><%=l(:label_issue_new)%></h2>
|
<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| %>
|
:html => {:multipart => true, :id => 'issue-form'} do |f| %>
|
||||||
<%= error_messages_for 'issue' %>
|
<%= error_messages_for 'issue' %>
|
||||||
<div class="box">
|
<div class="box">
|
||||||
|
|
|
@ -120,10 +120,10 @@ ActionController::Routing::Routes.draw do |map|
|
||||||
end
|
end
|
||||||
issues_routes.with_options :conditions => {:method => :post} do |issues_actions|
|
issues_routes.with_options :conditions => {:method => :post} do |issues_actions|
|
||||||
issues_actions.connect 'issues', :action => 'index'
|
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/quoted', :action => 'reply', :id => /\d+/
|
||||||
issues_actions.connect 'issues/:id/:action', :action => /edit|move|destroy/, :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
|
end
|
||||||
issues_routes.with_options :conditions => {:method => :put} do |issues_actions|
|
issues_routes.with_options :conditions => {:method => :put} do |issues_actions|
|
||||||
issues_actions.connect 'issues/:id/edit', :action => 'update', :id => /\d+/
|
issues_actions.connect 'issues/:id/edit', :action => 'update', :id => /\d+/
|
||||||
|
|
|
@ -62,7 +62,7 @@ Redmine::AccessControl.map do |map|
|
||||||
:versions => [:show, :status_by],
|
:versions => [:show, :status_by],
|
||||||
:queries => :index,
|
:queries => :index,
|
||||||
:reports => [:issue_report, :issue_report_details]}
|
: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 :edit_issues, {:issues => [:edit, :update, :reply, :bulk_edit, :update_form]}
|
||||||
map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]}
|
map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]}
|
||||||
map.permission :manage_subtasks, {}
|
map.permission :manage_subtasks, {}
|
||||||
|
|
|
@ -446,10 +446,10 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
assert_equal 'This is the test_new issue', issue.subject
|
assert_equal 'This is the test_new issue', issue.subject
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_post_new
|
def test_post_create
|
||||||
@request.session[:user_id] = 2
|
@request.session[:user_id] = 2
|
||||||
assert_difference 'Issue.count' do
|
assert_difference 'Issue.count' do
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 3,
|
:issue => {:tracker_id => 3,
|
||||||
:status_id => 2,
|
:status_id => 2,
|
||||||
:subject => 'This is the test_new issue',
|
:subject => 'This is the test_new issue',
|
||||||
|
@ -471,9 +471,9 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
assert_equal 'Value for field 2', v.value
|
assert_equal 'Value for field 2', v.value
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_post_new_and_continue
|
def test_post_create_and_continue
|
||||||
@request.session[:user_id] = 2
|
@request.session[:user_id] = 2
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 3,
|
:issue => {:tracker_id => 3,
|
||||||
:subject => 'This is first issue',
|
:subject => 'This is first issue',
|
||||||
:priority_id => 5},
|
:priority_id => 5},
|
||||||
|
@ -481,10 +481,10 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
assert_redirected_to :controller => 'issues', :action => 'new', :issue => {:tracker_id => 3}
|
assert_redirected_to :controller => 'issues', :action => 'new', :issue => {:tracker_id => 3}
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_post_new_without_custom_fields_param
|
def test_post_create_without_custom_fields_param
|
||||||
@request.session[:user_id] = 2
|
@request.session[:user_id] = 2
|
||||||
assert_difference 'Issue.count' do
|
assert_difference 'Issue.count' do
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 1,
|
:issue => {:tracker_id => 1,
|
||||||
:subject => 'This is the test_new issue',
|
:subject => 'This is the test_new issue',
|
||||||
:description => 'This is the description',
|
:description => 'This is the description',
|
||||||
|
@ -493,12 +493,12 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
assert_redirected_to :controller => 'issues', :action => 'show', :id => Issue.last.id
|
assert_redirected_to :controller => 'issues', :action => 'show', :id => Issue.last.id
|
||||||
end
|
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 = IssueCustomField.find_by_name('Database')
|
||||||
field.update_attribute(:is_required, true)
|
field.update_attribute(:is_required, true)
|
||||||
|
|
||||||
@request.session[:user_id] = 2
|
@request.session[:user_id] = 2
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 1,
|
:issue => {:tracker_id => 1,
|
||||||
:subject => 'This is the test_new issue',
|
:subject => 'This is the test_new issue',
|
||||||
:description => 'This is the description',
|
:description => 'This is the description',
|
||||||
|
@ -510,12 +510,12 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
assert_equal I18n.translate('activerecord.errors.messages.invalid'), issue.errors.on(:custom_values)
|
assert_equal I18n.translate('activerecord.errors.messages.invalid'), issue.errors.on(:custom_values)
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_post_new_with_watchers
|
def test_post_create_with_watchers
|
||||||
@request.session[:user_id] = 2
|
@request.session[:user_id] = 2
|
||||||
ActionMailer::Base.deliveries.clear
|
ActionMailer::Base.deliveries.clear
|
||||||
|
|
||||||
assert_difference 'Watcher.count', 2 do
|
assert_difference 'Watcher.count', 2 do
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 1,
|
:issue => {:tracker_id => 1,
|
||||||
:subject => 'This is a new issue with watchers',
|
:subject => 'This is a new issue with watchers',
|
||||||
:description => 'This is the description',
|
:description => 'This is the description',
|
||||||
|
@ -535,11 +535,11 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail)
|
assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail)
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_post_new_subissue
|
def test_post_create_subissue
|
||||||
@request.session[:user_id] = 2
|
@request.session[:user_id] = 2
|
||||||
|
|
||||||
assert_difference 'Issue.count' do
|
assert_difference 'Issue.count' do
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 1,
|
:issue => {:tracker_id => 1,
|
||||||
:subject => 'This is a child issue',
|
:subject => 'This is a child issue',
|
||||||
:parent_issue_id => 2}
|
:parent_issue_id => 2}
|
||||||
|
@ -549,11 +549,11 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
assert_equal Issue.find(2), issue.parent
|
assert_equal Issue.find(2), issue.parent
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_post_new_should_send_a_notification
|
def test_post_create_should_send_a_notification
|
||||||
ActionMailer::Base.deliveries.clear
|
ActionMailer::Base.deliveries.clear
|
||||||
@request.session[:user_id] = 2
|
@request.session[:user_id] = 2
|
||||||
assert_difference 'Issue.count' do
|
assert_difference 'Issue.count' do
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 3,
|
:issue => {:tracker_id => 3,
|
||||||
:subject => 'This is the test_new issue',
|
:subject => 'This is the test_new issue',
|
||||||
:description => 'This is the description',
|
:description => 'This is the description',
|
||||||
|
@ -566,9 +566,9 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
assert_equal 1, ActionMailer::Base.deliveries.size
|
assert_equal 1, ActionMailer::Base.deliveries.size
|
||||||
end
|
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
|
@request.session[:user_id] = 2
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 1,
|
:issue => {:tracker_id => 1,
|
||||||
# empty subject
|
# empty subject
|
||||||
:subject => '',
|
:subject => '',
|
||||||
|
@ -593,10 +593,10 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
:value => 'Value for field 2'}
|
:value => 'Value for field 2'}
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_post_new_should_ignore_non_safe_attributes
|
def test_post_create_should_ignore_non_safe_attributes
|
||||||
@request.session[:user_id] = 2
|
@request.session[:user_id] = 2
|
||||||
assert_nothing_raised do
|
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
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -619,7 +619,7 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
|
|
||||||
should "accept default status" do
|
should "accept default status" do
|
||||||
assert_difference 'Issue.count' do
|
assert_difference 'Issue.count' do
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 1,
|
:issue => {:tracker_id => 1,
|
||||||
:subject => 'This is an issue',
|
:subject => 'This is an issue',
|
||||||
:status_id => 1}
|
:status_id => 1}
|
||||||
|
@ -630,7 +630,7 @@ class IssuesControllerTest < ActionController::TestCase
|
||||||
|
|
||||||
should "ignore unauthorized status" do
|
should "ignore unauthorized status" do
|
||||||
assert_difference 'Issue.count' do
|
assert_difference 'Issue.count' do
|
||||||
post :new, :project_id => 1,
|
post :create, :project_id => 1,
|
||||||
:issue => {:tracker_id => 1,
|
:issue => {:tracker_id => 1,
|
||||||
:subject => 'This is an issue',
|
:subject => 'This is an issue',
|
||||||
:status_id => 3}
|
:status_id => 3}
|
||||||
|
|
|
@ -70,7 +70,8 @@ class RoutingTest < ActionController::IntegrationTest
|
||||||
should_route :get, "/issues/64.xml", :controller => 'issues', :action => 'show', :id => '64', :format => 'xml'
|
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 :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'
|
should_route :get, "/issues/64/edit", :controller => 'issues', :action => 'edit', :id => '64'
|
||||||
# TODO: Should use PUT
|
# TODO: Should use PUT
|
||||||
|
|
Loading…
Reference in New Issue