Merge branch 'ee-1012-assign-code-owner-as-approver' into 'master'

(EE Port) Assign approvers based on code owners

See merge request gitlab-org/gitlab-ce!22513

16 files changed, 186 insertions, 13 deletions

diff --git a/app/models/compare.rb b/app/models/compare.rb
index b2d46ada831..f1ed84ab5a5 100644
--- a/app/models/compare.rb
+++ b/app/models/compare.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'set'
+
 class Compare
   include Gitlab::Utils::StrongMemoize
 
@@ -77,4 +79,13 @@ class Compare
       head_sha: head_commit_sha
     )
   end
+
+  def modified_paths
+    paths = Set.new
+    diffs.diff_files.each do |diff|
+      paths.add diff.old_path
+      paths.add diff.new_path
+    end
+    paths.to_a
+  end
 end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 735d9fba966..df5678ec2f1 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -409,6 +409,18 @@ class MergeRequest < ActiveRecord::Base
     merge_request_diff&.real_size || diffs.real_size
   end
 
+  def modified_paths(past_merge_request_diff: nil)
+    diffs = if past_merge_request_diff
+              past_merge_request_diff
+            elsif compare
+              compare
+            else
+              self.merge_request_diff
+            end
+
+    diffs.modified_paths
+  end
+
   def diff_base_commit
     if persisted?
       merge_request_diff.base_commit
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index bb6ff8921df..74583af1a29 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -6,6 +6,7 @@ class MergeRequestDiff < ActiveRecord::Base
   include ManualInverseAssociation
   include IgnorableColumn
   include EachBatch
+  include Gitlab::Utils::StrongMemoize
 
   # Don't display more than 100 commits at once
   COMMITS_SAFE_SIZE = 100
@@ -234,6 +235,12 @@ class MergeRequestDiff < ActiveRecord::Base
   end
   # rubocop: enable CodeReuse/ServiceClass
 
+  def modified_paths
+    strong_memoize(:modified_paths) do
+      merge_request_diff_files.pluck(:new_path, :old_path).flatten.uniq
+    end
+  end
+
   private
 
   def create_merge_request_diff_files(diffs)
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 53768ff2cbe..5fe48da1cd6 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -2,18 +2,18 @@
 
 module MergeRequests
   class RefreshService < MergeRequests::BaseService
+    attr_reader :push
+
     def execute(oldrev, newrev, ref)
-      push = Gitlab::Git::Push.new(@project, oldrev, newrev, ref)
-      return true unless push.branch_push?
+      @push = Gitlab::Git::Push.new(@project, oldrev, newrev, ref)
+      return true unless @push.branch_push?
 
-      refresh_merge_requests!(push)
+      refresh_merge_requests!
     end
 
     private
 
-    def refresh_merge_requests!(push)
-      @push = push
-
+    def refresh_merge_requests!
       Gitlab::GitalyClient.allow_n_plus_1_calls(&method(:find_new_commits))
       # Be sure to close outstanding MRs before reloading them to avoid generating an
       # empty diff during a manual merge
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index 3190f1ce9d4..ccd4fc4db3a 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Projects::MilestonesController do
-  let(:project) { create(:project) }
+  let(:project) { create(:project, :repository) }
   let(:user)    { create(:user) }
   let(:milestone) { create(:milestone, project: project) }
   let(:issue) { create(:issue, project: project, milestone: milestone) }
diff --git a/spec/factories/merge_request_diff_files.rb b/spec/factories/merge_request_diff_files.rb
new file mode 100644
index 00000000000..469a7a0ac8d
--- /dev/null
+++ b/spec/factories/merge_request_diff_files.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :merge_request_diff_file do
+    association :merge_request_diff
+
+    relative_order 0
+    new_file true
+    renamed_file false
+    deleted_file false
+    too_large false
+    a_mode 0
+    b_mode 100644
+    new_path 'foo'
+    old_path 'foo'
+    diff ''
+    binary false
+
+    trait :new_file do
+      relative_order 0
+      new_file true
+      renamed_file false
+      deleted_file false
+      too_large false
+      a_mode 0
+      b_mode 100644
+      new_path 'foo'
+      old_path 'foo'
+      diff ''
+      binary false
+    end
+
+    trait :renamed_file do
+      relative_order 662
+      new_file false
+      renamed_file true
+      deleted_file false
+      too_large false
+      a_mode 100644
+      b_mode 100644
+      new_path 'bar'
+      old_path 'baz'
+      diff ''
+      binary false
+    end
+  end
+end
diff --git a/spec/factories/merge_request_diffs.rb b/spec/factories/merge_request_diffs.rb
new file mode 100644
index 00000000000..e7b51189538
--- /dev/null
+++ b/spec/factories/merge_request_diffs.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :merge_request_diff do
+    association :merge_request
+    state :collected
+    commits_count 1
+
+    base_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
+    head_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
+    start_commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
+  end
+end
diff --git a/spec/models/compare_spec.rb b/spec/models/compare_spec.rb
index 8e88bb81162..0bc3ee014e6 100644
--- a/spec/models/compare_spec.rb
+++ b/spec/models/compare_spec.rb
@@ -92,4 +92,33 @@ describe Compare do
       expect(subject.diff_refs.head_sha).to eq(head_commit.id)
     end
   end
+
+  describe '#modified_paths' do
+    context 'changes are present' do
+      let(:raw_compare) do
+        Gitlab::Git::Compare.new(
+          project.repository.raw_repository, 'before-create-delete-modify-move', 'after-create-delete-modify-move'
+        )
+      end
+
+      it 'returns affected file paths, without duplication' do
+        expect(subject.modified_paths).to contain_exactly(*%w{
+          foo/for_move.txt
+          foo/bar/for_move.txt
+          foo/for_create.txt
+          foo/for_delete.txt
+          foo/for_edit.txt
+        })
+      end
+    end
+
+    context 'changes are absent' do
+      let(:start_commit) { sample_commit }
+      let(:head_commit) { sample_commit }
+
+      it 'returns empty array' do
+        expect(subject.modified_paths).to eq([])
+      end
+    end
+  end
 end
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 47e8f04e728..cbe60b3a4a5 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -232,4 +232,17 @@ describe MergeRequestDiff do
       expect(commits.map(&:sha)).to match_array(commit_shas)
     end
   end
+
+  describe '#modified_paths' do
+    subject do
+      diff = create(:merge_request_diff)
+      create(:merge_request_diff_file, :new_file, merge_request_diff: diff)
+      create(:merge_request_diff_file, :renamed_file, merge_request_diff: diff)
+      diff
+    end
+
+    it 'returns affected file paths' do
+      expect(subject.modified_paths).to eq(%w{foo bar baz})
+    end
+  end
 end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 3a54725c7ec..c7202b481d3 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -631,6 +631,44 @@ describe MergeRequest do
     end
   end
 
+  describe '#modified_paths' do
+    let(:paths) { double(:paths) }
+    subject(:merge_request) { build(:merge_request) }
+
+    before do
+      expect(diff).to receive(:modified_paths).and_return(paths)
+    end
+
+    context 'when past_merge_request_diff is specified' do
+      let(:another_diff) { double(:merge_request_diff) }
+      let(:diff) { another_diff }
+
+      it 'returns affected file paths from specified past_merge_request_diff' do
+        expect(merge_request.modified_paths(past_merge_request_diff: another_diff)).to eq(paths)
+      end
+    end
+
+    context 'when compare is present' do
+      let(:compare) { double(:compare) }
+      let(:diff) { compare }
+
+      it 'returns affected file paths from compare' do
+        merge_request.compare = compare
+
+        expect(merge_request.modified_paths).to eq(paths)
+      end
+    end
+
+    context 'when no arguments provided' do
+      let(:diff) { merge_request.merge_request_diff }
+      subject(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
+
+      it 'returns affected file paths for merge_request_diff' do
+        expect(merge_request.modified_paths).to eq(paths)
+      end
+    end
+  end
+
   describe "#related_notes" do
     let!(:merge_request) { create(:merge_request) }
 
diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb
index 53c85f73cde..f0b0f7956ce 100644
--- a/spec/services/issuable/bulk_update_service_spec.rb
+++ b/spec/services/issuable/bulk_update_service_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Issuable::BulkUpdateService do
   let(:user)    { create(:user) }
-  let(:project) { create(:project, namespace: user.namespace) }
+  let(:project) { create(:project, :repository, namespace: user.namespace) }
 
   def bulk_update(issuables, extra_params = {})
     bulk_update_params = extra_params
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 1b599ba11b6..be5ad849ba7 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -593,8 +593,8 @@ describe MergeRequests::UpdateService, :mailer do
     end
 
     context 'setting `allow_collaboration`' do
-      let(:target_project) { create(:project, :public) }
-      let(:source_project) { fork_project(target_project) }
+      let(:target_project) { create(:project, :repository, :public) }
+      let(:source_project) { fork_project(target_project, nil, repository: true) }
       let(:user) { create(:user) }
       let(:merge_request) do
         create(:merge_request,
diff --git a/spec/services/milestones/destroy_service_spec.rb b/spec/services/milestones/destroy_service_spec.rb
index 8680e428517..9d2be30c636 100644
--- a/spec/services/milestones/destroy_service_spec.rb
+++ b/spec/services/milestones/destroy_service_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Milestones::DestroyService do
   let(:user) { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, :repository) }
   let(:milestone) { create(:milestone, title: 'Milestone v1.0', project: project) }
 
   before do
diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb
index a8c994c101c..14d62763a5b 100644
--- a/spec/services/notes/quick_actions_service_spec.rb
+++ b/spec/services/notes/quick_actions_service_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Notes::QuickActionsService do
   shared_context 'note on noteable' do
-    let(:project) { create(:project) }
+    let(:project) { create(:project, :repository) }
     let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } }
     let(:assignee) { create(:user) }
 
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 1746721b0d0..c52515aefd8 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -10,7 +10,7 @@ describe TodoService do
   let(:john_doe) { create(:user) }
   let(:skipped) { create(:user) }
   let(:skip_users) { [skipped] }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, :repository) }
   let(:mentions) { 'FYI: ' + [author, assignee, john_doe, member, guest, non_member, admin, skipped].map(&:to_reference).join(' ') }
   let(:directly_addressed) { [author, assignee, john_doe, member, guest, non_member, admin, skipped].map(&:to_reference).join(' ') }
   let(:directly_addressed_and_mentioned) { member.to_reference + ", what do you think? cc: " + [guest, admin, skipped].map(&:to_reference).join(' ') }
diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb
index 71d72ff27e9..80b96f20e3f 100644
--- a/spec/support/helpers/test_env.rb
+++ b/spec/support/helpers/test_env.rb
@@ -55,6 +55,9 @@ module TestEnv
     'update-gitlab-shell-v-6-0-1'        => '2f61d70',
     'update-gitlab-shell-v-6-0-3'        => 'de78448',
     '2-mb-file'                          => 'bf12d25',
+    'before-create-delete-modify-move'   => '845009f',
+    'between-create-delete-modify-move'  => '3f5f443',
+    'after-create-delete-modify-move'    => 'ba3faa7',
     'with-codeowners'                    => '219560e'
   }.freeze