From a188abbe2813372d426afd2ab05841f0503f00c1 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Mon, 30 Aug 2010 15:30:28 +0000 Subject: [PATCH] Refactor: move method, ProjectsController#roadmap to VersionsController#index. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4050 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/projects_controller.rb | 32 --------------- app/controllers/versions_controller.rb | 39 +++++++++++++++++-- .../roadmap.rhtml => versions/index.html.erb} | 0 config/routes.rb | 3 +- lib/redmine.rb | 7 ++-- test/functional/projects_controller_test.rb | 32 --------------- test/functional/versions_controller_test.rb | 33 ++++++++++++++++ test/integration/routing_test.rb | 2 +- 8 files changed, 75 insertions(+), 73 deletions(-) rename app/views/{projects/roadmap.rhtml => versions/index.html.erb} (100%) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 6bb766386..845ab3c15 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -287,30 +287,6 @@ class ProjectsController < ApplicationController render :layout => !request.xhr? end - def roadmap - @trackers = @project.trackers.find(:all, :order => 'position') - retrieve_selected_tracker_ids(@trackers, @trackers.select {|t| t.is_in_roadmap?}) - @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_issues? : (params[:with_subprojects] == '1') - project_ids = @with_subprojects ? @project.self_and_descendants.collect(&:id) : [@project.id] - - @versions = @project.shared_versions || [] - @versions += @project.rolled_up_versions.visible if @with_subprojects - @versions = @versions.uniq.sort - @versions.reject! {|version| version.closed? || version.completed? } unless params[:completed] - - @issues_by_version = {} - unless @selected_tracker_ids.empty? - @versions.each do |version| - issues = version.fixed_issues.visible.find(:all, - :include => [:project, :status, :tracker, :priority], - :conditions => {:tracker_id => @selected_tracker_ids, :project_id => project_ids}, - :order => "#{Project.table_name}.lft, #{Tracker.table_name}.position, #{Issue.table_name}.id") - @issues_by_version[version] = issues - end - end - @versions.reject! {|version| !project_ids.include?(version.project_id) && @issues_by_version[version].blank?} - end - private def find_optional_project return true unless params[:id] @@ -320,14 +296,6 @@ private render_404 end - def retrieve_selected_tracker_ids(selectable_trackers, default_trackers=nil) - if ids = params[:tracker_ids] - @selected_tracker_ids = (ids.is_a? Array) ? ids.collect { |id| id.to_i.to_s } : ids.split('/').collect { |id| id.to_i.to_s } - else - @selected_tracker_ids = (default_trackers || selectable_trackers).collect {|t| t.id.to_s } - end - end - # Validates parent_id param according to user's permissions # TODO: move it to Project model in a validation that depends on User.current def validate_parent_id diff --git a/app/controllers/versions_controller.rb b/app/controllers/versions_controller.rb index 46b4778d4..dd01da95b 100644 --- a/app/controllers/versions_controller.rb +++ b/app/controllers/versions_controller.rb @@ -18,13 +18,37 @@ class VersionsController < ApplicationController menu_item :roadmap model_object Version - before_filter :find_model_object, :except => [:new, :close_completed] - before_filter :find_project_from_association, :except => [:new, :close_completed] - before_filter :find_project, :only => [:new, :close_completed] + before_filter :find_model_object, :except => [:index, :new, :close_completed] + before_filter :find_project_from_association, :except => [:index, :new, :close_completed] + before_filter :find_project, :only => [:index, :new, :close_completed] before_filter :authorize helper :custom_fields helper :projects + + def index + @trackers = @project.trackers.find(:all, :order => 'position') + retrieve_selected_tracker_ids(@trackers, @trackers.select {|t| t.is_in_roadmap?}) + @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_issues? : (params[:with_subprojects] == '1') + project_ids = @with_subprojects ? @project.self_and_descendants.collect(&:id) : [@project.id] + + @versions = @project.shared_versions || [] + @versions += @project.rolled_up_versions.visible if @with_subprojects + @versions = @versions.uniq.sort + @versions.reject! {|version| version.closed? || version.completed? } unless params[:completed] + + @issues_by_version = {} + unless @selected_tracker_ids.empty? + @versions.each do |version| + issues = version.fixed_issues.visible.find(:all, + :include => [:project, :status, :tracker, :priority], + :conditions => {:tracker_id => @selected_tracker_ids, :project_id => project_ids}, + :order => "#{Project.table_name}.lft, #{Tracker.table_name}.position, #{Issue.table_name}.id") + @issues_by_version[version] = issues + end + end + @versions.reject! {|version| !project_ids.include?(version.project_id) && @issues_by_version[version].blank?} + end def show @issues = @version.fixed_issues.visible.find(:all, @@ -105,4 +129,13 @@ private rescue ActiveRecord::RecordNotFound render_404 end + + def retrieve_selected_tracker_ids(selectable_trackers, default_trackers=nil) + if ids = params[:tracker_ids] + @selected_tracker_ids = (ids.is_a? Array) ? ids.collect { |id| id.to_i.to_s } : ids.split('/').collect { |id| id.to_i.to_s } + else + @selected_tracker_ids = (default_trackers || selectable_trackers).collect {|t| t.id.to_s } + end + end + end diff --git a/app/views/projects/roadmap.rhtml b/app/views/versions/index.html.erb similarity index 100% rename from app/views/projects/roadmap.rhtml rename to app/views/versions/index.html.erb diff --git a/config/routes.rb b/config/routes.rb index 1465e1ffe..1414e1000 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -180,7 +180,7 @@ ActionController::Routing::Routes.draw do |map| project_views.connect 'projects/new', :action => 'add' project_views.connect 'projects/:id', :action => 'show' project_views.connect 'projects/:id.:format', :action => 'show' - project_views.connect 'projects/:id/:action', :action => /roadmap|destroy|settings/ + project_views.connect 'projects/:id/:action', :action => /destroy|settings/ project_views.connect 'projects/:id/files', :action => 'list_files' project_views.connect 'projects/:id/files/new', :action => 'add_file' project_views.connect 'projects/:id/settings/:tab', :action => 'settings' @@ -215,6 +215,7 @@ ActionController::Routing::Routes.draw do |map| map.with_options :controller => 'versions' do |versions| versions.connect 'projects/:project_id/versions/new', :action => 'new' + versions.connect 'projects/:project_id/roadmap', :action => 'index' versions.with_options :conditions => {:method => :post} do |version_actions| version_actions.connect 'projects/:project_id/versions/close_completed', :action => 'close_completed' end diff --git a/lib/redmine.rb b/lib/redmine.rb index 7040a1f99..ecfc46252 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -57,11 +57,10 @@ Redmine::AccessControl.map do |map| # Issue categories map.permission :manage_categories, {:projects => :settings, :issue_categories => [:new, :edit, :destroy]}, :require => :member # Issues - map.permission :view_issues, {:projects => :roadmap, - :issues => [:index, :show], + map.permission :view_issues, {:issues => [:index, :show], :auto_complete => [:issues], :context_menus => [:issues], - :versions => [:show, :status_by], + :versions => [:index, :show, :status_by], :journals => :index, :queries => :index, :reports => [:issue_report, :issue_report_details]} @@ -186,7 +185,7 @@ end Redmine::MenuManager.map :project_menu do |menu| menu.push :overview, { :controller => 'projects', :action => 'show' } menu.push :activity, { :controller => 'activities', :action => 'index' } - menu.push :roadmap, { :controller => 'projects', :action => 'roadmap' }, + menu.push :roadmap, { :controller => 'versions', :action => 'index' }, :param => :project_id, :if => Proc.new { |p| p.shared_versions.any? } menu.push :issues, { :controller => 'issues', :action => 'index' }, :param => :project_id, :caption => :label_issue_plural menu.push :new_issue, { :controller => 'issues', :action => 'new' }, :param => :project_id, :caption => :label_issue_new, diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index f9c8ce022..36d60c859 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -368,38 +368,6 @@ class ProjectsControllerTest < ActionController::TestCase :attributes => { :href => '/attachments/download/9/version_file.zip' } end - def test_roadmap - get :roadmap, :id => 1 - assert_response :success - assert_template 'roadmap' - assert_not_nil assigns(:versions) - # Version with no date set appears - assert assigns(:versions).include?(Version.find(3)) - # Completed version doesn't appear - assert !assigns(:versions).include?(Version.find(1)) - end - - def test_roadmap_with_completed_versions - get :roadmap, :id => 1, :completed => 1 - assert_response :success - assert_template 'roadmap' - assert_not_nil assigns(:versions) - # Version with no date set appears - assert assigns(:versions).include?(Version.find(3)) - # Completed version appears - assert assigns(:versions).include?(Version.find(1)) - end - - def test_roadmap_showing_subprojects_versions - @subproject_version = Version.generate!(:project => Project.find(3)) - get :roadmap, :id => 1, :with_subprojects => 1 - assert_response :success - assert_template 'roadmap' - assert_not_nil assigns(:versions) - - assert assigns(:versions).include?(Version.find(4)), "Shared version not found" - assert assigns(:versions).include?(@subproject_version), "Subproject version not found" - end def test_archive @request.session[:user_id] = 1 # admin post :archive, :id => 1 diff --git a/test/functional/versions_controller_test.rb b/test/functional/versions_controller_test.rb index e4864c908..5fcb1d05a 100644 --- a/test/functional/versions_controller_test.rb +++ b/test/functional/versions_controller_test.rb @@ -31,6 +31,39 @@ class VersionsControllerTest < ActionController::TestCase User.current = nil end + def test_index + get :index, :project_id => 1 + assert_response :success + assert_template 'index' + assert_not_nil assigns(:versions) + # Version with no date set appears + assert assigns(:versions).include?(Version.find(3)) + # Completed version doesn't appear + assert !assigns(:versions).include?(Version.find(1)) + end + + def test_index_with_completed_versions + get :index, :project_id => 1, :completed => 1 + assert_response :success + assert_template 'index' + assert_not_nil assigns(:versions) + # Version with no date set appears + assert assigns(:versions).include?(Version.find(3)) + # Completed version appears + assert assigns(:versions).include?(Version.find(1)) + end + + def test_index_showing_subprojects_versions + @subproject_version = Version.generate!(:project => Project.find(3)) + get :index, :project_id => 1, :with_subprojects => 1 + assert_response :success + assert_template 'index' + assert_not_nil assigns(:versions) + + assert assigns(:versions).include?(Version.find(4)), "Shared version not found" + assert assigns(:versions).include?(@subproject_version), "Subproject version not found" + end + def test_show get :show, :id => 2 assert_response :success diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index 62fdaf47a..590bd9119 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -174,7 +174,7 @@ class RoutingTest < ActionController::IntegrationTest should_route :get, "/projects/567/destroy", :controller => 'projects', :action => 'destroy', :id => '567' should_route :get, "/projects/33/files", :controller => 'projects', :action => 'list_files', :id => '33' should_route :get, "/projects/33/files/new", :controller => 'projects', :action => 'add_file', :id => '33' - should_route :get, "/projects/33/roadmap", :controller => 'projects', :action => 'roadmap', :id => '33' + should_route :get, "/projects/33/roadmap", :controller => 'versions', :action => 'index', :project_id => '33' should_route :get, "/projects/33/activity", :controller => 'activities', :action => 'index', :id => '33' should_route :get, "/projects/33/activity.atom", :controller => 'activities', :action => 'index', :id => '33', :format => 'atom'