ProjectsController#add_issue moved to IssuesController#new.

Tracker can now be changed/selected on the new issue form. This action can be invoked without the tracker_id parameter (the first enabled tracker will be used by default).

git-svn-id: http://redmine.rubyforge.org/svn/trunk@1080 e93f8b46-1217-0410-a6f0-8f06a7374b81

13 files changed, 128 insertions, 74 deletions

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 11112289e..7a48282c9 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -17,11 +17,13 @@
 
 class IssuesController < ApplicationController
   layout 'base'
-  before_filter :find_project, :authorize, :except => [:index, :changes, :preview]
+  before_filter :find_issue, :except => [:index, :changes, :preview, :new, :update_form]
+  before_filter :find_project, :only => [:new, :update_form]
+  before_filter :authorize, :except => [:index, :changes, :preview, :update_form]
   before_filter :find_optional_project, :only => [:index, :changes]
   accept_key_auth :index, :changes
   
-  cache_sweeper :issue_sweeper, :only => [ :edit, :update, :destroy ]
+  cache_sweeper :issue_sweeper, :only => [ :new, :edit, :update, :destroy ]
 
   helper :projects
   include ProjectsHelper   
@@ -90,6 +92,51 @@ class IssuesController < ApplicationController
     end
   end
 
+  # Add a new issue
+  # The new issue will be created from an existing one if copy_from parameter is given
+  def new
+    @issue = params[:copy_from] ? Issue.new.copy_from(params[:copy_from]) : Issue.new(params[:issue])
+    @issue.project = @project
+    @issue.author = User.current
+    @issue.tracker ||= @project.trackers.find(params[:tracker_id] ? params[:tracker_id] : :first)
+    if @issue.tracker.nil?
+      flash.now[:error] = 'No tracker is associated to this project. Please check the Project settings.'
+      render :nothing => true, :layout => true
+      return
+    end
+    
+    default_status = IssueStatus.default
+    unless default_status
+      flash.now[:error] = 'No default issue status is defined. Please check your configuration (Go to "Administration -> Issue statuses").'
+      render :nothing => true, :layout => true
+      return
+    end    
+    @issue.status = default_status
+    @allowed_statuses = ([default_status] + default_status.find_new_statuses_allowed_to(User.current.role_for_project(@project), @issue.tracker))
+    
+    if request.get? || request.xhr?
+      @issue.start_date ||= Date.today
+      @custom_values = @issue.custom_values.empty? ?
+        @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue) } :
+        @issue.custom_values
+    else
+      requested_status = IssueStatus.find_by_id(params[:issue][:status_id])
+      # Check that the user is allowed to apply the requested status
+      @issue.status = (@allowed_statuses.include? requested_status) ? requested_status : default_status
+      @custom_values = @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue, :value => params["custom_fields"][x.id.to_s]) }
+      @issue.custom_values = @custom_values
+      if @issue.save
+        attach_files(@issue, params[:attachments])
+        flash[:notice] = l(:notice_successful_create)
+        Mailer.deliver_issue_add(@issue) if Setting.notified_events.include?('issue_added')
+        redirect_to :controller => 'issues', :action => 'index', :project_id => @project
+        return
+      end		
+    end	
+    @priorities = Enumeration::get_values('IPRI')
+    render :layout => !request.xhr?
+  end
+  
   def edit
     @priorities = Enumeration::get_values('IPRI')
     @custom_values = []
@@ -185,6 +232,11 @@ class IssuesController < ApplicationController
     render :layout => false
   end
 
+  def update_form
+    @issue = Issue.new(params[:issue])
+    render :action => :new, :layout => false
+  end
+  
   def preview
     issue = Issue.find_by_id(params[:id])
     @attachements = issue.attachments if issue
@@ -193,13 +245,19 @@ class IssuesController < ApplicationController
   end
   
 private
-  def find_project
+  def find_issue
     @issue = Issue.find(params[:id], :include => [:project, :tracker, :status, :author, :priority, :category])
     @project = @issue.project
   rescue ActiveRecord::RecordNotFound
     render_404
   end
   
+  def find_project
+    @project = Project.find(params[:project_id])
+  rescue ActiveRecord::RecordNotFound
+    render_404
+  end
+  
   def find_optional_project
     return true unless params[:project_id]
     @project = Project.find(params[:project_id])
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index f38ca828e..2342e8074 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -22,7 +22,7 @@ class ProjectsController < ApplicationController
   menu_item :roadmap, :only => :roadmap
   menu_item :files, :only => [:list_files, :add_file]
   menu_item :settings, :only => :settings
-  menu_item :issues, :only => [:add_issue, :bulk_edit_issues, :changelog, :move_issues]
+  menu_item :issues, :only => [:bulk_edit_issues, :changelog, :move_issues]
   
   before_filter :find_project, :except => [ :index, :list, :add ]
   before_filter :authorize, :except => [ :index, :list, :add, :archive, :unarchive, :destroy ]
@@ -30,7 +30,6 @@ class ProjectsController < ApplicationController
   accept_key_auth :activity, :calendar
   
   cache_sweeper :project_sweeper, :only => [ :add, :edit, :archive, :unarchive, :destroy ]
-  cache_sweeper :issue_sweeper, :only => [ :add_issue ]
   cache_sweeper :version_sweeper, :only => [ :add_version ]
 
   helper :sort
@@ -184,45 +183,6 @@ class ProjectsController < ApplicationController
   	end
   end
 
-  # Add a new issue to @project
-  # The new issue will be created from an existing one if copy_from parameter is given
-  def add_issue
-    @issue = params[:copy_from] ? Issue.new.copy_from(params[:copy_from]) : Issue.new(params[:issue])
-    @issue.project = @project
-    @issue.author = User.current
-    @issue.tracker ||= @project.trackers.find(params[:tracker_id])
-    
-    default_status = IssueStatus.default
-    unless default_status
-      flash.now[:error] = 'No default issue status is defined. Please check your configuration (Go to "Administration -> Issue statuses").'
-      render :nothing => true, :layout => true
-      return
-    end    
-    @issue.status = default_status
-    @allowed_statuses = ([default_status] + default_status.find_new_statuses_allowed_to(User.current.role_for_project(@project), @issue.tracker))
-    
-    if request.get?
-      @issue.start_date ||= Date.today
-      @custom_values = @issue.custom_values.empty? ?
-        @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue) } :
-        @issue.custom_values
-    else
-      requested_status = IssueStatus.find_by_id(params[:issue][:status_id])
-      # Check that the user is allowed to apply the requested status
-      @issue.status = (@allowed_statuses.include? requested_status) ? requested_status : default_status
-      @custom_values = @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue, :value => params["custom_fields"][x.id.to_s]) }
-      @issue.custom_values = @custom_values
-      if @issue.save
-        attach_files(@issue, params[:attachments])
-        flash[:notice] = l(:notice_successful_create)
-        Mailer.deliver_issue_add(@issue) if Setting.notified_events.include?('issue_added')
-        redirect_to :controller => 'issues', :action => 'index', :project_id => @project
-        return
-      end		
-    end	
-    @priorities = Enumeration::get_values('IPRI')
-  end
-
   # Bulk edit issues
   def bulk_edit_issues
     if request.post?
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index df4b9c334..ee61d88dd 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -194,6 +194,6 @@ module ProjectsHelper
     # can't use form tag inside helper
     content_tag('form',
       select_tag('tracker_id', '<option></option>' + options_from_collection_for_select(trackers, 'id', 'name'), :onchange => "if (this.value != '') {this.form.submit()}"),
-      :action => url_for(:controller => 'projects', :action => 'add_issue', :id => @project), :method => 'get')
+      :action => url_for(:controller => 'issues', :action => 'new', :project_id => @project), :method => 'get')
   end
 end
diff --git a/app/views/issues/_form.rhtml b/app/views/issues/_form.rhtml
index 203d1cca3..779846f4d 100644
--- a/app/views/issues/_form.rhtml
+++ b/app/views/issues/_form.rhtml
@@ -1,6 +1,13 @@
 <%= error_messages_for 'issue' %>
 <div class="box">
 
+<% if @issue.new_record? %>
+<p><%= f.select :tracker_id, @project.trackers.collect {|t| [t.name, t.id]}, :required => true %></p>
+<%= observe_field :issue_tracker_id, :url => { :action => :new },
+                                     :update => :content,
+                                     :with => "Form.serialize('issue-form')" %>
+<% end %>
+
 <div class="splitcontentleft">
 <% if @issue.new_record? %>
 <p><%= f.select :status_id, (@allowed_statuses.collect {|p| [p.name, p.id]}), :required => true %></p>
diff --git a/app/views/issues/_sidebar.rhtml b/app/views/issues/_sidebar.rhtml
index e6c63896b..1e1fc87d0 100644
--- a/app/views/issues/_sidebar.rhtml
+++ b/app/views/issues/_sidebar.rhtml
@@ -1,4 +1,4 @@
-<% if authorize_for('projects', 'add_issue') && @project.trackers.any? %>
+<% if authorize_for('issues', 'new') && @project.trackers.any? %>
 <h3><%= l(:label_issue_new) %></h3>
 <%= l(:label_tracker) %>: <%= new_issue_selector %>
 <% end %>
diff --git a/app/views/issues/context_menu.rhtml b/app/views/issues/context_menu.rhtml
index 0f11bb943..9691a7713 100644
--- a/app/views/issues/context_menu.rhtml
+++ b/app/views/issues/context_menu.rhtml
@@ -31,7 +31,7 @@
 		                              :selected => @issue.assigned_to.nil?, :disabled => !@can[:assign] %></li>
 		</ul>
 	</li>
-	<li><%= context_menu_link l(:button_copy), {:controller => 'projects', :action => 'add_issue', :id => @project, :copy_from => @issue},
+	<li><%= context_menu_link l(:button_copy), {:controller => 'issues', :action => 'new', :project_id => @project, :copy_from => @issue},
 	        :class => 'icon-copy', :disabled => !@can[:copy] %></li>
 	<li><%= context_menu_link l(:button_move), {:controller => 'projects', :action => 'move_issues', :id => @project, "issue_ids[]" => @issue.id },
 	                          :class => 'icon-move', :disabled => !@can[:move]  %>
diff --git a/app/views/projects/add_issue.rhtml b/app/views/issues/new.rhtml
index a68922906..4fedc6895 100644
--- a/app/views/projects/add_issue.rhtml
+++ b/app/views/issues/new.rhtml
@@ -1,9 +1,7 @@
 <h2><%=l(:label_issue_new)%>: <%= @issue.tracker %></h2>
 
 <% labelled_tabular_form_for :issue, @issue, 
-                             :url => {:action => 'add_issue'},
                              :html => {:multipart => true, :id => 'issue-form'} do |f| %>
-    <%= f.hidden_field :tracker_id %>
     <%= render :partial => 'issues/form', :locals => {:f => f} %>
     <%= submit_tag l(:button_create) %>
     <%= link_to_remote l(:label_preview), 
diff --git a/app/views/issues/show.rhtml b/app/views/issues/show.rhtml
index 8cd44424c..7208e37be 100644
--- a/app/views/issues/show.rhtml
+++ b/app/views/issues/show.rhtml
@@ -3,7 +3,7 @@
 <%= link_to_if_authorized l(:button_edit), {:controller => 'issues', :action => 'edit', :id => @issue}, :class => 'icon icon-edit', :accesskey => accesskey(:edit) %>
 <%= link_to_if_authorized l(:button_log_time), {:controller => 'timelog', :action => 'edit', :issue_id => @issue}, :class => 'icon icon-time' %>
 <%= watcher_tag(@issue, User.current) %>
-<%= link_to_if_authorized l(:button_copy), {:controller => 'projects', :action => 'add_issue', :id => @project, :copy_from => @issue }, :class => 'icon icon-copy' %>
+<%= link_to_if_authorized l(:button_copy), {:controller => 'issues', :action => 'new', :project_id => @project, :copy_from => @issue }, :class => 'icon icon-copy' %>
 <%= link_to_if_authorized l(:button_move), {:controller => 'projects', :action => 'move_issues', :id => @project, "issue_ids[]" => @issue.id }, :class => 'icon icon-move' %>
 <%= link_to_if_authorized l(:button_delete), {:controller => 'issues', :action => 'destroy', :id => @issue}, :confirm => l(:text_are_you_sure), :method => :post, :class => 'icon icon-del' %>
 </div>
diff --git a/app/views/projects/show.rhtml b/app/views/projects/show.rhtml
index 2b930e01e..ecaa9750a 100644
--- a/app/views/projects/show.rhtml
+++ b/app/views/projects/show.rhtml
@@ -56,7 +56,7 @@
 </div>
 
 <% content_for :sidebar do %>
-    <% if authorize_for('projects', 'add_issue') && @project.trackers.any? %>
+    <% if authorize_for('issues', 'new') && @project.trackers.any? %>
     <h3><%= l(:label_issue_new) %></h3>
     <%= l(:label_tracker) %>: <%= new_issue_selector %>
     <% end %>
diff --git a/lib/redmine.rb b/lib/redmine.rb
index d0e152467..ea05d6e80 100644
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -30,7 +30,7 @@ Redmine::AccessControl.map do |map|
                                   :versions => [:show, :status_by],
                                   :queries => :index,
                                   :reports => :issue_report}, :public => true                    
-    map.permission :add_issues, {:projects => :add_issue}
+    map.permission :add_issues, {:issues => :new}
     map.permission :edit_issues, {:projects => :bulk_edit_issues,
                                   :issues => [:edit, :update, :destroy_attachment]}
     map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]}
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index d60e32200..05bfc96e3 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -29,6 +29,7 @@ class IssuesControllerTest < Test::Unit::TestCase
            :issues,
            :issue_statuses,
            :trackers,
+           :projects_trackers,
            :issue_categories,
            :enabled_modules,
            :enumerations,
@@ -126,6 +127,55 @@ class IssuesControllerTest < Test::Unit::TestCase
                                             :content => /Notes/ } }
   end
 
+  def test_get_new
+    @request.session[:user_id] = 2
+    get :new, :project_id => 1, :tracker_id => 1
+    assert_response :success
+    assert_template 'new'
+  end
+
+  def test_get_new_without_tracker_id
+    @request.session[:user_id] = 2
+    get :new, :project_id => 1
+    assert_response :success
+    assert_template 'new'
+    
+    issue = assigns(:issue)
+    assert_not_nil issue
+    assert_equal Project.find(1).trackers.first, issue.tracker
+  end
+  
+  def test_update_new_form
+    @request.session[:user_id] = 2
+    xhr :post, :new, :project_id => 1,
+                     :issue => {:tracker_id => 2, 
+                                :subject => 'This is the test_new issue',
+                                :description => 'This is the description',
+                                :priority_id => 5}
+    assert_response :success
+    assert_template 'new'
+  end
+  
+  def test_post_new
+    @request.session[:user_id] = 2
+    post :new, :project_id => 1, 
+               :issue => {:tracker_id => 1,
+                          :subject => 'This is the test_new issue',
+                          :description => 'This is the description',
+                          :priority_id => 5}
+    assert_redirected_to 'projects/ecookbook/issues'
+    assert Issue.find_by_subject('This is the test_new issue')
+  end
+  
+  def test_copy_issue
+    @request.session[:user_id] = 2
+    get :new, :project_id => 1, :copy_from => 1
+    assert_template 'new'
+    assert_not_nil assigns(:issue)
+    orig = Issue.find(1)
+    assert_equal orig.subject, assigns(:issue).subject
+  end
+  
   def test_get_edit
     @request.session[:user_id] = 2
     get :edit, :id => 1
diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb
index f19aa7748..61047079e 100644
--- a/test/functional/projects_controller_test.rb
+++ b/test/functional/projects_controller_test.rb
@@ -236,23 +236,4 @@ class ProjectsControllerTest < Test::Unit::TestCase
     assert_redirected_to 'admin/projects'
     assert Project.find(1).active?
   end
-  
-  def test_add_issue
-    @request.session[:user_id] = 2
-    get :add_issue, :id => 1, :tracker_id => 1
-    assert_response :success
-    assert_template 'add_issue'
-    post :add_issue, :id => 1, :issue => {:tracker_id => 1, :subject => 'This is the test_add_issue issue', :description => 'This is the description', :priority_id => 5}
-    assert_redirected_to 'projects/ecookbook/issues'
-    assert Issue.find_by_subject('This is the test_add_issue issue')
-  end
-  
-  def test_copy_issue
-    @request.session[:user_id] = 2
-    get :add_issue, :id => 1, :copy_from => 1
-    assert_template 'add_issue'
-    assert_not_nil assigns(:issue)
-    orig = Issue.find(1)
-    assert_equal orig.subject, assigns(:issue).subject
-  end
 end
diff --git a/test/integration/issues_test.rb b/test/integration/issues_test.rb
index a9d3f9c74..81d27c30f 100644
--- a/test/integration/issues_test.rb
+++ b/test/integration/issues_test.rb
@@ -6,11 +6,11 @@ class IssuesTest < ActionController::IntegrationTest
   # create an issue
   def test_add_issue
     log_user('jsmith', 'jsmith')
-    get "projects/add_issue/1", :tracker_id => "1"
+    get 'projects/1/issues/new', :tracker_id => '1'
     assert_response :success
-    assert_template "projects/add_issue"
+    assert_template 'issues/new'
     
-    post "projects/add_issue/1", :tracker_id => "1",
+    post 'projects/1/issues/new', :tracker_id => "1",
                                  :issue => { :start_date => "2006-12-26", 
                                              :priority_id => "3", 
                                              :subject => "new test issue",