diff options
Diffstat (limited to 'app/services')
77 files changed, 1652 insertions, 710 deletions
diff --git a/app/services/akismet_service.rb b/app/services/akismet_service.rb index 76b9f1feda7..8e11a2a36a7 100644 --- a/app/services/akismet_service.rb +++ b/app/services/akismet_service.rb @@ -16,7 +16,7 @@ class AkismetService created_at: DateTime.now, author: owner.name, author_email: owner.email, - referrer: options[:referrer], + referrer: options[:referrer] } begin diff --git a/app/services/audit_event_service.rb b/app/services/audit_event_service.rb index 8a000585e89..5ad9a50687c 100644 --- a/app/services/audit_event_service.rb +++ b/app/services/audit_event_service.rb @@ -8,7 +8,7 @@ class AuditEventService with: @details[:with], target_id: @author.id, target_type: 'User', - target_details: @author.name, + target_details: @author.name } self diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index db82b8f6c30..5e151b0f044 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -17,6 +17,7 @@ module Auth end def self.full_access_token(*names) + names = names.flatten registry = Gitlab.config.registry token = JSONWebToken::RSAToken.new(registry.key) token.issuer = registry.issuer @@ -37,13 +38,13 @@ module Auth private def authorized_token(*accesses) - token = JSONWebToken::RSAToken.new(registry.key) - token.issuer = registry.issuer - token.audience = params[:service] - token.subject = current_user.try(:username) - token.expire_time = self.class.token_expire_at - token[:access] = accesses.compact - token + JSONWebToken::RSAToken.new(registry.key).tap do |token| + token.issuer = registry.issuer + token.audience = params[:service] + token.subject = current_user.try(:username) + token.expire_time = self.class.token_expire_at + token[:access] = accesses.compact + end end def scope @@ -55,20 +56,43 @@ module Auth def process_scope(scope) type, name, actions = scope.split(':', 3) actions = actions.split(',') + path = ContainerRegistry::Path.new(name) + return unless type == 'repository' - process_repository_access(type, name, actions) + process_repository_access(type, path, actions) end - def process_repository_access(type, name, actions) - requested_project = Project.find_by_full_path(name) + def process_repository_access(type, path, actions) + return unless path.valid? + + requested_project = path.repository_project + return unless requested_project actions = actions.select do |action| can_access?(requested_project, action) end - { type: type, name: name, actions: actions } if actions.present? + return unless actions.present? + + # At this point user/build is already authenticated. + # + ensure_container_repository!(path, actions) + + { type: type, name: path.to_s, actions: actions } + end + + ## + # Because we do not have two way communication with registry yet, + # we create a container repository image resource when push to the + # registry is successfuly authorized. + # + def ensure_container_repository!(path, actions) + return if path.has_repository? + return unless actions.include?('push') + + ContainerRepository.create_from_path!(path) end def can_access?(requested_project, requested_action) @@ -101,6 +125,11 @@ module Auth can?(current_user, :read_container_image, requested_project) end + ## + # We still support legacy pipeline triggers which do not have associated + # actor. New permissions model and new triggers are always associated with + # an actor, so this should be improved in 10.0 version of GitLab. + # def build_can_push?(requested_project) # Build can push only to the project from which it originates has_authentication_ability?(:build_create_container_image) && @@ -113,14 +142,11 @@ module Auth end def error(code, status:, message: '') - { - errors: [{ code: code, message: message }], - http_status: status - } + { errors: [{ code: code, message: message }], http_status: status } end def has_authentication_ability?(capability) - (@authentication_abilities || []).include?(capability) + @authentication_abilities.to_a.include?(capability) end end end diff --git a/app/services/base_service.rb b/app/services/base_service.rb index 745c2c4b681..a0cb00dba58 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -24,6 +24,10 @@ class BaseService Gitlab::AppLogger.info message end + def log_error(message) + Gitlab::AppLogger.error message + end + def system_hook_service SystemHooksService.new end diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index d5735f13c1e..ecabb2a48e4 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -38,7 +38,7 @@ module Boards attrs.merge!( add_label_ids: add_label_ids, remove_label_ids: remove_label_ids, - state_event: issue_state, + state_event: issue_state ) end @@ -61,7 +61,7 @@ module Boards if moving_to_list.movable? moving_from_list.label_id else - project.boards.joins(:lists).merge(List.movable).pluck(:label_id) + Label.on_project_boards(project.id).pluck(:label_id) end Array(label_ids).compact diff --git a/app/services/ci/create_pipeline_schedule_service.rb b/app/services/ci/create_pipeline_schedule_service.rb new file mode 100644 index 00000000000..cd40deb6187 --- /dev/null +++ b/app/services/ci/create_pipeline_schedule_service.rb @@ -0,0 +1,13 @@ +module Ci + class CreatePipelineScheduleService < BaseService + def execute + project.pipeline_schedules.create(pipeline_schedule_params) + end + + private + + def pipeline_schedule_params + params.merge(owner: current_user) + end + end +end diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 38a85e9fc42..1f6c1f4a7f6 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -2,7 +2,7 @@ module Ci class CreatePipelineService < BaseService attr_reader :pipeline - def execute(ignore_skip_ci: false, save_on_errors: true, trigger_request: nil) + def execute(ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil) @pipeline = Ci::Pipeline.new( project: project, ref: ref, @@ -10,7 +10,8 @@ module Ci before_sha: before_sha, tag: tag?, trigger_requests: Array(trigger_request), - user: current_user + user: current_user, + pipeline_schedule: schedule ) unless project.builds_enabled? @@ -46,13 +47,15 @@ module Ci end Ci::Pipeline.transaction do - pipeline.save + update_merge_requests_head_pipeline if pipeline.save Ci::CreatePipelineBuildsService .new(project, current_user) .execute(pipeline) end + cancel_pending_pipelines if project.auto_cancel_pending_pipelines? + pipeline.tap(&:process!) end @@ -63,6 +66,22 @@ module Ci pipeline.git_commit_message =~ /\[(ci[ _-]skip|skip[ _-]ci)\]/i end + def cancel_pending_pipelines + Gitlab::OptimisticLocking.retry_lock(auto_cancelable_pipelines) do |cancelables| + cancelables.find_each do |cancelable| + cancelable.auto_cancel_running(pipeline) + end + end + end + + def auto_cancelable_pipelines + project.pipelines + .where(ref: pipeline.ref) + .where.not(id: pipeline.id) + .where.not(sha: project.repository.sha_from_ref(pipeline.ref)) + .created_or_pending + end + def commit @commit ||= project.commit(origin_sha || origin_ref) end @@ -99,6 +118,11 @@ module Ci origin_sha && origin_sha != Gitlab::Git::BLANK_SHA end + def update_merge_requests_head_pipeline + MergeRequest.where(source_branch: @pipeline.ref, source_project: @pipeline.project). + update_all(head_pipeline_id: @pipeline.id) + end + def error(message, save: false) pipeline.errors.add(:base, message) pipeline.drop if save diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index dca5aa9f5d7..8362f01ddb8 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -5,9 +5,8 @@ module Ci pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref). execute(ignore_skip_ci: true, trigger_request: trigger_request) - if pipeline.persisted? - trigger_request - end + + trigger_request if pipeline.persisted? end end end diff --git a/app/services/ci/play_build_service.rb b/app/services/ci/play_build_service.rb new file mode 100644 index 00000000000..e24f48c2d16 --- /dev/null +++ b/app/services/ci/play_build_service.rb @@ -0,0 +1,17 @@ +module Ci + class PlayBuildService < ::BaseService + def execute(build) + unless can?(current_user, :update_build, build) + raise Gitlab::Access::AccessDeniedError + end + + # Try to enqueue the build, otherwise create a duplicate. + # + if build.enqueue + build.tap { |action| action.update(user: current_user) } + else + Ci::Build.retry(build, current_user) + end + end + end +end diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index 2935d00c075..55af193d717 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -5,7 +5,7 @@ module Ci def execute(pipeline) @pipeline = pipeline - ensure_created_builds! # TODO, remove me in 9.0 + update_retried new_builds = stage_indexes_of_created_builds.map do |index| @@ -52,7 +52,7 @@ module Ci when 'always' %w[success failed skipped] when 'manual' - %w[success] + %w[success skipped] else [] end @@ -74,17 +74,22 @@ module Ci pipeline.builds.created end - # This method is DEPRECATED and should be removed in 9.0. - # - # We need it to maintain backwards compatibility with previous versions - # when builds were not created within one transaction with the pipeline. - # - def ensure_created_builds! - return if created_builds.any? - - Ci::CreatePipelineBuildsService - .new(project, current_user) - .execute(pipeline) + # This method is for compatibility and data consistency and should be removed with 9.3 version of GitLab + # This replicates what is db/post_migrate/20170416103934_upate_retried_for_ci_build.rb + # and ensures that functionality will not be broken before migration is run + # this updates only when there are data that needs to be updated, there are two groups with no retried flag + def update_retried + # find the latest builds for each name + latest_statuses = pipeline.statuses.latest + .group(:name) + .having('count(*) > 1') + .pluck('max(id)', 'name') + + # mark builds that are retried + pipeline.statuses.latest + .where(name: latest_statuses.map(&:second)) + .where.not(id: latest_statuses.map(&:first)) + .update_all(retried: true) if latest_statuses.any? end end end diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index 89da05b72bb..f51e9fd1d54 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -6,7 +6,7 @@ module Ci description tag_list].freeze def execute(build) - reprocess(build).tap do |new_build| + reprocess!(build).tap do |new_build| build.pipeline.mark_as_processable_after_stage(build.stage_idx) new_build.enqueue! @@ -17,7 +17,7 @@ module Ci end end - def reprocess(build) + def reprocess!(build) unless can?(current_user, :update_build, build) raise Gitlab::Access::AccessDeniedError end @@ -28,7 +28,14 @@ module Ci attributes.push([:user, current_user]) - project.builds.create(Hash[attributes]) + Ci::Build.transaction do + # mark all other builds of that name as retried + build.pipeline.builds.latest + .where(name: build.name) + .update_all(retried: true) + + project.builds.create!(Hash[attributes]) + end end end end diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb index f72ddbf690c..c5a43869990 100644 --- a/app/services/ci/retry_pipeline_service.rb +++ b/app/services/ci/retry_pipeline_service.rb @@ -7,11 +7,11 @@ module Ci raise Gitlab::Access::AccessDeniedError end - pipeline.builds.latest.failed_or_canceled.find_each do |build| - next unless build.retryable? + pipeline.retryable_builds.find_each do |build| + next unless can?(current_user, :update_build, build) Ci::RetryBuildService.new(project, current_user) - .reprocess(build) + .reprocess!(build) end pipeline.builds.latest.skipped.find_each do |skipped| diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb index 42c72aba7dd..43c9a065fcf 100644 --- a/app/services/ci/stop_environments_service.rb +++ b/app/services/ci/stop_environments_service.rb @@ -5,10 +5,11 @@ module Ci def execute(branch_name) @ref = branch_name - return unless has_ref? + return unless @ref.present? environments.each do |environment| - next unless can?(current_user, :create_deployment, project) + next unless environment.stop_action? + next unless can?(current_user, :stop_environment, environment) environment.stop_with_action!(current_user) end @@ -16,13 +17,10 @@ module Ci private - def has_ref? - @ref.present? - end - def environments - @environments ||= - EnvironmentsFinder.new(project, current_user, ref: @ref, recently_updated: true).execute + @environments ||= EnvironmentsFinder + .new(project, current_user, ref: @ref, recently_updated: true) + .execute end end end diff --git a/app/services/cohorts_service.rb b/app/services/cohorts_service.rb new file mode 100644 index 00000000000..6781533af28 --- /dev/null +++ b/app/services/cohorts_service.rb @@ -0,0 +1,100 @@ +class CohortsService + MONTHS_INCLUDED = 12 + + def execute + { + months_included: MONTHS_INCLUDED, + cohorts: cohorts + } + end + + # Get an array of hashes that looks like: + # + # [ + # { + # registration_month: Date.new(2017, 3), + # activity_months: [3, 2, 1], + # total: 3 + # inactive: 0 + # }, + # etc. + # + # The `months` array is always from oldest to newest, so it's always + # non-strictly decreasing from left to right. + def cohorts + months = Array.new(MONTHS_INCLUDED) { |i| i.months.ago.beginning_of_month.to_date } + + Array.new(MONTHS_INCLUDED) do + registration_month = months.last + activity_months = running_totals(months, registration_month) + + # Even if no users registered in this month, we always want to have a + # value to fill in the table. + inactive = counts_by_month[[registration_month, nil]].to_i + + months.pop + + { + registration_month: registration_month, + activity_months: activity_months, + total: activity_months.first[:total], + inactive: inactive + } + end + end + + private + + # Calculate a running sum of active users, so users active in later months + # count as active in this month, too. Start with the most recent month first, + # for calculating the running totals, and then reverse for displaying in the + # table. + # + # Each month has a total, and a percentage of the overall total, as keys. + def running_totals(all_months, registration_month) + month_totals = + all_months + .map { |activity_month| counts_by_month[[registration_month, activity_month]] } + .reduce([]) { |result, total| result << result.last.to_i + total.to_i } + .reverse + + overall_total = month_totals.first + + month_totals.map do |total| + { total: total, percentage: total.zero? ? 0 : 100 * total / overall_total } + end + end + + # Get a hash that looks like: + # + # { + # [created_at_month, last_activity_on_month] => count, + # [created_at_month, last_activity_on_month_2] => count_2, + # # etc. + # } + # + # created_at_month can never be nil, but last_activity_on_month can (when a + # user has never logged in, just been created). This covers the last + # MONTHS_INCLUDED months. + def counts_by_month + @counts_by_month ||= + begin + created_at_month = column_to_date('created_at') + last_activity_on_month = column_to_date('last_activity_on') + + User + .where('created_at > ?', MONTHS_INCLUDED.months.ago.end_of_month) + .group(created_at_month, last_activity_on_month) + .reorder("#{created_at_month} ASC", "#{last_activity_on_month} ASC") + .count + end + end + + def column_to_date(column) + if Gitlab::Database.postgresql? + "CAST(DATE_TRUNC('month', #{column}) AS date)" + else + "STR_TO_DATE(DATE_FORMAT(#{column}, '%Y-%m-01'), '%Y-%m-%d')" + end + end +end diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb index 1297a792259..a48d6a976f0 100644 --- a/app/services/commits/change_service.rb +++ b/app/services/commits/change_service.rb @@ -1,69 +1,27 @@ module Commits - class ChangeService < ::BaseService - ValidationError = Class.new(StandardError) - ChangeError = Class.new(StandardError) + class ChangeService < Commits::CreateService + def initialize(*args) + super - def execute - @start_project = params[:start_project] || @project - @start_branch = params[:start_branch] - @target_branch = params[:target_branch] @commit = params[:commit] - - check_push_permissions - - commit - rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, - ValidationError, ChangeError => ex - error(ex.message) end private - def commit - raise NotImplementedError - end - def commit_change(action) raise NotImplementedError unless repository.respond_to?(action) - validate_target_branch if different_branch? - repository.public_send( action, current_user, @commit, - @target_branch, + @branch_name, start_project: @start_project, start_branch_name: @start_branch) - - success rescue Repository::CreateTreeError error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically. - A #{action.to_s.dasherize} may have already been performed with this #{@commit.change_type_title(current_user)}, or a more recent commit may have updated some of its content." + This #{@commit.change_type_title(current_user)} may already have been #{action.to_s.dasherize}ed, or a more recent commit may have updated some of its content." raise ChangeError, error_msg end - - def check_push_permissions - allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch) - - unless allowed - raise ValidationError.new('You are not allowed to push into this branch') - end - - true - end - - def validate_target_branch - result = ValidateNewBranchService.new(@project, current_user) - .execute(@target_branch) - - if result[:status] == :error - raise ChangeError, "There was an error creating the source branch: #{result[:message]}" - end - end - - def different_branch? - @start_branch != @target_branch || @start_project != @project - end end end diff --git a/app/services/commits/cherry_pick_service.rb b/app/services/commits/cherry_pick_service.rb index 605cca36f9c..320e229560d 100644 --- a/app/services/commits/cherry_pick_service.rb +++ b/app/services/commits/cherry_pick_service.rb @@ -1,6 +1,6 @@ module Commits class CherryPickService < ChangeService - def commit + def create_commit! commit_change(:cherry_pick) end end diff --git a/app/services/commits/create_service.rb b/app/services/commits/create_service.rb new file mode 100644 index 00000000000..c58f04a252b --- /dev/null +++ b/app/services/commits/create_service.rb @@ -0,0 +1,74 @@ +module Commits + class CreateService < ::BaseService + ValidationError = Class.new(StandardError) + ChangeError = Class.new(StandardError) + + def initialize(*args) + super + + @start_project = params[:start_project] || @project + @start_branch = params[:start_branch] + @branch_name = params[:branch_name] + end + + def execute + validate! + + new_commit = create_commit! + + success(result: new_commit) + rescue ValidationError, ChangeError, Gitlab::Git::Index::IndexError, Repository::CommitError, GitHooksService::PreReceiveError => ex + error(ex.message) + end + + private + + def create_commit! + raise NotImplementedError + end + + def raise_error(message) + raise ValidationError, message + end + + def different_branch? + @start_branch != @branch_name || @start_project != @project + end + + def validate! + validate_permissions! + validate_on_branch! + validate_branch_existance! + + validate_new_branch_name! if different_branch? + end + + def validate_permissions! + allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@branch_name) + + unless allowed + raise_error("You are not allowed to push into this branch") + end + end + + def validate_on_branch! + if !@start_project.empty_repo? && !@start_project.repository.branch_exists?(@start_branch) + raise_error('You can only create or edit files when you are on a branch') + end + end + + def validate_branch_existance! + if !project.empty_repo? && different_branch? && repository.branch_exists?(@branch_name) + raise_error("A branch called '#{@branch_name}' already exists. Switch to that branch in order to make changes") + end + end + + def validate_new_branch_name! + result = ValidateNewBranchService.new(project, current_user).execute(@branch_name) + + if result[:status] == :error + raise_error("Something went wrong when we tried to create '#{@branch_name}' for you: #{result[:message]}") + end + end + end +end diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb index addd55cb32f..dc27399e047 100644 --- a/app/services/commits/revert_service.rb +++ b/app/services/commits/revert_service.rb @@ -1,6 +1,6 @@ module Commits class RevertService < ChangeService - def commit + def create_commit! commit_change(:revert) end end diff --git a/app/services/concerns/issues/resolve_discussions.rb b/app/services/concerns/issues/resolve_discussions.rb index 297c7d696c3..910a2a15e5d 100644 --- a/app/services/concerns/issues/resolve_discussions.rb +++ b/app/services/concerns/issues/resolve_discussions.rb @@ -21,11 +21,11 @@ module Issues @discussions_to_resolve ||= if discussion_to_resolve_id discussion_or_nil = merge_request_to_resolve_discussions_of - .find_diff_discussion(discussion_to_resolve_id) + .find_discussion(discussion_to_resolve_id) Array(discussion_or_nil) else merge_request_to_resolve_discussions_of - .resolvable_discussions + .discussions_to_be_resolved end end end diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb index 11a045f4c31..64b3c0118fb 100644 --- a/app/services/delete_branch_service.rb +++ b/app/services/delete_branch_service.rb @@ -3,22 +3,14 @@ class DeleteBranchService < BaseService repository = project.repository branch = repository.find_branch(branch_name) - unless branch - return error('No such branch', 404) - end - - if branch_name == repository.root_ref - return error('Cannot remove HEAD branch', 405) - end - - if project.protected_branch?(branch_name) - return error('Protected branch cant be removed', 405) - end - unless current_user.can?(:push_code, project) return error('You dont have push access to repo', 405) end + unless branch + return error('No such branch', 404) + end + if repository.rm_branch(current_user, branch_name) success('Branch was removed') else diff --git a/app/services/delete_merged_branches_service.rb b/app/services/delete_merged_branches_service.rb index 1b5623baebe..3b611588466 100644 --- a/app/services/delete_merged_branches_service.rb +++ b/app/services/delete_merged_branches_service.rb @@ -8,9 +8,20 @@ class DeleteMergedBranchesService < BaseService branches = project.repository.branch_names branches = branches.select { |branch| project.repository.merged_to_root_ref?(branch) } + # Prevent deletion of branches relevant to open merge requests + branches -= merge_request_branch_names branches.each do |branch| DeleteBranchService.new(project, current_user).execute(branch) end end + + private + + def merge_request_branch_names + # reorder(nil) is necessary for SELECT DISTINCT because default scope adds an ORDER BY + source_names = project.origin_merge_requests.opened.reorder(nil).uniq.pluck(:source_branch) + target_names = project.merge_requests.opened.reorder(nil).uniq.pluck(:target_branch) + (source_names + target_names).uniq + end end diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index e24cc66e0fe..0f3a485a3fd 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -72,6 +72,8 @@ class EventCreateService def push(project, current_user, push_data) create_event(project, current_user, Event::PUSHED, data: push_data) + + Users::ActivityService.new(current_user, 'push').execute end private diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index c8a60422bf4..38231f66009 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -1,79 +1,17 @@ module Files - class BaseService < ::BaseService - ValidationError = Class.new(StandardError) - - def execute - @start_project = params[:start_project] || @project - @start_branch = params[:start_branch] - @target_branch = params[:target_branch] + class BaseService < Commits::CreateService + def initialize(*args) + super + @author_email = params[:author_email] + @author_name = params[:author_name] @commit_message = params[:commit_message] - @file_path = params[:file_path] - @previous_path = params[:previous_path] - @file_content = if params[:file_content_encoding] == 'base64' - Base64.decode64(params[:file_content]) - else - params[:file_content] - end - @last_commit_sha = params[:last_commit_sha] - @author_email = params[:author_email] - @author_name = params[:author_name] - - # Validate parameters - validate - - # Create new branch if it different from start_branch - validate_target_branch if different_branch? - - result = commit - if result - success(result: result) - else - error('Something went wrong. Your changes were not committed') - end - rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, ValidationError => ex - error(ex.message) - end - - private - - def different_branch? - @start_branch != @target_branch || @start_project != @project - end - - def file_has_changed? - return false unless @last_commit_sha && last_commit - - @last_commit_sha != last_commit.sha - end - - def raise_error(message) - raise ValidationError.new(message) - end - - def validate - allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch) - - unless allowed - raise_error("You are not allowed to push into this branch") - end - - if !@start_project.empty_repo? && !@start_project.repository.branch_exists?(@start_branch) - raise ValidationError, 'You can only create or edit files when you are on a branch' - end - - if !project.empty_repo? && different_branch? && repository.branch_exists?(@branch_name) - raise ValidationError, "A branch called #{@branch_name} already exists. Switch to that branch in order to make changes" - end - end - def validate_target_branch - result = ValidateNewBranchService.new(project, current_user). - execute(@target_branch) + @file_path = params[:file_path] + @previous_path = params[:previous_path] - if result[:status] == :error - raise_error("Something went wrong when we tried to create #{@target_branch} for you: #{result[:message]}") - end + @file_content = params[:file_content] + @file_content = Base64.decode64(@file_content) if params[:file_content_encoding] == 'base64' end end end diff --git a/app/services/files/create_dir_service.rb b/app/services/files/create_dir_service.rb index 083ffdc634c..8ecac6115bd 100644 --- a/app/services/files/create_dir_service.rb +++ b/app/services/files/create_dir_service.rb @@ -1,26 +1,15 @@ module Files class CreateDirService < Files::BaseService - def commit + def create_commit! repository.create_dir( current_user, @file_path, message: @commit_message, - branch_name: @target_branch, + branch_name: @branch_name, author_email: @author_email, author_name: @author_name, start_project: @start_project, start_branch_name: @start_branch) end - - def validate - super - - unless @file_path =~ Gitlab::Regex.file_path_regex - raise_error( - 'Your changes could not be committed, because the file path ' + - Gitlab::Regex.file_path_regex_message - ) - end - end end end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 65b5537fb68..00a8dcf0934 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -1,48 +1,16 @@ module Files class CreateService < Files::BaseService - def commit + def create_commit! repository.create_file( current_user, @file_path, @file_content, message: @commit_message, - branch_name: @target_branch, + branch_name: @branch_name, author_email: @author_email, author_name: @author_name, start_project: @start_project, start_branch_name: @start_branch) end - - def validate - super - - if @file_content.nil? - raise_error("You must provide content.") - end - - if @file_path =~ Gitlab::Regex.directory_traversal_regex - raise_error( - 'Your changes could not be committed, because the file name ' + - Gitlab::Regex.directory_traversal_regex_message - ) - end - - unless @file_path =~ Gitlab::Regex.file_path_regex - raise_error( - 'Your changes could not be committed, because the file name ' + - Gitlab::Regex.file_path_regex_message - ) - end - - unless project.empty_repo? - @file_path.slice!(0) if @file_path.start_with?('/') - - blob = repository.blob_at_branch(@start_branch, @file_path) - - if blob - raise_error('Your changes could not be committed because a file with the same name already exists') - end - end - end end end diff --git a/app/services/files/destroy_service.rb b/app/services/files/delete_service.rb index e294659bc98..7952e5c95d4 100644 --- a/app/services/files/destroy_service.rb +++ b/app/services/files/delete_service.rb @@ -1,11 +1,11 @@ module Files - class DestroyService < Files::BaseService - def commit + class DeleteService < Files::BaseService + def create_commit! repository.delete_file( current_user, @file_path, message: @commit_message, - branch_name: @target_branch, + branch_name: @branch_name, author_email: @author_email, author_name: @author_name, start_project: @start_project, diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb index 700f9f4f6f0..bfacc462847 100644 --- a/app/services/files/multi_service.rb +++ b/app/services/files/multi_service.rb @@ -1,14 +1,10 @@ module Files class MultiService < Files::BaseService - FileChangedError = Class.new(StandardError) - - ACTIONS = %w[create update delete move].freeze - - def commit + def create_commit! repository.multi_action( user: current_user, message: @commit_message, - branch_name: @target_branch, + branch_name: @branch_name, actions: params[:actions], author_email: @author_email, author_name: @author_name, @@ -19,122 +15,17 @@ module Files private - def validate + def validate! super - params[:actions].each_with_index do |action, index| - if ACTIONS.include?(action[:action].to_s) - action[:action] = action[:action].to_sym - else - raise_error("Unknown action type `#{action[:action]}`.") - end - - unless action[:file_path].present? - raise_error("You must specify a file_path.") - end - - action[:file_path].slice!(0) if action[:file_path] && action[:file_path].start_with?('/') - action[:previous_path].slice!(0) if action[:previous_path] && action[:previous_path].start_with?('/') - - regex_check(action[:file_path]) - regex_check(action[:previous_path]) if action[:previous_path] - - if project.empty_repo? && action[:action] != :create - raise_error("No files to #{action[:action]}.") - end - - validate_file_exists(action) - - case action[:action] - when :create - validate_create(action) - when :update - validate_update(action) - when :delete - validate_delete(action) - when :move - validate_move(action, index) - end - end - end - - def validate_file_exists(action) - return if action[:action] == :create - - file_path = action[:file_path] - file_path = action[:previous_path] if action[:action] == :move - - blob = repository.blob_at_branch(params[:branch], file_path) - - unless blob - raise_error("File to be #{action[:action]}d `#{file_path}` does not exist.") + params[:actions].each do |action| + validate_action!(action) end end - def last_commit - Gitlab::Git::Commit.last_for_path(repository, @start_branch, @file_path) - end - - def regex_check(file) - if file =~ Gitlab::Regex.directory_traversal_regex - raise_error( - 'Your changes could not be committed, because the file name, `' + - file + - '` ' + - Gitlab::Regex.directory_traversal_regex_message - ) - end - - unless file =~ Gitlab::Regex.file_path_regex - raise_error( - 'Your changes could not be committed, because the file name, `' + - file + - '` ' + - Gitlab::Regex.file_path_regex_message - ) - end - end - - def validate_create(action) - return if project.empty_repo? - - if repository.blob_at_branch(params[:branch], action[:file_path]) - raise_error("Your changes could not be committed because a file with the name `#{action[:file_path]}` already exists.") - end - - if action[:content].nil? - raise_error("You must provide content.") - end - end - - def validate_update(action) - if action[:content].nil? - raise_error("You must provide content.") - end - - if file_has_changed? - raise FileChangedError.new("You are attempting to update a file `#{action[:file_path]}` that has changed since you started editing it.") - end - end - - def validate_delete(action) - end - - def validate_move(action, index) - if action[:previous_path].nil? - raise_error("You must supply the original file path when moving file `#{action[:file_path]}`.") - end - - blob = repository.blob_at_branch(params[:branch], action[:file_path]) - - if blob - raise_error("Move destination `#{action[:file_path]}` already exists.") - end - - if action[:content].nil? - blob = repository.blob_at_branch(params[:branch], action[:previous_path]) - blob.load_all_data!(repository) if blob.truncated? - params[:actions][index][:content] = blob.data + def validate_action!(action) + unless Gitlab::Git::Index::ACTIONS.include?(action[:action].to_s) + raise_error("Unknown action '#{action[:action]}'") end end end diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index fbbab97632e..f23a9f6d57c 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -2,10 +2,16 @@ module Files class UpdateService < Files::BaseService FileChangedError = Class.new(StandardError) - def commit + def initialize(*args) + super + + @last_commit_sha = params[:last_commit_sha] + end + + def create_commit! repository.update_file(current_user, @file_path, @file_content, message: @commit_message, - branch_name: @target_branch, + branch_name: @branch_name, previous_path: @previous_path, author_email: @author_email, author_name: @author_name, @@ -15,21 +21,23 @@ module Files private - def validate - super - - if @file_content.nil? - raise_error("You must provide content.") - end + def file_has_changed? + return false unless @last_commit_sha && last_commit - if file_has_changed? - raise FileChangedError.new("You are attempting to update a file that has changed since you started editing it.") - end + @last_commit_sha != last_commit.sha end def last_commit @last_commit ||= Gitlab::Git::Commit. last_for_path(@start_project.repository, @start_branch, @file_path) end + + def validate! + super + + if file_has_changed? + raise FileChangedError, "You are attempting to update a file that has changed since you started editing it." + end + end end end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index bc7431c89a8..d22236b961b 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -67,7 +67,7 @@ class GitPushService < BaseService paths = Set.new @push_commits.each do |commit| - commit.raw_diffs(deltas_only: true).each do |diff| + commit.raw_deltas.each do |diff| paths << diff.new_path end end @@ -85,8 +85,10 @@ class GitPushService < BaseService default = is_default_branch? push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit| - ProcessCommitWorker. - perform_async(project.id, current_user.id, commit.to_hash, default) + if commit.matches_cross_reference_regex? + ProcessCommitWorker. + perform_async(project.id, current_user.id, commit.to_hash, default) + end end end @@ -127,7 +129,7 @@ class GitPushService < BaseService project.change_head(branch_name) # Set protection on the default branch if configured - if current_application_settings.default_branch_protection != PROTECTION_NONE && !@project.protected_branch?(@project.default_branch) + if current_application_settings.default_branch_protection != PROTECTION_NONE && !ProtectedBranch.protected?(@project, @project.default_branch) params = { name: @project.default_branch, diff --git a/app/services/issuable/bulk_update_service.rb b/app/services/issuable/bulk_update_service.rb index 60891cbb255..5d42a89fced 100644 --- a/app/services/issuable/bulk_update_service.rb +++ b/app/services/issuable/bulk_update_service.rb @@ -7,10 +7,14 @@ module Issuable ids = params.delete(:issuable_ids).split(",") items = model_class.where(id: ids) - %i(state_event milestone_id assignee_id add_label_ids remove_label_ids subscription_event).each do |key| + permitted_attrs(type).each do |key| params.delete(key) unless params[key].present? end + if params[:assignee_ids] == [IssuableFinder::NONE.to_s] + params[:assignee_ids] = [] + end + items.each do |issuable| next unless can?(current_user, :"update_#{type}", issuable) @@ -22,5 +26,17 @@ module Issuable success: !items.count.zero? } end + + private + + def permitted_attrs(type) + attrs = %i(state_event milestone_id assignee_id assignee_ids add_label_ids remove_label_ids subscription_event) + + if type == 'issue' + attrs.push(:assignee_ids) + else + attrs.push(:assignee_id) + end + end end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index b071a398481..e94ab3e64db 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -1,11 +1,6 @@ class IssuableBaseService < BaseService private - def create_assignee_note(issuable) - SystemNoteService.change_assignee( - issuable, issuable.project, current_user, issuable.assignee) - end - def create_milestone_note(issuable) SystemNoteService.change_milestone( issuable, issuable.project, current_user, issuable.milestone) @@ -24,6 +19,10 @@ class IssuableBaseService < BaseService issuable, issuable.project, current_user, old_title) end + def create_description_change_note(issuable) + SystemNoteService.change_description(issuable, issuable.project, current_user) + end + def create_branch_change_note(issuable, branch_type, old_branch, new_branch) SystemNoteService.change_branch( issuable, issuable.project, current_user, branch_type, @@ -53,6 +52,7 @@ class IssuableBaseService < BaseService params.delete(:add_label_ids) params.delete(:remove_label_ids) params.delete(:label_ids) + params.delete(:assignee_ids) params.delete(:assignee_id) params.delete(:due_date) end @@ -77,7 +77,7 @@ class IssuableBaseService < BaseService def assignee_can_read?(issuable, assignee_id) new_assignee = User.find_by_id(assignee_id) - return false unless new_assignee.present? + return false unless new_assignee ability_name = :"read_#{issuable.to_ability_name}" resource = issuable.persisted? ? issuable : project @@ -178,6 +178,7 @@ class IssuableBaseService < BaseService after_create(issuable) issuable.create_cross_references!(current_user) execute_hooks(issuable) + invalidate_cache_counts(issuable.assignees, issuable) end issuable @@ -207,6 +208,7 @@ class IssuableBaseService < BaseService filter_params(issuable) old_labels = issuable.labels.to_a old_mentioned_users = issuable.mentioned_users.to_a + old_assignees = issuable.assignees.to_a label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids) params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids) @@ -214,6 +216,10 @@ class IssuableBaseService < BaseService if issuable.changed? || params.present? issuable.assign_attributes(params.merge(updated_by: current_user)) + if has_title_or_description_changed?(issuable) + issuable.assign_attributes(last_edited_at: Time.now, last_edited_by: current_user) + end + before_update(issuable) if issuable.with_transaction_returning_status { issuable.save } @@ -222,7 +228,18 @@ class IssuableBaseService < BaseService handle_common_system_notes(issuable, old_labels: old_labels) end - handle_changes(issuable, old_labels: old_labels, old_mentioned_users: old_mentioned_users) + handle_changes( + issuable, + old_labels: old_labels, + old_mentioned_users: old_mentioned_users, + old_assignees: old_assignees + ) + + if old_assignees != issuable.assignees + assignees = old_assignees + issuable.assignees.to_a + invalidate_cache_counts(assignees.compact, issuable) + end + after_update(issuable) issuable.create_new_cross_references!(current_user) execute_hooks(issuable, 'update') @@ -236,6 +253,10 @@ class IssuableBaseService < BaseService old_label_ids.sort != new_label_ids.sort end + def has_title_or_description_changed?(issuable) + issuable.title_changed? || issuable.description_changed? + end + def change_state(issuable) case params.delete(:state_event) when 'reopen' @@ -272,7 +293,7 @@ class IssuableBaseService < BaseService end end - def has_changes?(issuable, old_labels: []) + def has_changes?(issuable, old_labels: [], old_assignees: []) valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch] attrs_changed = valid_attrs.any? do |attr| @@ -281,7 +302,9 @@ class IssuableBaseService < BaseService labels_changed = issuable.labels != old_labels - attrs_changed || labels_changed + assignees_changed = issuable.assignees != old_assignees + + attrs_changed || labels_changed || assignees_changed end def handle_common_system_notes(issuable, old_labels: []) @@ -289,6 +312,10 @@ class IssuableBaseService < BaseService create_title_change_note(issuable, issuable.previous_changes['title'].first) end + if issuable.previous_changes.include?('description') + create_description_change_note(issuable) + end + if issuable.previous_changes.include?('description') && issuable.tasks? create_task_status_note(issuable) end @@ -303,4 +330,10 @@ class IssuableBaseService < BaseService create_labels_note(issuable, old_labels) if issuable.labels != old_labels end + + def invalidate_cache_counts(users, issuable) + users.each do |user| + user.public_send("invalidate_#{issuable.model_name.singular}_cache_counts") + end + end end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index ee1b40db718..34199eb5d13 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -9,11 +9,33 @@ module Issues private + def create_assignee_note(issue, old_assignees) + SystemNoteService.change_issue_assignees( + issue, issue.project, current_user, old_assignees) + end + def execute_hooks(issue, action = 'open') issue_data = hook_data(issue, action) hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks issue.project.execute_hooks(issue_data, hooks_scope) issue.project.execute_services(issue_data, hooks_scope) end + + def filter_assignee(issuable) + return if params[:assignee_ids].blank? + + # The number of assignees is limited by one for GitLab CE + params[:assignee_ids] = params[:assignee_ids][0, 1] + + assignee_ids = params[:assignee_ids].select { |assignee_id| assignee_can_read?(issuable, assignee_id) } + + if params[:assignee_ids].map(&:to_s) == [IssuableFinder::NONE] + params[:assignee_ids] = [] + elsif assignee_ids.any? + params[:assignee_ids] = assignee_ids + else + params.delete(:assignee_ids) + end + end end end diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb index 77bced4bd5c..3a4f7b159f1 100644 --- a/app/services/issues/build_service.rb +++ b/app/services/issues/build_service.rb @@ -35,14 +35,19 @@ module Issues end def item_for_discussion(discussion) - first_note = discussion.first_note_to_resolve || discussion.first_note + first_note_to_resolve = discussion.first_note_to_resolve || discussion.first_note + + is_very_first_note = first_note_to_resolve == discussion.first_note + action = is_very_first_note ? "started" : "commented on" + + note_url = Gitlab::UrlBuilder.build(first_note_to_resolve) + other_note_count = discussion.notes.size - 1 - note_url = Gitlab::UrlBuilder.build(first_note) - discussion_info = "- [ ] #{first_note.author.to_reference} commented on a [discussion](#{note_url}): " + discussion_info = "- [ ] #{first_note_to_resolve.author.to_reference} #{action} a [discussion](#{note_url}): " discussion_info << " (+#{other_note_count} #{'comment'.pluralize(other_note_count)})" if other_note_count > 0 - note_without_block_quotes = Banzai::Filter::BlockquoteFenceFilter.new(first_note.note).call + note_without_block_quotes = Banzai::Filter::BlockquoteFenceFilter.new(first_note_to_resolve.note).call spaces = ' ' * 4 quote = note_without_block_quotes.lines.map { |line| "#{spaces}> #{line}" }.join diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index b7fe5cb168b..cd9f9a4a16e 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -12,8 +12,12 @@ module Issues spam_check(issue, current_user) end - def handle_changes(issue, old_labels: [], old_mentioned_users: []) - if has_changes?(issue, old_labels: old_labels) + def handle_changes(issue, options) + old_labels = options[:old_labels] || [] + old_mentioned_users = options[:old_mentioned_users] || [] + old_assignees = options[:old_assignees] || [] + + if has_changes?(issue, old_labels: old_labels, old_assignees: old_assignees) todo_service.mark_pending_todos_as_done(issue, current_user) end @@ -26,9 +30,9 @@ module Issues create_milestone_note(issue) end - if issue.previous_changes.include?('assignee_id') - create_assignee_note(issue) - notification_service.reassigned_issue(issue, current_user) + if issue.assignees != old_assignees + create_assignee_note(issue, old_assignees) + notification_service.reassigned_issue(issue, current_user, old_assignees) todo_service.reassigned_issue(issue, current_user) end diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb index b7a244c2029..f846d72498f 100644 --- a/app/services/members/authorized_destroy_service.rb +++ b/app/services/members/authorized_destroy_service.rb @@ -9,7 +9,11 @@ module Members def execute return false if member.is_a?(GroupMember) && member.source.last_owner?(member.user) - member.destroy + Member.transaction do + unassign_issues_and_merge_requests(member) unless member.invite? + + member.destroy + end if member.request? && member.user != user notification_service.decline_access_request(member) @@ -17,5 +21,40 @@ module Members member end + + private + + def unassign_issues_and_merge_requests(member) + if member.is_a?(GroupMember) + issues = Issue.unscoped.select(1). + joins(:project). + where('issues.id = issue_assignees.issue_id AND projects.namespace_id = ?', member.source_id) + + # DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...) + IssueAssignee.unscoped. + where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues). + delete_all + + MergeRequestsFinder.new(user, group_id: member.source_id, assignee_id: member.user_id). + execute. + update_all(assignee_id: nil) + else + project = member.source + + # SELECT 1 FROM issues WHERE issues.id = issue_assignees.issue_id AND issues.project_id = X + issues = Issue.unscoped.select(1). + where('issues.id = issue_assignees.issue_id'). + where(project_id: project.id) + + # DELETE FROM issue_assignees WHERE user_id = X AND EXISTS (...) + IssueAssignee.unscoped. + where('user_id = :user_id AND EXISTS (:sub)', user_id: member.user_id, sub: issues). + delete_all + + project.merge_requests.opened.assigned_to(member.user).update_all(assignee_id: nil) + end + + member.user.invalidate_cache_counts + end end end diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index e4b24ccef92..3a58f6c065d 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -1,9 +1,15 @@ module Members class CreateService < BaseService + def initialize(source, current_user, params = {}) + @source = source + @current_user = current_user + @params = params + end + def execute return false if params[:user_ids].blank? - project.team.add_users( + @source.add_users( params[:user_ids].split(','), params[:access_level], expires_at: params[:expires_at], diff --git a/app/services/merge_requests/assign_issues_service.rb b/app/services/merge_requests/assign_issues_service.rb index 066efa1acc3..8c6c4841020 100644 --- a/app/services/merge_requests/assign_issues_service.rb +++ b/app/services/merge_requests/assign_issues_service.rb @@ -4,7 +4,7 @@ module MergeRequests @assignable_issues ||= begin if current_user == merge_request.author closes_issues.select do |issue| - !issue.is_a?(ExternalIssue) && !issue.assignee_id? && can?(current_user, :admin_issue, issue) + !issue.is_a?(ExternalIssue) && !issue.assignees.present? && can?(current_user, :admin_issue, issue) end else [] @@ -14,7 +14,7 @@ module MergeRequests def execute assignable_issues.each do |issue| - Issues::UpdateService.new(issue.project, current_user, assignee_id: current_user.id).execute(issue) + Issues::UpdateService.new(issue.project, current_user, assignee_ids: [current_user.id]).execute(issue) end { diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 5a53b973059..3542a41ac83 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -38,8 +38,13 @@ module MergeRequests private + def create_assignee_note(merge_request) + SystemNoteService.change_assignee( + merge_request, merge_request.project, current_user, merge_request.assignee) + end + # Returns all origin and fork merge requests from `@project` satisfying passed arguments. - def merge_requests_for(source_branch, mr_states: [:opened]) + def merge_requests_for(source_branch, mr_states: [:opened, :reopened]) MergeRequest .with_state(mr_states) .where(source_branch: source_branch, source_project_id: @project.id) diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index fdce542bd9e..bc0e7ad4e39 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -21,12 +21,14 @@ module MergeRequests delegate :target_branch, :source_branch, :source_project, :target_project, :compare_commits, :wip_title, :description, :errors, to: :merge_request def find_source_project - source_project || project + return source_project if source_project.present? && can?(current_user, :read_project, source_project) + + project end def find_target_project return target_project if target_project.present? && can?(current_user, :read_project, target_project) - project.forked_from_project || project + project.default_merge_request_target end def find_target_branch diff --git a/app/services/merge_requests/conflicts/base_service.rb b/app/services/merge_requests/conflicts/base_service.rb new file mode 100644 index 00000000000..b50875347d9 --- /dev/null +++ b/app/services/merge_requests/conflicts/base_service.rb @@ -0,0 +1,11 @@ +module MergeRequests + module Conflicts + class BaseService + attr_reader :merge_request + + def initialize(merge_request) + @merge_request = merge_request + end + end + end +end diff --git a/app/services/merge_requests/conflicts/list_service.rb b/app/services/merge_requests/conflicts/list_service.rb new file mode 100644 index 00000000000..9835606812c --- /dev/null +++ b/app/services/merge_requests/conflicts/list_service.rb @@ -0,0 +1,36 @@ +module MergeRequests + module Conflicts + class ListService < MergeRequests::Conflicts::BaseService + delegate :file_for_path, :to_json, to: :conflicts + + def can_be_resolved_by?(user) + return false unless merge_request.source_project + + access = ::Gitlab::UserAccess.new(user, project: merge_request.source_project) + access.can_push_to_branch?(merge_request.source_branch) + end + + def can_be_resolved_in_ui? + return @conflicts_can_be_resolved_in_ui if defined?(@conflicts_can_be_resolved_in_ui) + + return @conflicts_can_be_resolved_in_ui = false unless merge_request.cannot_be_merged? + return @conflicts_can_be_resolved_in_ui = false unless merge_request.has_complete_diff_refs? + return @conflicts_can_be_resolved_in_ui = false if merge_request.branch_missing? + + begin + # Try to parse each conflict. If the MR's mergeable status hasn't been + # updated, ensure that we don't say there are conflicts to resolve + # when there are no conflict files. + conflicts.files.each(&:lines) + @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0 + rescue Rugged::OdbError, Gitlab::Conflict::Parser::UnresolvableError, Gitlab::Conflict::FileCollection::ConflictSideMissing + @conflicts_can_be_resolved_in_ui = false + end + end + + def conflicts + @conflicts ||= Gitlab::Conflict::FileCollection.read_only(merge_request) + end + end + end +end diff --git a/app/services/merge_requests/conflicts/resolve_service.rb b/app/services/merge_requests/conflicts/resolve_service.rb new file mode 100644 index 00000000000..d74a82effd6 --- /dev/null +++ b/app/services/merge_requests/conflicts/resolve_service.rb @@ -0,0 +1,53 @@ +module MergeRequests + module Conflicts + class ResolveService < MergeRequests::Conflicts::BaseService + MissingFiles = Class.new(Gitlab::Conflict::ResolutionError) + + def execute(current_user, params) + rugged = merge_request.source_project.repository.rugged + + Gitlab::Conflict::FileCollection.for_resolution(merge_request) do |conflicts_for_resolution| + merge_index = conflicts_for_resolution.merge_index + + params[:files].each do |file_params| + conflict_file = conflicts_for_resolution.file_for_path(file_params[:old_path], file_params[:new_path]) + + write_resolved_file_to_index(merge_index, rugged, conflict_file, file_params) + end + + unless merge_index.conflicts.empty? + missing_files = merge_index.conflicts.map { |file| file[:ours][:path] } + + raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}" + end + + commit_params = { + message: params[:commit_message] || conflicts_for_resolution.default_commit_message, + parents: [conflicts_for_resolution.our_commit, conflicts_for_resolution.their_commit].map(&:oid), + tree: merge_index.write_tree(rugged) + } + + conflicts_for_resolution. + project. + repository. + resolve_conflicts(current_user, merge_request.source_branch, commit_params) + end + end + + private + + def write_resolved_file_to_index(merge_index, rugged, file, params) + new_file = if params[:sections] + file.resolve_lines(params[:sections]).map(&:text).join("\n") + elsif params[:content] + file.resolve_content(params[:content]) + end + + our_path = file.our_path + + merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode) + merge_index.conflict_remove(our_path) + end + end + end +end diff --git a/app/services/merge_requests/create_from_issue_service.rb b/app/services/merge_requests/create_from_issue_service.rb new file mode 100644 index 00000000000..738cedbaed7 --- /dev/null +++ b/app/services/merge_requests/create_from_issue_service.rb @@ -0,0 +1,54 @@ +module MergeRequests + class CreateFromIssueService < MergeRequests::CreateService + def execute + return error('Invalid issue iid') unless issue_iid.present? && issue.present? + + result = CreateBranchService.new(project, current_user).execute(branch_name, ref) + return result if result[:status] == :error + + SystemNoteService.new_issue_branch(issue, project, current_user, branch_name) + + new_merge_request = create(merge_request) + + if new_merge_request.valid? + success(new_merge_request) + else + error(new_merge_request.errors) + end + end + + private + + def issue_iid + @isssue_iid ||= params.delete(:issue_iid) + end + + def issue + @issue ||= IssuesFinder.new(current_user, project_id: project.id).find_by(iid: issue_iid) + end + + def branch_name + @branch_name ||= issue.to_branch_name + end + + def ref + project.default_branch || 'master' + end + + def merge_request + MergeRequests::BuildService.new(project, current_user, merge_request_params).execute + end + + def merge_request_params + { + source_project_id: project.id, + source_branch: branch_name, + target_project_id: project.id + } + end + + def success(merge_request) + super().merge(merge_request: merge_request) + end + end +end diff --git a/app/services/merge_requests/resolve_service.rb b/app/services/merge_requests/resolve_service.rb deleted file mode 100644 index 82cd89d9a0b..00000000000 --- a/app/services/merge_requests/resolve_service.rb +++ /dev/null @@ -1,65 +0,0 @@ -module MergeRequests - class ResolveService < MergeRequests::BaseService - MissingFiles = Class.new(Gitlab::Conflict::ResolutionError) - - attr_accessor :conflicts, :rugged, :merge_index, :merge_request - - def execute(merge_request) - @conflicts = merge_request.conflicts - @rugged = project.repository.rugged - @merge_index = conflicts.merge_index - @merge_request = merge_request - - fetch_their_commit! - - params[:files].each do |file_params| - conflict_file = merge_request.conflicts.file_for_path(file_params[:old_path], file_params[:new_path]) - - write_resolved_file_to_index(conflict_file, file_params) - end - - unless merge_index.conflicts.empty? - missing_files = merge_index.conflicts.map { |file| file[:ours][:path] } - - raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}" - end - - commit_params = { - message: params[:commit_message] || conflicts.default_commit_message, - parents: [conflicts.our_commit, conflicts.their_commit].map(&:oid), - tree: merge_index.write_tree(rugged) - } - - project.repository.resolve_conflicts(current_user, merge_request.source_branch, commit_params) - end - - def write_resolved_file_to_index(file, params) - new_file = if params[:sections] - file.resolve_lines(params[:sections]).map(&:text).join("\n") - elsif params[:content] - file.resolve_content(params[:content]) - end - - our_path = file.our_path - - merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode) - merge_index.conflict_remove(our_path) - end - - # If their commit (in the target project) doesn't exist in the source project, it - # can't be a parent for the merge commit we're about to create. If that's the case, - # fetch the target branch ref into the source project so the commit exists in both. - # - def fetch_their_commit! - return if rugged.include?(conflicts.their_commit.oid) - - random_string = SecureRandom.hex - - project.repository.fetch_ref( - merge_request.target_project.repository.path_to_repo, - "refs/heads/#{merge_request.target_branch}", - "refs/tmp/#{random_string}/head" - ) - end - end -end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index ab7fcf3b6e2..5c843a258fb 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -21,7 +21,10 @@ module MergeRequests update(merge_request) end - def handle_changes(merge_request, old_labels: [], old_mentioned_users: []) + def handle_changes(merge_request, options) + old_labels = options[:old_labels] || [] + old_mentioned_users = options[:old_mentioned_users] || [] + if has_changes?(merge_request, old_labels: old_labels) todo_service.mark_pending_todos_as_done(merge_request, current_user) end diff --git a/app/services/notes/build_service.rb b/app/services/notes/build_service.rb new file mode 100644 index 00000000000..abf25bb778b --- /dev/null +++ b/app/services/notes/build_service.rb @@ -0,0 +1,39 @@ +module Notes + class BuildService < ::BaseService + def execute + in_reply_to_discussion_id = params.delete(:in_reply_to_discussion_id) + + if in_reply_to_discussion_id.present? + discussion = find_discussion(in_reply_to_discussion_id) + + unless discussion + note = Note.new + note.errors.add(:base, 'Discussion to reply to cannot be found') + return note + end + + params.merge!(discussion.reply_attributes) + end + + note = Note.new(params) + note.project = project + note.author = current_user + + note + end + + def find_discussion(discussion_id) + if project + project.notes.find_discussion(discussion_id) + else + # only PersonalSnippets can have discussions without project association + discussion = Note.find_discussion(discussion_id) + noteable = discussion.noteable + + return nil unless noteable.is_a?(PersonalSnippet) && can?(current_user, :comment_personal_snippet, noteable) + + discussion + end + end + end +end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 61d66a26932..f3954f6f8c4 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -1,12 +1,10 @@ module Notes - class CreateService < BaseService + class CreateService < ::BaseService def execute merge_request_diff_head_sha = params.delete(:merge_request_diff_head_sha) - note = Note.new(params) - note.project = project - note.author = current_user - note.system = false + note = Notes::BuildService.new(project, current_user, params).execute + return note unless note.valid? # We execute commands (extracted from `params[:note]`) on the noteable # **before** we save the note because if the note consists of commands diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 940e850600f..988bd0a7cdb 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -3,7 +3,7 @@ # class NotificationRecipientService attr_reader :project - + def initialize(project) @project = project end @@ -12,20 +12,21 @@ class NotificationRecipientService custom_action = build_custom_key(action, target) recipients = target.participants(current_user) - - unless NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(custom_action) - recipients = add_project_watchers(recipients) - end - + recipients = add_project_watchers(recipients) recipients = add_custom_notifications(recipients, custom_action) recipients = reject_mention_users(recipients) # Re-assign is considered as a mention of the new assignee so we add the # new assignee to the list of recipients after we rejected users with # the "on mention" notification level - if [:reassign_merge_request, :reassign_issue].include?(custom_action) + case custom_action + when :reassign_merge_request recipients << previous_assignee if previous_assignee recipients << target.assignee + when :reassign_issue + previous_assignees = Array(previous_assignee) + recipients.concat(previous_assignees) + recipients.concat(target.assignees) end recipients = reject_muted_users(recipients) @@ -43,6 +44,28 @@ class NotificationRecipientService recipients.uniq end + def build_pipeline_recipients(target, current_user, action:) + return [] unless current_user + + custom_action = + case action.to_s + when 'failed' + :failed_pipeline + when 'success' + :success_pipeline + end + + notification_setting = notification_setting_for_user_project(current_user, target.project) + + return [] if notification_setting.mention? || notification_setting.disabled? + + return [] if notification_setting.custom? && !notification_setting.public_send(custom_action) + + return [] if (notification_setting.watch? || notification_setting.participating?) && NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(custom_action) + + reject_users_without_access([current_user], target) + end + def build_relabeled_recipients(target, current_user, labels:) recipients = add_labels_subscribers([], target, labels: labels) recipients = reject_unsubscribed_users(recipients, target) @@ -290,4 +313,16 @@ class NotificationRecipientService def build_custom_key(action, object) "#{action}_#{object.class.model_name.name.underscore}".to_sym end + + def notification_setting_for_user_project(user, project) + project_setting = user.notification_settings_for(project) + + return project_setting unless project_setting.global? + + group_setting = user.notification_settings_for(project.group) + + return group_setting unless group_setting.global? + + user.global_notification_setting + end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 2c6f849259e..646ccbdb2bf 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -66,8 +66,25 @@ class NotificationService # * issue new assignee if their notification level is not Disabled # * users with custom level checked with "reassign issue" # - def reassigned_issue(issue, current_user) - reassign_resource_email(issue, issue.project, current_user, :reassigned_issue_email) + def reassigned_issue(issue, current_user, previous_assignees = []) + recipients = NotificationRecipientService.new(issue.project).build_recipients( + issue, + current_user, + action: "reassign", + previous_assignee: previous_assignees + ) + + previous_assignee_ids = previous_assignees.map(&:id) + + recipients.each do |recipient| + mailer.send( + :reassigned_issue_email, + recipient.id, + issue.id, + previous_assignee_ids, + current_user.id + ).deliver_later + end end # When we add labels to an issue we should send an email to: @@ -278,11 +295,11 @@ class NotificationService return unless mailer.respond_to?(email_template) - recipients ||= NotificationRecipientService.new(pipeline.project).build_recipients( + recipients ||= NotificationRecipientService.new(pipeline.project).build_pipeline_recipients( pipeline, pipeline.user, - action: pipeline.status, - skip_current_user: false).map(&:notification_email) + action: pipeline.status + ).map(&:notification_email) if recipients.any? mailer.public_send(email_template, pipeline, recipients).deliver_later @@ -367,10 +384,10 @@ class NotificationService end def previous_record(object, attribute) - if object && attribute - if object.previous_changes.include?(attribute) - object.previous_changes[attribute].first - end + return unless object && attribute + + if object.previous_changes.include?(attribute) + object.previous_changes[attribute].first end end end diff --git a/app/services/preview_markdown_service.rb b/app/services/preview_markdown_service.rb new file mode 100644 index 00000000000..10d45bbf73c --- /dev/null +++ b/app/services/preview_markdown_service.rb @@ -0,0 +1,45 @@ +class PreviewMarkdownService < BaseService + def execute + text, commands = explain_slash_commands(params[:text]) + users = find_user_references(text) + + success( + text: text, + users: users, + commands: commands.join(' ') + ) + end + + private + + def explain_slash_commands(text) + return text, [] unless %w(Issue MergeRequest).include?(commands_target_type) + + slash_commands_service = SlashCommands::InterpretService.new(project, current_user) + slash_commands_service.explain(text, find_commands_target) + end + + def find_user_references(text) + extractor = Gitlab::ReferenceExtractor.new(project, current_user) + extractor.analyze(text, author: current_user) + extractor.users.map(&:username) + end + + def find_commands_target + if commands_target_id.present? + finder = commands_target_type == 'Issue' ? IssuesFinder : MergeRequestsFinder + finder.new(current_user, project_id: project.id).find(commands_target_id) + else + collection = commands_target_type == 'Issue' ? project.issues : project.merge_requests + collection.build + end + end + + def commands_target_type + params[:slash_commands_target_type] + end + + def commands_target_id + params[:slash_commands_target_id] + end +end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index fbdaa455651..535d93385e6 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -58,6 +58,9 @@ module Projects fail(error: @project.errors.full_messages.join(', ')) end @project + rescue ActiveRecord::RecordInvalid => e + message = "Unable to save #{e.record.type}: #{e.record.errors.full_messages.join(", ")} " + fail(error: message) rescue => e fail(error: e.message) end @@ -94,7 +97,8 @@ module Projects system_hook_service.execute_hooks_for(@project, :create) unless @project.group || @project.gitlab_project_import? - @project.team << [current_user, :master, current_user] + owners = [current_user, @project.namespace.owner].compact.uniq + @project.add_master(owners, current_user: current_user) end @project.group&.refresh_members_authorized_projects diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index a7142d5950e..06d8d143231 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -31,16 +31,16 @@ module Projects project.team.truncate project.destroy! - unless remove_registry_tags - raise_error('Failed to remove project container registry. Please try again or contact administrator') + unless remove_legacy_registry_tags + raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.') end unless remove_repository(repo_path) - raise_error('Failed to remove project repository. Please try again or contact administrator') + raise_error('Failed to remove project repository. Please try again or contact administrator.') end unless remove_repository(wiki_path) - raise_error('Failed to remove wiki repository. Please try again or contact administrator') + raise_error('Failed to remove wiki repository. Please try again or contact administrator.') end end @@ -68,10 +68,16 @@ module Projects end end - def remove_registry_tags + ## + # This method makes sure that we correctly remove registry tags + # for legacy image repository (when repository path equals project path). + # + def remove_legacy_registry_tags return true unless Gitlab.config.registry.enabled - project.container_registry_repository.delete_tags + ContainerRepository.build_root_repository(project).tap do |repository| + return repository.has_tags? ? repository.delete_tags! : true + end end def raise_error(message) diff --git a/app/services/projects/enable_deploy_key_service.rb b/app/services/projects/enable_deploy_key_service.rb index 3cf4264ce9b..121385afca3 100644 --- a/app/services/projects/enable_deploy_key_service.rb +++ b/app/services/projects/enable_deploy_key_service.rb @@ -4,7 +4,10 @@ module Projects key = accessible_keys.find_by(id: params[:key_id] || params[:id]) return unless key - project.deploy_keys << key + unless project.deploy_keys.include?(key) + project.deploy_keys << key + end + key end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index d484a96f785..eea17e24903 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -11,7 +11,7 @@ module Projects success rescue => e - error(e.message) + error("Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}") end private @@ -32,23 +32,39 @@ module Projects end def import_repository + raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url) + begin - raise Error, "Blocked import URL." if Gitlab::UrlBlocker.blocked_url?(project.import_url) - gitlab_shell.import_repository(project.repository_storage_path, project.path_with_namespace, project.import_url) - rescue => e + if project.github_import? || project.gitea_import? + fetch_repository + else + clone_repository + end + rescue Gitlab::Shell::Error => e # Expire cache to prevent scenarios such as: # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true # 2. Retried import, repo is broken or not imported but +exists?+ still returns true - project.repository.before_import if project.repository_exists? + project.repository.expire_content_cache if project.repository_exists? - raise Error, "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}" + raise Error, e.message end end + def clone_repository + gitlab_shell.import_repository(project.repository_storage_path, project.path_with_namespace, project.import_url) + end + + def fetch_repository + project.create_repository + project.repository.add_remote(project.import_type, project.import_url) + project.repository.set_remote_as_mirror(project.import_type) + project.repository.fetch_remote(project.import_type, forced: true) + end + def import_data return unless has_importer? - project.repository.before_import unless project.gitlab_project_import? + project.repository.expire_content_cache unless project.gitlab_project_import? unless importer.execute raise Error, 'The remote data could not be imported.' diff --git a/app/services/projects/propagate_service_template.rb b/app/services/projects/propagate_service_template.rb new file mode 100644 index 00000000000..a8ef2108492 --- /dev/null +++ b/app/services/projects/propagate_service_template.rb @@ -0,0 +1,103 @@ +module Projects + class PropagateServiceTemplate + BATCH_SIZE = 100 + + def self.propagate(*args) + new(*args).propagate + end + + def initialize(template) + @template = template + end + + def propagate + return unless @template.active? + + Rails.logger.info("Propagating services for template #{@template.id}") + + propagate_projects_with_template + end + + private + + def propagate_projects_with_template + loop do + batch = project_ids_batch + + bulk_create_from_template(batch) unless batch.empty? + + break if batch.size < BATCH_SIZE + end + end + + def bulk_create_from_template(batch) + service_list = batch.map do |project_id| + service_hash.values << project_id + end + + Project.transaction do + bulk_insert_services(service_hash.keys << 'project_id', service_list) + run_callbacks(batch) + end + end + + def project_ids_batch + Project.connection.select_values( + <<-SQL + SELECT id + FROM projects + WHERE NOT EXISTS ( + SELECT true + FROM services + WHERE services.project_id = projects.id + AND services.type = '#{@template.type}' + ) + AND projects.pending_delete = false + AND projects.archived = false + LIMIT #{BATCH_SIZE} + SQL + ) + end + + def bulk_insert_services(columns, values_array) + ActiveRecord::Base.connection.execute( + <<-SQL.strip_heredoc + INSERT INTO services (#{columns.join(', ')}) + VALUES #{values_array.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')} + SQL + ) + end + + def service_hash + @service_hash ||= + begin + template_hash = @template.as_json(methods: :type).except('id', 'template', 'project_id') + + template_hash.each_with_object({}) do |(key, value), service_hash| + value = value.is_a?(Hash) ? value.to_json : value + + service_hash[ActiveRecord::Base.connection.quote_column_name(key)] = + ActiveRecord::Base.sanitize(value) + end + end + end + + def run_callbacks(batch) + if active_external_issue_tracker? + Project.where(id: batch).update_all(has_external_issue_tracker: true) + end + + if active_external_wiki? + Project.where(id: batch).update_all(has_external_wiki: true) + end + end + + def active_external_issue_tracker? + @template.issue_tracker? && !@template.default + end + + def active_external_wiki? + @template.type == 'ExternalWikiService' + end + end +end diff --git a/app/services/projects/update_pages_configuration_service.rb b/app/services/projects/update_pages_configuration_service.rb index eb4809afa85..cacb74b1205 100644 --- a/app/services/projects/update_pages_configuration_service.rb +++ b/app/services/projects/update_pages_configuration_service.rb @@ -27,7 +27,7 @@ module Projects { domain: domain.domain, certificate: domain.certificate, - key: domain.key, + key: domain.key } end end diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 523b9f41916..17cf71cf098 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -46,6 +46,7 @@ module Projects end def error(message, http_status = nil) + log_error("Projects::UpdatePagesService: #{message}") @status.allow_failure = !latest? @status.description = message @status.drop diff --git a/app/services/projects/upload_service.rb b/app/services/projects/upload_service.rb deleted file mode 100644 index be34d4fa9b8..00000000000 --- a/app/services/projects/upload_service.rb +++ /dev/null @@ -1,22 +0,0 @@ -module Projects - class UploadService < BaseService - def initialize(project, file) - @project, @file = project, file - end - - def execute - return nil unless @file && @file.size <= max_attachment_size - - uploader = FileUploader.new(@project) - uploader.store!(@file) - - uploader.to_h - end - - private - - def max_attachment_size - current_application_settings.max_attachment_size.megabytes.to_i - end - end -end diff --git a/app/services/protected_branches/update_service.rb b/app/services/protected_branches/update_service.rb index 89d8ba60134..4b3337a5c9d 100644 --- a/app/services/protected_branches/update_service.rb +++ b/app/services/protected_branches/update_service.rb @@ -1,13 +1,10 @@ module ProtectedBranches class UpdateService < BaseService - attr_reader :protected_branch - def execute(protected_branch) raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) - @protected_branch = protected_branch - @protected_branch.update(params) - @protected_branch + protected_branch.update(params) + protected_branch end end end diff --git a/app/services/protected_tags/create_service.rb b/app/services/protected_tags/create_service.rb new file mode 100644 index 00000000000..faba7865a17 --- /dev/null +++ b/app/services/protected_tags/create_service.rb @@ -0,0 +1,11 @@ +module ProtectedTags + class CreateService < BaseService + attr_reader :protected_tag + + def execute + raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) + + project.protected_tags.create(params) + end + end +end diff --git a/app/services/protected_tags/update_service.rb b/app/services/protected_tags/update_service.rb new file mode 100644 index 00000000000..aea6a48968d --- /dev/null +++ b/app/services/protected_tags/update_service.rb @@ -0,0 +1,10 @@ +module ProtectedTags + class UpdateService < BaseService + def execute(protected_tag) + raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) + + protected_tag.update(params) + protected_tag + end + end +end diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb index 781cd13b44b..ff188102b62 100644 --- a/app/services/search/global_service.rb +++ b/app/services/search/global_service.rb @@ -7,14 +7,19 @@ module Search end def execute - group = Group.find_by(id: params[:group_id]) if params[:group_id].present? - projects = ProjectsFinder.new.execute(current_user) + Gitlab::SearchResults.new(current_user, projects, params[:search]) + end - if group - projects = projects.inside_path(group.full_path) - end + def projects + @projects ||= ProjectsFinder.new(current_user: current_user).execute + end - Gitlab::SearchResults.new(current_user, projects, params[:search]) + def scope + @scope ||= begin + allowed_scopes = %w[issues merge_requests milestones] + + allowed_scopes.delete(params[:scope]) { 'projects' } + end end end end diff --git a/app/services/search/group_service.rb b/app/services/search/group_service.rb new file mode 100644 index 00000000000..29478e3251f --- /dev/null +++ b/app/services/search/group_service.rb @@ -0,0 +1,18 @@ +module Search + class GroupService < Search::GlobalService + attr_accessor :group + + def initialize(user, group, params) + super(user, params) + + @group = group + end + + def projects + return Project.none unless group + return @projects if defined? @projects + + @projects = super.inside_path(group.full_path) + end + end +end diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb index 4b500914cfb..9a22abae635 100644 --- a/app/services/search/project_service.rb +++ b/app/services/search/project_service.rb @@ -12,5 +12,9 @@ module Search params[:search], params[:repository_ref]) end + + def scope + @scope ||= %w[notes issues merge_requests milestones wiki_blobs commits].delete(params[:scope]) { 'blobs' } + end end end diff --git a/app/services/search/snippet_service.rb b/app/services/search/snippet_service.rb index 0b3e713e220..85da0be6fff 100644 --- a/app/services/search/snippet_service.rb +++ b/app/services/search/snippet_service.rb @@ -7,9 +7,13 @@ module Search end def execute - snippets = Snippet.accessible_to(current_user) + snippets = SnippetsFinder.new(current_user).execute Gitlab::SnippetSearchResults.new(snippets, params[:search]) end + + def scope + @scope ||= %w[snippet_titles].delete(params[:scope]) { 'snippet_blobs' } + end end end diff --git a/app/services/search_service.rb b/app/services/search_service.rb new file mode 100644 index 00000000000..22736c71725 --- /dev/null +++ b/app/services/search_service.rb @@ -0,0 +1,65 @@ +class SearchService + include Gitlab::Allowable + + def initialize(current_user, params = {}) + @current_user = current_user + @params = params.dup + end + + def project + return @project if defined?(@project) + + @project = + if params[:project_id].present? + the_project = Project.find_by(id: params[:project_id]) + can?(current_user, :download_code, the_project) ? the_project : nil + else + nil + end + end + + def group + return @group if defined?(@group) + + @group = + if params[:group_id].present? + the_group = Group.find_by(id: params[:group_id]) + can?(current_user, :read_group, the_group) ? the_group : nil + else + nil + end + end + + def show_snippets? + return @show_snippets if defined?(@show_snippets) + + @show_snippets = params[:snippets] == 'true' + end + + delegate :scope, to: :search_service + + def search_results + @search_results ||= search_service.execute + end + + def search_objects + @search_objects ||= search_results.objects(scope, params[:page]) + end + + private + + def search_service + @search_service ||= + if project + Search::ProjectService.new(project, current_user, params) + elsif show_snippets? + Search::SnippetService.new(current_user, params) + elsif group + Search::GroupService.new(current_user, group, params) + else + Search::GlobalService.new(current_user, params) + end + end + + attr_reader :current_user, :params +end diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index 595653ea58a..a7e13648b54 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -2,31 +2,31 @@ module SlashCommands class InterpretService < BaseService include Gitlab::SlashCommands::Dsl - attr_reader :issuable, :options + attr_reader :issuable # Takes a text and interprets the commands that are extracted from it. # Returns the content without commands, and hash of changes to be applied to a record. def execute(content, issuable) + return [content, {}] unless current_user.can?(:use_slash_commands) + @issuable = issuable @updates = {} - opts = { - issuable: issuable, - current_user: current_user, - project: project, - params: params - } - - content, commands = extractor.extract_commands(content, opts) + content, commands = extractor.extract_commands(content, context) + extract_updates(commands, context) + [content, @updates] + end - commands.each do |name, arg| - definition = self.class.command_definitions_by_name[name.to_sym] - next unless definition + # Takes a text and interprets the commands that are extracted from it. + # Returns the content without commands, and array of changes explained. + def explain(content, issuable) + return [content, []] unless current_user.can?(:use_slash_commands) - definition.execute(self, opts, arg) - end + @issuable = issuable - [content, @updates] + content, commands = extractor.extract_commands(content, context) + commands = explain_commands(commands, context) + [content, commands] end private @@ -38,6 +38,9 @@ module SlashCommands desc do "Close this #{issuable.to_ability_name.humanize(capitalize: false)}" end + explanation do + "Closes this #{issuable.to_ability_name.humanize(capitalize: false)}." + end condition do issuable.persisted? && issuable.open? && @@ -50,6 +53,9 @@ module SlashCommands desc do "Reopen this #{issuable.to_ability_name.humanize(capitalize: false)}" end + explanation do + "Reopens this #{issuable.to_ability_name.humanize(capitalize: false)}." + end condition do issuable.persisted? && issuable.closed? && @@ -60,6 +66,7 @@ module SlashCommands end desc 'Merge (when the pipeline succeeds)' + explanation 'Merges this merge request when the pipeline succeeds.' condition do last_diff_sha = params && params[:merge_request_diff_head_sha] issuable.is_a?(MergeRequest) && @@ -71,6 +78,9 @@ module SlashCommands end desc 'Change title' + explanation do |title_param| + "Changes the title to \"#{title_param}\"." + end params '<New title>' condition do issuable.persisted? && @@ -81,41 +91,70 @@ module SlashCommands end desc 'Assign' + explanation do |users| + "Assigns #{users.map(&:to_reference).to_sentence}." if users.any? + end params '@user' condition do current_user.can?(:"admin_#{issuable.to_ability_name}", project) end - command :assign do |assignee_param| - user = extract_references(assignee_param, :user).first - user ||= User.find_by(username: assignee_param) + parse_params do |assignee_param| + users = extract_references(assignee_param, :user) - @updates[:assignee_id] = user.id if user + if users.empty? + users = User.where(username: assignee_param.split(' ').map(&:strip)) + end + + users + end + command :assign do |users| + next if users.empty? + + if issuable.is_a?(Issue) + @updates[:assignee_ids] = users.map(&:id) + else + @updates[:assignee_id] = users.last.id + end end desc 'Remove assignee' + explanation do + "Removes assignee #{issuable.assignees.first.to_reference}." + end condition do issuable.persisted? && - issuable.assignee_id? && + issuable.assignees.any? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end command :unassign do - @updates[:assignee_id] = nil + if issuable.is_a?(Issue) + @updates[:assignee_ids] = [] + else + @updates[:assignee_id] = nil + end end desc 'Set milestone' + explanation do |milestone| + "Sets the milestone to #{milestone.to_reference}." if milestone + end params '%"milestone"' condition do current_user.can?(:"admin_#{issuable.to_ability_name}", project) && project.milestones.active.any? end - command :milestone do |milestone_param| - milestone = extract_references(milestone_param, :milestone).first - milestone ||= project.milestones.find_by(title: milestone_param.strip) - + parse_params do |milestone_param| + extract_references(milestone_param, :milestone).first || + project.milestones.find_by(title: milestone_param.strip) + end + command :milestone do |milestone| @updates[:milestone_id] = milestone.id if milestone end desc 'Remove milestone' + explanation do + "Removes #{issuable.milestone.to_reference(format: :name)} milestone." + end condition do issuable.persisted? && issuable.milestone_id? && @@ -126,6 +165,11 @@ module SlashCommands end desc 'Add label(s)' + explanation do |labels_param| + labels = find_label_references(labels_param) + + "Adds #{labels.join(' ')} #{'label'.pluralize(labels.count)}." if labels.any? + end params '~label1 ~"label 2"' condition do available_labels = LabelsFinder.new(current_user, project_id: project.id).execute @@ -145,6 +189,14 @@ module SlashCommands end desc 'Remove all or specific label(s)' + explanation do |labels_param = nil| + if labels_param.present? + labels = find_label_references(labels_param) + "Removes #{labels.join(' ')} #{'label'.pluralize(labels.count)}." if labels.any? + else + 'Removes all labels.' + end + end params '~label1 ~"label 2"' condition do issuable.persisted? && @@ -167,6 +219,10 @@ module SlashCommands end desc 'Replace all label(s)' + explanation do |labels_param| + labels = find_label_references(labels_param) + "Replaces all labels with #{labels.join(' ')} #{'label'.pluralize(labels.count)}." if labels.any? + end params '~label1 ~"label 2"' condition do issuable.persisted? && @@ -185,6 +241,7 @@ module SlashCommands end desc 'Add a todo' + explanation 'Adds a todo.' condition do issuable.persisted? && !TodoService.new.todo_exist?(issuable, current_user) @@ -194,6 +251,7 @@ module SlashCommands end desc 'Mark todo as done' + explanation 'Marks todo as done.' condition do issuable.persisted? && TodoService.new.todo_exist?(issuable, current_user) @@ -203,6 +261,9 @@ module SlashCommands end desc 'Subscribe' + explanation do + "Subscribes to this #{issuable.to_ability_name.humanize(capitalize: false)}." + end condition do issuable.persisted? && !issuable.subscribed?(current_user, project) @@ -212,6 +273,9 @@ module SlashCommands end desc 'Unsubscribe' + explanation do + "Unsubscribes from this #{issuable.to_ability_name.humanize(capitalize: false)}." + end condition do issuable.persisted? && issuable.subscribed?(current_user, project) @@ -221,18 +285,23 @@ module SlashCommands end desc 'Set due date' + explanation do |due_date| + "Sets the due date to #{due_date.to_s(:medium)}." if due_date + end params '<in 2 days | this Friday | December 31st>' condition do issuable.respond_to?(:due_date) && current_user.can?(:"admin_#{issuable.to_ability_name}", project) end - command :due do |due_date_param| - due_date = Chronic.parse(due_date_param).try(:to_date) - + parse_params do |due_date_param| + Chronic.parse(due_date_param).try(:to_date) + end + command :due do |due_date| @updates[:due_date] = due_date if due_date end desc 'Remove due date' + explanation 'Removes the due date.' condition do issuable.persisted? && issuable.respond_to?(:due_date) && @@ -243,8 +312,11 @@ module SlashCommands @updates[:due_date] = nil end - desc do - "Toggle the Work In Progress status" + desc 'Toggle the Work In Progress status' + explanation do + verb = issuable.work_in_progress? ? 'Unmarks' : 'Marks' + noun = issuable.to_ability_name.humanize(capitalize: false) + "#{verb} this #{noun} as Work In Progress." end condition do issuable.persisted? && @@ -255,45 +327,72 @@ module SlashCommands @updates[:wip_event] = issuable.work_in_progress? ? 'unwip' : 'wip' end - desc 'Toggle emoji reward' + desc 'Toggle emoji award' + explanation do |name| + "Toggles :#{name}: emoji award." if name + end params ':emoji:' condition do issuable.persisted? end - command :award do |emoji| - name = award_emoji_name(emoji) + parse_params do |emoji_param| + match = emoji_param.match(Banzai::Filter::EmojiFilter.emoji_pattern) + match[1] if match + end + command :award do |name| if name && issuable.user_can_award?(current_user, name) @updates[:emoji_award] = name end end desc 'Set time estimate' + explanation do |time_estimate| + time_estimate = Gitlab::TimeTrackingFormatter.output(time_estimate) + + "Sets time estimate to #{time_estimate}." if time_estimate + end params '<1w 3d 2h 14m>' condition do current_user.can?(:"admin_#{issuable.to_ability_name}", project) end - command :estimate do |raw_duration| - time_estimate = Gitlab::TimeTrackingFormatter.parse(raw_duration) - + parse_params do |raw_duration| + Gitlab::TimeTrackingFormatter.parse(raw_duration) + end + command :estimate do |time_estimate| if time_estimate @updates[:time_estimate] = time_estimate end end desc 'Add or substract spent time' + explanation do |time_spent| + if time_spent + if time_spent > 0 + verb = 'Adds' + value = time_spent + else + verb = 'Substracts' + value = -time_spent + end + + "#{verb} #{Gitlab::TimeTrackingFormatter.output(value)} spent time." + end + end params '<1h 30m | -1h 30m>' condition do current_user.can?(:"admin_#{issuable.to_ability_name}", issuable) end - command :spend do |raw_duration| - time_spent = Gitlab::TimeTrackingFormatter.parse(raw_duration) - + parse_params do |raw_duration| + Gitlab::TimeTrackingFormatter.parse(raw_duration) + end + command :spend do |time_spent| if time_spent @updates[:spend_time] = { duration: time_spent, user: current_user } end end desc 'Remove time estimate' + explanation 'Removes time estimate.' condition do issuable.persisted? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) @@ -303,6 +402,7 @@ module SlashCommands end desc 'Remove spent time' + explanation 'Removes spent time.' condition do issuable.persisted? && current_user.can?(:"admin_#{issuable.to_ability_name}", project) @@ -316,23 +416,78 @@ module SlashCommands params '@user' command :cc - desc 'Defines target branch for MR' + desc 'Define target branch for MR' + explanation do |branch_name| + "Sets target branch to #{branch_name}." + end params '<Local branch name>' condition do issuable.respond_to?(:target_branch) && (current_user.can?(:"update_#{issuable.to_ability_name}", issuable) || issuable.new_record?) end - command :target_branch do |target_branch_param| - branch_name = target_branch_param.strip + parse_params do |target_branch_param| + target_branch_param.strip + end + command :target_branch do |branch_name| @updates[:target_branch] = branch_name if project.repository.branch_names.include?(branch_name) end + desc 'Move issue from one column of the board to another' + explanation do |target_list_name| + label = find_label_references(target_list_name).first + "Moves issue to #{label} column in the board." if label + end + params '~"Target column"' + condition do + issuable.is_a?(Issue) && + current_user.can?(:"update_#{issuable.to_ability_name}", issuable) && + issuable.project.boards.count == 1 + end + command :board_move do |target_list_name| + label_ids = find_label_ids(target_list_name) + + if label_ids.size == 1 + label_id = label_ids.first + + # Ensure this label corresponds to a list on the board + next unless Label.on_project_boards(issuable.project_id).where(id: label_id).exists? + + @updates[:remove_label_ids] = + issuable.labels.on_project_boards(issuable.project_id).where.not(id: label_id).pluck(:id) + @updates[:add_label_ids] = [label_id] + end + end + + def find_labels(labels_param) + extract_references(labels_param, :label) | + LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute + end + + def find_label_references(labels_param) + find_labels(labels_param).map(&:to_reference) + end + def find_label_ids(labels_param) - label_ids_by_reference = extract_references(labels_param, :label).map(&:id) - labels_ids_by_name = LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute.select(:id) + find_labels(labels_param).map(&:id) + end + + def explain_commands(commands, opts) + commands.map do |name, arg| + definition = self.class.definition_by_name(name) + next unless definition - label_ids_by_reference | labels_ids_by_name + definition.explain(self, opts, arg) + end.compact + end + + def extract_updates(commands, opts) + commands.each do |name, arg| + definition = self.class.definition_by_name(name) + next unless definition + + definition.execute(self, opts, arg) + end end def extract_references(arg, type) @@ -342,9 +497,13 @@ module SlashCommands ext.references(type) end - def award_emoji_name(emoji) - match = emoji.match(Banzai::Filter::EmojiFilter.emoji_pattern) - match[1] if match + def context + { + issuable: issuable, + current_user: current_user, + project: project, + params: params + } end end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index af0ddbe5934..ed476fc9d0c 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -51,7 +51,7 @@ class SystemHooksService path: model.path, group_id: model.id, owner_name: owner.respond_to?(:name) ? owner.name : nil, - owner_email: owner.respond_to?(:email) ? owner.email : nil, + owner_email: owner.respond_to?(:email) ? owner.email : nil ) when GroupMember data.merge!(group_member_data(model)) @@ -113,7 +113,7 @@ class SystemHooksService user_name: model.user.name, user_email: model.user.email, user_id: model.user.id, - group_access: model.human_access, + group_access: model.human_access } end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index d3e502b66dd..93bf1fb1615 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -49,6 +49,44 @@ module SystemNoteService create_note(NoteSummary.new(noteable, project, author, body, action: 'assignee')) end + # Called when the assignees of an Issue is changed or removed + # + # issue - Issue object + # project - Project owning noteable + # author - User performing the change + # assignees - Users being assigned, or nil + # + # Example Note text: + # + # "removed all assignees" + # + # "assigned to @user1 additionally to @user2" + # + # "assigned to @user1, @user2 and @user3 and unassigned from @user4 and @user5" + # + # "assigned to @user1 and @user2" + # + # Returns the created Note object + def change_issue_assignees(issue, project, author, old_assignees) + body = + if issue.assignees.any? && old_assignees.any? + unassigned_users = old_assignees - issue.assignees + added_users = issue.assignees.to_a - old_assignees + + text_parts = [] + text_parts << "assigned to #{added_users.map(&:to_reference).to_sentence}" if added_users.any? + text_parts << "unassigned #{unassigned_users.map(&:to_reference).to_sentence}" if unassigned_users.any? + + text_parts.join(' and ') + elsif old_assignees.any? + "removed assignee" + elsif issue.assignees.any? + "assigned to #{issue.assignees.map(&:to_reference).to_sentence}" + end + + create_note(NoteSummary.new(issue, project, author, body, action: 'assignee')) + end + # Called when one or more labels on a Noteable are added and/or removed # # noteable - Noteable object @@ -183,7 +221,9 @@ module SystemNoteService body = status.dup body << " via #{source.gfm_reference(project)}" if source - create_note(NoteSummary.new(noteable, project, author, body, action: 'status')) + action = status == 'reopened' ? 'opened' : status + + create_note(NoteSummary.new(noteable, project, author, body, action: action)) end # Called when 'merge when pipeline succeeds' is executed @@ -226,12 +266,10 @@ module SystemNoteService def discussion_continued_in_issue(discussion, project, author, issue) body = "created #{issue.to_reference} to continue this discussion" + note_attributes = discussion.reply_attributes.merge(project: project, author: author, note: body) - note_params = discussion.reply_attributes.merge(project: project, author: author, note: body) - note_params[:type] = note_params.delete(:note_type) - - note = Note.create(note_params.merge(system: true)) - note.system_note_metadata = SystemNoteMetadata.new({ action: 'discussion' }) + note = Note.create(note_attributes.merge(system: true)) + note.system_note_metadata = SystemNoteMetadata.new(action: 'discussion') note end @@ -253,14 +291,31 @@ module SystemNoteService old_diffs, new_diffs = Gitlab::Diff::InlineDiff.new(old_title, new_title).inline_diffs - marked_old_title = Gitlab::Diff::InlineDiffMarker.new(old_title).mark(old_diffs, mode: :deletion, markdown: true) - marked_new_title = Gitlab::Diff::InlineDiffMarker.new(new_title).mark(new_diffs, mode: :addition, markdown: true) + marked_old_title = Gitlab::Diff::InlineDiffMarkdownMarker.new(old_title).mark(old_diffs, mode: :deletion) + marked_new_title = Gitlab::Diff::InlineDiffMarkdownMarker.new(new_title).mark(new_diffs, mode: :addition) body = "changed title from **#{marked_old_title}** to **#{marked_new_title}**" create_note(NoteSummary.new(noteable, project, author, body, action: 'title')) end + # Called when the description of a Noteable is changed + # + # noteable - Noteable object that responds to `description` + # project - Project owning noteable + # author - User performing the change + # + # Example Note text: + # + # "changed the description" + # + # Returns the created Note object + def change_description(noteable, project, author) + body = 'changed the description' + + create_note(NoteSummary.new(noteable, project, author, body, action: 'description')) + end + # Called when the confidentiality changes # # issue - Issue object @@ -273,9 +328,15 @@ module SystemNoteService # # Returns the created Note object def change_issue_confidentiality(issue, project, author) - body = issue.confidential ? 'made the issue confidential' : 'made the issue visible to everyone' + if issue.confidential + body = 'made the issue confidential' + action = 'confidential' + else + body = 'made the issue visible to everyone' + action = 'visible' + end - create_note(NoteSummary.new(issue, project, author, body, action: 'confidentiality')) + create_note(NoteSummary.new(issue, project, author, body, action: action)) end # Called when a branch in Noteable is changed diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 2c56cb4c680..322c6286365 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -204,7 +204,7 @@ class TodoService # Only update those that are not really on that state todos = todos.where.not(state: state) todos_ids = todos.pluck(:id) - todos.update_all(state: state) + todos.unscope(:order).update_all(state: state) current_user.update_todos_count_cache todos_ids end @@ -251,9 +251,9 @@ class TodoService end def create_assignment_todo(issuable, author) - if issuable.assignee + if issuable.assignees.any? attributes = attributes_for_todo(issuable.project, issuable, author, Todo::ASSIGNED) - create_todos(issuable.assignee, attributes) + create_todos(issuable.assignees, attributes) end end @@ -281,7 +281,7 @@ class TodoService def attributes_for_target(target) attributes = { - project_id: target.project.id, + project_id: target&.project&.id, target_id: target.id, target_type: target.class.name, commit_id: nil diff --git a/app/services/upload_service.rb b/app/services/upload_service.rb new file mode 100644 index 00000000000..6c5b2baff41 --- /dev/null +++ b/app/services/upload_service.rb @@ -0,0 +1,20 @@ +class UploadService + def initialize(model, file, uploader_class = FileUploader) + @model, @file, @uploader_class = model, file, uploader_class + end + + def execute + return nil unless @file && @file.size <= max_attachment_size + + uploader = @uploader_class.new(@model) + uploader.store!(@file) + + uploader.to_h + end + + private + + def max_attachment_size + current_application_settings.max_attachment_size.megabytes.to_i + end +end diff --git a/app/services/users/activity_service.rb b/app/services/users/activity_service.rb new file mode 100644 index 00000000000..facf21a7f5c --- /dev/null +++ b/app/services/users/activity_service.rb @@ -0,0 +1,22 @@ +module Users + class ActivityService + def initialize(author, activity) + @author = author.respond_to?(:user) ? author.user : author + @activity = activity + end + + def execute + return unless @author && @author.is_a?(User) + + record_activity + end + + private + + def record_activity + Gitlab::UserActivities.record(@author.id) + + Rails.logger.debug("Recorded activity: #{@activity} for User ID: #{@author.id} (username: #{@author.username}") + end + end +end diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb new file mode 100644 index 00000000000..363135ef09b --- /dev/null +++ b/app/services/users/build_service.rb @@ -0,0 +1,107 @@ +module Users + # Service for building a new user. + class BuildService < BaseService + def initialize(current_user, params = {}) + @current_user = current_user + @params = params.dup + end + + def execute(skip_authorization: false) + raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_create_user? + + user_params = build_user_params(skip_authorization: skip_authorization) + user = User.new(user_params) + + if current_user&.admin? + @reset_token = user.generate_reset_token if params[:reset_password] + + if user_params[:force_random_password] + random_password = Devise.friendly_token.first(Devise.password_length.min) + user.password = user.password_confirmation = random_password + end + end + + identity_attrs = params.slice(:extern_uid, :provider) + + if identity_attrs.any? + user.identities.build(identity_attrs) + end + + user + end + + private + + def can_create_user? + (current_user.nil? && current_application_settings.signup_enabled?) || current_user&.admin? + end + + # Allowed params for creating a user (admins only) + def admin_create_params + [ + :access_level, + :admin, + :avatar, + :bio, + :can_create_group, + :color_scheme_id, + :email, + :external, + :force_random_password, + :hide_no_password, + :hide_no_ssh_key, + :key_id, + :linkedin, + :name, + :password, + :password_automatically_set, + :password_expires_at, + :projects_limit, + :remember_me, + :skip_confirmation, + :skype, + :theme_id, + :twitter, + :username, + :website_url + ] + end + + # Allowed params for user signup + def signup_params + [ + :email, + :email_confirmation, + :password_automatically_set, + :name, + :password, + :username + ] + end + + def build_user_params(skip_authorization:) + if current_user&.admin? + user_params = params.slice(*admin_create_params) + user_params[:created_by_id] = current_user&.id + + if params[:reset_password] + user_params.merge!(force_random_password: true, password_expires_at: nil) + end + else + allowed_signup_params = signup_params + allowed_signup_params << :skip_confirmation if skip_authorization + + user_params = params.slice(*allowed_signup_params) + if user_params[:skip_confirmation].nil? + user_params[:skip_confirmation] = skip_user_confirmation_email_from_setting + end + end + + user_params + end + + def skip_user_confirmation_email_from_setting + !current_application_settings.send_user_confirmation_email + end + end +end diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb index 193fcd85896..e22f7225ae2 100644 --- a/app/services/users/create_service.rb +++ b/app/services/users/create_service.rb @@ -6,34 +6,10 @@ module Users @params = params.dup end - def build - raise Gitlab::Access::AccessDeniedError unless can_create_user? + def execute(skip_authorization: false) + user = Users::BuildService.new(current_user, params).execute(skip_authorization: skip_authorization) - user = User.new(build_user_params) - - if current_user&.is_admin? - if params[:reset_password] - @reset_token = user.generate_reset_token - params[:force_random_password] = true - end - - if params[:force_random_password] - random_password = Devise.friendly_token.first(Devise.password_length.min) - user.password = user.password_confirmation = random_password - end - end - - identity_attrs = params.slice(:extern_uid, :provider) - - if identity_attrs.any? - user.identities.build(identity_attrs) - end - - user - end - - def execute - user = build + @reset_token = user.generate_reset_token if user.recently_sent_password_reset? if user.save log_info("User \"#{user.name}\" (#{user.email}) was created") @@ -43,68 +19,5 @@ module Users user end - - private - - def can_create_user? - (current_user.nil? && current_application_settings.signup_enabled?) || current_user&.is_admin? - end - - # Allowed params for creating a user (admins only) - def admin_create_params - [ - :access_level, - :admin, - :avatar, - :bio, - :can_create_group, - :color_scheme_id, - :email, - :external, - :force_random_password, - :hide_no_password, - :hide_no_ssh_key, - :key_id, - :linkedin, - :name, - :password, - :password_expires_at, - :projects_limit, - :remember_me, - :skip_confirmation, - :skype, - :theme_id, - :twitter, - :username, - :website_url - ] - end - - # Allowed params for user signup - def signup_params - [ - :email, - :email_confirmation, - :name, - :password, - :username - ] - end - - def build_user_params - if current_user&.is_admin? - user_params = params.slice(*admin_create_params) - user_params[:created_by_id] = current_user&.id - - if params[:reset_password] - user_params.merge!(force_random_password: true, password_expires_at: nil) - end - else - user_params = params.slice(*signup_params) - user_params[:skip_confirmation] = !current_application_settings.send_user_confirmation_email - end - - user_params - end end end diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 833da5bc5d1..9eb6a600f6b 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -20,13 +20,13 @@ module Users Groups::DestroyService.new(group, current_user).execute end - user.personal_projects.each do |project| + user.personal_projects.with_deleted.each do |project| # Skip repository removal because we remove directory with namespace # that contain all this repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).async_execute + ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute end - move_issues_to_ghost_user(user) + MigrateToGhostUserService.new(user).execute unless options[:hard_delete] # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing namespace = user.namespace @@ -35,22 +35,5 @@ module Users user_data end - - private - - def move_issues_to_ghost_user(user) - # Block the user before moving issues to prevent a data race. - # If the user creates an issue after `move_issues_to_ghost_user` - # runs and before the user is destroyed, the destroy will fail with - # an exception. We block the user so that issues can't be created - # after `move_issues_to_ghost_user` runs and before the destroy happens. - user.block - - ghost_user = User.ghost - - user.issues.update_all(author_id: ghost_user.id) - - user.reload - end end end diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb new file mode 100644 index 00000000000..4628c4c6f6e --- /dev/null +++ b/app/services/users/migrate_to_ghost_user_service.rb @@ -0,0 +1,71 @@ +# When a user is destroyed, some of their associated records are +# moved to a "Ghost User", to prevent these associated records from +# being destroyed. +# +# For example, all the issues/MRs a user has created are _not_ destroyed +# when the user is destroyed. +module Users + class MigrateToGhostUserService + extend ActiveSupport::Concern + + attr_reader :ghost_user, :user + + def initialize(user) + @user = user + end + + def execute + transition = user.block_transition + + user.transaction do + # Block the user before moving records to prevent a data race. + # For example, if the user creates an issue after `migrate_issues` + # runs and before the user is destroyed, the destroy will fail with + # an exception. + user.block + + # Reverse the user block if record migration fails + if !migrate_records && transition + transition.rollback + user.save! + end + end + + user.reload + end + + private + + def migrate_records + user.transaction(requires_new: true) do + @ghost_user = User.ghost + + migrate_issues + migrate_merge_requests + migrate_notes + migrate_abuse_reports + migrate_award_emojis + end + end + + def migrate_issues + user.issues.update_all(author_id: ghost_user.id) + end + + def migrate_merge_requests + user.merge_requests.update_all(author_id: ghost_user.id) + end + + def migrate_notes + user.notes.update_all(author_id: ghost_user.id) + end + + def migrate_abuse_reports + user.reported_abuse_reports.update_all(reporter_id: ghost_user.id) + end + + def migrate_award_emojis + user.award_emoji.update_all(user_id: ghost_user.id) + end + end +end diff --git a/app/services/validate_new_branch_service.rb b/app/services/validate_new_branch_service.rb index 2f61be184ce..d232e85cd33 100644 --- a/app/services/validate_new_branch_service.rb +++ b/app/services/validate_new_branch_service.rb @@ -8,10 +8,7 @@ class ValidateNewBranchService < BaseService return error('Branch name is invalid') end - repository = project.repository - existing_branch = repository.find_branch(branch_name) - - if existing_branch + if project.repository.branch_exists?(branch_name) return error('Branch already exists') end |