From 411d84f385364be987380da5f0216048073609c9 Mon Sep 17 00:00:00 2001 From: randx Date: Fri, 30 Mar 2012 08:05:04 +0300 Subject: [PATCH] Better merge handling. show if MR can be accepted or not --- app/controllers/merge_requests_controller.rb | 23 ++++--------- app/models/merge_request.rb | 32 ++++++++++++++++-- app/views/merge_requests/show.html.haml | 18 +++++----- ...0120329170745_add_automerge_to_merge_request.rb | 3 +- db/schema.rb | 38 +++++++++++----------- lib/gitlab_merge.rb | 27 ++++++++++----- 6 files changed, 83 insertions(+), 58 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index b45338d79..d0750f681 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/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 diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 6d59ce563..6eb54343d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1,6 +1,10 @@ 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 # diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index ea4669536..1b8a25f2d 100644 --- a/app/views/merge_requests/show.html.haml +++ b/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" diff --git a/db/migrate/20120329170745_add_automerge_to_merge_request.rb b/db/migrate/20120329170745_add_automerge_to_merge_request.rb index 609c3094c..de7c68ee1 100644 --- a/db/migrate/20120329170745_add_automerge_to_merge_request.rb +++ b/db/migrate/20120329170745_add_automerge_to_merge_request.rb @@ -1,6 +1,5 @@ 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 diff --git a/db/schema.rb b/db/schema.rb index ff03d504b..7b9d431de 100644 --- a/db/schema.rb +++ b/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| diff --git a/lib/gitlab_merge.rb b/lib/gitlab_merge.rb index 85cc1667f..35ae5d1d2 100644 --- a/lib/gitlab_merge.rb +++ b/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 -- 2.11.0