Refactor file upload in Personal Snippets

parent aa13a6c9
......@@ -49,23 +49,19 @@ class SnippetsController < ApplicationController
end
def create
create_params = snippet_params.merge(spammable_params)
create_params = snippet_params.merge(files: params.delete(:files))
service_response = Snippets::CreateService.new(nil, current_user, create_params).execute
@snippet = service_response.payload[:snippet]
if service_response.error? && @snippet.errors[:repository].present?
handle_repository_error(:new)
else
move_temporary_files if @snippet.valid? && params[:files]
recaptcha_check_with_fallback { render :new }
end
end
def update
update_params = snippet_params.merge(spammable_params)
service_response = Snippets::UpdateService.new(nil, current_user, update_params).execute(@snippet)
service_response = Snippets::UpdateService.new(nil, current_user, snippet_params).execute(@snippet)
@snippet = service_response.payload[:snippet]
handle_repository_error(:edit)
......@@ -150,12 +146,6 @@ class SnippetsController < ApplicationController
end
def snippet_params
params.require(:personal_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description)
end
def move_temporary_files
params[:files].each do |file|
FileMover.new(file, from_model: current_user, to_model: @snippet).execute
end
params.require(:personal_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description).merge(spammable_params)
end
end
......@@ -9,6 +9,8 @@ module Snippets
def execute
filter_spam_check_params
@files = Array(params.delete(:files).presence)
@snippet = if project
project.snippets.build(params)
else
......@@ -29,6 +31,8 @@ module Snippets
UserAgentDetailService.new(@snippet, @request).create
Gitlab::UsageDataCounters::SnippetCounter.count(:create)
move_temporary_files
ServiceResponse.success(payload: { snippet: @snippet } )
else
snippet_error_response(@snippet, 400)
......@@ -83,5 +87,13 @@ module Snippets
def snippet_files
[{ file_path: params[:file_name], content: params[:content] }]
end
def move_temporary_files
return unless @snippet.is_a?(PersonalSnippet)
@files.each do |file|
FileMover.new(file, from_model: current_user, to_model: @snippet).execute
end
end
end
end
......@@ -243,39 +243,13 @@ describe SnippetsController do
end
end
context 'when the snippet description contains a file' do
include FileMoverHelpers
context 'when the controller receives the files param' do
let(:files) { %w(foo bar) }
let(:picture_secret) { SecureRandom.hex }
let(:text_secret) { SecureRandom.hex }
let(:picture_file) { "/-/system/user/#{user.id}/#{picture_secret}/picture.jpg" }
let(:text_file) { "/-/system/user/#{user.id}/#{text_secret}/text.txt" }
let(:description) do
"Description with picture: ![picture](/uploads#{picture_file}) and "\
"text: [text.txt](/uploads#{text_file})"
end
before do
allow(FileUtils).to receive(:mkdir_p)
allow(FileUtils).to receive(:move)
stub_file_mover(text_file)
stub_file_mover(picture_file)
end
subject { create_snippet({ description: description }, { files: [picture_file, text_file] }) }
it 'creates the snippet' do
expect { subject }.to change { Snippet.count }.by(1)
end
it 'stores the snippet description correctly' do
snippet = subject
expected_description = "Description with picture: "\
"![picture](/uploads/-/system/personal_snippet/#{snippet.id}/#{picture_secret}/picture.jpg) and "\
"text: [text.txt](/uploads/-/system/personal_snippet/#{snippet.id}/#{text_secret}/text.txt)"
it 'passes the files param to the snippet create service' do
expect(Snippets::CreateService).to receive(:new).with(nil, user, hash_including(files: files)).and_call_original
expect(snippet.description).to eq(expected_description)
create_snippet({ title: nil }, { files: files })
end
end
......
......@@ -285,6 +285,20 @@ describe Snippets::CreateService do
it_behaves_like 'an error service response when save fails'
it_behaves_like 'creates repository and files'
it_behaves_like 'after_save callback to store_mentions', ProjectSnippet
context 'when uploaded files are passed to the service' do
let(:extra_opts) { { files: ['foo'] } }
it 'does not move uploaded files to the snippet' do
expect_next_instance_of(described_class) do |instance|
expect(instance).to receive(:move_temporary_files).and_call_original
end
expect_any_instance_of(FileMover).not_to receive(:execute)
subject
end
end
end
context 'when PersonalSnippet' do
......@@ -297,6 +311,51 @@ describe Snippets::CreateService do
it_behaves_like 'an error service response when save fails'
it_behaves_like 'creates repository and files'
it_behaves_like 'after_save callback to store_mentions', PersonalSnippet
context 'when the snippet description contains files' do
include FileMoverHelpers
let(:title) { 'Title' }
let(:picture_secret) { SecureRandom.hex }
let(:text_secret) { SecureRandom.hex }
let(:picture_file) { "/-/system/user/#{creator.id}/#{picture_secret}/picture.jpg" }
let(:text_file) { "/-/system/user/#{creator.id}/#{text_secret}/text.txt" }
let(:files) { [picture_file, text_file] }
let(:description) do
"Description with picture: ![picture](/uploads#{picture_file}) and "\
"text: [text.txt](/uploads#{text_file})"
end
before do
allow(FileUtils).to receive(:mkdir_p)
allow(FileUtils).to receive(:move)
end
let(:extra_opts) { { description: description, title: title, files: files } }
it 'stores the snippet description correctly' do
stub_file_mover(text_file)
stub_file_mover(picture_file)
snippet = subject.payload[:snippet]
expected_description = "Description with picture: "\
"![picture](/uploads/-/system/personal_snippet/#{snippet.id}/#{picture_secret}/picture.jpg) and "\
"text: [text.txt](/uploads/-/system/personal_snippet/#{snippet.id}/#{text_secret}/text.txt)"
expect(snippet.description).to eq(expected_description)
end
context 'when there is a validation error' do
let(:title) { nil }
it 'does not move uploaded files to the snippet' do
expect_any_instance_of(described_class).not_to receive(:move_temporary_files)
subject
end
end
end
end
end
end
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