jro_apps/redmine Commit - r11556:1269e6c7d3f5

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

Jean-Philippe Lang -

r11556:1269e6c7d3f5

parent child

Context file:

r11556:1269e6c7d3f5

Collapse all files

app/controllers/application_controller.rb 0 -15

                  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

app/controllers/issues_controller.rb +19 -14

                  end
                  @safe_attributes = @issues.map(&:safe_attribute_names).reduce(:&)
-                 render :layout => false if request.xhr?
                end
                def bulk_update
                  @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
                    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

app/helpers/issues_helper.rb +14 0

                  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 = {

app/views/issues/bulk_edit.html.erb +16 0

              <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) %>

test/functional/issues_controller_test.rb +24 -5

                  assert_redirected_to :controller => 'issues', :action => 'index', :project_id => Project.find(1).identifier
                end
-               def test_bulk_update_with_failure_should_set_flash
+               def test_bulk_update_with_all_failures_should_show_errors
                  @request.session[:user_id] = 2
-                 Issue.update_all("subject = ''", "id = 2") # Make it invalid
-                 post :bulk_update, :ids => [1, 2], :issue => {:priority_id => 6}
+                 post :bulk_update, :ids => [1, 2], :issue => {:start_date => 'foo'}
-                 assert_redirected_to :controller => 'issues', :action => 'index', :project_id => 'ecookbook'
-                 assert_equal 'Failed to save 1 issue(s) on 2 selected: #2.', flash[:error]
+                 assert_response :success
+                 assert_template 'bulk_edit'
+                 assert_select '#errorExplanation span', :text => 'Failed to save 2 issue(s) on 2 selected: #1, #2.'
+                 assert_select '#errorExplanation ul li', :text => 'Start date is not a valid date: #1, #2'
+                 assert_equal [1, 2], assigns[:issues].map(&:id)
+               end
+               def test_bulk_update_with_some_failures_should_show_errors
+                 issue1 = Issue.generate!(:start_date => '2013-05-12')
+                 issue2 = Issue.generate!(:start_date => '2013-05-15')
+                 issue3 = Issue.generate!
+                 @request.session[:user_id] = 2
+                 post :bulk_update, :ids => [issue1.id, issue2.id, issue3.id], :issue => {:due_date => '2013-05-01'}
+                 assert_response :success
+                 assert_template 'bulk_edit'
+                 assert_select '#errorExplanation span', :text => "Failed to save 2 issue(s) on 3 selected: ##{issue1.id}, ##{issue2.id}."
+                 assert_select '#errorExplanation ul li', :text => "Due date must be greater than start date: ##{issue1.id}, ##{issue2.id}"
+                 assert_equal [issue1.id, issue2.id], assigns[:issues].map(&:id)
                end
                def test_get_bulk_copy

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages