Commit 4a5d04fb authored by Grzegorz Bizon's avatar Grzegorz Bizon

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
parents 3cf13f2f 7b50f2eb
# frozen_string_literal: true # frozen_string_literal: true
require 'set'
class Compare class Compare
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
...@@ -77,4 +79,13 @@ class Compare ...@@ -77,4 +79,13 @@ class Compare
head_sha: head_commit_sha head_sha: head_commit_sha
) )
end 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 end
...@@ -409,6 +409,18 @@ class MergeRequest < ActiveRecord::Base ...@@ -409,6 +409,18 @@ class MergeRequest < ActiveRecord::Base
merge_request_diff&.real_size || diffs.real_size merge_request_diff&.real_size || diffs.real_size
end 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 def diff_base_commit
if persisted? if persisted?
merge_request_diff.base_commit merge_request_diff.base_commit
......
...@@ -6,6 +6,7 @@ class MergeRequestDiff < ActiveRecord::Base ...@@ -6,6 +6,7 @@ class MergeRequestDiff < ActiveRecord::Base
include ManualInverseAssociation include ManualInverseAssociation
include IgnorableColumn include IgnorableColumn
include EachBatch include EachBatch
include Gitlab::Utils::StrongMemoize
# Don't display more than 100 commits at once # Don't display more than 100 commits at once
COMMITS_SAFE_SIZE = 100 COMMITS_SAFE_SIZE = 100
...@@ -234,6 +235,12 @@ class MergeRequestDiff < ActiveRecord::Base ...@@ -234,6 +235,12 @@ class MergeRequestDiff < ActiveRecord::Base
end end
# rubocop: enable CodeReuse/ServiceClass # 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 private
def create_merge_request_diff_files(diffs) def create_merge_request_diff_files(diffs)
......
...@@ -2,18 +2,18 @@ ...@@ -2,18 +2,18 @@
module MergeRequests module MergeRequests
class RefreshService < MergeRequests::BaseService class RefreshService < MergeRequests::BaseService
attr_reader :push
def execute(oldrev, newrev, ref) def execute(oldrev, newrev, ref)
push = Gitlab::Git::Push.new(@project, oldrev, newrev, ref) @push = Gitlab::Git::Push.new(@project, oldrev, newrev, ref)
return true unless push.branch_push? return true unless @push.branch_push?
refresh_merge_requests!(push) refresh_merge_requests!
end end
private private
def refresh_merge_requests!(push) def refresh_merge_requests!
@push = push
Gitlab::GitalyClient.allow_n_plus_1_calls(&method(:find_new_commits)) Gitlab::GitalyClient.allow_n_plus_1_calls(&method(:find_new_commits))
# Be sure to close outstanding MRs before reloading them to avoid generating an # Be sure to close outstanding MRs before reloading them to avoid generating an
# empty diff during a manual merge # empty diff during a manual merge
......
require 'spec_helper' require 'spec_helper'
describe Projects::MilestonesController do describe Projects::MilestonesController do
let(:project) { create(:project) } let(:project) { create(:project, :repository) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:milestone) { create(:milestone, project: project) } let(:milestone) { create(:milestone, project: project) }
let(:issue) { create(:issue, project: project, milestone: milestone) } let(:issue) { create(:issue, project: project, milestone: milestone) }
......
# 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
# 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
...@@ -92,4 +92,33 @@ describe Compare do ...@@ -92,4 +92,33 @@ describe Compare do
expect(subject.diff_refs.head_sha).to eq(head_commit.id) expect(subject.diff_refs.head_sha).to eq(head_commit.id)
end end
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 end
...@@ -232,4 +232,17 @@ describe MergeRequestDiff do ...@@ -232,4 +232,17 @@ describe MergeRequestDiff do
expect(commits.map(&:sha)).to match_array(commit_shas) expect(commits.map(&:sha)).to match_array(commit_shas)
end end
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 end
...@@ -631,6 +631,44 @@ describe MergeRequest do ...@@ -631,6 +631,44 @@ describe MergeRequest do
end end
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 describe "#related_notes" do
let!(:merge_request) { create(:merge_request) } let!(:merge_request) { create(:merge_request) }
......
...@@ -2,7 +2,7 @@ require 'spec_helper' ...@@ -2,7 +2,7 @@ require 'spec_helper'
describe Issuable::BulkUpdateService do describe Issuable::BulkUpdateService do
let(:user) { create(:user) } 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 = {}) def bulk_update(issuables, extra_params = {})
bulk_update_params = extra_params bulk_update_params = extra_params
......
...@@ -593,8 +593,8 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -593,8 +593,8 @@ describe MergeRequests::UpdateService, :mailer do
end end
context 'setting `allow_collaboration`' do context 'setting `allow_collaboration`' do
let(:target_project) { create(:project, :public) } let(:target_project) { create(:project, :repository, :public) }
let(:source_project) { fork_project(target_project) } let(:source_project) { fork_project(target_project, nil, repository: true) }
let(:user) { create(:user) } let(:user) { create(:user) }
let(:merge_request) do let(:merge_request) do
create(:merge_request, create(:merge_request,
......
...@@ -2,7 +2,7 @@ require 'spec_helper' ...@@ -2,7 +2,7 @@ require 'spec_helper'
describe Milestones::DestroyService do describe Milestones::DestroyService do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project) } let(:project) { create(:project, :repository) }
let(:milestone) { create(:milestone, title: 'Milestone v1.0', project: project) } let(:milestone) { create(:milestone, title: 'Milestone v1.0', project: project) }
before do before do
......
...@@ -2,7 +2,7 @@ require 'spec_helper' ...@@ -2,7 +2,7 @@ require 'spec_helper'
describe Notes::QuickActionsService do describe Notes::QuickActionsService do
shared_context 'note on noteable' 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(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } }
let(:assignee) { create(:user) } let(:assignee) { create(:user) }
......
...@@ -10,7 +10,7 @@ describe TodoService do ...@@ -10,7 +10,7 @@ describe TodoService do
let(:john_doe) { create(:user) } let(:john_doe) { create(:user) }
let(:skipped) { create(:user) } let(:skipped) { create(:user) }
let(:skip_users) { [skipped] } 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(: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) { [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(' ') } let(:directly_addressed_and_mentioned) { member.to_reference + ", what do you think? cc: " + [guest, admin, skipped].map(&:to_reference).join(' ') }
......
...@@ -55,6 +55,9 @@ module TestEnv ...@@ -55,6 +55,9 @@ module TestEnv
'update-gitlab-shell-v-6-0-1' => '2f61d70', 'update-gitlab-shell-v-6-0-1' => '2f61d70',
'update-gitlab-shell-v-6-0-3' => 'de78448', 'update-gitlab-shell-v-6-0-3' => 'de78448',
'2-mb-file' => 'bf12d25', '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' 'with-codeowners' => '219560e'
}.freeze }.freeze
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment