From 4464c22d6d23d893494682d309aec3fb31c11ae3 Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Wed, 3 May 2017 17:26:49 +0200 Subject: Support descriptions for snippets --- app/assets/javascripts/dispatcher.js | 10 +++ app/assets/javascripts/dropzone_input.js | 7 +- app/controllers/projects/snippets_controller.rb | 2 +- app/controllers/snippets_controller.rb | 10 ++- app/controllers/uploads_controller.rb | 4 + app/helpers/gitlab_routing_helper.rb | 2 +- app/models/snippet.rb | 1 + app/uploaders/file_mover.rb | 48 ++++++++++++ app/uploaders/personal_file_uploader.rb | 6 +- app/views/layouts/snippets.html.haml | 5 +- .../shared/form_elements/_description.html.haml | 23 ++++++ app/views/shared/issuable/_form.html.haml | 2 +- .../shared/issuable/form/_description.html.haml | 22 ------ app/views/shared/snippets/_form.html.haml | 7 +- app/views/shared/snippets/_header.html.haml | 7 ++ .../unreleased/12910-snippets-description.yml | 4 + config/routes/snippets.rb | 3 + config/routes/uploads.rb | 2 +- .../20170503114228_add_description_to_snippets.rb | 12 +++ db/schema.rb | 2 + doc/api/project_snippets.md | 5 +- doc/api/snippets.md | 33 +++++---- lib/api/entities.rb | 4 +- lib/api/project_snippets.rb | 2 + lib/api/snippets.rb | 2 + .../projects/snippets_controller_spec.rb | 12 ++- spec/controllers/snippets_controller_spec.rb | 40 +++++++++- spec/factories/snippets.rb | 1 + .../projects/snippets/create_snippet_spec.rb | 86 ++++++++++++++++++++++ spec/features/snippets/create_snippet_spec.rb | 51 ++++++++++++- spec/features/snippets/edit_snippet_spec.rb | 38 ++++++++++ .../gitlab/import_export/safe_model_attributes.yml | 1 + spec/requests/api/project_snippets_spec.rb | 28 ++++++- spec/requests/api/snippets_spec.rb | 27 ++++++- spec/uploaders/file_mover_spec.rb | 25 +++++++ 35 files changed, 478 insertions(+), 56 deletions(-) create mode 100644 app/uploaders/file_mover.rb create mode 100644 app/views/shared/form_elements/_description.html.haml delete mode 100644 app/views/shared/issuable/form/_description.html.haml create mode 100644 changelogs/unreleased/12910-snippets-description.yml create mode 100644 db/migrate/20170503114228_add_description_to_snippets.rb create mode 100644 spec/features/projects/snippets/create_snippet_spec.rb create mode 100644 spec/features/snippets/edit_snippet_spec.rb create mode 100644 spec/uploaders/file_mover_spec.rb diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 6e2f06112dd..f75f2662cea 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -218,6 +218,16 @@ import ShortcutsBlob from './shortcuts_blob'; new gl.GLForm($('.tag-form')); new RefSelectDropdown($('.js-branch-select'), window.gl.availableRefs); break; + case 'projects:snippets:new': + case 'projects:snippets:edit': + case 'projects:snippets:create': + case 'projects:snippets:update': + case 'snippets:new': + case 'snippets:edit': + case 'snippets:create': + case 'snippets:update': + new gl.GLForm($('.snippet-form')); + break; case 'projects:releases:edit': new ZenMode(); new gl.GLForm($('.release-form')); diff --git a/app/assets/javascripts/dropzone_input.js b/app/assets/javascripts/dropzone_input.js index 266cd3966c6..f886ce21493 100644 --- a/app/assets/javascripts/dropzone_input.js +++ b/app/assets/javascripts/dropzone_input.js @@ -5,7 +5,7 @@ import './preview_markdown'; window.DropzoneInput = (function() { function DropzoneInput(form) { - var updateAttachingMessage, $attachingFileMessage, $mdArea, $attachButton, $cancelButton, $retryLink, $uploadingErrorContainer, $uploadingErrorMessage, $uploadProgress, $uploadingProgressContainer, appendToTextArea, btnAlert, child, closeAlertMessage, closeSpinner, divHover, divSpinner, dropzone, $formDropzone, formTextarea, getFilename, handlePaste, iconPaperclip, iconSpinner, insertToTextArea, isImage, maxFileSize, pasteText, uploadsPath, showError, showSpinner, uploadFile; + var updateAttachingMessage, $attachingFileMessage, $mdArea, $attachButton, $cancelButton, $retryLink, $uploadingErrorContainer, $uploadingErrorMessage, $uploadProgress, $uploadingProgressContainer, appendToTextArea, btnAlert, child, closeAlertMessage, closeSpinner, divHover, divSpinner, dropzone, $formDropzone, formTextarea, getFilename, handlePaste, iconPaperclip, iconSpinner, insertToTextArea, isImage, maxFileSize, pasteText, uploadsPath, showError, showSpinner, uploadFile, addFileToForm; Dropzone.autoDiscover = false; divHover = '
'; iconPaperclip = ''; @@ -71,6 +71,7 @@ window.DropzoneInput = (function() { pasteText(response.link.markdown, shouldPad); // Show 'Attach a file' link only when all files have been uploaded. if (!processingFileCount) $attachButton.removeClass('hide'); + addFileToForm(response.link.url); }, error: function(file, errorMessage = 'Attaching the file failed.', xhr) { // If 'error' event is fired by dropzone, the second parameter is error message. @@ -197,6 +198,10 @@ window.DropzoneInput = (function() { return formTextarea.trigger('input'); }; + addFileToForm = function(path) { + $(form).append(''); + }; + getFilename = function(e) { var value; if (window.clipboardData && window.clipboardData.getData) { diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index 3b2b0d9e502..08f339d98fc 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -107,6 +107,6 @@ class Projects::SnippetsController < Projects::ApplicationController end def snippet_params - params.require(:project_snippet).permit(:title, :content, :file_name, :private, :visibility_level) + params.require(:project_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description) end end diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 7445f61195d..1334f7daa44 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -45,6 +45,8 @@ class SnippetsController < ApplicationController @snippet = CreateSnippetService.new(nil, current_user, create_params).execute + move_temporary_files if params[:files] + recaptcha_check_with_fallback { render :new } end @@ -124,6 +126,12 @@ class SnippetsController < ApplicationController end def snippet_params - params.require(:personal_snippet).permit(:title, :content, :file_name, :private, :visibility_level) + 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, @snippet).execute + end end end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index eef53730291..5cb3de3d4f5 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -9,6 +9,8 @@ class UploadsController < ApplicationController private def find_model + return nil unless params[:id] + return render_404 unless upload_model && upload_mount @model = upload_model.find(params[:id]) @@ -33,6 +35,8 @@ class UploadsController < ApplicationController end def authorize_create_access! + return unless model + # for now we support only personal snippets comments authorized = can?(current_user, :comment_personal_snippet, model) diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index fc308b3960e..7bf2600fd1a 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -128,7 +128,7 @@ module GitlabRoutingHelper def preview_markdown_path(project, *args) if @snippet.is_a?(PersonalSnippet) - preview_markdown_snippet_path(@snippet) + preview_markdown_snippets_path else preview_markdown_namespace_project_path(project.namespace, project, *args) end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 882e2fa0594..ace684b820b 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -10,6 +10,7 @@ class Snippet < ActiveRecord::Base include Spammable cache_markdown_field :title, pipeline: :single_line + cache_markdown_field :description cache_markdown_field :content # Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with snippets. diff --git a/app/uploaders/file_mover.rb b/app/uploaders/file_mover.rb new file mode 100644 index 00000000000..21e37a08a82 --- /dev/null +++ b/app/uploaders/file_mover.rb @@ -0,0 +1,48 @@ +class FileMover + attr_reader :secret, :file_name, :model + + def initialize(file_path, model, update_field = :description) + @secret = File.split(File.dirname(file_path)).last + @file_name = File.basename(file_path) + @model = model + end + + def execute + move + update_markdown + end + + private + + def move + FileUtils.mkdir_p(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) + end + + def temp_file_path + temp_file_uploader.retrieve_from_store!(file_name) + + temp_file_uploader.file.path + end + + def file_path + return @file_path if @file_path + + uploader.retrieve_from_store!(file_name) + + @file_path = uploader.file.path + end + + def uploader + @uploader ||= PersonalFileUploader.new(model, secret) + end + + def temp_file_uploader + @temp_file_uploader ||= PersonalFileUploader.new(nil, secret) + end +end diff --git a/app/uploaders/personal_file_uploader.rb b/app/uploaders/personal_file_uploader.rb index 969b0a20d38..7f857765fbf 100644 --- a/app/uploaders/personal_file_uploader.rb +++ b/app/uploaders/personal_file_uploader.rb @@ -10,6 +10,10 @@ class PersonalFileUploader < FileUploader end def self.model_path(model) - File.join("/#{base_dir}", model.class.to_s.underscore, model.id.to_s) + if model + File.join("/#{base_dir}", model.class.to_s.underscore, model.id.to_s) + else + File.join("/#{base_dir}", 'temp') + end end end diff --git a/app/views/layouts/snippets.html.haml b/app/views/layouts/snippets.html.haml index 98b75cea03f..57971205e0e 100644 --- a/app/views/layouts/snippets.html.haml +++ b/app/views/layouts/snippets.html.haml @@ -1,9 +1,8 @@ - header_title "Snippets", snippets_path - content_for :page_specific_javascripts do - - if @snippet&.persisted? && current_user + - if @snippet && current_user :javascript - window.uploads_path = "#{upload_path('personal_snippet', @snippet)}"; - window.preview_markdown_path = "#{preview_markdown_snippet_path(@snippet)}"; + window.uploads_path = "#{upload_path('personal_snippet', id: @snippet.id)}"; = render template: "layouts/application" diff --git a/app/views/shared/form_elements/_description.html.haml b/app/views/shared/form_elements/_description.html.haml new file mode 100644 index 00000000000..91224e232ca --- /dev/null +++ b/app/views/shared/form_elements/_description.html.haml @@ -0,0 +1,23 @@ +- project = local_assigns.fetch(:project) +- model = local_assigns.fetch(:model) + +- form = local_assigns.fetch(:form) +- supports_slash_commands = !model.persisted? + +- if supports_slash_commands + - preview_url = preview_markdown_path(project, slash_commands_target_type: model.class.name) +- else + - preview_url = preview_markdown_path(project) + +.form-group.detail-page-description + = form.label :description, 'Description', class: 'control-label' + .col-sm-10 + + = render layout: 'projects/md_preview', locals: { url: preview_url, referenced_users: true } do + = render 'projects/zen', f: form, attr: :description, + classes: 'note-textarea', + placeholder: "Write a comment or drag your files here...", + supports_slash_commands: supports_slash_commands + = render 'shared/notes/hints', supports_slash_commands: supports_slash_commands + .clearfix + .error-alert diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 7748351b333..c016aa2abcd 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -17,7 +17,7 @@ = render 'shared/issuable/form/template_selector', issuable: issuable = render 'shared/issuable/form/title', issuable: issuable, form: form, has_wip_commits: commits && commits.detect(&:work_in_progress?) -= render 'shared/issuable/form/description', issuable: issuable, form: form, project: project += render 'shared/form_elements/description', model: issuable, form: form, project: project - if issuable.respond_to?(:confidential) .form-group diff --git a/app/views/shared/issuable/form/_description.html.haml b/app/views/shared/issuable/form/_description.html.haml deleted file mode 100644 index 7ef0ae96be2..00000000000 --- a/app/views/shared/issuable/form/_description.html.haml +++ /dev/null @@ -1,22 +0,0 @@ -- project = local_assigns.fetch(:project) -- issuable = local_assigns.fetch(:issuable) -- form = local_assigns.fetch(:form) -- supports_slash_commands = issuable.new_record? - -- if supports_slash_commands - - preview_url = preview_markdown_path(project, slash_commands_target_type: issuable.class.name) -- else - - preview_url = preview_markdown_path(project) - -.form-group.detail-page-description - = form.label :description, 'Description', class: 'control-label' - .col-sm-10 - - = render layout: 'projects/md_preview', locals: { url: preview_url, referenced_users: true } do - = render 'projects/zen', f: form, attr: :description, - classes: 'note-textarea', - placeholder: "Write a comment or drag your files here...", - supports_slash_commands: supports_slash_commands - = render 'shared/notes/hints', supports_slash_commands: supports_slash_commands - .clearfix - .error-alert diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml index 0296597b294..8549cb91b03 100644 --- a/app/views/shared/snippets/_form.html.haml +++ b/app/views/shared/snippets/_form.html.haml @@ -3,7 +3,7 @@ = page_specific_javascript_bundle_tag('snippet') .snippet-form-holder - = form_for @snippet, url: url, html: { class: "form-horizontal snippet-form js-requires-input js-quick-submit" } do |f| + = form_for @snippet, url: url, html: { class: "form-horizontal snippet-form js-requires-input js-quick-submit common-note-form" } do |f| = form_errors(@snippet) .form-group @@ -11,6 +11,8 @@ .col-sm-10 = f.text_field :title, class: 'form-control', required: true, autofocus: true + = render 'shared/form_elements/description', model: @snippet, project: @project, form: f + = render 'shared/visibility_level', f: f, visibility_level: @snippet.visibility_level, can_change_visibility_level: true, form_model: @snippet .file-editor @@ -23,6 +25,9 @@ .file-content.code %pre#editor= @snippet.content = f.hidden_field :content, class: 'snippet-file-content' + - if params[:files] + - params[:files].each_with_index do |file, index| + = hidden_field_tag "files[]", file, id: "files_#{index}" .form-actions - if @snippet.new_record? diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index 501c09d71d5..d2b94ed4c0b 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -22,3 +22,10 @@ - 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 diff --git a/changelogs/unreleased/12910-snippets-description.yml b/changelogs/unreleased/12910-snippets-description.yml new file mode 100644 index 00000000000..ac3d754fee1 --- /dev/null +++ b/changelogs/unreleased/12910-snippets-description.yml @@ -0,0 +1,4 @@ +--- +title: Support descriptions for snippets +merge_request: +author: diff --git a/config/routes/snippets.rb b/config/routes/snippets.rb index dae83734fe6..0a4ebac3ca3 100644 --- a/config/routes/snippets.rb +++ b/config/routes/snippets.rb @@ -2,6 +2,9 @@ resources :snippets, concerns: :awardable do member do get :raw post :mark_as_spam + end + + collection do post :preview_markdown end diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb index b315186b178..76c31260394 100644 --- a/config/routes/uploads.rb +++ b/config/routes/uploads.rb @@ -20,7 +20,7 @@ scope path: :uploads do constraints: { namespace_id: /[a-zA-Z.0-9_\-]+/, project_id: /[a-zA-Z.0-9_\-]+/, filename: /[^\/]+/ } # create uploads for models, snippets (notes) available for now - post ':model/:id/', + post ':model', to: 'uploads#create', constraints: { model: /personal_snippet/, id: /\d+/ }, as: 'upload' diff --git a/db/migrate/20170503114228_add_description_to_snippets.rb b/db/migrate/20170503114228_add_description_to_snippets.rb new file mode 100644 index 00000000000..3fc960b2da5 --- /dev/null +++ b/db/migrate/20170503114228_add_description_to_snippets.rb @@ -0,0 +1,12 @@ +class AddDescriptionToSnippets < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def change + add_column :snippets, :description, :text + add_column :snippets, :description_html, :text + end +end diff --git a/db/schema.rb b/db/schema.rb index 4c73f74ef1f..2f83e30f6ce 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1153,6 +1153,8 @@ ActiveRecord::Schema.define(version: 20170523091700) do t.text "title_html" t.text "content_html" t.integer "cached_markdown_version" + t.text "description" + t.text "description_html" end add_index "snippets", ["author_id"], name: "index_snippets_on_author_id", using: :btree diff --git a/doc/api/project_snippets.md b/doc/api/project_snippets.md index ff379473961..80c4b5f1c34 100644 --- a/doc/api/project_snippets.md +++ b/doc/api/project_snippets.md @@ -43,6 +43,7 @@ Parameters: "id": 1, "title": "test", "file_name": "add.rb", + "description": "Ruby test snippet", "author": { "id": 1, "username": "john_smith", @@ -70,8 +71,9 @@ Parameters: - `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user - `title` (required) - The title of a snippet - `file_name` (required) - The name of a snippet file +- `description` (optional) - The description of a snippet - `code` (required) - The content of a snippet -- `visibility` (required) - The snippet's visibility +- `visibility` (optional) - The snippet's visibility ## Update snippet @@ -87,6 +89,7 @@ Parameters: - `snippet_id` (required) - The ID of a project's snippet - `title` (optional) - The title of a snippet - `file_name` (optional) - The name of a snippet file +- `description` (optional) - The description of a snippet - `code` (optional) - The content of a snippet - `visibility` (optional) - The snippet's visibility diff --git a/doc/api/snippets.md b/doc/api/snippets.md index fb8cf97896c..efaab712367 100644 --- a/doc/api/snippets.md +++ b/doc/api/snippets.md @@ -48,6 +48,7 @@ Example response: "id": 1, "title": "test", "file_name": "add.rb", + "description": "Ruby test snippet", "author": { "id": 1, "username": "john_smith", @@ -73,16 +74,17 @@ POST /snippets Parameters: -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `title` | String | yes | The title of a snippet | -| `file_name` | String | yes | The name of a snippet file | -| `content` | String | yes | The content of a snippet | -| `visibility` | String | yes | The snippet's visibility | +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `title` | String | yes | The title of a snippet | +| `file_name` | String | yes | The name of a snippet file | +| `content` | String | yes | The content of a snippet | +| `description` | String | no | The description of a snippet | +| `visibility` | String | no | The snippet's visibility | ``` bash -curl --request POST --data '{"title": "This is a snippet", "content": "Hello world", "file_name": "test.txt", "visibility": "internal" }' --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/snippets +curl --request POST --data '{"title": "This is a snippet", "content": "Hello world", "description": "Hello World snippet", "file_name": "test.txt", "visibility": "internal" }' --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/snippets ``` Example response: @@ -92,6 +94,7 @@ Example response: "id": 1, "title": "This is a snippet", "file_name": "test.txt", + "description": "Hello World snippet", "author": { "id": 1, "username": "john_smith", @@ -117,13 +120,14 @@ PUT /snippets/:id Parameters: -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `id` | Integer | yes | The ID of a snippet | -| `title` | String | no | The title of a snippet | -| `file_name` | String | no | The name of a snippet file | -| `content` | String | no | The content of a snippet | -| `visibility` | String | no | The snippet's visibility | +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | Integer | yes | The ID of a snippet | +| `title` | String | no | The title of a snippet | +| `file_name` | String | no | The name of a snippet file | +| `description` | String | no | The description of a snippet | +| `content` | String | no | The content of a snippet | +| `visibility` | String | no | The snippet's visibility | ``` bash @@ -137,6 +141,7 @@ Example response: "id": 1, "title": "test", "file_name": "add.rb", + "description": "description of snippet", "author": { "id": 1, "username": "john_smith", diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 8c5e5c91769..22880066d84 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -224,7 +224,7 @@ module API end class ProjectSnippet < Grape::Entity - expose :id, :title, :file_name + expose :id, :title, :file_name, :description expose :author, using: Entities::UserBasic expose :updated_at, :created_at @@ -234,7 +234,7 @@ module API end class PersonalSnippet < Grape::Entity - expose :id, :title, :file_name + expose :id, :title, :file_name, :description expose :author, using: Entities::UserBasic expose :updated_at, :created_at diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index 98bc9c28527..64efe82a937 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -49,6 +49,7 @@ module API requires :title, type: String, desc: 'The title of the snippet' requires :file_name, type: String, desc: 'The file name of the snippet' requires :code, type: String, desc: 'The content of the snippet' + optional :description, type: String, desc: 'The description of a snippet' requires :visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The visibility of the snippet' @@ -77,6 +78,7 @@ module API optional :title, type: String, desc: 'The title of the snippet' optional :file_name, type: String, desc: 'The file name of the snippet' optional :code, type: String, desc: 'The content of the snippet' + optional :description, type: String, desc: 'The description of a snippet' optional :visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The visibility of the snippet' diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index 53f5953a8fb..c630c24c339 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -58,6 +58,7 @@ module API requires :title, type: String, desc: 'The title of a snippet' requires :file_name, type: String, desc: 'The name of a snippet file' requires :content, type: String, desc: 'The content of a snippet' + optional :description, type: String, desc: 'The description of a snippet' optional :visibility, type: String, values: Gitlab::VisibilityLevel.string_values, default: 'internal', @@ -85,6 +86,7 @@ module API optional :title, type: String, desc: 'The title of a snippet' optional :file_name, type: String, desc: 'The name of a snippet file' optional :content, type: String, desc: 'The content of a snippet' + optional :description, type: String, desc: 'The description of a snippet' optional :visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The visibility of the snippet' diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index 24a59caff4e..8c23c46798e 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -78,8 +78,18 @@ describe Projects::SnippetsController do post :create, { namespace_id: project.namespace.to_param, project_id: project, - project_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params) + project_snippet: { title: 'Title', content: 'Content', description: 'Description' }.merge(snippet_params) }.merge(additional_params) + + Snippet.last + end + + it 'creates the snippet correctly' do + snippet = create_snippet(project, visibility_level: Snippet::PRIVATE) + + expect(snippet.title).to eq('Title') + expect(snippet.content).to eq('Content') + expect(snippet.description).to eq('Description') end context 'when the snippet is spam' do diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 930415a4778..9073c39f562 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -171,12 +171,50 @@ describe SnippetsController do sign_in(user) post :create, { - personal_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params) + personal_snippet: { title: 'Title', content: 'Content', description: 'Description' }.merge(snippet_params) }.merge(additional_params) Snippet.last end + it 'creates the snippet correctly' do + snippet = create_snippet(visibility_level: Snippet::PRIVATE) + + expect(snippet.title).to eq('Title') + expect(snippet.content).to eq('Content') + expect(snippet.description).to eq('Description') + end + + context 'when the snippet description contains a file' do + let(:picture_file) { '/temp/secret56/picture.jpg' } + let(:text_file) { '/temp/secret78/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) + 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/personal_snippet/#{snippet.id}/secret56/picture.jpg) and "\ + "text: [text.txt](/uploads/personal_snippet/#{snippet.id}/secret78/text.txt)" + + expect(snippet.description).to eq(expected_description) + end + end + context 'when the snippet is spam' do before do allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) diff --git a/spec/factories/snippets.rb b/spec/factories/snippets.rb index 18cb0f5de26..388f662e6e5 100644 --- a/spec/factories/snippets.rb +++ b/spec/factories/snippets.rb @@ -3,6 +3,7 @@ FactoryGirl.define do author title { generate(:title) } content { generate(:title) } + description { generate(:title) } file_name { generate(:filename) } trait :public do diff --git a/spec/features/projects/snippets/create_snippet_spec.rb b/spec/features/projects/snippets/create_snippet_spec.rb new file mode 100644 index 00000000000..be9cc9f7029 --- /dev/null +++ b/spec/features/projects/snippets/create_snippet_spec.rb @@ -0,0 +1,86 @@ +require 'rails_helper' + +feature 'Create Snippet', :js, feature: true do + include DropzoneHelper + + let(:user) { create(:user) } + let(:project) { create(:project, :repository, :public) } + + def fill_form + fill_in 'project_snippet_title', with: 'My Snippet Title' + fill_in 'project_snippet_description', with: 'My Snippet **Description**' + page.within('.file-editor') do + find('.ace_editor').native.send_keys('Hello World!') + end + end + + context 'when a user is authenticated' do + before do + project.team << [user, :master] + login_as(user) + + visit namespace_project_snippets_path(project.namespace, project) + + click_on('New snippet') + end + + it 'creates a new snippet' do + fill_form + click_button('Create snippet') + wait_for_ajax + + expect(page).to have_content('My Snippet Title') + expect(page).to have_content('Hello World!') + page.within('.snippet-header .description') do + expect(page).to have_content('My Snippet Description') + expect(page).to have_selector('strong') + end + end + + it 'uploads a file when dragging into textarea' do + fill_form + dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') + + expect(page.find_field("project_snippet_description").value).to have_content('banana_sample') + + click_button('Create snippet') + wait_for_ajax + + 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}) + end + + it 'creates a snippet when all reuiqred fields are filled in after validation failing' do + fill_in 'project_snippet_title', with: 'My Snippet Title' + click_button('Create snippet') + + expect(page).to have_selector('#error_explanation') + + fill_form + dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') + + click_button('Create snippet') + wait_for_ajax + + expect(page).to have_content('My Snippet Title') + expect(page).to have_content('Hello World!') + page.within('.snippet-header .description') do + expect(page).to have_content('My Snippet Description') + expect(page).to have_selector('strong') + end + 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}) + end + end + + context 'when a user is not authenticated' do + it 'shows a public snippet on the index page but not the New snippet button' do + snippet = create(:project_snippet, :public, project: project) + + visit namespace_project_snippets_path(project.namespace, project) + + expect(page).to have_content(snippet.title) + expect(page).not_to have_content('New snippet') + end + end +end diff --git a/spec/features/snippets/create_snippet_spec.rb b/spec/features/snippets/create_snippet_spec.rb index 31a2d4ae984..f006057669b 100644 --- a/spec/features/snippets/create_snippet_spec.rb +++ b/spec/features/snippets/create_snippet_spec.rb @@ -1,22 +1,69 @@ require 'rails_helper' feature 'Create Snippet', :js, feature: true do + include DropzoneHelper + before do login_as :user visit new_snippet_path end - scenario 'Authenticated user creates a snippet' do + def fill_form fill_in 'personal_snippet_title', with: 'My Snippet Title' + fill_in 'personal_snippet_description', with: 'My Snippet **Description**' page.within('.file-editor') do find('.ace_editor').native.send_keys 'Hello World!' end + end - click_button 'Create snippet' + scenario 'Authenticated user creates a snippet' do + fill_form + + click_button('Create snippet') wait_for_requests expect(page).to have_content('My Snippet Title') + page.within('.snippet-header .description') do + expect(page).to have_content('My Snippet Description') + expect(page).to have_selector('strong') + end + expect(page).to have_content('Hello World!') + end + + scenario 'uploads a file when dragging into textarea' do + fill_form + + dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') + + expect(page.find_field("personal_snippet_description").value).to have_content('banana_sample') + + click_button('Create snippet') + wait_for_requests + + 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}) + end + + scenario 'validation fails for the first time' do + fill_in 'personal_snippet_title', with: 'My Snippet Title' + click_button('Create snippet') + + expect(page).to have_selector('#error_explanation') + + fill_form + dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') + + click_button('Create snippet') + wait_for_requests + + expect(page).to have_content('My Snippet Title') + page.within('.snippet-header .description') do + expect(page).to have_content('My Snippet Description') + expect(page).to have_selector('strong') + end 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}) end scenario 'Authenticated user creates a snippet with + in filename' do diff --git a/spec/features/snippets/edit_snippet_spec.rb b/spec/features/snippets/edit_snippet_spec.rb new file mode 100644 index 00000000000..ee04792ef73 --- /dev/null +++ b/spec/features/snippets/edit_snippet_spec.rb @@ -0,0 +1,38 @@ +require 'rails_helper' + +feature 'Edit Snippet', :js, feature: true do + include DropzoneHelper + + let(:file_name) { 'test.rb' } + let(:content) { 'puts "test"' } + + let(:user) { create(:user) } + let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, author: user) } + + before do + login_as(user) + + visit edit_snippet_path(snippet) + wait_for_ajax + end + + it 'updates the snippet' do + fill_in 'personal_snippet_title', with: 'New Snippet Title' + + click_button('Save changes') + wait_for_ajax + + expect(page).to have_content('New Snippet Title') + end + + it 'updates the snippet with files attached' do + dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') + expect(page.find_field("personal_snippet_description").value).to have_content('banana_sample') + + click_button('Save changes') + wait_for_ajax + + 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}) + end +end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 96054c996fd..1dd0c748c7b 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -92,6 +92,7 @@ Milestone: ProjectSnippet: - id - title +- description - content - author_id - project_id diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 3ab1764f5c3..4d4631322b1 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -36,11 +36,34 @@ describe API::ProjectSnippets do end end + describe 'GET /projects/:project_id/snippets/:id' do + let(:user) { create(:user) } + let(:snippet) { create(:project_snippet, :public, project: project) } + + it 'returns snippet json' do + get api("/projects/#{project.id}/snippets/#{snippet.id}", user) + + expect(response).to have_http_status(200) + + expect(json_response['title']).to eq(snippet.title) + expect(json_response['description']).to eq(snippet.description) + expect(json_response['file_name']).to eq(snippet.file_name) + end + + it 'returns 404 for invalid snippet id' do + get api("/projects/#{project.id}/snippets/1234", user) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Not found') + end + end + describe 'POST /projects/:project_id/snippets/' do let(:params) do { title: 'Test Title', file_name: 'test.rb', + description: 'test description', code: 'puts "hello world"', visibility: 'public' } @@ -52,6 +75,7 @@ describe API::ProjectSnippets do expect(response).to have_http_status(201) snippet = ProjectSnippet.find(json_response['id']) expect(snippet.content).to eq(params[:code]) + expect(snippet.description).to eq(params[:description]) expect(snippet.title).to eq(params[:title]) expect(snippet.file_name).to eq(params[:file_name]) expect(snippet.visibility_level).to eq(Snippet::PUBLIC) @@ -106,12 +130,14 @@ describe API::ProjectSnippets do it 'updates snippet' do new_content = 'New content' + new_description = 'New description' - put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin), code: new_content + put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin), code: new_content, description: new_description expect(response).to have_http_status(200) snippet.reload expect(snippet.content).to eq(new_content) + expect(snippet.description).to eq(new_description) end it 'returns 404 for invalid snippet id' do diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index e429cddcf6a..8741cbd4e80 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -80,11 +80,33 @@ describe API::Snippets do end end + describe 'GET /snippets/:id' do + let(:snippet) { create(:personal_snippet, author: user) } + + it 'returns snippet json' do + get api("/snippets/#{snippet.id}", user) + + expect(response).to have_http_status(200) + + expect(json_response['title']).to eq(snippet.title) + expect(json_response['description']).to eq(snippet.description) + expect(json_response['file_name']).to eq(snippet.file_name) + end + + it 'returns 404 for invalid snippet id' do + get api("/snippets/1234", user) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Not found') + end + end + describe 'POST /snippets/' do let(:params) do { title: 'Test Title', file_name: 'test.rb', + description: 'test description', content: 'puts "hello world"', visibility: 'public' } @@ -97,6 +119,7 @@ describe API::Snippets do expect(response).to have_http_status(201) expect(json_response['title']).to eq(params[:title]) + expect(json_response['description']).to eq(params[:description]) expect(json_response['file_name']).to eq(params[:file_name]) end @@ -150,12 +173,14 @@ describe API::Snippets do it 'updates snippet' do new_content = 'New content' + new_description = 'New description' - put api("/snippets/#{snippet.id}", user), content: new_content + put api("/snippets/#{snippet.id}", user), content: new_content, description: new_description expect(response).to have_http_status(200) snippet.reload expect(snippet.content).to eq(new_content) + expect(snippet.description).to eq(new_description) end it 'returns 404 for invalid snippet id' do diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb new file mode 100644 index 00000000000..99d50e78240 --- /dev/null +++ b/spec/uploaders/file_mover_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +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_file_path) { File.join('secret55', filename).to_s } + let(:file_path) { File.join('uploads', 'personal_snippet', snippet.id.to_s, 'secret55', filename).to_s } + + let(:snippet) { create(:personal_snippet, description: temp_description) } + + 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)) + expect(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path)) + + subject + + expect(snippet.reload.description) + .to eq("test ![banana_sample](/uploads/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)") + end + end +end -- cgit v1.2.3 From 2e311d9d1aac58bbd9c7d6c97c7cbcccf2715347 Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Mon, 29 May 2017 09:54:35 +0200 Subject: Support uploads for newly created personal snippets --- app/assets/javascripts/dropzone_input.js | 2 +- app/controllers/snippets_controller.rb | 2 +- app/controllers/uploads_controller.rb | 11 ++++- app/uploaders/file_mover.rb | 29 ++++++++++--- app/uploaders/records_uploads.rb | 7 +-- .../shared/form_elements/_description.html.haml | 2 +- app/views/shared/snippets/_header.html.haml | 13 +++--- config/routes/uploads.rb | 5 +++ doc/api/project_snippets.md | 2 +- spec/controllers/uploads_controller_spec.rb | 34 +++++++++++++++ .../projects/snippets/create_snippet_spec.rb | 6 +-- spec/features/snippets/create_snippet_spec.rb | 22 ++++++++++ spec/features/snippets/edit_snippet_spec.rb | 6 +-- spec/uploaders/file_mover_spec.rb | 50 +++++++++++++++++++--- spec/uploaders/records_uploads_spec.rb | 9 +++- 15 files changed, 164 insertions(+), 36 deletions(-) diff --git a/app/assets/javascripts/dropzone_input.js b/app/assets/javascripts/dropzone_input.js index f886ce21493..8837341153b 100644 --- a/app/assets/javascripts/dropzone_input.js +++ b/app/assets/javascripts/dropzone_input.js @@ -199,7 +199,7 @@ window.DropzoneInput = (function() { }; addFileToForm = function(path) { - $(form).append(''); + $(form).append(''); }; getFilename = function(e) { diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 1334f7daa44..6c25f59ccbb 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -45,7 +45,7 @@ class SnippetsController < ApplicationController @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 diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 5cb3de3d4f5..dc882b17143 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -17,6 +17,8 @@ class UploadsController < ApplicationController end def authorize_access! + return nil unless model + authorized = case model when Note @@ -35,7 +37,7 @@ class UploadsController < ApplicationController 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 @@ class UploadsController < ApplicationController 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]) diff --git a/app/uploaders/file_mover.rb b/app/uploaders/file_mover.rb index 21e37a08a82..00c2888d224 100644 --- a/app/uploaders/file_mover.rb +++ b/app/uploaders/file_mover.rb @@ -1,33 +1,42 @@ 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 @@ class FileMover 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 diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb index 4c127f29250..feb4f04d7b7 100644 --- a/app/uploaders/records_uploads.rb +++ b/app/uploaders/records_uploads.rb @@ -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` diff --git a/app/views/shared/form_elements/_description.html.haml b/app/views/shared/form_elements/_description.html.haml index 91224e232ca..307d4919224 100644 --- a/app/views/shared/form_elements/_description.html.haml +++ b/app/views/shared/form_elements/_description.html.haml @@ -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) diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index d2b94ed4c0b..813d8d69d8d 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -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 diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb index 76c31260394..ae8747d766d 100644 --- a/config/routes/uploads.rb +++ b/config/routes/uploads.rb @@ -9,6 +9,11 @@ scope path: :uploads do 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", diff --git a/doc/api/project_snippets.md b/doc/api/project_snippets.md index 80c4b5f1c34..92491de4daa 100644 --- a/doc/api/project_snippets.md +++ b/doc/api/project_snippets.md @@ -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 diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 8000c9dec61..01a0659479b 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -92,6 +92,40 @@ describe UploadsController do 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 diff --git a/spec/features/projects/snippets/create_snippet_spec.rb b/spec/features/projects/snippets/create_snippet_spec.rb index be9cc9f7029..5ac1ca45c74 100644 --- a/spec/features/projects/snippets/create_snippet_spec.rb +++ b/spec/features/projects/snippets/create_snippet_spec.rb @@ -27,7 +27,7 @@ feature 'Create Snippet', :js, feature: true do 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 @@ feature 'Create Snippet', :js, feature: true do 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 @@ feature 'Create Snippet', :js, feature: true do 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!') diff --git a/spec/features/snippets/create_snippet_spec.rb b/spec/features/snippets/create_snippet_spec.rb index f006057669b..ddd31ede064 100644 --- a/spec/features/snippets/create_snippet_spec.rb +++ b/spec/features/snippets/create_snippet_spec.rb @@ -30,6 +30,22 @@ feature 'Create Snippet', :js, feature: true do 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 @@ feature 'Create Snippet', :js, feature: true do 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 @@ feature 'Create Snippet', :js, feature: true do 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 diff --git a/spec/features/snippets/edit_snippet_spec.rb b/spec/features/snippets/edit_snippet_spec.rb index ee04792ef73..89ae593db88 100644 --- a/spec/features/snippets/edit_snippet_spec.rb +++ b/spec/features/snippets/edit_snippet_spec.rb @@ -13,14 +13,14 @@ feature 'Edit Snippet', :js, feature: true do 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 @@ feature 'Edit Snippet', :js, feature: true do 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}) diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb index 99d50e78240..896cb410ed5 100644 --- a/spec/uploaders/file_mover_spec.rb +++ b/spec/uploaders/file_mover_spec.rb @@ -3,7 +3,10 @@ require 'spec_helper' 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 @@ describe FileMover do 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 diff --git a/spec/uploaders/records_uploads_spec.rb b/spec/uploaders/records_uploads_spec.rb index 5c26e334a6e..bb32ee62ccb 100644 --- a/spec/uploaders/records_uploads_spec.rb +++ b/spec/uploaders/records_uploads_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe RecordsUploads do - let(:uploader) do + let!(:uploader) do class RecordsUploadsExampleUploader < GitlabUploader include RecordsUploads @@ -57,6 +57,13 @@ describe RecordsUploads do 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'), -- cgit v1.2.3