Commit 2e311d9d authored by Jarka Kadlecova's avatar Jarka Kadlecova

Support uploads for newly created personal snippets

parent 4464c22d
......@@ -199,7 +199,7 @@ window.DropzoneInput = (function() {
};
addFileToForm = function(path) {
$(form).append('<input type="hidden" name="files[]" value="' + path + '">');
$(form).append('<input type="hidden" name="files[]" value="' + _.escape(path) + '">');
};
getFilename = function(e) {
......
......@@ -45,7 +45,7 @@ def create
@snippet = CreateSnippetService.new(nil, current_user, create_params).execute
move_temporary_files if params[:files]
move_temporary_files if @snippet.valid? && params[:files]
recaptcha_check_with_fallback { render :new }
end
......
......@@ -17,6 +17,8 @@ def find_model
end
def authorize_access!
return nil unless model
authorized =
case model
when Note
......@@ -35,7 +37,7 @@ def authorize_access!
end
def authorize_create_access!
return unless model
return nil unless model
# for now we support only personal snippets comments
authorized = can?(current_user, :comment_personal_snippet, model)
......@@ -77,7 +79,12 @@ def upload_mount
def uploader
return @uploader if defined?(@uploader)
if model.is_a?(PersonalSnippet)
case model
when nil
@uploader = PersonalFileUploader.new(nil, params[:secret])
@uploader.retrieve_from_store!(params[:filename])
when PersonalSnippet
@uploader = PersonalFileUploader.new(model, params[:secret])
@uploader.retrieve_from_store!(params[:filename])
......
class FileMover
attr_reader :secret, :file_name, :model
attr_reader :secret, :file_name, :model, :update_field
def initialize(file_path, model, update_field = :description)
@secret = File.split(File.dirname(file_path)).last
@file_name = File.basename(file_path)
@model = model
@update_field = update_field
end
def execute
move
update_markdown
uploader.record_upload if update_markdown
end
private
def move
FileUtils.mkdir_p(file_path)
FileUtils.mkdir_p(File.dirname(file_path))
FileUtils.move(temp_file_path, file_path)
end
def update_markdown(field = :description)
updated_text = model.send(field).sub(temp_file_uploader.to_markdown, uploader.to_markdown)
model.update_attribute(field, updated_text)
def update_markdown
updated_text = model.read_attribute(update_field).gsub(temp_file_uploader.to_markdown, uploader.to_markdown)
model.update_attribute(update_field, updated_text)
true
rescue
revert
false
end
def temp_file_path
return @temp_file_path if @temp_file_path
temp_file_uploader.retrieve_from_store!(file_name)
temp_file_uploader.file.path
@temp_file_path = temp_file_uploader.file.path
end
def file_path
......@@ -45,4 +54,10 @@ def uploader
def temp_file_uploader
@temp_file_uploader ||= PersonalFileUploader.new(nil, secret)
end
def revert
Rails.logger.warn("Markdown not updated, file move reverted for #{model}")
FileUtils.move(file_path, temp_file_path)
end
end
......@@ -6,8 +6,6 @@ module RecordsUploads
before :remove, :destroy_upload
end
private
# After storing an attachment, create a corresponding Upload record
#
# NOTE: We're ignoring the argument passed to this callback because we want
......@@ -15,13 +13,16 @@ module RecordsUploads
# `Tempfile` object the callback gets.
#
# Called `after :store`
def record_upload(_tempfile)
def record_upload(_tempfile = nil)
return unless model
return unless file_storage?
return unless file.exists?
Upload.record(self)
end
private
# Before removing an attachment, destroy any Upload records at the same path
#
# Called `before :remove`
......
......@@ -2,7 +2,7 @@
- model = local_assigns.fetch(:model)
- form = local_assigns.fetch(:form)
- supports_slash_commands = !model.persisted?
- supports_slash_commands = model.new_record?
- if supports_slash_commands
- preview_url = preview_markdown_path(project, slash_commands_target_type: model.class.name)
......
......@@ -22,10 +22,9 @@
- if @snippet.updated_at != @snippet.created_at
= edited_time_ago_with_tooltip(@snippet, placement: 'bottom', html_class: 'snippet-edited-ago', exclude_author: true)
%div
- if @snippet.description.present?
.description
.wiki
= markdown_field(@snippet, :description)
%textarea.hidden.js-task-list-field
= @snippet.description
- if @snippet.description.present?
.description
.wiki
= markdown_field(@snippet, :description)
%textarea.hidden.js-task-list-field
= @snippet.description
......@@ -9,6 +9,11 @@
to: 'uploads#show',
constraints: { model: /personal_snippet/, id: /\d+/, filename: /[^\/]+/ }
# show temporary uploads
get 'temp/:secret/:filename',
to: 'uploads#show',
constraints: { filename: /[^\/]+/ }
# Appearance
get ":model/:mounted_as/:id/:filename",
to: "uploads#show",
......
......@@ -73,7 +73,7 @@ Parameters:
- `file_name` (required) - The name of a snippet file
- `description` (optional) - The description of a snippet
- `code` (required) - The content of a snippet
- `visibility` (optional) - The snippet's visibility
- `visibility` (required) - The snippet's visibility
## Update snippet
......
......@@ -92,6 +92,40 @@
end
end
end
context 'temporal with valid image' do
subject do
post :create, model: 'personal_snippet', file: jpg, format: :json
end
it 'returns a content with original filename, new link, and correct type.' do
subject
expect(response.body).to match '\"alt\":\"rails_sample\"'
expect(response.body).to match "\"url\":\"/uploads/temp"
end
it 'does not create an Upload record' do
expect { subject }.not_to change { Upload.count }
end
end
context 'temporal with valid non-image file' do
subject do
post :create, model: 'personal_snippet', file: txt, format: :json
end
it 'returns a content with original filename, new link, and correct type.' do
subject
expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
expect(response.body).to match "\"url\":\"/uploads/temp"
end
it 'does not create an Upload record' do
expect { subject }.not_to change { Upload.count }
end
end
end
end
......
......@@ -27,7 +27,7 @@ def fill_form
it 'creates a new snippet' do
fill_form
click_button('Create snippet')
wait_for_ajax
wait_for_requests
expect(page).to have_content('My Snippet Title')
expect(page).to have_content('Hello World!')
......@@ -44,7 +44,7 @@ def fill_form
expect(page.find_field("project_snippet_description").value).to have_content('banana_sample')
click_button('Create snippet')
wait_for_ajax
wait_for_requests
link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
expect(link).to match(%r{/#{Regexp.escape(project.full_path) }/uploads/\h{32}/banana_sample\.gif\z})
......@@ -60,7 +60,7 @@ def fill_form
dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif')
click_button('Create snippet')
wait_for_ajax
wait_for_requests
expect(page).to have_content('My Snippet Title')
expect(page).to have_content('Hello World!')
......
......@@ -30,6 +30,22 @@ def fill_form
expect(page).to have_content('Hello World!')
end
scenario 'previews a snippet with file' do
fill_in 'personal_snippet_description', with: 'My Snippet'
dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif')
find('.js-md-preview-button').click
page.within('#new_personal_snippet .md-preview') do
expect(page).to have_content('My Snippet')
link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
expect(link).to match(%r{/uploads/temp/\h{32}/banana_sample\.gif\z})
visit(link)
expect(page.status_code).to eq(200)
end
end
scenario 'uploads a file when dragging into textarea' do
fill_form
......@@ -42,6 +58,9 @@ def fill_form
link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
expect(link).to match(%r{/uploads/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z})
visit(link)
expect(page.status_code).to eq(200)
end
scenario 'validation fails for the first time' do
......@@ -64,6 +83,9 @@ def fill_form
expect(page).to have_content('Hello World!')
link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
expect(link).to match(%r{/uploads/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z})
visit(link)
expect(page.status_code).to eq(200)
end
scenario 'Authenticated user creates a snippet with + in filename' do
......
......@@ -13,14 +13,14 @@
login_as(user)
visit edit_snippet_path(snippet)
wait_for_ajax
wait_for_requests
end
it 'updates the snippet' do
fill_in 'personal_snippet_title', with: 'New Snippet Title'
click_button('Save changes')
wait_for_ajax
wait_for_requests
expect(page).to have_content('New Snippet Title')
end
......@@ -30,7 +30,7 @@
expect(page.find_field("personal_snippet_description").value).to have_content('banana_sample')
click_button('Save changes')
wait_for_ajax
wait_for_requests
link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
expect(link).to match(%r{/uploads/personal_snippet/#{snippet.id}/\h{32}/banana_sample\.gif\z})
......
......@@ -3,7 +3,10 @@
describe FileMover do
let(:filename) { 'banana_sample.gif' }
let(:file) { fixture_file_upload(Rails.root.join('spec', 'fixtures', filename)) }
let(:temp_description) { 'test ![banana_sample](/uploads/temp/secret55/banana_sample.gif)' }
let(:temp_description) do
'test ![banana_sample](/uploads/temp/secret55/banana_sample.gif) same ![banana_sample]'\
'(/uploads/temp/secret55/banana_sample.gif)'
end
let(:temp_file_path) { File.join('secret55', filename).to_s }
let(:file_path) { File.join('uploads', 'personal_snippet', snippet.id.to_s, 'secret55', filename).to_s }
......@@ -12,14 +15,49 @@
subject { described_class.new(file_path, snippet).execute }
describe '#execute' do
it 'updates the description correctly' do
expect(FileUtils).to receive(:mkdir_p).with(a_string_including(file_path))
before do
expect(FileUtils).to receive(:mkdir_p).with(a_string_including(File.dirname(file_path)))
expect(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path))
allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true)
allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10)
end
context 'when move and field update successful' do
it 'updates the description correctly' do
subject
expect(snippet.reload.description)
.to eq(
"test ![banana_sample](/uploads/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"\
" same ![banana_sample](/uploads/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"
)
end
it 'creates a new update record' do
expect { subject }.to change { Upload.count }.by(1)
end
end
context 'when update_markdown fails' do
before do
expect(FileUtils).to receive(:move).with(a_string_including(file_path), a_string_including(temp_file_path))
end
subject { described_class.new(file_path, snippet, :non_existing_field).execute }
it 'does not update the description' do
subject
subject
expect(snippet.reload.description)
.to eq(
"test ![banana_sample](/uploads/temp/secret55/banana_sample.gif)"\
" same ![banana_sample](/uploads/temp/secret55/banana_sample.gif)"
)
end
expect(snippet.reload.description)
.to eq("test ![banana_sample](/uploads/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)")
it 'does not create a new update record' do
expect { subject }.not_to change { Upload.count }
end
end
end
end
require 'rails_helper'
describe RecordsUploads do
let(:uploader) do
let!(:uploader) do
class RecordsUploadsExampleUploader < GitlabUploader
include RecordsUploads
......@@ -57,6 +57,13 @@ def upload_fixture(filename)
uploader.store!(upload_fixture('rails_sample.jpg'))
end
it 'does not create an Upload record if model is missing' do
expect_any_instance_of(RecordsUploadsExampleUploader).to receive(:model).and_return(nil)
expect(Upload).not_to receive(:record).with(uploader)
uploader.store!(upload_fixture('rails_sample.jpg'))
end
it 'it destroys Upload records at the same path before recording' do
existing = Upload.create!(
path: File.join('uploads', 'rails_sample.jpg'),
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment