diff --git a/app/models/commit.rb b/app/models/commit.rb
index 11ecfcace14ed24fccc7082f4b5cad4916457cac..d1f07ccd55ce6934b8792510fce4ed3934a43355 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -154,7 +154,7 @@ class Commit
id: id,
message: safe_message,
timestamp: committed_date.xmlschema,
- url: commit_url,
+ url: Gitlab::UrlBuilder.build(self),
author: {
name: author_name,
email: author_email
@@ -168,10 +168,6 @@ class Commit
data
end
- def commit_url
- project.present? ? "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/commit/#{id}" : ""
- end
-
# Discover issues should be closed when this commit is pushed to a project's
# default branch.
def closes_issues(current_user = self.committer)
diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb
index 770f32de9448277bdbc9c7d6cc74ecf5c70b7cad..772f5c5fffad6b69f302f2578b405ae14d88e225 100644
--- a/app/services/issues/base_service.rb
+++ b/app/services/issues/base_service.rb
@@ -3,7 +3,7 @@ module Issues
def hook_data(issue, action)
issue_data = issue.to_hook_data(current_user)
- issue_url = Gitlab::UrlBuilder.new(:issue).build(issue.id)
+ issue_url = Gitlab::UrlBuilder.build(issue)
issue_data[:object_attributes].merge!(url: issue_url, action: action)
issue_data
end
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index ac5b58db8623755e252981c02b9e5bb5d6b43bae..e6837a186963e1df39d02bec64ed44cade17efcd 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -20,8 +20,7 @@ module MergeRequests
def hook_data(merge_request, action)
hook_data = merge_request.to_hook_data(current_user)
- merge_request_url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id)
- hook_data[:object_attributes][:url] = merge_request_url
+ hook_data[:object_attributes][:url] = Gitlab::UrlBuilder.build(merge_request)
hook_data[:object_attributes][:action] = action
hook_data
end
diff --git a/app/views/search/results/_note.html.haml b/app/views/search/results/_note.html.haml
index 9544e3d3e17ac89858365ba0ca91bf4398ff49c9..d9400b1d9fa63e76c97000c6703c63fb01bca098 100644
--- a/app/views/search/results/_note.html.haml
+++ b/app/views/search/results/_note.html.haml
@@ -1,5 +1,5 @@
- project = note.project
-- note_url = Gitlab::UrlBuilder.new(:note).build(note.id)
+- note_url = Gitlab::UrlBuilder.build(note)
- noteable_identifier = note.noteable.try(:iid) || note.noteable.id
.search-result-row
%h5.note-search-caption.str-truncated
diff --git a/lib/gitlab/note_data_builder.rb b/lib/gitlab/note_data_builder.rb
index 18523e0aefe460a25dd5818b77ccafe58daf651a..8bdc89a7751f3f77a8a001b51b57dcf2641e9dd1 100644
--- a/lib/gitlab/note_data_builder.rb
+++ b/lib/gitlab/note_data_builder.rb
@@ -59,8 +59,7 @@ module Gitlab
repository: project.hook_attrs.slice(:name, :url, :description, :homepage)
}
- base_data[:object_attributes][:url] =
- Gitlab::UrlBuilder.new(:note).build(note.id)
+ base_data[:object_attributes][:url] = Gitlab::UrlBuilder.build(note)
base_data
end
diff --git a/lib/gitlab/url_builder.rb b/lib/gitlab/url_builder.rb
index f301d42939d17570ca570f0a50aee6a808f8168f..f1943222edfac3aa81e590c80fce830573de7477 100644
--- a/lib/gitlab/url_builder.rb
+++ b/lib/gitlab/url_builder.rb
@@ -4,50 +4,58 @@ module Gitlab
include GitlabRoutingHelper
include ActionView::RecordIdentifier
- def initialize(type)
- @type = type
- end
+ attr_reader :object
- def build(id)
- case @type
- when :issue
- build_issue_url(id)
- when :merge_request
- build_merge_request_url(id)
- when :note
- build_note_url(id)
+ def self.build(object)
+ new(object).url
+ end
+ def url
+ case object
+ when Commit
+ commit_url
+ when Issue
+ issue_url(object)
+ when MergeRequest
+ merge_request_url(object)
+ when Note
+ note_url
+ else
+ raise NotImplementedError.new("No URL builder defined for #{object.class}")
end
end
private
- def build_issue_url(id)
- issue = Issue.find(id)
- issue_url(issue)
+ def initialize(object)
+ @object = object
end
- def build_merge_request_url(id)
- merge_request = MergeRequest.find(id)
- merge_request_url(merge_request)
+ def commit_url(opts = {})
+ return '' if object.project.nil?
+
+ namespace_project_commit_url({
+ namespace_id: object.project.namespace,
+ project_id: object.project,
+ id: object.id
+ }.merge!(opts))
end
- def build_note_url(id)
- note = Note.find(id)
- if note.for_commit?
- namespace_project_commit_url(namespace_id: note.project.namespace,
- id: note.commit_id,
- project_id: note.project,
- anchor: dom_id(note))
- elsif note.for_issue?
- issue = Issue.find(note.noteable_id)
- issue_url(issue, anchor: dom_id(note))
- elsif note.for_merge_request?
- merge_request = MergeRequest.find(note.noteable_id)
- merge_request_url(merge_request, anchor: dom_id(note))
- elsif note.for_snippet?
- snippet = Snippet.find(note.noteable_id)
- project_snippet_url(snippet, anchor: dom_id(note))
+ def note_url
+ if object.for_commit?
+ commit_url(id: object.commit_id, anchor: dom_id(object))
+
+ elsif object.for_issue?
+ issue = Issue.find(object.noteable_id)
+ issue_url(issue, anchor: dom_id(object))
+
+ elsif object.for_merge_request?
+ merge_request = MergeRequest.find(object.noteable_id)
+ merge_request_url(merge_request, anchor: dom_id(object))
+
+ elsif object.for_snippet?
+ snippet = Snippet.find(object.noteable_id)
+ project_snippet_url(snippet, anchor: dom_id(object))
end
end
end
diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb
new file mode 100644
index 0000000000000000000000000000000000000000..ac6eb0a7897440e504d5ca5367a60817b89c904b
--- /dev/null
+++ b/spec/factories/commits.rb
@@ -0,0 +1,12 @@
+require_relative '../support/repo_helpers'
+
+FactoryGirl.define do
+ factory :commit do
+ git_commit RepoHelpers.sample_commit
+ project factory: :empty_project
+
+ initialize_with do
+ new(git_commit, project)
+ end
+ end
+end
diff --git a/spec/lib/gitlab/note_data_builder_spec.rb b/spec/lib/gitlab/note_data_builder_spec.rb
index da6526774437aa24bc4551ffce44db080520ab13..f093d0a0d8b3a7dd5b481841cf7e92d68abdc783 100644
--- a/spec/lib/gitlab/note_data_builder_spec.rb
+++ b/spec/lib/gitlab/note_data_builder_spec.rb
@@ -4,13 +4,12 @@ describe 'Gitlab::NoteDataBuilder', lib: true do
let(:project) { create(:project) }
let(:user) { create(:user) }
let(:data) { Gitlab::NoteDataBuilder.build(note, user) }
- let(:note_url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
let(:fixed_time) { Time.at(1425600000) } # Avoid time precision errors
before(:each) do
expect(data).to have_key(:object_attributes)
expect(data[:object_attributes]).to have_key(:url)
- expect(data[:object_attributes][:url]).to eq(note_url)
+ expect(data[:object_attributes][:url]).to eq(Gitlab::UrlBuilder.build(note))
expect(data[:object_kind]).to eq('note')
expect(data[:user]).to eq(user.hook_attrs)
end
diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb
index f023be6ae45dd3e7374a5a70d6e4703eafb17f1c..6ffc0d6e658fc3ad153af35e8fe1c7fe820f89aa 100644
--- a/spec/lib/gitlab/url_builder_spec.rb
+++ b/spec/lib/gitlab/url_builder_spec.rb
@@ -1,77 +1,110 @@
require 'spec_helper'
describe Gitlab::UrlBuilder, lib: true do
- describe 'When asking for an issue' do
- it 'returns the issue url' do
- issue = create(:issue)
- url = Gitlab::UrlBuilder.new(:issue).build(issue.id)
- expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}"
- end
- end
+ describe '.build' do
+ context 'when passing a Commit' do
+ it 'returns a proper URL' do
+ commit = build_stubbed(:commit)
- describe 'When asking for an merge request' do
- it 'returns the merge request url' do
- merge_request = create(:merge_request)
- url = Gitlab::UrlBuilder.new(:merge_request).build(merge_request.id)
- expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}"
+ url = described_class.build(commit)
+
+ expect(url).to eq "#{Settings.gitlab['url']}/#{commit.project.path_with_namespace}/commit/#{commit.id}"
+ end
end
- end
- describe 'When asking for a note on commit' do
- let(:note) { create(:note_on_commit) }
- let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+ context 'when passing an Issue' do
+ it 'returns a proper URL' do
+ issue = build_stubbed(:issue, iid: 42)
- it 'returns the note url' do
- expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}"
+ url = described_class.build(issue)
+
+ expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}"
+ end
end
- end
- describe 'When asking for a note on commit diff' do
- let(:note) { create(:note_on_commit_diff) }
- let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+ context 'when passing a MergeRequest' do
+ it 'returns a proper URL' do
+ merge_request = build_stubbed(:merge_request, iid: 42)
+
+ url = described_class.build(merge_request)
- it 'returns the note url' do
- expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}"
+ expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}"
+ end
end
- end
- describe 'When asking for a note on issue' do
- let(:issue) { create(:issue) }
- let(:note) { create(:note_on_issue, noteable_id: issue.id) }
- let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+ context 'when passing a Note' do
+ context 'on a Commit' do
+ it 'returns a proper URL' do
+ note = build_stubbed(:note_on_commit)
- it 'returns the note url' do
- expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}#note_#{note.id}"
- end
- end
+ url = described_class.build(note)
- describe 'When asking for a note on merge request' do
- let(:merge_request) { create(:merge_request) }
- let(:note) { create(:note_on_merge_request, noteable_id: merge_request.id) }
- let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+ expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}"
+ end
+ end
- it 'returns the note url' do
- expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}#note_#{note.id}"
- end
- end
+ context 'on a CommitDiff' do
+ it 'returns a proper URL' do
+ note = build_stubbed(:note_on_commit_diff)
- describe 'When asking for a note on merge request diff' do
- let(:merge_request) { create(:merge_request) }
- let(:note) { create(:note_on_merge_request_diff, noteable_id: merge_request.id) }
- let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+ url = described_class.build(note)
- it 'returns the note url' do
- expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}#note_#{note.id}"
- end
- end
+ expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}"
+ end
+ end
+
+ context 'on an Issue' do
+ it 'returns a proper URL' do
+ issue = create(:issue, iid: 42)
+ note = build_stubbed(:note_on_issue, noteable: issue)
+
+ url = described_class.build(note)
+
+ expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}#note_#{note.id}"
+ end
+ end
+
+ context 'on a MergeRequest' do
+ it 'returns a proper URL' do
+ merge_request = create(:merge_request, iid: 42)
+ note = build_stubbed(:note_on_merge_request, noteable: merge_request)
+
+ url = described_class.build(note)
+
+ expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}#note_#{note.id}"
+ end
+ end
+
+ context 'on a MergeRequestDiff' do
+ it 'returns a proper URL' do
+ merge_request = create(:merge_request, iid: 42)
+ note = build_stubbed(:note_on_merge_request_diff, noteable: merge_request)
+
+ url = described_class.build(note)
+
+ expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}#note_#{note.id}"
+ end
+ end
+
+ context 'on a ProjectSnippet' do
+ it 'returns a proper URL' do
+ project_snippet = create(:project_snippet)
+ note = build_stubbed(:note_on_project_snippet, noteable: project_snippet)
+
+ url = described_class.build(note)
+
+ expect(url).to eq "#{Settings.gitlab['url']}/#{project_snippet.project.path_with_namespace}/snippets/#{note.noteable_id}#note_#{note.id}"
+ end
+ end
- describe 'When asking for a note on project snippet' do
- let(:snippet) { create(:project_snippet) }
- let(:note) { create(:note_on_project_snippet, noteable_id: snippet.id) }
- let(:url) { Gitlab::UrlBuilder.new(:note).build(note.id) }
+ context 'on another object' do
+ it 'returns a proper URL' do
+ project = build_stubbed(:project)
- it 'returns the note url' do
- expect(url).to eq "#{Settings.gitlab['url']}/#{snippet.project.path_with_namespace}/snippets/#{note.noteable_id}#note_#{note.id}"
+ expect { described_class.build(project) }.
+ to raise_error(NotImplementedError, 'No URL builder defined for Project')
+ end
+ end
end
end
end