Refactor: Move gantts to a separate controller.

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

10 files changed, 131 insertions, 93 deletions

diff --git a/app/controllers/gantts_controller.rb b/app/controllers/gantts_controller.rb
new file mode 100644
index 000000000..91312e8ae
--- /dev/null
+++ b/app/controllers/gantts_controller.rb
@@ -0,0 +1,63 @@
+class GanttsController < ApplicationController
+  before_filter :find_optional_project
+
+  rescue_from Query::StatementInvalid, :with => :query_statement_invalid
+
+  helper :issues
+  helper :projects
+  helper :queries
+  include QueriesHelper
+  include Redmine::Export::PDF
+  
+  def show
+    @gantt = Redmine::Helpers::Gantt.new(params)
+    retrieve_query
+    @query.group_by = nil
+    if @query.valid?
+      events = []
+      # Issues that have start and due dates
+      events += @query.issues(:include => [:tracker, :assigned_to, :priority],
+                              :order => "start_date, due_date",
+                              :conditions => ["(((start_date>=? and start_date<=?) or (due_date>=? and due_date<=?) or (start_date<? and due_date>?)) and start_date is not null and due_date is not null)", @gantt.date_from, @gantt.date_to, @gantt.date_from, @gantt.date_to, @gantt.date_from, @gantt.date_to]
+                              )
+      # Issues that don't have a due date but that are assigned to a version with a date
+      events += @query.issues(:include => [:tracker, :assigned_to, :priority, :fixed_version],
+                              :order => "start_date, effective_date",
+                              :conditions => ["(((start_date>=? and start_date<=?) or (effective_date>=? and effective_date<=?) or (start_date<? and effective_date>?)) and start_date is not null and due_date is null and effective_date is not null)", @gantt.date_from, @gantt.date_to, @gantt.date_from, @gantt.date_to, @gantt.date_from, @gantt.date_to]
+                              )
+      # Versions
+      events += @query.versions(:conditions => ["effective_date BETWEEN ? AND ?", @gantt.date_from, @gantt.date_to])
+                                   
+      @gantt.events = events
+    end
+    
+    basename = (@project ? "#{@project.identifier}-" : '') + 'gantt'
+    
+    respond_to do |format|
+      format.html { render :action => "show", :layout => !request.xhr? }
+      format.png  { send_data(@gantt.to_image, :disposition => 'inline', :type => 'image/png', :filename => "#{basename}.png") } if @gantt.respond_to?('to_image')
+      format.pdf  { send_data(gantt_to_pdf(@gantt, @project), :type => 'application/pdf', :filename => "#{basename}.pdf") }
+    end
+  end
+
+  private
+
+  # Rescues an invalid query statement. Just in case...
+  # TODO: Refactor, move to ApplicationController with IssuesController
+  def query_statement_invalid(exception)
+    logger.error "Query::StatementInvalid: #{exception.message}" if logger
+    session.delete(:query)
+    sort_clear
+    render_error "An error occurred while executing the query and has been logged. Please report this error to your Redmine administrator."
+  end
+
+  # TODO: Refactor, duplicates IssuesController
+  def find_optional_project
+    @project = Project.find(params[:project_id]) unless params[:project_id].blank?
+    allowed = User.current.allowed_to?({:controller => params[:controller], :action => params[:action]}, @project, :global => true)
+    allowed ? true : deny_access
+  rescue ActiveRecord::RecordNotFound
+    render_404
+  end
+
+end
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 32bded8e8..3eb7f18a2 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -22,8 +22,8 @@ class IssuesController < ApplicationController
   before_filter :find_issue, :only => [:show, :edit, :update, :reply]
   before_filter :find_issues, :only => [:bulk_edit, :move, :destroy]
   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 :find_optional_project, :only => [:index, :changes, :gantt, :calendar]
+  before_filter :authorize, :except => [:index, :changes, :calendar, :preview, :context_menu]
+  before_filter :find_optional_project, :only => [:index, :changes, :calendar]
   before_filter :check_for_default_issue_status, :only => [:new, :create]
   before_filter :build_new_issue_from_params, :only => [:new, :create]
   accept_key_auth :index, :show, :changes
@@ -318,37 +318,6 @@ class IssuesController < ApplicationController
     end
   end
   
-  def gantt
-    @gantt = Redmine::Helpers::Gantt.new(params)
-    retrieve_query
-    @query.group_by = nil
-    if @query.valid?
-      events = []
-      # Issues that have start and due dates
-      events += @query.issues(:include => [:tracker, :assigned_to, :priority],
-                              :order => "start_date, due_date",
-                              :conditions => ["(((start_date>=? and start_date<=?) or (due_date>=? and due_date<=?) or (start_date<? and due_date>?)) and start_date is not null and due_date is not null)", @gantt.date_from, @gantt.date_to, @gantt.date_from, @gantt.date_to, @gantt.date_from, @gantt.date_to]
-                              )
-      # Issues that don't have a due date but that are assigned to a version with a date
-      events += @query.issues(:include => [:tracker, :assigned_to, :priority, :fixed_version],
-                              :order => "start_date, effective_date",
-                              :conditions => ["(((start_date>=? and start_date<=?) or (effective_date>=? and effective_date<=?) or (start_date<? and effective_date>?)) and start_date is not null and due_date is null and effective_date is not null)", @gantt.date_from, @gantt.date_to, @gantt.date_from, @gantt.date_to, @gantt.date_from, @gantt.date_to]
-                              )
-      # Versions
-      events += @query.versions(:conditions => ["effective_date BETWEEN ? AND ?", @gantt.date_from, @gantt.date_to])
-                                   
-      @gantt.events = events
-    end
-    
-    basename = (@project ? "#{@project.identifier}-" : '') + 'gantt'
-    
-    respond_to do |format|
-      format.html { render :template => "issues/gantt.rhtml", :layout => !request.xhr? }
-      format.png  { send_data(@gantt.to_image, :disposition => 'inline', :type => 'image/png', :filename => "#{basename}.png") } if @gantt.respond_to?('to_image')
-      format.pdf  { send_data(gantt_to_pdf(@gantt, @project), :type => 'application/pdf', :filename => "#{basename}.pdf") }
-    end
-  end
-  
   def calendar
     if params[:year] and params[:year].to_i > 1900
       @year = params[:year].to_i
diff --git a/app/views/issues/gantt.rhtml b/app/views/gantts/show.html.erb
index f0c6f4622..f0c6f4622 100644
--- a/app/views/issues/gantt.rhtml
+++ b/app/views/gantts/show.html.erb
diff --git a/app/views/issues/_sidebar.rhtml b/app/views/issues/_sidebar.rhtml
index 2296cc8e9..bcf0837f8 100644
--- a/app/views/issues/_sidebar.rhtml
+++ b/app/views/issues/_sidebar.rhtml
@@ -9,7 +9,7 @@
 	<%= link_to(l(:label_calendar), :controller => 'issues', :action => 'calendar', :project_id => @project) %><br />
 <% end %>
 <% if User.current.allowed_to?(:view_gantt, @project, :global => true) %>
-	<%= link_to(l(:label_gantt), :controller => 'issues', :action => 'gantt', :project_id => @project) %><br />
+	<%= link_to(l(:label_gantt), :controller => 'gantts', :action => 'show', :project_id => @project) %><br />
 <% end %>
 <%= call_hook(:view_issues_sidebar_planning_bottom) %>
 
diff --git a/app/views/projects/show.rhtml b/app/views/projects/show.rhtml
index e98c3504c..909c876fc 100644
--- a/app/views/projects/show.rhtml
+++ b/app/views/projects/show.rhtml
@@ -42,7 +42,7 @@
 				| <%= link_to(l(:label_calendar), :controller => 'issues', :action => 'calendar', :project_id => @project) %>
 			<% end %>
 			<% if User.current.allowed_to?(:view_gantt, @project, :global => true) %>
-				| <%= link_to(l(:label_gantt), :controller => 'issues', :action => 'gantt', :project_id => @project) %>
+				| <%= link_to(l(:label_gantt), :controller => 'gantts', :action => 'show', :project_id => @project) %>
 			<% end %>
 		</p>
   </div>
diff --git a/config/routes.rb b/config/routes.rb
index 2e10cd651..8571e77d3 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -110,7 +110,7 @@ ActionController::Routing::Routes.draw do |map|
       issues_views.connect 'projects/:project_id/issues', :action => 'index'
       issues_views.connect 'projects/:project_id/issues.:format', :action => 'index'
       issues_views.connect 'projects/:project_id/issues/new', :action => 'new'
-      issues_views.connect 'projects/:project_id/issues/gantt', :action => 'gantt'
+      issues_views.connect 'projects/:project_id/issues/gantt', :controller => 'gantts', :action => 'show'
       issues_views.connect 'projects/:project_id/issues/calendar', :action => 'calendar'
       issues_views.connect 'projects/:project_id/issues/:copy_from/copy', :action => 'new'
       issues_views.connect 'issues/:id', :action => 'show', :id => /\d+/
@@ -121,6 +121,7 @@ ActionController::Routing::Routes.draw do |map|
     issues_routes.with_options :conditions => {:method => :post} do |issues_actions|
       issues_actions.connect 'issues', :action => 'index'
       issues_actions.connect 'projects/:project_id/issues', :action => 'create'
+      issues_actions.connect 'projects/:project_id/issues/gantt', :controller => 'gantts', :action => 'show'
       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 => 'create', :format => /xml/
@@ -132,6 +133,7 @@ ActionController::Routing::Routes.draw do |map|
     issues_routes.with_options :conditions => {:method => :delete} do |issues_actions|
       issues_actions.connect 'issues/:id.:format', :action => 'destroy', :id => /\d+/, :format => /xml/
     end
+    issues_routes.connect 'issues/gantt', :controller => 'gantts', :action => 'show'
     issues_routes.connect 'issues/:action'
   end
   
diff --git a/lib/redmine.rb b/lib/redmine.rb
index ce8e3211b..d2a1cdb91 100644
--- a/lib/redmine.rb
+++ b/lib/redmine.rb
@@ -75,7 +75,7 @@ Redmine::AccessControl.map do |map|
     map.permission :manage_public_queries, {:queries => [:new, :edit, :destroy]}, :require => :member
     map.permission :save_queries, {:queries => [:new, :edit, :destroy]}, :require => :loggedin
     # Gantt & calendar
-    map.permission :view_gantt, :issues => :gantt
+    map.permission :view_gantt, :gantts => :show
     map.permission :view_calendar, :issues => :calendar
     # Watchers
     map.permission :view_issue_watchers, {}
diff --git a/test/functional/gantts_controller_test.rb b/test/functional/gantts_controller_test.rb
new file mode 100644
index 000000000..4c27de7cd
--- /dev/null
+++ b/test/functional/gantts_controller_test.rb
@@ -0,0 +1,56 @@
+require 'test_helper'
+
+class GanttsControllerTest < ActionController::TestCase
+  fixtures :all
+
+  context "#gantt" do
+    should "work" do
+      get :show, :project_id => 1
+      assert_response :success
+      assert_template 'show.html.erb'
+      assert_not_nil assigns(:gantt)
+      events = assigns(:gantt).events
+      assert_not_nil events
+      # Issue with start and due dates
+      i = Issue.find(1)
+      assert_not_nil i.due_date
+      assert events.include?(Issue.find(1))
+      # Issue with without due date but targeted to a version with date
+      i = Issue.find(2)
+      assert_nil i.due_date
+      assert events.include?(i)
+    end
+
+    should "work cross project" do
+      get :show
+      assert_response :success
+      assert_template 'show.html.erb'
+      assert_not_nil assigns(:gantt)
+      events = assigns(:gantt).events
+      assert_not_nil events
+    end
+
+    should "export to pdf" do
+      get :show, :project_id => 1, :format => 'pdf'
+      assert_response :success
+      assert_equal 'application/pdf', @response.content_type
+      assert @response.body.starts_with?('%PDF')
+      assert_not_nil assigns(:gantt)
+    end
+
+    should "export to pdf cross project" do
+      get :show, :format => 'pdf'
+      assert_response :success
+      assert_equal 'application/pdf', @response.content_type
+      assert @response.body.starts_with?('%PDF')
+      assert_not_nil assigns(:gantt)
+    end
+    
+    should "export to png" do
+      get :show, :project_id => 1, :format => 'png'
+      assert_response :success
+      assert_equal 'image/png', @response.content_type
+    end if Object.const_defined?(:Magick)
+
+  end
+end
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 97835133f..edf07dde6 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -231,58 +231,6 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal columns, session[:query][:column_names].map(&:to_s)
   end
 
-  def test_gantt
-    get :gantt, :project_id => 1
-    assert_response :success
-    assert_template 'gantt.rhtml'
-    assert_not_nil assigns(:gantt)
-    events = assigns(:gantt).events
-    assert_not_nil events
-    # Issue with start and due dates
-    i = Issue.find(1)
-    assert_not_nil i.due_date
-    assert events.include?(Issue.find(1))
-    # Issue with without due date but targeted to a version with date
-    i = Issue.find(2)
-    assert_nil i.due_date
-    assert events.include?(i)
-  end
-
-  def test_cross_project_gantt
-    get :gantt
-    assert_response :success
-    assert_template 'gantt.rhtml'
-    assert_not_nil assigns(:gantt)
-    events = assigns(:gantt).events
-    assert_not_nil events
-  end
-
-  def test_gantt_export_to_pdf
-    get :gantt, :project_id => 1, :format => 'pdf'
-    assert_response :success
-    assert_equal 'application/pdf', @response.content_type
-    assert @response.body.starts_with?('%PDF')
-    assert_not_nil assigns(:gantt)
-  end
-
-  def test_cross_project_gantt_export_to_pdf
-    get :gantt, :format => 'pdf'
-    assert_response :success
-    assert_equal 'application/pdf', @response.content_type
-    assert @response.body.starts_with?('%PDF')
-    assert_not_nil assigns(:gantt)
-  end
-  
-  if Object.const_defined?(:Magick)
-    def test_gantt_image
-      get :gantt, :project_id => 1, :format => 'png'
-      assert_response :success
-      assert_equal 'image/png', @response.content_type
-    end
-  else
-    puts "RMagick not installed. Skipping tests !!!"
-  end
-  
   def test_calendar
     get :calendar, :project_id => 1
     assert_response :success
diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb
index abb182623..c4736d415 100644
--- a/test/integration/routing_test.rb
+++ b/test/integration/routing_test.rb
@@ -95,10 +95,10 @@ class RoutingTest < ActionController::IntegrationTest
     should_route :get, "/projects/project-name/issues/calendar", :controller => 'issues', :action => 'calendar', :project_id => 'project-name'
     should_route :post, "/projects/project-name/issues/calendar", :controller => 'issues', :action => 'calendar', :project_id => 'project-name'
 
-    should_route :get, "/issues/gantt", :controller => 'issues', :action => 'gantt'
-    should_route :post, "/issues/gantt", :controller => 'issues', :action => 'gantt'
-    should_route :get, "/projects/project-name/issues/gantt", :controller => 'issues', :action => 'gantt', :project_id => 'project-name'
-    should_route :post, "/projects/project-name/issues/gantt", :controller => 'issues', :action => 'gantt', :project_id => 'project-name'
+    should_route :get, "/issues/gantt", :controller => 'gantts', :action => 'show'
+    should_route :post, "/issues/gantt", :controller => 'gantts', :action => 'show'
+    should_route :get, "/projects/project-name/issues/gantt", :controller => 'gantts', :action => 'show', :project_id => 'project-name'
+    should_route :post, "/projects/project-name/issues/gantt", :controller => 'gantts', :action => 'show', :project_id => 'project-name'
 
     should_route :get, "/issues/auto_complete", :controller => 'issues', :action => 'auto_complete'
   end