From 78d620b3b63fdd78b2cd1e4386822f93a701bc42 Mon Sep 17 00:00:00 2001 From: randx Date: Thu, 29 Mar 2012 18:03:05 +0300 Subject: [PATCH 01/11] Raw gitlab automerge feature --- app/controllers/merge_requests_controller.rb | 18 +++++++++++++- config/routes.rb | 1 + lib/gitlab_merge.rb | 26 ++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 lib/gitlab_merge.rb diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index f882028fcab..609589d6b0b 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -2,7 +2,7 @@ class MergeRequestsController < ApplicationController before_filter :authenticate_user! before_filter :project before_filter :module_enabled - before_filter :merge_request, :only => [:edit, :update, :destroy, :show, :commits, :diffs] + before_filter :merge_request, :only => [:edit, :update, :destroy, :show, :commits, :diffs, :automerge] layout "project" # Authorize @@ -95,6 +95,22 @@ class MergeRequestsController < ApplicationController end end + def automerge + message = "" + if GitlabMerge.new(@merge_request).merge + @merge_request.update_attributes( + :author_id_of_changes => current_user.id, + :closed => true + ) + @merge_request.reload_code + message = "Successfully merged" + else + message = "Can not be merged" + end + + redirect_to [@merge_request.project, @merge_request], :alert => message + end + def destroy @merge_request.destroy diff --git a/config/routes.rb b/config/routes.rb index eb999f26ba2..a1c54565f49 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -102,6 +102,7 @@ Gitlab::Application.routes.draw do resources :merge_requests do member do get :diffs + get :automerge end collection do diff --git a/lib/gitlab_merge.rb b/lib/gitlab_merge.rb new file mode 100644 index 00000000000..85cc1667f65 --- /dev/null +++ b/lib/gitlab_merge.rb @@ -0,0 +1,26 @@ +class GitlabMerge + attr_accessor :project, :merge_path, :merge_request + + 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) + FileUtils.rm_rf(merge_path) + FileUtils.mkdir_p merge_path + end + + def merge + 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 + end + end +end -- GitLab From cd74f9da91cb1b798a8654f725b502bac256228d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 30 Mar 2012 00:27:42 +0300 Subject: [PATCH 02/11] Link for auto merge, db field for MR --- app/controllers/merge_requests_controller.rb | 15 ++++--- app/models/merge_request.rb | 9 +++++ app/views/merge_requests/show.html.haml | 17 +++++++- ...29170745_add_automerge_to_merge_request.rb | 6 +++ db/schema.rb | 39 ++++++++++--------- 5 files changed, 61 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20120329170745_add_automerge_to_merge_request.rb diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 609589d6b0b..b45338d7938 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -96,18 +96,23 @@ class MergeRequestsController < ApplicationController end def automerge + render_404 unless @merge_request.open? + message = "" + if GitlabMerge.new(@merge_request).merge - @merge_request.update_attributes( - :author_id_of_changes => current_user.id, - :closed => true - ) - @merge_request.reload_code + @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 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 edf073d4e88..6d59ce563e1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -56,6 +56,10 @@ class MergeRequest < ActiveRecord::Base self.reloaded_diffs end + def can_be_merged? + auto_merge + end + def new? today? && created_at == updated_at end @@ -118,6 +122,11 @@ class MergeRequest < ActiveRecord::Base save end + def mark_as_unmergable + self.auto_merge = false + save + end + def reloaded_commits if open? && unmerged_commits.any? self.st_commits = unmerged_commits diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index ef83b1e49e7..ea46695364c 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -8,7 +8,6 @@ %span.right - if can?(current_user, :modify_merge_request, @merge_request) - if @merge_request.open? - = link_to "Show how to merge", "#", :class => "how_to_merge_link btn small padded", :title => "How To Merge" = link_to 'Close', project_merge_request_path(@project, @merge_request, :merge_request => {:closed => true }, :status_only => true), :method => :put, :class => "btn small padded", :title => "Close merge request" = link_to edit_project_merge_request_path(@project, @merge_request), :class => "btn small padded" do Edit @@ -53,6 +52,22 @@ Closed by #{@merge_request.closed_event.author_name} %small #{time_ago_in_words(@merge_request.closed_event.created_at)} ago. +- 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-actions + = link_to "Try Merge it!", automerge_project_merge_request_path(@project, @merge_request), :class => "btn small success" +   + = 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" + + = render "merge_requests/commits" - unless @commits.empty? diff --git a/db/migrate/20120329170745_add_automerge_to_merge_request.rb b/db/migrate/20120329170745_add_automerge_to_merge_request.rb new file mode 100644 index 00000000000..609c3094c8c --- /dev/null +++ b/db/migrate/20120329170745_add_automerge_to_merge_request.rb @@ -0,0 +1,6 @@ +class AddAutomergeToMergeRequest < ActiveRecord::Migration + def change + add_column :merge_requests, :auto_merge, :boolean, :null => false, :default => true + + end +end diff --git a/db/schema.rb b/db/schema.rb index 98993a9ae5e..ff03d504b24 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120323221339) do +ActiveRecord::Schema.define(:version => 20120329170745) do create_table "events", :force => true do |t| t.string "target_type" @@ -30,8 +30,8 @@ ActiveRecord::Schema.define(:version => 20120323221339) do t.integer "assignee_id" t.integer "author_id" t.integer "project_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false 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 => 20120323221339) do create_table "keys", :force => true do |t| t.integer "user_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.text "key" t.string "title" t.string "identifier" @@ -59,11 +59,12 @@ ActiveRecord::Schema.define(:version => 20120323221339) do t.integer "assignee_id" t.string "title" t.boolean "closed", :default => false, :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.text "st_commits" t.text "st_diffs" t.boolean "merged", :default => false, :null => false + t.boolean "auto_merge", :default => true, :null => false end add_index "merge_requests", ["project_id"], :name => "index_merge_requests_on_project_id" @@ -73,8 +74,8 @@ ActiveRecord::Schema.define(:version => 20120323221339) do t.string "noteable_id" t.string "noteable_type" t.integer "author_id" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.integer "project_id" t.string "attachment" t.string "line_code" @@ -87,8 +88,8 @@ ActiveRecord::Schema.define(:version => 20120323221339) do t.string "name" t.string "path" t.text "description" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.boolean "private_flag", :default => true, :null => false t.string "code" t.integer "owner_id" @@ -111,8 +112,8 @@ ActiveRecord::Schema.define(:version => 20120323221339) do t.text "content" t.integer "author_id", :null => false t.integer "project_id", :null => false - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "file_name" t.datetime "expires_at" end @@ -145,8 +146,8 @@ ActiveRecord::Schema.define(:version => 20120323221339) do t.datetime "last_sign_in_at" t.string "current_sign_in_ip" t.string "last_sign_in_ip" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.string "name" t.boolean "admin", :default => false, :null => false t.integer "projects_limit", :default => 10 @@ -165,16 +166,16 @@ ActiveRecord::Schema.define(:version => 20120323221339) 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "wikis", :force => true do |t| -- GitLab From 411d84f385364be987380da5f0216048073609c9 Mon Sep 17 00:00:00 2001 From: randx Date: Fri, 30 Mar 2012 08:05:04 +0300 Subject: [PATCH 03/11] 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 ++++----- ...29170745_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 b45338d7938..d0750f68135 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 6d59ce563e1..6eb54343d42 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 ea46695364c..1b8a25f2dd4 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 609c3094c8c..de7c68ee1cb 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 ff03d504b24..7b9d431de4b 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 85cc1667f65..35ae5d1d2c8 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 -- GitLab From 3824f9b372fe9451d101eec478440d129fb2d0b2 Mon Sep 17 00:00:00 2001 From: randx Date: Fri, 30 Mar 2012 08:15:04 +0300 Subject: [PATCH 04/11] Unverify MR on every push. Ajax for MR accept. better UI --- app/controllers/merge_requests_controller.rb | 4 ++-- app/models/merge_request.rb | 6 +++++- app/models/project/hooks_trait.rb | 2 +- app/views/merge_requests/automerge.js.haml | 2 ++ app/views/merge_requests/show.html.haml | 13 ++++++++----- 5 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 app/views/merge_requests/automerge.js.haml diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index d0750f68135..f4b97de93a6 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -90,6 +90,7 @@ class MergeRequestsController < ApplicationController respond_to do |format| if @merge_request.update_attributes(params[:merge_request].merge(:author_id_of_changes => current_user.id)) @merge_request.reload_code + @merge_request.mark_as_unchecked format.html { redirect_to [@project, @merge_request], notice: 'Merge request was successfully updated.' } format.json { head :ok } else @@ -101,8 +102,7 @@ class MergeRequestsController < ApplicationController def automerge 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 + @merge_request.automerge!(current_user) end def destroy diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 6eb54343d42..85492e802e3 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -64,6 +64,10 @@ class MergeRequest < ActiveRecord::Base state == UNCHECKED end + def mark_as_unchecked + self.update_attributes(:state => UNCHECKED) + end + def can_be_merged? state == CAN_BE_MERGED end @@ -170,7 +174,7 @@ class MergeRequest < ActiveRecord::Base ) end - def automerge! + def automerge!(current_user) if GitlabMerge.new(self).merge self.merge!(current_user.id) true diff --git a/app/models/project/hooks_trait.rb b/app/models/project/hooks_trait.rb index fc32cf4c277..fc8c98d45dd 100644 --- a/app/models/project/hooks_trait.rb +++ b/app/models/project/hooks_trait.rb @@ -19,7 +19,7 @@ module Project::HooksTrait # Update code for merge requests mrs = self.merge_requests.opened.find_all_by_branch(branch_name).all - mrs.each { |merge_request| merge_request.reload_code } + mrs.each { |merge_request| merge_request.reload_code; merge_request.mark_as_unchecked } # Close merge requests mrs = self.merge_requests.opened.where(:target_branch => branch_name).all diff --git a/app/views/merge_requests/automerge.js.haml b/app/views/merge_requests/automerge.js.haml new file mode 100644 index 00000000000..5827a518b51 --- /dev/null +++ b/app/views/merge_requests/automerge.js.haml @@ -0,0 +1,2 @@ +:plain + location.reload(); diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index 1b8a25f2dd4..7e14a61c661 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -54,12 +54,11 @@ - if @merge_request.open? && @commits.any? - if @merge_request.can_be_merged? - .alert-message.info + .ui-box.padded %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 "Accept Merge Request", automerge_project_merge_request_path(@project, @merge_request), :class => "btn small info" -   + You can accept this request automatically. If you still want to do it manually - #{link_to "click here", "#", :class => "how_to_merge_link vlink", :title => "How To Merge"} for instructions + = link_to "Accept Merge Request", automerge_project_merge_request_path(@project, @merge_request), :class => "btn small info accept_merge_request", :remote => true +   - else .alert-message @@ -88,6 +87,10 @@ :javascript $(function(){ MergeRequest.init(); + + $(".accept_merge_request").live("ajax:beforeSend", function() { + $(this).replaceWith('#{image_tag "ajax_loader.gif"}'); + }) }) = render "notes/per_line_form" -- GitLab From 464cd59dff0e543cc367c0d468a70ca590334e42 Mon Sep 17 00:00:00 2001 From: randx Date: Fri, 30 Mar 2012 08:25:04 +0300 Subject: [PATCH 05/11] Rake task to provider full repo access for gitolite-owner.\n Automerge requires gitlab user be able to push to any repo --- lib/gitlabhq/gitolite.rb | 29 ++++++++++++++++++++++++++ lib/tasks/gitlab_enable_automerge.rake | 9 ++++++++ 2 files changed, 38 insertions(+) create mode 100644 lib/tasks/gitlab_enable_automerge.rake diff --git a/lib/gitlabhq/gitolite.rb b/lib/gitlabhq/gitolite.rb index 701b9a8f8b4..fabeb7d0fca 100644 --- a/lib/gitlabhq/gitolite.rb +++ b/lib/gitlabhq/gitolite.rb @@ -123,5 +123,34 @@ module Gitlabhq repo end + + def admin_all_repo + ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(@local_dir,'gitolite')) + conf = ga_repo.config + owner_name = "" + + # Read gitolite-admin user + # + begin + repo = conf.get_repo("gitolite-admin") + owner_name = repo.permissions[0]["RW+"][""][0] + raise StandardError if owner_name.blank? + rescue => ex + puts "Cant determine gitolite-admin owner".red + raise StandardError + end + + # @ALL repos premission for gitolite owner + repo_name = "@all" + repo = if conf.has_repo?(repo_name) + conf.get_repo(repo_name) + else + ::Gitolite::Config::Repo.new(repo_name) + end + + repo.add_permission("RW+", "", owner_name) + conf.add_repo(repo, true) + ga_repo.save + end end end diff --git a/lib/tasks/gitlab_enable_automerge.rake b/lib/tasks/gitlab_enable_automerge.rake new file mode 100644 index 00000000000..6ff5003eac1 --- /dev/null +++ b/lib/tasks/gitlab_enable_automerge.rake @@ -0,0 +1,9 @@ +desc "Give gitlab user full access to every repo" +task :gitlab_enable_automerge => :environment do + + Gitlabhq::GitHost.system.new.configure do |git| + git.admin_all_repo + end + + puts "Done!".green +end -- GitLab From 90748cf724d4b019c954d316621b04cefd69dbaf Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 30 Mar 2012 20:49:34 +0300 Subject: [PATCH 06/11] Merge Button v1 complete --- app/controllers/merge_requests_controller.rb | 1 + app/models/ability.rb | 1 + app/models/merge_request.rb | 4 +-- app/views/merge_requests/show.html.haml | 3 +- db/schema.rb | 38 ++++++++++---------- lib/gitlab_merge.rb | 9 +++-- 6 files changed, 31 insertions(+), 25 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index f4b97de93a6..b3776c0af27 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -102,6 +102,7 @@ class MergeRequestsController < ApplicationController def automerge render_404 unless @merge_request.open? && @merge_request.can_be_merged? + return access_denied! unless can?(current_user, :accept_mr, @project) @merge_request.automerge!(current_user) end diff --git a/app/models/ability.rb b/app/models/ability.rb index e97b662b8ce..8ff45b6d17e 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -46,6 +46,7 @@ class Ability :admin_team_member, :admin_merge_request, :admin_note, + :accept_mr, :admin_wiki ] if project.master_access_for?(user) || project.owner == user diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 85492e802e3..d7863c2edb0 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -73,7 +73,7 @@ class MergeRequest < ActiveRecord::Base end def check_if_can_be_merged - self.state = if GitlabMerge.new(self).can_be_merged? + self.state = if GitlabMerge.new(self, self.author).can_be_merged? CAN_BE_MERGED else CANNOT_BE_MERGED @@ -175,7 +175,7 @@ class MergeRequest < ActiveRecord::Base end def automerge!(current_user) - if GitlabMerge.new(self).merge + if GitlabMerge.new(self, current_user).merge self.merge!(current_user.id) true end diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index 7e14a61c661..68c165605ea 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -52,7 +52,8 @@ Closed by #{@merge_request.closed_event.author_name} %small #{time_ago_in_words(@merge_request.closed_event.created_at)} ago. -- if @merge_request.open? && @commits.any? + +- if @merge_request.open? && @commits.any? && can?(current_user, :accept_mr, @project) - if @merge_request.can_be_merged? .ui-box.padded %p diff --git a/db/schema.rb b/db/schema.rb index 7b9d431de4b..ff03d504b24 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false t.text "st_commits" t.text "st_diffs" t.boolean "merged", :default => false, :null => false - t.integer "state", :default => 1, :null => false + t.boolean "auto_merge", :default => true, :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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false 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" - t.datetime "updated_at" + t.datetime "created_at", :null => false + t.datetime "updated_at", :null => false end create_table "wikis", :force => true do |t| diff --git a/lib/gitlab_merge.rb b/lib/gitlab_merge.rb index 35ae5d1d2c8..3913aa647cf 100644 --- a/lib/gitlab_merge.rb +++ b/lib/gitlab_merge.rb @@ -1,7 +1,8 @@ class GitlabMerge - attr_accessor :project, :merge_path, :merge_request + attr_accessor :project, :merge_path, :merge_request, :user - def initialize(merge_request) + def initialize(merge_request, user) + self.user = user self.merge_request = merge_request self.project = merge_request.project self.merge_path = File.join(Rails.root, "tmp", "merge_repo", project.path, merge_request.id.to_s) @@ -30,7 +31,9 @@ class GitlabMerge self.project.repo.git.clone({:branch => merge_request.target_branch}, project.url_to_repo, merge_path) Dir.chdir(merge_path) do merge_repo = Grit::Repo.new('.') - output = merge_repo.git.pull({}, "origin", merge_request.source_branch) + merge_repo.git.sh "git config user.name \"#{user.name}\"" + merge_repo.git.sh "git config user.email \"#{user.email}\"" + output = merge_repo.git.pull({}, "--no-ff", "origin", merge_request.source_branch) yield(merge_repo, output) end end -- GitLab From f145450415ed1e9db10ebcb8e02eb15c6dcffbf1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sat, 31 Mar 2012 13:48:30 +0300 Subject: [PATCH 07/11] fixed mr unmergable method --- app/models/merge_request.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index d7863c2edb0..3845a0eced2 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -144,7 +144,7 @@ class MergeRequest < ActiveRecord::Base end def mark_as_unmergable - # TODO: write some code here + self.update_attributes :state => CANNOT_BE_MERGED end def reloaded_commits -- GitLab From 8269a3a735d6b1195ef0263716845ef3d2ab3327 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 16 Apr 2012 22:08:03 +0300 Subject: [PATCH 08/11] Auto-merge: checking via AJAX --- app/assets/javascripts/merge_requests.js | 12 +++++++++++- app/controllers/merge_requests_controller.rb | 13 ++++++++----- app/models/merge_request.rb | 9 +++++++++ app/views/merge_requests/show.html.haml | 17 +++++++++++++---- config/routes.rb | 1 + lib/gitlab_merge.rb | 1 + 6 files changed, 43 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/merge_requests.js b/app/assets/javascripts/merge_requests.js index b051928e0de..eba9c9cab43 100644 --- a/app/assets/javascripts/merge_requests.js +++ b/app/assets/javascripts/merge_requests.js @@ -1,9 +1,19 @@ var MergeRequest = { diffs_loaded: false, commits_loaded: false, + opts: false, init: - function() { + function(opts) { + this.opts = opts; + + if($(".automerge_widget").lenght){ + $.get(opts.url_to_automerge_check, function(data){ + $(".automerge_widget").hide(); + $(".automerge_widget." + data.state).show(); + }, "json"); + } + $(".tabs a").live("click", function() { $(".tabs a").parent().removeClass("active"); $(this).parent().addClass("active"); diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index b3776c0af27..18ad578fc83 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -2,7 +2,7 @@ class MergeRequestsController < ApplicationController before_filter :authenticate_user! before_filter :project before_filter :module_enabled - before_filter :merge_request, :only => [:edit, :update, :destroy, :show, :commits, :diffs, :automerge] + before_filter :merge_request, :only => [:edit, :update, :destroy, :show, :commits, :diffs, :automerge, :automerge_check] layout "project" # Authorize @@ -45,10 +45,6 @@ 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 @@ -100,6 +96,13 @@ class MergeRequestsController < ApplicationController end end + def automerge_check + if @merge_request.unchecked? + @merge_request.check_if_can_be_merged + end + render :json => {:state => @merge_request.human_state} + end + def automerge render_404 unless @merge_request.open? && @merge_request.can_be_merged? return access_denied! unless can?(current_user, :accept_mr, @project) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 3845a0eced2..248bbdacad4 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -49,6 +49,15 @@ class MergeRequest < ActiveRecord::Base where("source_branch like :branch or target_branch like :branch", :branch => branch_name) end + def human_state + states = { + CAN_BE_MERGED => "can_be_merged", + CANNOT_BE_MERGED => "cannot_be_merged", + UNCHECKED => "unchecked" + } + states[self.state] + end + def validate_branches if target_branch == source_branch errors.add :base, "You can not use same branch for source and target branches" diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index 68c165605ea..aef74b8dce6 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -54,18 +54,25 @@ - if @merge_request.open? && @commits.any? && can?(current_user, :accept_mr, @project) - - if @merge_request.can_be_merged? + .automerge_widget.can_be_merged{:style => "display:none"} .ui-box.padded %p You can accept this request automatically. If you still want to do it manually - #{link_to "click here", "#", :class => "how_to_merge_link vlink", :title => "How To Merge"} for instructions = link_to "Accept Merge Request", automerge_project_merge_request_path(@project, @merge_request), :class => "btn small info accept_merge_request", :remote => true   - - - else + + .automerge_widget.cannot_be_merged{:style => "display:none"} .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" + + .automerge_widget.unchecked + .alert-message + %p + %strong Checking for ability to automatically merge… + + = render "merge_requests/commits" @@ -87,7 +94,9 @@ :javascript $(function(){ - MergeRequest.init(); + MergeRequest.init({ + url_to_automerge_check: "#{automerge_check_project_merge_request_path(@project, @merge_request)}", + }); $(".accept_merge_request").live("ajax:beforeSend", function() { $(this).replaceWith('#{image_tag "ajax_loader.gif"}'); diff --git a/config/routes.rb b/config/routes.rb index a1c54565f49..9118c19f8f0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -103,6 +103,7 @@ Gitlab::Application.routes.draw do member do get :diffs get :automerge + get :automerge_check end collection do diff --git a/lib/gitlab_merge.rb b/lib/gitlab_merge.rb index 3913aa647cf..421750b7638 100644 --- a/lib/gitlab_merge.rb +++ b/lib/gitlab_merge.rb @@ -29,6 +29,7 @@ class GitlabMerge def pull self.project.repo.git.clone({:branch => merge_request.target_branch}, project.url_to_repo, merge_path) + #TODO When user do not have permissions then raise exception Dir.chdir(merge_path) do merge_repo = Grit::Repo.new('.') merge_repo.git.sh "git config user.name \"#{user.name}\"" -- GitLab From a8e4f3ed888f27629a5e35174cdb19cb6c751c16 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 16 Apr 2012 23:05:08 +0300 Subject: [PATCH 09/11] Auto-merge: 'already can not be merged' alert --- app/assets/javascripts/merge_requests.js | 8 +++++++- app/controllers/merge_requests_controller.rb | 8 ++++++-- app/views/merge_requests/automerge.js.haml | 9 +++++++-- app/views/merge_requests/show.html.haml | 5 +++++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/merge_requests.js b/app/assets/javascripts/merge_requests.js index eba9c9cab43..132a05739d9 100644 --- a/app/assets/javascripts/merge_requests.js +++ b/app/assets/javascripts/merge_requests.js @@ -7,7 +7,7 @@ var MergeRequest = { function(opts) { this.opts = opts; - if($(".automerge_widget").lenght){ + if($(".automerge_widget").length){ $.get(opts.url_to_automerge_check, function(data){ $(".automerge_widget").hide(); $(".automerge_widget." + data.state).show(); @@ -48,5 +48,11 @@ var MergeRequest = { $('.status').removeClass("loading"); }, dataType: "script"}); + }, + + already_cannot_be_merged: + function(){ + $(".automerge_widget").hide(); + $(".automerge_widget.already_cannot_be_merged").show(); } } diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 18ad578fc83..1077cef718d 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -104,9 +104,13 @@ class MergeRequestsController < ApplicationController end def automerge - render_404 unless @merge_request.open? && @merge_request.can_be_merged? return access_denied! unless can?(current_user, :accept_mr, @project) - @merge_request.automerge!(current_user) + if @merge_request.open? && @merge_request.can_be_merged? + @merge_request.automerge!(current_user) + @status = true + else + @status = false + end end def destroy diff --git a/app/views/merge_requests/automerge.js.haml b/app/views/merge_requests/automerge.js.haml index 5827a518b51..93e184455af 100644 --- a/app/views/merge_requests/automerge.js.haml +++ b/app/views/merge_requests/automerge.js.haml @@ -1,2 +1,7 @@ -:plain - location.reload(); +-if @status + :plain + location.reload(); +-else + :plain + MergeRequest.already_cannot_be_merged() + diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index aef74b8dce6..1b23e5b225f 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -71,6 +71,11 @@ .alert-message %p %strong Checking for ability to automatically merge… + + .automerge_widget.already_cannot_be_merged{:style => "display:none"} + .alert-message + %p + %strong This merge request already can not be merged -- GitLab From 5abbada41e101ff832d036d5d1946d6f89fda771 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 16 Apr 2012 23:37:04 +0300 Subject: [PATCH 10/11] Auto-merge: implemented lock file --- lib/gitlab_merge.rb | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/gitlab_merge.rb b/lib/gitlab_merge.rb index 421750b7638..95942d5ca50 100644 --- a/lib/gitlab_merge.rb +++ b/lib/gitlab_merge.rb @@ -28,14 +28,19 @@ class GitlabMerge end def pull - self.project.repo.git.clone({:branch => merge_request.target_branch}, project.url_to_repo, merge_path) - #TODO When user do not have permissions then raise exception - Dir.chdir(merge_path) do - merge_repo = Grit::Repo.new('.') - merge_repo.git.sh "git config user.name \"#{user.name}\"" - merge_repo.git.sh "git config user.email \"#{user.email}\"" - output = merge_repo.git.pull({}, "--no-ff", "origin", merge_request.source_branch) - yield(merge_repo, output) + File.open(File.join(Rails.root, "tmp", "merge_repo", "#{project.path}.lock"), "w+") do |f| + f.flock(File::LOCK_EX) + + self.project.repo.git.clone({:branch => merge_request.target_branch}, project.url_to_repo, merge_path) + #TODO When user do not have permissions then raise exception + Dir.chdir(merge_path) do + merge_repo = Grit::Repo.new('.') + merge_repo.git.sh "git config user.name \"#{user.name}\"" + merge_repo.git.sh "git config user.email \"#{user.email}\"" + output = merge_repo.git.pull({}, "--no-ff", "origin", merge_request.source_branch) + yield(merge_repo, output) + end + end end end -- GitLab From 3a6694b5516f4c5015a7745cf8182e74597b39ad Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 16 Apr 2012 23:49:57 +0300 Subject: [PATCH 11/11] Auto-merge: reise exception when gitlab do not have access to repo --- lib/gitlab_merge.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/gitlab_merge.rb b/lib/gitlab_merge.rb index 95942d5ca50..66a0e7ec635 100644 --- a/lib/gitlab_merge.rb +++ b/lib/gitlab_merge.rb @@ -32,7 +32,9 @@ class GitlabMerge f.flock(File::LOCK_EX) self.project.repo.git.clone({:branch => merge_request.target_branch}, project.url_to_repo, merge_path) - #TODO When user do not have permissions then raise exception + unless File.exist?(self.merge_path) + raise "Gitlab user do not have access to repo. You should run: rake gitlab_enable_automerge" + end Dir.chdir(merge_path) do merge_repo = Grit::Repo.new('.') merge_repo.git.sh "git config user.name \"#{user.name}\"" -- GitLab