diff options
Diffstat (limited to 'app/services/todos')
4 files changed, 60 insertions, 5 deletions
diff --git a/app/services/todos/destroy/base_service.rb b/app/services/todos/destroy/base_service.rb index 7378f10e7c4..4e971246185 100644 --- a/app/services/todos/destroy/base_service.rb +++ b/app/services/todos/destroy/base_service.rb @@ -13,7 +13,7 @@ module Todos # rubocop: disable CodeReuse/ActiveRecord def without_authorized(items) - items.where('todos.user_id NOT IN (?)', authorized_users) + items.where.not('todos.user_id' => authorized_users) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/todos/destroy/destroyed_issuable_service.rb b/app/services/todos/destroy/destroyed_issuable_service.rb new file mode 100644 index 00000000000..db12965224b --- /dev/null +++ b/app/services/todos/destroy/destroyed_issuable_service.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module Todos + module Destroy + class DestroyedIssuableService + BATCH_SIZE = 100 + + def initialize(target_id, target_type) + @target_id = target_id + @target_type = target_type + end + + def execute + inner_query = Todo.select(:id).for_target(target_id).for_type(target_type).limit(BATCH_SIZE) + + delete_query = <<~SQL + DELETE FROM "#{Todo.table_name}" + WHERE id IN (#{inner_query.to_sql}) + RETURNING user_id + SQL + + loop do + result = ActiveRecord::Base.connection.execute(delete_query) + + break if result.cmd_tuples == 0 + + user_ids = result.map { |row| row['user_id'] }.uniq + + invalidate_todos_cache_counts(user_ids) + end + end + + private + + attr_reader :target_id, :target_type + + def invalidate_todos_cache_counts(user_ids) + user_ids.each do |id| + # Only build a user instance since we only need its ID for + # `User#invalidate_todos_cache_counts` to work. + User.new(id: id).invalidate_todos_cache_counts + end + end + end + end +end diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb index 7cfedc2233a..6d4fc3865ac 100644 --- a/app/services/todos/destroy/entity_leave_service.rb +++ b/app/services/todos/destroy/entity_leave_service.rb @@ -65,8 +65,10 @@ module Todos end def remove_group_todos + return unless entity.is_a?(Namespace) + Todo - .for_group(non_authorized_groups) + .for_group(non_authorized_non_public_groups) .for_user(user) .delete_all end @@ -102,12 +104,19 @@ module Todos GroupsFinder.new(user, min_access_level: Gitlab::Access::REPORTER).execute.select(:id) end - def non_authorized_groups + # since the entity is a private group, we can assume all subgroups are also + # private. We can therefore limit GroupsFinder with `all_available: false`. + # Otherwise it tries to include all public groups. This generates an expensive + # SQL queries: https://gitlab.com/gitlab-org/gitlab/-/issues/325133 + # rubocop: disable CodeReuse/ActiveRecord + def non_authorized_non_public_groups return [] unless entity.is_a?(Namespace) + return [] unless entity.private? entity.self_and_descendants.select(:id) - .id_not_in(GroupsFinder.new(user).execute.select(:id)) + .id_not_in(GroupsFinder.new(user, all_available: false).execute.select(:id).reorder(nil)) end + # rubocop: enable CodeReuse/ActiveRecord def non_authorized_reporter_groups entity.self_and_descendants.select(:id) diff --git a/app/services/todos/destroy/private_features_service.rb b/app/services/todos/destroy/private_features_service.rb index bd49519d694..44c3ff231f8 100644 --- a/app/services/todos/destroy/private_features_service.rb +++ b/app/services/todos/destroy/private_features_service.rb @@ -36,7 +36,7 @@ module Todos items = Todo.where(project_id: project_id) items = items.where(user_id: user_id) if user_id - items.where('user_id NOT IN (?)', authorized_users) + items.where.not(user_id: authorized_users) .where(target_type: target_types) .delete_all end |