refactored and added missing spec to light URL builder

6 files changed, 174 insertions, 112 deletions

diff --git a/lib/gitlab/cycle_analytics/events.rb b/lib/gitlab/cycle_analytics/events.rb
index 7d06c5e890f..68a2de79409 100644
--- a/lib/gitlab/cycle_analytics/events.rb
+++ b/lib/gitlab/cycle_analytics/events.rb
@@ -52,7 +52,7 @@ module Gitlab
       private
 
       def parse_event(event, entity: :issue)
-        event['url'] = Gitlab::LightUrlBuilder.build(entity: entity, project: @project, id: event['id'])
+        event['url'] = Gitlab::LightUrlBuilder.build(entity: entity, project: @project, id: event['iid'])
         event['total_time'] = distance_of_time_in_words(event['total_time'].to_f)
         event['created_at'] = interval_in_words(event['created_at'])
         event['author_profile_url'] = Gitlab::LightUrlBuilder.build(entity: :user, id: event['author_username'])
@@ -64,11 +64,11 @@ module Gitlab
       def parse_build_event(event)
         build = ::Ci::Build.find(event['id'])
         event['name'] = build.name
-        event['url'] = Gitlab::LightUrlBuilder.build(entity: :build_url, project: @project, id: build.id)
+        event['url'] = Gitlab::LightUrlBuilder.build(entity: :build, project: @project, id: build.id)
         event['branch'] = build.ref
-        event['branch_url'] = Gitlab::LightUrlBuilder.build(entity: :branch_url, project: @project, id: build.ref)
+        event['branch'] = Gitlab::LightUrlBuilder.build(entity: :branch, project: @project, id: build.ref)
         event['sha'] = build.short_sha
-        event['commit_url'] = Gitlab::LightUrlBuilder.build(entity: :commit_url, project: @project, id: build.sha)
+        event['commit'] = Gitlab::LightUrlBuilder.build(entity: :commit, project: @project, id: build.sha)
         event['date'] = build.started_at
         event['total_time'] = build.duration
         event['author_name'] = build.author.try(:name)
diff --git a/lib/gitlab/cycle_analytics/metrics_fetcher.rb b/lib/gitlab/cycle_analytics/metrics_fetcher.rb
index f6522905c10..6743cbcc9e0 100644
--- a/lib/gitlab/cycle_analytics/metrics_fetcher.rb
+++ b/lib/gitlab/cycle_analytics/metrics_fetcher.rb
@@ -6,10 +6,6 @@ module Gitlab
 
       DEPLOYMENT_METRIC_STAGES = %i[production staging]
 
-      def self.included(klass)
-        klass.extend self
-      end
-
       private
 
       def calculate_metric(name, start_time_attrs, end_time_attrs)
diff --git a/lib/gitlab/light_url_builder.rb b/lib/gitlab/light_url_builder.rb
index 0b589217273..7875491a1d8 100644
--- a/lib/gitlab/light_url_builder.rb
+++ b/lib/gitlab/light_url_builder.rb
@@ -22,15 +22,15 @@ module Gitlab
         issue_url
       when :user
         user_url(@id)
-      when :user_avatar_url
+      when :user_avatar
         user_avatar_url
-      when :commit_url
+      when :commit
         commit_url
       when :merge_request
         mr_url
-      when :build_url
+      when :build
         namespace_project_build_url(@project.namespace, @project, @id)
-      when :branch_url
+      when :branch
         branch_url
       else
         raise NotImplementedError.new("No URL builder defined for #{object.class}")
@@ -67,14 +67,6 @@ module Gitlab
                                           }.merge!(@opts))
     end
 
-    def pipeline_url
-      namespace_project_build_url({
-                                       namespace_id: @project.namespace,
-                                       project_id: @project,
-                                       id: @id
-                                     }.merge!(@opts))
-    end
-
     def branch_url
       "#{project_url(@project)}/commits/#{@id}"
     end
diff --git a/spec/lib/gitlab/cycle_analytics/events_spec.rb b/spec/lib/gitlab/cycle_analytics/events_spec.rb
index 29cc3011542..0fb6a62f10d 100644
--- a/spec/lib/gitlab/cycle_analytics/events_spec.rb
+++ b/spec/lib/gitlab/cycle_analytics/events_spec.rb
@@ -196,7 +196,7 @@ describe Gitlab::CycleAnalytics::Events do
     end
 
     it "has the author's name" do
-      expect(subject.review_events.first['author_name']).to eq(context.author.name)
+      expect(subject.review_events.first['author_name']).to eq(MergeRequest.first.author.name)
     end
   end
 
diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb
index a826b24419a..5d876fdb603 100644
--- a/spec/lib/gitlab/url_builder_spec.rb
+++ b/spec/lib/gitlab/url_builder_spec.rb
@@ -1,119 +1,74 @@
 require 'spec_helper'
 
-describe Gitlab::UrlBuilder, lib: true do
-  describe '.build' do
-    context 'when passing a Commit' do
-      it 'returns a proper URL' do
-        commit = build_stubbed(:commit)
+describe Gitlab::LightUrlBuilder, lib: true do
+  context 'when passing a Commit' do
+    it 'returns a proper URL' do
+      commit = build_stubbed(:commit)
 
-        url = described_class.build(commit)
+      url = described_class.build(entity: :commit, project: commit.project, id: commit.id)
 
-        expect(url).to eq "#{Settings.gitlab['url']}/#{commit.project.path_with_namespace}/commit/#{commit.id}"
-      end
+      expect(url).to eq "#{Settings.gitlab['url']}/#{commit.project.path_with_namespace}/commit/#{commit.id}"
     end
+  end
 
-    context 'when passing an Issue' do
-      it 'returns a proper URL' do
-        issue = build_stubbed(:issue, iid: 42)
+  context 'when passing an Issue' do
+    it 'returns a proper URL' do
+      issue = build_stubbed(:issue, iid: 42)
 
-        url = described_class.build(issue)
+      url = described_class.build(entity: :issue, project: issue.project, id: issue.iid)
 
-        expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}"
-      end
+      expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}"
     end
+  end
 
-    context 'when passing a MergeRequest' do
-      it 'returns a proper URL' do
-        merge_request = build_stubbed(:merge_request, iid: 42)
+  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)
+      url = described_class.build(entity: :merge_request, project: merge_request.project, id: merge_request.iid)
 
-        expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}"
-      end
+      expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}"
     end
+  end
 
-    context 'when passing a Note' do
-      context 'on a Commit' do
-        it 'returns a proper URL' do
-          note = build_stubbed(:note_on_commit)
-
-          url = described_class.build(note)
-
-          expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}"
-        end
-      end
-
-      context 'on a Commit Diff' do
-        it 'returns a proper URL' do
-          note = build_stubbed(:diff_note_on_commit)
-
-          url = described_class.build(note)
-
-          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 'when passing a build' do
+    it 'returns a proper URL' do
+      build = build_stubbed(:ci_build, project: build_stubbed(:empty_project))
 
-      context 'on a MergeRequest Diff' do
-        it 'returns a proper URL' do
-          merge_request = create(:merge_request, iid: 42)
-          note = build_stubbed(:diff_note_on_merge_request, noteable: merge_request)
+      url = described_class.build(entity: :build, project: build.project, id: build.id)
 
-          url = described_class.build(note)
+      expect(url).to eq "#{Settings.gitlab['url']}/#{build.project.path_with_namespace}/builds/#{build.id}"
+    end
+  end
 
-          expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}#note_#{note.id}"
-        end
-      end
+  context 'when passing a branch' do
+    it 'returns a proper URL' do
+      branch = 'branch_name'
+      project = build_stubbed(:empty_project)
 
-      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(entity: :branch, project: project, id: branch)
 
-          url = described_class.build(note)
+      expect(url).to eq "#{Settings.gitlab['url']}/#{project.path_with_namespace}/commits/#{branch}"
+    end
+  end
 
-          expect(url).to eq "#{Settings.gitlab['url']}/#{project_snippet.project.path_with_namespace}/snippets/#{note.noteable_id}#note_#{note.id}"
-        end
-      end
+  context 'on a User' do
+    it 'returns a proper URL' do
+      user = build_stubbed(:user)
 
-      context 'on another object' do
-        it 'returns a proper URL' do
-          project = build_stubbed(:project)
+      url = described_class.build(entity: :user, id: user.username)
 
-          expect { described_class.build(project) }.
-            to raise_error(NotImplementedError, 'No URL builder defined for Project')
-        end
-      end
+      expect(url).to eq "#{Settings.gitlab['url']}/#{user.username}"
     end
+  end
+
+  context 'on a user avatar' do
+    it 'returns a proper URL' do
+      user = create(:user)
 
-    context 'when passing a WikiPage' do
-      it 'returns a proper URL' do
-        wiki_page = build(:wiki_page)
-        url = described_class.build(wiki_page)
+      url = described_class.build(entity: :user_avatar, id: user.id)
 
-        expect(url).to eq "#{Gitlab.config.gitlab.url}#{wiki_page.wiki.wiki_base_path}/#{wiki_page.slug}"
-      end
+      expect(url).to eq user.avatar_url
     end
   end
 end
diff --git a/spec/lib/light_url_builder_spec.rb b/spec/lib/light_url_builder_spec.rb
new file mode 100644
index 00000000000..a826b24419a
--- /dev/null
+++ b/spec/lib/light_url_builder_spec.rb
@@ -0,0 +1,119 @@
+require 'spec_helper'
+
+describe Gitlab::UrlBuilder, lib: true do
+  describe '.build' do
+    context 'when passing a Commit' do
+      it 'returns a proper URL' do
+        commit = build_stubbed(:commit)
+
+        url = described_class.build(commit)
+
+        expect(url).to eq "#{Settings.gitlab['url']}/#{commit.project.path_with_namespace}/commit/#{commit.id}"
+      end
+    end
+
+    context 'when passing an Issue' do
+      it 'returns a proper URL' do
+        issue = build_stubbed(:issue, iid: 42)
+
+        url = described_class.build(issue)
+
+        expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.path_with_namespace}/issues/#{issue.iid}"
+      end
+    end
+
+    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)
+
+        expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.path_with_namespace}/merge_requests/#{merge_request.iid}"
+      end
+    end
+
+    context 'when passing a Note' do
+      context 'on a Commit' do
+        it 'returns a proper URL' do
+          note = build_stubbed(:note_on_commit)
+
+          url = described_class.build(note)
+
+          expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.path_with_namespace}/commit/#{note.commit_id}#note_#{note.id}"
+        end
+      end
+
+      context 'on a Commit Diff' do
+        it 'returns a proper URL' do
+          note = build_stubbed(:diff_note_on_commit)
+
+          url = described_class.build(note)
+
+          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 MergeRequest Diff' do
+        it 'returns a proper URL' do
+          merge_request = create(:merge_request, iid: 42)
+          note = build_stubbed(:diff_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 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
+
+      context 'on another object' do
+        it 'returns a proper URL' do
+          project = build_stubbed(:project)
+
+          expect { described_class.build(project) }.
+            to raise_error(NotImplementedError, 'No URL builder defined for Project')
+        end
+      end
+    end
+
+    context 'when passing a WikiPage' do
+      it 'returns a proper URL' do
+        wiki_page = build(:wiki_page)
+        url = described_class.build(wiki_page)
+
+        expect(url).to eq "#{Gitlab.config.gitlab.url}#{wiki_page.wiki.wiki_base_path}/#{wiki_page.slug}"
+      end
+    end
+  end
+end