Merge Request on forked projects

The good:

 - You can do a merge request for a forked commit and it will merge properly (i.e. it does work).
 - Push events take into account merge requests on forked projects
 - Tests around merge_actions now present, spinach, and other rspec tests
 - Satellites now clean themselves up rather then recreate

The questionable:

 - Events only know about target projects
 - Project's merge requests only hold on to MR's where they are the target
 - All operations performed in the satellite

The bad:

  -  Duplication between project's repositories and satellites (e.g. commits_between)

(for reference: http://feedback.gitlab.com/forums/176466-general/suggestions/3456722-merge-requests-between-projects-repos)

Fixes:

Make test repos/satellites only create when needed
-Spinach/Rspec now only initialize test directory, and setup stubs (things that are relatively cheap)
-project_with_code, source_project_with_code, and target_project_with_code now create/destroy their repos individually
-fixed remote removal
-How to merge renders properly
-Update emails to show project/branches
-Edit MR doesn't set target branch
-Fix some failures on editing/creating merge requests, added a test
-Added back a test around merge request observer
-Clean up project_transfer_spec, Remove duplicate enable/disable observers
-Ensure satellite lock files are cleaned up, Attempted to add some testing around these as well
-Signifant speed ups for tests
-Update formatting ordering in notes_on_merge_requests
-Remove wiki schema update
Fixes for search/search results
-Search results was using by_project for a list of projects, updated this to use in_projects
-updated search results to reference the correct (target) project
-udpated search results to print both sides of the merge request

Change-Id: I19407990a0950945cc95d62089cbcc6262dab1a8

5 files changed, 100 insertions, 60 deletions

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 8868e818daa..88de1a037aa 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -9,19 +9,14 @@ module Issuable
   include Mentionable
 
   included do
-    belongs_to :project
     belongs_to :author, class_name: "User"
     belongs_to :assignee, class_name: "User"
     belongs_to :milestone
     has_many :notes, as: :noteable, dependent: :destroy
 
-    validates :project, presence: true
     validates :author, presence: true
     validates :title, presence: true, length: { within: 0..255 }
 
-    scope :opened, -> { with_state(:opened) }
-    scope :closed, -> { with_state(:closed) }
-    scope :of_group, ->(group) { where(project_id: group.project_ids) }
     scope :assigned_to, ->(u) { where(assignee_id: u.id)}
     scope :recent, -> { order("created_at DESC") }
     scope :assigned, -> { where("assignee_id IS NOT NULL") }
diff --git a/app/models/issue.rb b/app/models/issue.rb
index f56928891dc..c171941928c 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -17,8 +17,18 @@
 #
 
 class Issue < ActiveRecord::Base
+
   include Issuable
 
+  belongs_to :project
+  validates :project, presence: true
+
+  scope :of_group, ->(group) { where(project_id: group.project_ids) }
+  scope :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) }
+  scope :opened, -> { with_state(:opened) }
+  scope :closed, -> { with_state(:closed) }
+  scope :by_project, ->(project_id) {where(project_id:project_id)}
+
   attr_accessible :title, :assignee_id, :position, :description,
                   :milestone_id, :label_list, :author_id_of_changes,
                   :state_event
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 2a476355404..0e0ae3c3a07 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -2,30 +2,37 @@
 #
 # Table name: merge_requests
 #
-#  id            :integer          not null, primary key
-#  target_branch :string(255)      not null
-#  source_branch :string(255)      not null
-#  project_id    :integer          not null
-#  author_id     :integer
-#  assignee_id   :integer
-#  title         :string(255)
-#  created_at    :datetime
-#  updated_at    :datetime
-#  st_commits    :text(2147483647)
-#  st_diffs      :text(2147483647)
-#  milestone_id  :integer
-#  state         :string(255)
-#  merge_status  :string(255)
+#  id             :integer          not null, primary key
+#  target_project_id :integer       not null
+#  target_branch  :string(255)      not null
+#  source_project_id :integer       not null
+#  source_branch  :string(255)      not null
+#  author_id      :integer
+#  assignee_id    :integer
+#  title          :string(255)
+#  created_at     :datetime
+#  updated_at     :datetime
+#  st_commits     :text(2147483647)
+#  st_diffs       :text(2147483647)
+#  milestone_id   :integer
+#  state          :string(255)
+#  merge_status   :string(255)
 #
 
 require Rails.root.join("app/models/commit")
 require Rails.root.join("lib/static_model")
 
 class MergeRequest < ActiveRecord::Base
+
   include Issuable
 
-  attr_accessible :title, :assignee_id, :target_branch, :source_branch, :milestone_id,
-                  :author_id_of_changes, :state_event
+  belongs_to :target_project,:foreign_key => :target_project_id, class_name: "Project"
+  belongs_to :source_project, :foreign_key => :source_project_id,class_name: "Project"
+
+  BROKEN_DIFF = "--broken-diff"
+
+  attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch,  :milestone_id,:author_id_of_changes, :state_event
+
 
   attr_accessor :should_remove_source_branch
 
@@ -74,22 +81,29 @@ class MergeRequest < ActiveRecord::Base
   serialize :st_commits
   serialize :st_diffs
 
+  validates :source_project, presence: true
   validates :source_branch, presence: true
+  validates :target_project, presence: true
   validates :target_branch, presence: true
-  validate  :validate_branches
+  validate :validate_branches
 
+  scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)",group_project_ids:group.project_ids) }
+  scope :of_user_team, ->(team) { where("(source_project_id in (:team_project_ids) OR target_project_id in (:team_project_ids) AND assignee_id in (:team_member_ids))",team_project_ids:team.project_ids,team_member_ids:team.member_ids) }
+  scope :opened, -> { with_state(:opened) }
+  scope :closed, -> { with_state(:closed) }
   scope :merged, -> { with_state(:merged) }
-  scope :by_branch, ->(branch_name) { where("source_branch LIKE :branch OR target_branch LIKE :branch", branch: branch_name) }
+  scope :by_branch, ->(branch_name) { where("(source_branch LIKE :branch) OR (target_branch LIKE :branch)", branch: branch_name) }
   scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) }
   scope :by_milestone, ->(milestone) { where(milestone_id: milestone) }
-
+  scope :by_project, ->(project_id) { where("source_project_id = :project_id OR target_project_id = :project_id", project_id: project_id) }
+  scope :in_projects, ->(project_ids) { where("source_project_id in (:project_ids) OR target_project_id in (:project_ids)", project_ids: project_ids) }
   # Closed scope for merge request should return
   # both merged and closed mr's
   scope :closed, -> { with_states(:closed, :merged) }
 
   def validate_branches
-    if target_branch == source_branch
-      errors.add :branch_conflict, "You can not use same branch for source and target branches"
+    if target_project==source_project && target_branch == source_branch
+      errors.add :branch_conflict, "You can not use same project/branch for source and target"
     end
 
     if opened? || reopened?
@@ -137,7 +151,14 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def unmerged_diffs
-    project.repository.diffs_between(source_branch, target_branch)
+    #TODO:[IA-8] this needs to be handled better -- logged etc
+     diffs = Gitlab::Satellite::MergeAction.new(author, self).diffs_between_satellite
+     if diffs
+      diffs = diffs.map { |diff| Gitlab::Git::Diff.new(diff) }
+     else
+       diffs = []
+     end
+    diffs
   end
 
   def last_commit
@@ -145,11 +166,11 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def merge_event
-    self.project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last
+    self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last
   end
 
   def closed_event
-    self.project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
+    self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
   end
 
   def commits
@@ -158,24 +179,30 @@ class MergeRequest < ActiveRecord::Base
 
   def probably_merged?
     unmerged_commits.empty? &&
-      commits.any? && opened?
+        commits.any? && opened?
   end
 
   def reloaded_commits
     if opened? && unmerged_commits.any?
       self.st_commits = dump_commits(unmerged_commits)
       save
+
     end
     commits
   end
 
   def unmerged_commits
-    self.project.repository.
-      commits_between(self.target_branch, self.source_branch).
-      sort_by(&:created_at).
-      reverse
+    commits = Gitlab::Satellite::MergeAction.new(self.author,self).commits_between
+    commits = commits.map{ |commit| Gitlab::Git::Commit.new(commit, nil) }
+    if commits.present?
+      commits = Commit.decorate(commits).
+        sort_by(&:created_at).
+        reverse
+    end
+    commits
   end
 
+
   def merge!(user_id)
     self.author_id_of_changes = user_id
     self.merge
@@ -195,25 +222,33 @@ class MergeRequest < ActiveRecord::Base
     commit_ids = commits.map(&:id)
     Note.where("(noteable_type = 'MergeRequest' AND noteable_id = :mr_id) OR (noteable_type = 'Commit' AND commit_id IN (:commit_ids))", mr_id: id, commit_ids: commit_ids)
   end
-
   # Returns the raw diff for this merge request
   #
   # see "git diff"
-  def to_diff
-    project.repo.git.native(:diff, {timeout: 30, raise: true}, "#{target_branch}...#{source_branch}")
+  def to_diff(current_user)
+    Gitlab::Satellite::MergeAction.new(current_user, self).diff_in_satellite
   end
 
+
   # Returns the commit as a series of email patches.
   #
   # see "git format-patch"
-  def to_patch
-    project.repo.git.format_patch({timeout: 30, raise: true, stdout: true}, "#{target_branch}..#{source_branch}")
+  def to_patch(current_user)
+    Gitlab::Satellite::MergeAction.new(current_user, self).format_patch
   end
 
   def last_commit_short_sha
     @last_commit_short_sha ||= last_commit.sha[0..10]
   end
 
+  def for_fork?
+    target_project != source_project
+  end
+
+  def disallow_source_branch_removal?
+    (source_project.root_ref? source_branch) || for_fork?
+  end
+
   private
 
   def dump_commits(commits)
diff --git a/app/models/note.rb b/app/models/note.rb
index c23aab03bcc..0175430be4d 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -32,8 +32,8 @@ class Note < ActiveRecord::Base
   delegate :name, :email, to: :author, prefix: true
 
   validates :note, :project, presence: true
-  validates :line_code, format: { with: /\A[a-z0-9]+_\d+_\d+\Z/ }, allow_blank: true
-  validates :attachment, file_size: { maximum: 10.megabytes.to_i }
+  validates :line_code, format: {with: /\A[a-z0-9]+_\d+_\d+\Z/}, allow_blank: true
+  validates :attachment, file_size: {maximum: 10.megabytes.to_i}
 
   validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' }
   validates :commit_id, presence: true, if: ->(n) { n.noteable_type == 'Commit' }
@@ -45,24 +45,24 @@ class Note < ActiveRecord::Base
   scope :inline, -> { where("line_code IS NOT NULL") }
   scope :not_inline, -> { where(line_code: [nil, '']) }
 
-  scope :common, ->{ where(noteable_type: ["", nil]) }
-  scope :fresh, ->{ order("created_at ASC, id ASC") }
-  scope :inc_author_project, ->{ includes(:project, :author) }
-  scope :inc_author, ->{ includes(:author) }
+  scope :common, -> { where(noteable_type: ["", nil]) }
+  scope :fresh, -> { order("created_at ASC, id ASC") }
+  scope :inc_author_project, -> { includes(:project, :author) }
+  scope :inc_author, -> { includes(:author) }
 
-  def self.create_status_change_note(noteable, author, status)
+  def self.create_status_change_note(noteable, project, author, status)
     create({
-      noteable: noteable,
-      project: noteable.project,
-      author: author,
-      note: "_Status changed to #{status}_"
-    }, without_protection: true)
+               noteable: noteable,
+               project: project,
+               author: author,
+               note: "_Status changed to #{status}_"
+           }, without_protection: true)
   end
 
   def commit_author
     @commit_author ||=
-      project.users.find_by_email(noteable.author_email) ||
-      project.users.find_by_name(noteable.author_name)
+        project.users.find_by_email(noteable.author_email) ||
+            project.users.find_by_name(noteable.author_name)
   rescue
     nil
   end
@@ -97,8 +97,8 @@ class Note < ActiveRecord::Base
   # otherwise false is returned
   def downvote?
     votable? && (note.start_with?('-1') ||
-                 note.start_with?(':-1:')
-                )
+        note.start_with?(':-1:')
+    )
   end
 
   def for_commit?
@@ -136,8 +136,8 @@ class Note < ActiveRecord::Base
     else
       super
     end
-  # Temp fix to prevent app crash
-  # if note commit id doesn't exist
+      # Temp fix to prevent app crash
+      # if note commit id doesn't exist
   rescue
     nil
   end
@@ -146,8 +146,8 @@ class Note < ActiveRecord::Base
   # otherwise false is returned
   def upvote?
     votable? && (note.start_with?('+1') ||
-                 note.start_with?(':+1:')
-                )
+        note.start_with?(':+1:')
+    )
   end
 
   def votable?
diff --git a/app/models/project.rb b/app/models/project.rb
index 8297c11ba8a..f373446f579 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -53,7 +53,7 @@ class Project < ActiveRecord::Base
 
   has_many :services,           dependent: :destroy
   has_many :events,             dependent: :destroy
-  has_many :merge_requests,     dependent: :destroy
+  has_many :merge_requests,     dependent: :destroy, foreign_key: "target_project_id"
   has_many :issues,             dependent: :destroy, order: "state DESC, created_at DESC"
   has_many :milestones,         dependent: :destroy
   has_many :notes,              dependent: :destroy