Persist Merge Request diff. Auto merge request close on push

Dmitriy Zaporozhets
1 parent 27e36998
Showing 10 changed files with 121 additions and 28 deletions Show diff stats
app/controllers/merge_requests_controller.rb
app/models/event.rb
app/models/merge_request.rb
app/models/project.rb
app/views/merge_requests/_commits.html.haml
app/views/merge_requests/show.html.haml
app/workers/post_receive.rb
db/migrate/20120315111711_add_commits_diff_store_to_merge_request.rb
db/migrate/20120315132931_add_merged_to_merge_request.rb
db/schema.rb
@@ -41,13 +41,12 @@ class MergeRequestsController &lt; 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 &lt; 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
@@ -7,6 +7,7 @@ class Event &lt; ActiveRecord::Base
   Reopened  = 4
   Pushed    = 5
   Commented = 6
+  Merged    = 7
   belongs_to :project
   belongs_to :target, :polymorphic => true
+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 &lt; 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 &lt; 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
 #
@@ -73,6 +73,40 @@ class Project &lt; 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('/')
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,5 @@
+class AddMergedToMergeRequest < ActiveRecord::Migration
+  def change
+    add_column :merge_requests, :merged, :true, :null => false, :default => false
+  end
+end
@@ -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 =&gt; 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"
	@@ -7,6 +7,7 @@ class Event < ActiveRecord::Base		@@ -7,6 +7,7 @@ class Event < ActiveRecord::Base
7	Reopened = 4	7	Reopened = 4
8	Pushed = 5	8	Pushed = 5
9	Commented = 6	9	Commented = 6
		10	+ Merged = 7
10		11
11	belongs_to :project	12	belongs_to :project
12	belongs_to :target, :polymorphic => true	13	belongs_to :target, :polymorphic => true
		1	+require File.join(Rails.root, "app/models/commit")
		2	+
1	class MergeRequest < ActiveRecord::Base	3	class MergeRequest < ActiveRecord::Base
2	belongs_to :project	4	belongs_to :project
3	belongs_to :author, :class_name => "User"	5	belongs_to :author, :class_name => "User"
4	belongs_to :assignee, :class_name => "User"	6	belongs_to :assignee, :class_name => "User"
5	has_many :notes, :as => :noteable, :dependent => :destroy	7	has_many :notes, :as => :noteable, :dependent => :destroy
6		8
		9	+ serialize :st_commits
		10	+ serialize :st_diffs
		11	+
7	attr_protected :author, :author_id, :project, :project_id	12	attr_protected :author, :author_id, :project, :project_id
8	attr_accessor :author_id_of_changes	13	attr_accessor :author_id_of_changes
9		14
	@@ -32,7 +37,6 @@ class MergeRequest < ActiveRecord::Base		@@ -32,7 +37,6 @@ class MergeRequest < ActiveRecord::Base
32	scope :closed, where(:closed => true)	37	scope :closed, where(:closed => true)
33	scope :assigned, lambda { \|u\| where(:assignee_id => u.id)}	38	scope :assigned, lambda { \|u\| where(:assignee_id => u.id)}
34		39
35	-
36	def validate_branches	40	def validate_branches
37	if target_branch == source_branch	41	if target_branch == source_branch
38	errors.add :base, "You can not use same branch for source and target branches"	42	errors.add :base, "You can not use same branch for source and target branches"
	@@ -44,18 +48,65 @@ class MergeRequest < ActiveRecord::Base		@@ -44,18 +48,65 @@ class MergeRequest < ActiveRecord::Base
44	end	48	end
45		49
46	def diffs	50	def diffs
		51	+ st_diffs \|\| []
		52	+ end
		53	+
		54	+ def reloaded_diffs
		55	+ if open? && unmerged_diffs.any?
		56	+ self.st_diffs = unmerged_diffs
		57	+ save
		58	+ end
		59	+ diffs
		60	+ end
		61	+
		62	+ def unmerged_diffs
47	commits = project.repo.commits_between(target_branch, source_branch).map {\|c\| Commit.new(c)}	63	commits = project.repo.commits_between(target_branch, source_branch).map {\|c\| Commit.new(c)}
48	diffs = project.repo.diff(commits.first.prev_commit.id, commits.last.id) rescue []	64	diffs = project.repo.diff(commits.first.prev_commit.id, commits.last.id) rescue []
49	end	65	end
50		66
51	def last_commit	67	def last_commit
52	- project.commit(source_branch)	68	+ commits.first
53	end	69	end
54		70
55	# Return the number of +1 comments (upvotes)	71	# Return the number of +1 comments (upvotes)
56	def upvotes	72	def upvotes
57	notes.select(&:upvote?).size	73	notes.select(&:upvote?).size
58	end	74	end
		75	+
		76	+ def commits
		77	+ st_commits \|\| []
		78	+ end
		79	+
		80	+ def probably_merged?
		81	+ unmerged_commits.empty? &&
		82	+ commits.any? && open?
		83	+ end
		84	+
		85	+ def open?
		86	+ !closed
		87	+ end
		88	+
		89	+ def mark_as_merged!
		90	+ self.merged = true
		91	+ self.closed = true
		92	+ save
		93	+ end
		94	+
		95	+ def reloaded_commits
		96	+ if open? && unmerged_commits.any?
		97	+ self.st_commits = unmerged_commits
		98	+ save
		99	+ end
		100	+ commits
		101	+ end
		102	+
		103	+ def unmerged_commits
		104	+ self.project.repo.
		105	+ commits_between(self.target_branch, self.source_branch).
		106	+ map {\|c\| Commit.new(c)}.
		107	+ sort_by(&:created_at).
		108	+ reverse
		109	+ end
59	end	110	end
60	# == Schema Information	111	# == Schema Information
61	#	112	#
	@@ -73,6 +73,40 @@ class Project < ActiveRecord::Base		@@ -73,6 +73,40 @@ class Project < ActiveRecord::Base
73	)	73	)
74	end	74	end
75		75
		76	+ def update_merge_requests(oldrev, newrev, ref, author_key_id)
		77	+ return true unless ref =~ /heads/
		78	+ branch_name = ref.gsub("refs/heads/", "")
		79	+
		80	+ key = Key.find_by_identifier(author_key_id)
		81	+ user = key.user
		82	+
		83	+ c_ids = self.commits_between(oldrev, newrev).map(&:id)
		84	+
		85	+ # update commits & diffs for existing MR
		86	+ mrs = self.merge_requests.opened.where(:source_branch => branch_name).all
		87	+ mrs.each do \|merge_request\|
		88	+ merge_request.reloaded_commits
		89	+ merge_request.reloaded_diffs
		90	+ end
		91	+
		92	+ # Close merge requests
		93	+ mrs = self.merge_requests.opened.where(:target_branch => branch_name).all
		94	+ mrs.each do \|merge_request\|
		95	+ next unless merge_request.last_commit
		96	+ # Mark as merged & create event if merged
		97	+ if c_ids.include?(merge_request.last_commit.id)
		98	+ merge_request.mark_as_merged!
		99	+ Event.create(
		100	+ :project => self,
		101	+ :action => Event::Merged,
		102	+ :data => {:merge_request_id => merge_request.id},
		103	+ :author_id => user.id
		104	+ )
		105	+ end
		106	+ end
		107	+ true
		108	+ end
		109	+
76	def execute_web_hooks(oldrev, newrev, ref, author_key_id)	110	def execute_web_hooks(oldrev, newrev, ref, author_key_id)
77	ref_parts = ref.split('/')	111	ref_parts = ref.split('/')
78		112
@@ -0,0 +1,6 @@		@@ -0,0 +1,6 @@
	1	+class AddCommitsDiffStoreToMergeRequest < ActiveRecord::Migration
	2	+ def change
	3	+ add_column :merge_requests, :st_commits, :text, :null => true
	4	+ add_column :merge_requests, :st_diffs, :text, :null => true
	5	+ end
	6	+end
@@ -0,0 +1,5 @@		@@ -0,0 +1,5 @@
	1	+class AddMergedToMergeRequest < ActiveRecord::Migration
	2	+ def change
	3	+ add_column :merge_requests, :merged, :true, :null => false, :default => false
	4	+ end
	5	+end