From 70bdb86c534f2acb074f48ddac3070eaf9a9e3d1 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Sat, 4 May 2013 15:03:07 +0000
Subject: [PATCH] Preserve field values on bulk edit failure (#13943).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11787 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/controllers/issues_controller.rb      |  3 ++
 app/helpers/application_helper.rb         |  6 ++-
 app/helpers/custom_fields_helper.rb       | 12 ++---
 app/helpers/projects_helper.rb            |  5 ++-
 app/views/issues/bulk_edit.html.erb       | 54 ++++++++++++++---------
 test/functional/issues_controller_test.rb | 12 +++++
 6 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index c95e25b12..5336d8c24 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -241,6 +241,9 @@ class IssuesController < ApplicationController
     end
 
     @safe_attributes = @issues.map(&:safe_attribute_names).reduce(:&)
+
+    @issue_params = params[:issue] || {}
+    @issue_params[:custom_field_values] ||= {}
   end
 
   def bulk_update
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 148780df9..215579180 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -330,7 +330,7 @@ module ApplicationHelper
     end
     groups = ''
     collection.sort.each do |element|
-      selected_attribute = ' selected="selected"' if option_value_selected?(element, selected)
+      selected_attribute = ' selected="selected"' if option_value_selected?(element, selected) || element.id.to_s == selected
       (element.is_a?(Group) ? groups : s) << %(<option value="#{element.id}"#{selected_attribute}>#{h element.name}</option>)
     end
     unless groups.empty?
@@ -348,6 +348,10 @@ module ApplicationHelper
     options
   end
 
+  def option_tag(name, text, value, selected=nil, options={})
+    content_tag 'option', value, options.merge(:value => value, :selected => (value == selected))
+  end
+
   # Truncates and returns the string as a single line
   def truncate_single_line(string, *args)
     truncate(string.to_s, *args).gsub(%r{[\r\n]+}m, ' ')
diff --git a/app/helpers/custom_fields_helper.rb b/app/helpers/custom_fields_helper.rb
index 803dbb6d3..80a3eecd7 100644
--- a/app/helpers/custom_fields_helper.rb
+++ b/app/helpers/custom_fields_helper.rb
@@ -77,7 +77,7 @@ module CustomFieldsHelper
     custom_field_label_tag(name, custom_value, options) + custom_field_tag(name, custom_value)
   end
 
-  def custom_field_tag_for_bulk_edit(name, custom_field, projects=nil)
+  def custom_field_tag_for_bulk_edit(name, custom_field, projects=nil, value=nil)
     field_name = "#{name}[custom_field_values][#{custom_field.id}]"
     field_name << "[]" if custom_field.multiple?
     field_id = "#{name}_custom_field_values_#{custom_field.id}"
@@ -87,22 +87,22 @@ module CustomFieldsHelper
     field_format = Redmine::CustomFieldFormat.find_by_name(custom_field.field_format)
     case field_format.try(:edit_as)
       when "date"
-        text_field_tag(field_name, '', tag_options.merge(:size => 10)) +
+        text_field_tag(field_name, value, tag_options.merge(:size => 10)) +
         calendar_for(field_id)
       when "text"
-        text_area_tag(field_name, '', tag_options.merge(:rows => 3))
+        text_area_tag(field_name, value, tag_options.merge(:rows => 3))
       when "bool"
         select_tag(field_name, options_for_select([[l(:label_no_change_option), ''],
                                                    [l(:general_text_yes), '1'],
-                                                   [l(:general_text_no), '0']]), tag_options)
+                                                   [l(:general_text_no), '0']], value), tag_options)
       when "list"
         options = []
         options << [l(:label_no_change_option), ''] unless custom_field.multiple?
         options << [l(:label_none), '__none__'] unless custom_field.is_required?
         options += custom_field.possible_values_options(projects)
-        select_tag(field_name, options_for_select(options), tag_options.merge(:multiple => custom_field.multiple?))
+        select_tag(field_name, options_for_select(options, value), tag_options.merge(:multiple => custom_field.multiple?))
       else
-        text_field_tag(field_name, '', tag_options)
+        text_field_tag(field_name, value, tag_options)
     end
   end
 
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 33e51b8a4..4ec54d8dd 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -69,10 +69,11 @@ module ProjectsHelper
       grouped[version.project.name] << [version.name, version.id]
     end
 
+    selected = selected.is_a?(Version) ? selected.id : selected
     if grouped.keys.size > 1
-      grouped_options_for_select(grouped, selected && selected.id)
+      grouped_options_for_select(grouped, selected)
     else
-      options_for_select((grouped.values.first || []), selected && selected.id)
+      options_for_select((grouped.values.first || []), selected)
     end
   end
 
diff --git a/app/views/issues/bulk_edit.html.erb b/app/views/issues/bulk_edit.html.erb
index cfcc5705a..6c532f42b 100644
--- a/app/views/issues/bulk_edit.html.erb
+++ b/app/views/issues/bulk_edit.html.erb
@@ -32,34 +32,43 @@
 <% if @allowed_projects.present? %>
 <p>
   <label for="issue_project_id"><%= l(:field_project) %></label>
-  <%= select_tag('issue[project_id]', content_tag('option', l(:label_no_change_option), :value => '') + project_tree_options_for_select(@allowed_projects, :selected => @target_project),
+  <%= select_tag('issue[project_id]',
+                 content_tag('option', l(:label_no_change_option), :value => '') + 
+                   project_tree_options_for_select(@allowed_projects, :selected => @target_project),
                  :onchange => "updateBulkEditFrom('#{escape_javascript url_for(:action => 'bulk_edit', :format => 'js')}')") %>
 </p>
 <% end %>
 <p>
   <label for="issue_tracker_id"><%= l(:field_tracker) %></label>
-  <%= select_tag('issue[tracker_id]', content_tag('option', l(:label_no_change_option), :value => '') + options_from_collection_for_select(@trackers, :id, :name)) %>
+  <%= select_tag('issue[tracker_id]',
+                 content_tag('option', l(:label_no_change_option), :value => '') +
+                   options_from_collection_for_select(@trackers, :id, :name, @issue_params[:tracker_id])) %>
 </p>
 <% if @available_statuses.any? %>
 <p>
   <label for='issue_status_id'><%= l(:field_status) %></label>
-  <%= select_tag('issue[status_id]',content_tag('option', l(:label_no_change_option), :value => '') + options_from_collection_for_select(@available_statuses, :id, :name)) %>
+  <%= select_tag('issue[status_id]',
+                 content_tag('option', l(:label_no_change_option), :value => '') +
+                   options_from_collection_for_select(@available_statuses, :id, :name, @issue_params[:status_id])) %>
 </p>
 <% end %>
 
 <% if @safe_attributes.include?('priority_id') -%>
 <p>
   <label for='issue_priority_id'><%= l(:field_priority) %></label>
-  <%= select_tag('issue[priority_id]', content_tag('option', l(:label_no_change_option), :value => '') + options_from_collection_for_select(IssuePriority.active, :id, :name)) %>
+  <%= select_tag('issue[priority_id]',
+                 content_tag('option', l(:label_no_change_option), :value => '') +
+                   options_from_collection_for_select(IssuePriority.active, :id, :name, @issue_params[:priority_id])) %>
 </p>
 <% end %>
 
 <% if @safe_attributes.include?('assigned_to_id') -%>
 <p>
   <label for='issue_assigned_to_id'><%= l(:field_assigned_to) %></label>
-  <%= select_tag('issue[assigned_to_id]', content_tag('option', l(:label_no_change_option), :value => '') +
-                                 content_tag('option', l(:label_nobody), :value => 'none') +
-                                 principals_options_for_select(@assignables)) %>
+  <%= select_tag('issue[assigned_to_id]',
+                 content_tag('option', l(:label_no_change_option), :value => '') +
+                   content_tag('option', l(:label_nobody), :value => 'none', :selected => (@issue_params[:assigned_to_id] == 'none')) +
+                   principals_options_for_select(@assignables, @issue_params[:assigned_to_id])) %>
 </p>
 <% end %>
 
@@ -67,8 +76,8 @@
 <p>
   <label for='issue_category_id'><%= l(:field_category) %></label>
   <%= select_tag('issue[category_id]', content_tag('option', l(:label_no_change_option), :value => '') +
-                                content_tag('option', l(:label_none), :value => 'none') +
-                                options_from_collection_for_select(@categories, :id, :name)) %>
+                                content_tag('option', l(:label_none), :value => 'none', :selected => (@issue_params[:category_id] == 'none')) +
+                                options_from_collection_for_select(@categories, :id, :name, @issue_params[:category_id])) %>
 </p>
 <% end %>
 
@@ -76,26 +85,31 @@
 <p>
   <label for='issue_fixed_version_id'><%= l(:field_fixed_version) %></label>
   <%= select_tag('issue[fixed_version_id]', content_tag('option', l(:label_no_change_option), :value => '') +
-                                   content_tag('option', l(:label_none), :value => 'none') +
-                                   version_options_for_select(@versions.sort)) %>
+                                   content_tag('option', l(:label_none), :value => 'none', :selected => (@issue_params[:fixed_version_id] == 'none')) +
+                                   version_options_for_select(@versions.sort, @issue_params[:fixed_version_id])) %>
 </p>
 <% end %>
 
 <% @custom_fields.each do |custom_field| %>
-  <p><label><%= h(custom_field.name) %></label> <%= custom_field_tag_for_bulk_edit('issue', custom_field, @projects) %></p>
+  <p>
+    <label><%= h(custom_field.name) %></label>
+    <%= custom_field_tag_for_bulk_edit('issue', custom_field, @projects, @issue_params[:custom_field_values][custom_field.id.to_s]) %>
+  </p>
 <% end %>
 
 <% if @copy && @attachments_present %>
+<%= hidden_field_tag 'copy_attachments', '0' %>
 <p>
   <label for='copy_attachments'><%= l(:label_copy_attachments) %></label>
-  <%= check_box_tag 'copy_attachments', '1', true %>
+  <%= check_box_tag 'copy_attachments', '1', params[:copy_attachments] != '0' %>
 </p>
 <% end %>
 
 <% if @copy && @subtasks_present %>
+<%= hidden_field_tag 'copy_subtasks', '0' %>
 <p>
   <label for='copy_subtasks'><%= l(:label_copy_subtasks) %></label>
-  <%= check_box_tag 'copy_subtasks', '1', true %>
+  <%= check_box_tag 'copy_subtasks', '1', params[:copy_subtasks] != '0' %>
 </p>
 <% end %>
 
@@ -107,15 +121,15 @@
 <p>
   <label for='issue_is_private'><%= l(:field_is_private) %></label>
   <%= select_tag('issue[is_private]', content_tag('option', l(:label_no_change_option), :value => '') +
-                                content_tag('option', l(:general_text_Yes), :value => '1') +
-                                content_tag('option', l(:general_text_No), :value => '0')) %>
+                                content_tag('option', l(:general_text_Yes), :value => '1', :selected => (@issue_params[:is_private] == '1')) +
+                                content_tag('option', l(:general_text_No), :value => '0', :selected => (@issue_params[:is_private] == '0'))) %>
 </p>
 <% end %>
 
 <% if @safe_attributes.include?('parent_issue_id') && @project %>
 <p>
   <label for='issue_parent_issue_id'><%= l(:field_parent_issue) %></label>
-  <%= text_field_tag 'issue[parent_issue_id]', '', :size => 10 %>
+  <%= text_field_tag 'issue[parent_issue_id]', '', :size => 10, :value => @issue_params[:parent_issue_id] %>
 </p>
 <%= javascript_tag "observeAutocompleteField('issue_parent_issue_id', '#{escape_javascript auto_complete_issues_path(:project_id => @project)}')" %>
 <% end %>
@@ -123,21 +137,21 @@
 <% if @safe_attributes.include?('start_date') %>
 <p>
   <label for='issue_start_date'><%= l(:field_start_date) %></label>
-  <%= text_field_tag 'issue[start_date]', '', :size => 10 %><%= calendar_for('issue_start_date') %>
+  <%= text_field_tag 'issue[start_date]', '', :value => @issue_params[:start_date], :size => 10 %><%= calendar_for('issue_start_date') %>
 </p>
 <% end %>
 
 <% if @safe_attributes.include?('due_date') %>
 <p>
   <label for='issue_due_date'><%= l(:field_due_date) %></label>
-  <%= text_field_tag 'issue[due_date]', '', :size => 10 %><%= calendar_for('issue_due_date') %>
+  <%= text_field_tag 'issue[due_date]', '', :value => @issue_params[:due_date], :size => 10 %><%= calendar_for('issue_due_date') %>
 </p>
 <% end %>
 
 <% if @safe_attributes.include?('done_ratio') && Issue.use_field_for_done_ratio? %>
 <p>
   <label for='issue_done_ratio'><%= l(:field_done_ratio) %></label>
-  <%= select_tag 'issue[done_ratio]', options_for_select([[l(:label_no_change_option), '']] + (0..10).to_a.collect {|r| ["#{r*10} %", r*10] }) %>
+  <%= select_tag 'issue[done_ratio]', options_for_select([[l(:label_no_change_option), '']] + (0..10).to_a.collect {|r| ["#{r*10} %", r*10] }, @issue_params[:done_ratio]) %>
 </p>
 <% end %>
 </div>
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 3e206db95..62c765125 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -3616,6 +3616,18 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal [issue1.id, issue2.id], assigns[:issues].map(&:id)
   end
 
+  def test_bulk_update_with_failure_should_preserved_form_values
+    @request.session[:user_id] = 2
+    post :bulk_update, :ids => [1, 2], :issue => {:tracker_id => '2', :start_date => 'foo'}
+
+    assert_response :success
+    assert_template 'bulk_edit'
+    assert_select 'select[name=?]', 'issue[tracker_id]' do
+      assert_select 'option[value=2][selected=selected]'
+    end
+    assert_select 'input[name=?][value=?]', 'issue[start_date]', 'foo'
+  end
+
   def test_get_bulk_copy
     @request.session[:user_id] = 2
     get :bulk_edit, :ids => [1, 2, 3], :copy => '1'
-- 
2.39.5