5 files changed, 73 insertions, 23 deletions

diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index 09e2c586f2a..1dfcf19b78d 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -15,6 +15,7 @@
 class TodosFinder
   prepend FinderWithCrossProjectAccess
   include FinderMethods
+  include Gitlab::Utils::StrongMemoize
 
   requires_cross_project_access unless: -> { project? }
 
@@ -34,9 +35,11 @@ class TodosFinder
     items = by_author(items)
     items = by_state(items)
     items = by_type(items)
+    items = by_group(items)
     # Filtering by project HAS TO be the last because we use
     # the project IDs yielded by the todos query thus far
     items = by_project(items)
+    items = visible_to_user(items)
 
     sort(items)
   end
@@ -82,6 +85,10 @@ class TodosFinder
     params[:project_id].present?
   end
 
+  def group?
+    params[:group_id].present?
+  end
+
   def project
     return @project if defined?(@project)
 
@@ -100,6 +107,12 @@ class TodosFinder
     @project
   end
 
+  def group
+    strong_memoize(:group) do
+      Group.find(params[:group_id])
+    end
+  end
+
   def project_ids(items)
     ids = items.except(:order).select(:project_id)
     if Gitlab::Database.mysql?
@@ -111,7 +124,7 @@ class TodosFinder
   end
 
   def type?
-    type.present? && %w(Issue MergeRequest).include?(type)
+    type.present? && %w(Issue MergeRequest Epic).include?(type)
   end
 
   def type
@@ -148,12 +161,32 @@ class TodosFinder
 
   def by_project(items)
     if project?
-      items.where(project: project)
-    else
-      projects = Project.public_or_visible_to_user(current_user)
+      items = items.where(project: project)
+    end
+
+    items
+  end
 
-      items.joins(:project).merge(projects)
+  def by_group(items)
+    if group?
+      items = items.where(group: group)
     end
+
+    items
+  end
+
+  def visible_to_user(items)
+    projects = Project.public_or_visible_to_user(current_user)
+    groups = Group.public_or_visible_to_user(current_user)
+
+    items
+      .joins('LEFT JOIN namespaces ON namespaces.id = todos.group_id')
+      .joins('LEFT JOIN projects ON projects.id = todos.project_id')
+      .where(
+        'project_id IN (?) OR group_id IN  (?)',
+        projects.map(&:id),
+        groups.map(&:id)
+      )
   end
 
   def by_state(items)
diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index f7620e0b6b8..7d78ceb1f9a 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -43,7 +43,7 @@ module TodosHelper
       project_commit_path(todo.project,
                                     todo.target, anchor: anchor)
     else
-      path = [todo.project.namespace.becomes(Namespace), todo.project, todo.target]
+      path = [todo.parent, todo.target]
 
       path.unshift(:pipelines) if todo.build_failed?
 
diff --git a/app/models/group.rb b/app/models/group.rb
index 9c171de7fc3..9baf2cfd810 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -39,6 +39,8 @@ class Group < Namespace
   has_many :boards
   has_many :badges, class_name: 'GroupBadge'
 
+  has_many :todos
+
   accepts_nested_attributes_for :variables, allow_destroy: true
 
   validate :visibility_level_allowed_by_projects
@@ -82,6 +84,13 @@ 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/models/todo.rb b/app/models/todo.rb
index a2ab405fdbe..5ce77d5ddc2 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -22,15 +22,18 @@ class Todo < ActiveRecord::Base
   belongs_to :author, class_name: "User"
   belongs_to :note
   belongs_to :project
+  belongs_to :group
   belongs_to :target, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations
   belongs_to :user
 
   delegate :name, :email, to: :author, prefix: true, allow_nil: true
 
-  validates :action, :project, :target_type, :user, presence: true
+  validates :action, :target_type, :user, presence: true
   validates :author, presence: true
   validates :target_id, presence: true, unless: :for_commit?
   validates :commit_id, presence: true, if: :for_commit?
+  validates :project, presence: true, unless: :group
+  validates :group, presence: true, unless: :project
 
   scope :pending, -> { with_state(:pending) }
   scope :done, -> { with_state(:done) }
@@ -44,7 +47,7 @@ class Todo < ActiveRecord::Base
     state :done
   end
 
-  after_save :keep_around_commit
+  after_save :keep_around_commit, if: :commit_id
 
   class << self
     # Priority sorting isn't displayed in the dropdown, because we don't show
@@ -79,6 +82,10 @@ class Todo < ActiveRecord::Base
     end
   end
 
+  def parent
+    project || group
+  end
+
   def unmergeable?
     action == UNMERGEABLE
   end
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index f91cd03bf5c..f355d6b8ea1 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -260,15 +260,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
 
@@ -299,36 +299,37 @@ 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,
+      group_id: target.respond_to?(:group) ? target.group.id : nil,
       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_issue? || target.for_merge_request? || target.for_epic?)
       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