From 5a3f23f395eef87bf3457e6474bac82333e71ec8 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Date: Thu, 15 Mar 2012 00:57:43 +0200 Subject: [PATCH] Persist Merge Request diff. Auto merge request close on push --- app/controllers/merge_requests_controller.rb | 13 +++-- app/models/event.rb | 1 + app/models/merge_request.rb | 55 ++++++++++++++++++- app/models/project.rb | 34 ++++++++++++ app/views/merge_requests/_commits.html.haml | 4 +- app/views/merge_requests/show.html.haml | 8 +-- app/workers/post_receive.rb | 6 ++ ...add_commits_diff_store_to_merge_request.rb | 6 ++ ...20315132931_add_merged_to_merge_request.rb | 5 ++ db/schema.rb | 17 +----- 10 files changed, 121 insertions(+), 28 deletions(-) create mode 100644 db/migrate/20120315111711_add_commits_diff_store_to_merge_request.rb create mode 100644 db/migrate/20120315132931_add_merged_to_merge_request.rb diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index d1d19efc98c..2de7ec31a43 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -41,13 +41,12 @@ class MergeRequestsController < ApplicationController @note = @project.notes.new(:noteable => @merge_request) - @commits = @project.repo. - commits_between(@merge_request.target_branch, @merge_request.source_branch). - map {|c| Commit.new(c)}. - sort_by(&:created_at). - reverse + # Get commits from repository + # or from cache if already merged + @commits = @merge_request.commits - render_full_content + # Close MR if nothing to merge + #@merge_request.mark_as_merged! if @merge_request.probably_merged? respond_to do |format| format.html @@ -76,6 +75,8 @@ class MergeRequestsController < ApplicationController respond_to do |format| if @merge_request.save + @merge_request.reloaded_commits + @merge_request.reloaded_diffs format.html { redirect_to [@project, @merge_request], notice: 'Merge request was successfully created.' } format.json { render json: @merge_request, status: :created, location: @merge_request } else diff --git a/app/models/event.rb b/app/models/event.rb index 8ab127c02a0..c8af9363133 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -7,6 +7,7 @@ class Event < ActiveRecord::Base Reopened = 4 Pushed = 5 Commented = 6 + Merged = 7 belongs_to :project belongs_to :target, :polymorphic => true diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 9bbbcd472aa..6782d738ca5 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1,9 +1,14 @@ +require File.join(Rails.root, "app/models/commit") + class MergeRequest < ActiveRecord::Base belongs_to :project belongs_to :author, :class_name => "User" belongs_to :assignee, :class_name => "User" has_many :notes, :as => :noteable, :dependent => :destroy + serialize :st_commits + serialize :st_diffs + attr_protected :author, :author_id, :project, :project_id attr_accessor :author_id_of_changes @@ -32,7 +37,6 @@ class MergeRequest < ActiveRecord::Base scope :closed, where(:closed => true) scope :assigned, lambda { |u| where(:assignee_id => u.id)} - def validate_branches if target_branch == source_branch errors.add :base, "You can not use same branch for source and target branches" @@ -44,18 +48,65 @@ class MergeRequest < ActiveRecord::Base end def diffs + st_diffs || [] + end + + def reloaded_diffs + if open? && unmerged_diffs.any? + self.st_diffs = unmerged_diffs + save + end + diffs + end + + def unmerged_diffs commits = project.repo.commits_between(target_branch, source_branch).map {|c| Commit.new(c)} diffs = project.repo.diff(commits.first.prev_commit.id, commits.last.id) rescue [] end def last_commit - project.commit(source_branch) + commits.first end # Return the number of +1 comments (upvotes) def upvotes notes.select(&:upvote?).size end + + def commits + st_commits || [] + end + + def probably_merged? + unmerged_commits.empty? && + commits.any? && open? + end + + def open? + !closed + end + + def mark_as_merged! + self.merged = true + self.closed = true + save + end + + def reloaded_commits + if open? && unmerged_commits.any? + self.st_commits = unmerged_commits + save + end + commits + end + + def unmerged_commits + self.project.repo. + commits_between(self.target_branch, self.source_branch). + map {|c| Commit.new(c)}. + sort_by(&:created_at). + reverse + end end # == Schema Information # diff --git a/app/models/project.rb b/app/models/project.rb index 2ba0c9077bc..28f0329784b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -73,6 +73,40 @@ class Project < ActiveRecord::Base ) end + def update_merge_requests(oldrev, newrev, ref, author_key_id) + return true unless ref =~ /heads/ + branch_name = ref.gsub("refs/heads/", "") + + key = Key.find_by_identifier(author_key_id) + user = key.user + + c_ids = self.commits_between(oldrev, newrev).map(&:id) + + # update commits & diffs for existing MR + mrs = self.merge_requests.opened.where(:source_branch => branch_name).all + mrs.each do |merge_request| + merge_request.reloaded_commits + merge_request.reloaded_diffs + end + + # Close merge requests + mrs = self.merge_requests.opened.where(:target_branch => branch_name).all + mrs.each do |merge_request| + next unless merge_request.last_commit + # Mark as merged & create event if merged + if c_ids.include?(merge_request.last_commit.id) + merge_request.mark_as_merged! + Event.create( + :project => self, + :action => Event::Merged, + :data => {:merge_request_id => merge_request.id}, + :author_id => user.id + ) + end + end + true + end + def execute_web_hooks(oldrev, newrev, ref, author_key_id) ref_parts = ref.split('/') diff --git a/app/views/merge_requests/_commits.html.haml b/app/views/merge_requests/_commits.html.haml index 7606e71e7c0..60cb21ba16e 100644 --- a/app/views/merge_requests/_commits.html.haml +++ b/app/views/merge_requests/_commits.html.haml @@ -2,7 +2,9 @@ .ui-box %h5 Commits .merge-request-commits - %ul.unstyled= render @commits + %ul.unstyled + - @commits.each do |commit| + = render "commits/commit", :commit => commit - else %h5 diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index 77bb6120ec7..7b9e31df70e 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -11,12 +11,10 @@ %span.right - if can?(current_user, :modify_merge_request, @merge_request) - - if @merge_request.closed - = link_to 'Reopen', project_merge_request_path(@project, @merge_request, :merge_request => {:closed => false }, :status_only => true), :method => :put, :class => "btn" - - else + - if @merge_request.open? = link_to 'Close', project_merge_request_path(@project, @merge_request, :merge_request => {:closed => true }, :status_only => true), :method => :put, :class => "btn", :title => "Close merge request" - = link_to edit_project_merge_request_path(@project, @merge_request), :class => "btn" do - Edit + = link_to edit_project_merge_request_path(@project, @merge_request), :class => "btn" do + Edit - if @merge_request.upvotes > 0 = link_to "#notes", :class => "btn success" do diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index d74f10a1ce5..28216ec33da 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -8,7 +8,13 @@ class PostReceive # Ignore push from non-gitlab users return false unless Key.find_by_identifier(author_key_id) + # Create push event project.observe_push(oldrev, newrev, ref, author_key_id) + + # Close merged MR + project.update_merge_requests(oldrev, newrev, ref, author_key_id) + + # Execute web hooks project.execute_web_hooks(oldrev, newrev, ref, author_key_id) end end diff --git a/db/migrate/20120315111711_add_commits_diff_store_to_merge_request.rb b/db/migrate/20120315111711_add_commits_diff_store_to_merge_request.rb new file mode 100644 index 00000000000..2dc1dfb4b6e --- /dev/null +++ b/db/migrate/20120315111711_add_commits_diff_store_to_merge_request.rb @@ -0,0 +1,6 @@ +class AddCommitsDiffStoreToMergeRequest < ActiveRecord::Migration + def change + add_column :merge_requests, :st_commits, :text, :null => true + add_column :merge_requests, :st_diffs, :text, :null => true + end +end diff --git a/db/migrate/20120315132931_add_merged_to_merge_request.rb b/db/migrate/20120315132931_add_merged_to_merge_request.rb new file mode 100644 index 00000000000..1db590ae416 --- /dev/null +++ b/db/migrate/20120315132931_add_merged_to_merge_request.rb @@ -0,0 +1,5 @@ +class AddMergedToMergeRequest < ActiveRecord::Migration + def change + add_column :merge_requests, :merged, :true, :null => false, :default => false + end +end diff --git a/db/schema.rb b/db/schema.rb index 45e18a68cf3..886fdf8f426 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 => 20120307095918) do +ActiveRecord::Schema.define(:version => 20120315132931) do create_table "events", :force => true do |t| t.string "target_type" @@ -50,19 +50,8 @@ ActiveRecord::Schema.define(:version => 20120307095918) do t.integer "project_id" end - create_table "merge_requests", :force => true do |t| - t.string "target_branch", :null => false - t.string "source_branch", :null => false - t.integer "project_id", :null => false - t.integer "author_id" - 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 - end - - add_index "merge_requests", ["project_id"], :name => "index_merge_requests_on_project_id" +# Could not dump table "merge_requests" because of following StandardError +# Unknown type 'true' for column 'merged' create_table "notes", :force => true do |t| t.text "note" -- 2.30.9