Merge branch 'fj-snippets-refactoring' into 'master'

Snippets views refactoring

See merge request gitlab-org/gitlab!19582

app/helpers/blob_helper.rb

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

app/helpers/snippets_helper.rb

@@ -11,22 +11,40 @@ module SnippetsHelper
     end
   end
 
-  def reliable_snippet_path(snippet, opts = nil)
+  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_path(snippet.project, snippet, opts)
+      project_snippet_url(snippet.project, snippet, nil, opts)
     else
-      snippet_path(snippet, opts)
+      snippet_url(snippet, nil, opts)
     end
   end
 
-  def download_snippet_path(snippet)
-    if snippet.project_id
-      raw_project_snippet_path(@project, snippet, inline: false)
+  def reliable_raw_snippet_url(snippet, opts = {})
+    if snippet.project_id?
+      raw_project_snippet_url(snippet.project, snippet, nil, opts)
     else
-      raw_snippet_path(snippet, inline: false)
+      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),
+            target: '_blank',
+            rel: 'noopener noreferrer',
+            class: "btn btn-sm has-tooltip",
+            title: 'Download',
+            data: { container: 'body' })
+  end
+
   # Return the path of a snippets index for a user or for a project
   #
   # @returns String, path to snippet index
@@ -114,30 +132,45 @@ module SnippetsHelper
     { snippet_object: snippet, snippet_chunks: snippet_chunks }
   end
 
-  def snippet_embed
-    "<script src=\"#{url_for(only_path: false, overwrite_params: nil)}.js\"></script>"
+  def snippet_embed_tag(snippet)
+    content_tag(:script, nil, src: reliable_snippet_url(snippet, format: :js, only_path: false))
   end
 
-  def embedded_snippet_raw_button
-    blob = @snippet.blob
-    return if blob.empty? || blob.binary? || blob.stored_externally?
+  def snippet_badge(snippet)
+    return unless attrs = snippet_badge_attributes(snippet)
 
-    snippet_raw_url = if @snippet.is_a?(PersonalSnippet)
-                        raw_snippet_url(@snippet)
-                      else
-                        raw_project_snippet_url(@snippet.project, @snippet)
+    css_class, text = attrs
+    tag.span(class: ['badge', 'badge-gray']) do
+      concat(tag.i(class: ['fa', css_class]))
+      concat(' ')
+      concat(text)
+    end
   end
 
-    link_to external_snippet_icon('doc-code'), snippet_raw_url, class: 'btn', target: '_blank', rel: 'noopener noreferrer', title: 'Open raw'
+  def snippet_badge_attributes(snippet)
+    if snippet.private?
+      ['fa-lock', _('private')]
+    end
   end
 
-  def embedded_snippet_download_button
-    download_url = if @snippet.is_a?(PersonalSnippet)
-                     raw_snippet_url(@snippet, inline: false)
-                   else
-                     raw_project_snippet_url(@snippet.project, @snippet, inline: false)
+  def embedded_raw_snippet_button
+    blob = @snippet.blob
+    return if blob.empty? || blob.binary? || blob.stored_externally?
+
+    link_to(external_snippet_icon('doc-code'),
+            reliable_raw_snippet_url(@snippet),
+            class: 'btn',
+            target: '_blank',
+            rel: 'noopener noreferrer',
+            title: 'Open raw')
   end
 
-    link_to external_snippet_icon('download'), download_url, class: 'btn', target: '_blank', title: 'Download', rel: 'noopener noreferrer'
+  def embedded_snippet_download_button
+    link_to(external_snippet_icon('download'),
+            reliable_raw_snippet_url(@snippet, inline: false),
+            class: 'btn',
+            target: '_blank',
+            title: 'Download',
+            rel: 'noopener noreferrer')
   end
 end

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)
 
 .search-result-row
   %span
@@ -11,7 +12,6 @@
       = snippet.author_name
     %span.light= time_ago_with_tooltip(snippet.created_at)
   %h4.snippet-title
-  - snippet_path = reliable_snippet_path(snippet)
   .file-holder
     .js-file-title.file-title
       = link_to snippet_path do

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

@@ -2,10 +2,7 @@
   %h4.snippet-title.term
     = link_to reliable_snippet_path(snippet_title) do
       = truncate(snippet_title.title, length: 60)
-      - if snippet_title.private?
-        %span.badge.badge-gray
-          %i.fa.fa-lock
-          = _("private")
+      = snippet_badge(snippet_title)
     %span.cgray.monospace.tiny.float-right.term
       = snippet_title.file_name
 

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

@@ -8,7 +8,6 @@
     .btn-group{ role: "group" }<
       = copy_blob_source_button(blob)
       = open_raw_blob_button(blob)
-
-      = link_to icon('download'), download_snippet_path(@snippet), target: '_blank', class: "btn btn-sm has-tooltip", title: 'Download', data: { container: 'body' }
+      = download_raw_snippet_button(@snippet)
 
 = render 'projects/blob/content', blob: blob

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

@@ -17,7 +17,7 @@
 
     .file-actions.d-none.d-sm-block
       .btn-group{ role: "group" }<
-        = embedded_snippet_raw_button
+        = embedded_raw_snippet_button
 
         = embedded_snippet_download_button
   %article.file-holder.snippet-file-content

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

@@ -44,7 +44,7 @@
             %li
               %button.js-share-btn.btn.btn-transparent{ type: 'button' }
                 %strong.embed-toggle-list-item= _("Share")
-        %input.js-snippet-url-area.snippet-embed-input.form-control{ type: "text", autocomplete: 'off', value: snippet_embed }
+        %input.js-snippet-url-area.snippet-embed-input.form-control{ type: "text", autocomplete: 'off', value: snippet_embed_tag(@snippet) }
         .input-group-append
           = clipboard_button(title: _('Copy'), class: 'js-clipboard-btn snippet-clipboard-btn btn btn-default', target: '.js-snippet-url-area')
     .clearfix

spec/helpers/snippets_helper_spec.rb

@@ -3,33 +3,217 @@
 require 'spec_helper'
 
 describe SnippetsHelper do
+  include Gitlab::Routing
   include IconsHelper
 
-  describe '#embedded_snippet_raw_button' do
-    it 'gives view raw button of embedded snippets for project snippets' do
-      @snippet = create(:project_snippet, :public)
+  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) }
 
-      expect(embedded_snippet_raw_button.to_s).to eq("<a class=\"btn\" target=\"_blank\" rel=\"noopener noreferrer\" title=\"Open raw\" href=\"#{raw_project_snippet_url(@snippet.project, @snippet)}\">#{external_snippet_icon('doc-code')}</a>")
+    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 'gives view raw button of embedded snippets for personal snippets' do
+    it 'returns view raw button of embedded snippets for personal snippets' do
       @snippet = create(:personal_snippet, :public)
 
-      expect(embedded_snippet_raw_button.to_s).to eq("<a class=\"btn\" target=\"_blank\" rel=\"noopener noreferrer\" title=\"Open raw\" href=\"#{raw_snippet_url(@snippet)}\">#{external_snippet_icon('doc-code')}</a>")
+      expect(subject).to eq(download_link("http://test.host/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"))
+    end
+
+    def download_link(url)
+      "<a class=\"btn\" target=\"_blank\" rel=\"noopener noreferrer\" title=\"Open raw\" href=\"#{url}\">#{external_snippet_icon('doc-code')}</a>"
     end
   end
 
   describe '#embedded_snippet_download_button' do
-    it 'gives download button of embedded snippets for project snippets' do
+    subject { embedded_snippet_download_button }
+
+    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"))
+    end
+
+    it 'returns download button of embedded snippets for project snippets' do
       @snippet = create(:project_snippet, :public)
 
-      expect(embedded_snippet_download_button.to_s).to eq("<a class=\"btn\" target=\"_blank\" title=\"Download\" rel=\"noopener noreferrer\" href=\"#{raw_project_snippet_url(@snippet.project, @snippet, inline: false)}\">#{external_snippet_icon('download')}</a>")
+      expect(subject).to eq(download_link("http://test.host/#{@snippet.project.path_with_namespace}/snippets/#{@snippet.id}/raw"))
     end
 
-    it 'gives download button of embedded snippets for personal snippets' do
-      @snippet = create(:personal_snippet, :public)
+    def download_link(url)
+      "<a class=\"btn\" target=\"_blank\" title=\"Download\" rel=\"noopener noreferrer\" href=\"#{url}?inline=false\">#{external_snippet_icon('download')}</a>"
+    end
+  end
+
+  describe '#snippet_embed_tag' do
+    subject { snippet_embed_tag(snippet) }
+
+    context 'personal snippets' do
+      let(:snippet) { public_personal_snippet }
+
+      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}"))
+        end
+      end
+    end
+
+    context 'project snippets' 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}"))
+      end
+    end
+
+    def script_embed(url)
+      "<script src=\"#{url}.js\"></script>"
+    end
+  end
+
+  describe '#download_raw_snippet_button' do
+    subject { download_raw_snippet_button(snippet) }
 
-      expect(embedded_snippet_download_button.to_s).to eq("<a class=\"btn\" target=\"_blank\" title=\"Download\" rel=\"noopener noreferrer\" href=\"#{raw_snippet_url(@snippet, inline: false)}\">#{external_snippet_icon('download')}</a>")
+    context 'with personal snippet' do
+      let(:snippet) { public_personal_snippet }
+
+      it 'returns the download button' do
+        expect(subject).to eq(download_link("/snippets/#{snippet.id}/raw"))
+      end
+    end
+
+    context 'with project snippet' do
+      let(:snippet) { public_project_snippet }
+
+      it 'returns the download button' do
+        expect(subject).to eq(download_link("/#{snippet.project.path_with_namespace}/snippets/#{snippet.id}/raw"))
+      end
+    end
+
+    def download_link(url)
+      "<a target=\"_blank\" rel=\"noopener noreferrer\" class=\"btn btn-sm has-tooltip\" title=\"Download\" data-container=\"body\" href=\"#{url}?inline=false\"><i aria-hidden=\"true\" data-hidden=\"true\" class=\"fa fa-download\"></i></a>"
+    end
+  end
+
+  describe '#snippet_badge' do
+    let(:snippet) { build(:personal_snippet, visibility) }
+
+    subject { snippet_badge(snippet) }
+
+    context 'when snippet is private' do
+      let(:visibility) { :private }
+
+      it 'returns the snippet badge' do
+        expect(subject).to eq "<span class=\"badge badge-gray\"><i class=\"fa fa-lock\"></i> private</span>"
+      end
+    end
+
+    context 'when snippet is public' do
+      let(:visibility) { :public }
+
+      it 'does not return anything' do
+        expect(subject).to be_nil
+      end
+    end
+
+    context 'when snippet is internal' do
+      let(:visibility) { :internal }
+
+      it 'does not return anything' do
+        expect(subject).to be_nil
+      end
     end
   end
 end

spec/models/personal_snippet_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe PersonalSnippet do
+  describe '#embeddable?' do
+    [
+      { snippet: :public,   embeddable: true },
+      { snippet: :internal, embeddable: false },
+      { snippet: :private,  embeddable: false }
+    ].each do |combination|
+      it 'returns true when snippet is public' do
+        snippet = build(:personal_snippet, combination[:snippet])
+
+        expect(snippet.embeddable?).to eq(combination[:embeddable])
+      end
+    end
+  end
+end

spec/models/project_snippet_spec.rb

@@ -10,4 +10,25 @@ describe ProjectSnippet do
   describe "Validation" do
     it { is_expected.to validate_presence_of(:project) }
   end
+
+  describe '#embeddable?' do
+    [
+      { project: :public,   snippet: :public,   embeddable: true },
+      { project: :internal, snippet: :public,   embeddable: false },
+      { project: :private,  snippet: :public,   embeddable: false },
+      { project: :public,   snippet: :internal, embeddable: false },
+      { project: :internal, snippet: :internal, embeddable: false },
+      { project: :private,  snippet: :internal, embeddable: false },
+      { project: :public,   snippet: :private,  embeddable: false },
+      { project: :internal, snippet: :private,  embeddable: false },
+      { project: :private,  snippet: :private,  embeddable: false }
+    ].each do |combination|
+      it 'only returns true when both project and snippet are public' do
+        project = create(:project, combination[:project])
+        snippet = build(:project_snippet, combination[:snippet], project: project)
+
+        expect(snippet.embeddable?).to eq(combination[:embeddable])
+      end
+    end
+  end
 end

spec/models/snippet_spec.rb

@@ -451,41 +451,4 @@ describe Snippet do
       expect(blob.data).to eq(snippet.content)
     end
   end
-
-  describe '#embeddable?' do
-    context 'project snippet' do
-      [
-        { project: :public,   snippet: :public,   embeddable: true },
-        { project: :internal, snippet: :public,   embeddable: false },
-        { project: :private,  snippet: :public,   embeddable: false },
-        { project: :public,   snippet: :internal, embeddable: false },
-        { project: :internal, snippet: :internal, embeddable: false },
-        { project: :private,  snippet: :internal, embeddable: false },
-        { project: :public,   snippet: :private,  embeddable: false },
-        { project: :internal, snippet: :private,  embeddable: false },
-        { project: :private,  snippet: :private,  embeddable: false }
-      ].each do |combination|
-        it 'only returns true when both project and snippet are public' do
-          project = create(:project, combination[:project])
-          snippet = create(:project_snippet, combination[:snippet], project: project)
-
-          expect(snippet.embeddable?).to eq(combination[:embeddable])
-        end
-      end
-    end
-
-    context 'personal snippet' do
-      [
-        { snippet: :public,   embeddable: true },
-        { snippet: :internal, embeddable: false },
-        { snippet: :private,  embeddable: false }
-      ].each do |combination|
-        it 'only returns true when snippet is public' do
-          snippet = create(:personal_snippet, combination[:snippet])
-
-          expect(snippet.embeddable?).to eq(combination[:embeddable])
-        end
-      end
-    end
-  end
 end