Merge branch 'rs-uploaders' into 'master'
Minor refactoring of Uploaders See merge request !9490
This commit is contained in:
commit
bbba5eb06a
|
@ -27,10 +27,6 @@ class ArtifactUploader < GitlabUploader
|
|||
File.join(self.class.artifacts_cache_path, @build.artifacts_path)
|
||||
end
|
||||
|
||||
def file_storage?
|
||||
self.class.storage == CarrierWave::Storage::File
|
||||
end
|
||||
|
||||
def filename
|
||||
file.try(:filename)
|
||||
end
|
||||
|
|
|
@ -4,6 +4,6 @@ class AttachmentUploader < GitlabUploader
|
|||
storage :file
|
||||
|
||||
def store_dir
|
||||
"uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
|
||||
"#{base_dir}/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
|
||||
end
|
||||
end
|
||||
|
|
|
@ -4,7 +4,7 @@ class AvatarUploader < GitlabUploader
|
|||
storage :file
|
||||
|
||||
def store_dir
|
||||
"uploads/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
|
||||
"#{base_dir}/#{model.class.to_s.underscore}/#{mounted_as}/#{model.id}"
|
||||
end
|
||||
|
||||
def exists?
|
||||
|
|
|
@ -4,15 +4,12 @@ class FileUploader < GitlabUploader
|
|||
|
||||
storage :file
|
||||
|
||||
attr_accessor :project, :secret
|
||||
attr_accessor :project
|
||||
attr_reader :secret
|
||||
|
||||
def initialize(project, secret = nil)
|
||||
@project = project
|
||||
@secret = secret || self.class.generate_secret
|
||||
end
|
||||
|
||||
def base_dir
|
||||
"uploads"
|
||||
@secret = secret || generate_secret
|
||||
end
|
||||
|
||||
def store_dir
|
||||
|
@ -23,10 +20,6 @@ class FileUploader < GitlabUploader
|
|||
File.join(base_dir, 'tmp', @project.path_with_namespace, @secret)
|
||||
end
|
||||
|
||||
def secure_url
|
||||
File.join("/uploads", @secret, file.filename)
|
||||
end
|
||||
|
||||
def to_markdown
|
||||
to_h[:markdown]
|
||||
end
|
||||
|
@ -35,17 +28,23 @@ class FileUploader < GitlabUploader
|
|||
filename = image_or_video? ? self.file.basename : self.file.filename
|
||||
escaped_filename = filename.gsub("]", "\\]")
|
||||
|
||||
markdown = "[#{escaped_filename}](#{self.secure_url})"
|
||||
markdown = "[#{escaped_filename}](#{secure_url})"
|
||||
markdown.prepend("!") if image_or_video? || dangerous?
|
||||
|
||||
{
|
||||
alt: filename,
|
||||
url: self.secure_url,
|
||||
url: secure_url,
|
||||
markdown: markdown
|
||||
}
|
||||
end
|
||||
|
||||
def self.generate_secret
|
||||
private
|
||||
|
||||
def generate_secret
|
||||
SecureRandom.hex
|
||||
end
|
||||
|
||||
def secure_url
|
||||
File.join('/uploads', @secret, file.filename)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,4 +1,14 @@
|
|||
class GitlabUploader < CarrierWave::Uploader::Base
|
||||
def self.base_dir
|
||||
'uploads'
|
||||
end
|
||||
|
||||
delegate :base_dir, to: :class
|
||||
|
||||
def file_storage?
|
||||
self.class.storage == CarrierWave::Storage::File
|
||||
end
|
||||
|
||||
# Reduce disk IO
|
||||
def move_to_cache
|
||||
true
|
||||
|
|
|
@ -27,6 +27,8 @@ module UploaderHelper
|
|||
extension_match?(DANGEROUS_EXT)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def extension_match?(extensions)
|
||||
return false unless file
|
||||
|
||||
|
@ -40,8 +42,4 @@ module UploaderHelper
|
|||
|
||||
extensions.include?(extension.downcase)
|
||||
end
|
||||
|
||||
def file_storage?
|
||||
self.class.storage == CarrierWave::Storage::File
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe 'Issues', feature: true do
|
||||
include DropzoneHelper
|
||||
include IssueHelpers
|
||||
include SortingHelper
|
||||
include WaitForAjax
|
||||
|
@ -570,19 +571,13 @@ describe 'Issues', feature: true do
|
|||
end
|
||||
|
||||
it 'uploads file when dragging into textarea' do
|
||||
drop_in_dropzone test_image_file
|
||||
|
||||
# Wait for the file to upload
|
||||
sleep 1
|
||||
dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif')
|
||||
|
||||
expect(page.find_field("issue_description").value).to have_content 'banana_sample'
|
||||
end
|
||||
|
||||
it 'adds double newline to end of attachment markdown' do
|
||||
drop_in_dropzone test_image_file
|
||||
|
||||
# Wait for the file to upload
|
||||
sleep 1
|
||||
dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif')
|
||||
|
||||
expect(page.find_field("issue_description").value).to match /\n\n$/
|
||||
end
|
||||
|
@ -665,25 +660,4 @@ describe 'Issues', feature: true do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
def drop_in_dropzone(file_path)
|
||||
# Generate a fake input selector
|
||||
page.execute_script <<-JS
|
||||
var fakeFileInput = window.$('<input/>').attr(
|
||||
{id: 'fakeFileInput', type: 'file'}
|
||||
).appendTo('body');
|
||||
JS
|
||||
# Attach the file to the fake input selector with Capybara
|
||||
attach_file("fakeFileInput", file_path)
|
||||
# Add the file to a fileList array and trigger the fake drop event
|
||||
page.execute_script <<-JS
|
||||
var fileList = [$('#fakeFileInput')[0].files[0]];
|
||||
var e = jQuery.Event('drop', { dataTransfer : { files : fileList } });
|
||||
$('.div-dropzone')[0].dropzone.listeners[0].events.drop(e);
|
||||
JS
|
||||
end
|
||||
|
||||
def test_image_file
|
||||
File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,26 @@
|
|||
require 'rails_helper'
|
||||
|
||||
feature 'User uploads avatar to group', feature: true do
|
||||
scenario 'they see the new avatar' do
|
||||
user = create(:user)
|
||||
group = create(:group)
|
||||
group.add_owner(user)
|
||||
login_as(user)
|
||||
|
||||
visit edit_group_path(group)
|
||||
attach_file(
|
||||
'group_avatar',
|
||||
Rails.root.join('spec', 'fixtures', 'dk.png'),
|
||||
visible: false
|
||||
)
|
||||
|
||||
click_button 'Save group'
|
||||
|
||||
visit group_path(group)
|
||||
|
||||
expect(page).to have_selector(%Q(img[src$="/uploads/group/avatar/#{group.id}/dk.png"]))
|
||||
|
||||
# Cheating here to verify something that isn't user-facing, but is important
|
||||
expect(group.reload.avatar.file).to exist
|
||||
end
|
||||
end
|
|
@ -0,0 +1,24 @@
|
|||
require 'rails_helper'
|
||||
|
||||
feature 'User uploads avatar to profile', feature: true do
|
||||
scenario 'they see their new avatar' do
|
||||
user = create(:user)
|
||||
login_as(user)
|
||||
|
||||
visit profile_path
|
||||
attach_file(
|
||||
'user_avatar',
|
||||
Rails.root.join('spec', 'fixtures', 'dk.png'),
|
||||
visible: false
|
||||
)
|
||||
|
||||
click_button 'Update profile settings'
|
||||
|
||||
visit user_path(user)
|
||||
|
||||
expect(page).to have_selector(%Q(img[src$="/uploads/user/avatar/#{user.id}/dk.png"]))
|
||||
|
||||
# Cheating here to verify something that isn't user-facing, but is important
|
||||
expect(user.reload.avatar.file).to exist
|
||||
end
|
||||
end
|
|
@ -0,0 +1,22 @@
|
|||
require 'rails_helper'
|
||||
|
||||
feature 'User uploads file to note', feature: true do
|
||||
include DropzoneHelper
|
||||
|
||||
let(:user) { create(:user) }
|
||||
let(:project) { create(:empty_project, creator: user, namespace: user.namespace) }
|
||||
|
||||
scenario 'they see the attached file', js: true do
|
||||
issue = create(:issue, project: project, author: user)
|
||||
|
||||
login_as(user)
|
||||
visit namespace_project_issue_path(project.namespace, project, issue)
|
||||
|
||||
dropzone_file(Rails.root.join('spec', 'fixtures', 'dk.png'))
|
||||
click_button 'Comment'
|
||||
wait_for_ajax
|
||||
|
||||
expect(find('a.no-attachment-icon img[alt="dk"]')['src'])
|
||||
.to match(%r{/#{project.full_path}/uploads/\h{32}/dk\.png$})
|
||||
end
|
||||
end
|
|
@ -0,0 +1,37 @@
|
|||
module DropzoneHelper
|
||||
# Provides a way to perform `attach_file` for a Dropzone-based file input
|
||||
#
|
||||
# This is accomplished by creating a standard HTML file input on the page,
|
||||
# performing `attach_file` on that field, and then triggering the appropriate
|
||||
# Dropzone events to perform the actual upload.
|
||||
#
|
||||
# This method waits for the upload to complete before returning.
|
||||
def dropzone_file(file_path)
|
||||
# Generate a fake file input that Capybara can attach to
|
||||
page.execute_script <<-JS.strip_heredoc
|
||||
var fakeFileInput = window.$('<input/>').attr(
|
||||
{id: 'fakeFileInput', type: 'file'}
|
||||
).appendTo('body');
|
||||
|
||||
window._dropzoneComplete = false;
|
||||
JS
|
||||
|
||||
# Attach the file to the fake input selector with Capybara
|
||||
attach_file('fakeFileInput', file_path)
|
||||
|
||||
# Manually trigger a Dropzone "drop" event with the fake input's file list
|
||||
page.execute_script <<-JS.strip_heredoc
|
||||
var fileList = [$('#fakeFileInput')[0].files[0]];
|
||||
var e = jQuery.Event('drop', { dataTransfer : { files : fileList } });
|
||||
|
||||
var dropzone = $('.div-dropzone')[0].dropzone;
|
||||
dropzone.on('queuecomplete', function() {
|
||||
window._dropzoneComplete = true;
|
||||
});
|
||||
dropzone.listeners[0].events.drop(e);
|
||||
JS
|
||||
|
||||
# Wait until Dropzone's fired `queuecomplete`
|
||||
loop until page.evaluate_script('window._dropzoneComplete === true')
|
||||
end
|
||||
end
|
|
@ -1,18 +1,17 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe AttachmentUploader do
|
||||
let(:issue) { build(:issue) }
|
||||
subject { described_class.new(issue) }
|
||||
let(:uploader) { described_class.new(build_stubbed(:user)) }
|
||||
|
||||
describe '#move_to_cache' do
|
||||
it 'is true' do
|
||||
expect(subject.move_to_cache).to eq(true)
|
||||
expect(uploader.move_to_cache).to eq(true)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#move_to_store' do
|
||||
it 'is true' do
|
||||
expect(subject.move_to_store).to eq(true)
|
||||
expect(uploader.move_to_store).to eq(true)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,18 +1,17 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe AvatarUploader do
|
||||
let(:user) { build(:user) }
|
||||
subject { described_class.new(user) }
|
||||
let(:uploader) { described_class.new(build_stubbed(:user)) }
|
||||
|
||||
describe '#move_to_cache' do
|
||||
it 'is false' do
|
||||
expect(subject.move_to_cache).to eq(false)
|
||||
expect(uploader.move_to_cache).to eq(false)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#move_to_store' do
|
||||
it 'is false' do
|
||||
expect(subject.move_to_store).to eq(false)
|
||||
expect(uploader.move_to_store).to eq(false)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,57 +1,35 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe FileUploader do
|
||||
let(:project) { create(:project) }
|
||||
let(:uploader) { described_class.new(build_stubbed(:project)) }
|
||||
|
||||
before do
|
||||
@previous_enable_processing = FileUploader.enable_processing
|
||||
FileUploader.enable_processing = false
|
||||
@uploader = FileUploader.new(project)
|
||||
end
|
||||
describe 'initialize' do
|
||||
it 'generates a secret if none is provided' do
|
||||
expect(SecureRandom).to receive(:hex).and_return('secret')
|
||||
|
||||
after do
|
||||
FileUploader.enable_processing = @previous_enable_processing
|
||||
@uploader.remove!
|
||||
end
|
||||
uploader = described_class.new(double)
|
||||
|
||||
describe '#image_or_video?' do
|
||||
context 'given an image file' do
|
||||
before do
|
||||
@uploader.store!(fixture_file_upload(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
|
||||
expect(uploader.secret).to eq 'secret'
|
||||
end
|
||||
|
||||
context 'given an video file' do
|
||||
before do
|
||||
video_file = fixture_file_upload(Rails.root.join('spec', 'fixtures', 'video_sample.mp4'))
|
||||
@uploader.store!(video_file)
|
||||
end
|
||||
it 'accepts a secret parameter' do
|
||||
expect(SecureRandom).not_to receive(:hex)
|
||||
|
||||
it 'detects a video based on file extension' do
|
||||
expect(@uploader.image_or_video?).to be true
|
||||
end
|
||||
end
|
||||
uploader = described_class.new(double, 'secret')
|
||||
|
||||
it 'does not return image_or_video? for other types' do
|
||||
@uploader.store!(fixture_file_upload(Rails.root.join('spec', 'fixtures', 'doc_sample.txt')))
|
||||
|
||||
expect(@uploader.image_or_video?).to be false
|
||||
expect(uploader.secret).to eq 'secret'
|
||||
end
|
||||
end
|
||||
|
||||
describe '#move_to_cache' do
|
||||
it 'is true' do
|
||||
expect(@uploader.move_to_cache).to eq(true)
|
||||
expect(uploader.move_to_cache).to eq(true)
|
||||
end
|
||||
end
|
||||
|
||||
describe '#move_to_store' do
|
||||
it 'is true' do
|
||||
expect(@uploader.move_to_store).to eq(true)
|
||||
expect(uploader.move_to_store).to eq(true)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,35 @@
|
|||
require 'rails_helper'
|
||||
|
||||
describe UploaderHelper do
|
||||
class ExampleUploader < CarrierWave::Uploader::Base
|
||||
include UploaderHelper
|
||||
|
||||
storage :file
|
||||
end
|
||||
|
||||
def upload_fixture(filename)
|
||||
fixture_file_upload(Rails.root.join('spec', 'fixtures', filename))
|
||||
end
|
||||
|
||||
describe '#image_or_video?' do
|
||||
let(:uploader) { ExampleUploader.new }
|
||||
|
||||
it 'returns true for an image file' do
|
||||
uploader.store!(upload_fixture('dk.png'))
|
||||
|
||||
expect(uploader).to be_image_or_video
|
||||
end
|
||||
|
||||
it 'it returns true for a video file' do
|
||||
uploader.store!(upload_fixture('video_sample.mp4'))
|
||||
|
||||
expect(uploader).to be_image_or_video
|
||||
end
|
||||
|
||||
it 'returns false for other extensions' do
|
||||
uploader.store!(upload_fixture('doc_sample.txt'))
|
||||
|
||||
expect(uploader).not_to be_image_or_video
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue