Refactor url_helpers for system note service and remove duplication of logic in spec
This commit is contained in:
parent
ec82cecf3c
commit
0c0b6f438e
2 changed files with 13 additions and 18 deletions
|
@ -21,10 +21,10 @@ module SystemNoteService
|
||||||
total_count = new_commits.length + existing_commits.length
|
total_count = new_commits.length + existing_commits.length
|
||||||
commits_text = "#{total_count} commit".pluralize(total_count)
|
commits_text = "#{total_count} commit".pluralize(total_count)
|
||||||
|
|
||||||
body = "[Compare with previous version](#{diff_comparison_url(noteable, project, oldrev)})\n\n"
|
body = "Added #{commits_text}:\n\n"
|
||||||
body << "Added #{commits_text}:\n\n"
|
|
||||||
body << existing_commit_summary(noteable, existing_commits, oldrev)
|
body << existing_commit_summary(noteable, existing_commits, oldrev)
|
||||||
body << new_commit_summary(new_commits).join("\n")
|
body << new_commit_summary(new_commits).join("\n")
|
||||||
|
body << "\n\n[Compare with previous version](#{diff_comparison_url(noteable, project, oldrev)})"
|
||||||
|
|
||||||
create_note(noteable: noteable, project: project, author: author, note: body)
|
create_note(noteable: noteable, project: project, author: author, note: body)
|
||||||
end
|
end
|
||||||
|
@ -255,8 +255,7 @@ module SystemNoteService
|
||||||
#
|
#
|
||||||
# "Started branch `201-issue-branch-button`"
|
# "Started branch `201-issue-branch-button`"
|
||||||
def new_issue_branch(issue, project, author, branch)
|
def new_issue_branch(issue, project, author, branch)
|
||||||
h = Gitlab::Routing.url_helpers
|
link = url_helpers.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch)
|
||||||
link = h.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch)
|
|
||||||
|
|
||||||
body = "Started branch [`#{branch}`](#{link})"
|
body = "Started branch [`#{branch}`](#{link})"
|
||||||
create_note(noteable: issue, project: project, author: author, note: body)
|
create_note(noteable: issue, project: project, author: author, note: body)
|
||||||
|
@ -468,10 +467,14 @@ module SystemNoteService
|
||||||
Rack::Utils.escape_html(text)
|
Rack::Utils.escape_html(text)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def url_helpers
|
||||||
|
@url_helpers ||= Gitlab::Routing.url_helpers
|
||||||
|
end
|
||||||
|
|
||||||
def diff_comparison_url(merge_request, project, oldrev)
|
def diff_comparison_url(merge_request, project, oldrev)
|
||||||
diff_id = merge_request.merge_request_diff.id
|
diff_id = merge_request.merge_request_diff.id
|
||||||
|
|
||||||
Gitlab::Routing.url_helpers.diffs_namespace_project_merge_request_url(
|
url_helpers.diffs_namespace_project_merge_request_url(
|
||||||
project.namespace,
|
project.namespace,
|
||||||
project,
|
project,
|
||||||
merge_request.iid,
|
merge_request.iid,
|
||||||
|
|
|
@ -41,34 +41,26 @@ describe SystemNoteService, services: true do
|
||||||
let(:note_lines) { subject.note.split("\n").reject(&:blank?) }
|
let(:note_lines) { subject.note.split("\n").reject(&:blank?) }
|
||||||
|
|
||||||
describe 'comparison diff link line' do
|
describe 'comparison diff link line' do
|
||||||
it 'adds the comparison link' do
|
it 'adds the comparison text' do
|
||||||
link = Gitlab::Routing.url_helpers.diffs_namespace_project_merge_request_url(
|
expect(note_lines[2]).to match "[Compare with previous version]"
|
||||||
project.namespace,
|
|
||||||
project,
|
|
||||||
noteable.iid,
|
|
||||||
diff_id: noteable.merge_request_diff.id,
|
|
||||||
start_sha: oldrev
|
|
||||||
)
|
|
||||||
|
|
||||||
expect(note_lines[0]).to eq "[Compare with previous version](#{link})"
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'without existing commits' do
|
context 'without existing commits' do
|
||||||
it 'adds a message header' do
|
it 'adds a message header' do
|
||||||
expect(note_lines[1]).to eq "Added #{new_commits.size} commits:"
|
expect(note_lines[0]).to eq "Added #{new_commits.size} commits:"
|
||||||
end
|
end
|
||||||
|
|
||||||
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 + 2]).to eq "* #{commit.short_id} - #{commit.title}"
|
expect(note_lines[i + 1]).to eq "* #{commit.short_id} - #{commit.title}"
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'summary line for existing commits' do
|
describe 'summary line for existing commits' do
|
||||||
let(:summary_line) { note_lines[2] }
|
let(:summary_line) { note_lines[1] }
|
||||||
|
|
||||||
context 'with one existing commit' do
|
context 'with one existing commit' do
|
||||||
let(:old_commits) { [noteable.commits.last] }
|
let(:old_commits) { [noteable.commits.last] }
|
||||||
|
|
Loading…
Reference in a new issue