Add changes from the EE

8 files changed, 33 insertions, 24 deletions

diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index c505a5cc8d5..6e9c8ea6fde 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -155,11 +155,11 @@ class TodosFinder
   def by_group(items)
     if group?
       groups = group.self_and_descendants
-      items = items.where(
-        'project_id IN (?) OR group_id IN (?)',
-        Project.where(group: groups).select(:id),
-        groups.select(:id)
-      )
+      project_todos = items.where(project_id: Project.where(group: groups).select(:id))
+      group_todos = items.where(group_id: groups.select(:id))
+
+      union = Gitlab::SQL::Union.new([project_todos, group_todos])
+      items = Todo.from("(#{union.to_sql}) #{Todo.table_name}")
     end
 
     items
diff --git a/app/models/group.rb b/app/models/group.rb
index 28677320e28..084cae3101a 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -84,12 +84,6 @@ class Group < Namespace
       where(id: user.authorized_groups.select(:id).reorder(nil))
     end
 
-    def public_or_visible_to_user(user)
-      where('id IN (?) OR namespaces.visibility_level IN (?)',
-        user.authorized_groups.select(:id),
-        Gitlab::VisibilityLevel.levels_for_user(user))
-    end
-
     def select_for_project_authorization
       if current_scope.joins_values.include?(:shared_projects)
         joins('INNER JOIN namespaces project_namespace ON project_namespace.id = projects.namespace_id')
diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb
index ef6add352e7..045f5ecaae7 100644
--- a/app/services/todos/destroy/entity_leave_service.rb
+++ b/app/services/todos/destroy/entity_leave_service.rb
@@ -2,7 +2,6 @@ module Todos
   module Destroy
     class EntityLeaveService < ::Todos::Destroy::BaseService
       extend ::Gitlab::Utils::Override
-      include Gitlab::Utils::StrongMemoize
 
       attr_reader :user, :entity
 
@@ -19,7 +18,7 @@ module Todos
         return unless entity && user
 
         # if at least reporter, all entities including confidential issues can be accessed
-        return if main_group_reporter?
+        return if user_has_reporter_access?
 
         remove_confidential_issue_todos
 
@@ -81,7 +80,7 @@ module Todos
           .where('id NOT IN (?)', user.membership_groups.select(:id))
       end
 
-      def main_group_reporter?
+      def user_has_reporter_access?
         return unless entity.is_a?(Namespace)
 
         entity.member?(User.find(user.id), Gitlab::Access::REPORTER)
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index e8b9999f83b..f95df7ecf03 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -77,6 +77,7 @@
 - todos_destroyer:todos_destroyer_entity_leave
 - todos_destroyer:todos_destroyer_project_private
 - todos_destroyer:todos_destroyer_private_features
+- todos_destroyer:todos_destroyer_group_private
 
 - default
 - mailers # ActionMailer::DeliveryJob.queue_name
diff --git a/db/migrate/20180608091413_add_group_to_todos.rb b/db/migrate/20180608091413_add_group_to_todos.rb
index af3ee48b29d..20ba4849057 100644
--- a/db/migrate/20180608091413_add_group_to_todos.rb
+++ b/db/migrate/20180608091413_add_group_to_todos.rb
@@ -5,8 +5,14 @@ class AddGroupToTodos < ActiveRecord::Migration
 
   disable_ddl_transaction!
 
+  class Todo < ActiveRecord::Base
+    self.table_name = 'todos'
+
+    include ::EachBatch
+  end
+
   def up
-    add_column :todos, :group_id, :integer
+    add_column(:todos, :group_id, :integer) unless group_id_exists?
     add_concurrent_foreign_key :todos, :namespaces, column: :group_id, on_delete: :cascade
     add_concurrent_index :todos, :group_id
 
@@ -14,13 +20,11 @@ class AddGroupToTodos < ActiveRecord::Migration
   end
 
   def down
-    return unless group_id_exists?
-
-    remove_foreign_key :todos, column: :group_id
-    remove_index :todos, :group_id if index_exists?(:todos, :group_id)
-    remove_column :todos, :group_id
+    remove_foreign_key_without_error(:todos, column: :group_id)
+    remove_concurrent_index(:todos, :group_id)
+    remove_column(:todos, :group_id) if group_id_exists?
 
-    execute "DELETE FROM todos WHERE project_id IS NULL"
+    Todo.where(project_id: nil).each_batch { |batch| batch.delete_all }
     change_column_null :todos, :project_id, false
   end
 
diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb
index 2ee8d150dc8..b5cf04e7f22 100644
--- a/spec/requests/api/todos_spec.rb
+++ b/spec/requests/api/todos_spec.rb
@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe API::Todos do
-  let(:project_1) { create(:project, :repository) }
+  let(:group)     { create(:group) }
+  let(:project_1) { create(:project, :repository, group: group) }
   let(:project_2) { create(:project) }
   let(:author_1) { create(:user) }
   let(:author_2) { create(:user) }
@@ -92,6 +93,17 @@ describe API::Todos do
         end
       end
 
+      context 'and using the group filter' do
+        it 'filters based on project_id param' do
+          get api('/todos', john_doe), { group_id: group.id, sort: :target_id }
+
+          expect(response.status).to eq(200)
+          expect(response).to include_pagination_headers
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(2)
+        end
+      end
+
       context 'and using the action filter' do
         it 'filters based on action param' do
           get api('/todos', john_doe), { action: 'mentioned' }
diff --git a/spec/services/todos/destroy/group_private_service_spec.rb b/spec/services/todos/destroy/group_private_service_spec.rb
index c4ee6ebed50..2f49b68f544 100644
--- a/spec/services/todos/destroy/group_private_service_spec.rb
+++ b/spec/services/todos/destroy/group_private_service_spec.rb
@@ -33,7 +33,7 @@ describe Todos::Destroy::GroupPrivateService do
         expect(project_member.todos).to match_array([todo_project_member])
       end
 
-      context 'with nested groups' do
+      context 'with nested groups', :nested_groups do
         let(:parent_group) { create(:group) }
         let(:subgroup)     { create(:group, :private, parent: group) }
         let(:subproject)   { create(:project, group: subgroup) }
diff --git a/spec/services/todos/destroy/project_private_service_spec.rb b/spec/services/todos/destroy/project_private_service_spec.rb
index 9ebc0231795..128d3487514 100644
--- a/spec/services/todos/destroy/project_private_service_spec.rb
+++ b/spec/services/todos/destroy/project_private_service_spec.rb
@@ -10,7 +10,6 @@ describe Todos::Destroy::ProjectPrivateService do
   let!(:todo_non_member)   { create(:todo, user: user, project: project) }
   let!(:todo2_non_member)  { create(:todo, user: user, project: project) }
   let!(:todo_member)       { create(:todo, user: project_member, project: project) }
-  let!(:todo_member)       { create(:todo, user: project_member, project: project) }
   let!(:todo_group_member) { create(:todo, user: group_member, project: project) }
 
   describe '#execute' do