Better handling of issue update conflicts (#8691).

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

app/controllers/issues_controller.rb

@@ -171,7 +171,7 @@ class IssuesController < ApplicationController
   end
 
   def edit
-    update_issue_from_params
+    return unless update_issue_from_params
 
     respond_to do |format|
       format.html { }
@@ -180,9 +180,23 @@ class IssuesController < ApplicationController
   end
 
   def update
-    update_issue_from_params
+    return unless update_issue_from_params
+    saved = false
+    begin
+      saved = @issue.save_issue_with_child_records(params, @time_entry)
+    rescue ActiveRecord::StaleObjectError
+      @conflict = true
+      if params[:last_journal_id]
+        if params[:last_journal_id].present?
+          last_journal_id = params[:last_journal_id].to_i
+          @conflict_journals = @issue.journals.all(:conditions => ["#{Journal.table_name}.id > ?", last_journal_id])
+        else
+          @conflict_journals = @issue.journals.all
+        end
+      end
+    end
 
-    if @issue.save_issue_with_child_records(params, @time_entry)
+    if saved
       render_attachment_warning_if_needed(@issue)
       flash[:notice] = l(:notice_successful_update) unless @issue.current_journal.new_record?
 
@@ -345,7 +359,22 @@ private
 
     @notes = params[:notes] || (params[:issue].present? ? params[:issue][:notes] : nil)
     @issue.init_journal(User.current, @notes)
-    @issue.safe_attributes = params[:issue]
+
+    issue_attributes = params[:issue]
+    if issue_attributes && params[:conflict_resolution]
+      case params[:conflict_resolution]
+      when 'overwrite'
+        issue_attributes = issue_attributes.dup
+        issue_attributes.delete(:lock_version)
+      when 'add_notes'
+        issue_attributes = {}
+      when 'cancel'
+        redirect_to issue_path(@issue)
+        return false
+      end
+    end
+    @issue.safe_attributes = issue_attributes
+    true
   end
 
   # TODO: Refactor, lots of extra code in here

app/models/issue.rb

@@ -436,6 +436,15 @@ class Issue < ActiveRecord::Base
     @current_journal
   end
 
+  # Returns the id of the last journal or nil
+  def last_journal_id
+    if new_record?
+      nil
+    else
+      journals.first(:order => "#{Journal.table_name}.id DESC").try(:id)
+    end
+  end
+
   # Return true if the issue is closed, otherwise false
   def closed?
     self.status.is_closed?
@@ -692,8 +701,7 @@ class Issue < ActiveRecord::Base
           end
         rescue ActiveRecord::StaleObjectError
           attachments[:files].each(&:destroy)
-          errors.add :base, l(:notice_locking_conflict)
-          raise ActiveRecord::Rollback
+          raise ActiveRecord::StaleObjectError
         end
       end
     end

app/views/issues/_conflict.html.erb

@@ -0,0 +1,26 @@
+<div class="conflict">
+	<%= l(:notice_issue_update_conflict) %> 
+  <% if @conflict_journals.present? %>
+	  <div class="conflict-details">
+    <% @conflict_journals.sort_by(&:id).each do |journal| %>
+		  <p><%= authoring journal.created_on, journal.user, :label => :label_updated_time_by %></p>
+	    <% if journal.details.any? %>
+	    <ul class="details">
+	      <% details_to_strings(journal.details).each do |string| %>
+	       <li><%= string %></li>
+	      <% end %>
+	    </ul>
+	    <% end %>
+	    <%= textilizable(journal, :notes) unless journal.notes.blank? %>
+    <% end %>
+		</div>
+  <% end %>
+</div>
+<p>
+  <label><%= radio_button_tag 'conflict_resolution', 'overwrite' %> <%= l(:text_issue_conflict_resolution_overwrite) %></label><br />
+  <% if @notes.present? %>
+  <label><%= radio_button_tag 'conflict_resolution', 'add_notes' %> <%= l(:text_issue_conflict_resolution_add_notes) %></label><br />
+  <% end %>
+	<label><%= radio_button_tag 'conflict_resolution', 'cancel' %> <%= l(:text_issue_conflict_resolution_cancel, :link => link_to_issue(@issue, :subject => false)) %></label>
+</p>
+<p><%= submit_tag l(:button_submit) %></p>

app/views/issues/_edit.html.erb

@@ -1,5 +1,6 @@
 <% labelled_form_for @issue, :html => {:id => 'issue-form', :multipart => true} do |f| %>
     <%= error_messages_for 'issue', 'time_entry' %>
+		<%= render :partial => 'conflict' if @conflict %>
     <div class="box">
     <% if @edit_allowed || !@allowed_statuses.empty? %>
         <fieldset class="tabular"><legend><%= l(:label_change_properties) %></legend>
@@ -35,6 +36,7 @@
     </div>
 
     <%= f.hidden_field :lock_version %>
+    <%= hidden_field_tag 'last_journal_id', params[:last_journal_id] || @issue.last_journal_id %>
     <%= submit_tag l(:button_submit) %>
     <%= link_to_remote l(:label_preview),
                        { :url => preview_issue_path(:project_id => @project, :id => @issue),