diff --git a/app/assets/javascripts/pages/dashboard/todos/index/todos.js b/app/assets/javascripts/pages/dashboard/todos/index/todos.js index ff19b9a9c301e08ebf80da56cb484e049eb130f2..9aa83ce62690cd9ff4ad68592dfb2157a4d7b8a9 100644 --- a/app/assets/javascripts/pages/dashboard/todos/index/todos.js +++ b/app/assets/javascripts/pages/dashboard/todos/index/todos.js @@ -39,6 +39,7 @@ 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'); @@ -53,7 +54,16 @@ export default class Todos { filterable: searchFields ? true : false, search: { fields: searchFields }, 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(); + }, }); } diff --git a/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue b/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue new file mode 100644 index 0000000000000000000000000000000000000000..ffaed9c7193e0f25d418f65134bea95288a83b0b --- /dev/null +++ b/app/assets/javascripts/sidebar/components/todo_toggle/todo.vue @@ -0,0 +1,98 @@ +<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 ac2e99abe77e2fd4835bcd00ba9ef904a4ccef98..80dc7d3557ca9ae3fea910db111dc8359188265f 100644 --- a/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue +++ b/app/assets/javascripts/vue_shared/components/sidebar/toggle_sidebar.vue @@ -12,6 +12,11 @@ export default { type: Boolean, required: true, }, + cssClasses: { + type: String, + required: false, + default: '', + }, }, computed: { tooltipLabel() { @@ -30,10 +35,12 @@ 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 226eded94234af69383cea1abe4de37ec09c8d22..526d079329b03a171199ec4db111f3af746a905a 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -462,6 +462,7 @@ .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 e5d7dd139159eb12218898d13d7da6377e0227f2..010a2c05a1c78aa5ad71db54acebefb31734d09b 100644 --- a/app/assets/stylesheets/pages/todos.scss +++ b/app/assets/stylesheets/pages/todos.scss @@ -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) { .todo { .avatar { @@ -199,6 +211,10 @@ } .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 new file mode 100644 index 0000000000000000000000000000000000000000..c0acdb3498dd509f2c199601b7ef955c7891ffdf --- /dev/null +++ b/app/controllers/concerns/todos_actions.rb @@ -0,0 +1,12 @@ +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 f9e8fe624e8572c8046b519c61e4febcf84c4adb..bd7111e28bcabfacac2e4e0bab5533d5a7e72699 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) + params.permit(:action_id, :author_id, :project_id, :type, :sort, :state, :group_id) end def redirect_out_of_range(todos) diff --git a/app/controllers/projects/todos_controller.rb b/app/controllers/projects/todos_controller.rb index a41fcb85c40791a0222e19236b64c80c4d65f153..93fb9da65107a364e19056e44a3fcb9456026cea 100644 --- a/app/controllers/projects/todos_controller.rb +++ b/app/controllers/projects/todos_controller.rb @@ -1,19 +1,13 @@ class Projects::TodosController < Projects::ApplicationController - before_action :authenticate_user!, only: [:create] - - def create - todo = TodoService.new.mark_todo(issuable, current_user) + include Gitlab::Utils::StrongMemoize + include TodosActions - render json: { - count: TodosFinder.new(current_user, state: :pending).execute.count, - delete_path: dashboard_todo_path(todo) - } - end + before_action :authenticate_user!, only: [:create] private def issuable - @issuable ||= begin + strong_memoize(:issuable) do 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 09e2c586f2a10053d833a0633e38c5554e05cf41..2156413fb26ec2b58c371dac0d759a8931ed4c8e 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -15,6 +15,7 @@ class TodosFinder prepend FinderWithCrossProjectAccess include FinderMethods + include Gitlab::Utils::StrongMemoize requires_cross_project_access unless: -> { project? } @@ -34,9 +35,11 @@ class TodosFinder items = by_author(items) items = by_state(items) items = by_type(items) + items = by_group(items) # Filtering by project HAS TO be the last because we use # the project IDs yielded by the todos query thus far items = by_project(items) + items = visible_to_user(items) sort(items) end @@ -82,6 +85,10 @@ class TodosFinder params[:project_id].present? end + def group? + params[:group_id].present? + end + def project return @project if defined?(@project) @@ -100,18 +107,14 @@ class TodosFinder @project end - 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") + def group + strong_memoize(:group) do + Group.find(params[:group_id]) end - - ids end def type? - type.present? && %w(Issue MergeRequest).include?(type) + type.present? && %w(Issue MergeRequest Epic).include?(type) end def type @@ -148,12 +151,37 @@ class TodosFinder def by_project(items) if project? - items.where(project: project) - else - projects = Project.public_or_visible_to_user(current_user) + items = items.where(project: project) + end + + items + end - items.joins(:project).merge(projects) + 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 + 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 776eae4a89206cfd53375724b3bdd85272733211..acec2ba2da60da2c4cafd8470d683885d645ecbe 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -133,6 +133,19 @@ 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 f7620e0b6b82a787fe83dc211f0476d732f14a5f..558e4456135d5a2303d0f57a0941a0d63a0fb8b8 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -1,4 +1,6 @@ module TodosHelper + prepend EE::NotesHelper + def todos_pending_count @todos_pending_count ||= current_user.todos_pending_count end @@ -43,7 +45,7 @@ module TodosHelper project_commit_path(todo.project, todo.target, anchor: anchor) else - path = [todo.project.namespace.becomes(Namespace), todo.project, todo.target] + path = [todo.parent, todo.target] path.unshift(:pipelines) if todo.build_failed? @@ -167,4 +169,12 @@ 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 5ab95a948012ae2f4d5321f92882e2678419fb0c..de5d2eea16a8136830ec4a29a74d9ea0ff7594f1 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -253,6 +253,12 @@ 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 7073317c8575869bd68ee8017c1f8d3c21f72247..5f67f719285c97fafd99e79a2afe35494dec2d36 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -43,6 +43,8 @@ class Group < Namespace has_many :boards has_many :badges, class_name: 'GroupBadge' + has_many :todos + accepts_nested_attributes_for :variables, allow_destroy: true validate :visibility_level_allowed_by_projects @@ -86,6 +88,12 @@ 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 56a08b41b8c97120e785a1bd1f35cdcd44bf980d..1a93af6f3a811febe24c0ed9a8c98836abbbd430 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -302,10 +302,6 @@ 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 e153d36f4b10b8a148406aadf438483f1538809b..d3c2b3c40e0c3cbd61effec94952a26ff734c89e 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -239,6 +239,10 @@ 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 5f5c2f9073de54848cd1605ccd229edcae93551a..e5e40d981c50386a761825a67a4b262934a98571 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true class Todo < ActiveRecord::Base + prepend EE::Todo include Sortable ASSIGNED = 1 @@ -24,15 +25,18 @@ class Todo < ActiveRecord::Base belongs_to :author, class_name: "User" belongs_to :note belongs_to :project + belongs_to :group belongs_to :target, polymorphic: true, touch: true # rubocop:disable Cop/PolymorphicAssociations belongs_to :user delegate :name, :email, to: :author, prefix: true, allow_nil: true - validates :action, :project, :target_type, :user, presence: true + validates :action, :target_type, :user, presence: true validates :author, presence: true validates :target_id, presence: true, unless: :for_commit? validates :commit_id, presence: true, if: :for_commit? + validates :project, presence: true, unless: :group_id + validates :group, presence: true, unless: :project_id scope :pending, -> { with_state(:pending) } scope :done, -> { with_state(:done) } @@ -46,7 +50,7 @@ class Todo < ActiveRecord::Base state :done end - after_save :keep_around_commit + after_save :keep_around_commit, if: :commit_id class << self # Priority sorting isn't displayed in the dropdown, because we don't show @@ -81,6 +85,10 @@ 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 83434c1a210baf846d29bd59f39eee1a33610b96..709d9394a7db321d6d58d7220155e1ff46a5697a 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -264,15 +264,15 @@ class TodoService end end - def create_mention_todos(project, target, author, note = nil, skip_users = []) + def create_mention_todos(parent, target, author, note = nil, skip_users = []) # Create Todos for directly addressed users - directly_addressed_users = filter_directly_addressed_users(project, note || target, author, skip_users) - attributes = attributes_for_todo(project, target, author, Todo::DIRECTLY_ADDRESSED, note) + directly_addressed_users = filter_directly_addressed_users(parent, note || target, author, skip_users) + attributes = attributes_for_todo(parent, target, author, Todo::DIRECTLY_ADDRESSED, note) create_todos(directly_addressed_users, attributes) # Create Todos for mentioned users - mentioned_users = filter_mentioned_users(project, note || target, author, skip_users) - attributes = attributes_for_todo(project, target, author, Todo::MENTIONED, note) + mentioned_users = filter_mentioned_users(parent, note || target, author, skip_users) + attributes = attributes_for_todo(parent, target, author, Todo::MENTIONED, note) create_todos(mentioned_users, attributes) end @@ -303,36 +303,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, project, target) - reject_users_without_access(users, project, target).uniq + def filter_todo_users(users, parent, target) + reject_users_without_access(users, parent, target).uniq end - def filter_mentioned_users(project, target, author, skip_users = []) + def filter_mentioned_users(parent, target, author, skip_users = []) mentioned_users = target.mentioned_users(author) - skip_users - filter_todo_users(mentioned_users, project, target) + filter_todo_users(mentioned_users, parent, target) end - def filter_directly_addressed_users(project, target, author, skip_users = []) + def filter_directly_addressed_users(parent, target, author, skip_users = []) directly_addressed_users = target.directly_addressed_users(author) - skip_users - filter_todo_users(directly_addressed_users, project, target) + filter_todo_users(directly_addressed_users, parent, target) end - def reject_users_without_access(users, project, target) - if target.is_a?(Note) && (target.for_issue? || target.for_merge_request?) + def reject_users_without_access(users, parent, target) + if target.is_a?(Note) && target.for_issuable? target = target.noteable end if target.is_a?(Issuable) select_users(users, :"read_#{target.to_ability_name}", target) else - select_users(users, :read_project, project) + select_users(users, :read_project, parent) end end diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index d5a9cc646a60f070e5dce032813487d86ae24c23..8b3974d97f8c2456e3ea268b6685ae9d5c26c7c4 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -30,27 +30,33 @@ .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' 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' } }) + = 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' } }) .filter-item.sort-filter .dropdown %button.dropdown-menu-toggle.dropdown-menu-toggle-sort{ type: 'button', 'data-toggle' => 'dropdown' } diff --git a/config/routes/group.rb b/config/routes/group.rb index 977394d83044376aa642d3c026c57a7a70c17937..77132cee41c686ee472a67366380f9875706c2d1 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -110,6 +110,8 @@ constraints(::Constraints::GroupUrlConstrainer.new) do end end + resources :todos, only: [:create] + # On CE only index and show are needed resources :boards, only: [:index, :show, :create, :update, :destroy] diff --git a/db/migrate/20180608091413_add_group_to_todos.rb b/db/migrate/20180608091413_add_group_to_todos.rb new file mode 100644 index 0000000000000000000000000000000000000000..af3ee48b29df8e9a1c7ab3fd3d2e9cdc1ab3e5e8 --- /dev/null +++ b/db/migrate/20180608091413_add_group_to_todos.rb @@ -0,0 +1,32 @@ +class AddGroupToTodos < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column :todos, :group_id, :integer + add_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 1f4d8bc34d5ab2f553029643559c180a08f03dfd..1b9cb6538464108b3a3bfe89f6d628d067d519d8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2598,7 +2598,7 @@ ActiveRecord::Schema.define(version: 20180726172057) do create_table "todos", force: :cascade do |t| t.integer "user_id", null: false - t.integer "project_id", null: false + t.integer "project_id" t.integer "target_id" t.string "target_type", null: false t.integer "author_id", null: false @@ -2608,10 +2608,12 @@ ActiveRecord::Schema.define(version: 20180726172057) 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 @@ -3085,6 +3087,7 @@ ActiveRecord::Schema.define(version: 20180726172057) 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/epics.md b/doc/api/epics.md index 741680ba0f2f21bb18357ae3c9293ace9ab73716..502e327a3bbb5ea879225427ecb2ce9315d8977b 100644 --- a/doc/api/epics.md +++ b/doc/api/epics.md @@ -210,3 +210,71 @@ DELETE /groups/:id/epics/:epic_iid ```bash curl --header DELETE "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/1/epics/5?title=New%20Title ``` + +## Create a todo + +Manually creates a todo for the current user on an epic. If +there already exists a todo for the user on that epic, status code `304` is +returned. + +``` +POST /groups/:id/epics/:epic_iid/todo +``` + +| Attribute | Type | Required | Description | +|-------------|---------|----------|--------------------------------------| +| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user | +| `epic_iid ` | integer | yes | The internal ID of a group's epic | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/1/epics/5/todo +``` + +Example response: + +```json +{ + "id": 112, + "group": { + "id": 1, + "name": "Gitlab", + "path": "gitlab", + "kind": "group", + "full_path": "base/gitlab", + "parent_id": null + }, + "author": { + "name": "Administrator", + "username": "root", + "id": 1, + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "web_url": "https://gitlab.example.com/root" + }, + "action_name": "marked", + "target_type": "epic", + "target": { + "id": 30, + "iid": 5, + "group_id": 1, + "title": "Ea cupiditate dolores ut vero consequatur quasi veniam voluptatem et non.", + "description": "Molestias dolorem eos vitae expedita impedit necessitatibus quo voluptatum.", + "author":{ + "id": 7, + "name": "Pamella Huel", + "username": "arnita", + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/a2f5c6fcef64c9c69cb8779cb292be1b?s=80&d=identicon", + "web_url": "http://localhost:3001/arnita" + }, + "start_date": null, + "end_date": null, + "created_at": "2018-01-21T06:21:13.165Z", + "updated_at": "2018-01-22T12:41:41.166Z" + }, + "target_url": "https://gitlab.example.com/groups/epics/5", + "body": "Vel voluptas atque dicta mollitia adipisci qui at.", + "state": "pending", + "created_at": "2016-07-01T11:09:13.992Z" +} +``` diff --git a/doc/api/todos.md b/doc/api/todos.md index 71afd5e7ecc34e1276af1eb5002ed3c245d6b12c..9543ec8089866c328499315855c8fbc0f62fec15 100644 --- a/doc/api/todos.md +++ b/doc/api/todos.md @@ -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`. | | `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 760cd87d4cc0c5c0fb5f4a6a32bce2159ffe0bad..d9a2871d588f22efc88c0eac546185651040681f 100644 --- a/doc/workflow/todos.md +++ b/doc/workflow/todos.md @@ -26,8 +26,8 @@ will still be shown in the body of the _To do_ tab. A Todo appears in your Todos dashboard when: - an issue or merge request is assigned to you, -- you are `@mentioned` in an issue or merge request, be it the description of - the issue/merge request or in a comment, +- you are `@mentioned` in an issue, merge request or epic, be it the description of + the issue/merge request/epic or in a comment, - you are `@mentioned` in a comment on a commit, - a job in the CI pipeline running for your merge request failed, but this job is not allowed to fail. @@ -60,14 +60,14 @@ for filtering; otherwise, they appear as normal. ### Manually creating a Todo -You can also add an issue or merge request to your Todos dashboard by clicking -the "Add todo" button in the issue or merge request sidebar. +You can also add an issue, merge request or epic to your Todos dashboard by clicking +the "Add todo" button in the issue, merge request or epic sidebar. ![Adding a Todo from the issuable sidebar](img/todos_add_todo_sidebar.png) ## Marking a Todo as done -Any action to the corresponding issue or merge request will mark your Todo as +Any action to the corresponding issue, merge request or epic will mark your Todo as **Done**. Actions that dismiss Todos include: - changing the assignee @@ -81,10 +81,10 @@ Todos are personal, and they're only marked as done if the action is coming from you. If you close the issue or merge request, your Todo will automatically be marked as done. -If someone else closes, merges, or takes action on the issue or merge +If someone else closes, merges, or takes action on the issue, epic or merge request, your Todo will remain pending. This prevents other users from closing issues without you being notified. -There is just one Todo per issue or merge request, so mentioning a user a +There is just one Todo per issue, epic or merge request, so mentioning a user a hundred times in an issue will only trigger one Todo. --- @@ -94,7 +94,7 @@ corresponding **Done** button, and it will disappear from your Todo list. ![A Todo in the Todos dashboard](img/todo_list_item.png) -A Todo can also be marked as done from the issue or merge request sidebar using +A Todo can also be marked as done from the issue, merge request or epic sidebar using the "Mark todo as done" button. ![Mark todo as done from the issuable sidebar](img/todos_mark_done_sidebar.png) @@ -109,8 +109,9 @@ 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 | +| Type | Filter by issue, merge request or epic | | Action | Filter by the action that triggered the Todo | You can also filter by more than one of these at the same time. The possible Actions are `Any Action`, `Assigned`, `Mentioned`, `Added`, `Pipelines`, and `Directly Addressed`, [as described above](#what-triggers-a-todo). diff --git a/ee/app/assets/javascripts/epics/epic_show/components/epic_show_app.vue b/ee/app/assets/javascripts/epics/epic_show/components/epic_show_app.vue index aea33428783bfdfe64347f89f4b89ab22cd27ed3..1d1f1f2820486c47b0bed2f80fc175e2b64e5bfb 100644 --- a/ee/app/assets/javascripts/epics/epic_show/components/epic_show_app.vue +++ b/ee/app/assets/javascripts/epics/epic_show/components/epic_show_app.vue @@ -16,6 +16,10 @@ relatedIssuesRoot, }, props: { + epicId: { + type: Number, + required: true, + }, endpoint: { type: String, required: true, @@ -98,6 +102,10 @@ type: Boolean, required: true, }, + todoExists: { + type: Boolean, + required: true, + }, namespace: { type: String, required: false, @@ -111,6 +119,15 @@ type: String, required: true, }, + todoPath: { + type: String, + required: true, + }, + todoDeletePath: { + type: String, + required: false, + default: '', + }, labelsWebUrl: { type: String, required: true, @@ -170,6 +187,7 @@ /> </div> <epic-sidebar + :epic-id="epicId" :endpoint="endpoint" :editable="canUpdate" :initial-start-date="startDate" @@ -177,10 +195,13 @@ :initial-labels="labels" :initial-participants="participants" :initial-subscribed="subscribed" + :initial-todo-exists="todoExists" :namespace="namespace" :update-path="updateEndpoint" :labels-path="labelsPath" :toggle-subscription-path="toggleSubscriptionPath" + :todo-path="todoPath" + :todo-delete-path="todoDeletePath" :labels-web-url="labelsWebUrl" :epics-web-url="epicsWebUrl" /> diff --git a/ee/app/assets/javascripts/epics/epic_show/epic_show_bundle.js b/ee/app/assets/javascripts/epics/epic_show/epic_show_bundle.js index 991e053245887332b8931edc064c8a18095d1be7..b898bade2af1d502acc9a846ff36b69ec47ad7b6 100644 --- a/ee/app/assets/javascripts/epics/epic_show/epic_show_bundle.js +++ b/ee/app/assets/javascripts/epics/epic_show/epic_show_bundle.js @@ -1,17 +1,14 @@ import Vue from 'vue'; +import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import EpicShowApp from './components/epic_show_app.vue'; export default () => { const el = document.querySelector('#epic-show-app'); - const metaData = JSON.parse(el.dataset.meta); + const metaData = convertObjectPropsToCamelCase(JSON.parse(el.dataset.meta)); const initialData = JSON.parse(el.dataset.initial); const props = Object.assign({}, initialData, metaData, el.dataset); - // Convert backend casing to match frontend style guide - props.startDate = props.start_date; - props.endDate = props.end_date; - return new Vue({ el, components: { diff --git a/ee/app/assets/javascripts/epics/sidebar/components/sidebar_app.vue b/ee/app/assets/javascripts/epics/sidebar/components/sidebar_app.vue index c9659022a7337ac03edcad19d9ca9cb0f0f41ce7..39e0cfbeec8f08afc855df16288bbd2b1ad45881 100644 --- a/ee/app/assets/javascripts/epics/sidebar/components/sidebar_app.vue +++ b/ee/app/assets/javascripts/epics/sidebar/components/sidebar_app.vue @@ -1,210 +1,302 @@ <script> - /* eslint-disable vue/require-default-prop */ - import _ from 'underscore'; - import Cookies from 'js-cookie'; - import Flash from '~/flash'; - import { __ } from '~/locale'; - import { capitalizeFirstCharacter } from '~/lib/utils/text_utility'; - import ListLabel from '~/vue_shared/models/label'; - import SidebarDatePicker from '~/vue_shared/components/sidebar/date_picker.vue'; - import SidebarCollapsedGroupedDatePicker from '~/vue_shared/components/sidebar/collapsed_grouped_date_picker.vue'; - import ToggleSidebar from '~/vue_shared/components/sidebar/toggle_sidebar.vue'; - import SidebarLabelsSelect from '~/vue_shared/components/sidebar/labels_select/base.vue'; - import SidebarParticipants from './sidebar_participants.vue'; - import SidebarSubscriptions from './sidebar_subscriptions.vue'; - import SidebarService from '../services/sidebar_service'; - import Store from '../stores/sidebar_store'; +/* eslint-disable vue/require-default-prop */ +import $ from 'jquery'; +import _ from 'underscore'; +import Cookies from 'js-cookie'; +import Flash from '~/flash'; +import { __ } from '~/locale'; +import { capitalizeFirstCharacter } from '~/lib/utils/text_utility'; +import ListLabel from '~/vue_shared/models/label'; +import SidebarTodo from '~/sidebar/components/todo_toggle/todo.vue'; +import SidebarDatePicker from '~/vue_shared/components/sidebar/date_picker.vue'; +import SidebarCollapsedGroupedDatePicker from '~/vue_shared/components/sidebar/collapsed_grouped_date_picker.vue'; +import ToggleSidebar from '~/vue_shared/components/sidebar/toggle_sidebar.vue'; +import SidebarLabelsSelect from '~/vue_shared/components/sidebar/labels_select/base.vue'; +import SidebarParticipants from './sidebar_participants.vue'; +import SidebarSubscriptions from './sidebar_subscriptions.vue'; +import SidebarService from '../services/sidebar_service'; +import Store from '../stores/sidebar_store'; - export default { - name: 'EpicSidebar', - components: { - ToggleSidebar, - SidebarDatePicker, - SidebarCollapsedGroupedDatePicker, - SidebarLabelsSelect, - SidebarParticipants, - SidebarSubscriptions, +export default { + name: 'EpicSidebar', + components: { + ToggleSidebar, + SidebarTodo, + SidebarDatePicker, + SidebarCollapsedGroupedDatePicker, + SidebarLabelsSelect, + SidebarParticipants, + SidebarSubscriptions, + }, + props: { + epicId: { + type: Number, + required: true, }, - props: { - endpoint: { - type: String, - required: true, - }, - editable: { - type: Boolean, - required: false, - default: false, - }, - initialStartDate: { - type: String, - required: false, - }, - initialEndDate: { - type: String, - required: false, - }, - initialLabels: { - type: Array, - required: true, - }, - initialParticipants: { - type: Array, - required: true, - }, - initialSubscribed: { - type: Boolean, - required: true, - }, - namespace: { - type: String, - required: false, - default: '#', - }, - updatePath: { - type: String, - required: true, - }, - labelsPath: { - type: String, - required: true, - }, - toggleSubscriptionPath: { - type: String, - required: true, - }, - labelsWebUrl: { - type: String, - required: true, - }, - epicsWebUrl: { - type: String, - required: true, - }, + endpoint: { + type: String, + required: true, }, - data() { - const store = new Store({ - startDate: this.initialStartDate, - endDate: this.initialEndDate, - subscribed: this.initialSubscribed, - }); - - return { - store, - // Backend will pass the appropriate css class for the contentContainer - collapsed: Cookies.get('collapsed_gutter') === 'true', - autoExpanded: false, - savingStartDate: false, - savingEndDate: false, - savingSubscription: false, - service: new SidebarService(this.endpoint, this.toggleSubscriptionPath), - epicContext: { - labels: this.initialLabels, - }, - }; + editable: { + type: Boolean, + required: false, + default: false, }, - methods: { - toggleSidebar() { - this.collapsed = !this.collapsed; - - const contentContainer = this.$el.closest('.page-with-contextual-sidebar'); - contentContainer.classList.toggle('right-sidebar-expanded'); - contentContainer.classList.toggle('right-sidebar-collapsed'); + initialStartDate: { + type: String, + required: false, + }, + initialEndDate: { + type: String, + required: false, + }, + initialLabels: { + type: Array, + required: true, + }, + initialParticipants: { + type: Array, + required: true, + }, + initialSubscribed: { + type: Boolean, + required: true, + }, + initialTodoExists: { + type: Boolean, + required: true, + }, + namespace: { + type: String, + required: false, + default: '#', + }, + updatePath: { + type: String, + required: true, + }, + labelsPath: { + type: String, + required: true, + }, + toggleSubscriptionPath: { + type: String, + required: true, + }, + todoPath: { + type: String, + required: true, + }, + todoDeletePath: { + type: String, + required: true, + }, + labelsWebUrl: { + type: String, + required: true, + }, + epicsWebUrl: { + type: String, + required: true, + }, + }, + data() { + const store = new Store({ + startDate: this.initialStartDate, + endDate: this.initialEndDate, + subscribed: this.initialSubscribed, + todoExists: this.initialTodoExists, + todoDeletePath: this.todoDeletePath, + }); - Cookies.set('collapsed_gutter', this.collapsed); - }, - toggleSidebarRevealLabelsDropdown() { - const contentContainer = this.$el.closest('.page-with-contextual-sidebar'); - this.toggleSidebar(); - // When sidebar is expanded, we need to wait - // for rendering to finish before opening - // dropdown as otherwise it causes `calc()` - // used in CSS to miscalculate collapsed - // sidebar size. - _.debounce(() => { - this.autoExpanded = true; - contentContainer - .querySelector('.js-sidebar-dropdown-toggle') - .dispatchEvent(new Event('click', { bubbles: true, cancelable: false })); - }, 100)(); + return { + store, + // Backend will pass the appropriate css class for the contentContainer + collapsed: Cookies.get('collapsed_gutter') === 'true', + isUserSignedIn: !!gon.current_user_id, + autoExpanded: false, + savingStartDate: false, + savingEndDate: false, + savingSubscription: false, + savingTodoAction: false, + service: new SidebarService({ + endpoint: this.endpoint, + subscriptionEndpoint: this.subscriptionEndpoint, + todoPath: this.todoPath, + }), + epicContext: { + labels: this.initialLabels, }, - saveDate(dateType = 'start', newDate) { - const type = dateType === 'start' ? dateType : 'end'; - const capitalizedType = capitalizeFirstCharacter(type); - const serviceMethod = `update${capitalizedType}Date`; - const savingBoolean = `saving${capitalizedType}Date`; + }; + }, + methods: { + toggleSidebar() { + this.collapsed = !this.collapsed; - this[savingBoolean] = true; + const contentContainer = this.$el.closest('.page-with-contextual-sidebar'); + contentContainer.classList.toggle('right-sidebar-expanded'); + contentContainer.classList.toggle('right-sidebar-collapsed'); - return this.service[serviceMethod](newDate) - .then(() => { - this[savingBoolean] = false; - this.store[`${type}Date`] = newDate; - }) - .catch(() => { - this[savingBoolean] = false; - Flash(`An error occurred while saving ${type} date`); - }); - }, - saveStartDate(date) { - return this.saveDate('start', date); - }, - saveEndDate(date) { - return this.saveDate('end', date); - }, - handleLabelClick(label) { - if (label.isAny) { - this.epicContext.labels = []; - } else { - const labelIndex = this.epicContext.labels.findIndex(l => l.id === label.id); + Cookies.set('collapsed_gutter', this.collapsed); + }, + toggleSidebarRevealLabelsDropdown() { + const contentContainer = this.$el.closest('.page-with-contextual-sidebar'); + this.toggleSidebar(); + // When sidebar is expanded, we need to wait + // for rendering to finish before opening + // dropdown as otherwise it causes `calc()` + // used in CSS to miscalculate collapsed + // sidebar size. + _.debounce(() => { + this.autoExpanded = true; + contentContainer + .querySelector('.js-sidebar-dropdown-toggle') + .dispatchEvent(new Event('click', { bubbles: true, cancelable: false })); + }, 100)(); + }, + saveDate(dateType = 'start', newDate) { + const type = dateType === 'start' ? dateType : 'end'; + const capitalizedType = capitalizeFirstCharacter(type); + const serviceMethod = `update${capitalizedType}Date`; + const savingBoolean = `saving${capitalizedType}Date`; + + this[savingBoolean] = true; - if (labelIndex === -1) { - this.epicContext.labels.push( - new ListLabel({ - id: label.id, - title: label.title, - color: label.color[0], - textColor: label.text_color, - }), - ); + return this.service[serviceMethod](newDate) + .then(() => { + this[savingBoolean] = false; + this.store[`${type}Date`] = newDate; + }) + .catch(() => { + this[savingBoolean] = false; + Flash(`An error occurred while saving ${type} date`); + }); + }, + saveStartDate(date) { + return this.saveDate('start', date); + }, + saveEndDate(date) { + return this.saveDate('end', date); + }, + saveTodoState({ count, deletePath }) { + this.savingTodoAction = false; + this.store.setTodoExists(!this.store.todoExists); + if (deletePath) { + this.store.setTodoDeletePath(deletePath); + } + $(document).trigger('todo:toggle', count); + }, + handleLabelClick(label) { + if (label.isAny) { + this.epicContext.labels = []; + } else { + const labelIndex = this.epicContext.labels.findIndex(l => l.id === label.id); + + if (labelIndex === -1) { + this.epicContext.labels.push( + new ListLabel({ + id: label.id, + title: label.title, + color: label.color[0], + textColor: label.text_color, + }), + ); + } else { + this.epicContext.labels.splice(labelIndex, 1); + } + } + }, + handleDropdownClose() { + if (this.autoExpanded) { + this.autoExpanded = false; + this.toggleSidebar(); + } + }, + handleToggleSubscribed() { + this.service + .toggleSubscribed() + .then(() => { + this.store.setSubscribed(!this.store.subscribed); + }) + .catch(() => { + if (this.store.subscribed) { + Flash(__('An error occurred while unsubscribing to notifications.')); } else { - this.epicContext.labels.splice(labelIndex, 1); + Flash(__('An error occurred while subscribing to notifications.')); } - } - }, - handleDropdownClose() { - if (this.autoExpanded) { - this.autoExpanded = false; - this.toggleSidebar(); - } - }, - handleToggleSubscribed() { - this.service.toggleSubscribed() - .then(() => { - this.store.setSubscribed(!this.store.subscribed); + }); + }, + handleToggleTodo() { + this.savingTodoAction = true; + if (!this.store.todoExists) { + this.service + .addTodo(this.epicId) + .then(({ data }) => { + this.saveTodoState({ + count: data.count, + deletePath: data.delete_path, + }); }) .catch(() => { - if (this.store.subscribed) { - Flash(__('An error occurred while unsubscribing to notifications.')); - } else { - Flash(__('An error occurred while subscribing to notifications.')); - } + this.savingTodoAction = false; + Flash(__('There was an error adding a todo.')); }); - }, + } else { + this.service + .deleteTodo(this.store.todoDeletePath) + .then(({ data }) => { + this.saveTodoState({ + count: data.count, + }); + }) + .catch(() => { + this.savingTodoAction = false; + Flash(__('There was an error deleting the todo.')); + }); + } }, - }; + }, +}; </script> <template> <aside :class="{ 'right-sidebar-expanded' : !collapsed, 'right-sidebar-collapsed': collapsed }" + v-bind="isUserSignedIn ? { 'data-signed-in': true } : {}" class="right-sidebar" > <div class="issuable-sidebar js-issuable-update"> <div class="block issuable-sidebar-header"> + <span class="issuable-header-text hide-collapsed float-left"> + {{ __('Todo') }} + </span> <toggle-sidebar :collapsed="collapsed" + css-classes="float-right" @toggle="toggleSidebar" /> + <sidebar-todo + v-if="!collapsed" + :collapsed="collapsed" + :issuable-id="epicId" + :is-todo="store.todoExists" + :is-action-active="savingTodoAction" + issuable-type="epic" + @toggleTodo="handleToggleTodo" + /> + </div> + <div + v-if="collapsed" + class="block todo" + > + <sidebar-todo + :collapsed="collapsed" + :issuable-id="epicId" + :is-todo="store.todoExists" + :is-action-active="savingTodoAction" + issuable-type="epic" + @toggleTodo="handleToggleTodo" + /> </div> <sidebar-date-picker v-if="!collapsed" @@ -213,7 +305,7 @@ :editable="editable" :selected-date="store.startDateTime" :max-date="store.endDateTime" - :show-toggle-sidebar="true" + :show-toggle-sidebar="!isUserSignedIn" block-class="start-date" label="Planned start date" @saveDate="saveStartDate" diff --git a/ee/app/assets/javascripts/epics/sidebar/services/sidebar_service.js b/ee/app/assets/javascripts/epics/sidebar/services/sidebar_service.js index 4afa3a746b22b7a1d9bf98b11ae78a2c19e4bae0..589743b77176220bb9308c818e18cecb2f32cd80 100644 --- a/ee/app/assets/javascripts/epics/sidebar/services/sidebar_service.js +++ b/ee/app/assets/javascripts/epics/sidebar/services/sidebar_service.js @@ -1,9 +1,10 @@ import axios from '~/lib/utils/axios_utils'; export default class SidebarService { - constructor(endpoint, subscriptionEndpoint) { + constructor({ endpoint, subscriptionEndpoint, todoPath }) { this.endpoint = endpoint; this.subscriptionEndpoint = subscriptionEndpoint; + this.todoPath = todoPath; } updateStartDate(startDate) { @@ -17,4 +18,16 @@ export default class SidebarService { toggleSubscribed() { return axios.post(this.subscriptionEndpoint); } + + addTodo(epicId) { + return axios.post(this.todoPath, { + issuable_id: epicId, + issuable_type: 'epic', + }); + } + + // eslint-disable-next-line class-methods-use-this + deleteTodo(todoDeletePath) { + return axios.delete(todoDeletePath); + } } diff --git a/ee/app/assets/javascripts/epics/sidebar/stores/sidebar_store.js b/ee/app/assets/javascripts/epics/sidebar/stores/sidebar_store.js index bc72a10979469b063cc10f6c053254fd3bb5689a..728b0ed5da9e76080435574b8421ac6d202b8bee 100644 --- a/ee/app/assets/javascripts/epics/sidebar/stores/sidebar_store.js +++ b/ee/app/assets/javascripts/epics/sidebar/stores/sidebar_store.js @@ -1,10 +1,12 @@ import { parsePikadayDate } from '~/lib/utils/datefix'; export default class SidebarStore { - constructor({ startDate, endDate, subscribed }) { + constructor({ startDate, endDate, subscribed, todoExists, todoDeletePath }) { this.startDate = startDate; this.endDate = endDate; this.subscribed = subscribed; + this.todoExists = todoExists; + this.todoDeletePath = todoDeletePath; } get startDateTime() { @@ -18,4 +20,12 @@ export default class SidebarStore { setSubscribed(subscribed) { this.subscribed = subscribed; } + + setTodoExists(todoExists) { + this.todoExists = todoExists; + } + + setTodoDeletePath(deletePath) { + this.todoDeletePath = deletePath; + } } diff --git a/ee/app/controllers/groups/todos_controller.rb b/ee/app/controllers/groups/todos_controller.rb new file mode 100644 index 0000000000000000000000000000000000000000..3ed3e99ee05f4bb2febb34676b77454e77e3e721 --- /dev/null +++ b/ee/app/controllers/groups/todos_controller.rb @@ -0,0 +1,16 @@ +class Groups::TodosController < Groups::ApplicationController + include Gitlab::Utils::StrongMemoize + include TodosActions + + before_action :authenticate_user!, only: [:create] + + private + + def issuable + strong_memoize(:epic) do + next if params[:issuable_type] != 'epic' + + @group.epics.find_by(id: params[:issuable_id]) + end + end +end diff --git a/ee/app/helpers/ee/todos_helper.rb b/ee/app/helpers/ee/todos_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..0a4196013cdcd38c9b56dc1d404faa03af6a8a0b --- /dev/null +++ b/ee/app/helpers/ee/todos_helper.rb @@ -0,0 +1,10 @@ +module EE + module TodosHelper + extend ::Gitlab::Utils::Override + + override :todo_types_options + def todo_types_options + super << { id: 'Epic', text: 'Epic' } + end + end +end diff --git a/ee/app/helpers/epics_helper.rb b/ee/app/helpers/epics_helper.rb index b41b8c6ed5492690ee49859f146a32164d934490..46992fc18b17926feda2fb9789829b2348993d4a 100644 --- a/ee/app/helpers/epics_helper.rb +++ b/ee/app/helpers/epics_helper.rb @@ -2,8 +2,10 @@ module EpicsHelper def epic_show_app_data(epic, opts) author = epic.author group = epic.group + todo = issuable_todo(epic) epic_meta = { + epic_id: epic.id, created: epic.created_at, author: { name: author.name, @@ -11,10 +13,14 @@ module EpicsHelper username: "@#{author.username}", src: opts[:author_icon] }, + todo_exists: todo.present?, + todo_path: group_todos_path(group), start_date: epic.start_date, end_date: epic.end_date } + epic_meta[:todo_delete_path] = dashboard_todo_path(todo) if todo.present? + participants = UserSerializer.new.represent(epic.participants) initial = opts[:initial].merge(labels: epic.labels, participants: participants, diff --git a/ee/app/models/ee/note.rb b/ee/app/models/ee/note.rb index 2c9ad851822b3abe5909911058227601b6e3b551..903e3927440615040b54343b934ed949ffb76150 100644 --- a/ee/app/models/ee/note.rb +++ b/ee/app/models/ee/note.rb @@ -16,11 +16,6 @@ module EE !for_epic? && super end - override :can_create_todo? - def can_create_todo? - !for_epic? && super - end - override :etag_key def etag_key if for_epic? @@ -44,6 +39,11 @@ module EE super.merge(banzai_context_params) end + override :for_issuable? + def for_issuable? + for_epic? || super + end + private def banzai_context_params diff --git a/ee/app/models/ee/todo.rb b/ee/app/models/ee/todo.rb new file mode 100644 index 0000000000000000000000000000000000000000..236fab5d2442482a2cbba9b19c67fd3e1975b2f4 --- /dev/null +++ b/ee/app/models/ee/todo.rb @@ -0,0 +1,10 @@ +module EE + module Todo + extend ::Gitlab::Utils::Override + + override :parent + def parent + project || group + end + end +end diff --git a/ee/app/services/ee/todo_service.rb b/ee/app/services/ee/todo_service.rb index 27e5330a9e3690097bb81f2c334d934444e7e9fd..16c4aaddc861438d24690fc382d6b8ea9877092b 100644 --- a/ee/app/services/ee/todo_service.rb +++ b/ee/app/services/ee/todo_service.rb @@ -19,8 +19,27 @@ module EE super end + def new_epic(epic, current_user) + create_mention_todos(nil, epic, current_user) + end + + def update_epic(epic, current_user, skip_users = []) + create_mention_todos(nil, epic, current_user, nil, skip_users) + end + private + override :attributes_for_target + def attributes_for_target(target) + attributes = super + + if target.is_a?(Epic) + attributes[:group_id] = target.group_id + end + + attributes + end + def create_approval_required_todos(merge_request, approvers, author) attributes = attributes_for_todo(merge_request.project, merge_request, author, ::Todo::APPROVAL_REQUIRED) create_todos(approvers.map(&:user), attributes) diff --git a/ee/app/services/epics/update_service.rb b/ee/app/services/epics/update_service.rb index e61578b69d0a8d7f0b2738d952091571645ee2e4..6519da04febadb04ca1adb555079a01e43db4e1b 100644 --- a/ee/app/services/epics/update_service.rb +++ b/ee/app/services/epics/update_service.rb @@ -2,6 +2,20 @@ module Epics class UpdateService < Epics::BaseService def execute(epic) update(epic) + + epic + end + + def handle_changes(epic, options) + old_associations = options.fetch(:old_associations, {}) + old_mentioned_users = old_associations.fetch(:mentioned_users, []) + old_labels = old_associations.fetch(:labels, []) + + if has_changes?(epic, old_labels: old_labels) + todo_service.mark_pending_todos_as_done(epic, current_user) + end + + todo_service.update_epic(epic, current_user, old_mentioned_users) end end end diff --git a/ee/changelogs/unreleased/5481-epic-todos.yml b/ee/changelogs/unreleased/5481-epic-todos.yml new file mode 100644 index 0000000000000000000000000000000000000000..ab393796c8ef76f5edda0393b7702cd0868eb0ad --- /dev/null +++ b/ee/changelogs/unreleased/5481-epic-todos.yml @@ -0,0 +1,5 @@ +--- +title: Add support for todos on epics +merge_request: 6142 +author: +type: added diff --git a/ee/lib/ee/api/entities.rb b/ee/lib/ee/api/entities.rb index 837c5ec1143e69d27db74d247428a4620a82bd21..c387bca3a9bd4f397f3f5c4b6ea936eccbc670fe 100644 --- a/ee/lib/ee/api/entities.rb +++ b/ee/lib/ee/api/entities.rb @@ -126,6 +126,16 @@ module EE end end + module Todo + extend ActiveSupport::Concern + + def todo_target_class(target_type) + ::EE::API::Entities.const_get(target_type, false) + rescue NameError + super + end + end + ######################## # EE-specific entities # ######################## diff --git a/ee/lib/ee/api/todos.rb b/ee/lib/ee/api/todos.rb new file mode 100644 index 0000000000000000000000000000000000000000..59ace24fe173d9600c248f182faebb61aab4bdb2 --- /dev/null +++ b/ee/lib/ee/api/todos.rb @@ -0,0 +1,38 @@ +module EE + module API + module Todos + extend ActiveSupport::Concern + + prepended do + helpers do + def epic + @epic ||= user_group.epics.find_by(iid: params[:epic_iid]) + end + + def authorize_can_read! + authorize!(:read_epic, epic) + end + end + + resource :groups, requirements: ::API::API::PROJECT_ENDPOINT_REQUIREMENTS do + desc 'Create a todo on an epic' do + success ::API::Entities::Todo + end + params do + requires :epic_iid, type: Integer, desc: 'The IID of an epic' + end + post ":id/epics/:epic_iid/todo" do + authorize_can_read! + todo = ::TodoService.new.mark_todo(epic, current_user).first + + if todo + present todo, with: ::API::Entities::Todo, current_user: current_user + else + not_modified! + end + end + end + end + end + end +end diff --git a/ee/spec/controllers/groups/todos_controller_spec.rb b/ee/spec/controllers/groups/todos_controller_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5e6a34fc747a874047f01cb17b87e5ab08afbe80 --- /dev/null +++ b/ee/spec/controllers/groups/todos_controller_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Groups::TodosController do + let(:user) { create(:user) } + let(:group) { create(:group, :private) } + let(:epic) { create(:epic, group: group) } + let(:parent) { group } + + describe 'POST create' do + def post_create + post :create, + group_id: group, + issuable_id: epic.id, + issuable_type: 'epic', + format: :json + end + + it_behaves_like 'todos actions' + end +end diff --git a/ee/spec/features/epics/todo_spec.rb b/ee/spec/features/epics/todo_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9ff78f8d32084f104fe9c6a21b106401dce75b78 --- /dev/null +++ b/ee/spec/features/epics/todo_spec.rb @@ -0,0 +1,43 @@ +require 'rails_helper' + +describe 'Manually create a todo item from epic', :js do + let(:group) { create(:group) } + let(:epic) { create(:epic, group: group) } + let(:user) { create(:user)} + + before do + stub_licensed_features(epics: true) + + sign_in(user) + visit group_epic_path(group, epic) + end + + it 'creates todo when clicking button' do + page.within '.issuable-sidebar' do + click_button 'Add todo' + + expect(page).to have_content 'Mark todo as done' + end + + page.within '.header-content .todos-count' do + expect(page).to have_content '1' + end + end + + it 'marks a todo as done' do + page.within '.issuable-sidebar' do + click_button 'Add todo' + end + + expect(page).to have_selector('.todos-count', visible: true) + page.within '.header-content .todos-count' do + expect(page).to have_content '1' + end + + page.within '.issuable-sidebar' do + click_button 'Mark todo as done' + end + + expect(page).to have_selector('.todos-count', visible: false) + end +end diff --git a/ee/spec/features/epics/update_epic_spec.rb b/ee/spec/features/epics/update_epic_spec.rb index cf2034a7d11a710e08402e1ac7283c0f2f854e5f..6da54899c4d5a978183f1007610421b23f2b6920 100644 --- a/ee/spec/features/epics/update_epic_spec.rb +++ b/ee/spec/features/epics/update_epic_spec.rb @@ -57,6 +57,32 @@ describe 'Update Epic', :js do expect(find('.issuable-details .description')).to have_content('New epic description') end + it 'creates a todo only for mentioned users' do + mentioned = create(:user) + + fill_in 'issue-description', with: "FYI #{mentioned.to_reference}" + + click_button 'Save changes' + + expect(find('.issuable-details h2.title')).to have_content('title') + + visit dashboard_todos_path + + expect(page).to have_selector('.todos-list .todo', count: 0) + + sign_in(mentioned) + + visit dashboard_todos_path + + page.within '.header-content .todos-count' do + expect(page).to have_content '1' + end + expect(page).to have_selector('.todos-list .todo', count: 1) + within first('.todo') do + expect(page).to have_content "epic #{epic.to_reference(full: true)}" + end + end + it 'edits full screen' do page.within('.detail-page-description') { find('.js-zen-enter').click } diff --git a/ee/spec/helpers/epics_helper_spec.rb b/ee/spec/helpers/epics_helper_spec.rb index f87e0f3484564d17c9345e75636b5ce32c4bc147..45c57dcc91cc62d10faf5521daf159e357052c3d 100644 --- a/ee/spec/helpers/epics_helper_spec.rb +++ b/ee/spec/helpers/epics_helper_spec.rb @@ -15,7 +15,7 @@ describe EpicsHelper do expected_keys = %i(initial meta namespace labels_path toggle_subscription_path labels_web_url epics_web_url) expect(data.keys).to match_array(expected_keys) - expect(meta_data.keys).to match_array(%w[created author start_date end_date]) + expect(meta_data.keys).to match_array(%w[created author start_date end_date epic_id todo_exists todo_path]) expect(meta_data['author']).to eq({ 'name' => user.name, 'url' => "/#{user.username}", diff --git a/ee/spec/javascripts/epics/epic_show/components/epic_show_app_spec.js b/ee/spec/javascripts/epics/epic_show/components/epic_show_app_spec.js index 43efbb17934896cf67d873583db737f598e61343..62d9868d496e90455b8c9290e7fbd4553e973e25 100644 --- a/ee/spec/javascripts/epics/epic_show/components/epic_show_app_spec.js +++ b/ee/spec/javascripts/epics/epic_show/components/epic_show_app_spec.js @@ -23,6 +23,7 @@ describe('EpicShowApp', () => { mock.onAny().reply(404, null); const { + epicId, canUpdate, canDestroy, endpoint, @@ -43,6 +44,9 @@ describe('EpicShowApp', () => { participants, subscribed, toggleSubscriptionPath, + todoExists, + todoPath, + todoDeletePath, } = props; const EpicShowApp = Vue.extend(epicShowApp); @@ -76,6 +80,7 @@ describe('EpicShowApp', () => { const EpicSidebar = Vue.extend(epicSidebar); sidebarVm = mountComponent(EpicSidebar, { + epicId, endpoint, editable: canUpdate, initialStartDate: startDate, @@ -83,12 +88,15 @@ describe('EpicShowApp', () => { initialLabels: labels, initialParticipants: participants, initialSubscribed: subscribed, + initialTodoExists: todoExists, updatePath: updateEndpoint, toggleSubscriptionPath, labelsPath, labelsWebUrl, epicsWebUrl, namespace, + todoPath, + todoDeletePath, }); setTimeout(done); diff --git a/ee/spec/javascripts/epics/epic_show/mock_data.js b/ee/spec/javascripts/epics/epic_show/mock_data.js index 930b6ce097315afda59b29e3a6cb7cd873192129..314a793fae0534143cb41d303c13a5592051e3d0 100644 --- a/ee/spec/javascripts/epics/epic_show/mock_data.js +++ b/ee/spec/javascripts/epics/epic_show/mock_data.js @@ -28,9 +28,12 @@ export const mockParticipants = [ ]; export const contentProps = { + epicId: 1, endpoint: '', toggleSubscriptionPath: gl.TEST_HOST, updateEndpoint: gl.TEST_HOST, + todoPath: gl.TEST_HOST, + todoDeletePath: gl.TEST_HOST, canAdmin: true, canUpdate: true, canDestroy: true, @@ -49,6 +52,7 @@ export const contentProps = { labels: mockLabels, participants: mockParticipants, subscribed: true, + todoExists: false, }; export const headerProps = { diff --git a/ee/spec/javascripts/epics/sidebar/components/sidebar_app_spec.js b/ee/spec/javascripts/epics/sidebar/components/sidebar_app_spec.js index 3b1ed9eb16ce1691c3dc09eed75bce3008be41f6..1c7c4bbde4e4b254f639e75205874a1b4bfae420 100644 --- a/ee/spec/javascripts/epics/sidebar/components/sidebar_app_spec.js +++ b/ee/spec/javascripts/epics/sidebar/components/sidebar_app_spec.js @@ -13,6 +13,7 @@ describe('epicSidebar', () => { let originalCookieState; let EpicSidebar; const { + epicId, updateEndpoint, labelsPath, labelsWebUrl, @@ -21,18 +22,25 @@ describe('epicSidebar', () => { participants, subscribed, toggleSubscriptionPath, + todoExists, + todoPath, + todoDeletePath, } = props; const defaultPropsData = { + epicId, endpoint: gl.TEST_HOST, initialLabels: labels, initialParticipants: participants, initialSubscribed: subscribed, + initialTodoExists: todoExists, updatePath: updateEndpoint, toggleSubscriptionPath, labelsPath, labelsWebUrl, epicsWebUrl, + todoPath, + todoDeletePath, }; beforeEach(() => { @@ -91,7 +99,7 @@ describe('epicSidebar', () => { }); it('should render collapsed grouped date picker', () => { - expect(vm.$el.querySelector('.sidebar-collapsed-icon span').innerText.trim()).toEqual('From Jan 1 2017'); + expect(vm.$el.querySelector('.sidebar-grouped-item .sidebar-collapsed-icon span').innerText.trim()).toEqual('From Jan 1 2017'); }); it('should render collapsed labels picker', () => { @@ -212,6 +220,96 @@ describe('epicSidebar', () => { }); }); + describe('handleToggleTodo', () => { + let mock; + + beforeEach(() => { + mock = new MockAdapter(axios); + setFixtures('<div class="flash-container"></div>'); + }); + + afterEach(() => { + document.querySelector('.flash-container').remove(); + mock.restore(); + }); + + it('calls `addTodo` on service object when `todoExists` prop is `false`', () => { + spyOn(vm.service, 'addTodo').and.callThrough(); + vm.store.setTodoExists(false); + expect(vm.savingTodoAction).toBe(false); + vm.handleToggleTodo(); + expect(vm.savingTodoAction).toBe(true); + expect(vm.service.addTodo).toHaveBeenCalledWith(epicId); + }); + + it('calls `addTodo` on service and sets response on store when request is successful', done => { + mock.onPost(gl.TEST_HOST).reply(200, { + delete_path: '/foo/bar', + count: 1, + }); + spyOn(vm.service, 'addTodo').and.callThrough(); + vm.store.setTodoExists(false); + + vm.handleToggleTodo(); + setTimeout(() => { + expect(vm.savingTodoAction).toBe(false); + expect(vm.store.todoDeletePath).toBe('/foo/bar'); + expect(vm.store.todoExists).toBe(true); + done(); + }, 0); + }); + + it('calls `addTodo` on service and shows Flash error when request is unsuccessful', done => { + mock.onPost(gl.TEST_HOST).reply(500, {}); + spyOn(vm.service, 'addTodo').and.callThrough(); + vm.store.setTodoExists(false); + + vm.handleToggleTodo(); + setTimeout(() => { + expect(vm.savingTodoAction).toBe(false); + expect(document.querySelector('.flash-text').innerText.trim()).toBe('There was an error adding a todo.'); + done(); + }, 0); + }); + + it('calls `deleteTodo` on service object when `todoExists` prop is `true`', () => { + spyOn(vm.service, 'deleteTodo').and.callThrough(); + vm.store.setTodoExists(true); + expect(vm.savingTodoAction).toBe(false); + vm.handleToggleTodo(); + expect(vm.savingTodoAction).toBe(true); + expect(vm.service.deleteTodo).toHaveBeenCalledWith(gl.TEST_HOST); + }); + + it('calls `deleteTodo` on service and sets response on store when request is successful', done => { + mock.onDelete(gl.TEST_HOST).reply(200, { + count: 1, + }); + spyOn(vm.service, 'deleteTodo').and.callThrough(); + vm.store.setTodoExists(true); + + vm.handleToggleTodo(); + setTimeout(() => { + expect(vm.savingTodoAction).toBe(false); + expect(vm.store.todoExists).toBe(false); + done(); + }, 0); + }); + + it('calls `deleteTodo` on service and shows Flash error when request is unsuccessful', done => { + mock.onDelete(gl.TEST_HOST).reply(500, {}); + spyOn(vm.service, 'deleteTodo').and.callThrough(); + vm.store.setTodoExists(true); + + vm.handleToggleTodo(); + setTimeout(() => { + expect(vm.savingTodoAction).toBe(false); + expect(document.querySelector('.flash-text').innerText.trim()).toBe('There was an error deleting the todo.'); + done(); + }, 0); + }); + }); + describe('saveDate error', () => { let interceptor; let component; diff --git a/ee/spec/javascripts/epics/sidebar/stores/sidebar_store_spec.js b/ee/spec/javascripts/epics/sidebar/stores/sidebar_store_spec.js index f66c945a503e3b5c1777b9437476c313a7fbc68d..f2e0626255c3f6d4f5c2098ef67a6784c7e4383d 100644 --- a/ee/spec/javascripts/epics/sidebar/stores/sidebar_store_spec.js +++ b/ee/spec/javascripts/epics/sidebar/stores/sidebar_store_spec.js @@ -63,4 +63,22 @@ describe('Sidebar Store', () => { expect(store.subscribed).toEqual(false); }); }); + + describe('setTodoExists', () => { + it('should set store.subscribed value', () => { + const store = new SidebarStore({ todoExists: true }); + + store.setTodoExists(false); + expect(store.todoExists).toEqual(false); + }); + }); + + describe('setTodoDeletePath', () => { + it('should set store.subscribed value', () => { + const store = new SidebarStore({ todoDeletePath: gl.TEST_HOST }); + + store.setTodoDeletePath('/foo/bar'); + expect(store.todoDeletePath).toEqual('/foo/bar'); + }); + }); }); diff --git a/ee/spec/requests/api/todos_spec.rb b/ee/spec/requests/api/todos_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..4982bfb5387d2556f8821e8fc18fb0c2f337a49b --- /dev/null +++ b/ee/spec/requests/api/todos_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper' + +describe API::Todos do + let(:group) { create(:group) } + let(:user) { create(:user) } + let(:epic) { create(:epic, group: group) } + + subject { post api("/groups/#{group.id}/epics/#{epic.iid}/todo", user) } + + describe 'POST :id/epics/:epic_iid/todo' do + context 'when epics feature is disabled' do + it 'returns 403 forbidden error' do + subject + + expect(response).to have_gitlab_http_status(403) + end + end + + context 'when epics feature is enabled' do + before do + stub_licensed_features(epics: true) + end + + it 'creates a todo on an epic' do + expect { subject }.to change { Todo.count }.by(1) + + expect(response.status).to eq(201) + expect(json_response['project']).to be_nil + expect(json_response['group']).to be_a(Hash) + expect(json_response['author']).to be_a(Hash) + expect(json_response['target_type']).to eq('Epic') + expect(json_response['target']).to be_a(Hash) + expect(json_response['target_url']).to be_present + expect(json_response['body']).to be_present + expect(json_response['state']).to eq('pending') + expect(json_response['action_name']).to eq('marked') + expect(json_response['created_at']).to be_present + end + + it 'returns 304 there already exist a todo on that epic' do + create(:todo, project: nil, group: group, user: user, target: epic) + + subject + + expect(response.status).to eq(304) + end + + it 'returns 404 if the epic is not found' do + post api("/groups/#{group.id}/epics/9999/todo", user) + + expect(response.status).to eq(403) + end + + it 'returns an error if the epic is not accessible' do + group.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + subject + + expect(response).to have_gitlab_http_status(404) + end + end + end +end diff --git a/ee/spec/services/epics/update_service_spec.rb b/ee/spec/services/epics/update_service_spec.rb index 2909d1e4914bb108fb15ff5f7a6995f90ff111f7..58a9525d502e93c5321f31033fe82d5c51701405 100644 --- a/ee/spec/services/epics/update_service_spec.rb +++ b/ee/spec/services/epics/update_service_spec.rb @@ -6,6 +6,9 @@ describe Epics::UpdateService do let(:epic) { create(:epic, group: group) } describe '#execute' do + before do + stub_licensed_features(epics: true) + end def update_epic(opts) described_class.new(group, user, opts).execute(epic) end @@ -56,5 +59,61 @@ describe Epics::UpdateService do expect(note.noteable).to eq(epic) end end + + context 'todos' do + before do + group.update(visibility: Gitlab::VisibilityLevel::PUBLIC) + end + + context 'creating todos' do + let(:mentioned1) { create(:user) } + let(:mentioned2) { create(:user) } + + before do + epic.update(description: "FYI: #{mentioned1.to_reference}") + end + + it 'creates todos for only newly mentioned users' do + expect do + update_epic(description: "FYI: #{mentioned1.to_reference} #{mentioned2.to_reference}") + end.to change { Todo.count }.by(1) + end + end + + context 'adding a label' do + let(:label) { create(:group_label, group: group) } + let(:user2) { create(:user) } + let!(:todo1) do + create(:todo, :mentioned, :pending, + target: epic, + group: group, + project: nil, + author: user, + user: user) + end + let!(:todo2) do + create(:todo, :mentioned, :pending, + target: epic, + group: group, + project: nil, + author: user2, + user: user2) + end + + before do + group.add_developer(user) + + update_epic(label_ids: [label.id]) + end + + it 'marks todo as done for a user who added a label' do + expect(todo1.reload.state).to eq('done') + end + + it 'does not mark todos as done for other users' do + expect(todo2.reload.state).to eq('pending') + end + end + end end end diff --git a/ee/spec/services/todo_service_spec.rb b/ee/spec/services/todo_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3876d72fecc3c0292be73b7e62f799a03d1d9d35 --- /dev/null +++ b/ee/spec/services/todo_service_spec.rb @@ -0,0 +1,248 @@ +require 'spec_helper' + +describe TodoService do + describe 'Epics' do + let(:author) { create(:user, username: 'author') } + let(:non_member) { create(:user, username: 'non_member') } + let(:member) { create(:user, username: 'member') } + let(:guest) { create(:user, username: 'guest') } + let(:admin) { create(:admin, username: 'administrator') } + let(:john_doe) { create(:user, username: 'john_doe') } + let(:skipped) { create(:user, username: 'skipped') } + let(:skip_users) { [skipped] } + let(:users) { [author, non_member, member, guest, admin, john_doe, skipped] } + let(:mentions) { users.map(&:to_reference).join(' ') } + let(:combined_mentions) { member.to_reference + ", what do you think? cc: " + [guest, admin, skipped].map(&:to_reference).join(' ') } + + let(:description_mentions) { "- [ ] Task 1\n- [ ] Task 2 FYI: #{mentions}" } + let(:description_directly_addressed) { "#{mentions}\n- [ ] Task 1\n- [ ] Task 2" } + + let(:group) { create(:group) } + let(:epic) { create(:epic, group: group, author: author, description: description_mentions) } + + let(:service) { described_class.new } + + let(:todos_for) { [] } + let(:todos_not_for) { [] } + let(:target) { epic } + + before do + stub_licensed_features(epics: true) + + group.add_guest(guest) + group.add_developer(author) + group.add_developer(member) + end + + shared_examples_for 'todos creation' do + it 'creates todos for users mentioned' do + if todos_for.count > 0 + params = todo_params + .merge(user: todos_for) + .reverse_merge(target: target, project: nil, group: group, author: author, state: :pending) + + expect { execute } + .to change { Todo.where(params).count }.from(0).to(todos_for.count) + end + end + + it 'does not create todos for users not mentioned or without permissions' do + execute + + params = todo_params + .reverse_merge(target: target, project: nil, group: group, author: author, state: :pending) + + todos_not_for.each_with_index do |user, index| + expect(Todo.where(params.merge(user: user)).count) + .to eq(0), "expected not to create a todo for user '#{user.username}''" + end + end + end + + context 'Epics' do + describe '#new_epic' do + let(:execute) { service.new_epic(epic, author) } + + context 'when an epic belongs to a public group' do + context 'for mentioned users' do + let(:todo_params) { { action: Todo::MENTIONED } } + let(:todos_for) { users } + + include_examples 'todos creation' + end + + context 'for directly addressed users' do + before do + epic.update(description: description_directly_addressed) + end + + let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } } + let(:todos_for) { users } + + include_examples 'todos creation' + end + + context 'combined' do + before do + epic.update(description: combined_mentions) + end + + context 'mentioned users' do + let(:todo_params) { { action: Todo::MENTIONED } } + let(:todos_for) { [guest, admin, skipped] } + + include_examples 'todos creation' + end + + context 'directly addressed users' do + let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } } + let(:todos_for) { [member] } + + include_examples 'todos creation' + end + end + end + + context 'when an epic belongs to a private group' do + before do + group.update(visibility: Gitlab::VisibilityLevel::PRIVATE) + end + + context 'for mentioned users' do + let(:todo_params) { { action: Todo::MENTIONED } } + let(:todos_for) { [member, author, guest, admin] } + let(:todos_not_for) { [non_member, john_doe, skipped] } + + include_examples 'todos creation' + end + + context 'for directly addressed users' do + before do + epic.update!(description: description_directly_addressed) + end + + let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } } + let(:todos_for) { [member, author, guest, admin] } + let(:todos_not_for) { [non_member, john_doe, skipped] } + + include_examples 'todos creation' + end + end + + context 'creates todos for group members when a group is mentioned' do + before do + epic.update(description: group.to_reference) + end + + let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } } + let(:todos_for) { [member, guest, author] } + let(:todos_not_for) { [non_member, admin, john_doe] } + + include_examples 'todos creation' + end + end + + describe '#update_epic' do + let(:execute) { service.update_epic(epic, author, skip_users) } + + context 'for mentioned users' do + let(:todo_params) { { action: Todo::MENTIONED } } + let(:todos_for) { [author, non_member, member, guest, admin, john_doe] } + let(:todos_not_for) { [skipped] } + + include_examples 'todos creation' + end + + context 'for directly addressed users' do + before do + epic.update(description: description_directly_addressed) + end + + let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } } + let(:todos_for) { [author, non_member, member, guest, admin, john_doe] } + let(:todos_not_for) { [skipped] } + + include_examples 'todos creation' + end + end + + describe '#new_note' do + let!(:first_todo) do + create(:todo, :assigned, + user: john_doe, project: nil, group: group, target: epic, author: author) + end + let!(:second_todo) do + create(:todo, :assigned, + user: john_doe, project: nil, group: group, target: epic, author: author) + end + let(:note) { create(:note, noteable: epic, project: nil, author: john_doe, note: mentions) } + + context 'when a note is created for an epic' do + it 'marks pending epic todos for the note author as done' do + service.new_note(note, john_doe) + + expect(first_todo.reload).to be_done + expect(second_todo.reload).to be_done + end + + it 'does not marka pending epic todos for the note author as done for system notes' do + system_note = create(:system_note, noteable: epic) + + service.new_note(system_note, john_doe) + + expect(first_todo.reload).to be_pending + expect(second_todo.reload).to be_pending + end + end + + context 'mentions' do + let(:execute) { service.new_note(note, author) } + + context 'for mentioned users' do + before do + note.update(note: description_mentions) + end + + let(:todo_params) { { action: Todo::MENTIONED } } + let(:todos_for) { [author, non_member, member, guest, admin, skipped] } + let(:todos_not_for) { [john_doe] } + + include_examples 'todos creation' + end + + context 'for directly addressed users' do + before do + note.update(note: description_directly_addressed) + end + + let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } } + let(:todos_for) { [author, non_member, member, guest, admin, skipped] } + let(:todos_not_for) { [john_doe] } + + include_examples 'todos creation' + end + + context 'combined' do + before do + note.update(note: combined_mentions) + end + + context 'mentioned users' do + let(:todo_params) { { action: Todo::MENTIONED } } + let(:todos_for) { [guest, admin, skipped] } + + include_examples 'todos creation' + end + + context 'directly addressed users' do + let(:todo_params) { { action: Todo::DIRECTLY_ADDRESSED } } + let(:todos_for) { [member] } + + include_examples 'todos creation' + end + end + end + end + end + end +end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 6e4265d4ed5931556bf5b91152c1695ef02bf20c..ab337a83053837f46fcd7bc7280a46cf912ed776 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -815,28 +815,33 @@ module API class Todo < Grape::Entity 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 :action_name expose :target_type 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 expose :target_url do |todo, options| target_type = todo.target_type.underscore - target_url = "namespace_project_#{target_type}_url" + target_url = "#{todo.parent.class.to_s.underscore}_#{target_type}_url" target_anchor = "note_#{todo.note_id}" if todo.note_id? Gitlab::Routing .url_helpers - .public_send(target_url, todo.project.namespace, todo.project, todo.target, anchor: target_anchor) # rubocop:disable GitlabSecurity/PublicSend + .public_send(target_url, todo.parent, todo.target, anchor: target_anchor) # rubocop:disable GitlabSecurity/PublicSend end expose :body expose :state expose :created_at + + def todo_target_class(target_type) + ::API::Entities.const_get(target_type) + end end class NamespaceBasic < Grape::Entity @@ -1446,3 +1451,4 @@ API::Entities.prepend_entity(::API::Entities::Project, with: EE::API::Entities:: API::Entities.prepend_entity(::API::Entities::ProtectedRefAccess, with: EE::API::Entities::ProtectedRefAccess) API::Entities.prepend_entity(::API::Entities::UserPublic, with: EE::API::Entities::UserPublic) API::Entities.prepend_entity(::API::Entities::Variable, with: EE::API::Entities::Variable) +API::Entities.prepend_entity(::API::Entities::Todo, with: EE::API::Entities::Todo) diff --git a/lib/api/todos.rb b/lib/api/todos.rb index c6dbcf84e3a8bbc25d9df0af5d44e70ae2ee2778..708c5f1ea0e1d5a0a0d45143ae067f2032cb42c9 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -1,6 +1,7 @@ module API class Todos < Grape::API include PaginationParams + prepend EE::API::Todos before { authenticate! } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 260bd6c73d929d6f20fc7a4ed1a10ba06fdac5fe..6d099a5280051dc6a8daf457184fc2a147cefaac 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -6305,6 +6305,12 @@ msgstr "" msgid "There are problems accessing Git storage: " msgstr "" +msgid "There was an error adding a todo." +msgstr "" + +msgid "There was an error deleting the todo." +msgstr "" + msgid "There was an error loading users activity calendar." msgstr "" diff --git a/spec/controllers/projects/todos_controller_spec.rb b/spec/controllers/projects/todos_controller_spec.rb index 1ce7e84bef94bf5aa420552d15be2fecec538a30..58f2817c7cc33e03f5809c4e44149405c9a5f206 100644 --- a/spec/controllers/projects/todos_controller_spec.rb +++ b/spec/controllers/projects/todos_controller_spec.rb @@ -5,10 +5,29 @@ 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 go + def post_create post :create, namespace_id: project.namespace, project_id: project, @@ -17,66 +36,13 @@ describe Projects::TodosController do format: 'html' end - 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 + it_behaves_like 'project todos actions' end end context 'Merge Requests' do describe 'POST create' do - def go + def post_create post :create, namespace_id: project.namespace, project_id: project, @@ -85,60 +51,7 @@ describe Projects::TodosController do format: 'html' end - 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 + it_behaves_like 'project todos actions' end end end diff --git a/spec/factories/todos.rb b/spec/factories/todos.rb index 94f8caedfa6d0a3ee172698be79f0b45b9d56976..14486c80341c591948f1df22b49363a6af8a1c1b 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 { project.creator } + author { project&.creator || user } + user { project&.creator || user } target factory: :issue action { Todo::ASSIGNED } diff --git a/spec/features/dashboard/todos/todos_filtering_spec.rb b/spec/features/dashboard/todos/todos_filtering_spec.rb index 85f865321cf7758e01a67cf58971fd4356763428..3ea427489363c13c5cb6664a7a6c1294300fceda 100644 --- a/spec/features/dashboard/todos/todos_filtering_spec.rb +++ b/spec/features/dashboard/todos/todos_filtering_spec.rb @@ -4,23 +4,36 @@ describe 'Dashboard > User filters todos', :js do let(:user_1) { create(:user, username: 'user_1', name: 'user_1') } let(:user_2) { create(:user, username: 'user_2', name: 'user_2') } - let(:project_1) { create(:project, name: 'project_1') } - let(:project_2) { create(:project, name: 'project_2') } + let(:group1) { create(:group) } + let(:group2) { create(:group) } - let(:issue) { create(:issue, title: 'issue', project: project_1) } + let(:project_1) { create(:project, name: 'project_1', namespace: group1) } + let(:project_2) { create(:project, name: 'project_2', namespace: group1) } + let(:project_3) { create(:project, name: 'project_3', namespace: group2) } + + let(:issue1) { create(:issue, title: 'issue', project: project_1) } + let(:issue2) { create(:issue, title: 'issue', project: project_3) } let!(:merge_request) { create(:merge_request, source_project: project_2, title: 'merge_request') } before do - create(:todo, user: user_1, author: user_2, project: project_1, target: issue, action: 1) + create(:todo, user: user_1, author: user_2, project: project_1, target: issue1, action: 1) + create(:todo, user: user_1, author: user_2, project: project_3, target: issue2, action: 1) create(:todo, user: user_1, author: user_1, project: project_2, target: merge_request, action: 2) project_1.add_developer(user_1) project_2.add_developer(user_1) + project_3.add_developer(user_1) sign_in(user_1) visit dashboard_todos_path end + it 'displays all todos without a filter' do + expect(page).to have_content issue1.to_reference(full: true) + expect(page).to have_content merge_request.to_reference(full: true) + expect(page).to have_content issue2.to_reference(full: true) + end + it 'filters by project' do click_button 'Project' within '.dropdown-menu-project' do @@ -34,6 +47,20 @@ describe 'Dashboard > User filters todos', :js do expect(page).not_to have_content project_2.full_name end + it 'filters by group' do + click_button 'Group' + within '.dropdown-menu-group' do + fill_in 'Search groups', with: group1.full_name + click_link group1.full_name + end + + wait_for_requests + + expect(page).to have_content issue1.to_reference(full: true) + expect(page).to have_content merge_request.to_reference(full: true) + expect(page).not_to have_content issue2.to_reference(full: true) + end + context 'Author filter' do it 'filters by author' do click_button 'Author' @@ -63,7 +90,7 @@ describe 'Dashboard > User filters todos', :js do it 'shows only authors of existing done todos' do user_3 = create :user user_4 = create :user - create(:todo, user: user_1, author: user_3, project: project_1, target: issue, action: 1, state: :done) + create(:todo, user: user_1, author: user_3, project: project_1, target: issue1, action: 1, state: :done) create(:todo, user: user_1, author: user_4, project: project_2, target: merge_request, action: 2, state: :done) project_1.add_developer(user_3) @@ -92,14 +119,15 @@ describe 'Dashboard > User filters todos', :js do wait_for_requests - expect(find('.todos-list')).to have_content issue.to_reference + expect(find('.todos-list')).to have_content issue1.to_reference + expect(find('.todos-list')).to have_content issue2.to_reference expect(find('.todos-list')).not_to have_content merge_request.to_reference end describe 'filter by action' do before do create(:todo, :build_failed, user: user_1, author: user_2, project: project_1) - create(:todo, :marked, user: user_1, author: user_2, project: project_1, target: issue) + create(:todo, :marked, user: user_1, author: user_2, project: project_1, target: issue1) end it 'filters by Assigned' do diff --git a/spec/finders/todos_finder_spec.rb b/spec/finders/todos_finder_spec.rb index 9747b9402a7fb5e73661f16abcffa58bdfe3fae4..6061021d3b0ec1749cf58266a96e868b23989021 100644 --- a/spec/finders/todos_finder_spec.rb +++ b/spec/finders/todos_finder_spec.rb @@ -5,12 +5,76 @@ 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 4efadecb356083dd45400cb67975306f88bd3fdb..68f8303dfd608a98fe65d97f3590b0d9edd5efac 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -21,6 +21,27 @@ 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/epics/sidebar/services/sidebar_service_spec.js b/spec/javascripts/epics/sidebar/services/sidebar_service_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..627beca379aaf22f3a5b15623210993535f8b1c9 --- /dev/null +++ b/spec/javascripts/epics/sidebar/services/sidebar_service_spec.js @@ -0,0 +1,65 @@ +import axios from '~/lib/utils/axios_utils'; + +import SidebarService from 'ee/epics/sidebar/services/sidebar_service'; + +describe('Sidebar Service', () => { + let service; + + beforeEach(() => { + service = new SidebarService({ + endpoint: gl.TEST_HOST, + subscriptionEndpoint: gl.TEST_HOST, + todoPath: gl.TEST_HOST, + }); + }); + + describe('updateStartDate', () => { + it('returns axios instance with PUT for `endpoint` and `start_date` as request body', () => { + spyOn(axios, 'put').and.stub(); + const startDate = '2018-06-21'; + service.updateStartDate(startDate); + expect(axios.put).toHaveBeenCalledWith(service.endpoint, { + start_date: startDate, + }); + }); + }); + + describe('updateEndDate', () => { + it('returns axios instance with PUT for `endpoint` and `end_date` as request body', () => { + spyOn(axios, 'put').and.stub(); + const endDate = '2018-06-21'; + service.updateEndDate(endDate); + expect(axios.put).toHaveBeenCalledWith(service.endpoint, { + end_date: endDate, + }); + }); + }); + + describe('toggleSubscribed', () => { + it('returns axios instance with POST for `subscriptionEndpoint`', () => { + spyOn(axios, 'post').and.stub(); + service.toggleSubscribed(); + expect(axios.post).toHaveBeenCalled(); + }); + }); + + describe('addTodo', () => { + it('returns axios instance with POST for `todoPath` with `issuable_id` and `issuable_type` as request body', () => { + spyOn(axios, 'post').and.stub(); + const epicId = 1; + service.addTodo(epicId); + expect(axios.post).toHaveBeenCalledWith(service.todoPath, { + issuable_id: epicId, + issuable_type: 'epic', + }); + }); + }); + + describe('deleteTodo', () => { + it('returns axios instance with DELETE for provided `todoDeletePath` param', () => { + spyOn(axios, 'delete').and.stub(); + service.deleteTodo('/foo/bar'); + expect(axios.delete).toHaveBeenCalledWith('/foo/bar'); + }); + }); +}); diff --git a/spec/javascripts/sidebar/todo_spec.js b/spec/javascripts/sidebar/todo_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..a929b804a29a3494b7f96861583ca69fa30fdf15 --- /dev/null +++ b/spec/javascripts/sidebar/todo_spec.js @@ -0,0 +1,158 @@ +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 bd498269798225f05e8a867a1cfa049b0abf5818..f29abcf536e55d06b9e76e484ec88483549aadd0 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -7,6 +7,7 @@ describe Todo do it { is_expected.to belong_to(:author).class_name("User") } it { is_expected.to belong_to(:note) } it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:target).touch(true) } it { is_expected.to belong_to(:user) } end diff --git a/spec/support/shared_examples/controllers/todos_shared_examples.rb b/spec/support/shared_examples/controllers/todos_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..bafd9bac8d03a7fb2fdabf0ba7c5e7c0615e7963 --- /dev/null +++ b/spec/support/shared_examples/controllers/todos_shared_examples.rb @@ -0,0 +1,43 @@ +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