Refactor snippet route helpers

In several places of the code we have conditions to check
whether the snippet is a `PersonalSnippet` or a `ProjectSnippet`.
In this commit we overwrite existing named routes helpers to
avoid that redundancy and in preparation for secret snippets,
where we will have to automatically add a query string param
to every personal snippet route.

app/helpers/blob_helper.rb

@@ -141,7 +141,7 @@ module BlobHelper
     if @build && @entry
       raw_project_job_artifacts_url(@project, @build, path: @entry.path, **kwargs)
     elsif @snippet
-      reliable_raw_snippet_url(@snippet)
+      raw_snippet_url(@snippet)
     elsif @blob
       project_raw_url(@project, @id, **kwargs)
     end

app/helpers/gitlab_routing_helper.rb

@@ -193,6 +193,101 @@ module GitlabRoutingHelper
     project = schedule.project
     take_ownership_project_pipeline_schedule_path(project, schedule, *args)
   end
+
+  def snippet_path(snippet, *args)
+    if snippet.is_a?(ProjectSnippet)
+      application_url_helpers.project_snippet_path(snippet.project, snippet, *args)
+    else
+      new_args = snippet_query_params(snippet, *args)
+      application_url_helpers.snippet_path(snippet, *new_args)
+    end
+  end
+
+  def snippet_url(snippet, *args)
+    if snippet.is_a?(ProjectSnippet)
+      application_url_helpers.project_snippet_url(snippet.project, snippet, *args)
+    else
+      new_args = snippet_query_params(snippet, *args)
+      application_url_helpers.snippet_url(snippet, *new_args)
+    end
+  end
+
+  def raw_snippet_path(snippet, *args)
+    if snippet.is_a?(ProjectSnippet)
+      application_url_helpers.raw_project_snippet_path(snippet.project, snippet, *args)
+    else
+      new_args = snippet_query_params(snippet, *args)
+      application_url_helpers.raw_snippet_path(snippet, *new_args)
+    end
+  end
+
+  def raw_snippet_url(snippet, *args)
+    if snippet.is_a?(ProjectSnippet)
+      application_url_helpers.raw_project_snippet_url(snippet.project, snippet, *args)
+    else
+      new_args = snippet_query_params(snippet, *args)
+      application_url_helpers.raw_snippet_url(snippet, *new_args)
+    end
+  end
+
+  def snippet_notes_path(snippet, *args)
+    new_args = snippet_query_params(snippet, *args)
+    application_url_helpers.snippet_notes_path(snippet, *new_args)
+  end
+
+  def snippet_notes_url(snippet, *args)
+    new_args = snippet_query_params(snippet, *args)
+    application_url_helpers.snippet_notes_url(snippet, *new_args)
+  end
+
+  def snippet_note_path(snippet, note, *args)
+    new_args = snippet_query_params(snippet, *args)
+    application_url_helpers.snippet_note_path(snippet, note, *new_args)
+  end
+
+  def snippet_note_url(snippet, note, *args)
+    new_args = snippet_query_params(snippet, *args)
+    application_url_helpers.snippet_note_url(snippet, note, *new_args)
+  end
+
+  def toggle_award_emoji_snippet_note_path(snippet, note, *args)
+    new_args = snippet_query_params(snippet, *args)
+    application_url_helpers.toggle_award_emoji_snippet_note_path(snippet, note, *new_args)
+  end
+
+  def toggle_award_emoji_snippet_note_url(snippet, note, *args)
+    new_args = snippet_query_params(snippet, *args)
+    application_url_helpers.toggle_award_emoji_snippet_note_url(snippet, note, *new_args)
+  end
+
+  def toggle_award_emoji_snippet_path(snippet, *args)
+    new_args = snippet_query_params(snippet, *args)
+    application_url_helpers.toggle_award_emoji_snippet_path(snippet, *new_args)
+  end
+
+  def toggle_award_emoji_snippet_url(snippet, *args)
+    new_args = snippet_query_params(snippet, *args)
+    application_url_helpers.toggle_award_emoji_snippet_url(snippet, *new_args)
+  end
+
+  private
+
+  def application_url_helpers
+    Gitlab::Routing.url_helpers
+  end
+
+  def snippet_query_params(snippet, *args)
+    opts = case args.last
+           when Hash
+             args.pop
+           when ActionController::Parameters
+             args.pop.to_h
+           else
+             {}
+           end
+
+    args << opts
+  end
 end
 
 GitlabRoutingHelper.include_if_ee('EE::GitlabRoutingHelper')

app/helpers/snippets_helper.rb

@@ -11,33 +11,9 @@ module SnippetsHelper
     end
   end
 
-  def reliable_snippet_path(snippet, opts = {})
-    reliable_snippet_url(snippet, opts.merge(only_path: true))
-  end
-
-  def reliable_raw_snippet_path(snippet, opts = {})
-    reliable_raw_snippet_url(snippet, opts.merge(only_path: true))
-  end
-
-  def reliable_snippet_url(snippet, opts = {})
-    if snippet.project_id?
-      project_snippet_url(snippet.project, snippet, nil, opts)
-    else
-      snippet_url(snippet, nil, opts)
-    end
-  end
-
-  def reliable_raw_snippet_url(snippet, opts = {})
-    if snippet.project_id?
-      raw_project_snippet_url(snippet.project, snippet, nil, opts)
-    else
-      raw_snippet_url(snippet, nil, opts)
-    end
-  end
-
   def download_raw_snippet_button(snippet)
     link_to(icon('download'),
-            reliable_raw_snippet_path(snippet, inline: false),
+            raw_snippet_path(snippet, inline: false),
             target: '_blank',
             rel: 'noopener noreferrer',
             class: "btn btn-sm has-tooltip",
@@ -133,7 +109,7 @@ module SnippetsHelper
   end
 
   def snippet_embed_tag(snippet)
-    content_tag(:script, nil, src: reliable_snippet_url(snippet, format: :js, only_path: false))
+    content_tag(:script, nil, src: snippet_url(snippet, format: :js))
   end
 
   def snippet_badge(snippet)
@@ -158,7 +134,7 @@ module SnippetsHelper
     return if blob.empty? || blob.binary? || blob.stored_externally?
 
     link_to(external_snippet_icon('doc-code'),
-            reliable_raw_snippet_url(@snippet),
+            raw_snippet_url(@snippet),
             class: 'btn',
             target: '_blank',
             rel: 'noopener noreferrer',
@@ -167,7 +143,7 @@ module SnippetsHelper
 
   def embedded_snippet_download_button
     link_to(external_snippet_icon('download'),
-            reliable_raw_snippet_url(@snippet, inline: false),
+            raw_snippet_url(@snippet, inline: false),
             class: 'btn',
             target: '_blank',
             title: 'Download',

app/views/search/results/_snippet_blob.html.haml

 - snippet_blob = chunk_snippet(snippet_blob, @search_term)
 - snippet = snippet_blob[:snippet_object]
 - snippet_chunks = snippet_blob[:snippet_chunks]
-- snippet_path = reliable_snippet_path(snippet)
+- snippet_path = snippet_path(snippet)
 
 .search-result-row
   %span

app/views/search/results/_snippet_title.html.haml

 .search-result-row
   %h4.snippet-title.term
-    = link_to reliable_snippet_path(snippet_title) do
+    = link_to snippet_path(snippet_title) do
       = truncate(snippet_title.title, length: 60)
       = snippet_badge(snippet_title)
     %span.cgray.monospace.tiny.float-right.term

app/views/shared/snippets/_snippet.html.haml

@@ -5,7 +5,7 @@
   = image_tag avatar_icon_for_user(snippet.author), class: "avatar s40 d-none d-sm-block", alt: ''
 
   .title
-    = link_to reliable_snippet_path(snippet) do
+    = link_to snippet_path(snippet) do
       = snippet.title
     - if snippet.file_name.present?
       %span.snippet-filename.d-none.d-sm-inline-block.ml-2
@@ -14,7 +14,7 @@
 
     %ul.controls
       %li
-        = link_to reliable_snippet_path(snippet, anchor: 'notes'), class: ('no-comments' if notes_count.zero?) do
+        = link_to snippet_path(snippet, anchor: 'notes'), class: ('no-comments' if notes_count.zero?) do
           = icon('comments')
           = notes_count
       %li

lib/api/entities.rb

@@ -532,7 +532,7 @@ module API
 
     class PersonalSnippet < Snippet
       expose :raw_url do |snippet|
-        Gitlab::UrlBuilder.build(snippet) + "/raw"
+        Gitlab::UrlBuilder.build(snippet, raw: true)
       end
     end
 

lib/gitlab/url_builder.rb

@@ -6,10 +6,10 @@ module Gitlab
     include GitlabRoutingHelper
     include ActionView::RecordIdentifier
 
-    attr_reader :object
+    attr_reader :object, :opts
 
-    def self.build(object)
-      new(object).url
+    def self.build(object, opts = {})
+      new(object, opts).url
     end
 
     def url
@@ -24,10 +24,8 @@ module Gitlab
         note_url
       when WikiPage
         wiki_page_url
-      when ProjectSnippet
-        project_snippet_url(object.project, object)
       when Snippet
-        snippet_url(object)
+        opts[:raw].present? ? raw_snippet_url(object) : snippet_url(object)
       when Milestone
         milestone_url(object)
       when ::Ci::Build
@@ -41,8 +39,9 @@ module Gitlab
 
     private
 
-    def initialize(object)
+    def initialize(object, opts = {})
       @object = object
+      @opts = opts
     end
 
     def commit_url(opts = {})
@@ -66,13 +65,7 @@ module Gitlab
         merge_request_url(object.noteable, anchor: dom_id(object))
 
       elsif object.for_snippet?
-        snippet = object.noteable
-
-        if snippet.is_a?(PersonalSnippet)
-          snippet_url(snippet, anchor: dom_id(object))
-        else
-          project_snippet_url(snippet.project, snippet, anchor: dom_id(object))
-        end
+        snippet_url(object.noteable, anchor: dom_id(object))
       end
     end
 

spec/helpers/award_emoji_helper_spec.rb

@@ -4,59 +4,69 @@ require 'spec_helper'
 
 describe AwardEmojiHelper do
   describe '.toggle_award_url' do
+    subject { helper.toggle_award_url(awardable) }
+
     context 'note on personal snippet' do
-      let(:note) { create(:note_on_personal_snippet) }
+      let(:snippet) { create(:personal_snippet) }
+      let(:note) { create(:note_on_personal_snippet, noteable: snippet) }
+      let(:awardable) { note }
+
+      subject { helper.toggle_award_url(note) }
 
       it 'returns correct url' do
         expected_url = "/snippets/#{note.noteable.id}/notes/#{note.id}/toggle_award_emoji"
 
-        expect(helper.toggle_award_url(note)).to eq(expected_url)
+        expect(subject).to eq(expected_url)
       end
     end
 
     context 'note on project item' do
       let(:note) { create(:note_on_project_snippet) }
+      let(:awardable) { note }
 
       it 'returns correct url' do
         @project = note.noteable.project
 
         expected_url = "/#{@project.namespace.path}/#{@project.path}/notes/#{note.id}/toggle_award_emoji"
 
-        expect(helper.toggle_award_url(note)).to eq(expected_url)
+        expect(subject).to eq(expected_url)
       end
     end
 
     context 'personal snippet' do
       let(:snippet) { create(:personal_snippet) }
+      let(:awardable) { snippet }
 
       it 'returns correct url' do
         expected_url = "/snippets/#{snippet.id}/toggle_award_emoji"
 
-        expect(helper.toggle_award_url(snippet)).to eq(expected_url)
+        expect(subject).to eq(expected_url)
       end
     end
 
     context 'merge request' do
       let(:merge_request) { create(:merge_request) }
+      let(:awardable) { merge_request }
 
       it 'returns correct url' do
         @project = merge_request.project
 
         expected_url = "/#{@project.namespace.path}/#{@project.path}/merge_requests/#{merge_request.iid}/toggle_award_emoji"
 
-        expect(helper.toggle_award_url(merge_request)).to eq(expected_url)
+        expect(subject).to eq(expected_url)
       end
     end
 
     context 'issue' do
       let(:issue) { create(:issue) }
+      let(:awardable) { issue }
 
       it 'returns correct url' do
         @project = issue.project
 
         expected_url = "/#{@project.namespace.path}/#{@project.path}/issues/#{issue.iid}/toggle_award_emoji"
 
-        expect(helper.toggle_award_url(issue)).to eq(expected_url)
+        expect(subject).to eq(expected_url)
       end
     end
   end

spec/helpers/gitlab_routing_helper_spec.rb

@@ -112,4 +112,98 @@ describe GitlabRoutingHelper do
       expect(edit_milestone_path(milestone)).to eq("/#{milestone.project.full_path}/-/milestones/#{milestone.iid}/edit")
     end
   end
+
+  context 'snippets' do
+    let_it_be(:personal_snippet) { create(:personal_snippet) }
+    let_it_be(:project_snippet) { create(:project_snippet) }
+    let_it_be(:note) { create(:note_on_personal_snippet, noteable: personal_snippet) }
+
+    describe '#snippet_path' do
+      it 'returns the personal snippet path' do
+        expect(snippet_path(personal_snippet)).to eq("/snippets/#{personal_snippet.id}")
+      end
+
+      it 'returns the project snippet path' do
+        expect(snippet_path(project_snippet)).to eq("/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}")
+      end
+    end
+
+    describe '#snippet_url' do
+      it 'returns the personal snippet url' do
+        expect(snippet_url(personal_snippet)).to eq("#{Settings.gitlab['url']}/snippets/#{personal_snippet.id}")
+      end
+
+      it 'returns the project snippet url' do
+        expect(snippet_url(project_snippet)).to eq("#{Settings.gitlab['url']}/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}")
+      end
+    end
+
+    describe '#raw_snippet_path' do
+      it 'returns the raw personal snippet path' do
+        expect(raw_snippet_path(personal_snippet)).to eq("/snippets/#{personal_snippet.id}/raw")
+      end
+
+      it 'returns the raw project snippet path' do
+        expect(raw_snippet_path(project_snippet)).to eq("/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}/raw")
+      end
+    end
+
+    describe '#raw_snippet_url' do
+      it 'returns the raw personal snippet url' do
+        expect(raw_snippet_url(personal_snippet)).to eq("#{Settings.gitlab['url']}/snippets/#{personal_snippet.id}/raw")
+      end
+
+      it 'returns the raw project snippet url' do
+        expect(raw_snippet_url(project_snippet)).to eq("#{Settings.gitlab['url']}/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}/raw")
+      end
+    end
+
+    describe '#snippet_notes_path' do
+      it 'returns the notes path for the personal snippet' do
+        expect(snippet_notes_path(personal_snippet)).to eq("/snippets/#{personal_snippet.id}/notes")
+      end
+    end
+
+    describe '#snippet_notes_url' do
+      it 'returns the notes url for the personal snippet' do
+        expect(snippet_notes_url(personal_snippet)).to eq("#{Settings.gitlab['url']}/snippets/#{personal_snippet.id}/notes")
+      end
+    end
+
+    describe '#snippet_note_path' do
+      it 'returns the note path for the personal snippet' do
+        expect(snippet_note_path(personal_snippet, note)).to eq("/snippets/#{personal_snippet.id}/notes/#{note.id}")
+      end
+    end
+
+    describe '#snippet_note_url' do
+      it 'returns the note url for the personal snippet' do
+        expect(snippet_note_url(personal_snippet, note)).to eq("#{Settings.gitlab['url']}/snippets/#{personal_snippet.id}/notes/#{note.id}")
+      end
+    end
+
+    describe '#toggle_award_emoji_snippet_note_path' do
+      it 'returns the note award emoji path for the personal snippet' do
+        expect(toggle_award_emoji_snippet_note_path(personal_snippet, note)).to eq("/snippets/#{personal_snippet.id}/notes/#{note.id}/toggle_award_emoji")
+      end
+    end
+
+    describe '#toggle_award_emoji_snippet_note_url' do
+      it 'returns the note award emoji url for the personal snippet' do
+        expect(toggle_award_emoji_snippet_note_url(personal_snippet, note)).to eq("#{Settings.gitlab['url']}/snippets/#{personal_snippet.id}/notes/#{note.id}/toggle_award_emoji")
+      end
+    end
+
+    describe '#toggle_award_emoji_snippet_path' do
+      it 'returns the award emoji path for the personal snippet' do
+        expect(toggle_award_emoji_snippet_path(personal_snippet)).to eq("/snippets/#{personal_snippet.id}/toggle_award_emoji")
+      end
+    end
+
+    describe '#toggle_award_emoji_snippet_url' do
+      it 'returns the award url for the personal snippet' do
+        expect(toggle_award_emoji_snippet_url(personal_snippet)).to eq("#{Settings.gitlab['url']}/snippets/#{personal_snippet.id}/toggle_award_emoji")
+      end
+    end
+  end
 end

spec/helpers/snippets_helper_spec.rb

@@ -9,107 +9,18 @@ describe SnippetsHelper do
   let_it_be(:public_personal_snippet) { create(:personal_snippet, :public) }
   let_it_be(:public_project_snippet) { create(:project_snippet, :public) }
 
-  describe '#reliable_snippet_path' do
-    subject { reliable_snippet_path(snippet) }
-
-    context 'personal snippets' do
-      let(:snippet) { public_personal_snippet }
-
-      context 'public' do
-        it 'returns a full path' do
-          expect(subject).to eq("/snippets/#{snippet.id}")
-        end
-      end
-    end
-
-    context 'project snippets' do
-      let(:snippet) { public_project_snippet }
-
-      it 'returns a full path' do
-        expect(subject).to eq("/#{snippet.project.full_path}/snippets/#{snippet.id}")
-      end
-    end
-  end
-
-  describe '#reliable_snippet_url' do
-    subject { reliable_snippet_url(snippet) }
-
-    context 'personal snippets' do
-      let(:snippet) { public_personal_snippet }
-
-      context 'public' do
-        it 'returns a full url' do
-          expect(subject).to eq("http://test.host/snippets/#{snippet.id}")
-        end
-      end
-    end
-
-    context 'project snippets' do
-      let(:snippet) { public_project_snippet }
-
-      it 'returns a full url' do
-        expect(subject).to eq("http://test.host/#{snippet.project.full_path}/snippets/#{snippet.id}")
-      end
-    end
-  end
-
-  describe '#reliable_raw_snippet_path' do
-    subject { reliable_raw_snippet_path(snippet) }
-
-    context 'personal snippets' do
-      let(:snippet) { public_personal_snippet }
-
-      context 'public' do
-        it 'returns a full path' do
-          expect(subject).to eq("/snippets/#{snippet.id}/raw")
-        end
-      end
-    end
-
-    context 'project snippets' do
-      let(:snippet) { public_project_snippet }
-
-      it 'returns a full path' do
-        expect(subject).to eq("/#{snippet.project.full_path}/snippets/#{snippet.id}/raw")
-      end
-    end
-  end
-
-  describe '#reliable_raw_snippet_url' do
-    subject { reliable_raw_snippet_url(snippet) }
-
-    context 'personal snippets' do
-      let(:snippet) { public_personal_snippet }
-
-      context 'public' do
-        it 'returns a full url' do
-          expect(subject).to eq("http://test.host/snippets/#{snippet.id}/raw")
-        end
-      end
-    end
-
-    context 'project snippets' do
-      let(:snippet) { public_project_snippet }
-
-      it 'returns a full url' do
-        expect(subject).to eq("http://test.host/#{snippet.project.full_path}/snippets/#{snippet.id}/raw")
-      end
-    end
-  end
-
   describe '#embedded_raw_snippet_button' do
     subject { embedded_raw_snippet_button.to_s }
 
     it 'returns view raw button of embedded snippets for personal snippets' do
       @snippet = create(:personal_snippet, :public)
-
-      expect(subject).to eq(download_link("http://test.host/snippets/#{@snippet.id}/raw"))
+      expect(subject).to eq(download_link("#{Settings.gitlab['url']}/snippets/#{@snippet.id}/raw"))
     end
 
     it 'returns view raw button of embedded snippets for project snippets' do
       @snippet = create(:project_snippet, :public)
 
-      expect(subject).to eq(download_link("http://test.host/#{@snippet.project.path_with_namespace}/snippets/#{@snippet.id}/raw"))
+      expect(subject).to eq(download_link("#{Settings.gitlab['url']}/#{@snippet.project.path_with_namespace}/snippets/#{@snippet.id}/raw"))
     end
 
     def download_link(url)
@@ -123,13 +34,13 @@ describe SnippetsHelper do
     it 'returns download button of embedded snippets for personal snippets' do
       @snippet = create(:personal_snippet, :public)
 
-      expect(subject).to eq(download_link("http://test.host/snippets/#{@snippet.id}/raw"))
+      expect(subject).to eq(download_link("#{Settings.gitlab['url']}/snippets/#{@snippet.id}/raw"))
     end
 
     it 'returns download button of embedded snippets for project snippets' do
       @snippet = create(:project_snippet, :public)
 
-      expect(subject).to eq(download_link("http://test.host/#{@snippet.project.path_with_namespace}/snippets/#{@snippet.id}/raw"))
+      expect(subject).to eq(download_link("#{Settings.gitlab['url']}/#{@snippet.project.path_with_namespace}/snippets/#{@snippet.id}/raw"))
     end
 
     def download_link(url)
@@ -145,7 +56,7 @@ describe SnippetsHelper do
 
       context 'public' do
         it 'returns a script tag with the snippet full url' do
-          expect(subject).to eq(script_embed("http://test.host/snippets/#{snippet.id}"))
+          expect(subject).to eq(script_embed("#{Settings.gitlab['url']}/snippets/#{snippet.id}"))
         end
       end
     end
@@ -154,7 +65,7 @@ describe SnippetsHelper do
       let(:snippet) { public_project_snippet }
 
       it 'returns a script tag with the snippet full url' do
-        expect(subject).to eq(script_embed("http://test.host/#{snippet.project.path_with_namespace}/snippets/#{snippet.id}"))
+        expect(subject).to eq(script_embed("#{Settings.gitlab['url']}/#{snippet.project.path_with_namespace}/snippets/#{snippet.id}"))
       end
     end
 

spec/lib/gitlab/url_builder_spec.rb

@@ -59,6 +59,26 @@ describe Gitlab::UrlBuilder do
       end
     end
 
+    context 'when passing a ProjectSnippet' do
+      it 'returns a proper URL' do
+        project_snippet = create(:project_snippet)
+
+        url = described_class.build(project_snippet)
+
+        expect(url).to eq "#{Settings.gitlab['url']}/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}"
+      end
+    end
+
+    context 'when passing a PersonalSnippet' do
+      it 'returns a proper URL' do
+        personal_snippet = create(:personal_snippet)
+
+        url = described_class.build(personal_snippet)
+
+        expect(url).to eq "#{Settings.gitlab['url']}/snippets/#{personal_snippet.id}"
+      end
+    end
+
     context 'when passing a Note' do
       context 'on a Commit' do
         it 'returns a proper URL' do