Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactor: Extract a new IssueMovesController from IssuesController.
git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3936 e93f8b46-1217-0410-a6f0-8f06a7374b81
- Loading branch information
Showing
12 changed files
with
217 additions
and
164 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| class IssueMovesController < ApplicationController | ||
| default_search_scope :issues | ||
| before_filter :find_issues | ||
| before_filter :authorize | ||
|
|
||
| def new | ||
| prepare_for_issue_move | ||
| render :layout => false if request.xhr? | ||
| end | ||
|
|
||
| def create | ||
| prepare_for_issue_move | ||
|
|
||
| if request.post? | ||
| new_tracker = params[:new_tracker_id].blank? ? nil : @target_project.trackers.find_by_id(params[:new_tracker_id]) | ||
| unsaved_issue_ids = [] | ||
| moved_issues = [] | ||
| @issues.each do |issue| | ||
| issue.reload | ||
| issue.init_journal(User.current) | ||
| call_hook(:controller_issues_move_before_save, { :params => params, :issue => issue, :target_project => @target_project, :copy => !!@copy }) | ||
| if r = issue.move_to_project(@target_project, new_tracker, {:copy => @copy, :attributes => extract_changed_attributes_for_move(params)}) | ||
| moved_issues << r | ||
| else | ||
| unsaved_issue_ids << issue.id | ||
| end | ||
| end | ||
| set_flash_from_bulk_issue_save(@issues, unsaved_issue_ids) | ||
|
|
||
| if params[:follow] | ||
| if @issues.size == 1 && moved_issues.size == 1 | ||
| redirect_to :controller => 'issues', :action => 'show', :id => moved_issues.first | ||
| else | ||
| redirect_to :controller => 'issues', :action => 'index', :project_id => (@target_project || @project) | ||
| end | ||
| else | ||
| redirect_to :controller => 'issues', :action => 'index', :project_id => @project | ||
| end | ||
| return | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def prepare_for_issue_move | ||
| @issues.sort! | ||
| @copy = params[:copy_options] && params[:copy_options][:copy] | ||
| @allowed_projects = Issue.allowed_target_projects_on_move | ||
| @target_project = @allowed_projects.detect {|p| p.id.to_s == params[:new_project_id]} if params[:new_project_id] | ||
| @target_project ||= @project | ||
| @trackers = @target_project.trackers | ||
| @available_statuses = Workflow.available_statuses(@project) | ||
| end | ||
|
|
||
| # Filter for bulk operations | ||
| # TODO: duplicated in IssuesController | ||
| def find_issues | ||
| @issues = Issue.find_all_by_id(params[:id] || params[:ids]) | ||
| raise ActiveRecord::RecordNotFound if @issues.empty? | ||
| projects = @issues.collect(&:project).compact.uniq | ||
| if projects.size == 1 | ||
| @project = projects.first | ||
| else | ||
| # TODO: let users bulk edit/move/destroy issues from different projects | ||
| render_error 'Can not bulk edit/move/destroy issues from different projects' | ||
| return false | ||
| end | ||
| rescue ActiveRecord::RecordNotFound | ||
| render_404 | ||
| end | ||
|
|
||
| # TODO: duplicated in IssuesController | ||
| def set_flash_from_bulk_issue_save(issues, unsaved_issue_ids) | ||
| if unsaved_issue_ids.empty? | ||
| flash[:notice] = l(:notice_successful_update) unless issues.empty? | ||
| else | ||
| flash[:error] = l(:notice_failed_to_save_issues, | ||
| :count => unsaved_issue_ids.size, | ||
| :total => issues.size, | ||
| :ids => '#' + unsaved_issue_ids.join(', #')) | ||
| end | ||
| end | ||
|
|
||
| def extract_changed_attributes_for_move(params) | ||
| changed_attributes = {} | ||
| [:assigned_to_id, :status_id, :start_date, :due_date].each do |valid_attribute| | ||
| unless params[valid_attribute].blank? | ||
| changed_attributes[valid_attribute] = (params[valid_attribute] == 'none' ? nil : params[valid_attribute]) | ||
| end | ||
| end | ||
| changed_attributes | ||
| end | ||
|
|
||
| end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| module IssueMovesHelper | ||
| end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| require 'test_helper' | ||
|
|
||
| class IssueMovesControllerTest < ActionController::TestCase | ||
| fixtures :all | ||
|
|
||
| def setup | ||
| User.current = nil | ||
| end | ||
|
|
||
| def test_create_one_issue_to_another_project | ||
| @request.session[:user_id] = 2 | ||
| post :create, :id => 1, :new_project_id => 2, :tracker_id => '', :assigned_to_id => '', :status_id => '', :start_date => '', :due_date => '' | ||
| assert_redirected_to :controller => 'issues', :action => 'index', :project_id => 'ecookbook' | ||
| assert_equal 2, Issue.find(1).project_id | ||
| end | ||
|
|
||
| def test_create_one_issue_to_another_project_should_follow_when_needed | ||
| @request.session[:user_id] = 2 | ||
| post :create, :id => 1, :new_project_id => 2, :follow => '1' | ||
| assert_redirected_to '/issues/1' | ||
| end | ||
|
|
||
| def test_bulk_create_to_another_project | ||
| @request.session[:user_id] = 2 | ||
| post :create, :ids => [1, 2], :new_project_id => 2 | ||
| assert_redirected_to :controller => 'issues', :action => 'index', :project_id => 'ecookbook' | ||
| # Issues moved to project 2 | ||
| assert_equal 2, Issue.find(1).project_id | ||
| assert_equal 2, Issue.find(2).project_id | ||
| # No tracker change | ||
| assert_equal 1, Issue.find(1).tracker_id | ||
| assert_equal 2, Issue.find(2).tracker_id | ||
| end | ||
|
|
||
| def test_bulk_create_to_another_tracker | ||
| @request.session[:user_id] = 2 | ||
| post :create, :ids => [1, 2], :new_tracker_id => 2 | ||
| assert_redirected_to :controller => 'issues', :action => 'index', :project_id => 'ecookbook' | ||
| assert_equal 2, Issue.find(1).tracker_id | ||
| assert_equal 2, Issue.find(2).tracker_id | ||
| end | ||
|
|
||
| def test_bulk_copy_to_another_project | ||
| @request.session[:user_id] = 2 | ||
| assert_difference 'Issue.count', 2 do | ||
| assert_no_difference 'Project.find(1).issues.count' do | ||
| post :create, :ids => [1, 2], :new_project_id => 2, :copy_options => {:copy => '1'} | ||
| end | ||
| end | ||
| assert_redirected_to 'projects/ecookbook/issues' | ||
| end | ||
|
|
||
| context "#create via bulk copy" do | ||
| should "allow not changing the issue's attributes" do | ||
| @request.session[:user_id] = 2 | ||
| issue_before_move = Issue.find(1) | ||
| assert_difference 'Issue.count', 1 do | ||
| assert_no_difference 'Project.find(1).issues.count' do | ||
| post :create, :ids => [1], :new_project_id => 2, :copy_options => {:copy => '1'}, :new_tracker_id => '', :assigned_to_id => '', :status_id => '', :start_date => '', :due_date => '' | ||
| end | ||
| end | ||
| issue_after_move = Issue.first(:order => 'id desc', :conditions => {:project_id => 2}) | ||
| assert_equal issue_before_move.tracker_id, issue_after_move.tracker_id | ||
| assert_equal issue_before_move.status_id, issue_after_move.status_id | ||
| assert_equal issue_before_move.assigned_to_id, issue_after_move.assigned_to_id | ||
| end | ||
|
|
||
| should "allow changing the issue's attributes" do | ||
| # Fixes random test failure with Mysql | ||
| # where Issue.all(:limit => 2, :order => 'id desc', :conditions => {:project_id => 2}) doesn't return the expected results | ||
| Issue.delete_all("project_id=2") | ||
|
|
||
| @request.session[:user_id] = 2 | ||
| assert_difference 'Issue.count', 2 do | ||
| assert_no_difference 'Project.find(1).issues.count' do | ||
| post :create, :ids => [1, 2], :new_project_id => 2, :copy_options => {:copy => '1'}, :new_tracker_id => '', :assigned_to_id => 4, :status_id => 3, :start_date => '2009-12-01', :due_date => '2009-12-31' | ||
| end | ||
| end | ||
|
|
||
| copied_issues = Issue.all(:limit => 2, :order => 'id desc', :conditions => {:project_id => 2}) | ||
| assert_equal 2, copied_issues.size | ||
| copied_issues.each do |issue| | ||
| assert_equal 2, issue.project_id, "Project is incorrect" | ||
| assert_equal 4, issue.assigned_to_id, "Assigned to is incorrect" | ||
| assert_equal 3, issue.status_id, "Status is incorrect" | ||
| assert_equal '2009-12-01', issue.start_date.to_s, "Start date is incorrect" | ||
| assert_equal '2009-12-31', issue.due_date.to_s, "Due date is incorrect" | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def test_copy_to_another_project_should_follow_when_needed | ||
| @request.session[:user_id] = 2 | ||
| post :create, :ids => [1], :new_project_id => 2, :copy_options => {:copy => '1'}, :follow => '1' | ||
| issue = Issue.first(:order => 'id DESC') | ||
| assert_redirected_to :controller => 'issues', :action => 'show', :id => issue | ||
| end | ||
|
|
||
| end |
Oops, something went wrong.
ff43bb1There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't you have just put this in the issues_helper.rb instead of creating a whole new controller for it?
ff43bb1There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, if this was put into a helper there wouldn't be any actions that a browser could hit (move/perform_move before, new/create after).
ff43bb1There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why couldn't those actions just remain inside issue_controller.rb?