diff options
| -rw-r--r-- | app/controllers/timelog_controller.rb | 39 | ||||
| -rw-r--r-- | app/views/timelog/bulk_edit.html.erb | 26 | ||||
| -rw-r--r-- | test/functional/timelog_controller_test.rb | 4 | ||||
| -rw-r--r-- | test/ui/timelog_test_ui.rb | 30 |
4 files changed, 73 insertions, 26 deletions
diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index 9cfe16312..0cfdd18db 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -172,19 +172,33 @@ class TimelogController < ApplicationController def bulk_update attributes = parse_params_for_bulk_update(params[:time_entry]) - unsaved_time_entry_ids = [] + unsaved_time_entries = [] + saved_time_entries = [] + @time_entries.each do |time_entry| time_entry.reload time_entry.safe_attributes = attributes call_hook(:controller_time_entries_bulk_edit_before_save, { :params => params, :time_entry => time_entry }) - unless time_entry.save - logger.info "time entry could not be updated: #{time_entry.errors.full_messages}" if logger && logger.info? - # Keep unsaved time_entry ids to display them in flash error - unsaved_time_entry_ids << time_entry.id + if time_entry.save + saved_time_entries << time_entry + else + unsaved_time_entries << time_entry end end - set_flash_from_bulk_time_entry_save(@time_entries, unsaved_time_entry_ids) - redirect_back_or_default project_time_entries_path(@projects.first) + + if unsaved_time_entries.empty? + flash[:notice] = l(:notice_successful_update) unless saved_time_entries.empty? + redirect_back_or_default project_time_entries_path(@projects.first) + else + @saved_time_entries = @time_entries + @unsaved_time_entries = unsaved_time_entries + @time_entries = TimeEntry.where(:id => unsaved_time_entries.map(&:id)). + preload(:project => :time_entry_activities). + preload(:user).to_a + + bulk_edit + render :action => 'bulk_edit' + end end def destroy @@ -243,17 +257,6 @@ private render_404 end - def set_flash_from_bulk_time_entry_save(time_entries, unsaved_time_entry_ids) - if unsaved_time_entry_ids.empty? - flash[:notice] = l(:notice_successful_update) unless time_entries.empty? - else - flash[:error] = l(:notice_failed_to_save_time_entries, - :count => unsaved_time_entry_ids.size, - :total => time_entries.size, - :ids => '#' + unsaved_time_entry_ids.join(', #')) - end - end - def find_optional_issue if params[:issue_id].present? @issue = Issue.find(params[:issue_id]) diff --git a/app/views/timelog/bulk_edit.html.erb b/app/views/timelog/bulk_edit.html.erb index 640f7d893..96c953151 100644 --- a/app/views/timelog/bulk_edit.html.erb +++ b/app/views/timelog/bulk_edit.html.erb @@ -1,5 +1,21 @@ <h2><%= l(:label_bulk_edit_selected_time_entries) %></h2> +<% if @unsaved_time_entries.present? %> +<div id="errorExplanation"> + <span> + <%= l(:notice_failed_to_save_time_entries, + :count => @unsaved_time_entries.size, + :total => @saved_time_entries.size, + :ids => @unsaved_time_entries.map {|i| "##{i.id}"}.join(', ')) %> + </span> + <ul> + <% bulk_edit_error_messages(@unsaved_time_entries).each do |message| %> + <li><%= message %></li> + <% end %> + </ul> +</div> +<% end %> + <ul id="bulk-selection"> <% @time_entries.each do |entry| %> <%= content_tag 'li', @@ -12,29 +28,29 @@ <div class="box tabular"> <div> <p> - <label><%= l(:field_issue) %></label> + <label for="time_entry_issue_id"><%= l(:field_issue) %></label> <%= text_field :time_entry, :issue_id, :size => 6 %> </p> <p> - <label><%= l(:field_spent_on) %></label> + <label for="time_entry_spent_on"><%= l(:field_spent_on) %></label> <%= date_field :time_entry, :spent_on, :size => 10 %><%= calendar_for('time_entry_spent_on') %> </p> <p> - <label><%= l(:field_hours) %></label> + <label for="time_entry_hours"><%= l(:field_hours) %></label> <%= text_field :time_entry, :hours, :size => 6 %> </p> <% if @available_activities.any? %> <p> - <label><%= l(:field_activity) %></label> + <label for="time_entry_activity_id"><%= l(:field_activity) %></label> <%= select_tag('time_entry[activity_id]', content_tag('option', l(:label_no_change_option), :value => '') + options_from_collection_for_select(@available_activities, :id, :name)) %> </p> <% end %> <p> - <label><%= l(:field_comments) %></label> + <label for="time_entry_comments"><%= l(:field_comments) %></label> <%= text_field(:time_entry, :comments, :size => 100) %> </p> diff --git a/test/functional/timelog_controller_test.rb b/test/functional/timelog_controller_test.rb index ead919006..4e6df7e1b 100644 --- a/test/functional/timelog_controller_test.rb +++ b/test/functional/timelog_controller_test.rb @@ -582,8 +582,8 @@ class TimelogControllerTest < Redmine::ControllerTest @request.session[:user_id] = 2 post :bulk_update, :params => {:ids => [1, 2], :time_entry => { :hours => 'A'}} - assert_response 302 - assert_match /Failed to save 2 time entrie/, flash[:error] + assert_response :success + assert_select_error /Failed to save 2 time entrie/ end def test_bulk_update_on_different_projects diff --git a/test/ui/timelog_test_ui.rb b/test/ui/timelog_test_ui.rb index e0da8b067..0495b903e 100644 --- a/test/ui/timelog_test_ui.rb +++ b/test/ui/timelog_test_ui.rb @@ -20,7 +20,8 @@ require File.expand_path('../base', __FILE__) class Redmine::UiTest::TimelogTest < Redmine::UiTest::Base fixtures :projects, :users, :email_addresses, :roles, :members, :member_roles, :trackers, :projects_trackers, :enabled_modules, :issue_statuses, :issues, - :enumerations, :custom_fields, :custom_values, :custom_fields_trackers + :enumerations, :custom_fields, :custom_values, :custom_fields_trackers, + :time_entries def test_changing_project_should_update_activities project = Project.find(1) @@ -41,4 +42,31 @@ class Redmine::UiTest::TimelogTest < Redmine::UiTest::Base assert !has_content?('Design') end end + + def test_bulk_edit + log_user 'jsmith', 'jsmith' + visit '/time_entries/bulk_edit?ids[]=1&ids[]=2&ids[]=3' + fill_in 'Hours', :with => '8.5' + select 'QA', :from => 'Activity' + page.first(:button, 'Submit').click + + entries = TimeEntry.where(:id => [1,2,3]).to_a + assert entries.all? {|entry| entry.hours == 8.5} + assert entries.all? {|entry| entry.activity.name == 'QA'} + end + + def test_bulk_edit_with_failure + log_user 'jsmith', 'jsmith' + visit '/time_entries/bulk_edit?ids[]=1&ids[]=2&ids[]=3' + fill_in 'Hours', :with => 'A' + page.first(:button, 'Submit').click + + assert page.has_css?('#errorExplanation') + fill_in 'Hours', :with => '7' + page.first(:button, 'Submit').click + + assert_equal "/projects/ecookbook/time_entries", current_path + entries = TimeEntry.where(:id => [1,2,3]).to_a + assert entries.all? {|entry| entry.hours == 7.0} + end end |