From 1886d727f738895bb552151d59d4024f405522e2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 7 Jan 2016 13:37:14 +0100 Subject: [PATCH 1/4] Add API project upload endpoint --- .../javascripts/dropzone_input.js.coffee | 14 ++-------- app/services/projects/download_service.rb | 7 ++++- app/services/projects/upload_service.rb | 7 ++++- doc/api/projects.md | 28 +++++++++++++++++++ lib/api/projects.rb | 12 +++++++- lib/gitlab/email/receiver.rb | 7 ++--- lib/gitlab/fogbugz_import/importer.rb | 4 +-- spec/requests/api/projects_spec.rb | 14 ++++++++++ 8 files changed, 70 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/dropzone_input.js.coffee b/app/assets/javascripts/dropzone_input.js.coffee index 30a35a04339..c714c0fa939 100644 --- a/app/assets/javascripts/dropzone_input.js.coffee +++ b/app/assets/javascripts/dropzone_input.js.coffee @@ -66,7 +66,7 @@ class @DropzoneInput success: (header, response) -> child = $(dropzone[0]).children("textarea") - $(child).val $(child).val() + formatLink(response.link) + "\n" + $(child).val $(child).val() + response.link.markdown + "\n" return error: (temp, errorMessage) -> @@ -99,11 +99,6 @@ class @DropzoneInput child = $(dropzone[0]).children("textarea") - formatLink = (link) -> - text = "[#{link.alt}](#{link.url})" - text = "!#{text}" if link.is_image - text - handlePaste = (event) -> pasteEvent = event.originalEvent if pasteEvent.clipboardData and pasteEvent.clipboardData.items @@ -162,7 +157,7 @@ class @DropzoneInput closeAlertMessage() success: (e, textStatus, response) -> - insertToTextArea(filename, formatLink(response.responseJSON.link)) + insertToTextArea(filename, response.responseJSON.link.markdown) error: (response) -> showError(response.responseJSON.message) @@ -202,8 +197,3 @@ class @DropzoneInput e.preventDefault() $(@).closest('.gfm-form').find('.div-dropzone').click() return - - formatLink: (link) -> - text = "[#{link.alt}](#{link.url})" - text = "!#{text}" if link.is_image - text diff --git a/app/services/projects/download_service.rb b/app/services/projects/download_service.rb index 99f22293d0d..b846a59ed94 100644 --- a/app/services/projects/download_service.rb +++ b/app/services/projects/download_service.rb @@ -18,10 +18,15 @@ module Projects filename = uploader.image? ? uploader.file.basename : uploader.file.filename + escaped_filename = filename.gsub("]", "\\]") + markdown = "[#{escaped_filename}](#{uploader.secure_url})" + markdown.prepend("!") if uploader.image? + { 'alt' => filename, 'url' => uploader.secure_url, - 'is_image' => uploader.image? + 'is_image' => uploader.image?, + 'markdown' => markdown } end diff --git a/app/services/projects/upload_service.rb b/app/services/projects/upload_service.rb index 279550d6f4a..36ccf1cda12 100644 --- a/app/services/projects/upload_service.rb +++ b/app/services/projects/upload_service.rb @@ -12,10 +12,15 @@ module Projects filename = uploader.image? ? uploader.file.basename : uploader.file.filename + escaped_filename = filename.gsub("]", "\\]") + markdown = "[#{escaped_filename}](#{uploader.secure_url})" + markdown.prepend("!") if uploader.image? + { alt: filename, url: uploader.secure_url, - is_image: uploader.image? + is_image: uploader.image?, + markdown: markdown } end diff --git a/doc/api/projects.md b/doc/api/projects.md index 0ca81ffd49e..37d74216c1b 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -482,6 +482,34 @@ Parameters: - `id` (required) - The ID of a project +## Uploads + +### Upload a file + +Uploads a file to the specified project to be used in an issue or merge request description, or a comment. + +``` +POST /projects/:id/uploads +``` + +Parameters: + +- `id` (required) - The ID of the project +- `file` (required) - The file to be uploaded + +```json +{ + "alt": "dk", + "url": "/uploads/66dbcd21ec5d24ed6ea225176098d52b/dk.png", + "is_image": true, + "markdown": "![dk](/uploads/66dbcd21ec5d24ed6ea225176098d52b/dk.png)" +} +``` + +**Note**: The returned `url` is relative to the project path. +In Markdown contexts, the link is automatically expanded when the format in `markdown` is used. + + ## Team members ### List project team members diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 0781236cf6d..8b1390e3289 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -269,7 +269,7 @@ module API # Remove a forked_from relationship # # Parameters: - # id: (required) - The ID of the project being marked as a fork + # id: (required) - The ID of the project being marked as a fork # Example Request: # DELETE /projects/:id/fork delete ":id/fork" do @@ -278,6 +278,16 @@ module API user_project.forked_project_link.destroy end end + + # Upload a file + # + # Parameters: + # id: (required) - The ID of the project + # file: (required) - The file to be uploaded + post ":id/uploads" do + ::Projects::UploadService.new(user_project, params[:file]).execute + end + # search for projects current_user has access to # # Parameters: diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index 2b252b32887..2ca21af5bc8 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -74,7 +74,7 @@ module Gitlab def sent_notification return nil unless reply_key - + SentNotification.for(reply_key) end @@ -82,10 +82,7 @@ module Gitlab attachments = Email::AttachmentUploader.new(message).execute(sent_notification.project) attachments.each do |link| - text = "[#{link[:alt]}](#{link[:url]})" - text.prepend("!") if link[:is_image] - - reply << "\n\n#{text}" + reply << "\n\n#{link[:markdown]}" end reply diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 403ebeec474..d5f755f90e5 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -232,9 +232,7 @@ module Gitlab return nil if res.nil? - text = "[#{res['alt']}](#{res['url']})" - text = "!#{text}" if res['is_image'] - text + text = res['markdown'] end def build_attachment_url(rel_url) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index ab2530859ea..6f4c336b66c 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -353,6 +353,20 @@ describe API::API, api: true do end end + describe "POST /projects/:id/uploads" do + before { project } + + it "uploads the file and returns its info" do + post api("/projects/#{project.id}/uploads", user), file: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png") + + expect(response.status).to be(201) + 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 + describe 'GET /projects/:id' do before { project } before { project_member } From 706d7eb0b7a1d54604a11e1202e44069c6acccee Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 7 Jan 2016 13:37:59 +0100 Subject: [PATCH 2/4] Satisfy Rubocp --- lib/gitlab/fogbugz_import/importer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index d5f755f90e5..0e6bee732f1 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -232,7 +232,7 @@ module Gitlab return nil if res.nil? - text = res['markdown'] + res['markdown'] end def build_attachment_url(rel_url) From 1e927d39b4bf6d1177dee0dd4a6c60bf270db3f2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 7 Jan 2016 15:51:12 +0100 Subject: [PATCH 3/4] Update spec --- Gemfile.lock | 10 +++++----- spec/lib/gitlab/email/receiver_spec.rb | 11 +++++++---- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a1168ed3b7a..3c7cb6cf439 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -443,6 +443,10 @@ GEM omniauth (1.2.2) hashie (>= 1.2, < 4) rack (~> 1.0) + omniauth-azure-oauth2 (0.0.6) + jwt (~> 1.0) + omniauth (~> 1.0) + omniauth-oauth2 (~> 1.1) omniauth-bitbucket (0.0.2) multi_json (~> 1.7) omniauth (~> 1.1) @@ -488,10 +492,6 @@ GEM activesupport nokogiri (>= 1.4.4) omniauth (~> 1.0) - omniauth-azure-oauth2 (0.0.6) - jwt (~> 1.0) - omniauth (~> 1.0) - omniauth-oauth2 (~> 1.1) opennebula (4.14.2) json nokogiri @@ -920,6 +920,7 @@ DEPENDENCIES oauth2 (~> 1.0.0) octokit (~> 3.7.0) omniauth (~> 1.2.2) + omniauth-azure-oauth2 omniauth-bitbucket (~> 0.0.2) omniauth-cas3 (~> 1.1.2) omniauth-facebook (~> 3.0.0) @@ -931,7 +932,6 @@ DEPENDENCIES omniauth-shibboleth (~> 1.2.0) omniauth-twitter (~> 1.2.0) omniauth_crowd - omniauth-azure-oauth2 org-ruby (~> 0.9.12) paranoia (~> 2.0) pg (~> 0.18.2) diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index b535413bbd4..abe179cd4af 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -42,7 +42,7 @@ describe Gitlab::Email::Receiver, lib: true do context "when the email was auto generated" do let!(:reply_key) { '636ca428858779856c226bb145ef4fad' } let!(:email_raw) { fixture_file("emails/auto_reply.eml") } - + it "raises an AutoGeneratedEmailError" do expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::AutoGeneratedEmailError) end @@ -90,7 +90,7 @@ describe Gitlab::Email::Receiver, lib: true do context "when the reply is blank" do let!(:email_raw) { fixture_file("emails/no_content_reply.eml") } - + it "raises an EmptyEmailError" do expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::EmptyEmailError) end @@ -107,13 +107,16 @@ describe Gitlab::Email::Receiver, lib: true do end context "when everything is fine" do + let(:markdown) { "![image](uploads/image.png)" } + before do allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return( [ { url: "uploads/image.png", is_image: true, - alt: "image" + alt: "image", + markdown: markdown } ] ) @@ -132,7 +135,7 @@ describe Gitlab::Email::Receiver, lib: true do note = noteable.notes.last - expect(note.note).to include("![image](uploads/image.png)") + expect(note.note).to include(markdown) end end end From 0614793b38db4711053cbcb4fa80d9c8cc492eec Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 8 Jan 2016 17:38:53 +0100 Subject: [PATCH 4/4] DRY up upload and download services --- app/services/projects/download_service.rb | 13 +--------- app/services/projects/upload_service.rb | 13 +--------- app/uploaders/file_uploader.rb | 15 ++++++++++++ lib/gitlab/fogbugz_import/importer.rb | 2 +- .../projects/download_service_spec.rb | 24 +++++++++---------- 5 files changed, 30 insertions(+), 37 deletions(-) diff --git a/app/services/projects/download_service.rb b/app/services/projects/download_service.rb index b846a59ed94..6386f57fb0d 100644 --- a/app/services/projects/download_service.rb +++ b/app/services/projects/download_service.rb @@ -16,18 +16,7 @@ module Projects uploader.download!(@url) uploader.store! - filename = uploader.image? ? uploader.file.basename : uploader.file.filename - - escaped_filename = filename.gsub("]", "\\]") - markdown = "[#{escaped_filename}](#{uploader.secure_url})" - markdown.prepend("!") if uploader.image? - - { - 'alt' => filename, - 'url' => uploader.secure_url, - 'is_image' => uploader.image?, - 'markdown' => markdown - } + uploader.to_h end private diff --git a/app/services/projects/upload_service.rb b/app/services/projects/upload_service.rb index 36ccf1cda12..012e82a7704 100644 --- a/app/services/projects/upload_service.rb +++ b/app/services/projects/upload_service.rb @@ -10,18 +10,7 @@ module Projects uploader = FileUploader.new(@project) uploader.store!(@file) - filename = uploader.image? ? uploader.file.basename : uploader.file.filename - - escaped_filename = filename.gsub("]", "\\]") - markdown = "[#{escaped_filename}](#{uploader.secure_url})" - markdown.prepend("!") if uploader.image? - - { - alt: filename, - url: uploader.secure_url, - is_image: uploader.image?, - markdown: markdown - } + uploader.to_h end private diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index ac920119a85..86d24469e05 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -30,4 +30,19 @@ class FileUploader < CarrierWave::Uploader::Base def secure_url File.join("/uploads", @secret, file.filename) end + + def to_h + filename = image? ? self.file.basename : self.file.filename + escaped_filename = filename.gsub("]", "\\]") + + markdown = "[#{escaped_filename}](#{self.secure_url})" + markdown.prepend("!") if image? + + { + alt: filename, + url: self.secure_url, + is_image: image?, + markdown: markdown + } + end end diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 0e6bee732f1..db580b5e578 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -232,7 +232,7 @@ module Gitlab return nil if res.nil? - res['markdown'] + res[:markdown] end def build_attachment_url(rel_url) diff --git a/spec/services/projects/download_service_spec.rb b/spec/services/projects/download_service_spec.rb index 5ceed5af9a5..f252e2c5902 100644 --- a/spec/services/projects/download_service_spec.rb +++ b/spec/services/projects/download_service_spec.rb @@ -33,12 +33,12 @@ describe Projects::DownloadService, services: true do @link_to_file = download_file(@project, url) end - 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') } + 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 context 'a txt file' do @@ -47,12 +47,12 @@ describe Projects::DownloadService, services: true do @link_to_file = download_file(@project, url) end - 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') } + 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 end end