diff --git a/CHANGELOG b/CHANGELOG index a355b622609..b631b8ca5a4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,7 @@ v 8.6.0 (unreleased) - Fix issue when pushing to projects ending in .wiki - Fix avatar stretching by providing a cropping feature (Johann Pardanaud) - Don't load all of GitLab in mail_room + - Indicate how much an MR diverged from the target branch (Pierre de La Morinerie) - Strip leading and trailing spaces in URL validator (evuez) - Return empty array instead of 404 when commit has no statuses in commit status API - Update documentation to reflect Guest role not being enforced on internal projects diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index cf26183c254..8271dc83116 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -537,6 +537,29 @@ class MergeRequest < ActiveRecord::Base end end + def diverged_commits_count + cache = Rails.cache.read(:"merge_request_#{id}_diverged_commits") + + if cache.blank? || cache[:source_sha] != source_sha || cache[:target_sha] != target_sha + cache = { + source_sha: source_sha, + target_sha: target_sha, + diverged_commits_count: compute_diverged_commits_count + } + Rails.cache.write(:"merge_request_#{id}_diverged_commits", cache) + end + + cache[:diverged_commits_count] + end + + def compute_diverged_commits_count + Gitlab::Git::Commit.between(target_project.repository.raw_repository, source_sha, target_sha).size + end + + def diverged_from_target_branch? + diverged_commits_count > 0 + end + def ci_commit @ci_commit ||= source_project.ci_commit(last_commit.id) if last_commit && source_project end diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 7b5e2991c09..b262892ac65 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -34,6 +34,8 @@ %span into = link_to namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch" do = @merge_request.target_branch + - if @merge_request.open? && @merge_request.diverged_from_target_branch? + %span (#{pluralize(@merge_request.diverged_commits_count, 'commit')} behind) = render "projects/merge_requests/show/how_to_merge" = render "projects/merge_requests/widget/show.html.haml" diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index 495f25f28e7..a69089f00c4 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -26,6 +26,16 @@ Feature: Project Merge Requests When I visit project "Shop" merge requests page Then I should see "other_branch" branch + Scenario: I should not see the numbers of diverged commits if the branch is rebased on the target + Given project "Shop" have "Bug NS-07" open merge request with rebased branch + When I visit merge request page "Bug NS-07" + Then I should not see the diverged commits count + + Scenario: I should see the numbers of diverged commits if the branch diverged from the target + Given project "Shop" have "Bug NS-08" open merge request with diverged branch + When I visit merge request page "Bug NS-08" + Then I should see the diverged commits count + Scenario: I should see rejected merge requests Given I click link "Closed" Then I should see "Feature NS-03" in merge requests diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index dde864f5180..8bf423cc64b 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -60,7 +60,6 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps expect(page).not_to have_content "Feature NS-03" end - step 'I should not see "Bug NS-04" in merge requests' do expect(page).not_to have_content "Bug NS-04" end @@ -121,6 +120,22 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps author: project.users.first) end + step 'project "Shop" have "Bug NS-07" open merge request with rebased branch' do + create(:merge_request, :rebased, + title: "Bug NS-07", + source_project: project, + target_project: project, + author: project.users.first) + end + + step 'project "Shop" have "Bug NS-08" open merge request with diverged branch' do + create(:merge_request, :diverged, + title: "Bug NS-08", + source_project: project, + target_project: project, + author: project.users.first) + end + step 'project "Shop" have "Feature NS-03" closed merge request' do create(:closed_merge_request, title: "Feature NS-03", @@ -490,6 +505,18 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end end + step 'I should see the diverged commits count' do + page.within ".mr-source-target" do + expect(page).to have_content /([0-9]+ commits behind)/ + end + end + + step 'I should not see the diverged commits count' do + page.within ".mr-source-target" do + expect(page).not_to have_content /([0-9]+ commit[s]? behind)/ + end + end + step 'I should see "Bug NS-05" at the top' do expect(page.find('ul.content-list.mr-list li.merge-request:first-child')).to have_content("Bug NS-05") end diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 6432a786bfc..da9d1503ebc 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -388,13 +388,19 @@ module SharedPaths end step 'I visit merge request page "Bug NS-04"' do - mr = MergeRequest.find_by(title: "Bug NS-04") - visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr) + visit merge_request_path("Bug NS-04") end step 'I visit merge request page "Bug NS-05"' do - mr = MergeRequest.find_by(title: "Bug NS-05") - visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr) + visit merge_request_path("Bug NS-05") + end + + step 'I visit merge request page "Bug NS-07"' do + visit merge_request_path("Bug NS-07") + end + + step 'I visit merge request page "Bug NS-08"' do + visit merge_request_path("Bug NS-08") end step 'I visit merge request page "Bug CO-01"' do @@ -503,6 +509,11 @@ module SharedPaths Project.find_by!(name: 'Shop') end + def merge_request_path(title) + mr = MergeRequest.find_by(title: title) + namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr) + end + # ---------------------------------------- # Errors # ---------------------------------------- diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 00de7bb5294..ca1c636fce4 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -69,6 +69,16 @@ FactoryGirl.define do target_branch "master" end + trait :rebased do + source_branch "markdown" + target_branch "improve/awesome" + end + + trait :diverged do + source_branch "feature" + target_branch "master" + end + trait :merge_when_build_succeeds do merge_when_build_succeeds true merge_user author diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c51f34034d7..59c40922abb 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -274,6 +274,70 @@ describe MergeRequest, models: true do end end + describe '#diverged_commits_count' do + let(:project) { create(:project) } + let(:fork_project) { create(:project, forked_from_project: project) } + + context 'diverged on same repository' do + subject(:merge_request_with_divergence) { create(:merge_request, :diverged, source_project: project, target_project: project) } + + it 'counts commits that are on target branch but not on source branch' do + expect(subject.diverged_commits_count).to eq(5) + end + end + + context 'diverged on fork' do + subject(:merge_request_fork_with_divergence) { create(:merge_request, :diverged, source_project: fork_project, target_project: project) } + + it 'counts commits that are on target branch but not on source branch' do + expect(subject.diverged_commits_count).to eq(5) + end + end + + context 'rebased on fork' do + subject(:merge_request_rebased) { create(:merge_request, :rebased, source_project: fork_project, target_project: project) } + + it 'counts commits that are on target branch but not on source branch' do + expect(subject.diverged_commits_count).to eq(0) + end + end + + describe 'caching' do + before(:example) do + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + end + + it 'caches the output' do + expect(subject).to receive(:compute_diverged_commits_count). + once. + and_return(2) + + subject.diverged_commits_count + subject.diverged_commits_count + end + + it 'invalidates the cache when the source sha changes' do + expect(subject).to receive(:compute_diverged_commits_count). + twice. + and_return(2) + + subject.diverged_commits_count + allow(subject).to receive(:source_sha).and_return('123abc') + subject.diverged_commits_count + end + + it 'invalidates the cache when the target sha changes' do + expect(subject).to receive(:compute_diverged_commits_count). + twice. + and_return(2) + + subject.diverged_commits_count + allow(subject).to receive(:target_sha).and_return('123abc') + subject.diverged_commits_count + end + end + end + it_behaves_like 'an editable mentionable' do subject { create(:merge_request) }