Support todos for epics backport
This commit is contained in:
parent
275fbf24b1
commit
57a44f2da3
|
@ -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.joins(:project).merge(projects)
|
||||
items = items.where(project: project)
|
||||
end
|
||||
|
||||
items
|
||||
end
|
||||
|
||||
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)
|
||||
|
|
|
@ -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?
|
||||
|
||||
|
|
|
@ -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')
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -0,0 +1,32 @@
|
|||
class AddGroupToTodos < ActiveRecord::Migration
|
||||
include Gitlab::Database::MigrationHelpers
|
||||
|
||||
DOWNTIME = false
|
||||
|
||||
disable_ddl_transaction!
|
||||
|
||||
def up
|
||||
add_column :todos, :group_id, :integer
|
||||
add_foreign_key :todos, :namespaces, column: :group_id, on_delete: :cascade
|
||||
add_concurrent_index :todos, :group_id
|
||||
|
||||
change_column_null :todos, :project_id, true
|
||||
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
|
||||
|
||||
execute "DELETE FROM todos WHERE project_id IS NULL"
|
||||
change_column_null :todos, :project_id, false
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def group_id_exists?
|
||||
column_exists?(:todos, :group_id)
|
||||
end
|
||||
end
|
|
@ -1929,7 +1929,7 @@ ActiveRecord::Schema.define(version: 20180626125654) do
|
|||
|
||||
create_table "todos", force: :cascade do |t|
|
||||
t.integer "user_id", null: false
|
||||
t.integer "project_id", null: false
|
||||
t.integer "project_id"
|
||||
t.integer "target_id"
|
||||
t.string "target_type", null: false
|
||||
t.integer "author_id", null: false
|
||||
|
@ -1939,10 +1939,12 @@ ActiveRecord::Schema.define(version: 20180626125654) do
|
|||
t.datetime "updated_at"
|
||||
t.integer "note_id"
|
||||
t.string "commit_id"
|
||||
t.integer "group_id"
|
||||
end
|
||||
|
||||
add_index "todos", ["author_id"], name: "index_todos_on_author_id", using: :btree
|
||||
add_index "todos", ["commit_id"], name: "index_todos_on_commit_id", using: :btree
|
||||
add_index "todos", ["group_id"], name: "index_todos_on_group_id", using: :btree
|
||||
add_index "todos", ["note_id"], name: "index_todos_on_note_id", using: :btree
|
||||
add_index "todos", ["project_id"], name: "index_todos_on_project_id", using: :btree
|
||||
add_index "todos", ["target_type", "target_id"], name: "index_todos_on_target_type_and_target_id", using: :btree
|
||||
|
@ -2313,6 +2315,7 @@ ActiveRecord::Schema.define(version: 20180626125654) do
|
|||
add_foreign_key "term_agreements", "users", on_delete: :cascade
|
||||
add_foreign_key "timelogs", "issues", name: "fk_timelogs_issues_issue_id", on_delete: :cascade
|
||||
add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade
|
||||
add_foreign_key "todos", "namespaces", column: "group_id", on_delete: :cascade
|
||||
add_foreign_key "todos", "notes", name: "fk_91d1f47b13", on_delete: :cascade
|
||||
add_foreign_key "todos", "projects", name: "fk_45054f9c45", on_delete: :cascade
|
||||
add_foreign_key "todos", "users", column: "author_id", name: "fk_ccf0373936", on_delete: :cascade
|
||||
|
|
|
@ -769,7 +769,8 @@ module API
|
|||
|
||||
class Todo < Grape::Entity
|
||||
expose :id
|
||||
expose :project, using: Entities::BasicProjectDetails
|
||||
expose :project, using: Entities::ProjectIdentity, if: -> (todo, _) { todo.project }
|
||||
expose :group, using: 'API::Entities::NamespaceBasic', if: -> (todo, _) { todo.group }
|
||||
expose :author, using: Entities::UserBasic
|
||||
expose :action_name
|
||||
expose :target_type
|
||||
|
@ -780,12 +781,12 @@ module API
|
|||
|
||||
expose :target_url do |todo, options|
|
||||
target_type = todo.target_type.underscore
|
||||
target_url = "namespace_project_#{target_type}_url"
|
||||
target_url = "#{todo.parent.class.to_s.underscore}_#{target_type}_url"
|
||||
target_anchor = "note_#{todo.note_id}" if todo.note_id?
|
||||
|
||||
Gitlab::Routing
|
||||
.url_helpers
|
||||
.public_send(target_url, todo.project.namespace, todo.project, todo.target, anchor: target_anchor) # rubocop:disable GitlabSecurity/PublicSend
|
||||
.public_send(target_url, todo.parent, todo.target, anchor: target_anchor) # rubocop:disable GitlabSecurity/PublicSend
|
||||
end
|
||||
|
||||
expose :body
|
||||
|
|
|
@ -1,8 +1,9 @@
|
|||
FactoryBot.define do
|
||||
factory :todo do
|
||||
project
|
||||
author { project.creator }
|
||||
user { project.creator }
|
||||
group
|
||||
author { project&.creator || user }
|
||||
user { project&.creator || user }
|
||||
target factory: :issue
|
||||
action { Todo::ASSIGNED }
|
||||
|
||||
|
|
|
@ -5,12 +5,65 @@ describe TodosFinder do
|
|||
let(:user) { create(:user) }
|
||||
let(:group) { create(:group) }
|
||||
let(:project) { create(:project, namespace: group) }
|
||||
let(:issue) { create(:issue, project: project) }
|
||||
let(:merge_request) { create(:merge_request, source_project: project) }
|
||||
let(:finder) { described_class }
|
||||
|
||||
before do
|
||||
group.add_developer(user)
|
||||
end
|
||||
|
||||
describe '#execute' do
|
||||
context 'visibility' do
|
||||
let(:private_group_access) { create(:group, :private) }
|
||||
let(:private_group_hidden) { create(:group, :private) }
|
||||
let(:public_project) { create(:project, :public) }
|
||||
let(:private_project_hidden) { create(:project) }
|
||||
let(:public_group) { create(:group) }
|
||||
|
||||
let!(:todo1) { create(:todo, user: user, project: project, group: nil) }
|
||||
let!(:todo2) { create(:todo, user: user, project: public_project, group: nil) }
|
||||
let!(:todo3) { create(:todo, user: user, project: private_project_hidden, group: nil) }
|
||||
let!(:todo4) { create(:todo, user: user, project: nil, group: group) }
|
||||
let!(:todo5) { create(:todo, user: user, project: nil, group: private_group_access) }
|
||||
let!(:todo6) { create(:todo, user: user, project: nil, group: private_group_hidden) }
|
||||
let!(:todo7) { create(:todo, user: user, project: nil, group: public_group) }
|
||||
|
||||
before do
|
||||
private_group_access.add_developer(user)
|
||||
end
|
||||
|
||||
it 'returns only todos with a target a user has access to' do
|
||||
todos = finder.new(user).execute
|
||||
|
||||
expect(todos).to match_array([todo1, todo2, todo4, todo5, todo7])
|
||||
end
|
||||
end
|
||||
|
||||
context 'filtering' do
|
||||
let!(:todo1) { create(:todo, user: user, project: project, target: issue) }
|
||||
let!(:todo2) { create(:todo, user: user, group: group, target: merge_request) }
|
||||
|
||||
it 'returns correct todos when filtered by a project' do
|
||||
todos = finder.new(user, { project_id: project.id }).execute
|
||||
|
||||
expect(todos).to match_array([todo1])
|
||||
end
|
||||
|
||||
it 'returns correct todos when filtered by a group' do
|
||||
todos = finder.new(user, { group_id: group.id }).execute
|
||||
|
||||
expect(todos).to match_array([todo2])
|
||||
end
|
||||
|
||||
it 'returns correct todos when filtered by a type' do
|
||||
todos = finder.new(user, { type: 'Issue' }).execute
|
||||
|
||||
expect(todos).to match_array([todo1])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#sort' do
|
||||
context 'by date' do
|
||||
let!(:todo1) { create(:todo, user: user, project: project) }
|
||||
|
|
|
@ -7,6 +7,7 @@ describe Todo do
|
|||
it { is_expected.to belong_to(:author).class_name("User") }
|
||||
it { is_expected.to belong_to(:note) }
|
||||
it { is_expected.to belong_to(:project) }
|
||||
it { is_expected.to belong_to(:group) }
|
||||
it { is_expected.to belong_to(:target).touch(true) }
|
||||
it { is_expected.to belong_to(:user) }
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue