Commit 1c146a1c authored by Jarka Kadlecová's avatar Jarka Kadlecová

Support todos for epics

parent 3c69d253
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
class TodosFinder class TodosFinder
prepend FinderWithCrossProjectAccess prepend FinderWithCrossProjectAccess
include FinderMethods include FinderMethods
include Gitlab::Utils::StrongMemoize
requires_cross_project_access unless: -> { project? } requires_cross_project_access unless: -> { project? }
...@@ -34,9 +35,11 @@ class TodosFinder ...@@ -34,9 +35,11 @@ class TodosFinder
items = by_author(items) items = by_author(items)
items = by_state(items) items = by_state(items)
items = by_type(items) items = by_type(items)
items = by_group(items)
# Filtering by project HAS TO be the last because we use # Filtering by project HAS TO be the last because we use
# the project IDs yielded by the todos query thus far # the project IDs yielded by the todos query thus far
items = by_project(items) items = by_project(items)
items = visible_to_user(items)
sort(items) sort(items)
end end
...@@ -82,6 +85,10 @@ class TodosFinder ...@@ -82,6 +85,10 @@ class TodosFinder
params[:project_id].present? params[:project_id].present?
end end
def group?
params[:group_id].present?
end
def project def project
return @project if defined?(@project) return @project if defined?(@project)
...@@ -100,6 +107,12 @@ class TodosFinder ...@@ -100,6 +107,12 @@ class TodosFinder
@project @project
end end
def group
strong_memoize(:group) do
Group.find(params[:group_id])
end
end
def project_ids(items) def project_ids(items)
ids = items.except(:order).select(:project_id) ids = items.except(:order).select(:project_id)
if Gitlab::Database.mysql? if Gitlab::Database.mysql?
...@@ -111,7 +124,7 @@ class TodosFinder ...@@ -111,7 +124,7 @@ class TodosFinder
end end
def type? def type?
type.present? && %w(Issue MergeRequest).include?(type) type.present? && %w(Issue MergeRequest Epic).include?(type)
end end
def type def type
...@@ -148,12 +161,32 @@ class TodosFinder ...@@ -148,12 +161,32 @@ class TodosFinder
def by_project(items) def by_project(items)
if project? if project?
items.where(project: project) items = items.where(project: project)
else end
projects = Project.public_or_visible_to_user(current_user)
items
end
items.joins(:project).merge(projects) def by_group(items)
if group?
items = items.where(group: group)
end 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 end
def by_state(items) def by_state(items)
......
...@@ -43,7 +43,7 @@ module TodosHelper ...@@ -43,7 +43,7 @@ module TodosHelper
project_commit_path(todo.project, project_commit_path(todo.project,
todo.target, anchor: anchor) todo.target, anchor: anchor)
else else
path = [todo.project.namespace.becomes(Namespace), todo.project, todo.target] path = [todo.parent, todo.target]
path.unshift(:pipelines) if todo.build_failed? path.unshift(:pipelines) if todo.build_failed?
......
...@@ -41,6 +41,8 @@ class Group < Namespace ...@@ -41,6 +41,8 @@ class Group < Namespace
has_many :boards has_many :boards
has_many :badges, class_name: 'GroupBadge' has_many :badges, class_name: 'GroupBadge'
has_many :todos
accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :variables, allow_destroy: true
validate :visibility_level_allowed_by_projects validate :visibility_level_allowed_by_projects
...@@ -84,6 +86,13 @@ class Group < Namespace ...@@ -84,6 +86,13 @@ class Group < Namespace
where(id: user.authorized_groups.select(:id).reorder(nil)) where(id: user.authorized_groups.select(:id).reorder(nil))
end 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 def select_for_project_authorization
if current_scope.joins_values.include?(:shared_projects) if current_scope.joins_values.include?(:shared_projects)
joins('INNER JOIN namespaces project_namespace ON project_namespace.id = projects.namespace_id') joins('INNER JOIN namespaces project_namespace ON project_namespace.id = projects.namespace_id')
......
...@@ -22,15 +22,18 @@ class Todo < ActiveRecord::Base ...@@ -22,15 +22,18 @@ class Todo < ActiveRecord::Base
belongs_to :author, class_name: "User" belongs_to :author, class_name: "User"
belongs_to :note belongs_to :note
belongs_to :project belongs_to :project
belongs_to :group
belongs_to :target, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations belongs_to :target, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations
belongs_to :user belongs_to :user
delegate :name, :email, to: :author, prefix: true, allow_nil: true 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 :author, presence: true
validates :target_id, presence: true, unless: :for_commit? validates :target_id, presence: true, unless: :for_commit?
validates :commit_id, presence: true, if: :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 :pending, -> { with_state(:pending) }
scope :done, -> { with_state(:done) } scope :done, -> { with_state(:done) }
...@@ -44,7 +47,7 @@ class Todo < ActiveRecord::Base ...@@ -44,7 +47,7 @@ class Todo < ActiveRecord::Base
state :done state :done
end end
after_save :keep_around_commit after_save :keep_around_commit, if: :commit_id
class << self class << self
# Priority sorting isn't displayed in the dropdown, because we don't show # Priority sorting isn't displayed in the dropdown, because we don't show
...@@ -79,6 +82,10 @@ class Todo < ActiveRecord::Base ...@@ -79,6 +82,10 @@ class Todo < ActiveRecord::Base
end end
end end
def parent
project || group
end
def unmergeable? def unmergeable?
action == UNMERGEABLE action == UNMERGEABLE
end end
......
...@@ -6,6 +6,8 @@ ...@@ -6,6 +6,8 @@
# TodoService.new.new_issue(issue, current_user) # TodoService.new.new_issue(issue, current_user)
# #
class TodoService class TodoService
prepend EE::TodoService
# When create an issue we should: # When create an issue we should:
# #
# * create a todo for assignee if issue is assigned # * create a todo for assignee if issue is assigned
...@@ -273,15 +275,15 @@ class TodoService ...@@ -273,15 +275,15 @@ class TodoService
end end
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 # Create Todos for directly addressed users
directly_addressed_users = filter_directly_addressed_users(project, note || target, author, skip_users) directly_addressed_users = filter_directly_addressed_users(parent, note || target, author, skip_users)
attributes = attributes_for_todo(project, target, author, Todo::DIRECTLY_ADDRESSED, note) attributes = attributes_for_todo(parent, target, author, Todo::DIRECTLY_ADDRESSED, note)
create_todos(directly_addressed_users, attributes) create_todos(directly_addressed_users, attributes)
# Create Todos for mentioned users # Create Todos for mentioned users
mentioned_users = filter_mentioned_users(project, note || target, author, skip_users) mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users)
attributes = attributes_for_todo(project, target, author, Todo::MENTIONED, note) attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note)
create_todos(mentioned_users, attributes) create_todos(mentioned_users, attributes)
end end
...@@ -317,36 +319,37 @@ class TodoService ...@@ -317,36 +319,37 @@ class TodoService
def attributes_for_todo(project, target, author, action, note = nil) def attributes_for_todo(project, target, author, action, note = nil)
attributes_for_target(target).merge!( 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, author_id: author.id,
action: action, action: action,
note: note note: note
) )
end end
def filter_todo_users(users, project, target) def filter_todo_users(users, parent, target)
reject_users_without_access(users, project, target).uniq reject_users_without_access(users, parent, target).uniq
end 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 mentioned_users = target.mentioned_users(author) - skip_users
filter_todo_users(mentioned_users, project, target) filter_todo_users(mentioned_users, parent, target)
end 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 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 end
def reject_users_without_access(users, project, target) def reject_users_without_access(users, parent, target)
if target.is_a?(Note) && (target.for_issue? || target.for_merge_request?) if target.is_a?(Note) && (target.for_issue? || target.for_merge_request? || target.for_epic?)
target = target.noteable target = target.noteable
end end
if target.is_a?(Issuable) if target.is_a?(Issuable)
select_users(users, :"read_#{target.to_ability_name}", target) select_users(users, :"read_#{target.to_ability_name}", target)
else else
select_users(users, :read_project, project) select_users(users, :read_project, parent)
end end
end end
......
...@@ -107,6 +107,8 @@ constraints(::Constraints::GroupUrlConstrainer.new) do ...@@ -107,6 +107,8 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
end end
end end
resources :todos, only: [:create]
# On CE only index and show are needed # On CE only index and show are needed
resources :boards, only: [:index, :show, :create, :update, :destroy] resources :boards, only: [:index, :show, :create, :update, :destroy]
......
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
...@@ -2502,7 +2502,7 @@ ActiveRecord::Schema.define(version: 20180626125654) do ...@@ -2502,7 +2502,7 @@ ActiveRecord::Schema.define(version: 20180626125654) do
create_table "todos", force: :cascade do |t| create_table "todos", force: :cascade do |t|
t.integer "user_id", null: false t.integer "user_id", null: false
t.integer "project_id", null: false t.integer "project_id"
t.integer "target_id" t.integer "target_id"
t.string "target_type", null: false t.string "target_type", null: false
t.integer "author_id", null: false t.integer "author_id", null: false
...@@ -2512,10 +2512,12 @@ ActiveRecord::Schema.define(version: 20180626125654) do ...@@ -2512,10 +2512,12 @@ ActiveRecord::Schema.define(version: 20180626125654) do
t.datetime "updated_at" t.datetime "updated_at"
t.integer "note_id" t.integer "note_id"
t.string "commit_id" t.string "commit_id"
t.integer "group_id"
end end
add_index "todos", ["author_id"], name: "index_todos_on_author_id", using: :btree 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", ["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", ["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", ["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 add_index "todos", ["target_type", "target_id"], name: "index_todos_on_target_type_and_target_id", using: :btree
...@@ -2973,6 +2975,7 @@ ActiveRecord::Schema.define(version: 20180626125654) do ...@@ -2973,6 +2975,7 @@ ActiveRecord::Schema.define(version: 20180626125654) do
add_foreign_key "term_agreements", "users", on_delete: :cascade 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", "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 "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", "notes", name: "fk_91d1f47b13", on_delete: :cascade
add_foreign_key "todos", "projects", name: "fk_45054f9c45", 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 add_foreign_key "todos", "users", column: "author_id", name: "fk_ccf0373936", on_delete: :cascade
......
class Groups::TodosController < Groups::ApplicationController
include Gitlab::Utils::StrongMemoize
before_action :authenticate_user!, only: [:create]
def create
todo = TodoService.new.mark_todo(epic, current_user)
render json: {
count: TodosFinder.new(current_user, state: :pending).execute.count,
delete_path: dashboard_todo_path(todo)
}
end
private
def epic
strong_memoize(:epic) do
case params[:issuable_type]
when "epic"
@group.epics.find_by(id: params[:issuable_id])
end
end
end
end
...@@ -34,11 +34,6 @@ module EE ...@@ -34,11 +34,6 @@ module EE
!for_epic? && super !for_epic? && super
end end
override :can_create_todo?
def can_create_todo?
!for_epic? && super
end
override :etag_key override :etag_key
def etag_key def etag_key
if for_epic? if for_epic?
......
module EE
module TodoService
def new_epic(epic, current_user)
create_mention_todos(nil, epic, current_user)
end
def update_epic(epic, current_user, skip_users = [])
create_mention_todos(nil, epic, current_user, nil, skip_users)
end
end
end
...@@ -2,6 +2,12 @@ module Epics ...@@ -2,6 +2,12 @@ module Epics
class UpdateService < Epics::BaseService class UpdateService < Epics::BaseService
def execute(epic) def execute(epic)
update(epic) update(epic)
# TODO: old_mentioned_users
# TODO: move to handle_changes method
todo_service.update_epic(epic, current_user, [])
epic
end end
end end
end end
module EE
module API
module Todos
extend ActiveSupport::Concern
prepended do
helpers do
def epic
@epic ||= user_group.epics.find_by(iid: params[:epic_iid])
end
def authorize_can_read!
authorize!(:read_epic, epic)
end
end
resource :groups, requirements: ::API::API::PROJECT_ENDPOINT_REQUIREMENTS do
desc 'Create a todo on an epic' do
success ::API::Entities::Todo
end
params do
requires :epic_iid, type: Integer, desc: 'The IID of an epic'
end
post ":id/epics/:epic_iid/todo" do
authorize_can_read!
todo = ::TodoService.new.mark_todo(epic, current_user).first
if todo
present todo, with: ::API::Entities::Todo, current_user: current_user
else
not_modified!
end
end
end
end
end
end
end
require('spec_helper')
describe Groups::TodosController do
let(:user) { create(:user) }
let(:group) { create(:group) }
let(:epic) { create(:epic, group: group) }
describe 'POST create' do
subject { post :create, group_id: group, issuable_id: epic.id, issuable_type: 'epic', format: :json }
context 'when authorized' do
before do
sign_in(user)
end
it 'creates todo for epic' do
expect { subject }.to change { user.todos.count }.by(1)
expect(response).to have_gitlab_http_status(200)
end
it 'returns todo path and pending count' do
subject
expect(json_response['count']).to eq 1
expect(json_response['delete_path']).to match(%r{/dashboard/todos/\d{1}})
end
end
context 'when not authorized for project' do
it 'does not create todo for epic that user has no access to' do
group.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
sign_in(user)
expect { subject }.not_to change { user.todos.count }
expect(response).to have_gitlab_http_status(404)
end
it 'does not create todo for epic when user not logged in' do
expect { subject }.not_to change { user.todos.count }
expect(response).to have_gitlab_http_status(401)
end
end
end
end
require 'spec_helper'
describe API::Todos do
let(:group) { create(:group) }
let(:user) { create(:user) }
let(:epic) { create(:epic, group: group) }
subject { post api("/groups/#{group.id}/epics/#{epic.iid}/todo", user) }
describe 'POST :id/epics/:epic_iid/todo' do
context 'when epics feature is disabled' do
it 'returns 403 forbidden error' do
subject
expect(response).to have_gitlab_http_status(403)
end
end
context 'when epics feature is enabled' do
before do
stub_licensed_features(epics: true)
end
it 'creates a todo on an epic' do
expect { subject }.to change { Todo.count }.by(1)
expect(response.status).to eq(201)
expect(json_response['project']).to be_nil
expect(json_response['group']).to be_a(Hash)
expect(json_response['author']).to be_a(Hash)
expect(json_response['target_type']).to eq('Epic')
expect(json_response['target']).to be_a(Hash)
expect(json_response['target_url']).to be_present
expect(json_response['body']).to be_present
expect(json_response['state']).to eq('pending')
expect(json_response['action_name']).to eq('marked')
expect(json_response['created_at']).to be_present
end
it 'returns 304 there already exist a todo on that epic' do
create(:todo, project: nil, group: group, user: user, target: epic)
subject
expect(response.status).to eq(304)
end
it 'returns 404 if the epic is not found' do
post api("/groups/#{group.id}/epics/9999/todo", user)
expect(response.status).to eq(403)
end
it 'returns an error if the epic is not accessible' do
group.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
subject
expect(response).to have_gitlab_http_status(404)
end
end
end
end
require 'spec_helper'
describe TodoService do
describe 'Epics' do
let(:author) { create(:user, username: 'author') }
let(:non_member) { create(:user, username: 'non_member') }
let(:member) { create(:user, username: 'member') }
let(:guest) { create(:user, username: 'guest') }
let(:admin) { create(:admin, username: 'administrator') }
let(:john_doe) { create(:user, username: 'john_doe') }
let(:skipped) { create(:user, username: 'skipped') }
let(:skip_users) { [skipped] }
let(:users) { [author, non_member, member, guest, admin, john_doe, skipped] }
let(:mentions) { users.map(&:to_reference).join(' ') }
let(:combined_mentions) { member.to_reference + ", what do you think? cc: " + [guest, admin, skipped].map(&:to_reference).join(' ') }
let(:description_mentions) { "- [ ] Task 1\n- [ ] Task 2 FYI: #{mentions}" }
let(:description_directly_addressed) { "#{mentions}\n- [ ] Task 1\n- [ ] Task 2" }
let(:group) { create(:group) }
let(:epic) { create(:epic, group: group, author: author, description: description_mentions) }
let(:service) { described_class.new }
let(:todos_for) { [] }
let(:todos_not_for) { [] }
let(:target) { epic }
before do
stub_licensed_features(epics: true)
group.add_guest(guest)
group.add_developer(author)
group.add_developer(member)
end
shared_examples_for 'todos creation' do
it 'creates todos for users mentioned' do
if todos_for.count > 0
params = todo_params
.merge(user: todos_for)
.reverse_merge(target: target, project: nil, group: group, author: author, state: :pending)
expect { execute }
.to change { Todo.where(params).count }.from(0).to(todos_for.count)
end
end
it 'does not create todos for users not mentioned or without permissions' do
execute
params = todo_params
.reverse_merge(target: target, project: nil, group: group, author: author, state: :pending)
todos_not_for.each_with_index do |user, index|
expect(Todo.where(params.merge(user: user)).count)
.to eq(0), "expected not to create a todo for user '#{user.username}''"
end
end
end
context 'Epics' do
describe '#new_epic' do
let(:execute) { service.new_epic(epic, author) }
context 'when an epic belongs to a public group' do
context 'for mentioned users' do
let(:todo_params) { { action: Todo::MENTIONED } }
let(:todos_for) { users }
include_examples 'todos creation'
end
context 'for directly addressed users' do
before do
epic.update(description: description_directly_addressed)
end
let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } }
let(:todos_for) { users }
include_examples 'todos creation'
end
context 'combined' do
before do
epic.update(description: combined_mentions)
end
context 'mentioned users' do
let(:todo_params) { { action: Todo::MENTIONED } }
let(:todos_for) { [guest, admin, skipped] }
include_examples 'todos creation'
end
context 'directly addressed users' do
let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } }
let(:todos_for) { [member] }
include_examples 'todos creation'
end
end
end
context 'when an epic belongs to a private group' do
before do
group.update(visibility: Gitlab::VisibilityLevel::PRIVATE)
end
context 'for mentioned users' do
let(:todo_params) { { action: Todo::MENTIONED } }
let(:todos_for) { [member, author, guest, admin] }
let(:todos_not_for) { [non_member, john_doe, skipped] }
include_examples 'todos creation'
end
context 'for directly addressed users' do
before do
epic.update!(description: description_directly_addressed)
end
let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } }
let(:todos_for) { [member, author, guest, admin] }
let(:todos_not_for) { [non_member, john_doe, skipped] }
include_examples 'todos creation'
end
end
context 'creates todos for group members when a group is mentioned' do
before do
epic.update(description: group.to_reference)
end
let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } }
let(:todos_for) { [member, guest, author] }
let(:todos_not_for) { [non_member, admin, john_doe] }
include_examples 'todos creation'
end
end
describe '#update_epic' do
let(:execute) { service.update_epic(epic, author, skip_users) }
context 'for mentioned users' do
let(:todo_params) { { action: Todo::MENTIONED } }
let(:todos_for) { [author, non_member, member, guest, admin, john_doe] }
let(:todos_not_for) { [skipped] }
include_examples 'todos creation'
end
context 'for directly addressed users' do
before do
epic.update(description: description_directly_addressed)
end
let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } }
let(:todos_for) { [author, non_member, member, guest, admin, john_doe] }
let(:todos_not_for) { [skipped] }
include_examples 'todos creation'
end
end
describe '#new_note' do
let!(:first_todo) do
create(:todo, :assigned,
user: john_doe, project: nil, group: group, target: epic, author: author)
end
let!(:second_todo) do
create(:todo, :assigned,
user: john_doe, project: nil, group: group, target: epic, author: author)
end
let(:note) { create(:note, noteable: epic, project: nil, author: john_doe, note: mentions) }
context 'when a note is created for an epic' do
it 'marks pending epic todos for the note author as done' do
service.new_note(note, john_doe)
expect(first_todo.reload).to be_done
expect(second_todo.reload).to be_done
end
it 'does not marka pending epic todos for the note author as done for system notes' do
system_note = create(:system_note, noteable: epic)
service.new_note(system_note, john_doe)
expect(first_todo.reload).to be_pending
expect(second_todo.reload).to be_pending
end
end
context 'mentions' do
let(:execute) { service.new_note(note, author) }
context 'for mentioned users' do
before do
note.update(note: description_mentions)
end
let(:todo_params) { { action: Todo::MENTIONED } }
let(:todos_for) { [author, non_member, member, guest, admin, skipped] }
let(:todos_not_for) { [john_doe] }
include_examples 'todos creation'
end
context 'for directly addressed users' do
before do
note.update(note: description_directly_addressed)
end
let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } }
let(:todos_for) { [author, non_member, member, guest, admin, skipped] }
let(:todos_not_for) { [john_doe] }
include_examples 'todos creation'
end
context 'combined' do
before do
note.update(note: combined_mentions)
end
context 'mentioned users' do
let(:todo_params) { { action: Todo::MENTIONED } }
let(:todos_for) { [guest, admin, skipped] }
include_examples 'todos creation'
end
context 'directly addressed users' do
let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } }
let(:todos_for) { [member] }
include_examples 'todos creation'
end
end
end
end
end
end
end
...@@ -797,23 +797,30 @@ module API ...@@ -797,23 +797,30 @@ module API
class Todo < Grape::Entity class Todo < Grape::Entity
expose :id 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 :author, using: Entities::UserBasic
expose :action_name expose :action_name
expose :target_type expose :target_type
expose :target do |todo, options| expose :target do |todo, options|
Entities.const_get(todo.target_type).represent(todo.target, options) begin
klass = "EE::API::Entities::#{todo.target_type}".constantize
rescue
klass = "API::Entities::#{todo.target_type}".constantize
end
klass.represent(todo.target, options)
end end
expose :target_url do |todo, options| expose :target_url do |todo, options|
target_type = todo.target_type.underscore 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? target_anchor = "note_#{todo.note_id}" if todo.note_id?
Gitlab::Routing Gitlab::Routing
.url_helpers .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 end
expose :body expose :body
......
module API module API
class Todos < Grape::API class Todos < Grape::API
include PaginationParams include PaginationParams
prepend EE::API::Todos
before { authenticate! } before { authenticate! }
......
FactoryBot.define do FactoryBot.define do
factory :todo do factory :todo do
project project
author { project.creator } group
user { project.creator } author { project&.creator || user }
user { project&.creator || user }
target factory: :issue target factory: :issue
action { Todo::ASSIGNED } action { Todo::ASSIGNED }
......
...@@ -5,12 +5,65 @@ describe TodosFinder do ...@@ -5,12 +5,65 @@ describe TodosFinder do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:group) { create(:group) } let(:group) { create(:group) }
let(:project) { create(:project, namespace: 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 } let(:finder) { described_class }
before do before do
group.add_developer(user) group.add_developer(user)
end 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 describe '#sort' do
context 'by date' do context 'by date' do
let!(:todo1) { create(:todo, user: user, project: project) } let!(:todo1) { create(:todo, user: user, project: project) }
......
...@@ -7,6 +7,7 @@ describe Todo do ...@@ -7,6 +7,7 @@ describe Todo do
it { is_expected.to belong_to(:author).class_name("User") } it { is_expected.to belong_to(:author).class_name("User") }
it { is_expected.to belong_to(:note) } it { is_expected.to belong_to(:note) }
it { is_expected.to belong_to(:project) } 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(:target).touch(true) }
it { is_expected.to belong_to(:user) } it { is_expected.to belong_to(:user) }
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment