diff options
Diffstat (limited to 'app/services')
| -rw-r--r-- | app/services/ci/compare_test_reports_service.rb | 43 | ||||
| -rw-r--r-- | app/services/groups/update_service.rb | 11 | ||||
| -rw-r--r-- | app/services/todo_service.rb | 30 | ||||
| -rw-r--r-- | app/services/todos/destroy/entity_leave_service.rb | 90 | ||||
| -rw-r--r-- | app/services/todos/destroy/group_private_service.rb | 30 | ||||
| -rw-r--r-- | app/services/todos/destroy/project_private_service.rb | 2 |
6 files changed, 150 insertions, 56 deletions
diff --git a/app/services/ci/compare_test_reports_service.rb b/app/services/ci/compare_test_reports_service.rb index 7a112211d94..ec25e934a27 100644 --- a/app/services/ci/compare_test_reports_service.rb +++ b/app/services/ci/compare_test_reports_service.rb @@ -2,23 +2,36 @@ module Ci class CompareTestReportsService < ::BaseService - def execute(base_pipeline_iid, head_pipeline_iid) - base_pipeline = project.pipelines.find_by_iid(base_pipeline_iid) if base_pipeline_iid - head_pipeline = project.pipelines.find_by_iid(head_pipeline_iid) + def execute(base_pipeline, head_pipeline) + comparer = Gitlab::Ci::Reports::TestReportsComparer + .new(base_pipeline&.test_reports, head_pipeline.test_reports) - begin - comparer = Gitlab::Ci::Reports::TestReportsComparer - .new(base_pipeline&.test_reports, head_pipeline.test_reports) + { + status: :parsed, + key: key(base_pipeline, head_pipeline), + data: TestReportsComparerSerializer + .new(project: project) + .represent(comparer).as_json + } + rescue => e + { + status: :error, + key: key(base_pipeline, head_pipeline), + status_reason: e.message + } + end + + def latest?(base_pipeline, head_pipeline, data) + data&.fetch(:key, nil) == key(base_pipeline, head_pipeline) + end + + private - { - status: :parsed, - data: TestReportsComparerSerializer - .new(project: project) - .represent(comparer).as_json - } - rescue => e - { status: :error, status_reason: e.message } - end + def key(base_pipeline, head_pipeline) + [ + base_pipeline&.id, base_pipeline&.updated_at, + head_pipeline&.id, head_pipeline&.updated_at + ] end end end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 436a6b18cb1..fe47aa2f140 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -14,7 +14,9 @@ module Groups group.assign_attributes(params) begin - group.save + after_update if group.save + + true rescue Gitlab::UpdatePathError => e group.errors.add(:base, e.message) @@ -24,6 +26,13 @@ module Groups private + def after_update + if group.previous_changes.include?(:visibility_level) && group.private? + # don't enqueue immediately to prevent todos removal in case of a mistake + TodosDestroyer::GroupPrivateWorker.perform_in(1.hour, group.id) + end + end + def reject_parent_id! params.except!(:parent_id) end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 0bcd53c76a9..0df61ad3bce 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -262,15 +262,15 @@ class TodoService end end - def create_mention_todos(project, target, author, note = nil, skip_users = []) + def create_mention_todos(parent, target, author, note = nil, skip_users = []) # Create Todos for directly addressed users - directly_addressed_users = filter_directly_addressed_users(project, note || target, author, skip_users) - attributes = attributes_for_todo(project, target, author, Todo::DIRECTLY_ADDRESSED, note) + directly_addressed_users = filter_directly_addressed_users(parent, note || target, author, skip_users) + attributes = attributes_for_todo(parent, target, author, Todo::DIRECTLY_ADDRESSED, note) create_todos(directly_addressed_users, attributes) # Create Todos for mentioned users - mentioned_users = filter_mentioned_users(project, note || target, author, skip_users) - attributes = attributes_for_todo(project, target, author, Todo::MENTIONED, note) + mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users) + attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note) create_todos(mentioned_users, attributes) end @@ -301,36 +301,36 @@ class TodoService def attributes_for_todo(project, target, author, action, note = nil) attributes_for_target(target).merge!( - project_id: project.id, + project_id: project&.id, author_id: author.id, action: action, note: note ) end - def filter_todo_users(users, project, target) - reject_users_without_access(users, project, target).uniq + def filter_todo_users(users, parent, target) + reject_users_without_access(users, parent, target).uniq end - def filter_mentioned_users(project, target, author, skip_users = []) + def filter_mentioned_users(parent, target, author, skip_users = []) mentioned_users = target.mentioned_users(author) - skip_users - filter_todo_users(mentioned_users, project, target) + filter_todo_users(mentioned_users, parent, target) end - def filter_directly_addressed_users(project, target, author, skip_users = []) + def filter_directly_addressed_users(parent, target, author, skip_users = []) directly_addressed_users = target.directly_addressed_users(author) - skip_users - filter_todo_users(directly_addressed_users, project, target) + filter_todo_users(directly_addressed_users, parent, target) end - def reject_users_without_access(users, project, target) - if target.is_a?(Note) && (target.for_issue? || target.for_merge_request?) + def reject_users_without_access(users, parent, target) + if target.is_a?(Note) && target.for_issuable? target = target.noteable end if target.is_a?(Issuable) select_users(users, :"read_#{target.to_ability_name}", target) else - select_users(users, :read_project, project) + select_users(users, :read_project, parent) end end diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb index 2ff9f94b718..045f5ecaae7 100644 --- a/app/services/todos/destroy/entity_leave_service.rb +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -3,55 +3,97 @@ module Todos class EntityLeaveService < ::Todos::Destroy::BaseService extend ::Gitlab::Utils::Override - attr_reader :user_id, :entity + attr_reader :user, :entity def initialize(user_id, entity_id, entity_type) unless %w(Group Project).include?(entity_type) raise ArgumentError.new("#{entity_type} is not an entity user can leave") end - @user_id = user_id + @user = User.find_by(id: user_id) @entity = entity_type.constantize.find_by(id: entity_id) end - private + def execute + return unless entity && user + + # if at least reporter, all entities including confidential issues can be accessed + return if user_has_reporter_access? + + remove_confidential_issue_todos - override :todos - def todos if entity.private? - Todo.where(project_id: project_ids, user_id: user_id) + remove_project_todos + remove_group_todos else - project_ids.each do |project_id| - TodosDestroyer::PrivateFeaturesWorker.perform_async(project_id, user_id) - end + enqueue_private_features_worker + end + end + + private - Todo.where( - target_id: confidential_issues.select(:id), target_type: Issue, user_id: user_id - ) + def enqueue_private_features_worker + project_ids.each do |project_id| + TodosDestroyer::PrivateFeaturesWorker.perform_async(project_id, user.id) end end + def remove_confidential_issue_todos + Todo.where( + target_id: confidential_issues.select(:id), target_type: Issue, user_id: user.id + ).delete_all + end + + def remove_project_todos + Todo.where(project_id: non_authorized_projects, user_id: user.id).delete_all + end + + def remove_group_todos + Todo.where(group_id: non_authorized_groups, user_id: user.id).delete_all + end + override :project_ids def project_ids - case entity - when Project - [entity.id] - when Namespace - Project.select(:id).where(namespace_id: entity.self_and_descendants.select(:id)) - end + condition = case entity + when Project + { id: entity.id } + when Namespace + { namespace_id: non_member_groups } + end + + Project.where(condition).select(:id) end - override :todos_to_remove? - def todos_to_remove? - # if an entity is provided we want to check always at least private features - !!entity + def non_authorized_projects + project_ids.where('id NOT IN (?)', user.authorized_projects.select(:id)) + end + + def non_authorized_groups + return [] unless entity.is_a?(Namespace) + + entity.self_and_descendants.select(:id) + .where('id NOT IN (?)', GroupsFinder.new(user).execute.select(:id)) + end + + def non_member_groups + entity.self_and_descendants.select(:id) + .where('id NOT IN (?)', user.membership_groups.select(:id)) + end + + def user_has_reporter_access? + return unless entity.is_a?(Namespace) + + entity.member?(User.find(user.id), Gitlab::Access::REPORTER) end def confidential_issues - assigned_ids = IssueAssignee.select(:issue_id).where(user_id: user_id) + assigned_ids = IssueAssignee.select(:issue_id).where(user_id: user.id) + authorized_reporter_projects = user + .authorized_projects(Gitlab::Access::REPORTER).select(:id) Issue.where(project_id: project_ids, confidential: true) - .where('author_id != ?', user_id) + .where('project_id NOT IN(?)', authorized_reporter_projects) + .where('author_id != ?', user.id) .where('id NOT IN (?)', assigned_ids) end end diff --git a/app/services/todos/destroy/group_private_service.rb b/app/services/todos/destroy/group_private_service.rb new file mode 100644 index 00000000000..d13fa7a6516 --- /dev/null +++ b/app/services/todos/destroy/group_private_service.rb @@ -0,0 +1,30 @@ +module Todos + module Destroy + class GroupPrivateService < ::Todos::Destroy::BaseService + extend ::Gitlab::Utils::Override + + attr_reader :group + + def initialize(group_id) + @group = Group.find_by(id: group_id) + end + + private + + override :todos + def todos + Todo.where(group_id: group.id) + end + + override :authorized_users + def authorized_users + group.direct_and_indirect_users.select(:id) + end + + override :todos_to_remove? + def todos_to_remove? + group&.private? + end + end + end +end diff --git a/app/services/todos/destroy/project_private_service.rb b/app/services/todos/destroy/project_private_service.rb index 171933e7cbc..315a0c33398 100644 --- a/app/services/todos/destroy/project_private_service.rb +++ b/app/services/todos/destroy/project_private_service.rb @@ -13,7 +13,7 @@ module Todos override :todos def todos - Todo.where(project_id: project_ids) + Todo.where(project_id: project.id) end override :project_ids |