summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2018-07-11 17:08:45 +0000
committerJose <jvargas@gitlab.com>2018-07-11 15:07:18 -0500
commit23b0ba4e296e19eb6a0856f2bb2bf4519a990572 (patch)
tree77b7797633c376a65cd8e4dbbaf8333eb7787e77
parentae95fde0f7f4976909b8cdd150d7dfbd4c3ef77c (diff)
downloadgitlab-ce-11-1-stable-prepare-rc8.tar.gz
Merge branch 'revert-todos-epic' into 'master'11-1-stable-prepare-rc8
Revert "Merge branch 'ee-5481-epic-todos' into 'master'" See merge request gitlab-org/gitlab-ce!20560
-rw-r--r--app/assets/javascripts/pages/dashboard/todos/index/todos.js12
-rw-r--r--app/assets/javascripts/sidebar/components/todo_toggle/todo.vue98
-rw-r--r--app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue7
-rw-r--r--app/assets/stylesheets/pages/issuable.scss1
-rw-r--r--app/assets/stylesheets/pages/todos.scss16
-rw-r--r--app/controllers/concerns/todos_actions.rb12
-rw-r--r--app/controllers/dashboard/todos_controller.rb2
-rw-r--r--app/controllers/projects/todos_controller.rb14
-rw-r--r--app/finders/todos_finder.rb52
-rw-r--r--app/helpers/issuables_helper.rb13
-rw-r--r--app/helpers/todos_helper.rb10
-rw-r--r--app/models/concerns/issuable.rb6
-rw-r--r--app/models/group.rb8
-rw-r--r--app/models/issue.rb4
-rw-r--r--app/models/note.rb4
-rw-r--r--app/models/todo.rb11
-rw-r--r--app/services/todo_service.rb30
-rw-r--r--app/views/dashboard/todos/index.html.haml48
-rw-r--r--db/migrate/20180608091413_add_group_to_todos.rb32
-rw-r--r--db/schema.rb5
-rw-r--r--doc/api/todos.md1
-rw-r--r--doc/workflow/todos.md1
-rw-r--r--lib/api/entities.rb13
-rw-r--r--spec/controllers/projects/todos_controller_spec.rb133
-rw-r--r--spec/factories/todos.rb4
-rw-r--r--spec/finders/todos_finder_spec.rb64
-rw-r--r--spec/helpers/issuables_helper_spec.rb21
-rw-r--r--spec/javascripts/sidebar/todo_spec.js158
-rw-r--r--spec/models/todo_spec.rb1
-rw-r--r--spec/support/shared_examples/controllers/todos_shared_examples.rb43
30 files changed, 184 insertions, 640 deletions
diff --git a/app/assets/javascripts/pages/dashboard/todos/index/todos.js b/app/assets/javascripts/pages/dashboard/todos/index/todos.js
index 9aa83ce6269..ff19b9a9c30 100644
--- a/app/assets/javascripts/pages/dashboard/todos/index/todos.js
+++ b/app/assets/javascripts/pages/dashboard/todos/index/todos.js
@@ -39,7 +39,6 @@ export default class Todos {
}
initFilters() {
- this.initFilterDropdown($('.js-group-search'), 'group_id', ['text']);
this.initFilterDropdown($('.js-project-search'), 'project_id', ['text']);
this.initFilterDropdown($('.js-type-search'), 'type');
this.initFilterDropdown($('.js-action-search'), 'action_id');
@@ -54,16 +53,7 @@ export default class Todos {
filterable: searchFields ? true : false,
search: { fields: searchFields },
data: $dropdown.data('data'),
- 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();
- },
+ clicked: () => $dropdown.closest('form.filter-form').submit(),
});
}
diff --git a/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue b/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue
deleted file mode 100644
index ffaed9c7193..00000000000
--- a/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue
+++ /dev/null
@@ -1,98 +0,0 @@
-<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>
diff --git a/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue b/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue
index 80dc7d3557c..ac2e99abe77 100644
--- a/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue
+++ b/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue
@@ -12,11 +12,6 @@ export default {
type: Boolean,
required: true,
},
- cssClasses: {
- type: String,
- required: false,
- default: '',
- },
},
computed: {
tooltipLabel() {
@@ -35,12 +30,10 @@ export default {
<button
v-tooltip
:title="tooltipLabel"
- :class="cssClasses"
type="button"
class="btn btn-blank gutter-toggle btn-sidebar-action"
data-container="body"
data-placement="left"
- data-boundary="viewport"
@click="toggle"
>
<i
diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss
index f6617380cc0..f9fd9f1ab8b 100644
--- a/app/assets/stylesheets/pages/issuable.scss
+++ b/app/assets/stylesheets/pages/issuable.scss
@@ -449,7 +449,6 @@
.todo-undone {
color: $gl-link-color;
- fill: $gl-link-color;
}
.author {
diff --git a/app/assets/stylesheets/pages/todos.scss b/app/assets/stylesheets/pages/todos.scss
index 010a2c05a1c..e5d7dd13915 100644
--- a/app/assets/stylesheets/pages/todos.scss
+++ b/app/assets/stylesheets/pages/todos.scss
@@ -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) {
.todo {
.avatar {
@@ -211,10 +199,6 @@
}
.todos-filters {
- .filter-categories {
- width: auto;
- }
-
.dropdown-menu-toggle {
width: 100%;
}
diff --git a/app/controllers/concerns/todos_actions.rb b/app/controllers/concerns/todos_actions.rb
deleted file mode 100644
index c0acdb3498d..00000000000
--- a/app/controllers/concerns/todos_actions.rb
+++ /dev/null
@@ -1,12 +0,0 @@
-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
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb
index bd7111e28bc..f9e8fe624e8 100644
--- a/app/controllers/dashboard/todos_controller.rb
+++ b/app/controllers/dashboard/todos_controller.rb
@@ -70,7 +70,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
end
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
def redirect_out_of_range(todos)
diff --git a/app/controllers/projects/todos_controller.rb b/app/controllers/projects/todos_controller.rb
index 93fb9da6510..a41fcb85c40 100644
--- a/app/controllers/projects/todos_controller.rb
+++ b/app/controllers/projects/todos_controller.rb
@@ -1,13 +1,19 @@
class Projects::TodosController < Projects::ApplicationController
- include Gitlab::Utils::StrongMemoize
- include TodosActions
-
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
def issuable
- strong_memoize(:issuable) do
+ @issuable ||= begin
case params[:issuable_type]
when "issue"
IssuesFinder.new(current_user, project_id: @project.id).find(params[:issuable_id])
diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index 2156413fb26..09e2c586f2a 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -15,7 +15,6 @@
class TodosFinder
prepend FinderWithCrossProjectAccess
include FinderMethods
- include Gitlab::Utils::StrongMemoize
requires_cross_project_access unless: -> { project? }
@@ -35,11 +34,9 @@ class TodosFinder
items = by_author(items)
items = by_state(items)
items = by_type(items)
- items = by_group(items)
# Filtering by project HAS TO be the last because we use
# the project IDs yielded by the todos query thus far
items = by_project(items)
- items = visible_to_user(items)
sort(items)
end
@@ -85,10 +82,6 @@ class TodosFinder
params[:project_id].present?
end
- def group?
- params[:group_id].present?
- end
-
def project
return @project if defined?(@project)
@@ -107,14 +100,18 @@ class TodosFinder
@project
end
- def group
- strong_memoize(:group) do
- Group.find(params[:group_id])
+ def project_ids(items)
+ ids = items.except(:order).select(:project_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
+
+ ids
end
def type?
- type.present? && %w(Issue MergeRequest Epic).include?(type)
+ type.present? && %w(Issue MergeRequest).include?(type)
end
def type
@@ -151,37 +148,12 @@ class TodosFinder
def by_project(items)
if project?
- items = items.where(project: project)
- end
-
- items
- end
+ items.where(project: project)
+ else
+ projects = Project.public_or_visible_to_user(current_user)
- 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)
- )
+ items.joins(:project).merge(projects)
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
def by_state(items)
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 7bbdc798ddd..8766bb43cac 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -131,19 +131,6 @@ module IssuablesHelper
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")
title =
case milestone_title
diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index 7cd74358168..f7620e0b6b8 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -43,7 +43,7 @@ module TodosHelper
project_commit_path(todo.project,
todo.target, anchor: anchor)
else
- path = [todo.parent, todo.target]
+ path = [todo.project.namespace.becomes(Namespace), todo.project, todo.target]
path.unshift(:pipelines) if todo.build_failed?
@@ -167,12 +167,4 @@ module TodosHelper
def show_todo_state?(todo)
(todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && %w(closed merged).include?(todo.target.state)
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
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 7a459078151..b93c1145f82 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -243,12 +243,6 @@ module Issuable
opened?
end
- def overdue?
- return false unless respond_to?(:due_date)
-
- due_date.try(:past?) || false
- end
-
def user_notes_count
if notes.loaded?
# Use the in-memory association to select and count to avoid hitting the db
diff --git a/app/models/group.rb b/app/models/group.rb
index b0392774379..9c171de7fc3 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -39,8 +39,6 @@ class Group < Namespace
has_many :boards
has_many :badges, class_name: 'GroupBadge'
- has_many :todos
-
accepts_nested_attributes_for :variables, allow_destroy: true
validate :visibility_level_allowed_by_projects
@@ -84,12 +82,6 @@ class Group < Namespace
where(id: user.authorized_groups.select(:id).reorder(nil))
end
- def public_or_visible_to_user(user)
- where('id IN (?) OR namespaces.visibility_level IN (?)',
- user.authorized_groups.select(:id),
- Gitlab::VisibilityLevel.levels_for_user(user))
- end
-
def select_for_project_authorization
if current_scope.joins_values.include?(:shared_projects)
joins('INNER JOIN namespaces project_namespace ON project_namespace.id = projects.namespace_id')
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 983684a5e05..4715d942c8d 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -275,6 +275,10 @@ class Issue < ActiveRecord::Base
user ? readable_by?(user) : publicly_visible?
end
+ def overdue?
+ due_date.try(:past?) || false
+ end
+
def check_for_spam?
project.public? && (title_changed? || description_changed?)
end
diff --git a/app/models/note.rb b/app/models/note.rb
index 3918bbee194..abc40d9016e 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -229,10 +229,6 @@ class Note < ActiveRecord::Base
!for_personal_snippet?
end
- def for_issuable?
- for_issue? || for_merge_request?
- end
-
def skip_project_check?
!for_project_noteable?
end
diff --git a/app/models/todo.rb b/app/models/todo.rb
index 942cbb754e3..a2ab405fdbe 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -22,18 +22,15 @@ class Todo < ActiveRecord::Base
belongs_to :author, class_name: "User"
belongs_to :note
belongs_to :project
- belongs_to :group
belongs_to :target, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations
belongs_to :user
delegate :name, :email, to: :author, prefix: true, allow_nil: true
- validates :action, :target_type, :user, presence: true
+ validates :action, :project, :target_type, :user, presence: true
validates :author, presence: true
validates :target_id, presence: true, unless: :for_commit?
validates :commit_id, presence: true, if: :for_commit?
- validates :project, presence: true, unless: :group_id
- validates :group, presence: true, unless: :project_id
scope :pending, -> { with_state(:pending) }
scope :done, -> { with_state(:done) }
@@ -47,7 +44,7 @@ class Todo < ActiveRecord::Base
state :done
end
- after_save :keep_around_commit, if: :commit_id
+ after_save :keep_around_commit
class << self
# Priority sorting isn't displayed in the dropdown, because we don't show
@@ -82,10 +79,6 @@ class Todo < ActiveRecord::Base
end
end
- def parent
- project
- end
-
def unmergeable?
action == UNMERGEABLE
end
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index 46f12086555..f91cd03bf5c 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -260,15 +260,15 @@ class TodoService
end
end
- def create_mention_todos(parent, target, author, note = nil, skip_users = [])
+ def create_mention_todos(project, target, author, note = nil, skip_users = [])
# Create Todos for directly addressed users
- directly_addressed_users = filter_directly_addressed_users(parent, note || target, author, skip_users)
- attributes = attributes_for_todo(parent, target, author, Todo::DIRECTLY_ADDRESSED, note)
+ directly_addressed_users = filter_directly_addressed_users(project, note || target, author, skip_users)
+ attributes = attributes_for_todo(project, target, author, Todo::DIRECTLY_ADDRESSED, note)
create_todos(directly_addressed_users, attributes)
# Create Todos for mentioned users
- mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users)
- attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note)
+ mentioned_users = filter_mentioned_users(project, note || target, author, skip_users)
+ attributes = attributes_for_todo(project, target, author, Todo::MENTIONED, note)
create_todos(mentioned_users, attributes)
end
@@ -299,36 +299,36 @@ class TodoService
def attributes_for_todo(project, target, author, action, note = nil)
attributes_for_target(target).merge!(
- project_id: project&.id,
+ project_id: project.id,
author_id: author.id,
action: action,
note: note
)
end
- def filter_todo_users(users, parent, target)
- reject_users_without_access(users, parent, target).uniq
+ def filter_todo_users(users, project, target)
+ reject_users_without_access(users, project, target).uniq
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
- filter_todo_users(mentioned_users, parent, target)
+ filter_todo_users(mentioned_users, project, target)
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
- filter_todo_users(directly_addressed_users, parent, target)
+ filter_todo_users(directly_addressed_users, project, target)
end
- def reject_users_without_access(users, parent, target)
- if target.is_a?(Note) && target.for_issuable?
+ def reject_users_without_access(users, project, target)
+ if target.is_a?(Note) && (target.for_issue? || target.for_merge_request?)
target = target.noteable
end
if target.is_a?(Issuable)
select_users(users, :"read_#{target.to_ability_name}", target)
else
- select_users(users, :read_project, parent)
+ select_users(users, :read_project, project)
end
end
diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml
index 8b3974d97f8..d5a9cc646a6 100644
--- a/app/views/dashboard/todos/index.html.haml
+++ b/app/views/dashboard/todos/index.html.haml
@@ -30,33 +30,27 @@
.todos-filters
.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
- .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
- - if params[:project_id].present?
- = hidden_field_tag(:project_id, params[:project_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',
- placeholder: 'Search projects', data: { data: todo_projects_options, default_label: 'Project', display: 'static' } })
- .filter-item.inline
- - if params[:author_id].present?
- = hidden_field_tag(:author_id, params[:author_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',
- 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
- - if params[:type].present?
- = hidden_field_tag(:type, params[:type])
- = 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_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' } })
+ = form_tag todos_filter_path(without: [:project_id, :author_id, :type, :action_id]), method: :get, class: 'filter-form' do
+ .filter-item.inline
+ - if params[:project_id].present?
+ = hidden_field_tag(:project_id, params[:project_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',
+ placeholder: 'Search projects', data: { data: todo_projects_options, default_label: 'Project', display: 'static' } })
+ .filter-item.inline
+ - if params[:author_id].present?
+ = hidden_field_tag(:author_id, params[:author_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',
+ 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
+ - if params[:type].present?
+ = hidden_field_tag(:type, params[:type])
+ = 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_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
.dropdown
%button.dropdown-menu-toggle.dropdown-menu-toggle-sort{ type: 'button', 'data-toggle' => 'dropdown' }
diff --git a/db/migrate/20180608091413_add_group_to_todos.rb b/db/migrate/20180608091413_add_group_to_todos.rb
deleted file mode 100644
index af3ee48b29d..00000000000
--- a/db/migrate/20180608091413_add_group_to_todos.rb
+++ /dev/null
@@ -1,32 +0,0 @@
-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
diff --git a/db/schema.rb b/db/schema.rb
index 1898dfc6022..79091892441 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1949,7 +1949,7 @@ ActiveRecord::Schema.define(version: 20180702120647) do
create_table "todos", force: :cascade do |t|
t.integer "user_id", null: false
- t.integer "project_id"
+ t.integer "project_id", null: false
t.integer "target_id"
t.string "target_type", null: false
t.integer "author_id", null: false
@@ -1959,12 +1959,10 @@ ActiveRecord::Schema.define(version: 20180702120647) do
t.datetime "updated_at"
t.integer "note_id"
t.string "commit_id"
- t.integer "group_id"
end
add_index "todos", ["author_id"], name: "index_todos_on_author_id", using: :btree
add_index "todos", ["commit_id"], name: "index_todos_on_commit_id", using: :btree
- add_index "todos", ["group_id"], name: "index_todos_on_group_id", using: :btree
add_index "todos", ["note_id"], name: "index_todos_on_note_id", using: :btree
add_index "todos", ["project_id"], name: "index_todos_on_project_id", using: :btree
add_index "todos", ["target_type", "target_id"], name: "index_todos_on_target_type_and_target_id", using: :btree
@@ -2338,7 +2336,6 @@ ActiveRecord::Schema.define(version: 20180702120647) do
add_foreign_key "term_agreements", "users", on_delete: :cascade
add_foreign_key "timelogs", "issues", name: "fk_timelogs_issues_issue_id", on_delete: :cascade
add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade
- add_foreign_key "todos", "namespaces", column: "group_id", on_delete: :cascade
add_foreign_key "todos", "notes", name: "fk_91d1f47b13", on_delete: :cascade
add_foreign_key "todos", "projects", name: "fk_45054f9c45", on_delete: :cascade
add_foreign_key "todos", "users", column: "author_id", name: "fk_ccf0373936", on_delete: :cascade
diff --git a/doc/api/todos.md b/doc/api/todos.md
index 0843e4eedc6..27e623007cc 100644
--- a/doc/api/todos.md
+++ b/doc/api/todos.md
@@ -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`. |
| `author_id` | integer | no | The ID of an author |
| `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` |
| `type` | string | no | The type of a todo. Can be either `Issue` or `MergeRequest` |
diff --git a/doc/workflow/todos.md b/doc/workflow/todos.md
index dda82352c67..760cd87d4cc 100644
--- a/doc/workflow/todos.md
+++ b/doc/workflow/todos.md
@@ -109,7 +109,6 @@ There are four kinds of filters you can use on your Todos dashboard.
| Filter | Description |
| ------- | ----------- |
| Project | Filter by project |
-| Group | Filter by group |
| Author | Filter by the author that triggered the Todo |
| Type | Filter by issue or merge request |
| Action | Filter by the action that triggered the Todo |
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 40df1e79bc7..66b62d9ee2e 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -775,33 +775,28 @@ module API
class Todo < Grape::Entity
expose :id
- expose :project, using: Entities::ProjectIdentity, if: -> (todo, _) { todo.project_id }
- expose :group, using: 'API::Entities::NamespaceBasic', if: -> (todo, _) { todo.group_id }
+ expose :project, using: Entities::BasicProjectDetails
expose :author, using: Entities::UserBasic
expose :action_name
expose :target_type
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
expose :target_url do |todo, options|
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?
Gitlab::Routing
.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
expose :body
expose :state
expose :created_at
-
- def todo_target_class(target_type)
- ::API::Entities.const_get(target_type)
- end
end
class NamespaceBasic < Grape::Entity
diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb
index 58f2817c7cc..1ce7e84bef9 100644
--- a/spec/controllers/projects/todos_controller_spec.rb
+++ b/spec/controllers/projects/todos_controller_spec.rb
@@ -5,29 +5,10 @@ describe Projects::TodosController do
let(:project) { create(:project) }
let(:issue) { create(:issue, 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
describe 'POST create' do
- def post_create
+ def go
post :create,
namespace_id: project.namespace,
project_id: project,
@@ -36,13 +17,66 @@ describe Projects::TodosController do
format: 'html'
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
context 'Merge Requests' do
describe 'POST create' do
- def post_create
+ def go
post :create,
namespace_id: project.namespace,
project_id: project,
@@ -51,7 +85,60 @@ describe Projects::TodosController do
format: 'html'
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
diff --git a/spec/factories/todos.rb b/spec/factories/todos.rb
index 14486c80341..94f8caedfa6 100644
--- a/spec/factories/todos.rb
+++ b/spec/factories/todos.rb
@@ -1,8 +1,8 @@
FactoryBot.define do
factory :todo do
project
- author { project&.creator || user }
- user { project&.creator || user }
+ author { project.creator }
+ user { project.creator }
target factory: :issue
action { Todo::ASSIGNED }
diff --git a/spec/finders/todos_finder_spec.rb b/spec/finders/todos_finder_spec.rb
index 6061021d3b0..9747b9402a7 100644
--- a/spec/finders/todos_finder_spec.rb
+++ b/spec/finders/todos_finder_spec.rb
@@ -5,76 +5,12 @@ describe TodosFinder do
let(:user) { create(:user) }
let(:group) { create(:group) }
let(:project) { create(:project, namespace: group) }
- let(:issue) { create(:issue, project: project) }
- let(:merge_request) { create(:merge_request, source_project: project) }
let(:finder) { described_class }
before do
group.add_developer(user)
end
- describe '#execute' do
- context 'visibility' do
- let(:private_group_access) { create(:group, :private) }
- let(:private_group_hidden) { create(:group, :private) }
- let(:public_project) { create(:project, :public) }
- let(:private_project_hidden) { create(:project) }
- let(:public_group) { create(:group) }
-
- let!(:todo1) { create(:todo, user: user, project: project, group: nil) }
- let!(:todo2) { create(:todo, user: user, project: public_project, group: nil) }
- let!(:todo3) { create(:todo, user: user, project: private_project_hidden, group: nil) }
- let!(:todo4) { create(:todo, user: user, project: nil, group: group) }
- let!(:todo5) { create(:todo, user: user, project: nil, group: private_group_access) }
- let!(:todo6) { create(:todo, user: user, project: nil, group: private_group_hidden) }
- let!(:todo7) { create(:todo, user: user, project: nil, group: public_group) }
-
- before do
- private_group_access.add_developer(user)
- end
-
- it 'returns only todos with a target a user has access to' do
- todos = finder.new(user).execute
-
- expect(todos).to match_array([todo1, todo2, todo4, todo5, todo7])
- end
- end
-
- context 'filtering' do
- let!(:todo1) { create(:todo, user: user, project: project, target: issue) }
- let!(:todo2) { create(:todo, user: user, group: group, target: merge_request) }
-
- it 'returns correct todos when filtered by a project' do
- todos = finder.new(user, { project_id: project.id }).execute
-
- expect(todos).to match_array([todo1])
- end
-
- it 'returns correct todos when filtered by a group' do
- todos = finder.new(user, { group_id: group.id }).execute
-
- expect(todos).to match_array([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
context 'by date' do
let!(:todo1) { create(:todo, user: user, project: project) }
diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb
index f76ed4bfda4..77410e0070c 100644
--- a/spec/helpers/issuables_helper_spec.rb
+++ b/spec/helpers/issuables_helper_spec.rb
@@ -21,27 +21,6 @@ describe IssuablesHelper do
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
it 'returns label text with no labels' do
expect(issuable_labels_tooltip([])).to eq("Labels")
diff --git a/spec/javascripts/sidebar/todo_spec.js b/spec/javascripts/sidebar/todo_spec.js
deleted file mode 100644
index a929b804a29..00000000000
--- a/spec/javascripts/sidebar/todo_spec.js
+++ /dev/null
@@ -1,158 +0,0 @@
-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);
- });
- });
-});
diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb
index f29abcf536e..bd498269798 100644
--- a/spec/models/todo_spec.rb
+++ b/spec/models/todo_spec.rb
@@ -7,7 +7,6 @@ describe Todo do
it { is_expected.to belong_to(:author).class_name("User") }
it { is_expected.to belong_to(:note) }
it { is_expected.to belong_to(:project) }
- it { is_expected.to belong_to(:group) }
it { is_expected.to belong_to(:target).touch(true) }
it { is_expected.to belong_to(:user) }
end
diff --git a/spec/support/shared_examples/controllers/todos_shared_examples.rb b/spec/support/shared_examples/controllers/todos_shared_examples.rb
deleted file mode 100644
index bafd9bac8d0..00000000000
--- a/spec/support/shared_examples/controllers/todos_shared_examples.rb
+++ /dev/null
@@ -1,43 +0,0 @@
-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