Index: app/models/issue.rb =================================================================== --- app/models/issue.rb (revision 8743) +++ app/models/issue.rb (working copy) @@ -693,7 +693,7 @@ rescue ActiveRecord::StaleObjectError attachments[:files].each(&:destroy) errors.add :base, l(:notice_locking_conflict) - raise ActiveRecord::Rollback + raise ActiveRecord::StaleObjectError end end end Index: app/controllers/issues_controller.rb =================================================================== --- app/controllers/issues_controller.rb (revision 8743) +++ app/controllers/issues_controller.rb (working copy) @@ -107,6 +107,7 @@ def show @journals = @issue.journals.find(:all, :include => [:user, :details], :order => "#{Journal.table_name}.created_on ASC") @journals.each_with_index {|j,i| j.indice = i+1} + @latest_journal = @journals.last @journals.reverse! if User.current.wants_comments_in_reverse_order? if User.current.allowed_to?(:view_changesets, @project) @@ -184,23 +185,65 @@ end def update + if params[:conflict_resolution].present? + case params[:conflict_resolution] + when 'none' + redirect_to :controller => 'issues', :action => 'show', :id => @issue + return + when 'notes' + # update notes only + params.delete(:attachments) + params.delete(:issue) + params.delete(:time_entry) + when 'all' + ; + end + end + update_issue_from_params - if @issue.save_issue_with_child_records(params, @time_entry) - render_attachment_warning_if_needed(@issue) - flash[:notice] = l(:notice_successful_update) unless @issue.current_journal.new_record? - - respond_to do |format| - format.html { redirect_back_or_default({:action => 'show', :id => @issue}) } - format.api { head :ok } + begin + if @issue.save_issue_with_child_records(params, @time_entry) + render_attachment_warning_if_needed(@issue) + flash[:notice] = l(:notice_successful_update) unless @issue.current_journal.new_record? + + respond_to do |format| + format.html { redirect_back_or_default({:action => 'show', :id => @issue}) } + format.api { head :ok } + end + else + render_attachment_warning_if_needed(@issue) + flash[:notice] = l(:notice_successful_update) unless @issue.current_journal.new_record? + @journal = @issue.current_journal + + respond_to do |format| + format.html { render :action => 'edit' } + format.api { render_validation_errors(@issue) } + end end - else + rescue ActiveRecord::StaleObjectError + # mid-air conflict + @conflict_detected = true + + @issue.reload + params[:issue].delete(:lock_version) if params[:issue] + update_issue_from_params + + @journals = @issue.journals.find(:all, :include => [:user, :details], :order => "#{Journal.table_name}.created_on ASC") + @journals.each_with_index {|j,i| j.indice = i+1} + @latest_journal = @journals.last + if params[:latest_journal].present? + latest_journal_id = params[:latest_journal].to_i + @journals = @journals.select{|journal| latest_journal_id < journal.id } + end + @journals.reverse! if User.current.wants_comments_in_reverse_order? + render_attachment_warning_if_needed(@issue) flash[:notice] = l(:notice_successful_update) unless @issue.current_journal.new_record? @journal = @issue.current_journal - + respond_to do |format| - format.html { render :action => 'edit' } + format.html { render :action => 'conflict' } format.api { render_validation_errors(@issue) } end end Index: app/views/issues/_edit.html.erb =================================================================== --- app/views/issues/_edit.html.erb (revision 8743) +++ app/views/issues/_edit.html.erb (working copy) @@ -1,15 +1,34 @@ <% labelled_form_for @issue, :html => {:id => 'issue-form', :multipart => true} do |f| %> <%= error_messages_for 'issue', 'time_entry' %>
+ <% if @conflict_detected %> +
<%= l(:label_conflict_resolution) %> + +
+ <% if @notes.present? %> + +
+ <% end %> + +
+ <% end %> + <% if @latest_journal %> + <%= hidden_field_tag 'latest_journal', @latest_journal.id %> + <% end %> + <% if @edit_allowed || !@allowed_statuses.empty? %> -
<%= l(:label_change_properties) %> +
<%= l(:label_change_properties) %>
<%= render :partial => 'form', :locals => {:f => f} %>
<% end %> <% if User.current.allowed_to?(:log_time, @project) %> -
<%= l(:button_log_time) %> +
<%= l(:button_log_time) %> <% labelled_fields_for :time_entry, @time_entry do |time_entry| %>

<%= time_entry.text_field :hours, :size => 6, :label => :label_spent_time %> <%= l(:field_hours) %>

@@ -25,24 +44,31 @@
<% end %> -
<%= l(:field_notes) %> +
<%= l(:field_notes) %> <%= text_area_tag 'notes', @notes, :cols => 60, :rows => 10, :class => 'wiki-edit' %> <%= wikitoolbar_for 'notes' %> <%= call_hook(:view_issues_edit_notes_bottom, { :issue => @issue, :notes => @notes, :form => f }) %> -

<%=l(:label_attachment_plural)%>
<%= render :partial => 'attachments/form' %>

+

<%=l(:label_attachment_plural)%>
<%= render :partial => 'attachments/form' %>

<%= f.hidden_field :lock_version %> - <%= submit_tag l(:button_submit) %> + <%= submit_tag l(:button_submit), :class => 'conflict_resolution_notes conflict_resolution_all' %> + <% if @conflict_detected %> + <%= submit_tag l(:button_conflict_resolution_none, :id => "##{@issue.id}"), :class => 'conflict_resolution_none' %> + <% end %> <%= link_to_remote l(:label_preview), { :url => preview_issue_path(:project_id => @project, :id => @issue), :method => 'post', :update => 'preview', :with => 'Form.serialize("issue-form")', :complete => "Element.scrollTo('preview')" - }, :accesskey => accesskey(:preview) %> + }, :accesskey => accesskey(:preview), :class => 'conflict_resolution_notes conflict_resolution_all' %> <% end %> -
+
+ +<% if @conflict_detected %> + <%= javascript_tag 'setupShowHideByRadio("conflict_resolution_", [ "none", "notes", "all" ])' %> +<% end %> Index: app/views/issues/conflict.html.erb =================================================================== --- app/views/issues/conflict.html.erb (revision 0) +++ app/views/issues/conflict.html.erb (working copy) @@ -0,0 +1,13 @@ +

<%=h "#{@issue.tracker.name} ##{@issue.id}" %>

+ +<% if @journals.present? %> +
+

<%=l(:label_conflict_history)%>

+<%= render :partial => 'history', :locals => { :issue => @issue, :journals => @journals } %> +
+<% end %> + +<%= render :partial => 'edit' %> +<% content_for :header_tags do %> + <%= robot_exclusion_tag %> +<% end %> Index: app/views/issues/conflict.html.erb =================================================================== --- app/views/issues/conflict.html.erb (revision 8743) +++ app/views/issues/conflict.html.erb (working copy) Property changes on: app/views/issues/conflict.html.erb ___________________________________________________________________ Added: svn:eol-style ## -0,0 +1 ## +native Index: public/javascripts/application.js =================================================================== --- public/javascripts/application.js (revision 8743) +++ public/javascripts/application.js (working copy) @@ -515,3 +515,63 @@ } Event.observe(window, 'load', hideOnLoad); + +/* + * Setup observers of radio buttons to show/hide associated elements + * automatically. + * + * id_base: + * A base name of radio button ids to observe. + * id_suffixes: + * An array of radio button id suffixes to observe. + * id_base + id_suffixes[i] is an id. + * css_class_base: + * A base name of css classes to show/hide. + * css_class_base + css_class_suffixes[i] is a class. + * When a radio button is clicked, + * its corresponding css class is shown, and the others are hidden. + * This parameter is an option, and defaults to id_base. + * css_class_suffixes: + * An array of css class suffixes to show/hide. + * This parameter is an option, and defaults to id_suffixes. + */ +function setupShowHideByRadio( + id_base, id_suffixes, css_class_base, css_class_suffixes) { + if (css_class_base === undefined) { + css_class_base = id_base; + } + if (css_class_suffixes === undefined) { + css_class_suffixes = id_suffixes; + } + function f() { + /* find a checked radio button */ + selected_index = -1; + id_suffixes.each(function(id_suffix, index){ + radio = $(id_base + id_suffix); + if (radio && radio.checked) { + selected_index = index; + throw $break; + } + }); + /* hide unselected */ + css_class_suffixes.each(function(css_class_suffix, index){ + if (index != selected_index) { + $$('.' + css_class_base + css_class_suffix).each( + function(ele) { ele.hide(); }); + } + }) + /* show selected */ + if (0 <= selected_index) { + $$('.' + css_class_base + css_class_suffixes[selected_index]).each( + function(ele) { ele.show(); }); + } + } + /* observe radio buttons */ + id_suffixes.each(function(id_suffix, index){ + radio = $(id_base + id_suffix) + if (radio) { + Event.observe(radio, 'change', f); + } + }) + f(); +} Index: test/functional/issues_controller_transaction_test.rb =================================================================== --- test/functional/issues_controller_transaction_test.rb (revision 8743) +++ test/functional/issues_controller_transaction_test.rb (working copy) @@ -53,6 +53,18 @@ end def test_put_update_stale_issue + assert_put_update_stale_issue('') + assert_no_tag :tag => 'input', :attributes => { :id => 'conflict_resolution_notes' } + assert_tag :tag => 'input', :attributes => { :id => 'conflict_resolution_all', :checked => 'checked' } + end + + def test_put_update_stale_issue_with_notes + assert_put_update_stale_issue('test notes') + assert_tag :tag => 'input', :attributes => { :id => 'conflict_resolution_notes', :checked => 'checked' } + assert_tag :tag => 'input', :attributes => { :id => 'conflict_resolution_all' } + end + + def assert_put_update_stale_issue(notes) issue = Issue.find(2) @request.session[:user_id] = 2 @@ -65,7 +77,7 @@ :fixed_version_id => 4, :lock_version => (issue.lock_version - 1) }, - :notes => '', + :notes => notes, :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}}, :time_entry => { :hours => '2.5', :comments => '', :activity_id => TimeEntryActivity.first.id } end @@ -73,11 +85,84 @@ end assert_response :success - assert_template 'edit' + assert_template 'conflict' assert_tag :tag => 'div', :attributes => { :id => 'errorExplanation' }, :content => /Data has been updated by another user/ + assert_tag :tag => 'input', :attributes => { :id => 'issue_lock_version', :value => issue.lock_version } + assert_tag :tag => 'legend', :content => I18n.t(:label_conflict_resolution) + + journals = issue.journals.find(:all, :include => [:user, :details], :order => "#{Journal.table_name}.created_on ASC") + assert_tag :tag => 'input', :attributes => { :id => 'latest_journal', :value => journals.last.id } + journals.each{|journal| + assert_tag :tag => 'div', :attributes => { :id => "change-#{journal.id}" } + } + end + + def test_conflict_resolution_none + issue = Issue.find(2) + @request.session[:user_id] = 2 + + assert_no_difference 'Journal.count' do + assert_no_difference 'TimeEntry.count' do + assert_no_difference 'Attachment.count' do + put_conflict_resolution(issue, 'none') + end + end + end + end + + def test_conflict_resolution_notes + issue = Issue.find(2) + @request.session[:user_id] = 2 + prev_status = issue.status + + assert_difference 'Journal.count' do + assert_no_difference 'TimeEntry.count' do + assert_no_difference 'Attachment.count' do + put_conflict_resolution(issue, 'notes') + end + end + end + + issue.reload + assert_equal prev_status, issue.status + end + + def test_conflict_resolution_all + issue = Issue.find(2) + @request.session[:user_id] = 2 + prev_status = issue.status + + assert_difference 'Journal.count' do + assert_difference 'TimeEntry.count' do + assert_difference 'Attachment.count' do + put_conflict_resolution(issue, 'all') + end + end + end + + issue.reload + assert_not_equal prev_status, issue.status + end + + def put_conflict_resolution(issue, conflict_resolution) + set_tmp_attachments_directory + put :update, + :id => issue.id, + :issue => { + :status_id => 3, + :fixed_version_id => 4, + :lock_version => issue.lock_version, + }, + :notes => 'test notes', + :attachments => {'1' => {'file' => uploaded_test_file('testfile.txt', 'text/plain')}}, + :time_entry => { :hours => '2.5', :comments => '', :activity_id => TimeEntryActivity.first.id }, + :conflict_resolution => conflict_resolution + + assert_redirected_to :controller => 'issues', :action => 'show', :id => issue.id end + def test_index_should_rescue_invalid_sql_query Query.any_instance.stubs(:statement).returns("INVALID STATEMENT") Index: config/locales/en.yml =================================================================== --- config/locales/en.yml (revision 8743) +++ config/locales/en.yml (working copy) @@ -840,6 +840,11 @@ label_copy_attachments: Copy attachments label_item_position: %{position} of %{count} label_completed_versions: Completed versions + label_conflict_resolution: Conflict Resolution + label_conflict_resolution_none: "Throw away my changes, and show %{id}" + label_conflict_resolution_notes: Submit only my new notes + label_conflict_resolution_all: Edit again + label_conflict_history: Another user's updates button_login: Login button_submit: Submit @@ -889,6 +894,7 @@ button_show: Show button_edit_section: Edit this section button_export: Export + button_conflict_resolution_none: "Show %{id}" status_active: active status_registered: registered Index: config/locales/ja.yml =================================================================== --- config/locales/ja.yml (revision 8743) +++ config/locales/ja.yml (working copy) @@ -837,6 +837,11 @@ label_git_report_last_commit: ファイルとディレクトリの最新コミットを表示する label_parent_revision: 親 label_child_revision: 子 + label_conflict_resolution: 自分の更新データの反映方法 + label_conflict_resolution_none: "自分の更新データを 破棄 し %{id} を表示する" + label_conflict_resolution_notes: 自分の 注記のみ を反映する + label_conflict_resolution_all: 自分の更新データを修正する + label_conflict_history: 別のユーザによる更新内容 button_login: ログイン button_submit: 変更 @@ -884,6 +889,7 @@ button_quote: 引用 button_duplicate: 複製 button_show: 表示 + button_conflict_resolution_none: "%{id} を表示" status_active: 有効 status_registered: 登録