From 8c7f4e9d5f36cff46365a7f8c4b9c21578c1e781 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 18 Jun 2020 11:18:50 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-1-stable-ee --- app/services/snippets/base_service.rb | 33 +++++++++++++++++++-- app/services/snippets/bulk_destroy_service.rb | 6 ++-- app/services/snippets/create_service.rb | 31 +++++++++++++------- app/services/snippets/update_service.rb | 41 ++++++++++++++++++--------- 4 files changed, 82 insertions(+), 29 deletions(-) (limited to 'app/services/snippets') diff --git a/app/services/snippets/base_service.rb b/app/services/snippets/base_service.rb index 81d12997335..5d1fe815d83 100644 --- a/app/services/snippets/base_service.rb +++ b/app/services/snippets/base_service.rb @@ -6,12 +6,13 @@ module Snippets CreateRepositoryError = Class.new(StandardError) - attr_reader :uploaded_files + attr_reader :uploaded_assets, :snippet_files def initialize(project, user = nil, params = {}) super - @uploaded_files = Array(@params.delete(:files).presence) + @uploaded_assets = Array(@params.delete(:files).presence) + @snippet_files = SnippetInputActionCollection.new(Array(@params.delete(:snippet_files).presence)) filter_spam_check_params end @@ -22,12 +23,30 @@ module Snippets Gitlab::VisibilityLevel.allowed_for?(current_user, visibility_level) end - def error_forbidden_visibility(snippet) + def forbidden_visibility_error(snippet) deny_visibility_level(snippet) snippet_error_response(snippet, 403) end + def valid_params? + return true if snippet_files.empty? + + (params.keys & [:content, :file_name]).none? && snippet_files.valid? + end + + def invalid_params_error(snippet) + if snippet_files.valid? + [:content, :file_name].each do |key| + snippet.errors.add(key, 'and snippet files cannot be used together') if params.key?(key) + end + else + snippet.errors.add(:snippet_files, 'have invalid data') + end + + snippet_error_response(snippet, 403) + end + def snippet_error_response(snippet, http_status) ServiceResponse.error( message: snippet.errors.full_messages.to_sentence, @@ -52,5 +71,13 @@ module Snippets message end + + def files_to_commit(snippet) + snippet_files.to_commit_actions.presence || build_actions_from_params(snippet) + end + + def build_actions_from_params(snippet) + raise NotImplementedError + end end end diff --git a/app/services/snippets/bulk_destroy_service.rb b/app/services/snippets/bulk_destroy_service.rb index d9cc383a5a6..a612d8f8dfc 100644 --- a/app/services/snippets/bulk_destroy_service.rb +++ b/app/services/snippets/bulk_destroy_service.rb @@ -14,12 +14,12 @@ module Snippets @snippets = snippets end - def execute + def execute(options = {}) return ServiceResponse.success(message: 'No snippets found.') if snippets.empty? - user_can_delete_snippets! + user_can_delete_snippets! unless options[:hard_delete] attempt_delete_repositories! - snippets.destroy_all # rubocop: disable DestroyAll + snippets.destroy_all # rubocop: disable Cop/DestroyAll ServiceResponse.success(message: 'Snippets were deleted.') rescue SnippetAccessError diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb index ed6da3a0ad0..7b477621da3 100644 --- a/app/services/snippets/create_service.rb +++ b/app/services/snippets/create_service.rb @@ -5,13 +5,15 @@ module Snippets def execute @snippet = build_from_params + return invalid_params_error(@snippet) unless valid_params? + unless visibility_allowed?(@snippet, @snippet.visibility_level) - return error_forbidden_visibility(@snippet) + return forbidden_visibility_error(@snippet) end @snippet.author = current_user - spam_check(@snippet, current_user) + spam_check(@snippet, current_user, action: :create) if save_and_commit UserAgentDetailService.new(@snippet, @request).create @@ -29,12 +31,21 @@ module Snippets def build_from_params if project - project.snippets.build(params) + project.snippets.build(create_params) else - PersonalSnippet.new(params) + PersonalSnippet.new(create_params) end end + # If the snippet_files param is present + # we need to fill content and file_name from + # the model + def create_params + return params if snippet_files.empty? + + params.merge(content: snippet_files[0].content, file_name: snippet_files[0].file_path) + end + def save_and_commit snippet_saved = @snippet.save @@ -75,19 +86,19 @@ module Snippets message: 'Initial commit' } - @snippet.snippet_repository.multi_files_action(current_user, snippet_files, commit_attrs) - end - - def snippet_files - [{ file_path: params[:file_name], content: params[:content] }] + @snippet.snippet_repository.multi_files_action(current_user, files_to_commit(@snippet), commit_attrs) end def move_temporary_files return unless @snippet.is_a?(PersonalSnippet) - uploaded_files.each do |file| + uploaded_assets.each do |file| FileMover.new(file, from_model: current_user, to_model: @snippet).execute end end + + def build_actions_from_params(_snippet) + [{ file_path: params[:file_name], content: params[:content] }] + end end end diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb index 250120c1c19..6cdc2c374da 100644 --- a/app/services/snippets/update_service.rb +++ b/app/services/snippets/update_service.rb @@ -7,12 +7,14 @@ module Snippets UpdateError = Class.new(StandardError) def execute(snippet) + return invalid_params_error(snippet) unless valid_params? + if visibility_changed?(snippet) && !visibility_allowed?(snippet, visibility_level) - return error_forbidden_visibility(snippet) + return forbidden_visibility_error(snippet) end - snippet.assign_attributes(params) - spam_check(snippet, current_user) + update_snippet_attributes(snippet) + spam_check(snippet, current_user, action: :update) if save_and_commit(snippet) Gitlab::UsageDataCounters::SnippetCounter.count(:update) @@ -29,6 +31,19 @@ module Snippets visibility_level && visibility_level.to_i != snippet.visibility_level end + def update_snippet_attributes(snippet) + # We can remove the following condition once + # https://gitlab.com/gitlab-org/gitlab/-/issues/217801 + # is implemented. + # Once we can perform different operations through this service + # we won't need to keep track of the `content` and `file_name` fields + if snippet_files.any? + params.merge!(content: snippet_files[0].content, file_name: snippet_files[0].file_path) + end + + snippet.assign_attributes(params) + end + def save_and_commit(snippet) return false unless snippet.save @@ -81,15 +96,7 @@ module Snippets message: 'Update snippet' } - snippet.snippet_repository.multi_files_action(current_user, snippet_files(snippet), commit_attrs) - end - - def snippet_files(snippet) - file_name_on_repo = snippet.file_name_on_repo - - [{ previous_path: file_name_on_repo, - file_path: params[:file_name] || file_name_on_repo, - content: params[:content] }] + snippet.snippet_repository.multi_files_action(current_user, files_to_commit(snippet), commit_attrs) end # Because we are removing repositories we don't want to remove @@ -101,7 +108,15 @@ module Snippets end def committable_attributes? - (params.stringify_keys.keys & COMMITTABLE_ATTRIBUTES).present? + (params.stringify_keys.keys & COMMITTABLE_ATTRIBUTES).present? || snippet_files.any? + end + + def build_actions_from_params(snippet) + file_name_on_repo = snippet.file_name_on_repo + + [{ previous_path: file_name_on_repo, + file_path: params[:file_name] || file_name_on_repo, + content: params[:content] }] end end end -- cgit v1.2.1