From 1fcc1bdc89ef4e92e4e02d917593a8ef2b4f370e Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Thu, 19 Aug 2010 18:16:54 +0000 Subject: [PATCH] Refactor: move IssuesController#context_menu to a new controller. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4006 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/context_menus_controller.rb | 33 +++++++ app/controllers/issues_controller.rb | 31 +------ .../issues.html.erb} | 0 app/views/issues/index.rhtml | 2 +- app/views/issues/show.rhtml | 2 +- config/routes.rb | 1 + lib/redmine.rb | 3 +- .../context_menus_controller_test.rb | 89 +++++++++++++++++++ test/functional/issues_controller_test.rb | 83 ----------------- test/integration/routing_test.rb | 2 + 10 files changed, 130 insertions(+), 116 deletions(-) create mode 100644 app/controllers/context_menus_controller.rb rename app/views/{issues/context_menu.rhtml => context_menus/issues.html.erb} (100%) create mode 100644 test/functional/context_menus_controller_test.rb diff --git a/app/controllers/context_menus_controller.rb b/app/controllers/context_menus_controller.rb new file mode 100644 index 000000000..442f85b37 --- /dev/null +++ b/app/controllers/context_menus_controller.rb @@ -0,0 +1,33 @@ +class ContextMenusController < ApplicationController + helper :watchers + + def issues + @issues = Issue.find_all_by_id(params[:ids], :include => :project) + if (@issues.size == 1) + @issue = @issues.first + @allowed_statuses = @issue.new_statuses_allowed_to(User.current) + end + projects = @issues.collect(&:project).compact.uniq + @project = projects.first if projects.size == 1 + + @can = {:edit => (@project && User.current.allowed_to?(:edit_issues, @project)), + :log_time => (@project && User.current.allowed_to?(:log_time, @project)), + :update => (@project && (User.current.allowed_to?(:edit_issues, @project) || (User.current.allowed_to?(:change_status, @project) && @allowed_statuses && !@allowed_statuses.empty?))), + :move => (@project && User.current.allowed_to?(:move_issues, @project)), + :copy => (@issue && @project.trackers.include?(@issue.tracker) && User.current.allowed_to?(:add_issues, @project)), + :delete => (@project && User.current.allowed_to?(:delete_issues, @project)) + } + if @project + @assignables = @project.assignable_users + @assignables << @issue.assigned_to if @issue && @issue.assigned_to && !@assignables.include?(@issue.assigned_to) + @trackers = @project.trackers + end + + @priorities = IssuePriority.all.reverse + @statuses = IssueStatus.find(:all, :order => 'position') + @back = back_url + + render :layout => false + end + +end diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index a0264bc56..83c2f2112 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -22,7 +22,7 @@ class IssuesController < ApplicationController before_filter :find_issue, :only => [:show, :edit, :update] before_filter :find_issues, :only => [:bulk_edit, :move, :perform_move, :destroy] before_filter :find_project, :only => [:new, :create, :update_form] - before_filter :authorize, :except => [:index, :changes, :context_menu] + before_filter :authorize, :except => [:index, :changes] before_filter :find_optional_project, :only => [:index, :changes] before_filter :check_for_default_issue_status, :only => [:new, :create] before_filter :build_new_issue_from_params, :only => [:new, :create] @@ -257,35 +257,6 @@ class IssuesController < ApplicationController format.json { head :ok } end end - - def context_menu - @issues = Issue.find_all_by_id(params[:ids], :include => :project) - if (@issues.size == 1) - @issue = @issues.first - @allowed_statuses = @issue.new_statuses_allowed_to(User.current) - end - projects = @issues.collect(&:project).compact.uniq - @project = projects.first if projects.size == 1 - - @can = {:edit => (@project && User.current.allowed_to?(:edit_issues, @project)), - :log_time => (@project && User.current.allowed_to?(:log_time, @project)), - :update => (@project && (User.current.allowed_to?(:edit_issues, @project) || (User.current.allowed_to?(:change_status, @project) && @allowed_statuses && !@allowed_statuses.empty?))), - :move => (@project && User.current.allowed_to?(:move_issues, @project)), - :copy => (@issue && @project.trackers.include?(@issue.tracker) && User.current.allowed_to?(:add_issues, @project)), - :delete => (@project && User.current.allowed_to?(:delete_issues, @project)) - } - if @project - @assignables = @project.assignable_users - @assignables << @issue.assigned_to if @issue && @issue.assigned_to && !@assignables.include?(@issue.assigned_to) - @trackers = @project.trackers - end - - @priorities = IssuePriority.all.reverse - @statuses = IssueStatus.find(:all, :order => 'position') - @back = back_url - - render :layout => false - end def update_form if params[:id].blank? diff --git a/app/views/issues/context_menu.rhtml b/app/views/context_menus/issues.html.erb similarity index 100% rename from app/views/issues/context_menu.rhtml rename to app/views/context_menus/issues.html.erb diff --git a/app/views/issues/index.rhtml b/app/views/issues/index.rhtml index 1778f4d64..04298b5ca 100644 --- a/app/views/issues/index.rhtml +++ b/app/views/issues/index.rhtml @@ -81,4 +81,4 @@ <%= auto_discovery_link_tag(:atom, {:action => 'changes', :query_id => @query, :format => 'atom', :page => nil, :key => User.current.rss_key}, :title => l(:label_changes_details)) %> <% end %> -<%= context_menu :controller => 'issues', :action => 'context_menu' %> +<%= context_menu issues_context_menu_path %> diff --git a/app/views/issues/show.rhtml b/app/views/issues/show.rhtml index b48ff2cd1..83652b3e0 100644 --- a/app/views/issues/show.rhtml +++ b/app/views/issues/show.rhtml @@ -130,4 +130,4 @@ <%= stylesheet_link_tag 'context_menu' %> <% end %> -<%= javascript_tag "new ContextMenu('#{url_for(:controller => 'issues', :action => 'context_menu')}')" %> +<%= javascript_tag "new ContextMenu('#{issues_context_menu_path}')" %> diff --git a/config/routes.rb b/config/routes.rb index 956ce054b..7b22328bf 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -107,6 +107,7 @@ ActionController::Routing::Routes.draw do |map| map.auto_complete_issues '/issues/auto_complete', :controller => 'auto_completes', :action => 'issues' # TODO: would look nicer as /issues/:id/preview map.preview_issue '/issues/preview/:id', :controller => 'previews', :action => 'issue' + map.issues_context_menu '/issues/context_menu', :controller => 'context_menus', :action => 'issues' map.with_options :controller => 'issues' do |issues_routes| issues_routes.with_options :conditions => {:method => :get} do |issues_views| diff --git a/lib/redmine.rb b/lib/redmine.rb index aa9fe75d1..dc18ca280 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -58,8 +58,9 @@ Redmine::AccessControl.map do |map| map.permission :manage_categories, {:projects => :settings, :issue_categories => [:new, :edit, :destroy]}, :require => :member # Issues map.permission :view_issues, {:projects => :roadmap, - :issues => [:index, :changes, :show, :context_menu], + :issues => [:index, :changes, :show], :auto_complete => [:issues], + :context_menus => [:issues], :versions => [:show, :status_by], :queries => :index, :reports => [:issue_report, :issue_report_details]} diff --git a/test/functional/context_menus_controller_test.rb b/test/functional/context_menus_controller_test.rb new file mode 100644 index 000000000..eee9dc608 --- /dev/null +++ b/test/functional/context_menus_controller_test.rb @@ -0,0 +1,89 @@ +require File.dirname(__FILE__) + '/../test_helper' + +class ContextMenusControllerTest < ActionController::TestCase + fixtures :all + + def test_context_menu_one_issue + @request.session[:user_id] = 2 + get :issues, :ids => [1] + assert_response :success + assert_template 'context_menu' + assert_tag :tag => 'a', :content => 'Edit', + :attributes => { :href => '/issues/1/edit', + :class => 'icon-edit' } + assert_tag :tag => 'a', :content => 'Closed', + :attributes => { :href => '/issues/1/edit?issue%5Bstatus_id%5D=5', + :class => '' } + assert_tag :tag => 'a', :content => 'Immediate', + :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&issue%5Bpriority_id%5D=8', + :class => '' } + # Versions + assert_tag :tag => 'a', :content => '2.0', + :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&issue%5Bfixed_version_id%5D=3', + :class => '' } + assert_tag :tag => 'a', :content => 'eCookbook Subproject 1 - 2.0', + :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&issue%5Bfixed_version_id%5D=4', + :class => '' } + + assert_tag :tag => 'a', :content => 'Dave Lopper', + :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&issue%5Bassigned_to_id%5D=3', + :class => '' } + assert_tag :tag => 'a', :content => 'Duplicate', + :attributes => { :href => '/projects/ecookbook/issues/1/copy', + :class => 'icon-duplicate' } + assert_tag :tag => 'a', :content => 'Copy', + :attributes => { :href => '/issues/move/new?copy_options%5Bcopy%5D=t&ids%5B%5D=1', + :class => 'icon-copy' } + assert_tag :tag => 'a', :content => 'Move', + :attributes => { :href => '/issues/move/new?ids%5B%5D=1', + :class => 'icon-move' } + assert_tag :tag => 'a', :content => 'Delete', + :attributes => { :href => '/issues/destroy?ids%5B%5D=1', + :class => 'icon-del' } + end + + def test_context_menu_one_issue_by_anonymous + get :issues, :ids => [1] + assert_response :success + assert_template 'context_menu' + assert_tag :tag => 'a', :content => 'Delete', + :attributes => { :href => '#', + :class => 'icon-del disabled' } + end + + def test_context_menu_multiple_issues_of_same_project + @request.session[:user_id] = 2 + get :issues, :ids => [1, 2] + assert_response :success + assert_template 'context_menu' + assert_tag :tag => 'a', :content => 'Edit', + :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&ids%5B%5D=2', + :class => 'icon-edit' } + assert_tag :tag => 'a', :content => 'Immediate', + :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&ids%5B%5D=2&issue%5Bpriority_id%5D=8', + :class => '' } + assert_tag :tag => 'a', :content => 'Dave Lopper', + :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&ids%5B%5D=2&issue%5Bassigned_to_id%5D=3', + :class => '' } + assert_tag :tag => 'a', :content => 'Copy', + :attributes => { :href => '/issues/move/new?copy_options%5Bcopy%5D=t&ids%5B%5D=1&ids%5B%5D=2', + :class => 'icon-copy' } + assert_tag :tag => 'a', :content => 'Move', + :attributes => { :href => '/issues/move/new?ids%5B%5D=1&ids%5B%5D=2', + :class => 'icon-move' } + assert_tag :tag => 'a', :content => 'Delete', + :attributes => { :href => '/issues/destroy?ids%5B%5D=1&ids%5B%5D=2', + :class => 'icon-del' } + end + + def test_context_menu_multiple_issues_of_different_project + @request.session[:user_id] = 2 + get :issues, :ids => [1, 2, 4] + assert_response :success + assert_template 'context_menu' + assert_tag :tag => 'a', :content => 'Delete', + :attributes => { :href => '#', + :class => 'icon-del disabled' } + end + +end diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 22f528b7b..d52f63834 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1024,89 +1024,6 @@ class IssuesControllerTest < ActionController::TestCase assert_redirected_to :controller => 'issues', :action => 'index', :project_id => Project.find(1).identifier end - def test_context_menu_one_issue - @request.session[:user_id] = 2 - get :context_menu, :ids => [1] - assert_response :success - assert_template 'context_menu' - assert_tag :tag => 'a', :content => 'Edit', - :attributes => { :href => '/issues/1/edit', - :class => 'icon-edit' } - assert_tag :tag => 'a', :content => 'Closed', - :attributes => { :href => '/issues/1/edit?issue%5Bstatus_id%5D=5', - :class => '' } - assert_tag :tag => 'a', :content => 'Immediate', - :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&issue%5Bpriority_id%5D=8', - :class => '' } - # Versions - assert_tag :tag => 'a', :content => '2.0', - :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&issue%5Bfixed_version_id%5D=3', - :class => '' } - assert_tag :tag => 'a', :content => 'eCookbook Subproject 1 - 2.0', - :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&issue%5Bfixed_version_id%5D=4', - :class => '' } - - assert_tag :tag => 'a', :content => 'Dave Lopper', - :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&issue%5Bassigned_to_id%5D=3', - :class => '' } - assert_tag :tag => 'a', :content => 'Duplicate', - :attributes => { :href => '/projects/ecookbook/issues/1/copy', - :class => 'icon-duplicate' } - assert_tag :tag => 'a', :content => 'Copy', - :attributes => { :href => '/issues/move/new?copy_options%5Bcopy%5D=t&ids%5B%5D=1', - :class => 'icon-copy' } - assert_tag :tag => 'a', :content => 'Move', - :attributes => { :href => '/issues/move/new?ids%5B%5D=1', - :class => 'icon-move' } - assert_tag :tag => 'a', :content => 'Delete', - :attributes => { :href => '/issues/destroy?ids%5B%5D=1', - :class => 'icon-del' } - end - - def test_context_menu_one_issue_by_anonymous - get :context_menu, :ids => [1] - assert_response :success - assert_template 'context_menu' - assert_tag :tag => 'a', :content => 'Delete', - :attributes => { :href => '#', - :class => 'icon-del disabled' } - end - - def test_context_menu_multiple_issues_of_same_project - @request.session[:user_id] = 2 - get :context_menu, :ids => [1, 2] - assert_response :success - assert_template 'context_menu' - assert_tag :tag => 'a', :content => 'Edit', - :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&ids%5B%5D=2', - :class => 'icon-edit' } - assert_tag :tag => 'a', :content => 'Immediate', - :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&ids%5B%5D=2&issue%5Bpriority_id%5D=8', - :class => '' } - assert_tag :tag => 'a', :content => 'Dave Lopper', - :attributes => { :href => '/issues/bulk_edit?ids%5B%5D=1&ids%5B%5D=2&issue%5Bassigned_to_id%5D=3', - :class => '' } - assert_tag :tag => 'a', :content => 'Copy', - :attributes => { :href => '/issues/move/new?copy_options%5Bcopy%5D=t&ids%5B%5D=1&ids%5B%5D=2', - :class => 'icon-copy' } - assert_tag :tag => 'a', :content => 'Move', - :attributes => { :href => '/issues/move/new?ids%5B%5D=1&ids%5B%5D=2', - :class => 'icon-move' } - assert_tag :tag => 'a', :content => 'Delete', - :attributes => { :href => '/issues/destroy?ids%5B%5D=1&ids%5B%5D=2', - :class => 'icon-del' } - end - - def test_context_menu_multiple_issues_of_different_project - @request.session[:user_id] = 2 - get :context_menu, :ids => [1, 2, 4] - assert_response :success - assert_template 'context_menu' - assert_tag :tag => 'a', :content => 'Delete', - :attributes => { :href => '#', - :class => 'icon-del disabled' } - end - def test_destroy_issue_with_no_time_entries assert_nil TimeEntry.find_by_issue_id(2) @request.session[:user_id] = 2 diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index 01a96b7aa..63293d594 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -104,6 +104,8 @@ class RoutingTest < ActionController::IntegrationTest should_route :get, "/issues/preview/123", :controller => 'previews', :action => 'issue', :id => '123' should_route :post, "/issues/preview/123", :controller => 'previews', :action => 'issue', :id => '123' + should_route :get, "/issues/context_menu", :controller => 'context_menus', :action => 'issues' + should_route :post, "/issues/context_menu", :controller => 'context_menus', :action => 'issues' end context "issue categories" do -- 2.39.5