Better merge handling. show if MR can be accepted or not

app/controllers/merge_requests_controller.rb

@@ -45,6 +45,10 @@ class MergeRequestsController < ApplicationController
     # or from cache if already merged
     @commits = @merge_request.commits
 
+    if @merge_request.unchecked? 
+      @merge_request.check_if_can_be_merged
+    end
+
     respond_to do |format|
       format.html
       format.js
@@ -96,23 +100,8 @@ class MergeRequestsController < ApplicationController
   end
 
   def automerge
-    render_404 unless @merge_request.open?
-
-    message = ""
-
-    if GitlabMerge.new(@merge_request).merge
-      @merge_request.merge!(current_user.id)
-      message = "Successfully merged"
-    else
-      @merge_request.mark_as_unmergable
-      message = "Can not be merged"
-    end
-
-    redirect_to [@merge_request.project, @merge_request], :alert => message
-  rescue => ex
-    @merge_request.mark_as_unmergable
-    message = "Can not be merged"
-  ensure
+    render_404 unless @merge_request.open? && @merge_request.can_be_merged?
+    message = @merge_request.automerge! ? "Successfully merged" : "Can not be merged"
     redirect_to [@merge_request.project, @merge_request], :alert => message
   end
 

app/models/merge_request.rb

 require File.join(Rails.root, "app/models/commit")
 
 class MergeRequest < ActiveRecord::Base
+  UNCHECKED = 1
+  CAN_BE_MERGED = 2
+  CANNOT_BE_MERGED = 3
+
   belongs_to :project
   belongs_to :author, :class_name => "User"
   belongs_to :assignee, :class_name => "User"
@@ -56,8 +60,21 @@ class MergeRequest < ActiveRecord::Base
     self.reloaded_diffs
   end
 
+  def unchecked?
+    state == UNCHECKED
+  end
+
   def can_be_merged?
-    auto_merge
+    state == CAN_BE_MERGED
+  end
+
+  def check_if_can_be_merged
+    self.state = if GitlabMerge.new(self).can_be_merged?
+                   CAN_BE_MERGED
+                 else
+                   CANNOT_BE_MERGED
+                 end
+    self.save
   end
 
   def new?
@@ -123,8 +140,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def mark_as_unmergable
-    self.auto_merge = false
-    save
+    # TODO: write some code here
   end
 
   def reloaded_commits 
@@ -153,6 +169,16 @@ class MergeRequest < ActiveRecord::Base
       :author_id => user_id
     )
   end
+
+  def automerge!
+    if GitlabMerge.new(self).merge
+      self.merge!(current_user.id)
+      true
+    end
+  rescue 
+    self.mark_as_unmergable
+    false
+  end
 end
 # == Schema Information
 #

app/views/merge_requests/show.html.haml

@@ -54,18 +54,18 @@
 
 - if @merge_request.open? && @commits.any?
   - if @merge_request.can_be_merged?
-    .alert-message.block-message.success
-      %p You can try to merge this request with GitLab. If failed you can always do it manually
+    .alert-message.info
+      %p 
+        You can accept this request automatically. If you still want to do it manually - #{link_to "click here", "#", :class => "how_to_merge_link cwhite", :title => "How To Merge"} for instructions
       .alert-actions
-        = link_to "Try Merge it!", automerge_project_merge_request_path(@project, @merge_request), :class => "btn small success"
+        = link_to "Accept Merge Request", automerge_project_merge_request_path(@project, @merge_request), :class => "btn small info"
          
-        = link_to "Show how to merge", "#", :class => "how_to_merge_link btn small padded", :title => "How To Merge"
+        
   - else
-    .alert-message.block-message
-      %p This request cant be merged with GitLab. You should do it manually
-      .alert-actions
-        %span.btn.small.disabled Try Merge it!
-        = link_to "Show how to merge", "#", :class => "how_to_merge_link btn small padded success", :title => "How To Merge"
+    .alert-message
+      %p
+        %strong This request cant be merged with GitLab. You should do it manually  
+        = link_to "Show how to merge", "#", :class => "how_to_merge_link btn small padded", :title => "How To Merge"
 
 
 = render "merge_requests/commits"

db/migrate/20120329170745_add_automerge_to_merge_request.rb

 class AddAutomergeToMergeRequest < ActiveRecord::Migration
   def change
-    add_column :merge_requests, :auto_merge, :boolean, :null => false, :default => true
-
+    add_column :merge_requests, :state, :integer, :null => false, :default => 1
   end
 end

db/schema.rb

@@ -30,8 +30,8 @@ ActiveRecord::Schema.define(:version => 20120329170745) do
     t.integer  "assignee_id"
     t.integer  "author_id"
     t.integer  "project_id"
-    t.datetime "created_at",                     :null => false
-    t.datetime "updated_at",                     :null => false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.boolean  "closed",      :default => false, :null => false
     t.integer  "position",    :default => 0
     t.boolean  "critical",    :default => false, :null => false
@@ -43,8 +43,8 @@ ActiveRecord::Schema.define(:version => 20120329170745) do
 
   create_table "keys", :force => true do |t|
     t.integer  "user_id"
-    t.datetime "created_at", :null => false
-    t.datetime "updated_at", :null => false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.text     "key"
     t.string   "title"
     t.string   "identifier"
@@ -59,12 +59,12 @@ ActiveRecord::Schema.define(:version => 20120329170745) do
     t.integer  "assignee_id"
     t.string   "title"
     t.boolean  "closed",        :default => false, :null => false
-    t.datetime "created_at",                       :null => false
-    t.datetime "updated_at",                       :null => false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.text     "st_commits"
     t.text     "st_diffs"
     t.boolean  "merged",        :default => false, :null => false
-    t.boolean  "auto_merge",    :default => true,  :null => false
+    t.integer  "state",         :default => 1,     :null => false
   end
 
   add_index "merge_requests", ["project_id"], :name => "index_merge_requests_on_project_id"
@@ -74,8 +74,8 @@ ActiveRecord::Schema.define(:version => 20120329170745) do
     t.string   "noteable_id"
     t.string   "noteable_type"
     t.integer  "author_id"
-    t.datetime "created_at",    :null => false
-    t.datetime "updated_at",    :null => false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer  "project_id"
     t.string   "attachment"
     t.string   "line_code"
@@ -88,8 +88,8 @@ ActiveRecord::Schema.define(:version => 20120329170745) do
     t.string   "name"
     t.string   "path"
     t.text     "description"
-    t.datetime "created_at",                                   :null => false
-    t.datetime "updated_at",                                   :null => false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.boolean  "private_flag",           :default => true,     :null => false
     t.string   "code"
     t.integer  "owner_id"
@@ -112,8 +112,8 @@ ActiveRecord::Schema.define(:version => 20120329170745) do
     t.text     "content"
     t.integer  "author_id",  :null => false
     t.integer  "project_id", :null => false
-    t.datetime "created_at", :null => false
-    t.datetime "updated_at", :null => false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.string   "file_name"
     t.datetime "expires_at"
   end
@@ -146,8 +146,8 @@ ActiveRecord::Schema.define(:version => 20120329170745) do
     t.datetime "last_sign_in_at"
     t.string   "current_sign_in_ip"
     t.string   "last_sign_in_ip"
-    t.datetime "created_at",                                               :null => false
-    t.datetime "updated_at",                                               :null => false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.string   "name"
     t.boolean  "admin",                                 :default => false, :null => false
     t.integer  "projects_limit",                        :default => 10
@@ -166,16 +166,16 @@ ActiveRecord::Schema.define(:version => 20120329170745) do
   create_table "users_projects", :force => true do |t|
     t.integer  "user_id",                       :null => false
     t.integer  "project_id",                    :null => false
-    t.datetime "created_at",                    :null => false
-    t.datetime "updated_at",                    :null => false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer  "project_access", :default => 0, :null => false
   end
 
   create_table "web_hooks", :force => true do |t|
     t.string   "url"
     t.integer  "project_id"
-    t.datetime "created_at", :null => false
-    t.datetime "updated_at", :null => false
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
   create_table "wikis", :force => true do |t|

lib/gitlab_merge.rb

@@ -4,23 +4,34 @@ class GitlabMerge
   def initialize(merge_request)
     self.merge_request = merge_request
     self.project = merge_request.project
-    self.merge_path = File.join(Rails.root, "tmp", "merge_repo", project.path)
+    self.merge_path = File.join(Rails.root, "tmp", "merge_repo", project.path, merge_request.id.to_s)
     FileUtils.rm_rf(merge_path)
     FileUtils.mkdir_p merge_path
   end
 
+  def can_be_merged?
+    pull do |repo, output|
+      !(output =~ /Automatic merge failed/)
+    end
+  end
+
   def merge
+    pull do |repo, output|
+      if output =~ /Automatic merge failed/
+        false  
+      else 
+        repo.git.push({}, "origin", merge_request.target_branch)
+        true
+      end
+    end
+  end
+
+  def pull
     self.project.repo.git.clone({:branch => merge_request.target_branch}, project.url_to_repo, merge_path)
-    output = ""
     Dir.chdir(merge_path) do
       merge_repo = Grit::Repo.new('.')
       output = merge_repo.git.pull({}, "origin", merge_request.source_branch)
-      if output =~ /Automatic merge failed/
-        return false  
-      else 
-        merge_repo.git.push({}, "origin", merge_request.target_branch)
-        return true
-      end
+      yield(merge_repo, output)
     end
   end
 end