Display the bulk edit form with error messages when some issues can not be saved (#13943).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11786 e93f8b46-1217-0410-a6f0-8f06a7374b81

4 files changed, 49 insertions, 29 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 6bbc4d1fc..a3fbae5aa 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -555,21 +555,6 @@ class ApplicationController < ActionController::Base
     flash[:warning] = l(:warning_attachments_not_saved, obj.unsaved_attachments.size) if obj.unsaved_attachments.present?
   end
 
-  # Sets the `flash` notice or error based the number of issues that did not save
-  #
-  # @param [Array, Issue] issues all of the saved and unsaved Issues
-  # @param [Array, Integer] unsaved_issue_ids the issue ids that were not saved
-  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
-
   # Rescues an invalid query statement. Just in case...
   def query_statement_invalid(exception)
     logger.error "Query::StatementInvalid: #{exception.message}" if logger
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 3f3966aba..c95e25b12 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -241,7 +241,6 @@ class IssuesController < ApplicationController
     end
 
     @safe_attributes = @issues.map(&:safe_attribute_names).reduce(:&)
-    render :layout => false if request.xhr?
   end
 
   def bulk_update
@@ -249,8 +248,8 @@ class IssuesController < ApplicationController
     @copy = params[:copy].present?
     attributes = parse_params_for_bulk_issue_attributes(params)
 
-    unsaved_issue_ids = []
-    moved_issues = []
+    unsaved_issues = []
+    saved_issues = []
 
     if @copy && params[:copy_subtasks].present?
       # Descendant issues will be copied with the parent task
@@ -270,23 +269,29 @@ class IssuesController < ApplicationController
       issue.safe_attributes = attributes
       call_hook(:controller_issues_bulk_edit_before_save, { :params => params, :issue => issue })
       if issue.save
-        moved_issues << issue
+        saved_issues << issue
       else
-        logger.info "issue could not be updated or copied: #{issue.errors.full_messages}" if logger && logger.info
-        # Keep unsaved issue ids to display them in flash error
-        unsaved_issue_ids << issue.id
+        unsaved_issues << issue
       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 issue_path(moved_issues.first)
-      elsif moved_issues.map(&:project).uniq.size == 1
-        redirect_to project_issues_path(moved_issues.map(&:project).first)
+    if unsaved_issues.empty?
+      flash[:notice] = l(:notice_successful_update) unless saved_issues.empty?
+      if params[:follow]
+        if @issues.size == 1 && saved_issues.size == 1
+          redirect_to issue_path(saved_issues.first)
+        elsif saved_issues.map(&:project).uniq.size == 1
+          redirect_to project_issues_path(saved_issues.map(&:project).first)
+        end
+      else
+        redirect_back_or_default _project_issues_path(@project)
       end
     else
-      redirect_back_or_default _project_issues_path(@project)
+      @saved_issues = @issues
+      @unsaved_issues = unsaved_issues
+      @issues = Issue.visible.find_all_by_id(@unsaved_issues.map(&:id))
+      bulk_edit
+      render :action => 'bulk_edit'
     end
   end
 
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 7d54bf22e..39b24fe04 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -94,6 +94,20 @@ module IssuesHelper
     s.html_safe
   end
 
+  # Returns an array of error messages for bulk edited issues
+  def bulk_edit_error_messages(issues)
+    messages = {}
+    issues.each do |issue|
+      issue.errors.full_messages.each do |message|
+        messages[message] ||= []
+        messages[message] << issue
+      end
+    end
+    messages.map { |message, issues|
+      "#{message}: " + issues.map {|i| "##{i.id}"}.join(', ')
+    }
+ end
+
   # Returns a link for adding a new subtask to the given issue
   def link_to_new_subtask(issue)
     attrs = {
diff --git a/app/views/issues/bulk_edit.html.erb b/app/views/issues/bulk_edit.html.erb
index 903a0daf8..cfcc5705a 100644
--- a/app/views/issues/bulk_edit.html.erb
+++ b/app/views/issues/bulk_edit.html.erb
@@ -1,5 +1,21 @@
 <h2><%= @copy ? l(:button_copy) : l(:label_bulk_edit_selected_issues) %></h2>
 
+<% if @saved_issues && @unsaved_issues.present? %>
+<div id="errorExplanation">
+  <span>
+    <%= l(:notice_failed_to_save_issues,
+        :count => @unsaved_issues.size,
+        :total => @saved_issues.size,
+        :ids => @unsaved_issues.map {|i| "##{i.id}"}.join(', ')) %>
+  </span>
+  <ul>
+  <% bulk_edit_error_messages(@unsaved_issues).each do |message| %>
+    <li><%= message %></li>
+  <% end %>
+  </ul>
+</div>
+<% end %>
+
 <ul id="bulk-selection">
 <% @issues.each do |issue| %>
   <%= content_tag 'li', link_to_issue(issue) %>