Merge branch '4142-show-inline-video' into 'master'

Add support for inline videos in issue, MR and notes (on issue, commit, MR, and MR diff)

## What does this MR do?

It adds support for inline videos in issue, MR and notes (on issue, commit, MR, and MR diff). Most of the work was done by @hayesr in !3508 but a few improvements were still missing.

## Why was this MR needed?

To be able to play uploaded videos in GitLab!

## What are the relevant issue numbers?

Closes #4142.

## Screenshots

### Video players

![Screen_Shot_2016-07-19_at_18.44.09](/uploads/e85e531b455a41c3e66b26b356abaafd/Screen_Shot_2016-07-19_at_18.44.09.png)

-----

![Screen_Shot_2016-07-19_at_18.44.29](/uploads/05f52a812760210d1eae86a7f8fc48bc/Screen_Shot_2016-07-19_at_18.44.29.png)

-----

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- Tests
  - [x] Test `VideoLinkFilter`
  - [x] Test in `spec/features/markdown_spec.rb`
  - [x] Improve `spec/uploaders/file_uploader_spec.rb`
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !5215

12 files changed, 128 insertions, 22 deletions

diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb
index 0893ee89f6a..71d0e4be834 100644
--- a/spec/controllers/projects/uploads_controller_spec.rb
+++ b/spec/controllers/projects/uploads_controller_spec.rb
@@ -14,9 +14,9 @@ describe Projects::UploadsController do
 
     context "without params['file']" do
       it "returns an error" do
-        post :create, 
+        post :create,
           namespace_id: project.namespace.to_param,
-          project_id: project.to_param, 
+          project_id: project.to_param,
           format: :json
         expect(response).to have_http_status(422)
       end
@@ -34,23 +34,21 @@ describe Projects::UploadsController do
       it 'returns a content with original filename, new link, and correct type.' do
         expect(response.body).to match '\"alt\":\"rails_sample\"'
         expect(response.body).to match "\"url\":\"/uploads"
-        expect(response.body).to match '\"is_image\":true'
       end
     end
 
     context 'with valid non-image file' do
       before do
-        post :create, 
+        post :create,
           namespace_id: project.namespace.to_param,
-          project_id: project.to_param, 
-          file: txt, 
+          project_id: project.to_param,
+          file: txt,
           format: :json
       end
 
       it 'returns a content with original filename, new link, and correct type.' do
         expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
         expect(response.body).to match "\"url\":\"/uploads"
-        expect(response.body).to match '\"is_image\":false'
       end
     end
   end
diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb
index 09ccc77c101..32159559c37 100644
--- a/spec/features/markdown_spec.rb
+++ b/spec/features/markdown_spec.rb
@@ -236,6 +236,14 @@ describe 'GitLab Markdown', feature: true do
     it 'includes TaskListFilter' do
       expect(doc).to parse_task_lists
     end
+
+    it 'includes InlineDiffFilter' do
+      expect(doc).to parse_inline_diffs
+    end
+
+    it 'includes VideoLinkFilter' do
+      expect(doc).to parse_video_links
+    end
   end
 
   context 'wiki pipeline' do
@@ -293,6 +301,10 @@ describe 'GitLab Markdown', feature: true do
     it 'includes InlineDiffFilter' do
       expect(doc).to parse_inline_diffs
     end
+
+    it 'includes VideoLinkFilter' do
+      expect(doc).to parse_video_links
+    end
   end
 
   # Fake a `current_user` helper
diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb
index c75d28d9801..f3e7c2d1a9f 100644
--- a/spec/fixtures/markdown.md.erb
+++ b/spec/fixtures/markdown.md.erb
@@ -256,3 +256,7 @@ However the wrapping tags can not be mixed as such -
 - [+ additions +}
 - {- delletions -]
 - [- delletions -}
+
+### Videos
+
+![My Video](/assets/videos/gitlab-demo.mp4)
diff --git a/spec/fixtures/video_sample.mp4 b/spec/fixtures/video_sample.mp4
new file mode 100644
index 00000000000..acd45190998
--- /dev/null
+++ b/spec/fixtures/video_sample.mp4
diff --git a/spec/lib/banzai/filter/video_link_filter_spec.rb b/spec/lib/banzai/filter/video_link_filter_spec.rb
new file mode 100644
index 00000000000..cc4349f80ba
--- /dev/null
+++ b/spec/lib/banzai/filter/video_link_filter_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe Banzai::Filter::VideoLinkFilter, lib: true do
+  def filter(doc, contexts = {})
+    contexts.reverse_merge!({
+      project: project
+    })
+
+    described_class.call(doc, contexts)
+  end
+
+  def link_to_image(path)
+    %(<img src="#{path}" />)
+  end
+
+  let(:project) { create(:project) }
+
+  context 'when the element src has a video extension' do
+    UploaderHelper::VIDEO_EXT.each do |ext|
+      it "replaces the image tag 'path/video.#{ext}' with a video tag" do
+        container = filter(link_to_image("/path/video.#{ext}")).children.first
+
+        expect(container.name).to eq 'div'
+        expect(container['class']).to eq 'video-container'
+
+        video, paragraph = container.children
+
+        expect(video.name).to eq 'video'
+        expect(video['src']).to eq "/path/video.#{ext}"
+
+        expect(paragraph.name).to eq 'p'
+
+        link = paragraph.children.first
+
+        expect(link.name).to eq 'a'
+        expect(link['href']).to eq "/path/video.#{ext}"
+        expect(link['target']).to eq '_blank'
+      end
+    end
+  end
+
+  context 'when the element src is an image' do
+    it 'leaves the document unchanged' do
+      element = filter(link_to_image('/path/my_image.jpg')).children.first
+
+      expect(element.name).to eq 'img'
+      expect(element['src']).to eq '/path/my_image.jpg'
+    end
+  end
+
+end
diff --git a/spec/lib/gitlab/email/attachment_uploader_spec.rb b/spec/lib/gitlab/email/attachment_uploader_spec.rb
index 476a21bf996..08b2577ecc4 100644
--- a/spec/lib/gitlab/email/attachment_uploader_spec.rb
+++ b/spec/lib/gitlab/email/attachment_uploader_spec.rb
@@ -11,7 +11,6 @@ describe Gitlab::Email::AttachmentUploader, lib: true do
       link = links.first
 
       expect(link).not_to be_nil
-      expect(link[:is_image]).to be_truthy
       expect(link[:alt]).to eq("bricks")
       expect(link[:url]).to include("bricks.png")
     end
diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb
index 36267faeb93..84d2584a791 100644
--- a/spec/lib/gitlab/email/receiver_spec.rb
+++ b/spec/lib/gitlab/email/receiver_spec.rb
@@ -115,7 +115,6 @@ describe Gitlab::Email::Receiver, lib: true do
         [
           {
             url: "uploads/image.png",
-            is_image: true,
             alt: "image",
             markdown: markdown
           }
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 152cd802839..1e049e9e99f 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -396,7 +396,6 @@ describe API::API, api: true  do
       expect(json_response['alt']).to eq("dk")
       expect(json_response['url']).to start_with("/uploads/")
       expect(json_response['url']).to end_with("/dk.png")
-      expect(json_response['is_image']).to eq(true)
     end
   end
 
diff --git a/spec/services/projects/download_service_spec.rb b/spec/services/projects/download_service_spec.rb
index f252e2c5902..122a7cea2a1 100644
--- a/spec/services/projects/download_service_spec.rb
+++ b/spec/services/projects/download_service_spec.rb
@@ -35,8 +35,6 @@ describe Projects::DownloadService, services: true do
 
         it { expect(@link_to_file).to have_key(:alt) }
         it { expect(@link_to_file).to have_key(:url) }
-        it { expect(@link_to_file).to have_key(:is_image) }
-        it { expect(@link_to_file[:is_image]).to be true }
         it { expect(@link_to_file[:url]).to match('rails_sample.jpg') }
         it { expect(@link_to_file[:alt]).to eq('rails_sample') }
       end
@@ -49,8 +47,6 @@ describe Projects::DownloadService, services: true do
 
         it { expect(@link_to_file).to have_key(:alt) }
         it { expect(@link_to_file).to have_key(:url) }
-        it { expect(@link_to_file).to have_key(:is_image) }
-        it { expect(@link_to_file[:is_image]).to be false }
         it { expect(@link_to_file[:url]).to match('doc_sample.txt') }
         it { expect(@link_to_file[:alt]).to eq('doc_sample.txt') }
       end
diff --git a/spec/services/projects/upload_service_spec.rb b/spec/services/projects/upload_service_spec.rb
index 9268a9fb1a2..c42eeba4b9c 100644
--- a/spec/services/projects/upload_service_spec.rb
+++ b/spec/services/projects/upload_service_spec.rb
@@ -15,9 +15,7 @@ describe Projects::UploadService, services: true do
 
       it { expect(@link_to_file).to have_key(:alt) }
       it { expect(@link_to_file).to have_key(:url) }
-      it { expect(@link_to_file).to have_key(:is_image) }
       it { expect(@link_to_file).to have_value('banana_sample') }
-      it { expect(@link_to_file[:is_image]).to equal(true) }
       it { expect(@link_to_file[:url]).to match('banana_sample.gif') }
     end
 
@@ -31,8 +29,6 @@ describe Projects::UploadService, services: true do
       it { expect(@link_to_file).to have_key(:alt) }
       it { expect(@link_to_file).to have_key(:url) }
       it { expect(@link_to_file).to have_value('dk') }
-      it { expect(@link_to_file).to have_key(:is_image) }
-      it { expect(@link_to_file[:is_image]).to equal(true) }
       it { expect(@link_to_file[:url]).to match('dk.png') }
     end
 
@@ -44,9 +40,7 @@ describe Projects::UploadService, services: true do
 
       it { expect(@link_to_file).to have_key(:alt) }
       it { expect(@link_to_file).to have_key(:url) }
-      it { expect(@link_to_file).to have_key(:is_image) }
       it { expect(@link_to_file).to have_value('rails_sample') }
-      it { expect(@link_to_file[:is_image]).to equal(true) }
       it { expect(@link_to_file[:url]).to match('rails_sample.jpg') }
     end
 
@@ -58,9 +52,7 @@ describe Projects::UploadService, services: true do
 
       it { expect(@link_to_file).to have_key(:alt) }
       it { expect(@link_to_file).to have_key(:url) }
-      it { expect(@link_to_file).to have_key(:is_image) }
       it { expect(@link_to_file).to have_value('doc_sample.txt') }
-      it { expect(@link_to_file[:is_image]).to equal(false) }
       it { expect(@link_to_file[:url]).to match('doc_sample.txt') }
     end
 
diff --git a/spec/support/matchers/markdown_matchers.rb b/spec/support/matchers/markdown_matchers.rb
index e005058ba5b..8c98b1f988c 100644
--- a/spec/support/matchers/markdown_matchers.rb
+++ b/spec/support/matchers/markdown_matchers.rb
@@ -178,6 +178,17 @@ module MarkdownMatchers
       expect(actual).to have_selector('span.idiff.deletion', count: 2)
     end
   end
+
+  # VideoLinkFilter
+  matcher :parse_video_links do
+    set_default_markdown_messages
+
+    match do |actual|
+      video = actual.at_css('video')
+
+      expect(video['src']).to end_with('/assets/videos/gitlab-demo.mp4')
+    end
+  end
 end
 
 # Monkeypatch the matcher DSL so that we can reduce some noisy duplication for
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
new file mode 100644
index 00000000000..e8300abed5d
--- /dev/null
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -0,0 +1,45 @@
+require 'spec_helper'
+
+describe FileUploader do
+  let(:project) { create(:project) }
+
+  before do
+    @previous_enable_processing = FileUploader.enable_processing
+    FileUploader.enable_processing = false
+    @uploader = FileUploader.new(project)
+  end
+
+  after do
+    FileUploader.enable_processing = @previous_enable_processing
+    @uploader.remove!
+  end
+
+  describe '#image_or_video?' do
+    context 'given an image file' do
+      before do
+        @uploader.store!(File.new(Rails.root.join('spec', 'fixtures', 'rails_sample.jpg')))
+      end
+
+      it 'detects an image based on file extension' do
+        expect(@uploader.image_or_video?).to be true
+      end
+    end
+
+    context 'given an video file' do
+      before do
+        video_file = File.new(Rails.root.join('spec', 'fixtures', 'video_sample.mp4'))
+        @uploader.store!(video_file)
+      end
+
+      it 'detects a video based on file extension' do
+        expect(@uploader.image_or_video?).to be true
+      end
+    end
+
+    it 'does not return image_or_video? for other types' do
+      @uploader.store!(File.new(Rails.root.join('spec', 'fixtures', 'doc_sample.txt')))
+
+      expect(@uploader.image_or_video?).to be false
+    end
+  end
+end