Merge branch 'revert_revert_issuable_lock' into 'master'

Revert the revert of Optimistic Locking

## What does this MR do?

It reverts the revert of optimistic lock https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5146 (and revert MR https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5245)

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/19871

Actually the code is already reviewed, except:
* Monkey patch `config/initializers/ar_monkey_patch.rb`
* We removed default values from migration `db/migrate/20160707104333_add_lock_to_issuables.rb`

See merge request !5623

4 files changed, 24 insertions, 2 deletions

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 639cf4c0ef2..7b0189150f8 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -125,6 +125,10 @@ class Projects::IssuesController < Projects::ApplicationController
         render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } })
       end
     end
+
+  rescue ActiveRecord::StaleObjectError
+    @conflict = true
+    render :edit
   end
 
   def referenced_merge_requests
@@ -230,7 +234,7 @@ class Projects::IssuesController < Projects::ApplicationController
   def issue_params
     params.require(:issue).permit(
       :title, :assignee_id, :position, :description, :confidential,
-      :milestone_id, :due_date, :state_event, :task_num, label_ids: []
+      :milestone_id, :due_date, :state_event, :task_num, :lock_version, label_ids: []
     )
   end
 
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index d3fe441c4d2..6a8c7166b39 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -258,6 +258,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     else
       render "edit"
     end
+  rescue ActiveRecord::StaleObjectError
+    @conflict = true
+    render :edit
   end
 
   def remove_wip
@@ -493,7 +496,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       :title, :assignee_id, :source_project_id, :source_branch,
       :target_project_id, :target_branch, :milestone_id,
       :state_event, :description, :task_num, :force_remove_source_branch,
-      label_ids: []
+      :lock_version, label_ids: []
     )
   end
 
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index afb5ce37c06..8e11d4f57cf 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -87,6 +87,12 @@ module Issuable
       User.find(assignee_id_was).update_cache_counts if assignee_id_was
       assignee.update_cache_counts if assignee
     end
+
+    # We want to use optimistic lock for cases when only title or description are involved
+    # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html
+    def locking_enabled?
+      title_changed? || description_changed?
+    end
   end
 
   module ClassMethods
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 544ed6203aa..d8cfa1fca72 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -1,5 +1,12 @@
 = form_errors(issuable)
 
+- if @conflict
+  .alert.alert-danger
+    Someone edited the #{issuable.class.model_name.human.downcase} the same time you did.
+    Please check out
+    = link_to "the #{issuable.class.model_name.human.downcase}", polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), target: "_blank"
+    and make sure your changes will not unintentionally remove theirs
+
 .form-group
   = f.label :title, class: 'control-label'
 
@@ -172,3 +179,5 @@
         = link_to 'Delete', polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), data: { confirm: "#{issuable.class.name.titleize} will be removed! Are you sure?" },
                                                                                                   method: :delete, class: 'btn btn-danger btn-grouped'
       = link_to 'Cancel', polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), class: 'btn btn-grouped btn-cancel'
+
+= f.hidden_field :lock_version