Commit 8717c7da authored by Jarka Kadlecová's avatar Jarka Kadlecová

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

This reverts commit 4d9a3f42, reversing
changes made to ecf9c145.
parent c7c630f1
...@@ -39,7 +39,6 @@ export default class Todos { ...@@ -39,7 +39,6 @@ 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');
...@@ -54,16 +53,7 @@ export default class Todos { ...@@ -54,16 +53,7 @@ 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: () => { clicked: () => $dropdown.closest('form.filter-form').submit(),
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,11 +12,6 @@ export default { ...@@ -12,11 +12,6 @@ export default {
type: Boolean, type: Boolean,
required: true, required: true,
}, },
cssClasses: {
type: String,
required: false,
default: '',
},
}, },
computed: { computed: {
tooltipLabel() { tooltipLabel() {
...@@ -35,12 +30,10 @@ export default { ...@@ -35,12 +30,10 @@ 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,7 +449,6 @@ ...@@ -449,7 +449,6 @@
.todo-undone { .todo-undone {
color: $gl-link-color; color: $gl-link-color;
fill: $gl-link-color;
} }
.author { .author {
......
...@@ -174,18 +174,6 @@ ...@@ -174,18 +174,6 @@
} }
} }
@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 {
...@@ -211,10 +199,6 @@ ...@@ -211,10 +199,6 @@
} }
.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, :group_id) params.permit(:action_id, :author_id, :project_id, :type, :sort, :state)
end end
def redirect_out_of_range(todos) def redirect_out_of_range(todos)
......
class Projects::TodosController < Projects::ApplicationController class Projects::TodosController < Projects::ApplicationController
include Gitlab::Utils::StrongMemoize
include TodosActions
before_action :authenticate_user!, only: [:create] before_action :authenticate_user!, only: [:create]
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
private private
def issuable def issuable
strong_memoize(:issuable) do @issuable ||= begin
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,7 +15,6 @@ ...@@ -15,7 +15,6 @@
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? }
...@@ -35,11 +34,9 @@ class TodosFinder ...@@ -35,11 +34,9 @@ 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
...@@ -85,10 +82,6 @@ class TodosFinder ...@@ -85,10 +82,6 @@ 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)
...@@ -107,14 +100,18 @@ class TodosFinder ...@@ -107,14 +100,18 @@ class TodosFinder
@project @project
end end
def group def project_ids(items)
strong_memoize(:group) do ids = items.except(:order).select(:project_id)
Group.find(params[:group_id]) if Gitlab::Database.mysql?
# 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 Epic).include?(type) type.present? && %w(Issue MergeRequest).include?(type)
end end
def type def type
...@@ -151,37 +148,12 @@ class TodosFinder ...@@ -151,37 +148,12 @@ class TodosFinder
def by_project(items) def by_project(items)
if project? if project?
items = items.where(project: project) items.where(project: project)
end else
projects = Project.public_or_visible_to_user(current_user)
items
end
def by_group(items) items.joins(:project).merge(projects)
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 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.select(:id),
groups.select(:id)
)
end end
def by_state(items) def by_state(items)
......
...@@ -131,19 +131,6 @@ module IssuablesHelper ...@@ -131,19 +131,6 @@ 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.parent, todo.target] path = [todo.project.namespace.becomes(Namespace), todo.project, todo.target]
path.unshift(:pipelines) if todo.build_failed? path.unshift(:pipelines) if todo.build_failed?
...@@ -167,12 +167,4 @@ module TodosHelper ...@@ -167,12 +167,4 @@ 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,12 +243,6 @@ module Issuable ...@@ -243,12 +243,6 @@ 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,8 +39,6 @@ class Group < Namespace ...@@ -39,8 +39,6 @@ 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,12 +82,6 @@ class Group < Namespace ...@@ -84,12 +82,6 @@ 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,6 +275,10 @@ class Issue < ActiveRecord::Base ...@@ -275,6 +275,10 @@ 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,10 +229,6 @@ class Note < ActiveRecord::Base ...@@ -229,10 +229,6 @@ 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,18 +22,15 @@ class Todo < ActiveRecord::Base ...@@ -22,18 +22,15 @@ 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, :target_type, :user, presence: true validates :action, :project, :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) }
...@@ -47,7 +44,7 @@ class Todo < ActiveRecord::Base ...@@ -47,7 +44,7 @@ class Todo < ActiveRecord::Base
state :done state :done
end end
after_save :keep_around_commit, if: :commit_id after_save :keep_around_commit
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
...@@ -82,10 +79,6 @@ class Todo < ActiveRecord::Base ...@@ -82,10 +79,6 @@ 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(parent, target, author, note = nil, skip_users = []) def create_mention_todos(project, 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(parent, note || target, author, skip_users) directly_addressed_users = filter_directly_addressed_users(project, note || target, author, skip_users)
attributes = attributes_for_todo(parent, target, author, Todo::DIRECTLY_ADDRESSED, note) attributes = attributes_for_todo(project, 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(parent, note || target, author, skip_users) mentioned_users = filter_mentioned_users(project, note || target, author, skip_users)
attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note) attributes = attributes_for_todo(project, 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, parent, target) def filter_todo_users(users, project, target)
reject_users_without_access(users, parent, target).uniq reject_users_without_access(users, project, target).uniq
end end
def filter_mentioned_users(parent, target, author, skip_users = []) def filter_mentioned_users(project, 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, parent, target) filter_todo_users(mentioned_users, project, target)
end end
def filter_directly_addressed_users(parent, target, author, skip_users = []) def filter_directly_addressed_users(project, 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, parent, target) filter_todo_users(directly_addressed_users, project, target)
end end
def reject_users_without_access(users, parent, target) def reject_users_without_access(users, project, target)
if target.is_a?(Note) && target.for_issuable? if target.is_a?(Note) && (target.for_issue? || target.for_merge_request?)
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, parent) select_users(users, :read_project, project)
end end
end end
......
...@@ -30,33 +30,27 @@ ...@@ -30,33 +30,27 @@
.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 d-sm-flex' do = form_tag todos_filter_path(without: [:project_id, :author_id, :type, :action_id]), method: :get, class: 'filter-form' do
.filter-categories.flex-fill .filter-item.inline
.filter-item.inline - if params[:project_id].present?
- if params[:group_id].present? = hidden_field_tag(:project_id, params[:project_id])
= hidden_field_tag(:group_id, params[:group_id]) = dropdown_tag(project_dropdown_label(params[:project_id], 'Project'), options: { toggle_class: 'js-project-search js-filter-submit', title: 'Filter by project', filter: true, filterInput: 'input#project-search', dropdown_class: 'dropdown-menu-selectable dropdown-menu-project js-filter-submit',
= 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 projects', data: { data: todo_projects_options, default_label: 'Project', display: 'static' } })
placeholder: 'Search groups', data: { data: todo_group_options, default_label: 'Group', display: 'static' } }) .filter-item.inline
.filter-item.inline - if params[:author_id].present?
- if params[:project_id].present? = hidden_field_tag(:author_id, params[:author_id])
= hidden_field_tag(:project_id, params[:project_id]) = dropdown_tag(user_dropdown_label(params[:author_id], 'Author'), options: { toggle_class: 'js-user-search js-filter-submit js-author-search', title: 'Filter by author', filter: true, filterInput: 'input#author-search', dropdown_class: 'dropdown-menu-user dropdown-menu-selectable dropdown-menu-author js-filter-submit',
= dropdown_tag(project_dropdown_label(params[:project_id], 'Project'), options: { toggle_class: 'js-project-search js-filter-submit', title: 'Filter by project', filter: true, filterInput: 'input#project-search', dropdown_class: 'dropdown-menu-selectable dropdown-menu-project js-filter-submit', placeholder: 'Search authors', data: { any_user: 'Any Author', first_user: (current_user.username if current_user), project_id: (@project.id if @project), selected: params[:author_id], field_name: 'author_id', default_label: 'Author', todo_filter: true, todo_state_filter: params[:state] || 'pending' } })
placeholder: 'Search projects', data: { data: todo_projects_options, default_label: 'Project', display: 'static' } }) .filter-item.inline
.filter-item.inline - if params[:type].present?
- if params[:author_id].present? = hidden_field_tag(:type, params[:type])
= hidden_field_tag(:author_id, params[:author_id]) = dropdown_tag(todo_types_dropdown_label(params[:type], 'Type'), options: { toggle_class: 'js-type-search js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-type js-filter-submit',
= dropdown_tag(user_dropdown_label(params[:author_id], 'Author'), options: { toggle_class: 'js-user-search js-filter-submit js-author-search', title: 'Filter by author', filter: true, filterInput: 'input#author-search', dropdown_class: 'dropdown-menu-user dropdown-menu-selectable dropdown-menu-author js-filter-submit', data: { data: todo_types_options, default_label: 'Type' } })
placeholder: 'Search authors', data: { any_user: 'Any Author', first_user: (current_user.username if current_user), project_id: (@project.id if @project), selected: params[:author_id], field_name: 'author_id', default_label: 'Author', todo_filter: true, todo_state_filter: params[:state] || 'pending' } }) .filter-item.inline.actions-filter
.filter-item.inline - if params[:action_id].present?
- if params[:type].present? = hidden_field_tag(:action_id, params[:action_id])
= hidden_field_tag(:type, params[:type]) = dropdown_tag(todo_actions_dropdown_label(params[:action_id], 'Action'), options: { toggle_class: 'js-action-search js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-action js-filter-submit',
= dropdown_tag(todo_types_dropdown_label(params[:type], 'Type'), options: { toggle_class: 'js-type-search js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-type js-filter-submit', data: { data: todo_actions_options, default_label: 'Action' } })
data: { data: todo_types_options, default_label: 'Type' } })
.filter-item.inline.actions-filter
- if params[:action_id].present?
= hidden_field_tag(:action_id, params[:action_id])
= dropdown_tag(todo_actions_dropdown_label(params[:action_id], 'Action'), options: { toggle_class: 'js-action-search js-filter-submit', dropdown_class: 'dropdown-menu-selectable dropdown-menu-action js-filter-submit',
data: { data: todo_actions_options, default_label: 'Action' } })
.filter-item.sort-filter .filter-item.sort-filter
.dropdown .dropdown
%button.dropdown-menu-toggle.dropdown-menu-toggle-sort{ type: 'button', 'data-toggle' => 'dropdown' } %button.dropdown-menu-toggle.dropdown-menu-toggle-sort{ type: 'button', 'data-toggle' => 'dropdown' }
......
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
...@@ -1950,7 +1950,7 @@ ActiveRecord::Schema.define(version: 20180704204006) do ...@@ -1950,7 +1950,7 @@ ActiveRecord::Schema.define(version: 20180704204006) 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" t.integer "project_id", null: false
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
...@@ -1960,12 +1960,10 @@ ActiveRecord::Schema.define(version: 20180704204006) do ...@@ -1960,12 +1960,10 @@ ActiveRecord::Schema.define(version: 20180704204006) 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
...@@ -2339,7 +2337,6 @@ ActiveRecord::Schema.define(version: 20180704204006) do ...@@ -2339,7 +2337,6 @@ ActiveRecord::Schema.define(version: 20180704204006) 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,7 +18,6 @@ Parameters: ...@@ -18,7 +18,6 @@ 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,7 +109,6 @@ There are four kinds of filters you can use on your Todos dashboard. ...@@ -109,7 +109,6 @@ 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 |
......
...@@ -775,33 +775,28 @@ module API ...@@ -775,33 +775,28 @@ module API
class Todo < Grape::Entity class Todo < Grape::Entity
expose :id expose :id
expose :project, using: Entities::ProjectIdentity, if: -> (todo, _) { todo.project_id } expose :project, using: Entities::BasicProjectDetails
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|
todo_target_class(todo.target_type).represent(todo.target, options) Entities.const_get(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 = "#{todo.parent.class.to_s.underscore}_#{target_type}_url" target_url = "namespace_project_#{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.parent, todo.target, anchor: target_anchor) # rubocop:disable GitlabSecurity/PublicSend .public_send(target_url, todo.project.namespace, todo.project, 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,29 +5,10 @@ describe Projects::TodosController do ...@@ -5,29 +5,10 @@ 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 }
shared_examples 'project todos actions' do
it_behaves_like 'todos actions'
context 'when not authorized for resource' do
before do
project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
sign_in(user)
end
it "doesn't create todo" do
expect { post_create }.not_to change { user.todos.count }
expect(response).to have_gitlab_http_status(404)
end
end
end
context 'Issues' do context 'Issues' do
describe 'POST create' do describe 'POST create' do
def post_create def go
post :create, post :create,
namespace_id: project.namespace, namespace_id: project.namespace,
project_id: project, project_id: project,
...@@ -36,13 +17,66 @@ describe Projects::TodosController do ...@@ -36,13 +17,66 @@ describe Projects::TodosController do
format: 'html' format: 'html'
end end
it_behaves_like 'project todos actions' context 'when authorized' 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
project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE)
sign_in(user)
end
it "doesn't create todo" do
expect { go }.not_to change { user.todos.count }
expect(response).to have_gitlab_http_status(404)
end
end
end end
end end
context 'Merge Requests' do context 'Merge Requests' do
describe 'POST create' do describe 'POST create' do
def post_create def go
post :create, post :create,
namespace_id: project.namespace, namespace_id: project.namespace,
project_id: project, project_id: project,
...@@ -51,7 +85,60 @@ describe Projects::TodosController do ...@@ -51,7 +85,60 @@ describe Projects::TodosController do
format: 'html' format: 'html'
end end
it_behaves_like 'project todos actions' context 'when authorized' do
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
context 'when not authorized for project' do
it 'does not create todo for merge request 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 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
it "doesn't create todo" do
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 || user } author { project.creator }
user { project&.creator || user } user { project.creator }
target factory: :issue target factory: :issue
action { Todo::ASSIGNED } action { Todo::ASSIGNED }
......
...@@ -5,76 +5,12 @@ describe TodosFinder do ...@@ -5,76 +5,12 @@ 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,27 +21,6 @@ describe IssuablesHelper do ...@@ -21,27 +21,6 @@ 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,7 +7,6 @@ describe Todo do ...@@ -7,7 +7,6 @@ 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