Commit d1eab555 authored by Douwe Maan's avatar Douwe Maan

Merge branch '20708-new-branch-is-immediatelly-tagged-as-merged' into 'master'

Removes inconsistency regarding tagging immediately as merged once you create a …

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

## What are the relevant issue numbers?


Closes #20708

See merge request !6408
parents d3a98380 c9048340
...@@ -150,6 +150,7 @@ v 8.12.1 ...@@ -150,6 +150,7 @@ v 8.12.1
- Fix issue with search filter labels not displaying - Fix issue with search filter labels not displaying
v 8.12.0 v 8.12.0
- Removes inconsistency regarding tagging immediatelly as merged once you create a new branch. !6408
- Update the rouge gem to 2.0.6, which adds highlighting support for JSX, Prometheus, and others. !6251 - Update the rouge gem to 2.0.6, which adds highlighting support for JSX, Prometheus, and others. !6251
- Only check :can_resolve permission if the note is resolvable - Only check :can_resolve permission if the note is resolvable
- Bump fog-aws to v0.11.0 to support ap-south-1 region - Bump fog-aws to v0.11.0 to support ap-south-1 region
......
...@@ -1016,7 +1016,8 @@ class Repository ...@@ -1016,7 +1016,8 @@ class Repository
root_ref_commit = commit(root_ref) root_ref_commit = commit(root_ref)
if branch_commit if branch_commit
is_ancestor?(branch_commit.id, root_ref_commit.id) same_head = branch_commit.id == root_ref_commit.id
!same_head && is_ancestor?(branch_commit.id, root_ref_commit.id)
else else
nil nil
end end
......
...@@ -102,15 +102,16 @@ describe Projects::CommitController do ...@@ -102,15 +102,16 @@ describe Projects::CommitController do
describe "as patch" do describe "as patch" do
include_examples "export as", :patch include_examples "export as", :patch
let(:format) { :patch } let(:format) { :patch }
let(:commit2) { project.commit('498214de67004b1da3d820901307bed2a68a8ef6') }
it "is a git email patch" do it "is a git email patch" do
go(id: commit.id, format: format) go(id: commit2.id, format: format)
expect(response.body).to start_with("From #{commit.id}") expect(response.body).to start_with("From #{commit2.id}")
end end
it "contains a git diff" do it "contains a git diff" do
go(id: commit.id, format: format) go(id: commit2.id, format: format)
expect(response.body).to match(/^diff --git/) expect(response.body).to match(/^diff --git/)
end end
...@@ -135,6 +136,8 @@ describe Projects::CommitController do ...@@ -135,6 +136,8 @@ describe Projects::CommitController do
describe "GET branches" do describe "GET branches" do
it "contains branch and tags information" do it "contains branch and tags information" do
commit = project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e')
get(:branches, get(:branches,
namespace_id: project.namespace.to_param, namespace_id: project.namespace.to_param,
project_id: project.to_param, project_id: project.to_param,
...@@ -254,16 +257,17 @@ describe Projects::CommitController do ...@@ -254,16 +257,17 @@ describe Projects::CommitController do
end end
let(:existing_path) { '.gitmodules' } let(:existing_path) { '.gitmodules' }
let(:commit2) { project.commit('5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
context 'when the commit exists' do context 'when the commit exists' do
context 'when the user has access to the project' do context 'when the user has access to the project' do
context 'when the path exists in the diff' do context 'when the path exists in the diff' do
it 'enables diff notes' do it 'enables diff notes' do
diff_for_path(id: commit.id, old_path: existing_path, new_path: existing_path) diff_for_path(id: commit2.id, old_path: existing_path, new_path: existing_path)
expect(assigns(:diff_notes_disabled)).to be_falsey expect(assigns(:diff_notes_disabled)).to be_falsey
expect(assigns(:comments_target)).to eq(noteable_type: 'Commit', expect(assigns(:comments_target)).to eq(noteable_type: 'Commit',
commit_id: commit.id) commit_id: commit2.id)
end end
it 'only renders the diffs for the path given' do it 'only renders the diffs for the path given' do
...@@ -272,7 +276,7 @@ describe Projects::CommitController do ...@@ -272,7 +276,7 @@ describe Projects::CommitController do
meth.call(diffs) meth.call(diffs)
end end
diff_for_path(id: commit.id, old_path: existing_path, new_path: existing_path) diff_for_path(id: commit2.id, old_path: existing_path, new_path: existing_path)
end end
end end
......
...@@ -19,7 +19,7 @@ describe SearchHelper do ...@@ -19,7 +19,7 @@ describe SearchHelper do
expect(subject.filename).to eq('CHANGELOG') expect(subject.filename).to eq('CHANGELOG')
expect(subject.basename).to eq('CHANGELOG') expect(subject.basename).to eq('CHANGELOG')
expect(subject.ref).to eq('master') expect(subject.ref).to eq('master')
expect(subject.startline).to eq(186) expect(subject.startline).to eq(188)
expect(subject.data.lines[2]).to eq(" - Feature: Replace teams with group membership\n") expect(subject.data.lines[2]).to eq(" - Feature: Replace teams with group membership\n")
end end
......
...@@ -8,13 +8,13 @@ describe Gitlab::DataBuilder::Push, lib: true do ...@@ -8,13 +8,13 @@ describe Gitlab::DataBuilder::Push, lib: true do
let(:data) { described_class.build_sample(project, user) } let(:data) { described_class.build_sample(project, user) }
it { expect(data).to be_a(Hash) } it { expect(data).to be_a(Hash) }
it { expect(data[:before]).to eq('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } it { expect(data[:before]).to eq('1b12f15a11fc6e62177bef08f47bc7b5ce50b141') }
it { expect(data[:after]).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } it { expect(data[:after]).to eq('b83d6e391c22777fca1ed3012fce84f633d7fed0') }
it { expect(data[:ref]).to eq('refs/heads/master') } it { expect(data[:ref]).to eq('refs/heads/master') }
it { expect(data[:commits].size).to eq(3) } it { expect(data[:commits].size).to eq(3) }
it { expect(data[:total_commits_count]).to eq(3) } it { expect(data[:total_commits_count]).to eq(3) }
it { expect(data[:commits].first[:added]).to eq(['gitlab-grack']) } it { expect(data[:commits].first[:added]).to eq(['bar/branch-test.txt']) }
it { expect(data[:commits].first[:modified]).to eq(['.gitmodules']) } it { expect(data[:commits].first[:modified]).to eq([]) }
it { expect(data[:commits].first[:removed]).to eq([]) } it { expect(data[:commits].first[:removed]).to eq([]) }
include_examples 'project hook data with deprecateds' include_examples 'project hook data with deprecateds'
......
...@@ -628,7 +628,7 @@ describe Notify do ...@@ -628,7 +628,7 @@ describe Notify do
it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'a user cannot unsubscribe through footer link'
it 'has the correct subject' do it 'has the correct subject' do
is_expected.to have_subject /#{commit.title} \(#{commit.short_id}\)/ is_expected.to have_subject /Re: #{project.name} | #{commit.title} \(#{commit.short_id}\)/
end end
it 'contains a link to the commit' do it 'contains a link to the commit' do
......
...@@ -164,10 +164,10 @@ eos ...@@ -164,10 +164,10 @@ eos
let(:data) { commit.hook_attrs(with_changed_files: true) } let(:data) { commit.hook_attrs(with_changed_files: true) }
it { expect(data).to be_a(Hash) } it { expect(data).to be_a(Hash) }
it { expect(data[:message]).to include('Add submodule from gitlab.com') } it { expect(data[:message]).to include('adds bar folder and branch-test text file to check Repository merged_to_root_ref method') }
it { expect(data[:timestamp]).to eq('2014-02-27T11:01:38+02:00') } it { expect(data[:timestamp]).to eq('2016-09-27T14:37:46+00:00') }
it { expect(data[:added]).to eq(["gitlab-grack"]) } it { expect(data[:added]).to eq(["bar/branch-test.txt"]) }
it { expect(data[:modified]).to eq([".gitmodules"]) } it { expect(data[:modified]).to eq([]) }
it { expect(data[:removed]).to eq([]) } it { expect(data[:removed]).to eq([]) }
end end
......
...@@ -6,9 +6,9 @@ describe MergeRequestDiff, models: true do ...@@ -6,9 +6,9 @@ describe MergeRequestDiff, models: true do
it { expect(subject).to be_valid } it { expect(subject).to be_valid }
it { expect(subject).to be_persisted } it { expect(subject).to be_persisted }
it { expect(subject.commits.count).to eq(5) } it { expect(subject.commits.count).to eq(29) }
it { expect(subject.diffs.count).to eq(8) } it { expect(subject.diffs.count).to eq(20) }
it { expect(subject.head_commit_sha).to eq('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } it { expect(subject.head_commit_sha).to eq('b83d6e391c22777fca1ed3012fce84f633d7fed0') }
it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') } it { expect(subject.base_commit_sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') } it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
end end
......
...@@ -489,7 +489,7 @@ describe MergeRequest, models: true do ...@@ -489,7 +489,7 @@ describe MergeRequest, models: true do
subject(:merge_request_with_divergence) { create(:merge_request, :diverged, source_project: project, target_project: project) } 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 it 'counts commits that are on target branch but not on source branch' do
expect(subject.diverged_commits_count).to eq(5) expect(subject.diverged_commits_count).to eq(29)
end end
end end
...@@ -497,7 +497,7 @@ describe MergeRequest, models: true do ...@@ -497,7 +497,7 @@ describe MergeRequest, models: true do
subject(:merge_request_fork_with_divergence) { create(:merge_request, :diverged, source_project: fork_project, target_project: project) } 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 it 'counts commits that are on target branch but not on source branch' do
expect(subject.diverged_commits_count).to eq(5) expect(subject.diverged_commits_count).to eq(29)
end end
end end
......
...@@ -122,11 +122,30 @@ describe Repository, models: true do ...@@ -122,11 +122,30 @@ describe Repository, models: true do
end end
describe '#merged_to_root_ref?' do describe '#merged_to_root_ref?' do
context 'merged branch' do context 'merged branch without ff' do
subject { repository.merged_to_root_ref?('branch-merged') }
it { is_expected.to be_truthy }
end
# If the HEAD was ff then it will be false
context 'merged with ff' do
subject { repository.merged_to_root_ref?('improve/awesome') } subject { repository.merged_to_root_ref?('improve/awesome') }
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
context 'not merged branch' do
subject { repository.merged_to_root_ref?('not-merged-branch') }
it { is_expected.to be_falsey }
end
context 'default branch' do
subject { repository.merged_to_root_ref?('master') }
it { is_expected.to be_falsey }
end
end end
describe '#can_be_merged?' do describe '#can_be_merged?' do
...@@ -324,7 +343,7 @@ describe Repository, models: true do ...@@ -324,7 +343,7 @@ describe Repository, models: true do
subject { results.first } subject { results.first }
it { is_expected.to be_an String } it { is_expected.to be_an String }
it { expect(subject.lines[2]).to eq("master:CHANGELOG:188: - Feature: Replace teams with group membership\n") } it { expect(subject.lines[2]).to eq("master:CHANGELOG:190: - Feature: Replace teams with group membership\n") }
end end
end end
...@@ -968,10 +987,10 @@ describe Repository, models: true do ...@@ -968,10 +987,10 @@ describe Repository, models: true do
context 'cherry-picking a merge commit' do context 'cherry-picking a merge commit' do
it 'cherry-picks the changes' do it 'cherry-picks the changes' do
expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).to be_nil expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).to be_nil
repository.cherry_pick(user, pickable_merge, 'master') repository.cherry_pick(user, pickable_merge, 'improve/awesome')
expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).not_to be_nil expect(repository.blob_at_branch('improve/awesome', 'foo/bar/.gitkeep')).not_to be_nil
end end
end end
end end
......
...@@ -53,7 +53,12 @@ describe API::API, api: true do ...@@ -53,7 +53,12 @@ describe API::API, api: true do
get api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user) get api("/projects/#{project.id}/repository/commits?until=#{before.utc.iso8601}", user)
if commits.size >= 20
expect(json_response.size).to eq(20)
else
expect(json_response.size).to eq(commits.size - 1) expect(json_response.size).to eq(commits.size - 1)
end
expect(json_response.first["id"]).to eq(commits.second.id) expect(json_response.first["id"]).to eq(commits.second.id)
expect(json_response.second["id"]).to eq(commits.third.id) expect(json_response.second["id"]).to eq(commits.third.id)
end end
...@@ -447,11 +452,12 @@ describe API::API, api: true do ...@@ -447,11 +452,12 @@ describe API::API, api: true do
end end
it 'returns the inline comment' do it 'returns the inline comment' do
post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment', path: project.repository.commit.raw_diffs.first.new_path, line: 7, line_type: 'new' post api("/projects/#{project.id}/repository/commits/#{project.repository.commit.id}/comments", user), note: 'My comment', path: project.repository.commit.raw_diffs.first.new_path, line: 1, line_type: 'new'
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(json_response['note']).to eq('My comment') expect(json_response['note']).to eq('My comment')
expect(json_response['path']).to eq(project.repository.commit.raw_diffs.first.new_path) expect(json_response['path']).to eq(project.repository.commit.raw_diffs.first.new_path)
expect(json_response['line']).to eq(7) expect(json_response['line']).to eq(1)
expect(json_response['line_type']).to eq('new') expect(json_response['line_type']).to eq('new')
end end
......
...@@ -21,7 +21,7 @@ describe API::API, api: true do ...@@ -21,7 +21,7 @@ describe API::API, api: true do
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response).to be_an Array expect(json_response).to be_an Array
expect(json_response.first['name']).to eq('encoding') expect(json_response.first['name']).to eq('bar')
expect(json_response.first['type']).to eq('tree') expect(json_response.first['type']).to eq('tree')
expect(json_response.first['mode']).to eq('040000') expect(json_response.first['mode']).to eq('040000')
end end
...@@ -166,9 +166,9 @@ describe API::API, api: true do ...@@ -166,9 +166,9 @@ describe API::API, api: true do
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response).to be_an Array expect(json_response).to be_an Array
contributor = json_response.first contributor = json_response.first
expect(contributor['email']).to eq('dmitriy.zaporozhets@gmail.com') expect(contributor['email']).to eq('tiagonbotelho@hotmail.com')
expect(contributor['name']).to eq('Dmitriy Zaporozhets') expect(contributor['name']).to eq('tiagonbotelho')
expect(contributor['commits']).to eq(13) expect(contributor['commits']).to eq(1)
expect(contributor['additions']).to eq(0) expect(contributor['additions']).to eq(0)
expect(contributor['deletions']).to eq(0) expect(contributor['deletions']).to eq(0)
end end
......
...@@ -439,8 +439,8 @@ describe 'Git HTTP requests', lib: true do ...@@ -439,8 +439,8 @@ describe 'Git HTTP requests', lib: true do
before do before do
# Provide a dummy file in its place # Provide a dummy file in its place
allow_any_instance_of(Repository).to receive(:blob_at).and_call_original allow_any_instance_of(Repository).to receive(:blob_at).and_call_original
allow_any_instance_of(Repository).to receive(:blob_at).with('5937ac0a7beb003549fc5fd26fc247adbce4a52e', 'info/refs') do allow_any_instance_of(Repository).to receive(:blob_at).with('b83d6e391c22777fca1ed3012fce84f633d7fed0', 'info/refs') do
Gitlab::Git::Blob.find(project.repository, 'master', '.gitignore') Gitlab::Git::Blob.find(project.repository, 'master', 'bar/branch-test.txt')
end end
get "/#{project.path_with_namespace}/blob/master/info/refs" get "/#{project.path_with_namespace}/blob/master/info/refs"
......
...@@ -118,7 +118,7 @@ describe MergeRequests::RefreshService, services: true do ...@@ -118,7 +118,7 @@ describe MergeRequests::RefreshService, services: true do
it { expect(@merge_request.notes).to be_empty } it { expect(@merge_request.notes).to be_empty }
it { expect(@merge_request).to be_open } it { expect(@merge_request).to be_open }
it { expect(@fork_merge_request.notes.last.note).to include('Added 4 commits') } it { expect(@fork_merge_request.notes.last.note).to include('Added 28 commits') }
it { expect(@fork_merge_request).to be_open } it { expect(@fork_merge_request).to be_open }
it { expect(@build_failed_todo).to be_pending } it { expect(@build_failed_todo).to be_pending }
it { expect(@fork_build_failed_todo).to be_pending } it { expect(@fork_build_failed_todo).to be_pending }
...@@ -169,7 +169,7 @@ describe MergeRequests::RefreshService, services: true do ...@@ -169,7 +169,7 @@ describe MergeRequests::RefreshService, services: true do
notes = @fork_merge_request.notes.reorder(:created_at).map(&:note) notes = @fork_merge_request.notes.reorder(:created_at).map(&:note)
expect(notes[0]).to include('Restored source branch `master`') expect(notes[0]).to include('Restored source branch `master`')
expect(notes[1]).to include('Added 4 commits') expect(notes[1]).to include('Added 28 commits')
expect(@fork_merge_request).to be_open expect(@fork_merge_request).to be_open
end end
end end
......
...@@ -54,7 +54,7 @@ describe SystemNoteService, services: true do ...@@ -54,7 +54,7 @@ describe SystemNoteService, services: true do
it 'adds a message line for each commit' do it 'adds a message line for each commit' do
new_commits.each_with_index do |commit, i| new_commits.each_with_index do |commit, i|
# Skip the header # Skip the header
expect(note_lines[i + 1]).to eq "* #{commit.short_id} - #{commit.title}" expect(HTMLEntities.new.decode(note_lines[i + 1])).to eq "* #{commit.short_id} - #{commit.title}"
end end
end end
end end
...@@ -81,7 +81,7 @@ describe SystemNoteService, services: true do ...@@ -81,7 +81,7 @@ describe SystemNoteService, services: true do
end end
it 'includes a commit count' do it 'includes a commit count' do
expect(summary_line).to end_with " - 2 commits from branch `feature`" expect(summary_line).to end_with " - 26 commits from branch `feature`"
end end
end end
...@@ -91,7 +91,7 @@ describe SystemNoteService, services: true do ...@@ -91,7 +91,7 @@ describe SystemNoteService, services: true do
end end
it 'includes a commit count' do it 'includes a commit count' do
expect(summary_line).to end_with " - 2 commits from branch `feature`" expect(summary_line).to end_with " - 26 commits from branch `feature`"
end end
end end
...@@ -537,7 +537,7 @@ describe SystemNoteService, services: true do ...@@ -537,7 +537,7 @@ describe SystemNoteService, services: true do
let(:mergereq) { create(:merge_request, :simple, target_project: project, source_project: project) } let(:mergereq) { create(:merge_request, :simple, target_project: project, source_project: project) }
let(:jira_issue) { ExternalIssue.new("JIRA-1", project)} let(:jira_issue) { ExternalIssue.new("JIRA-1", project)}
let(:jira_tracker) { project.jira_service } let(:jira_tracker) { project.jira_service }
let(:commit) { project.commit } let(:commit) { project.repository.commits('master').find { |commit| commit.id == '5937ac0a7beb003549fc5fd26fc247adbce4a52e' } }
context 'in JIRA issue tracker' do context 'in JIRA issue tracker' do
before do before do
......
...@@ -5,6 +5,8 @@ module TestEnv ...@@ -5,6 +5,8 @@ module TestEnv
# When developing the seed repository, comment out the branch you will modify. # When developing the seed repository, comment out the branch you will modify.
BRANCH_SHA = { BRANCH_SHA = {
'not-merged-branch' => 'b83d6e3',
'branch-merged' => '498214d',
'empty-branch' => '7efb185', 'empty-branch' => '7efb185',
'ends-with.json' => '98b0d8b', 'ends-with.json' => '98b0d8b',
'flatten-dir' => 'e56497b', 'flatten-dir' => 'e56497b',
...@@ -14,7 +16,7 @@ module TestEnv ...@@ -14,7 +16,7 @@ module TestEnv
'improve/awesome' => '5937ac0', 'improve/awesome' => '5937ac0',
'markdown' => '0ed8c6c', 'markdown' => '0ed8c6c',
'lfs' => 'be93687', 'lfs' => 'be93687',
'master' => '5937ac0', 'master' => 'b83d6e3',
"'test'" => 'e56497b', "'test'" => 'e56497b',
'orphaned-branch' => '45127a9', 'orphaned-branch' => '45127a9',
'binary-encoding' => '7b1cf43', 'binary-encoding' => '7b1cf43',
......
...@@ -57,7 +57,7 @@ describe EmailsOnPushWorker do ...@@ -57,7 +57,7 @@ describe EmailsOnPushWorker do
end end
it "sends a mail with the correct subject" do it "sends a mail with the correct subject" do
expect(email.subject).to include('Change some files') expect(email.subject).to include('adds bar folder and branch-test text file')
end end
it "mentions force pushing in the body" do it "mentions force pushing in the body" do
...@@ -73,7 +73,7 @@ describe EmailsOnPushWorker do ...@@ -73,7 +73,7 @@ describe EmailsOnPushWorker do
before { perform } before { perform }
it "sends a mail with the correct subject" do it "sends a mail with the correct subject" do
expect(email.subject).to include('Change some files') expect(email.subject).to include('adds bar folder and branch-test text file')
end end
it "does not mention force pushing in the body" do it "does not mention force pushing in the body" do
......
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