From e65c3cfc7df3da58071c8de158c98861dab7d852 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Wed, 28 Apr 2010 15:54:46 +0000 Subject: [PATCH] Refactor: Move gantts to a separate controller. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3695 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/gantts_controller.rb | 63 +++++++++++++++++++ app/controllers/issues_controller.rb | 35 +---------- .../gantt.rhtml => gantts/show.html.erb} | 0 app/views/issues/_sidebar.rhtml | 2 +- app/views/projects/show.rhtml | 2 +- config/routes.rb | 4 +- lib/redmine.rb | 2 +- test/functional/gantts_controller_test.rb | 56 +++++++++++++++++ test/functional/issues_controller_test.rb | 52 --------------- test/integration/routing_test.rb | 8 +-- 10 files changed, 131 insertions(+), 93 deletions(-) create mode 100644 app/controllers/gantts_controller.rb rename app/views/{issues/gantt.rhtml => gantts/show.html.erb} (100%) create mode 100644 test/functional/gantts_controller_test.rb 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 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 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 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 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 similarity index 100% rename from app/views/issues/gantt.rhtml rename to 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) %>
<% 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 %> <%= 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 %>

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 -- 2.39.5