diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -181,6 +181,68 @@ class IssuesController < ApplicationCont end def update + if request.xhr? + if params[:check_go_to_close_confirm] + result = true + status = nil + status_id = params[:status_id].to_i + can_close_all_descendants = true + if status_id <= 0 + result = false + else + status = IssueStatus.find(status_id) + if !status.is_closed + result = false + else + opened_descendants = [] + @issue.descendants.open.sort_by(&:lft).each do |issue| + unless issue.visible? + can_close_all_descendants = false + break + end + + unless issue.new_statuses_allowed_to.map{|i| i.id}.include?(status.id) + can_close_all_descendants = false + break + end + + opened_descendants << issue + end + if can_close_all_descendants && opened_descendants.empty? + result = false + can_close_all_descendants = false + end + end + if result + session[:can_close_descendant] = {} + session[:can_close_descendant][:can_close_all] = can_close_all_descendants + if can_close_all_descendants + session[:can_close_descendant][:status_id] = status.id + session[:can_close_descendant][:status_name] = status.name + session[:can_close_descendant][:ids] = opened_descendants.map{|i| i.id} + end + end + end + render :json => {:result => result} + return + end + + if !session.has_key?(:can_close_descendant) + return + end + + render_data = { + :status_name => session[:can_close_descendant][:status_name], + :can_close_all => session[:can_close_descendant][:can_close_all] + } + render( + :partial => 'close_confirm', + :layout => false, + :locals => render_data + ) + return + end + return unless update_issue_from_params @issue.save_attachments(params[:attachments] || @@ -200,6 +262,17 @@ class IssuesController < ApplicationCont if saved render_attachment_warning_if_needed(@issue) + if params[:close_descendants] && session.has_key?(:can_close_descendant) + close_descendant_ids = session[:can_close_descendant][:ids] + session.delete(:can_close_descendant) + close_descendant_ids.each do |id| + issue = Issue.find(id) + issue.init_journal(User.current, params[:issue][:notes]) + issue.safe_attributes = {'status_id' => params[:issue][:status_id].to_i} + issue.save + end + end + unless @issue.current_journal.new_record? || params[:no_flash] flash[:notice] = l(:notice_successful_update) end diff --git a/app/views/issues/_close_confirm.js.erb b/app/views/issues/_close_confirm.js.erb new file mode 100644 --- /dev/null +++ b/app/views/issues/_close_confirm.js.erb @@ -0,0 +1,13 @@ +$('#ajax-modal').html( + '<%= escape_javascript( + render( + :partial => 'close_confirm_dialog', + :locals => { + :status_name => status_name, + :can_close_all => can_close_all + } + ) + ) + %>' +); +showModal('ajax-modal', '400px'); diff --git a/app/views/issues/_close_confirm_dialog.html.erb b/app/views/issues/_close_confirm_dialog.html.erb new file mode 100644 --- /dev/null +++ b/app/views/issues/_close_confirm_dialog.html.erb @@ -0,0 +1,43 @@ +

 

+<% if can_close_all %> +

+ <%= l(:text_close_parent_issue_and_open_subtasks, + :issue_id => params[:id]) %> +

+

+ + +

+<% else %> +

+ <%= l(:text_close_parent_issue_with_open_subtasks_confirmation) %> +

+<% end %> +

+ <%= button_tag l(:button_apply), :type => "button", :onclick => "run_submit();" %> + <%= link_to_function l(:button_cancel), "hideModal(this);" %> +

+ +<%= javascript_tag do %> + function run_submit() { + var canCloseAll = <%= can_close_all %>; + if (canCloseAll) { + var radioVal = $("input[name='choice']:checked").val(); + if (radioVal == '1') { + $('').attr('type', 'hidden') + .attr('name', "close_descendants") + .appendTo('#issue-form'); + } + } + $("#issue-form").off('submit'); + $("#issue-form").submit(); + } +<% end %> diff --git a/app/views/issues/_edit.html.erb b/app/views/issues/_edit.html.erb --- a/app/views/issues/_edit.html.erb +++ b/app/views/issues/_edit.html.erb @@ -82,3 +82,34 @@ <%= hidden_field_tag 'issue_position', @issue_position if @issue_position %> <%= hidden_field_tag 'issue_count', @issue_count if @issue_count %> <% end %> +<%= javascript_tag do %> + $('#issue-form').submit(function(){ + var status_id = 0; + if ($("#issue_status_id").length > 0) { + status_id = $("#issue_status_id").val(); + } + $.ajax({ + url: $("#issue-form").attr('action'), + type: 'patch', + data: { + "check_go_to_close_confirm": "", + "status_id": status_id + }, + }) + .then( + function(data){ + if (data["result"]) { + $.ajax({ + url: $("#issue-form").attr('action'), + type: 'patch', + data: {} + }); + } else { + $("#issue-form").off('submit'); + $("#issue-form").submit(); + } + } + ); + return false; + }); +<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1201,6 +1201,10 @@ en: text_time_logged_by_changeset: "Applied in changeset %{value}." text_issues_destroy_confirmation: 'Are you sure you want to delete the selected issue(s)?' text_issues_destroy_descendants_confirmation: "This will also delete %{count} subtask(s)." + text_close_parent_issue_with_open_subtasks_confirmation: Are you sure you want to close parent issue with open subtasks? + text_close_parent_issue_and_open_subtasks: Issue %{issue_id} has open subtasks. + text_close_parent_issue_and_open_subtasks_choice_close_also_all_subtasks: Close all open subtasks by "%{status}" status too + text_close_parent_issue_and_open_subtasks_choice_close_only_parent: Close only issue %{issue_id} text_time_entries_destroy_confirmation: 'Are you sure you want to delete the selected time entr(y/ies)?' text_select_project_modules: 'Select modules to enable for this project:' text_default_administrator_account_changed: Default administrator account changed diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -6370,6 +6370,354 @@ class IssuesControllerTest < Redmine::Co assert_equal 2, issue.reload.assigned_to_id end + test "check_go_to_close_confirm returns false if status_id is not close" do + issue = Issue.find(1) + user = User.find(2) + assert issue.visible?(user) + assert_not issue.closed? + @request.session[:user_id] = user.id + put( + :update, + :params => { + :id => issue.id, + :check_go_to_close_confirm => "", + :status_id => 1 + }, + :xhr => true + ) + assert_response :success + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + assert_equal({"result" => false}, json) + assert_not session.has_key?(:can_close_descendant) + end + + test "check_go_to_close_confirm returns false if status_id is 0" do + issue = Issue.find(1) + user = User.find(2) + assert issue.visible?(user) + assert_not issue.closed? + @request.session[:user_id] = user.id + put( + :update, + :params => { + :id => issue.id, + :check_go_to_close_confirm => "", + :status_id => 0 + }, + :xhr => true + ) + assert_response :success + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + assert_equal({"result" => false}, json) + assert_not session.has_key?(:can_close_descendant) + end + + test "check_go_to_close_confirm returns false if issue does not have child" do + issue = Issue.generate! + user = User.find(2) + assert issue.visible?(user) + assert_not issue.closed? + @request.session[:user_id] = user.id + put( + :update, + :params => { + :id => issue.id, + :check_go_to_close_confirm => "", + :status_id => 5 + }, + :xhr => true + ) + assert_response :success + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + + assert_equal 1, issue.reload.status.id + assert_equal({"result" => false}, json) + assert_not session.has_key?(:can_close_descendant) + end + + test "check_go_to_close_confirm returns true if issue have open child" do + parent = Issue.generate! + child = Issue.generate!(:parent_issue_id => parent.id) + user = User.find(2) + assert parent.reload.visible?(user) + assert_not parent.closed? + assert child.reload.visible?(user) + assert_not child.closed? + + @request.session[:user_id] = user.id + put( + :update, + :params => { + :id => parent.id, + :check_go_to_close_confirm => "", + :status_id => 5 + }, + :xhr => true + ) + assert_response :success + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + + assert_equal 1, parent.reload.status.id + assert_equal({"result" => true}, json) + assert session.has_key?(:can_close_descendant) + end + + test "check_go_to_close_confirm returns false if child is closed" do + parent = Issue.generate! + child = Issue. + generate!( + :parent_issue_id => parent.id, + :status_id => 5 + ) + user = User.find(2) + assert parent.reload.visible?(user) + assert_not parent.closed? + assert child.reload.visible?(user) + assert child.closed? + + @request.session[:user_id] = user.id + put( + :update, + :params => { + :id => parent.id, + :check_go_to_close_confirm => "", + :status_id => 5 + }, + :xhr => true + ) + assert_response :success + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + + assert_equal 1, parent.reload.status.id + assert_equal({"result" => false}, json) + assert_not session.has_key?(:can_close_descendant) + end + + test "check_go_to_close_confirm returns true if child is open and not visible" do + user = User.generate! + project = Project.generate! + role = Role.generate! + role.add_permission! :view_issues, :edit_issues + role.set_permission_trackers :view_issues, [2] + role.set_permission_trackers :edit_issues, [2] + role.save! + User.add_to_project(user, project, role) + parent = Issue. + generate!( + :project => project, + :tracker_id => 2, + :status_id => 1 + ) + child = Issue. + generate!( + :project => project, + :tracker_id => 1, + :parent_issue_id => parent.id, + :status_id => 1 + ) + assert parent.reload.visible?(user) + assert_not parent.closed? + assert_not child.reload.visible?(user) + assert_not child.closed? + + @request.session[:user_id] = user.id + put( + :update, + :params => { + :id => parent.id, + :check_go_to_close_confirm => "", + :status_id => 5 + }, + :xhr => true + ) + assert_response :success + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + + assert_equal 1, parent.reload.status.id + assert_equal({"result" => true}, json) + assert session.has_key?(:can_close_descendant) + assert_not session[:can_close_descendant][:can_close_all] + end + + test "close all descendants which are open" do + with_settings :closed_parent_issues_with_open_subtasks => 1 do + parent = Issue.generate! + child = + Issue. + generate!( + :parent_issue_id => parent.id + ) + grandchild1 = + Issue. + generate!( + :parent_issue_id => child.id + ) + grandchild2 = + Issue. + generate!( + :parent_issue_id => child.id, + :status_id => 6 + ) + user = User.find(2) + assert parent.reload.visible?(user) + assert_not parent.closed? + assert child.reload.visible?(user) + assert_not child.closed? + assert grandchild1.reload.visible?(user) + assert_not grandchild1.closed? + assert grandchild2.reload.visible?(user) + assert grandchild2.closed? + + assert [parent, child, grandchild1, grandchild2]. + map{|i| i.new_statuses_allowed_to(user)}. + reduce(:&).map{|i| i.id}.include?(5) + @request.session[:user_id] = user.id + put( + :update, + :params => { + :id => parent.id, + :check_go_to_close_confirm => "", + :status_id => 5 + }, + :xhr => true + ) + assert_response :success + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + + assert_equal 1, parent.reload.status.id + assert_equal({"result" => true}, json) + assert session.has_key?(:can_close_descendant) + assert session[:can_close_descendant][:can_close_all] + assert_equal [child.id, grandchild1.id], session[:can_close_descendant][:ids] + + notes = 'close all' + + assert_difference( + -> {parent.journals.count} => 1, + -> {child.journals.count} => 1, + -> {grandchild1.journals.count} => 1, + -> {grandchild2.journals.count} => 0 + ) do + put( + :update, + :params => { + :id => parent.id, + :close_descendants => "", + :issue => { + :status_id => 5, + :notes => notes + } + } + ) + assert_response 302 + end + assert 5, parent.reload.status.id + assert_equal notes, parent.journals.last.notes + assert 5, child.reload.status.id + assert_equal notes, child.journals.last.notes + assert 5, grandchild1.reload.status.id + assert_equal notes, grandchild1.journals.last.notes + assert 6, grandchild2.reload.status.id + end + end + + test "session generated by check_go_to_close_confirm should respect subtask statuses" do + with_settings :closed_parent_issues_with_open_subtasks => 1 do + WorkflowTransition.delete_all + WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, + :old_status_id => 1, :new_status_id => 5) + WorkflowTransition.create!(:role_id => 1, :tracker_id => 1, + :old_status_id => 1, :new_status_id => 6) + WorkflowTransition.create!(:role_id => 1, :tracker_id => 2, + :old_status_id => 1, :new_status_id => 6) + user = User.find(2) + parent = + Issue. + generate!( + :author_id => 2, + :tracker_id => 1, + :status_id => 1 + ) + child1 = + Issue. + generate!( + :author_id => 2, + :parent_issue_id => parent.id, + :tracker_id => 1, + :status_id => 1 + ) + child2 = + Issue. + generate!( + :author_id => 2, + :parent_issue_id => parent.id, + :tracker_id => 2, + :status_id => 1 + ) + assert parent.reload.visible?(user) + assert child1.reload.visible?(user) + assert child2.reload.visible?(user) + + assert [5, 6].all? do |id| + parent.new_statuses_allowed_to(user).map{|s| s.id}.include?(id) + end + assert [5, 6].all? do |id| + child1.new_statuses_allowed_to(user).map{|s| s.id}.include?(id) + end + assert_not child2.new_statuses_allowed_to(user).map{|s| s.id}.include?(5) + assert child2.new_statuses_allowed_to(user).map{|s| s.id}.include?(6) + + @request.session[:user_id] = user.id + put( + :update, + :params => { + :id => parent.id, + :check_go_to_close_confirm => "", + :status_id => 6 + }, + :xhr => true + ) + assert_response :success + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + + assert_equal 1, parent.reload.status.id + assert_equal({"result" => true}, json) + assert session.has_key?(:can_close_descendant) + assert session[:can_close_descendant][:can_close_all] + assert_equal [child1.id, child2.id], session[:can_close_descendant][:ids] + + @request.session.delete(:can_close_descendant) + + @request.session[:user_id] = user.id + put( + :update, + :params => { + :id => parent.id, + :check_go_to_close_confirm => "", + :status_id => 5 + }, + :xhr => true + ) + assert_response :success + assert_equal 'application/json', response.content_type + json = ActiveSupport::JSON.decode(response.body) + + assert_equal 1, parent.reload.status.id + assert_equal({"result" => true}, json) + assert session.has_key?(:can_close_descendant) + assert_not session[:can_close_descendant][:can_close_all] + assert_not session[:can_close_descendant].has_key?(:ids) + end + end + def test_get_bulk_edit @request.session[:user_id] = 2 get(:bulk_edit, :params => {:ids => [1, 3]}) diff --git a/test/system/issues_test.rb b/test/system/issues_test.rb --- a/test/system/issues_test.rb +++ b/test/system/issues_test.rb @@ -234,6 +234,173 @@ class IssuesSystemTest < ApplicationSyst assert_equal 5, issue.reload.status.id end + test "add confirm dialog to issue submit button" do + parent = Issue.generate!(:project_id => 1) + child = Issue.generate!(:project_id => 1, :parent_issue_id => parent.id) + hidden_child = + Issue. + generate!( + :project_id => 1, + :parent_issue_id => parent.id, + :author_id => 2, + :is_private => true + ) + assert_not hidden_child.visible?(User.find(3)) + with_settings :closed_parent_issues_with_open_subtasks => 1 do + log_user('dlopper', 'foo') + visit "/issues/#{parent.id}" + + page.first(:link, 'Edit').click + assert page.has_select?("issue_status_id", {:selected => "New"}) + page.find("#issue_status_id").select("Closed") + assert_no_difference ['Issue.count', 'child.journals.count'] do + assert_no_difference 'parent.journals.count' do + page.first(:button, 'Submit').click + within('#ajax-modal') do + assert page.has_text?(/Are you sure/) + page.first(:link, 'Cancel').click + end + assert_equal 1, parent.reload.status.id + end + assert_difference 'parent.journals.count' do + page.first(:button, 'Submit').click + within('#ajax-modal') do + assert page.has_text?(/Are you sure/) + page.first(:button, 'Apply').click + end + assert page.has_css?('#flash_notice') + assert_equal 5, parent.reload.status.id + end + end + + page.first(:link, 'Edit').click + assert page.has_select?("issue_status_id", {:selected => "Closed"}) + fill_in 'Subject', :with => 'test of confirm dialog' + assert_no_difference ['Issue.count', 'child.journals.count'] do + assert_no_difference 'parent.journals.count' do + page.first(:button, 'Submit').click + within('#ajax-modal') do + assert page.has_text?(/Are you sure/) + page.first(:link, 'Cancel').click + end + assert_equal 5, parent.reload.status.id + end + assert_difference 'parent.journals.count' do + page.first(:button, 'Submit').click + within('#ajax-modal') do + assert page.has_text?(/Are you sure/) + page.first(:button, 'Apply').click + end + assert page.has_css?('#flash_notice') + assert_equal 5, parent.reload.status.id + assert_equal 'test of confirm dialog', parent.reload.subject + end + end + + page.first(:link, 'Edit').click + assert page.has_select?("issue_status_id", {:selected => "Closed"}) + page.find("#issue_status_id").select("New") + assert_no_difference ['Issue.count', 'child.journals.count'] do + assert_difference 'parent.journals.count' do + page.first(:button, 'Submit').click + assert page.has_css?('#flash_notice') + assert_equal 1, parent.reload.status.id + end + end + + visit "/issues/#{child.id}" + page.first(:link, 'Edit').click + assert page.has_select?("issue_status_id", {:selected => "New"}) + page.find("#issue_status_id").select("Closed") + assert_no_difference ['Issue.count', 'parent.journals.count'] do + assert_difference 'child.journals.count' do + page.first(:button, 'Submit').click + assert page.has_css?('#flash_notice') + assert_equal 5, child.reload.status.id + end + end + + hidden_child.status_id = 5 + hidden_child.save! + + visit "/issues/#{parent.id}" + page.first(:link, 'Edit').click + assert page.has_select?("issue_status_id", {:selected => "New"}) + page.find("#issue_status_id").select("Closed") + assert_no_difference ['Issue.count', 'child.journals.count'] do + assert_difference 'parent.journals.count' do + page.first(:button, 'Submit').click + assert page.has_css?('#flash_notice') + assert_equal 5, parent.reload.status.id + end + end + end + end + + test "close all open subtasks" do + parent = Issue.generate!(:project_id => 1) + child = Issue.generate!(:project_id => 1, :parent_issue_id => parent.id) + close_only_text = "Close only issue #{parent.id}" + + with_settings :closed_parent_issues_with_open_subtasks => 1 do + log_user('dlopper', 'foo') + visit "/issues/#{parent.id}" + page.first(:link, 'Edit').click + assert page.has_select?("issue_status_id", {:selected => "New"}) + page.find("#issue_status_id").select("Closed") + assert_no_difference ['Issue.count', 'child.journals.count'] do + assert_no_difference 'parent.journals.count' do + page.first(:button, 'Submit').click + within('#ajax-modal') do + assert page.has_text?(/has open subtasks/) + assert page.has_checked_field?(close_only_text) + page.first(:link, 'Cancel').click + end + assert_equal 1, parent.reload.status.id + end + assert_difference 'parent.journals.count' do + page.first(:button, 'Submit').click + within('#ajax-modal') do + assert page.has_text?(/has open subtasks/) + assert page.has_checked_field?(close_only_text) + page.first(:button, 'Apply').click + end + assert page.has_css?('#flash_notice') + assert_equal 5, parent.reload.status.id + end + end + page.first(:link, 'Edit').click + assert page.has_select?("issue_status_id", {:selected => "Closed"}) + page.find("#issue_status_id").select("New") + assert_no_difference ['Issue.count', 'child.journals.count'] do + assert_difference 'parent.journals.count' do + page.first(:button, 'Submit').click + assert page.has_css?('#flash_notice') + assert_equal 1, parent.reload.status.id + end + end + page.first(:link, 'Edit').click + assert page.has_select?("issue_status_id", {:selected => "New"}) + page.find("#issue_status_id").select("Closed") + assert_no_difference 'Issue.count' do + assert_difference ['parent.journals.count', 'child.journals.count'] do + page.first(:button, 'Submit').click + within('#ajax-modal') do + all_text = 'Close all open subtasks by "Closed" status' + assert page.has_text?(/has open subtasks/) + assert page.has_checked_field?(close_only_text) + page.choose(all_text) + assert page.has_checked_field?(all_text) + page.first(:button, 'Apply').click + end + assert page.has_css?('#flash_notice') + assert_equal 5, parent.reload.status.id + assert_equal 5, child.reload.status.id + end + end + end + end + test "removing issue shows confirm dialog" do log_user('jsmith', 'jsmith') visit '/issues/1'