diff options
Diffstat (limited to 'app/services')
-rw-r--r-- | app/services/ci/register_job_service.rb | 6 | ||||
-rw-r--r-- | app/services/create_branch_service.rb | 6 | ||||
-rw-r--r-- | app/services/merge_requests/build_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/get_urls_service.rb | 7 | ||||
-rw-r--r-- | app/services/notification_recipient_service.rb | 293 | ||||
-rw-r--r-- | app/services/notification_service.rb | 328 | ||||
-rw-r--r-- | app/services/projects/import_service.rb | 3 | ||||
-rw-r--r-- | app/services/system_hooks_service.rb | 9 | ||||
-rw-r--r-- | app/services/todo_service.rb | 8 | ||||
-rw-r--r-- | app/services/users/refresh_authorized_projects_service.rb | 4 |
10 files changed, 336 insertions, 330 deletions
diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 0ab9042bf24..d6a4280ce4c 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -55,13 +55,13 @@ module Ci new_builds. # don't run projects which have not enabled shared runners and builds joins(:project).where(projects: { shared_runners_enabled: true }). - joins('LEFT JOIN project_features ON ci_builds.gl_project_id = project_features.project_id'). + joins('LEFT JOIN project_features ON ci_builds.project_id = project_features.project_id'). where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0'). # Implement fair scheduling # this returns builds that are ordered by number of running builds # we prefer projects that don't use shared runners at all - joins("LEFT JOIN (#{running_builds_for_shared_runners.to_sql}) AS project_builds ON ci_builds.gl_project_id=project_builds.gl_project_id"). + joins("LEFT JOIN (#{running_builds_for_shared_runners.to_sql}) AS project_builds ON ci_builds.project_id=project_builds.project_id"). order('COALESCE(project_builds.running_builds, 0) ASC', 'ci_builds.id ASC') end @@ -71,7 +71,7 @@ module Ci def running_builds_for_shared_runners Ci::Build.running.where(runner: Ci::Runner.shared). - group(:gl_project_id).select(:gl_project_id, 'count(*) AS running_builds') + group(:project_id).select(:project_id, 'count(*) AS running_builds') end def new_builds diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb index b07338d500a..673ed02f952 100644 --- a/app/services/create_branch_service.rb +++ b/app/services/create_branch_service.rb @@ -25,12 +25,12 @@ class CreateBranchService < BaseService private def create_master_branch - project.repository.commit_file( + project.repository.create_file( current_user, '/README.md', '', message: 'Add README.md', - branch_name: 'master', - update: false) + branch_name: 'master' + ) end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 9d4739e37bb..fdce542bd9e 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -6,7 +6,7 @@ module MergeRequests merge_request.source_project = find_source_project merge_request.target_project = find_target_project merge_request.target_branch = find_target_branch - merge_request.can_be_created = branches_valid? && source_branch_specified? && target_branch_specified? + merge_request.can_be_created = branches_valid? compare_branches if branches_present? assign_title_and_description if merge_request.can_be_created diff --git a/app/services/merge_requests/get_urls_service.rb b/app/services/merge_requests/get_urls_service.rb index 1262ecbc29a..f00a33969a8 100644 --- a/app/services/merge_requests/get_urls_service.rb +++ b/app/services/merge_requests/get_urls_service.rb @@ -7,6 +7,8 @@ module MergeRequests end def execute(changes) + return [] unless project.printing_merge_request_link_enabled + branches = get_branches(changes) merge_requests_map = opened_merge_requests_from_source_branches(branches) branches.map do |branch| @@ -23,10 +25,7 @@ module MergeRequests def opened_merge_requests_from_source_branches(branches) merge_requests = MergeRequest.from_project(project).opened.from_source_branches(branches) - merge_requests.inject({}) do |hash, mr| - hash[mr.source_branch] = mr - hash - end + merge_requests.index_by(&:source_branch) end def get_branches(changes) diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb new file mode 100644 index 00000000000..44ae23fad18 --- /dev/null +++ b/app/services/notification_recipient_service.rb @@ -0,0 +1,293 @@ +# +# Used by NotificationService to determine who should receive notification +# +class NotificationRecipientService + attr_reader :project + + def initialize(project) + @project = project + end + + def build_recipients(target, current_user, action: nil, previous_assignee: nil, skip_current_user: true) + 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_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) + recipients << previous_assignee if previous_assignee + recipients << target.assignee + end + + recipients = reject_muted_users(recipients) + recipients = add_subscribed_users(recipients, target) + + if [:new_issue, :new_merge_request].include?(custom_action) + recipients = add_labels_subscribers(recipients, target) + end + + recipients = reject_unsubscribed_users(recipients, target) + recipients = reject_users_without_access(recipients, target) + + recipients.delete(current_user) if skip_current_user + + recipients.uniq + end + + def build_relabeled_recipients(target, current_user, labels:) + recipients = add_labels_subscribers([], target, labels: labels) + recipients = reject_unsubscribed_users(recipients, target) + recipients = reject_users_without_access(recipients, target) + recipients.delete(current_user) + recipients.uniq + end + + def build_new_note_recipients(note) + target = note.noteable + + ability, subject = if note.for_personal_snippet? + [:read_personal_snippet, note.noteable] + else + [:read_project, note.project] + end + + mentioned_users = note.mentioned_users.select { |user| user.can?(ability, subject) } + + # Add all users participating in the thread (author, assignee, comment authors) + recipients = + if target.respond_to?(:participants) + target.participants(note.author) + else + mentioned_users + end + + unless note.for_personal_snippet? + # Merge project watchers + recipients = add_project_watchers(recipients) + + # Merge project with custom notification + recipients = add_custom_notifications(recipients, :new_note) + end + + # Reject users with Mention notification level, except those mentioned in _this_ note. + recipients = reject_mention_users(recipients - mentioned_users) + recipients = recipients + mentioned_users + + recipients = reject_muted_users(recipients) + + recipients = add_subscribed_users(recipients, note.noteable) + recipients = reject_unsubscribed_users(recipients, note.noteable) + recipients = reject_users_without_access(recipients, note.noteable) + + recipients.delete(note.author) + recipients.uniq + end + + # Remove users with disabled notifications from array + # Also remove duplications and nil recipients + def reject_muted_users(users) + reject_users(users, :disabled) + end + + protected + + # Get project/group users with CUSTOM notification level + def add_custom_notifications(recipients, action) + user_ids = [] + + # Users with a notification setting on group or project + user_ids += user_ids_notifiable_on(project, :custom, action) + user_ids += user_ids_notifiable_on(project.group, :custom, action) + + # Users with global level custom + user_ids_with_project_level_global = user_ids_notifiable_on(project, :global) + user_ids_with_group_level_global = user_ids_notifiable_on(project.group, :global) + + global_users_ids = user_ids_with_project_level_global.concat(user_ids_with_group_level_global) + user_ids += user_ids_with_global_level_custom(global_users_ids, action) + + recipients.concat(User.find(user_ids)) + end + + def add_project_watchers(recipients) + recipients.concat(project_watchers).compact + end + + # Get project users with WATCH notification level + def project_watchers + project_members_ids = user_ids_notifiable_on(project) + + user_ids_with_project_global = user_ids_notifiable_on(project, :global) + user_ids_with_group_global = user_ids_notifiable_on(project.group, :global) + + user_ids = user_ids_with_global_level_watch((user_ids_with_project_global + user_ids_with_group_global).uniq) + + user_ids_with_project_setting = select_project_members_ids(project, user_ids_with_project_global, user_ids) + user_ids_with_group_setting = select_group_members_ids(project.group, project_members_ids, user_ids_with_group_global, user_ids) + + User.where(id: user_ids_with_project_setting.concat(user_ids_with_group_setting).uniq).to_a + end + + # Remove users with notification level 'Mentioned' + def reject_mention_users(users) + reject_users(users, :mention) + end + + def add_subscribed_users(recipients, target) + return recipients unless target.respond_to? :subscribers + + recipients + target.subscribers(project) + end + + def user_ids_notifiable_on(resource, notification_level = nil, action = nil) + return [] unless resource + + if notification_level + settings = resource.notification_settings.where(level: NotificationSetting.levels[notification_level]) + settings = settings.select { |setting| setting.events[action] } if action.present? + settings.map(&:user_id) + else + resource.notification_settings.pluck(:user_id) + end + end + + # Build a list of user_ids based on project notification settings + def select_project_members_ids(project, global_setting, user_ids_global_level_watch) + user_ids = user_ids_notifiable_on(project, :watch) + + # If project setting is global, add to watch list if global setting is watch + global_setting.each do |user_id| + if user_ids_global_level_watch.include?(user_id) + user_ids << user_id + end + end + + user_ids + end + + # Build a list of user_ids based on group notification settings + def select_group_members_ids(group, project_members, global_setting, user_ids_global_level_watch) + uids = user_ids_notifiable_on(group, :watch) + + # Group setting is watch, add to user_ids list if user is not project member + user_ids = [] + uids.each do |user_id| + if project_members.exclude?(user_id) + user_ids << user_id + end + end + + # Group setting is global, add to user_ids list if global setting is watch + global_setting.each do |user_id| + if project_members.exclude?(user_id) && user_ids_global_level_watch.include?(user_id) + user_ids << user_id + end + end + + user_ids + end + + def user_ids_with_global_level_watch(ids) + settings_with_global_level_of(:watch, ids).pluck(:user_id) + end + + def user_ids_with_global_level_custom(ids, action) + settings = settings_with_global_level_of(:custom, ids) + settings = settings.select { |setting| setting.events[action] } + settings.map(&:user_id) + end + + def settings_with_global_level_of(level, ids) + NotificationSetting.where( + user_id: ids, + source_type: nil, + level: NotificationSetting.levels[level] + ) + end + + # Reject users which has certain notification level + # + # Example: + # reject_users(users, :watch, project) + # + def reject_users(users, level) + level = level.to_s + + unless NotificationSetting.levels.keys.include?(level) + raise 'Invalid notification level' + end + + users = users.to_a.compact.uniq + users = users.select { |u| u.can?(:receive_notifications) } + + users.reject do |user| + global_notification_setting = user.global_notification_setting + + next global_notification_setting.level == level unless project + + setting = user.notification_settings_for(project) + + if project.group && (setting.nil? || setting.global?) + setting = user.notification_settings_for(project.group) + end + + # reject users who globally set mention notification and has no setting per project/group + next global_notification_setting.level == level unless setting + + # reject users who set mention notification in project + next true if setting.level == level + + # reject users who have mention level in project and disabled in global settings + setting.global? && global_notification_setting.level == level + end + end + + def reject_unsubscribed_users(recipients, target) + return recipients unless target.respond_to? :subscriptions + + recipients.reject do |user| + subscription = target.subscriptions.find_by_user_id(user.id) + subscription && !subscription.subscribed + end + end + + def reject_users_without_access(recipients, target) + ability = case target + when Issuable + :"read_#{target.to_ability_name}" + when Ci::Pipeline + :read_build # We have build trace in pipeline emails + end + + return recipients unless ability + + recipients.select do |user| + user.can?(ability, target) + end + end + + def add_labels_subscribers(recipients, target, labels: nil) + return recipients unless target.respond_to? :labels + + (labels || target.labels).each do |label| + recipients += label.subscribers(project) + end + + recipients + end + + # Build event key to search on custom notification level + # Check NotificationSetting::EMAIL_EVENTS + def build_custom_key(action, object) + "#{action}_#{object.class.model_name.name.underscore}".to_sym + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index d12692ecc90..f9aa2346759 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -150,7 +150,10 @@ class NotificationService end def resolve_all_discussions(merge_request, current_user) - recipients = build_recipients(merge_request, merge_request.target_project, current_user, action: "resolve_all_discussions") + recipients = NotificationRecipientService.new(merge_request.target_project).build_recipients( + merge_request, + current_user, + action: "resolve_all_discussions") recipients.each do |recipient| mailer.resolved_all_discussions_email(recipient.id, merge_request.id, current_user.id).deliver_later @@ -164,64 +167,15 @@ class NotificationService end # Notify users on new note in system - # - # TODO: split on methods and refactor - # def new_note(note) return true unless note.noteable_type.present? # ignore gitlab service messages return true if note.cross_reference? && note.system? - target = note.noteable - - recipients = [] - - mentioned_users = note.mentioned_users - - ability, subject = if note.for_personal_snippet? - [:read_personal_snippet, note.noteable] - else - [:read_project, note.project] - end - - mentioned_users.select! do |user| - user.can?(ability, subject) - end - - # Add all users participating in the thread (author, assignee, comment authors) - participants = - if target.respond_to?(:participants) - target.participants(note.author) - else - mentioned_users - end - - recipients = recipients.concat(participants) - - unless note.for_personal_snippet? - # Merge project watchers - recipients = add_project_watchers(recipients, note.project) - - # Merge project with custom notification - recipients = add_custom_notifications(recipients, note.project, :new_note) - end - - # Reject users with Mention notification level, except those mentioned in _this_ note. - recipients = reject_mention_users(recipients - mentioned_users, note.project) - recipients = recipients + mentioned_users - - recipients = reject_muted_users(recipients, note.project) - - recipients = add_subscribed_users(recipients, note.project, note.noteable) - recipients = reject_unsubscribed_users(recipients, note.noteable) - recipients = reject_users_without_access(recipients, note.noteable) - - recipients.delete(note.author) unless note.author.notified_of_own_activity? - recipients = recipients.uniq - notify_method = "note_#{note.to_ability_name}_email".to_sym + recipients = NotificationRecipientService.new(note.project).build_new_note_recipients(note) recipients.each do |recipient| mailer.send(notify_method, recipient.id, note.id).deliver_later end @@ -290,7 +244,7 @@ class NotificationService def project_was_moved(project, old_path_with_namespace) recipients = project.team.members - recipients = reject_muted_users(recipients, project) + recipients = NotificationRecipientService.new(project).reject_muted_users(recipients) recipients.each do |recipient| mailer.project_was_moved_email( @@ -302,7 +256,7 @@ class NotificationService end def issue_moved(issue, new_issue, current_user) - recipients = build_recipients(issue, issue.project, current_user) + recipients = NotificationRecipientService.new(issue.project).build_recipients(issue, current_user) recipients.map do |recipient| email = mailer.issue_moved_email(recipient, issue, new_issue, current_user) @@ -324,12 +278,10 @@ class NotificationService return unless mailer.respond_to?(email_template) - recipients ||= build_recipients( + recipients ||= NotificationRecipientService.new(pipeline.project).build_recipients( pipeline, - pipeline.project, - pipeline.user, - action: pipeline.status, - skip_current_user: false).map(&:notification_email) + nil, # The acting user, who won't be added to recipients + action: pipeline.status).map(&:notification_email) if recipients.any? mailer.public_send(email_template, pipeline, recipients).deliver_later @@ -338,199 +290,8 @@ class NotificationService protected - # Get project/group users with CUSTOM notification level - def add_custom_notifications(recipients, project, action) - user_ids = [] - - # Users with a notification setting on group or project - user_ids += notification_settings_for(project, :custom, action) - user_ids += notification_settings_for(project.group, :custom, action) - - # Users with global level custom - users_with_project_level_global = notification_settings_for(project, :global) - users_with_group_level_global = notification_settings_for(project.group, :global) - - global_users_ids = users_with_project_level_global.concat(users_with_group_level_global) - user_ids += users_with_global_level_custom(global_users_ids, action) - - recipients.concat(User.find(user_ids)) - end - - # Get project users with WATCH notification level - def project_watchers(project) - project_members = notification_settings_for(project) - - users_with_project_level_global = notification_settings_for(project, :global) - users_with_group_level_global = notification_settings_for(project.group, :global) - - users = users_with_global_level_watch([users_with_project_level_global, users_with_group_level_global].flatten.uniq) - - users_with_project_setting = select_project_member_setting(project, users_with_project_level_global, users) - users_with_group_setting = select_group_member_setting(project.group, project_members, users_with_group_level_global, users) - - User.where(id: users_with_project_setting.concat(users_with_group_setting).uniq).to_a - end - - def notification_settings_for(resource, notification_level = nil, action = nil) - return [] unless resource - - if notification_level - settings = resource.notification_settings.where(level: NotificationSetting.levels[notification_level]) - settings = settings.select { |setting| setting.events[action] } if action.present? - settings.map(&:user_id) - else - resource.notification_settings.pluck(:user_id) - end - end - - def users_with_global_level_watch(ids) - settings_with_global_level_of(:watch, ids).pluck(:user_id) - end - - def users_with_global_level_custom(ids, action) - settings = settings_with_global_level_of(:custom, ids) - settings = settings.select { |setting| setting.events[action] } - settings.map(&:user_id) - end - - def settings_with_global_level_of(level, ids) - NotificationSetting.where( - user_id: ids, - source_type: nil, - level: NotificationSetting.levels[level] - ) - end - - # Build a list of users based on project notification settings - def select_project_member_setting(project, global_setting, users_global_level_watch) - users = notification_settings_for(project, :watch) - - # If project setting is global, add to watch list if global setting is watch - global_setting.each do |user_id| - if users_global_level_watch.include?(user_id) - users << user_id - end - end - - users - end - - # Build a list of users based on group notification settings - def select_group_member_setting(group, project_members, global_setting, users_global_level_watch) - uids = notification_settings_for(group, :watch) - - # Group setting is watch, add to users list if user is not project member - users = [] - uids.each do |user_id| - if project_members.exclude?(user_id) - users << user_id - end - end - - # Group setting is global, add to users list if global setting is watch - global_setting.each do |user_id| - if project_members.exclude?(user_id) && users_global_level_watch.include?(user_id) - users << user_id - end - end - - users - end - - def add_project_watchers(recipients, project) - recipients.concat(project_watchers(project)).compact - end - - # Remove users with disabled notifications from array - # Also remove duplications and nil recipients - def reject_muted_users(users, project = nil) - reject_users(users, :disabled, project) - end - - # Remove users with notification level 'Mentioned' - def reject_mention_users(users, project = nil) - reject_users(users, :mention, project) - end - - # Reject users which has certain notification level - # - # Example: - # reject_users(users, :watch, project) - # - def reject_users(users, level, project = nil) - level = level.to_s - - unless NotificationSetting.levels.keys.include?(level) - raise 'Invalid notification level' - end - - users = users.to_a.compact.uniq - users = users.select { |u| u.can?(:receive_notifications) } - - users.reject do |user| - global_notification_setting = user.global_notification_setting - - next global_notification_setting.level == level unless project - - setting = user.notification_settings_for(project) - - if project.group && (setting.nil? || setting.global?) - setting = user.notification_settings_for(project.group) - end - - # reject users who globally set mention notification and has no setting per project/group - next global_notification_setting.level == level unless setting - - # reject users who set mention notification in project - next true if setting.level == level - - # reject users who have mention level in project and disabled in global settings - setting.global? && global_notification_setting.level == level - end - end - - def reject_unsubscribed_users(recipients, target) - return recipients unless target.respond_to? :subscriptions - - recipients.reject do |user| - subscription = target.subscriptions.find_by_user_id(user.id) - subscription && !subscription.subscribed - end - end - - def reject_users_without_access(recipients, target) - ability = case target - when Issuable - :"read_#{target.to_ability_name}" - when Ci::Pipeline - :read_build # We have build trace in pipeline emails - end - - return recipients unless ability - - recipients.select do |user| - user.can?(ability, target) - end - end - - def add_subscribed_users(recipients, project, target) - return recipients unless target.respond_to? :subscribers - - recipients + target.subscribers(project) - end - - def add_labels_subscribers(recipients, project, target, labels: nil) - return recipients unless target.respond_to? :labels - - (labels || target.labels).each do |label| - recipients += label.subscribers(project) - end - - recipients - end - def new_resource_email(target, project, method) - recipients = build_recipients(target, project, target.author, action: "new") + recipients = NotificationRecipientService.new(project).build_recipients(target, target.author, action: "new") recipients.each do |recipient| mailer.send(method, recipient.id, target.id).deliver_later @@ -538,7 +299,7 @@ class NotificationService end def new_mentions_in_resource_email(target, project, new_mentioned_users, current_user, method) - recipients = build_recipients(target, project, current_user, action: "new") + recipients = NotificationRecipientService.new(project).build_recipients(target, current_user, action: "new") recipients = recipients & new_mentioned_users recipients.each do |recipient| @@ -549,9 +310,8 @@ class NotificationService def close_resource_email(target, project, current_user, method, skip_current_user: true) action = method == :merged_merge_request_email ? "merge" : "close" - recipients = build_recipients( + recipients = NotificationRecipientService.new(project).build_recipients( target, - project, current_user, action: action, skip_current_user: skip_current_user @@ -566,7 +326,12 @@ class NotificationService previous_assignee_id = previous_record(target, 'assignee_id') previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id - recipients = build_recipients(target, project, current_user, action: "reassign", previous_assignee: previous_assignee) + recipients = NotificationRecipientService.new(project).build_recipients( + target, + current_user, + action: "reassign", + previous_assignee: previous_assignee + ) recipients.each do |recipient| mailer.send( @@ -580,7 +345,7 @@ class NotificationService end def relabeled_resource_email(target, project, labels, current_user, method) - recipients = build_relabeled_recipients(target, project, current_user, labels: labels) + recipients = NotificationRecipientService.new(project).build_relabeled_recipients(target, current_user, labels: labels) label_names = labels.map(&:name) recipients.each do |recipient| @@ -589,58 +354,13 @@ class NotificationService end def reopen_resource_email(target, project, current_user, method, status) - recipients = build_recipients(target, project, current_user, action: "reopen") + recipients = NotificationRecipientService.new(project).build_recipients(target, current_user, action: "reopen") recipients.each do |recipient| mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later end end - def build_recipients(target, project, current_user, action: nil, previous_assignee: nil, skip_current_user: true) - 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, project) - end - - recipients = add_custom_notifications(recipients, project, custom_action) - recipients = reject_mention_users(recipients, project) - - recipients = recipients.uniq - - # 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) - recipients << previous_assignee if previous_assignee - recipients << target.assignee - end - - recipients = reject_muted_users(recipients, project) - recipients = add_subscribed_users(recipients, project, target) - - if [:new_issue, :new_merge_request].include?(custom_action) - recipients = add_labels_subscribers(recipients, project, target) - end - - recipients = reject_unsubscribed_users(recipients, target) - recipients = reject_users_without_access(recipients, target) - - recipients.delete(current_user) if skip_current_user && !current_user.notified_of_own_activity? - - recipients.uniq - end - - def build_relabeled_recipients(target, project, current_user, labels:) - recipients = add_labels_subscribers([], project, target, labels: labels) - recipients = reject_unsubscribed_users(recipients, target) - recipients = reject_users_without_access(recipients, target) - recipients.delete(current_user) unless current_user.notified_of_own_activity? - recipients.uniq - end - def mailer Notify end @@ -652,10 +372,4 @@ class NotificationService end end end - - # Build event key to search on custom notification level - # Check NotificationSetting::EMAIL_EVENTS - def build_custom_key(action, object) - "#{action}_#{object.class.model_name.name.underscore}".to_sym - end end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index 1c5a549feb9..d484a96f785 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -33,6 +33,7 @@ module Projects def import_repository 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 # Expire cache to prevent scenarios such as: @@ -40,7 +41,7 @@ module Projects # 2. Retried import, repo is broken or not imported but +exists?+ still returns true project.repository.before_import if project.repository_exists? - raise Error, "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}" + raise Error, "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}" end end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 868fa7b3f21..af0ddbe5934 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -24,10 +24,9 @@ class SystemHooksService key: model.key, id: model.id ) + if model.user - data.merge!( - username: model.user.username - ) + data[:username] = model.user.username end when Project data.merge!(project_data(model)) @@ -35,8 +34,6 @@ class SystemHooksService if event == :rename || event == :transfer data[:old_path_with_namespace] = model.old_path_with_namespace end - - data when User data.merge!({ name: model.name, @@ -59,6 +56,8 @@ class SystemHooksService when GroupMember data.merge!(group_member_data(model)) end + + data end def build_event_name(model, event) diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 8787a1c93a9..bf7e76ec59e 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -201,10 +201,12 @@ class TodoService def update_todos_state_by_ids(ids, current_user, state) todos = current_user.todos.where(id: ids) - # Only return those that are not really on that state - marked_todos = todos.where.not(state: state).update_all(state: state) + # 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) current_user.update_todos_count_cache - marked_todos + todos_ids end def create_todos(users, attributes) diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index d9370bbb598..8f6f5b937c4 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -93,9 +93,7 @@ module Users end def current_authorizations_per_project - current_authorizations.each_with_object({}) do |row, hash| - hash[row.project_id] = row - end + current_authorizations.index_by(&:project_id) end def current_authorizations |