Commit 4d9a3f42 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'ee-5481-epic-todos' into 'master'

Port of Todos for epics

See merge request gitlab-org/gitlab-ce!19908
parents ecf9c145 0d11c4a8
...@@ -39,6 +39,7 @@ export default class Todos { ...@@ -39,6 +39,7 @@ export default class Todos {
} }
initFilters() { initFilters() {
this.initFilterDropdown($('.js-group-search'), 'group_id', ['text']);
this.initFilterDropdown($('.js-project-search'), 'project_id', ['text']); this.initFilterDropdown($('.js-project-search'), 'project_id', ['text']);
this.initFilterDropdown($('.js-type-search'), 'type'); this.initFilterDropdown($('.js-type-search'), 'type');
this.initFilterDropdown($('.js-action-search'), 'action_id'); this.initFilterDropdown($('.js-action-search'), 'action_id');
...@@ -53,7 +54,16 @@ export default class Todos { ...@@ -53,7 +54,16 @@ export default class Todos {
filterable: searchFields ? true : false, filterable: searchFields ? true : false,
search: { fields: searchFields }, search: { fields: searchFields },
data: $dropdown.data('data'), data: $dropdown.data('data'),
clicked: () => $dropdown.closest('form.filter-form').submit(), clicked: () => {
const $formEl = $dropdown.closest('form.filter-form');
const mutexDropdowns = {
group_id: 'project_id',
project_id: 'group_id',
};
$formEl.find(`input[name="${mutexDropdowns[fieldName]}"]`).remove();
$formEl.submit();
},
}); });
} }
......
<script>
import { __ } from '~/locale';
import tooltip from '~/vue_shared/directives/tooltip';
import Icon from '~/vue_shared/components/icon.vue';
import LoadingIcon from '~/vue_shared/components/loading_icon.vue';
const MARK_TEXT = __('Mark todo as done');
const TODO_TEXT = __('Add todo');
export default {
directives: {
tooltip,
},
components: {
Icon,
LoadingIcon,
},
props: {
issuableId: {
type: Number,
required: true,
},
issuableType: {
type: String,
required: true,
},
isTodo: {
type: Boolean,
required: false,
default: true,
},
isActionActive: {
type: Boolean,
required: false,
default: false,
},
collapsed: {
type: Boolean,
required: false,
default: false,
},
},
computed: {
buttonClasses() {
return this.collapsed ?
'btn-blank btn-todo sidebar-collapsed-icon dont-change-state' :
'btn btn-default btn-todo issuable-header-btn float-right';
},
buttonLabel() {
return this.isTodo ? MARK_TEXT : TODO_TEXT;
},
collapsedButtonIconClasses() {
return this.isTodo ? 'todo-undone' : '';
},
collapsedButtonIcon() {
return this.isTodo ? 'todo-done' : 'todo-add';
},
},
methods: {
handleButtonClick() {
this.$emit('toggleTodo');
},
},
};
</script>
<template>
<button
v-tooltip
:class="buttonClasses"
:title="buttonLabel"
:aria-label="buttonLabel"
:data-issuable-id="issuableId"
:data-issuable-type="issuableType"
type="button"
data-container="body"
data-placement="left"
data-boundary="viewport"
@click="handleButtonClick"
>
<icon
v-show="collapsed"
:css-classes="collapsedButtonIconClasses"
:name="collapsedButtonIcon"
/>
<span
v-show="!collapsed"
class="issuable-todo-inner"
>
{{ buttonLabel }}
</span>
<loading-icon
v-show="isActionActive"
:inline="true"
/>
</button>
</template>
...@@ -12,6 +12,11 @@ export default { ...@@ -12,6 +12,11 @@ export default {
type: Boolean, type: Boolean,
required: true, required: true,
}, },
cssClasses: {
type: String,
required: false,
default: '',
},
}, },
computed: { computed: {
tooltipLabel() { tooltipLabel() {
...@@ -30,10 +35,12 @@ export default { ...@@ -30,10 +35,12 @@ export default {
<button <button
v-tooltip v-tooltip
:title="tooltipLabel" :title="tooltipLabel"
:class="cssClasses"
type="button" type="button"
class="btn btn-blank gutter-toggle btn-sidebar-action" class="btn btn-blank gutter-toggle btn-sidebar-action"
data-container="body" data-container="body"
data-placement="left" data-placement="left"
data-boundary="viewport"
@click="toggle" @click="toggle"
> >
<i <i
......
...@@ -449,6 +449,7 @@ ...@@ -449,6 +449,7 @@
.todo-undone { .todo-undone {
color: $gl-link-color; color: $gl-link-color;
fill: $gl-link-color;
} }
.author { .author {
......
...@@ -174,6 +174,18 @@ ...@@ -174,6 +174,18 @@
} }
} }
@include media-breakpoint-down(lg) {
.todos-filters {
.filter-categories {
width: 75%;
.filter-item {
margin-bottom: 10px;
}
}
}
}
@include media-breakpoint-down(xs) { @include media-breakpoint-down(xs) {
.todo { .todo {
.avatar { .avatar {
...@@ -199,6 +211,10 @@ ...@@ -199,6 +211,10 @@
} }
.todos-filters { .todos-filters {
.filter-categories {
width: auto;
}
.dropdown-menu-toggle { .dropdown-menu-toggle {
width: 100%; width: 100%;
} }
......
module TodosActions
extend ActiveSupport::Concern
def create
todo = TodoService.new.mark_todo(issuable, current_user)
render json: {
count: TodosFinder.new(current_user, state: :pending).execute.count,
delete_path: dashboard_todo_path(todo)
}
end
end
...@@ -70,7 +70,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController ...@@ -70,7 +70,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
end end
def todo_params def todo_params
params.permit(:action_id, :author_id, :project_id, :type, :sort, :state) params.permit(:action_id, :author_id, :project_id, :type, :sort, :state, :group_id)
end end
def redirect_out_of_range(todos) def redirect_out_of_range(todos)
......
class Projects::TodosController < Projects::ApplicationController class Projects::TodosController < Projects::ApplicationController
before_action :authenticate_user!, only: [:create] include Gitlab::Utils::StrongMemoize
include TodosActions
def create
todo = TodoService.new.mark_todo(issuable, current_user)
render json: { before_action :authenticate_user!, only: [:create]
count: TodosFinder.new(current_user, state: :pending).execute.count,
delete_path: dashboard_todo_path(todo)
}
end
private private
def issuable def issuable
@issuable ||= begin strong_memoize(:issuable) do
case params[:issuable_type] case params[:issuable_type]
when "issue" when "issue"
IssuesFinder.new(current_user, project_id: @project.id).find(params[:issuable_id]) IssuesFinder.new(current_user, project_id: @project.id).find(params[:issuable_id])
......
...@@ -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,18 +107,14 @@ class TodosFinder ...@@ -100,18 +107,14 @@ class TodosFinder
@project @project
end end
def project_ids(items) def group
ids = items.except(:order).select(:project_id) strong_memoize(:group) do
if Gitlab::Database.mysql? Group.find(params[:group_id])
# To make UPDATE work on MySQL, wrap it in a SELECT with an alias
ids = Todo.except(:order).select('*').from("(#{ids.to_sql}) AS t")
end end
ids
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 +151,37 @@ class TodosFinder ...@@ -148,12 +151,37 @@ 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
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)
)
end
items.joins(:project).merge(projects) items
end 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.select(:id),
groups.select(:id)
)
end end
def by_state(items) def by_state(items)
......
...@@ -131,6 +131,19 @@ module IssuablesHelper ...@@ -131,6 +131,19 @@ module IssuablesHelper
end end
end end
def group_dropdown_label(group_id, default_label)
return default_label if group_id.nil?
return "Any group" if group_id == "0"
group = ::Group.find_by(id: group_id)
if group
group.full_name
else
default_label
end
end
def milestone_dropdown_label(milestone_title, default_label = "Milestone") def milestone_dropdown_label(milestone_title, default_label = "Milestone")
title = title =
case milestone_title case milestone_title
......
...@@ -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?
...@@ -167,4 +167,12 @@ module TodosHelper ...@@ -167,4 +167,12 @@ module TodosHelper
def show_todo_state?(todo) def show_todo_state?(todo)
(todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && %w(closed merged).include?(todo.target.state) (todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && %w(closed merged).include?(todo.target.state)
end end
def todo_group_options
groups = current_user.authorized_groups.map do |group|
{ id: group.id, text: group.full_name }
end
groups.unshift({ id: '', text: 'Any Group' }).to_json
end
end end
...@@ -243,6 +243,12 @@ module Issuable ...@@ -243,6 +243,12 @@ module Issuable
opened? opened?
end end
def overdue?
return false unless respond_to?(:due_date)
due_date.try(:past?) || false
end
def user_notes_count def user_notes_count
if notes.loaded? if notes.loaded?
# Use the in-memory association to select and count to avoid hitting the db # Use the in-memory association to select and count to avoid hitting the db
......
...@@ -39,6 +39,8 @@ class Group < Namespace ...@@ -39,6 +39,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
...@@ -82,6 +84,12 @@ class Group < Namespace ...@@ -82,6 +84,12 @@ 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')
......
...@@ -275,10 +275,6 @@ class Issue < ActiveRecord::Base ...@@ -275,10 +275,6 @@ class Issue < ActiveRecord::Base
user ? readable_by?(user) : publicly_visible? user ? readable_by?(user) : publicly_visible?
end end
def overdue?
due_date.try(:past?) || false
end
def check_for_spam? def check_for_spam?
project.public? && (title_changed? || description_changed?) project.public? && (title_changed? || description_changed?)
end end
......
...@@ -229,6 +229,10 @@ class Note < ActiveRecord::Base ...@@ -229,6 +229,10 @@ class Note < ActiveRecord::Base
!for_personal_snippet? !for_personal_snippet?
end end
def for_issuable?
for_issue? || for_merge_request?
end
def skip_project_check? def skip_project_check?
!for_project_noteable? !for_project_noteable?
end end
......
...@@ -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_id
validates :group, presence: true, unless: :project_id
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
end
def unmergeable? def unmergeable?
action == UNMERGEABLE action == UNMERGEABLE
end end
......
...@@ -260,15 +260,15 @@ class TodoService ...@@ -260,15 +260,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
...@@ -299,36 +299,36 @@ class TodoService ...@@ -299,36 +299,36 @@ 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,
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_issuable?
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
......
...@@ -30,7 +30,13 @@ ...@@ -30,7 +30,13 @@
.todos-filters .todos-filters
.row-content-block.second-block .row-content-block.second-block
= form_tag todos_filter_path(without: [:project_id, :author_id, :type, :action_id]), method: :get, class: 'filter-form' do = form_tag todos_filter_path(without: [:project_id, :author_id, :type, :action_id]), method: :get, class: 'filter-form d-sm-flex' do
.filter-categories.flex-fill
.filter-item.inline
- if params[:group_id].present?
= hidden_field_tag(:group_id, params[:group_id])
= dropdown_tag(group_dropdown_label(params[:group_id], 'Group'), options: { toggle_class: 'js-group-search js-filter-submit', title: 'Filter by group', filter: true, filterInput: 'input#group-search', dropdown_class: 'dropdown-menu-selectable dropdown-menu-group js-filter-submit',
placeholder: 'Search groups', data: { data: todo_group_options, default_label: 'Group', display: 'static' } })
.filter-item.inline .filter-item.inline
- if params[:project_id].present? - if params[:project_id].present?
= hidden_field_tag(:project_id, params[:project_id]) = hidden_field_tag(:project_id, params[:project_id])
......
class AddGroupToTodos < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column :todos, :group_id, :integer
add_concurrent_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
...@@ -1930,7 +1930,7 @@ ActiveRecord::Schema.define(version: 20180629191052) do ...@@ -1930,7 +1930,7 @@ ActiveRecord::Schema.define(version: 20180629191052) 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
...@@ -1940,10 +1940,12 @@ ActiveRecord::Schema.define(version: 20180629191052) do ...@@ -1940,10 +1940,12 @@ ActiveRecord::Schema.define(version: 20180629191052) 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
...@@ -2315,6 +2317,7 @@ ActiveRecord::Schema.define(version: 20180629191052) do ...@@ -2315,6 +2317,7 @@ ActiveRecord::Schema.define(version: 20180629191052) 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
......
...@@ -18,6 +18,7 @@ Parameters: ...@@ -18,6 +18,7 @@ Parameters:
| `action` | string | no | The action to be filtered. Can be `assigned`, `mentioned`, `build_failed`, `marked`, `approval_required`, `unmergeable` or `directly_addressed`. | | `action` | string | no | The action to be filtered. Can be `assigned`, `mentioned`, `build_failed`, `marked`, `approval_required`, `unmergeable` or `directly_addressed`. |
| `author_id` | integer | no | The ID of an author | | `author_id` | integer | no | The ID of an author |
| `project_id` | integer | no | The ID of a project | | `project_id` | integer | no | The ID of a project |
| `group_id` | integer | no | The ID of a group |
| `state` | string | no | The state of the todo. Can be either `pending` or `done` | | `state` | string | no | The state of the todo. Can be either `pending` or `done` |
| `type` | string | no | The type of a todo. Can be either `Issue` or `MergeRequest` | | `type` | string | no | The type of a todo. Can be either `Issue` or `MergeRequest` |
......
...@@ -109,6 +109,7 @@ There are four kinds of filters you can use on your Todos dashboard. ...@@ -109,6 +109,7 @@ There are four kinds of filters you can use on your Todos dashboard.
| Filter | Description | | Filter | Description |
| ------- | ----------- | | ------- | ----------- |
| Project | Filter by project | | Project | Filter by project |
| Group | Filter by group |
| Author | Filter by the author that triggered the Todo | | Author | Filter by the author that triggered the Todo |
| Type | Filter by issue or merge request | | Type | Filter by issue or merge request |
| Action | Filter by the action that triggered the Todo | | Action | Filter by the action that triggered the Todo |
......
...@@ -769,28 +769,33 @@ module API ...@@ -769,28 +769,33 @@ 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_id }
expose :group, using: 'API::Entities::NamespaceBasic', if: -> (todo, _) { todo.group_id }
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) todo_target_class(todo.target_type).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
expose :state expose :state
expose :created_at expose :created_at
def todo_target_class(target_type)
::API::Entities.const_get(target_type)
end
end end
class NamespaceBasic < Grape::Entity class NamespaceBasic < Grape::Entity
......
...@@ -5,140 +5,53 @@ describe Projects::TodosController do ...@@ -5,140 +5,53 @@ describe Projects::TodosController do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request, source_project: project) }
let(:parent) { project }
context 'Issues' do shared_examples 'project todos actions' do
describe 'POST create' do it_behaves_like 'todos actions'
def go
post :create,
namespace_id: project.namespace,
project_id: project,
issuable_id: issue.id,
issuable_type: 'issue',
format: 'html'
end
context 'when authorized' do context 'when not authorized for resource' do
before do
sign_in(user)
project.add_developer(user)
end
it 'creates todo for issue' do
expect do
go
end.to change { user.todos.count }.by(1)
expect(response).to have_gitlab_http_status(200)
end
it 'returns todo path and pending count' do
go
expect(response).to have_gitlab_http_status(200)
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 issue that user has no access to' do
sign_in(user)
expect do
go
end.to change { user.todos.count }.by(0)
expect(response).to have_gitlab_http_status(404)
end
it 'does not create todo for issue when user not logged in' do
expect do
go
end.to change { user.todos.count }.by(0)
expect(response).to have_gitlab_http_status(302)
end
end
context 'when not authorized for issue' do
before do before do
project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
sign_in(user) sign_in(user)
end end
it "doesn't create todo" do it "doesn't create todo" do
expect { go }.not_to change { user.todos.count } expect { post_create }.not_to change { user.todos.count }
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
end end
end end
end
context 'Merge Requests' do context 'Issues' do
describe 'POST create' do describe 'POST create' do
def go def post_create
post :create, post :create,
namespace_id: project.namespace, namespace_id: project.namespace,
project_id: project, project_id: project,
issuable_id: merge_request.id, issuable_id: issue.id,
issuable_type: 'merge_request', issuable_type: 'issue',
format: 'html' format: 'html'
end end
context 'when authorized' do it_behaves_like 'project todos actions'
before do
sign_in(user)
project.add_developer(user)
end
it 'creates todo for merge request' do
expect do
go
end.to change { user.todos.count }.by(1)
expect(response).to have_gitlab_http_status(200)
end
it 'returns todo path and pending count' do
go
expect(response).to have_gitlab_http_status(200)
expect(json_response['count']).to eq 1
expect(json_response['delete_path']).to match(%r{/dashboard/todos/\d{1}})
end end
end end
context 'when not authorized for project' do context 'Merge Requests' do
it 'does not create todo for merge request user has no access to' do describe 'POST create' do
sign_in(user) def post_create
expect do post :create,
go namespace_id: project.namespace,
end.to change { user.todos.count }.by(0) project_id: project,
issuable_id: merge_request.id,
expect(response).to have_gitlab_http_status(404) issuable_type: 'merge_request',
end format: 'html'
it 'does not create todo for merge request user has no access to' do
expect do
go
end.to change { user.todos.count }.by(0)
expect(response).to have_gitlab_http_status(302)
end
end
context 'when not authorized for merge_request' do
before do
project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
sign_in(user)
end end
it "doesn't create todo" do it_behaves_like 'project todos actions'
expect { go }.not_to change { user.todos.count }
expect(response).to have_gitlab_http_status(404)
end
end
end end
end end
end end
FactoryBot.define do FactoryBot.define do
factory :todo do factory :todo do
project project
author { project.creator } author { project&.creator || user }
user { project.creator } user { project&.creator || user }
target factory: :issue target factory: :issue
action { Todo::ASSIGNED } action { Todo::ASSIGNED }
......
...@@ -5,12 +5,76 @@ describe TodosFinder do ...@@ -5,12 +5,76 @@ 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([todo1, 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
context 'with subgroups', :nested_groups do
let(:subgroup) { create(:group, parent: group) }
let!(:todo3) { create(:todo, user: user, group: subgroup, target: issue) }
it 'returns todos from subgroups when filtered by a group' do
todos = finder.new(user, { group_id: group.id }).execute
expect(todos).to match_array([todo1, todo2, todo3])
end
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) }
......
...@@ -21,6 +21,27 @@ describe IssuablesHelper do ...@@ -21,6 +21,27 @@ describe IssuablesHelper do
end end
end end
describe '#group_dropdown_label' do
let(:group) { create(:group) }
let(:default) { 'default label' }
it 'returns default group label when group_id is nil' do
expect(group_dropdown_label(nil, default)).to eq('default label')
end
it 'returns "any group" when group_id is 0' do
expect(group_dropdown_label('0', default)).to eq('Any group')
end
it 'returns group full path when a group was found for the provided id' do
expect(group_dropdown_label(group.id, default)).to eq(group.full_name)
end
it 'returns default label when a group was not found for the provided id' do
expect(group_dropdown_label(9999, default)).to eq('default label')
end
end
describe '#issuable_labels_tooltip' do describe '#issuable_labels_tooltip' do
it 'returns label text with no labels' do it 'returns label text with no labels' do
expect(issuable_labels_tooltip([])).to eq("Labels") expect(issuable_labels_tooltip([])).to eq("Labels")
......
import Vue from 'vue';
import SidebarTodos from '~/sidebar/components/todo_toggle/todo.vue';
import mountComponent from 'spec/helpers/vue_mount_component_helper';
const createComponent = ({
issuableId = 1,
issuableType = 'epic',
isTodo,
isActionActive,
collapsed,
}) => {
const Component = Vue.extend(SidebarTodos);
return mountComponent(Component, {
issuableId,
issuableType,
isTodo,
isActionActive,
collapsed,
});
};
describe('SidebarTodo', () => {
let vm;
beforeEach(() => {
vm = createComponent({});
});
afterEach(() => {
vm.$destroy();
});
describe('computed', () => {
describe('buttonClasses', () => {
it('returns todo button classes for when `collapsed` prop is `false`', () => {
expect(vm.buttonClasses).toBe('btn btn-default btn-todo issuable-header-btn float-right');
});
it('returns todo button classes for when `collapsed` prop is `true`', done => {
vm.collapsed = true;
Vue.nextTick()
.then(() => {
expect(vm.buttonClasses).toBe('btn-blank btn-todo sidebar-collapsed-icon dont-change-state');
})
.then(done)
.catch(done.fail);
});
});
describe('buttonLabel', () => {
it('returns todo button text for marking todo as done when `isTodo` prop is `true`', () => {
expect(vm.buttonLabel).toBe('Mark todo as done');
});
it('returns todo button text for add todo when `isTodo` prop is `false`', done => {
vm.isTodo = false;
Vue.nextTick()
.then(() => {
expect(vm.buttonLabel).toBe('Add todo');
})
.then(done)
.catch(done.fail);
});
});
describe('collapsedButtonIconClasses', () => {
it('returns collapsed button icon class when `isTodo` prop is `true`', () => {
expect(vm.collapsedButtonIconClasses).toBe('todo-undone');
});
it('returns empty string when `isTodo` prop is `false`', done => {
vm.isTodo = false;
Vue.nextTick()
.then(() => {
expect(vm.collapsedButtonIconClasses).toBe('');
})
.then(done)
.catch(done.fail);
});
});
describe('collapsedButtonIcon', () => {
it('returns button icon name when `isTodo` prop is `true`', () => {
expect(vm.collapsedButtonIcon).toBe('todo-done');
});
it('returns button icon name when `isTodo` prop is `false`', done => {
vm.isTodo = false;
Vue.nextTick()
.then(() => {
expect(vm.collapsedButtonIcon).toBe('todo-add');
})
.then(done)
.catch(done.fail);
});
});
});
describe('methods', () => {
describe('handleButtonClick', () => {
it('emits `toggleTodo` event on component', () => {
spyOn(vm, '$emit');
vm.handleButtonClick();
expect(vm.$emit).toHaveBeenCalledWith('toggleTodo');
});
});
});
describe('template', () => {
it('renders component container element', () => {
const dataAttributes = {
issuableId: '1',
issuableType: 'epic',
originalTitle: 'Mark todo as done',
placement: 'left',
container: 'body',
boundary: 'viewport',
};
expect(vm.$el.nodeName).toBe('BUTTON');
const elDataAttrs = vm.$el.dataset;
Object.keys(elDataAttrs).forEach((attr) => {
expect(elDataAttrs[attr]).toBe(dataAttributes[attr]);
});
});
it('renders button label element when `collapsed` prop is `false`', () => {
const buttonLabelEl = vm.$el.querySelector('span.issuable-todo-inner');
expect(buttonLabelEl).not.toBeNull();
expect(buttonLabelEl.innerText.trim()).toBe('Mark todo as done');
});
it('renders button icon when `collapsed` prop is `true`', done => {
vm.collapsed = true;
Vue.nextTick()
.then(() => {
const buttonIconEl = vm.$el.querySelector('svg');
expect(buttonIconEl).not.toBeNull();
expect(buttonIconEl.querySelector('use').getAttribute('xlink:href')).toContain('todo-done');
})
.then(done)
.catch(done.fail);
});
it('renders loading icon when `isActionActive` prop is true', done => {
vm.isActionActive = true;
Vue.nextTick()
.then(() => {
const loadingEl = vm.$el.querySelector('span.loading-container');
expect(loadingEl).not.toBeNull();
})
.then(done)
.catch(done.fail);
});
});
});
...@@ -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
......
shared_examples 'todos actions' do
context 'when authorized' do
before do
sign_in(user)
parent.add_developer(user)
end
it 'creates todo' do
expect do
post_create
end.to change { user.todos.count }.by(1)
expect(response).to have_gitlab_http_status(200)
end
it 'returns todo path and pending count' do
post_create
expect(response).to have_gitlab_http_status(200)
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/group' do
it 'does not create todo for resource that user has no access to' do
sign_in(user)
expect do
post_create
end.to change { user.todos.count }.by(0)
expect(response).to have_gitlab_http_status(404)
end
it 'does not create todo when user is not logged in' do
expect do
post_create
end.to change { user.todos.count }.by(0)
expect(response).to have_gitlab_http_status(parent.is_a?(Group) ? 401 : 302)
end
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