diff options
66 files changed, 1744 insertions, 469 deletions
diff --git a/app/assets/javascripts/boards/components/board_card.vue b/app/assets/javascripts/boards/components/board_card.vue index 23fec503586..84885ca9306 100644 --- a/app/assets/javascripts/boards/components/board_card.vue +++ b/app/assets/javascripts/boards/components/board_card.vue @@ -1,4 +1,5 @@ <script> +/* eslint-disable vue/require-default-prop */ import './issue_card_inner'; import eventHub from '../eventhub'; @@ -34,6 +35,9 @@ export default { type: String, default: '', }, + groupId: { + type: Number, + }, }, data() { return { @@ -88,6 +92,7 @@ export default { :list="list" :issue="issue" :issue-link-base="issueLinkBase" + :group-id="groupId" :root-path="rootPath" :update-filters="true" /> diff --git a/app/assets/javascripts/boards/components/board_list.vue b/app/assets/javascripts/boards/components/board_list.vue index 6637904d87d..0d03c1c419c 100644 --- a/app/assets/javascripts/boards/components/board_list.vue +++ b/app/assets/javascripts/boards/components/board_list.vue @@ -15,6 +15,11 @@ export default { loadingIcon, }, props: { + groupId: { + type: Number, + required: false, + default: 0, + }, disabled: { type: Boolean, required: true, @@ -170,6 +175,7 @@ export default { <loading-icon /> </div> <board-new-issue + :group-id="groupId" :list="list" v-if="list.type !== 'closed' && showIssueForm"/> <ul @@ -185,6 +191,7 @@ export default { :list="list" :issue="issue" :issue-link-base="issueLinkBase" + :group-id="groupId" :root-path="rootPath" :disabled="disabled" :key="issue.id" /> diff --git a/app/assets/javascripts/boards/components/board_new_issue.vue b/app/assets/javascripts/boards/components/board_new_issue.vue index efface7143d..870d242e774 100644 --- a/app/assets/javascripts/boards/components/board_new_issue.vue +++ b/app/assets/javascripts/boards/components/board_new_issue.vue @@ -1,12 +1,21 @@ <script> import eventHub from '../eventhub'; +import ProjectSelect from './project_select.vue'; import ListIssue from '../models/issue'; const Store = gl.issueBoards.BoardsStore; export default { name: 'BoardNewIssue', + components: { + ProjectSelect, + }, props: { + groupId: { + type: Number, + required: false, + default: 0, + }, list: { type: Object, required: true, @@ -16,10 +25,20 @@ export default { return { title: '', error: false, + selectedProject: {}, }; }, + computed: { + disabled() { + if (this.groupId) { + return this.title === '' || !this.selectedProject.name; + } + return this.title === ''; + }, + }, mounted() { this.$refs.input.focus(); + eventHub.$on('setSelectedProject', this.setSelectedProject); }, methods: { submit(e) { @@ -34,6 +53,7 @@ export default { labels, subscribed: true, assignees: [], + project_id: this.selectedProject.id, }); eventHub.$emit(`scroll-board-list-${this.list.id}`); @@ -62,52 +82,62 @@ export default { this.title = ''; eventHub.$emit(`hide-issue-form-${this.list.id}`); }, + setSelectedProject(selectedProject) { + this.selectedProject = selectedProject; + }, }, }; </script> <template> - <div class="card board-new-issue-form"> - <form @submit="submit($event)"> - <div - class="flash-container" - v-if="error" - > - <div class="flash-alert"> - An error occurred. Please try again. - </div> - </div> - <label - class="label-light" - :for="list.id + '-title'" - > - Title - </label> - <input - class="form-control" - type="text" - v-model="title" - ref="input" - autocomplete="off" - :id="list.id + '-title'" - /> - <div class="clearfix prepend-top-10"> - <button - class="btn btn-success pull-left" - type="submit" - :disabled="title === ''" - ref="submit-button" + <div class="board-new-issue-form"> + <div class="card"> + <form @submit="submit($event)"> + <div + class="flash-container" + v-if="error" > - Submit issue - </button> - <button - class="btn btn-default pull-right" - type="button" - @click="cancel" + <div class="flash-alert"> + An error occurred. Please try again. + </div> + </div> + <label + class="label-light" + :for="list.id + '-title'" > - Cancel - </button> - </div> - </form> + Title + </label> + <input + class="form-control" + type="text" + v-model="title" + ref="input" + autocomplete="off" + :id="list.id + '-title'" + /> + <project-select + v-if="groupId" + :group-id="groupId" + /> + <div class="clearfix prepend-top-10"> + <button + class="btn btn-success pull-left" + type="submit" + :disabled="disabled" + ref="submit-button" + > + Submit issue + </button> + <button + class="btn btn-default pull-right" + type="button" + @click="cancel" + > + Cancel + </button> + </div> + </form> + </div> </div> </template> + diff --git a/app/assets/javascripts/boards/components/issue_card_inner.js b/app/assets/javascripts/boards/components/issue_card_inner.js index bf474879024..fc2bad2415f 100644 --- a/app/assets/javascripts/boards/components/issue_card_inner.js +++ b/app/assets/javascripts/boards/components/issue_card_inner.js @@ -31,6 +31,10 @@ gl.issueBoards.IssueCardInner = Vue.extend({ required: false, default: false, }, + groupId: { + type: Number, + required: false, + }, }, data() { return { @@ -64,7 +68,13 @@ gl.issueBoards.IssueCardInner = Vue.extend({ return this.issue.assignees.length > this.numberOverLimit; }, cardUrl() { - return `${this.issueLinkBase}/${this.issue.iid}`; + let baseUrl = this.issueLinkBase; + + if (this.groupId && this.issue.project) { + baseUrl = this.issueLinkBase.replace(':project_path', this.issue.project.path); + } + + return `${baseUrl}/${this.issue.iid}`; }, issueId() { if (this.issue.iid) { @@ -148,7 +158,7 @@ gl.issueBoards.IssueCardInner = Vue.extend({ class="card-number" v-if="issueId" > - {{ issueId }} + <template v-if="groupId && issue.project">{{issue.project.path}}</template>{{ issueId }} </span> </h4> <div class="card-assignee"> diff --git a/app/assets/javascripts/boards/components/project_select.vue b/app/assets/javascripts/boards/components/project_select.vue new file mode 100644 index 00000000000..d99b222c305 --- /dev/null +++ b/app/assets/javascripts/boards/components/project_select.vue @@ -0,0 +1,127 @@ +<script> + /* global ListIssue */ + import _ from 'underscore'; + import eventHub from '../eventhub'; + import loadingIcon from '../../vue_shared/components/loading_icon.vue'; + import Api from '../../api'; + + export default { + name: 'BoardProjectSelect', + components: { + loadingIcon, + }, + props: { + groupId: { + type: Number, + required: true, + default: 0, + }, + }, + data() { + return { + loading: true, + selectedProject: {}, + }; + }, + computed: { + selectedProjectName() { + return this.selectedProject.name || 'Select a project'; + }, + }, + mounted() { + $(this.$refs.projectsDropdown).glDropdown({ + filterable: true, + filterRemote: true, + search: { + fields: ['name_with_namespace'], + }, + clicked: ({ $el, e }) => { + e.preventDefault(); + this.selectedProject = { + id: $el.data('project-id'), + name: $el.data('project-name'), + }; + eventHub.$emit('setSelectedProject', this.selectedProject); + }, + selectable: true, + data: (term, callback) => { + this.loading = true; + return Api.groupProjects(this.groupId, term, (projects) => { + this.loading = false; + callback(projects); + }); + }, + renderRow(project) { + return ` + <li> + <a href='#' class='dropdown-menu-link' data-project-id="${project.id}" data-project-name="${project.name}"> + ${_.escape(project.name)} + </a> + </li> + `; + }, + text: project => project.name, + }); + }, + }; +</script> + +<template> + <div> + <label class="label-light prepend-top-10"> + Project + </label> + <div + ref="projectsDropdown" + class="dropdown" + > + <button + class="dropdown-menu-toggle wide" + type="button" + data-toggle="dropdown" + aria-expanded="false" + > + {{ selectedProjectName }} + <i + class="fa fa-chevron-down" + aria-hidden="true" + > + </i> + </button> + <div class="dropdown-menu dropdown-menu-selectable dropdown-menu-full-width"> + <div class="dropdown-title"> + <span>Projects</span> + <button + aria-label="Close" + type="button" + class="dropdown-title-button dropdown-menu-close" + > + <i + aria-hidden="true" + data-hidden="true" + class="fa fa-times dropdown-menu-close-icon" + > + </i> + </button> + </div> + <div class="dropdown-input"> + <input + class="dropdown-input-field" + type="search" + placeholder="Search projects" + /> + <i + aria-hidden="true" + data-hidden="true" + class="fa fa-search dropdown-input-search" + > + </i> + </div> + <div class="dropdown-content"></div> + <div class="dropdown-loading"> + <loading-icon /> + </div> + </div> + </div> + </div> +</template> diff --git a/app/assets/javascripts/boards/components/sidebar/remove_issue.js b/app/assets/javascripts/boards/components/sidebar/remove_issue.js index 0ae32bb4d0a..09c683ff621 100644 --- a/app/assets/javascripts/boards/components/sidebar/remove_issue.js +++ b/app/assets/javascripts/boards/components/sidebar/remove_issue.js @@ -24,7 +24,7 @@ gl.issueBoards.RemoveIssueBtn = Vue.extend({ }, computed: { updateUrl() { - return this.issueUpdate; + return this.issueUpdate.replace(':project_path', this.issue.project.path); }, }, methods: { @@ -32,17 +32,21 @@ gl.issueBoards.RemoveIssueBtn = Vue.extend({ const issue = this.issue; const lists = issue.getLists(); const listLabelIds = lists.map(list => list.label.id); - let labelIds = this.issue.labels + + let labelIds = issue.labels .map(label => label.id) .filter(id => !listLabelIds.includes(id)); if (labelIds.length === 0) { labelIds = ['']; } + const data = { issue: { label_ids: labelIds, }, }; + + // Post the remove data Vue.http.patch(this.updateUrl, data).catch(() => { Flash(__('Failed to remove issue from board, please try again.')); diff --git a/app/assets/javascripts/boards/filtered_search_boards.js b/app/assets/javascripts/boards/filtered_search_boards.js index 57a7cc4ca30..fb40b9f5565 100644 --- a/app/assets/javascripts/boards/filtered_search_boards.js +++ b/app/assets/javascripts/boards/filtered_search_boards.js @@ -6,6 +6,7 @@ export default class FilteredSearchBoards extends FilteredSearchManager { constructor(store, updateUrl = false, cantEdit = []) { super({ page: 'boards', + stateFiltersSelector: '.issues-state-filters', }); this.store = store; diff --git a/app/assets/javascripts/boards/index.js b/app/assets/javascripts/boards/index.js index 8e31f1865f0..b8749a13f68 100644 --- a/app/assets/javascripts/boards/index.js +++ b/app/assets/javascripts/boards/index.js @@ -13,6 +13,7 @@ import './models/issue'; import './models/label'; import './models/list'; import './models/milestone'; +import './models/project'; import './models/assignee'; import './stores/boards_store'; import './stores/modal_store'; @@ -89,7 +90,7 @@ export default () => { sidebarEventHub.$off('toggleSubscription', this.toggleSubscription); }, mounted () { - this.filterManager = new FilteredSearchBoards(Store.filter, true); + this.filterManager = new FilteredSearchBoards(Store.filter, true, Store.cantEdit); this.filterManager.setup(); Store.disabled = this.disabled; @@ -179,6 +180,7 @@ export default () => { return { modal: ModalStore.store, store: Store.state, + canAdminList: this.$options.el.hasAttribute('data-can-admin-list'), }; }, computed: { @@ -232,6 +234,7 @@ export default () => { :class="{ 'disabled': disabled }" :title="tooltipTitle" :aria-disabled="disabled" + v-if="canAdminList" @click="openModal"> Add issues </button> diff --git a/app/assets/javascripts/boards/mixins/sortable_default_options.js b/app/assets/javascripts/boards/mixins/sortable_default_options.js index 38a0eb12f92..5e31c6314b2 100644 --- a/app/assets/javascripts/boards/mixins/sortable_default_options.js +++ b/app/assets/javascripts/boards/mixins/sortable_default_options.js @@ -1,6 +1,8 @@ /* eslint-disable no-unused-vars, no-mixed-operators, comma-dangle */ /* global DocumentTouch */ +import sortableConfig from '../../sortable/sortable_config'; + window.gl = window.gl || {}; window.gl.issueBoards = window.gl.issueBoards || {}; @@ -18,19 +20,14 @@ gl.issueBoards.onEnd = () => { gl.issueBoards.touchEnabled = ('ontouchstart' in window) || window.DocumentTouch && document instanceof DocumentTouch; gl.issueBoards.getBoardSortableDefaultOptions = (obj) => { - const defaultSortOptions = { - animation: 200, - forceFallback: true, - fallbackClass: 'is-dragging', - fallbackOnBody: true, - ghostClass: 'is-ghost', + const defaultSortOptions = Object.assign({}, sortableConfig, { filter: '.board-delete, .btn', delay: gl.issueBoards.touchEnabled ? 100 : 0, scrollSensitivity: gl.issueBoards.touchEnabled ? 60 : 100, scrollSpeed: 20, onStart: gl.issueBoards.onStart, - onEnd: gl.issueBoards.onEnd - }; + onEnd: gl.issueBoards.onEnd, + }); Object.keys(obj).forEach((key) => { defaultSortOptions[key] = obj[key]; }); return defaultSortOptions; diff --git a/app/assets/javascripts/boards/models/issue.js b/app/assets/javascripts/boards/models/issue.js index 3bfb6d39ad5..4c5079efc8b 100644 --- a/app/assets/javascripts/boards/models/issue.js +++ b/app/assets/javascripts/boards/models/issue.js @@ -4,6 +4,7 @@ /* global ListAssignee */ import Vue from 'vue'; +import IssueProject from './project'; class ListIssue { constructor (obj, defaultAvatar) { @@ -23,6 +24,12 @@ class ListIssue { this.isLoading = {}; this.sidebarInfoEndpoint = obj.issue_sidebar_endpoint; this.toggleSubscriptionEndpoint = obj.toggle_subscription_endpoint; + this.milestone_id = obj.milestone_id; + this.project_id = obj.project_id; + + if (obj.project) { + this.project = new IssueProject(obj.project); + } if (obj.milestone) { this.milestone = new ListMilestone(obj.milestone); @@ -105,7 +112,8 @@ class ListIssue { data.issue.label_ids = ['']; } - return Vue.http.patch(url, data); + const projectPath = this.project ? this.project.path : ''; + return Vue.http.patch(url.replace(':project_path', projectPath), data); } } diff --git a/app/assets/javascripts/boards/models/project.js b/app/assets/javascripts/boards/models/project.js new file mode 100644 index 00000000000..a3d5c7af7ac --- /dev/null +++ b/app/assets/javascripts/boards/models/project.js @@ -0,0 +1,6 @@ +export default class IssueProject { + constructor(obj) { + this.id = obj.id; + this.path = obj.path; + } +} diff --git a/app/assets/javascripts/pages/groups/boards/index.js b/app/assets/javascripts/pages/groups/boards/index.js new file mode 100644 index 00000000000..5cfe8723204 --- /dev/null +++ b/app/assets/javascripts/pages/groups/boards/index.js @@ -0,0 +1,9 @@ +import UsersSelect from '~/users_select'; +import ShortcutsNavigation from '~/shortcuts_navigation'; +import initBoards from '~/boards'; + +document.addEventListener('DOMContentLoaded', () => { + new UsersSelect(); // eslint-disable-line no-new + new ShortcutsNavigation(); // eslint-disable-line no-new + initBoards(); +}); diff --git a/app/assets/javascripts/sortable/sortable_config.js b/app/assets/javascripts/sortable/sortable_config.js new file mode 100644 index 00000000000..43ef5d66422 --- /dev/null +++ b/app/assets/javascripts/sortable/sortable_config.js @@ -0,0 +1,7 @@ +export default { + animation: 200, + forceFallback: true, + fallbackClass: 'is-dragging', + fallbackOnBody: true, + ghostClass: 'is-ghost', +}; diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb index 352f12a89fd..35571fa9603 100644 --- a/app/controllers/boards/issues_controller.rb +++ b/app/controllers/boards/issues_controller.rb @@ -1,12 +1,14 @@ module Boards class IssuesController < Boards::ApplicationController include BoardsResponses + include ControllerWithCrossProjectAccessCheck + + requires_cross_project_access if: -> { board&.group_board? } before_action :whitelist_query_limiting, only: [:index, :update] before_action :authorize_read_issue, only: [:index] before_action :authorize_create_issue, only: [:create] before_action :authorize_update_issue, only: [:update] - skip_before_action :authenticate_user!, only: [:index] def index issues = Boards::Issues::ListService.new(board_parent, current_user, filter_params).execute @@ -64,11 +66,21 @@ module Boards end def issues_finder - IssuesFinder.new(current_user, project_id: board_parent.id) + if board.group_board? + IssuesFinder.new(current_user, group_id: board_parent.id) + else + IssuesFinder.new(current_user, project_id: board_parent.id) + end end def project - board_parent + @project ||= begin + if board.group_board? + Project.find(issue_params[:project_id]) + else + board_parent + end + end end def move_params diff --git a/app/controllers/concerns/boards_responses.rb b/app/controllers/concerns/boards_responses.rb index a145049dc7d..da830ec2cb1 100644 --- a/app/controllers/concerns/boards_responses.rb +++ b/app/controllers/concerns/boards_responses.rb @@ -1,10 +1,46 @@ module BoardsResponses + include Gitlab::Utils::StrongMemoize + + def board_params + params.require(:board).permit(:name, :weight, :milestone_id, :assignee_id, label_ids: []) + end + + def parent + strong_memoize(:parent) do + group? ? group : project + end + end + + def boards_path + if group? + group_boards_path(parent) + else + project_boards_path(parent) + end + end + + def board_path(board) + if group? + group_board_path(parent, board) + else + project_board_path(parent, board) + end + end + + def group? + instance_variable_defined?(:@group) + end + def authorize_read_list - authorize_action_for!(board.parent, :read_list) + ability = board.group_board? ? :read_group : :read_list + + authorize_action_for!(board.parent, ability) end def authorize_read_issue - authorize_action_for!(board.parent, :read_issue) + ability = board.group_board? ? :read_group : :read_issue + + authorize_action_for!(board.parent, ability) end def authorize_update_issue @@ -31,6 +67,10 @@ module BoardsResponses respond_with(@board) # rubocop:disable Gitlab/ModuleWithInstanceVariables end + def serialize_as_json(resource) + resource.as_json(only: [:id]) + end + def respond_with(resource) respond_to do |format| format.html diff --git a/app/controllers/groups/boards_controller.rb b/app/controllers/groups/boards_controller.rb new file mode 100644 index 00000000000..7c2016f0326 --- /dev/null +++ b/app/controllers/groups/boards_controller.rb @@ -0,0 +1,27 @@ +class Groups::BoardsController < Groups::ApplicationController + include BoardsResponses + + before_action :assign_endpoint_vars + + def index + @boards = Boards::ListService.new(group, current_user).execute + + respond_with_boards + end + + def show + @board = group.boards.find(params[:id]) + + respond_with_board + end + + def assign_endpoint_vars + @boards_endpoint = group_boards_url(group) + @namespace_path = group.to_param + @labels_endpoint = group_labels_url(group) + end + + def serialize_as_json(resource) + resource.as_json(only: [:id]) + end +end diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb index ac1d97dc54a..58be330f466 100644 --- a/app/controllers/groups/labels_controller.rb +++ b/app/controllers/groups/labels_controller.rb @@ -35,10 +35,18 @@ class Groups::LabelsController < Groups::ApplicationController def create @label = Labels::CreateService.new(label_params).execute(group: group) - if @label.valid? - redirect_to group_labels_path(@group) - else - render :new + respond_to do |format| + format.html do + if @label.valid? + redirect_to group_labels_path(@group) + else + render :new + end + end + + format.json do + render json: LabelSerializer.new.represent_appearance(@label) + end end end diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index 12b3d9bac1a..8304a83a952 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -17,23 +17,37 @@ module BoardsHelper end def build_issue_link_base - project_issues_path(@project) + if board.group_board? + "#{group_path(@board.group)}/:project_path/issues" + else + project_issues_path(@project) + end end def board_base_url - project_boards_path(@project) + if board.group_board? + group_boards_url(@group) + else + project_boards_path(@project) + end end def multiple_boards_available? - current_board_parent.multiple_issue_boards_available?(current_user) + current_board_parent.multiple_issue_boards_available? end def current_board_path(board) - @current_board_path ||= project_board_path(current_board_parent, board) + @current_board_path ||= begin + if board.group_board? + group_board_path(current_board_parent, board) + else + project_board_path(current_board_parent, board) + end + end end def current_board_parent - @current_board_parent ||= @project + @current_board_parent ||= @group || @project end def can_admin_issue? @@ -47,7 +61,8 @@ module BoardsHelper labels: labels_filter_path(true), labels_endpoint: @labels_endpoint, namespace_path: @namespace_path, - project_path: @project&.try(:path) + project_path: @project&.path, + group_path: @group&.path } end @@ -59,7 +74,8 @@ module BoardsHelper field_name: 'issue[assignee_ids][]', first_user: current_user&.username, current_user: 'true', - project_id: @project&.try(:id), + project_id: @project&.id, + group_id: @group&.id, null_user: 'true', multi_select: 'true', 'dropdown-header': dropdown_options[:data][:'dropdown-header'], diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb index e26ce6da030..905e2002592 100644 --- a/app/helpers/form_helper.rb +++ b/app/helpers/form_helper.rb @@ -27,7 +27,7 @@ module FormHelper first_user: current_user&.username, null_user: true, current_user: true, - project_id: @project.id, + project_id: @project&.id, field_name: 'issue[assignee_ids][]', default_label: 'Unassigned', 'max-select': 1, diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 7910de73c52..796623041bb 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -129,7 +129,7 @@ module GroupsHelper links = [:overview, :group_members] if can?(current_user, :read_cross_project) - links += [:activity, :issues, :labels, :milestones, :merge_requests] + links += [:activity, :issues, :labels, :milestones, :merge_requests, :boards] end if can?(current_user, :admin_group, @group) diff --git a/app/models/board.rb b/app/models/board.rb index 5bb7d3d3722..3cede6fc99a 100644 --- a/app/models/board.rb +++ b/app/models/board.rb @@ -1,20 +1,22 @@ class Board < ActiveRecord::Base + belongs_to :group belongs_to :project has_many :lists, -> { order(:list_type, :position) }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent validates :project, presence: true, if: :project_needed? + validates :group, presence: true, unless: :project def project_needed? - true + !group end def parent - project + @parent ||= group || project end def group_board? - false + group_id.present? end def backlog_list diff --git a/app/models/group.rb b/app/models/group.rb index 75bf013ecd2..32a0bd0c70b 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -31,6 +31,8 @@ class Group < Namespace has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :boards + accepts_nested_attributes_for :variables, allow_destroy: true validate :visibility_level_allowed_by_projects diff --git a/app/models/label.rb b/app/models/label.rb index 7538f2d8718..de7f1d56c64 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -35,6 +35,7 @@ class Label < ActiveRecord::Base scope :templates, -> { where(template: true) } scope :with_title, ->(title) { where(title: title) } scope :with_lists_and_board, -> { joins(lists: :board).merge(List.movable) } + scope :on_group_boards, ->(group_id) { with_lists_and_board.where(boards: { group_id: group_id }) } scope :on_project_boards, ->(project_id) { with_lists_and_board.where(boards: { project_id: project_id }) } def self.prioritized(project) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index db274ea8172..37e5b4615ce 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -222,6 +222,11 @@ class Namespace < ActiveRecord::Base has_parent? end + # Overriden on EE module + def multiple_issue_boards_available? + false + end + def full_path_was if parent_id_was.nil? path_was diff --git a/app/models/project.rb b/app/models/project.rb index ba278a49688..5206a7d4d25 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1695,8 +1695,9 @@ class Project < ActiveRecord::Base end end - def multiple_issue_boards_available?(user) - feature_available?(:multiple_issue_boards, user) + # Overridden on EE module + def multiple_issue_boards_available? + false end def issue_board_milestone_available?(user = nil) diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index f0bcba588a2..c9cb730c4e9 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -48,7 +48,12 @@ class GroupPolicy < BasePolicy rule { has_access }.enable :read_namespace rule { developer }.enable :admin_milestones - rule { reporter }.enable :admin_label + + rule { reporter }.policy do + enable :admin_label + enable :admin_list + enable :admin_issue + end rule { master }.policy do enable :create_projects diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index 6078fe38064..ecd74b74f8a 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -40,7 +40,11 @@ module Boards end def set_parent - params[:project_id] = parent.id + if parent.is_a?(Group) + params[:group_id] = parent.id + else + params[:project_id] = parent.id + end end def set_state diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index 797d6df7c1a..15fed7d17c1 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -60,8 +60,10 @@ module Boards label_ids = if moving_to_list.movable? moving_from_list.label_id + elsif board.group_board? + ::Label.on_group_boards(parent.id).pluck(:label_id) else - Label.on_project_boards(parent.id).pluck(:label_id) + ::Label.on_project_boards(parent.id).pluck(:label_id) end Array(label_ids).compact diff --git a/app/services/boards/lists/create_service.rb b/app/services/boards/lists/create_service.rb index 183556a1d6b..bebc90c7a8d 100644 --- a/app/services/boards/lists/create_service.rb +++ b/app/services/boards/lists/create_service.rb @@ -12,7 +12,11 @@ module Boards private def available_labels_for(board) - LabelsFinder.new(current_user, project_id: parent.id).execute + if board.group_board? + parent.labels + else + LabelsFinder.new(current_user, project_id: parent.id).execute + end end def next_position(board) diff --git a/app/views/groups/boards/index.html.haml b/app/views/groups/boards/index.html.haml new file mode 100644 index 00000000000..bb56769bd3f --- /dev/null +++ b/app/views/groups/boards/index.html.haml @@ -0,0 +1 @@ += render "shared/boards/show", board: @boards.first diff --git a/app/views/groups/boards/show.html.haml b/app/views/groups/boards/show.html.haml new file mode 100644 index 00000000000..92838fa4b11 --- /dev/null +++ b/app/views/groups/boards/show.html.haml @@ -0,0 +1 @@ += render "shared/boards/show", board: @board, group: true diff --git a/app/views/layouts/nav/sidebar/_group.html.haml b/app/views/layouts/nav/sidebar/_group.html.haml index b520f28123f..5ea19c9882d 100644 --- a/app/views/layouts/nav/sidebar/_group.html.haml +++ b/app/views/layouts/nav/sidebar/_group.html.haml @@ -1,6 +1,6 @@ - issues_count = group_issues_count(state: 'opened') - merge_requests_count = group_merge_requests_count(state: 'opened') -- issues_sub_menu_items = ['groups#issues', 'labels#index', 'milestones#index'] +- issues_sub_menu_items = ['groups#issues', 'labels#index', 'milestones#index', 'boards#index', 'boards#show'] .nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) } .nav-sidebar-inner-scroll @@ -51,12 +51,19 @@ %strong.fly-out-top-item-name #{ _('Issues') } %span.badge.count.issue_counter.fly-out-badge= number_with_delimiter(issues_count) + %li.divider.fly-out-top-item = nav_link(path: 'groups#issues', html_options: { class: 'home' }) do = link_to issues_group_path(@group), title: 'List' do %span List + - if group_sidebar_link?(:boards) + = nav_link(path: ['boards#index', 'boards#show']) do + = link_to group_boards_path(@group), title: boards_link_text do + %span + = boards_link_text + - if group_sidebar_link?(:labels) = nav_link(path: 'labels#index') do = link_to group_labels_path(@group), title: 'Labels' do diff --git a/app/views/shared/boards/_show.html.haml b/app/views/shared/boards/_show.html.haml index 014b8de1dc9..3ac4245a61b 100644 --- a/app/views/shared/boards/_show.html.haml +++ b/app/views/shared/boards/_show.html.haml @@ -1,3 +1,5 @@ +- board = local_assigns.fetch(:board, nil) +- group = local_assigns.fetch(:group, false) - @no_breadcrumb_container = true - @no_container = true - @content_class = "issue-boards-content" @@ -27,7 +29,7 @@ ":root-path" => "rootPath", ":board-id" => "boardId", ":key" => "_uid" } - = render "shared/boards/components/sidebar" + = render "shared/boards/components/sidebar", group: group - if @project %board-add-issues-modal{ "new-issue-path" => new_project_issue_path(@project), "milestone-path" => milestones_filter_dropdown_path, diff --git a/app/views/shared/boards/components/_board.html.haml b/app/views/shared/boards/components/_board.html.haml index c687e66fd43..2e9ad380012 100644 --- a/app/views/shared/boards/components/_board.html.haml +++ b/app/views/shared/boards/components/_board.html.haml @@ -42,6 +42,7 @@ ":disabled" => "disabled", ":issue-link-base" => "issueLinkBase", ":root-path" => "rootPath", + ":groupId" => ((current_board_parent.id if @group) || 'null'), "ref" => "board-list" } - if can?(current_user, :admin_list, current_board_parent) %board-blank-state{ "v-if" => 'list.id == "blank"' } diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index fabb17c7340..fc6f71ef60f 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -112,6 +112,7 @@ - if can?(current_user, :admin_label, board.parent) = render partial: "shared/issuable/label_page_create" = dropdown_loading - #js-add-issues-btn.prepend-left-10 + - if @project + #js-add-issues-btn.prepend-left-10{ data: { can_admin_list: can?(current_user, :admin_list, @project) } } - elsif type != :boards_modal = render 'shared/sort_dropdown' diff --git a/changelogs/unreleased/issue_38337.yml b/changelogs/unreleased/issue_38337.yml new file mode 100644 index 00000000000..a05eacefd68 --- /dev/null +++ b/changelogs/unreleased/issue_38337.yml @@ -0,0 +1,5 @@ +--- +title: Add one group port to CE +merge_request: +author: +type: added diff --git a/config/routes/group.rb b/config/routes/group.rb index 7a4740a4df7..612929dba09 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -56,6 +56,17 @@ constraints(GroupUrlConstrainer.new) do get ":secret/:filename", action: :show, as: :show, constraints: { filename: %r{[^/]+} } end end + + # On CE only index and show actions are needed + resources :boards, only: [:index, :show] + + legacy_ee_group_boards_redirect = redirect do |params, request| + path = "/groups/#{params[:group_id]}/-/boards" + path << "/#{params[:extra_params]}" if params[:extra_params].present? + path << "?#{request.query_string}" if request.query_string.present? + path + end + get 'boards(/*extra_params)', as: :legacy_ee_group_boards_redirect, to: legacy_ee_group_boards_redirect end scope(path: '*id', diff --git a/db/migrate/20180227182112_add_group_id_to_boards.rb b/db/migrate/20180227182112_add_group_id_to_boards.rb new file mode 100644 index 00000000000..d5da85bebcd --- /dev/null +++ b/db/migrate/20180227182112_add_group_id_to_boards.rb @@ -0,0 +1,39 @@ +# This is part of a backport from EE group boards feature which a few extra steps +# are required on this migration since it will be merged into EE which already +# contains the group_id column. +# like checking if the group_id column already exists before adding it. + +class AddGroupIdToBoards < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + DOWNTIME = false + + def up + unless group_id_exists? + add_column :boards, :group_id, :integer + add_foreign_key :boards, :namespaces, column: :group_id, on_delete: :cascade + add_concurrent_index :boards, :group_id + + change_column_null :boards, :project_id, true + end + end + + def down + if group_id_exists? + remove_foreign_key :boards, column: :group_id + remove_index :boards, :group_id if index_exists? :boards, :group_id + remove_column :boards, :group_id + + execute "DELETE from boards WHERE project_id IS NULL" + change_column_null :boards, :project_id, false + end + end + + private + + def group_id_exists? + column_exists?(:boards, :group_id) + end +end diff --git a/db/schema.rb b/db/schema.rb index db8bafe9677..1f68ad72308 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -184,11 +184,13 @@ ActiveRecord::Schema.define(version: 20180301084653) do add_index "award_emoji", ["user_id", "name"], name: "index_award_emoji_on_user_id_and_name", using: :btree create_table "boards", force: :cascade do |t| - t.integer "project_id", null: false + t.integer "project_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "group_id" end + add_index "boards", ["group_id"], name: "index_boards_on_group_id", using: :btree add_index "boards", ["project_id"], name: "index_boards_on_project_id", using: :btree create_table "broadcast_messages", force: :cascade do |t| @@ -1969,6 +1971,7 @@ ActiveRecord::Schema.define(version: 20180301084653) do add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree add_index "web_hooks", ["type"], name: "index_web_hooks_on_type", using: :btree + add_foreign_key "boards", "namespaces", column: "group_id", on_delete: :cascade add_foreign_key "boards", "projects", name: "fk_f15266b5f9", on_delete: :cascade add_foreign_key "chat_teams", "namespaces", on_delete: :cascade add_foreign_key "ci_build_trace_section_names", "projects", on_delete: :cascade diff --git a/doc/README.md b/doc/README.md index fb7a23e2750..05fa444657c 100644 --- a/doc/README.md +++ b/doc/README.md @@ -77,6 +77,7 @@ Manage your [repositories](user/project/repository/index.md) from the UI (user i - [Discussions](user/discussions/index.md): Threads, comments, and resolvable discussions in issues, commits, and merge requests. - [Issues](user/project/issues/index.md) - [Project issue Board](user/project/issue_board.md) +- [Group Issue Board](user/project/issue_board.md#group-issue-board) - [Issues and merge requests templates](user/project/description_templates.md): Create templates for submitting new issues and merge requests. - [Labels](user/project/labels.md): Categorize your issues or merge requests based on descriptive titles. - [Merge Requests](user/project/merge_requests/index.md) diff --git a/doc/api/README.md b/doc/api/README.md index b193ef4ab7f..b7981156086 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -27,6 +27,7 @@ following locations: - [Group Members](members.md) - [Issues](issues.md) - [Issue Boards](boards.md) +- [Group Issue Boards](group_boards.md) - [Jobs](jobs.md) - [Keys](keys.md) - [Labels](labels.md) diff --git a/doc/api/group_boards.md b/doc/api/group_boards.md new file mode 100644 index 00000000000..45a8544d6b1 --- /dev/null +++ b/doc/api/group_boards.md @@ -0,0 +1,284 @@ +# Group Issue Boards API + +Every API call to group boards must be authenticated. + +If a user is not a member of a group and the group is private, a `GET` +request will result in `404` status code. + +## Group Board + +Lists Issue Boards in the given group. + +``` +GET /groups/:id/boards +``` + +| 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 | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/5/boards +``` + +Example response: + +```json +[ + { + "id": 1, + "group_id": 5, + "lists" : [ + { + "id" : 1, + "label" : { + "name" : "Testing", + "color" : "#F0AD4E", + "description" : null + }, + "position" : 1 + }, + { + "id" : 2, + "label" : { + "name" : "Ready", + "color" : "#FF0000", + "description" : null + }, + "position" : 2 + }, + { + "id" : 3, + "label" : { + "name" : "Production", + "color" : "#FF5F00", + "description" : null + }, + "position" : 3 + } + ] + } +] +``` + +## Single board + +Gets a single board. + +``` +GET /groups/:id/boards/:board_id +``` + +| 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 | +| `board_id` | integer | yes | The ID of a board | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/5/boards/1 +``` + +Example response: + +```json + { + "id": 1, + "group_id": 5, + "lists" : [ + { + "id" : 1, + "label" : { + "name" : "Testing", + "color" : "#F0AD4E", + "description" : null + }, + "position" : 1 + }, + { + "id" : 2, + "label" : { + "name" : "Ready", + "color" : "#FF0000", + "description" : null + }, + "position" : 2 + }, + { + "id" : 3, + "label" : { + "name" : "Production", + "color" : "#FF5F00", + "description" : null + }, + "position" : 3 + } + ] + } +``` + +## List board lists + +Get a list of the board's lists. +Does not include `backlog` and `closed` lists + +``` +GET /groups/:id/boards/:board_id/lists +``` + +| 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 | +| `board_id` | integer | yes | The ID of a board | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/5/boards/1/lists +``` + +Example response: + +```json +[ + { + "id" : 1, + "label" : { + "name" : "Testing", + "color" : "#F0AD4E", + "description" : null + }, + "position" : 1 + }, + { + "id" : 2, + "label" : { + "name" : "Ready", + "color" : "#FF0000", + "description" : null + }, + "position" : 2 + }, + { + "id" : 3, + "label" : { + "name" : "Production", + "color" : "#FF5F00", + "description" : null + }, + "position" : 3 + } +] +``` + +## Single board list + +Get a single board list. + +``` +GET /groups/:id/boards/:board_id/lists/:list_id +``` + +| 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 | +| `board_id` | integer | yes | The ID of a board | +| `list_id` | integer | yes | The ID of a board's list | + +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/5/boards/1/lists/1 +``` + +Example response: + +```json +{ + "id" : 1, + "label" : { + "name" : "Testing", + "color" : "#F0AD4E", + "description" : null + }, + "position" : 1 +} +``` + +## New board list + +Creates a new Issue Board list. + +``` +POST /groups/:id/boards/:board_id/lists +``` + +| 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 | +| `board_id` | integer | yes | The ID of a board | +| `label_id` | integer | yes | The ID of a label | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/5/boards/1/lists?label_id=5 +``` + +Example response: + +```json +{ + "id" : 1, + "label" : { + "name" : "Testing", + "color" : "#F0AD4E", + "description" : null + }, + "position" : 1 +} +``` + +## Edit board list + +Updates an existing Issue Board list. This call is used to change list position. + +``` +PUT /groups/:id/boards/:board_id/lists/:list_id +``` + +| 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 | +| `board_id` | integer | yes | The ID of a board | +| `list_id` | integer | yes | The ID of a board's list | +| `position` | integer | yes | The position of the list | + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/group/5/boards/1/lists/1?position=2 +``` + +Example response: + +```json +{ + "id" : 1, + "label" : { + "name" : "Testing", + "color" : "#F0AD4E", + "description" : null + }, + "position" : 1 +} +``` + +## Delete a board list + +Only for admins and group owners. Soft deletes the board list in question. + +``` +DELETE /groups/:id/boards/:board_id/lists/:list_id +``` + +| 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 | +| `board_id` | integer | yes | The ID of a board | +| `list_id` | integer | yes | The ID of a board's list | + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/5/boards/1/lists/1 +``` diff --git a/doc/user/project/issue_board.md b/doc/user/project/issue_board.md index bc6306927e1..8df84db5139 100644 --- a/doc/user/project/issue_board.md +++ b/doc/user/project/issue_board.md @@ -235,6 +235,14 @@ to another list the label changes and a system not is recorded. [Developers and up](../permissions.md) can use all the functionality of the Issue Board, that is create/delete lists and drag issues around. +## Group Issue Board + +Group issue board i analogous to project-level issue board and it is accessible at the group +navigation level. A group-level issue board allows you to view all issues from all projects in that group +(currently, it does not see issues from projects in subgroups) Similarly, you can only filter by group labels for these +boards. When updating milestones and labels for an issue through the sidebar update mechanism, again only +group-level objects are available. + ## Tips A few things to remember: diff --git a/lib/api/api.rb b/lib/api/api.rb index 754549f72f0..254e1e786f5 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -120,6 +120,7 @@ module API mount ::API::Events mount ::API::Features mount ::API::Files + mount ::API::GroupBoards mount ::API::Groups mount ::API::GroupMilestones mount ::API::Internal diff --git a/lib/api/group_boards.rb b/lib/api/group_boards.rb new file mode 100644 index 00000000000..48709a5de6b --- /dev/null +++ b/lib/api/group_boards.rb @@ -0,0 +1,117 @@ +module API + class GroupBoards < Grape::API + include BoardsResponses + include PaginationParams + + before do + authenticate! + end + + helpers do + def board_parent + user_group + end + end + + params do + requires :id, type: String, desc: 'The ID of a group' + end + + resource :groups, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do + segment ':id/boards' do + desc 'Find a group board' do + detail 'This feature was introduced in 10.6' + success ::API::Entities::Board + end + get '/:board_id' do + present board, with: ::API::Entities::Board + end + + desc 'Get all group boards' do + detail 'This feature was introduced in 10.6' + success Entities::Board + end + params do + use :pagination + end + get '/' do + present paginate(board_parent.boards), with: Entities::Board + end + end + + params do + requires :board_id, type: Integer, desc: 'The ID of a board' + end + segment ':id/boards/:board_id' do + desc 'Get the lists of a group board' do + detail 'Does not include backlog and closed lists. This feature was introduced in 10.4' + success Entities::List + end + params do + use :pagination + end + get '/lists' do + present paginate(board_lists), with: Entities::List + end + + desc 'Get a list of a group board' do + detail 'This feature was introduced in 10.6' + success Entities::List + end + params do + requires :list_id, type: Integer, desc: 'The ID of a list' + end + get '/lists/:list_id' do + present board_lists.find(params[:list_id]), with: Entities::List + end + + desc 'Create a new board list' do + detail 'This feature was introduced in 10.6' + success Entities::List + end + params do + requires :label_id, type: Integer, desc: 'The ID of an existing label' + end + post '/lists' do + unless available_labels_for(board_parent).exists?(params[:label_id]) + render_api_error!({ error: 'Label not found!' }, 400) + end + + authorize!(:admin_list, user_group) + + create_list + end + + desc 'Moves a board list to a new position' do + detail 'This feature was introduced in 10.6' + success Entities::List + end + params do + requires :list_id, type: Integer, desc: 'The ID of a list' + requires :position, type: Integer, desc: 'The position of the list' + end + put '/lists/:list_id' do + list = board_lists.find(params[:list_id]) + + authorize!(:admin_list, user_group) + + move_list(list) + end + + desc 'Delete a board list' do + detail 'This feature was introduced in 10.6' + success Entities::List + end + params do + requires :list_id, type: Integer, desc: 'The ID of a board list' + end + delete "/lists/:list_id" do + authorize!(:admin_list, user_group) + list = board_lists.find(params[:list_id]) + + destroy_list(list) + end + end + end + end +end diff --git a/spec/controllers/groups/boards_controller_spec.rb b/spec/controllers/groups/boards_controller_spec.rb new file mode 100644 index 00000000000..0f5bde62006 --- /dev/null +++ b/spec/controllers/groups/boards_controller_spec.rb @@ -0,0 +1,136 @@ +require 'spec_helper' + +describe Groups::BoardsController do + let(:group) { create(:group) } + let(:user) { create(:user) } + + before do + group.add_master(user) + sign_in(user) + end + + describe 'GET index' do + it 'creates a new board when group does not have one' do + expect { list_boards }.to change(group.boards, :count).by(1) + end + + context 'when format is HTML' do + it 'renders template' do + list_boards + + expect(response).to render_template :index + expect(response.content_type).to eq 'text/html' + end + + context 'with unauthorized user' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_group, group).and_return(false) + end + + it 'returns a not found 404 response' do + list_boards + + expect(response).to have_gitlab_http_status(404) + expect(response.content_type).to eq 'text/html' + end + end + end + + context 'when format is JSON' do + it 'return an array with one group board' do + create(:board, group: group) + + list_boards format: :json + + parsed_response = JSON.parse(response.body) + + expect(response).to match_response_schema('boards') + expect(parsed_response.length).to eq 1 + end + + context 'with unauthorized user' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_group, group).and_return(false) + end + + it 'returns a not found 404 response' do + list_boards format: :json + + expect(response).to have_gitlab_http_status(404) + expect(response.content_type).to eq 'application/json' + end + end + end + + def list_boards(format: :html) + get :index, group_id: group, format: format + end + end + + describe 'GET show' do + let!(:board) { create(:board, group: group) } + + context 'when format is HTML' do + it 'renders template' do + read_board board: board + + expect(response).to render_template :show + expect(response.content_type).to eq 'text/html' + end + + context 'with unauthorized user' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_group, group).and_return(false) + end + + it 'returns a not found 404 response' do + read_board board: board + + expect(response).to have_gitlab_http_status(404) + expect(response.content_type).to eq 'text/html' + end + end + end + + context 'when format is JSON' do + it 'returns project board' do + read_board board: board, format: :json + + expect(response).to match_response_schema('board') + end + + context 'with unauthorized user' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(true) + allow(Ability).to receive(:allowed?).with(user, :read_group, group).and_return(false) + end + + it 'returns a not found 404 response' do + read_board board: board, format: :json + + expect(response).to have_gitlab_http_status(404) + expect(response.content_type).to eq 'application/json' + end + end + end + + context 'when board does not belong to group' do + it 'returns a not found 404 response' do + another_board = create(:board) + + read_board board: another_board + + expect(response).to have_gitlab_http_status(404) + end + end + + def read_board(board:, format: :html) + get :show, group_id: group, + id: board.to_param, + format: format + end + end +end diff --git a/spec/factories/boards.rb b/spec/factories/boards.rb index 1e125237ae8..6524bb4830c 100644 --- a/spec/factories/boards.rb +++ b/spec/factories/boards.rb @@ -1,6 +1,29 @@ FactoryBot.define do factory :board do - project + transient do + project nil + group nil + project_id nil + group_id nil + parent nil + end + + after(:build, :stub) do |board, evaluator| + if evaluator.group + board.group = evaluator.group + elsif evaluator.group_id + board.group_id = evaluator.group_id + elsif evaluator.project + board.project = evaluator.project + elsif evaluator.project_id + board.project_id = evaluator.project_id + elsif evaluator.parent + id = evaluator.parent.id + evaluator.parent.is_a?(Group) ? board.group_id = id : evaluator.project_id = id + else + board.project = create(:project, :empty_repo) + end + end after(:create) do |board| board.lists.create(list_type: :closed) diff --git a/spec/helpers/boards_helper_spec.rb b/spec/helpers/boards_helper_spec.rb index a3c5ab99c87..b22947911b9 100644 --- a/spec/helpers/boards_helper_spec.rb +++ b/spec/helpers/boards_helper_spec.rb @@ -1,6 +1,34 @@ require 'spec_helper' describe BoardsHelper do + describe '#build_issue_link_base' do + context 'project board' do + it 'returns correct path for project board' do + @project = create(:project) + @board = create(:board, project: @project) + + expect(build_issue_link_base).to eq("/#{@project.namespace.path}/#{@project.path}/issues") + end + end + + context 'group board' do + let(:base_group) { create(:group, path: 'base') } + + it 'returns correct path for base group' do + @board = create(:board, group: base_group) + + expect(build_issue_link_base).to eq('/base/:project_path/issues') + end + + it 'returns correct path for subgroup' do + subgroup = create(:group, parent: base_group, path: 'sub') + @board = create(:board, group: subgroup) + + expect(build_issue_link_base).to eq('/base/sub/:project_path/issues') + end + end + end + describe '#board_data' do let(:user) { create(:user) } let(:project) { create(:project) } diff --git a/spec/javascripts/api_spec.js b/spec/javascripts/api_spec.js index cf3a76d0d2e..5477581c1b9 100644 --- a/spec/javascripts/api_spec.js +++ b/spec/javascripts/api_spec.js @@ -137,6 +137,27 @@ describe('Api', () => { done(); }); }); + + it('creates a group label', (done) => { + const namespace = 'group/subgroup'; + const labelData = { some: 'data' }; + const expectedUrl = `${dummyUrlRoot}/groups/${namespace}/-/labels`; + const expectedData = { + label: labelData, + }; + mock.onPost(expectedUrl).reply((config) => { + expect(config.data).toBe(JSON.stringify(expectedData)); + + return [200, { + name: 'test', + }]; + }); + + Api.newLabel(namespace, undefined, labelData, (response) => { + expect(response.name).toBe('test'); + done(); + }); + }); }); describe('groupProjects', () => { diff --git a/spec/requests/api/group_boards_spec.rb b/spec/requests/api/group_boards_spec.rb new file mode 100644 index 00000000000..894c94688ba --- /dev/null +++ b/spec/requests/api/group_boards_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe API::GroupBoards do + set(:user) { create(:user) } + set(:non_member) { create(:user) } + set(:guest) { create(:user) } + set(:admin) { create(:user, :admin) } + set(:board_parent) { create(:group, :public) } + + before do + board_parent.add_owner(user) + end + + set(:project) { create(:project, :public, namespace: board_parent ) } + + set(:dev_label) do + create(:group_label, title: 'Development', color: '#FFAABB', group: board_parent) + end + + set(:test_label) do + create(:group_label, title: 'Testing', color: '#FFAACC', group: board_parent) + end + + set(:ux_label) do + create(:group_label, title: 'UX', color: '#FF0000', group: board_parent) + end + + set(:dev_list) do + create(:list, label: dev_label, position: 1) + end + + set(:test_list) do + create(:list, label: test_label, position: 2) + end + + set(:milestone) { create(:milestone, group: board_parent) } + set(:board_label) { create(:group_label, group: board_parent) } + + set(:board) { create(:board, group: board_parent, lists: [dev_list, test_list]) } + + it_behaves_like 'group and project boards', "/groups/:id/boards", false + + describe 'POST /groups/:id/boards/lists' do + let(:url) { "/groups/#{board_parent.id}/boards/#{board.id}/lists" } + + it 'does not create lists for child project labels' do + project_label = create(:label, project: project) + + post api(url, user), label_id: project_label.id + + expect(response).to have_gitlab_http_status(400) + end + end +end diff --git a/spec/services/boards/create_service_spec.rb b/spec/services/boards/create_service_spec.rb index db51a524e79..a715261cd6c 100644 --- a/spec/services/boards/create_service_spec.rb +++ b/spec/services/boards/create_service_spec.rb @@ -2,34 +2,20 @@ require 'spec_helper' describe Boards::CreateService do describe '#execute' do - let(:project) { create(:project) } + context 'when board parent is a project' do + let(:parent) { create(:project) } - subject(:service) { described_class.new(project, double) } + subject(:service) { described_class.new(parent, double) } - context 'when project does not have a board' do - it 'creates a new board' do - expect { service.execute }.to change(Board, :count).by(1) - end - - it 'creates the default lists' do - board = service.execute - - expect(board.lists.size).to eq 2 - expect(board.lists.first).to be_backlog - expect(board.lists.last).to be_closed - end + it_behaves_like 'boards create service' end - context 'when project has a board' do - before do - create(:board, project: project) - end + context 'when board parent is a group' do + let(:parent) { create(:group) } - it 'does not create a new board' do - expect(service).to receive(:can_create_board?) { false } + subject(:service) { described_class.new(parent, double) } - expect { service.execute }.not_to change(project.boards, :count) - end + it_behaves_like 'boards create service' end end end diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index ff5733b7064..b4efa3e44b6 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -2,98 +2,103 @@ require 'spec_helper' describe Boards::Issues::ListService do describe '#execute' do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:board) { create(:board, project: project) } - - let(:bug) { create(:label, project: project, name: 'Bug') } - let(:development) { create(:label, project: project, name: 'Development') } - let(:testing) { create(:label, project: project, name: 'Testing') } - let(:p1) { create(:label, title: 'P1', project: project, priority: 1) } - let(:p2) { create(:label, title: 'P2', project: project, priority: 2) } - let(:p3) { create(:label, title: 'P3', project: project, priority: 3) } - - let!(:backlog) { create(:backlog_list, board: board) } - let!(:list1) { create(:list, board: board, label: development, position: 0) } - let!(:list2) { create(:list, board: board, label: testing, position: 1) } - let!(:closed) { create(:closed_list, board: board) } - - let!(:opened_issue1) { create(:labeled_issue, project: project, labels: [bug]) } - let!(:opened_issue2) { create(:labeled_issue, project: project, labels: [p2]) } - let!(:reopened_issue1) { create(:issue, :opened, project: project) } - - let!(:list1_issue1) { create(:labeled_issue, project: project, labels: [p2, development]) } - let!(:list1_issue2) { create(:labeled_issue, project: project, labels: [development]) } - let!(:list1_issue3) { create(:labeled_issue, project: project, labels: [development, p1]) } - let!(:list2_issue1) { create(:labeled_issue, project: project, labels: [testing]) } - - let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) } - let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3]) } - let!(:closed_issue3) { create(:issue, :closed, project: project) } - let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1]) } - let!(:closed_issue5) { create(:labeled_issue, :closed, project: project, labels: [development]) } - - before do - project.add_developer(user) - end - - it 'delegates search to IssuesFinder' do - params = { board_id: board.id, id: list1.id } - - expect_any_instance_of(IssuesFinder).to receive(:execute).once.and_call_original + context 'when parent is a project' do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:board) { create(:board, project: project) } + + let(:m1) { create(:milestone, project: project) } + let(:m2) { create(:milestone, project: project) } + + let(:bug) { create(:label, project: project, name: 'Bug') } + let(:development) { create(:label, project: project, name: 'Development') } + let(:testing) { create(:label, project: project, name: 'Testing') } + let(:p1) { create(:label, title: 'P1', project: project, priority: 1) } + let(:p2) { create(:label, title: 'P2', project: project, priority: 2) } + let(:p3) { create(:label, title: 'P3', project: project, priority: 3) } + + let!(:backlog) { create(:backlog_list, board: board) } + let!(:list1) { create(:list, board: board, label: development, position: 0) } + let!(:list2) { create(:list, board: board, label: testing, position: 1) } + let!(:closed) { create(:closed_list, board: board) } + + let!(:opened_issue1) { create(:labeled_issue, project: project, milestone: m1, title: 'Issue 1', labels: [bug]) } + let!(:opened_issue2) { create(:labeled_issue, project: project, milestone: m2, title: 'Issue 2', labels: [p2]) } + let!(:reopened_issue1) { create(:issue, :opened, project: project, title: 'Issue 3' ) } + + let!(:list1_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [p2, development]) } + let!(:list1_issue2) { create(:labeled_issue, project: project, milestone: m2, labels: [development]) } + let!(:list1_issue3) { create(:labeled_issue, project: project, milestone: m1, labels: [development, p1]) } + let!(:list2_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [testing]) } + + let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) } + let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3]) } + let!(:closed_issue3) { create(:issue, :closed, project: project) } + let!(:closed_issue4) { create(:labeled_issue, :closed, project: project, labels: [p1]) } + let!(:closed_issue5) { create(:labeled_issue, :closed, project: project, labels: [development]) } + + let(:parent) { project } + + before do + project.add_developer(user) + end - described_class.new(project, user, params).execute + it_behaves_like 'issues list service' end - context 'issues are ordered by priority' do - it 'returns opened issues when list_id is missing' do - params = { board_id: board.id } - - issues = described_class.new(project, user, params).execute - - expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1] - end + context 'when parent is a group' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project) { create(:project, :empty_repo, namespace: group) } + let(:project1) { create(:project, :empty_repo, namespace: group) } + let(:board) { create(:board, group: group) } - it 'returns opened issues when listing issues from Backlog' do - params = { board_id: board.id, id: backlog.id } + let(:m1) { create(:milestone, group: group) } + let(:m2) { create(:milestone, group: group) } - issues = described_class.new(project, user, params).execute + let(:bug) { create(:group_label, group: group, name: 'Bug') } + let(:development) { create(:group_label, group: group, name: 'Development') } + let(:testing) { create(:group_label, group: group, name: 'Testing') } - expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1] - end + let(:p1) { create(:group_label, title: 'P1', group: group) } + let(:p2) { create(:group_label, title: 'P2', group: group) } + let(:p3) { create(:group_label, title: 'P3', group: group) } - it 'returns closed issues when listing issues from Closed' do - params = { board_id: board.id, id: closed.id } + let(:p1_project) { create(:label, title: 'P1_project', project: project, priority: 1) } + let(:p2_project) { create(:label, title: 'P2_project', project: project, priority: 2) } + let(:p3_project) { create(:label, title: 'P3_project', project: project, priority: 3) } - issues = described_class.new(project, user, params).execute + let(:p1_project1) { create(:label, title: 'P1_project1', project: project1, priority: 1) } + let(:p2_project1) { create(:label, title: 'P2_project1', project: project1, priority: 2) } + let(:p3_project1) { create(:label, title: 'P3_project1', project: project1, priority: 3) } - expect(issues).to eq [closed_issue4, closed_issue2, closed_issue5, closed_issue3, closed_issue1] - end + let!(:backlog) { create(:backlog_list, board: board) } + let!(:list1) { create(:list, board: board, label: development, position: 0) } + let!(:list2) { create(:list, board: board, label: testing, position: 1) } + let!(:closed) { create(:closed_list, board: board) } - it 'returns opened issues that have label list applied when listing issues from a label list' do - params = { board_id: board.id, id: list1.id } + let!(:opened_issue1) { create(:labeled_issue, project: project, milestone: m1, title: 'Issue 1', labels: [bug]) } + let!(:opened_issue2) { create(:labeled_issue, project: project, milestone: m2, title: 'Issue 2', labels: [p2, p2_project]) } + let!(:reopened_issue1) { create(:issue, state: 'opened', project: project, title: 'Issue 3', closed_at: Time.now ) } - issues = described_class.new(project, user, params).execute + let!(:list1_issue1) { create(:labeled_issue, project: project, milestone: m1, labels: [p2, p2_project, development]) } + let!(:list1_issue2) { create(:labeled_issue, project: project, milestone: m2, labels: [development]) } + let!(:list1_issue3) { create(:labeled_issue, project: project1, milestone: m1, labels: [development, p1, p1_project1]) } + let!(:list2_issue1) { create(:labeled_issue, project: project1, milestone: m1, labels: [testing]) } - expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2] - end - end + let!(:closed_issue1) { create(:labeled_issue, :closed, project: project, labels: [bug]) } + let!(:closed_issue2) { create(:labeled_issue, :closed, project: project, labels: [p3, p3_project]) } + let!(:closed_issue3) { create(:issue, :closed, project: project1) } + let!(:closed_issue4) { create(:labeled_issue, :closed, project: project1, labels: [p1, p1_project1]) } + let!(:closed_issue5) { create(:labeled_issue, :closed, project: project1, labels: [development]) } - context 'with list that does not belong to the board' do - it 'raises an error' do - list = create(:list) - service = described_class.new(project, user, board_id: board.id, id: list.id) + let(:parent) { group } - expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) + before do + group.add_developer(user) end - end - - context 'with invalid list id' do - it 'raises an error' do - service = described_class.new(project, user, board_id: board.id, id: nil) - expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) - end + it_behaves_like 'issues list service' end end end diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb index 280e411683e..0a6b6d880d3 100644 --- a/spec/services/boards/issues/move_service_spec.rb +++ b/spec/services/boards/issues/move_service_spec.rb @@ -2,108 +2,53 @@ require 'spec_helper' describe Boards::Issues::MoveService do describe '#execute' do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:board1) { create(:board, project: project) } - - let(:bug) { create(:label, project: project, name: 'Bug') } - let(:development) { create(:label, project: project, name: 'Development') } - let(:testing) { create(:label, project: project, name: 'Testing') } - - let!(:list1) { create(:list, board: board1, label: development, position: 0) } - let!(:list2) { create(:list, board: board1, label: testing, position: 1) } - let!(:closed) { create(:closed_list, board: board1) } - - before do - project.add_developer(user) - end - - context 'when moving an issue between lists' do - let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list2.id } } - - it 'delegates the label changes to Issues::UpdateService' do - expect_any_instance_of(Issues::UpdateService).to receive(:execute).with(issue).once - - described_class.new(project, user, params).execute(issue) - end - - it 'removes the label from the list it came from and adds the label of the list it goes to' do - described_class.new(project, user, params).execute(issue) - - expect(issue.reload.labels).to contain_exactly(bug, testing) - end - end - - context 'when moving to closed' do + context 'when parent is a project' do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:board1) { create(:board, project: project) } let(:board2) { create(:board, project: project) } + + let(:bug) { create(:label, project: project, name: 'Bug') } + let(:development) { create(:label, project: project, name: 'Development') } + let(:testing) { create(:label, project: project, name: 'Testing') } let(:regression) { create(:label, project: project, name: 'Regression') } - let!(:list3) { create(:list, board: board2, label: regression, position: 1) } - let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing, regression]) } - let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: closed.id } } + let!(:list1) { create(:list, board: board1, label: development, position: 0) } + let!(:list2) { create(:list, board: board1, label: testing, position: 1) } + let!(:closed) { create(:closed_list, board: board1) } - it 'delegates the close proceedings to Issues::CloseService' do - expect_any_instance_of(Issues::CloseService).to receive(:execute).with(issue).once + let(:parent) { project } - described_class.new(project, user, params).execute(issue) + before do + parent.add_developer(user) end - it 'removes all list-labels from project boards and close the issue' do - described_class.new(project, user, params).execute(issue) - issue.reload - - expect(issue.labels).to contain_exactly(bug) - expect(issue).to be_closed - end + it_behaves_like 'issues move service' end - context 'when moving from closed' do - let(:issue) { create(:labeled_issue, :closed, project: project, labels: [bug]) } - let(:params) { { board_id: board1.id, from_list_id: closed.id, to_list_id: list2.id } } - - it 'delegates the re-open proceedings to Issues::ReopenService' do - expect_any_instance_of(Issues::ReopenService).to receive(:execute).with(issue).once - - described_class.new(project, user, params).execute(issue) - end - - it 'adds the label of the list it goes to and reopen the issue' do - described_class.new(project, user, params).execute(issue) - issue.reload - - expect(issue.labels).to contain_exactly(bug, testing) - expect(issue).to be_opened - end - end + context 'when parent is a group' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project) { create(:project, namespace: group) } + let(:board1) { create(:board, group: group) } + let(:board2) { create(:board, group: group) } - context 'when moving to same list' do - let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:issue1) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:issue2) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } } + let(:bug) { create(:group_label, group: group, name: 'Bug') } + let(:development) { create(:group_label, group: group, name: 'Development') } + let(:testing) { create(:group_label, group: group, name: 'Testing') } + let(:regression) { create(:group_label, group: group, name: 'Regression') } - it 'returns false' do - expect(described_class.new(project, user, params).execute(issue)).to eq false - end + let!(:list1) { create(:list, board: board1, label: development, position: 0) } + let!(:list2) { create(:list, board: board1, label: testing, position: 1) } + let!(:closed) { create(:closed_list, board: board1) } - it 'keeps issues labels' do - described_class.new(project, user, params).execute(issue) + let(:parent) { group } - expect(issue.reload.labels).to contain_exactly(bug, development) + before do + parent.add_developer(user) end - it 'sorts issues' do - [issue, issue1, issue2].each do |issue| - issue.move_to_end && issue.save! - end - - params.merge!(move_after_id: issue1.id, move_before_id: issue2.id) - - described_class.new(project, user, params).execute(issue) - - expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) - end + it_behaves_like 'issues move service' end end end diff --git a/spec/services/boards/list_service_spec.rb b/spec/services/boards/list_service_spec.rb index 1d0be99fb35..7518e9e9b75 100644 --- a/spec/services/boards/list_service_spec.rb +++ b/spec/services/boards/list_service_spec.rb @@ -2,36 +2,20 @@ require 'spec_helper' describe Boards::ListService do describe '#execute' do - let(:project) { create(:project) } + context 'when board parent is a project' do + let(:parent) { create(:project) } - subject(:service) { described_class.new(project, double) } + subject(:service) { described_class.new(parent, double) } - context 'when project does not have a board' do - it 'creates a new project board' do - expect { service.execute }.to change(project.boards, :count).by(1) - end - - it 'delegates the project board creation to Boards::CreateService' do - expect_any_instance_of(Boards::CreateService).to receive(:execute).once - - service.execute - end + it_behaves_like 'boards list service' end - context 'when project has a board' do - before do - create(:board, project: project) - end - - it 'does not create a new board' do - expect { service.execute }.not_to change(project.boards, :count) - end - end + context 'when board parent is a group' do + let(:parent) { create(:group) } - it 'returns project boards' do - board = create(:board, project: project) + subject(:service) { described_class.new(parent, double) } - expect(service.execute).to match_array [board] + it_behaves_like 'boards list service' end end end diff --git a/spec/services/boards/lists/create_service_spec.rb b/spec/services/boards/lists/create_service_spec.rb index d5322e1bb21..7d3f5f86deb 100644 --- a/spec/services/boards/lists/create_service_spec.rb +++ b/spec/services/boards/lists/create_service_spec.rb @@ -2,62 +2,77 @@ require 'spec_helper' describe Boards::Lists::CreateService do describe '#execute' do - let(:project) { create(:project) } - let(:board) { create(:board, project: project) } - let(:user) { create(:user) } - let(:label) { create(:label, project: project, name: 'in-progress') } + shared_examples 'creating board lists' do + let(:user) { create(:user) } - subject(:service) { described_class.new(project, user, label_id: label.id) } + subject(:service) { described_class.new(parent, user, label_id: label.id) } - before do - project.add_developer(user) - end + before do + parent.add_developer(user) + end - context 'when board lists is empty' do - it 'creates a new list at beginning of the list' do - list = service.execute(board) + context 'when board lists is empty' do + it 'creates a new list at beginning of the list' do + list = service.execute(board) - expect(list.position).to eq 0 + expect(list.position).to eq 0 + end end - end - context 'when board lists has the done list' do - it 'creates a new list at beginning of the list' do - list = service.execute(board) + context 'when board lists has the done list' do + it 'creates a new list at beginning of the list' do + list = service.execute(board) - expect(list.position).to eq 0 + expect(list.position).to eq 0 + end end - end - context 'when board lists has labels lists' do - it 'creates a new list at end of the lists' do - create(:list, board: board, position: 0) - create(:list, board: board, position: 1) + context 'when board lists has labels lists' do + it 'creates a new list at end of the lists' do + create(:list, board: board, position: 0) + create(:list, board: board, position: 1) - list = service.execute(board) + list = service.execute(board) - expect(list.position).to eq 2 + expect(list.position).to eq 2 + end end - end - context 'when board lists has label and done lists' do - it 'creates a new list at end of the label lists' do - list1 = create(:list, board: board, position: 0) + context 'when board lists has label and done lists' do + it 'creates a new list at end of the label lists' do + list1 = create(:list, board: board, position: 0) - list2 = service.execute(board) + list2 = service.execute(board) - expect(list1.reload.position).to eq 0 - expect(list2.reload.position).to eq 1 + expect(list1.reload.position).to eq 0 + expect(list2.reload.position).to eq 1 + end end - end - context 'when provided label does not belongs to the project' do - it 'raises an error' do - label = create(:label, name: 'in-development') - service = described_class.new(project, user, label_id: label.id) + context 'when provided label does not belongs to the parent' do + it 'raises an error' do + label = create(:label, name: 'in-development') + service = described_class.new(parent, user, label_id: label.id) - expect { service.execute(board) }.to raise_error(ActiveRecord::RecordNotFound) + expect { service.execute(board) }.to raise_error(ActiveRecord::RecordNotFound) + end end end + + context 'when board parent is a project' do + let(:parent) { create(:project) } + let(:board) { create(:board, project: parent) } + let(:label) { create(:label, project: parent, name: 'in-progress') } + + it_behaves_like 'creating board lists' + end + + context 'when board parent is a group' do + let(:parent) { create(:group) } + let(:board) { create(:board, group: parent) } + let(:label) { create(:group_label, group: parent, name: 'in-progress') } + + it_behaves_like 'creating board lists' + end end end diff --git a/spec/services/boards/lists/destroy_service_spec.rb b/spec/services/boards/lists/destroy_service_spec.rb index bd98625b44f..3c4eb6b3fc5 100644 --- a/spec/services/boards/lists/destroy_service_spec.rb +++ b/spec/services/boards/lists/destroy_service_spec.rb @@ -2,37 +2,24 @@ require 'spec_helper' describe Boards::Lists::DestroyService do describe '#execute' do - let(:project) { create(:project) } - let(:board) { create(:board, project: project) } - let(:user) { create(:user) } + context 'when board parent is a project' do + let(:project) { create(:project) } + let(:board) { create(:board, project: project) } + let(:user) { create(:user) } - context 'when list type is label' do - it 'removes list from board' do - list = create(:list, board: board) - service = described_class.new(project, user) + let(:parent) { project } - expect { service.execute(list) }.to change(board.lists, :count).by(-1) - end - - it 'decrements position of higher lists' do - development = create(:list, board: board, position: 0) - review = create(:list, board: board, position: 1) - staging = create(:list, board: board, position: 2) - closed = board.closed_list - - described_class.new(project, user).execute(development) - - expect(review.reload.position).to eq 0 - expect(staging.reload.position).to eq 1 - expect(closed.reload.position).to be_nil - end + it_behaves_like 'lists destroy service' end - it 'does not remove list from board when list type is closed' do - list = board.closed_list - service = described_class.new(project, user) + context 'when board parent is a group' do + let(:group) { create(:group) } + let(:board) { create(:board, group: group) } + let(:user) { create(:user) } + + let(:parent) { group } - expect { service.execute(list) }.not_to change(board.lists, :count) + it_behaves_like 'lists destroy service' end end end diff --git a/spec/services/boards/lists/list_service_spec.rb b/spec/services/boards/lists/list_service_spec.rb index b189857e4f4..24e04eed642 100644 --- a/spec/services/boards/lists/list_service_spec.rb +++ b/spec/services/boards/lists/list_service_spec.rb @@ -1,33 +1,25 @@ require 'spec_helper' describe Boards::Lists::ListService do - let(:project) { create(:project) } - let(:board) { create(:board, project: project) } - let(:label) { create(:label, project: project) } - let!(:list) { create(:list, board: board, label: label) } - let(:service) { described_class.new(project, double) } - describe '#execute' do - context 'when the board has a backlog list' do - let!(:backlog_list) { create(:backlog_list, board: board) } - - it 'does not create a backlog list' do - expect { service.execute(board) }.not_to change(board.lists, :count) - end + context 'when board parent is a project' do + let(:project) { create(:project) } + let(:board) { create(:board, project: project) } + let(:label) { create(:label, project: project) } + let!(:list) { create(:list, board: board, label: label) } + let(:service) { described_class.new(project, double) } - it "returns board's lists" do - expect(service.execute(board)).to eq [backlog_list, list, board.closed_list] - end + it_behaves_like 'lists list service' end - context 'when the board does not have a backlog list' do - it 'creates a backlog list' do - expect { service.execute(board) }.to change(board.lists, :count).by(1) - end + context 'when board parent is a group' do + let(:group) { create(:group) } + let(:board) { create(:board, group: group) } + let(:label) { create(:group_label, group: group) } + let!(:list) { create(:list, board: board, label: label) } + let(:service) { described_class.new(group, double) } - it "returns board's lists" do - expect(service.execute(board)).to eq [board.backlog_list, list, board.closed_list] - end + it_behaves_like 'lists list service' end end end diff --git a/spec/services/boards/lists/move_service_spec.rb b/spec/services/boards/lists/move_service_spec.rb index a9d218bad49..16dfb2ae6af 100644 --- a/spec/services/boards/lists/move_service_spec.rb +++ b/spec/services/boards/lists/move_service_spec.rb @@ -2,100 +2,24 @@ require 'spec_helper' describe Boards::Lists::MoveService do describe '#execute' do - let(:project) { create(:project) } - let(:board) { create(:board, project: project) } - let(:user) { create(:user) } + context 'when board parent is a project' do + let(:project) { create(:project) } + let(:board) { create(:board, project: project) } + let(:user) { create(:user) } - let!(:planning) { create(:list, board: board, position: 0) } - let!(:development) { create(:list, board: board, position: 1) } - let!(:review) { create(:list, board: board, position: 2) } - let!(:staging) { create(:list, board: board, position: 3) } - let!(:closed) { create(:closed_list, board: board) } + let(:parent) { project } - context 'when list type is set to label' do - it 'keeps position of lists when new position is nil' do - service = described_class.new(project, user, position: nil) - - service.execute(planning) - - expect(current_list_positions).to eq [0, 1, 2, 3] - end - - it 'keeps position of lists when new positon is equal to old position' do - service = described_class.new(project, user, position: planning.position) - - service.execute(planning) - - expect(current_list_positions).to eq [0, 1, 2, 3] - end - - it 'keeps position of lists when new positon is negative' do - service = described_class.new(project, user, position: -1) - - service.execute(planning) - - expect(current_list_positions).to eq [0, 1, 2, 3] - end - - it 'keeps position of lists when new positon is equal to number of labels lists' do - service = described_class.new(project, user, position: board.lists.label.size) - - service.execute(planning) - - expect(current_list_positions).to eq [0, 1, 2, 3] - end - - it 'keeps position of lists when new positon is greater than number of labels lists' do - service = described_class.new(project, user, position: board.lists.label.size + 1) - - service.execute(planning) - - expect(current_list_positions).to eq [0, 1, 2, 3] - end - - it 'increments position of intermediate lists when new positon is equal to first position' do - service = described_class.new(project, user, position: 0) - - service.execute(staging) - - expect(current_list_positions).to eq [1, 2, 3, 0] - end - - it 'decrements position of intermediate lists when new positon is equal to last position' do - service = described_class.new(project, user, position: board.lists.label.last.position) - - service.execute(planning) - - expect(current_list_positions).to eq [3, 0, 1, 2] - end - - it 'decrements position of intermediate lists when new position is greater than old position' do - service = described_class.new(project, user, position: 2) - - service.execute(planning) - - expect(current_list_positions).to eq [2, 0, 1, 3] - end - - it 'increments position of intermediate lists when new position is lower than old position' do - service = described_class.new(project, user, position: 1) - - service.execute(staging) - - expect(current_list_positions).to eq [0, 2, 3, 1] - end + it_behaves_like 'lists move service' end - it 'keeps position of lists when list type is closed' do - service = described_class.new(project, user, position: 2) + context 'when board parent is a group' do + let(:group) { create(:group) } + let(:board) { create(:board, group: group) } + let(:user) { create(:user) } - service.execute(closed) + let(:parent) { group } - expect(current_list_positions).to eq [0, 1, 2, 3] + it_behaves_like 'lists move service' end end - - def current_list_positions - [planning, development, review, staging].map { |list| list.reload.position } - end end diff --git a/spec/support/shared_examples/services/boards/boards_create_service.rb b/spec/support/shared_examples/services/boards/boards_create_service.rb new file mode 100644 index 00000000000..5bdc04f660f --- /dev/null +++ b/spec/support/shared_examples/services/boards/boards_create_service.rb @@ -0,0 +1,27 @@ +shared_examples 'boards create service' do + context 'when parent does not have a board' do + it 'creates a new board' do + expect { service.execute }.to change(Board, :count).by(1) + end + + it 'creates the default lists' do + board = service.execute + + expect(board.lists.size).to eq 2 + expect(board.lists.first).to be_backlog + expect(board.lists.last).to be_closed + end + end + + context 'when parent has a board' do + before do + create(:board, parent: parent) + end + + it 'does not create a new board' do + expect(service).to receive(:can_create_board?) { false } + + expect { service.execute }.not_to change(parent.boards, :count) + end + end +end diff --git a/spec/support/shared_examples/services/boards/boards_list_service.rb b/spec/support/shared_examples/services/boards/boards_list_service.rb new file mode 100644 index 00000000000..e0d5a7c61f2 --- /dev/null +++ b/spec/support/shared_examples/services/boards/boards_list_service.rb @@ -0,0 +1,29 @@ +shared_examples 'boards list service' do + context 'when parent does not have a board' do + it 'creates a new parent board' do + expect { service.execute }.to change(parent.boards, :count).by(1) + end + + it 'delegates the parent board creation to Boards::CreateService' do + expect_any_instance_of(Boards::CreateService).to receive(:execute).once + + service.execute + end + end + + context 'when parent has a board' do + before do + create(:board, parent: parent) + end + + it 'does not create a new board' do + expect { service.execute }.not_to change(parent.boards, :count) + end + end + + it 'returns parent boards' do + board = create(:board, parent: parent) + + expect(service.execute).to eq [board] + end +end diff --git a/spec/support/shared_examples/services/boards/issues_list_service.rb b/spec/support/shared_examples/services/boards/issues_list_service.rb new file mode 100644 index 00000000000..3e744323cea --- /dev/null +++ b/spec/support/shared_examples/services/boards/issues_list_service.rb @@ -0,0 +1,60 @@ +shared_examples 'issues list service' do + it 'delegates search to IssuesFinder' do + params = { board_id: board.id, id: list1.id } + + expect_any_instance_of(IssuesFinder).to receive(:execute).once.and_call_original + + described_class.new(parent, user, params).execute + end + + context 'issues are ordered by priority' do + it 'returns opened issues when list_id is missing' do + params = { board_id: board.id } + + issues = described_class.new(parent, user, params).execute + + expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1] + end + + it 'returns opened issues when listing issues from Backlog' do + params = { board_id: board.id, id: backlog.id } + + issues = described_class.new(parent, user, params).execute + + expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1] + end + + it 'returns closed issues when listing issues from Closed' do + params = { board_id: board.id, id: closed.id } + + issues = described_class.new(parent, user, params).execute + + expect(issues).to eq [closed_issue4, closed_issue2, closed_issue5, closed_issue3, closed_issue1] + end + + it 'returns opened issues that have label list applied when listing issues from a label list' do + params = { board_id: board.id, id: list1.id } + + issues = described_class.new(parent, user, params).execute + + expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2] + end + end + + context 'with list that does not belong to the board' do + it 'raises an error' do + list = create(:list) + service = described_class.new(parent, user, board_id: board.id, id: list.id) + + expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'with invalid list id' do + it 'raises an error' do + service = described_class.new(parent, user, board_id: board.id, id: nil) + + expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) + end + end +end diff --git a/spec/support/shared_examples/services/boards/issues_move_service.rb b/spec/support/shared_examples/services/boards/issues_move_service.rb new file mode 100644 index 00000000000..4a4fbaa3a0e --- /dev/null +++ b/spec/support/shared_examples/services/boards/issues_move_service.rb @@ -0,0 +1,87 @@ +shared_examples 'issues move service' do + context 'when moving an issue between lists' do + let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } + let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list2.id } } + + it 'delegates the label changes to Issues::UpdateService' do + expect_any_instance_of(Issues::UpdateService).to receive(:execute).with(issue).once + + described_class.new(parent, user, params).execute(issue) + end + + it 'removes the label from the list it came from and adds the label of the list it goes to' do + described_class.new(parent, user, params).execute(issue) + + expect(issue.reload.labels).to contain_exactly(bug, testing) + end + end + + context 'when moving to closed' do + let!(:list3) { create(:list, board: board2, label: regression, position: 1) } + + let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing, regression]) } + let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: closed.id } } + + it 'delegates the close proceedings to Issues::CloseService' do + expect_any_instance_of(Issues::CloseService).to receive(:execute).with(issue).once + + described_class.new(parent, user, params).execute(issue) + end + + it 'removes all list-labels from boards and close the issue' do + described_class.new(parent, user, params).execute(issue) + issue.reload + + expect(issue.labels).to contain_exactly(bug) + expect(issue).to be_closed + end + end + + context 'when moving from closed' do + let(:issue) { create(:labeled_issue, :closed, project: project, labels: [bug]) } + let(:params) { { board_id: board1.id, from_list_id: closed.id, to_list_id: list2.id } } + + it 'delegates the re-open proceedings to Issues::ReopenService' do + expect_any_instance_of(Issues::ReopenService).to receive(:execute).with(issue).once + + described_class.new(parent, user, params).execute(issue) + end + + it 'adds the label of the list it goes to and reopen the issue' do + described_class.new(parent, user, params).execute(issue) + issue.reload + + expect(issue.labels).to contain_exactly(bug, testing) + expect(issue).to be_opened + end + end + + context 'when moving to same list' do + let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } + let(:issue1) { create(:labeled_issue, project: project, labels: [bug, development]) } + let(:issue2) { create(:labeled_issue, project: project, labels: [bug, development]) } + let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } } + + it 'returns false' do + expect(described_class.new(parent, user, params).execute(issue)).to eq false + end + + it 'keeps issues labels' do + described_class.new(parent, user, params).execute(issue) + + expect(issue.reload.labels).to contain_exactly(bug, development) + end + + it 'sorts issues' do + [issue, issue1, issue2].each do |issue| + issue.move_to_end && issue.save! + end + + params.merge!(move_after_id: issue1.id, move_before_id: issue2.id) + + described_class.new(parent, user, params).execute(issue) + + expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) + end + end +end diff --git a/spec/support/shared_examples/services/boards/lists_destroy_service.rb b/spec/support/shared_examples/services/boards/lists_destroy_service.rb new file mode 100644 index 00000000000..62b6ffe1836 --- /dev/null +++ b/spec/support/shared_examples/services/boards/lists_destroy_service.rb @@ -0,0 +1,30 @@ +shared_examples 'lists destroy service' do + context 'when list type is label' do + it 'removes list from board' do + list = create(:list, board: board) + service = described_class.new(parent, user) + + expect { service.execute(list) }.to change(board.lists, :count).by(-1) + end + + it 'decrements position of higher lists' do + development = create(:list, board: board, position: 0) + review = create(:list, board: board, position: 1) + staging = create(:list, board: board, position: 2) + closed = board.closed_list + + described_class.new(parent, user).execute(development) + + expect(review.reload.position).to eq 0 + expect(staging.reload.position).to eq 1 + expect(closed.reload.position).to be_nil + end + end + + it 'does not remove list from board when list type is closed' do + list = board.closed_list + service = described_class.new(parent, user) + + expect { service.execute(list) }.not_to change(board.lists, :count) + end +end diff --git a/spec/support/shared_examples/services/boards/lists_list_service.rb b/spec/support/shared_examples/services/boards/lists_list_service.rb new file mode 100644 index 00000000000..0a8220111ab --- /dev/null +++ b/spec/support/shared_examples/services/boards/lists_list_service.rb @@ -0,0 +1,23 @@ +shared_examples 'lists list service' do + context 'when the board has a backlog list' do + let!(:backlog_list) { create(:backlog_list, board: board) } + + it 'does not create a backlog list' do + expect { service.execute(board) }.not_to change(board.lists, :count) + end + + it "returns board's lists" do + expect(service.execute(board)).to eq [backlog_list, list, board.closed_list] + end + end + + context 'when the board does not have a backlog list' do + it 'creates a backlog list' do + expect { service.execute(board) }.to change(board.lists, :count).by(1) + end + + it "returns board's lists" do + expect(service.execute(board)).to eq [board.backlog_list, list, board.closed_list] + end + end +end diff --git a/spec/support/shared_examples/services/boards/lists_move_service.rb b/spec/support/shared_examples/services/boards/lists_move_service.rb new file mode 100644 index 00000000000..07c98cb29b7 --- /dev/null +++ b/spec/support/shared_examples/services/boards/lists_move_service.rb @@ -0,0 +1,93 @@ +shared_examples 'lists move service' do + let!(:planning) { create(:list, board: board, position: 0) } + let!(:development) { create(:list, board: board, position: 1) } + let!(:review) { create(:list, board: board, position: 2) } + let!(:staging) { create(:list, board: board, position: 3) } + let!(:closed) { create(:closed_list, board: board) } + + context 'when list type is set to label' do + it 'keeps position of lists when new position is nil' do + service = described_class.new(parent, user, position: nil) + + service.execute(planning) + + expect(current_list_positions).to eq [0, 1, 2, 3] + end + + it 'keeps position of lists when new positon is equal to old position' do + service = described_class.new(parent, user, position: planning.position) + + service.execute(planning) + + expect(current_list_positions).to eq [0, 1, 2, 3] + end + + it 'keeps position of lists when new positon is negative' do + service = described_class.new(parent, user, position: -1) + + service.execute(planning) + + expect(current_list_positions).to eq [0, 1, 2, 3] + end + + it 'keeps position of lists when new positon is equal to number of labels lists' do + service = described_class.new(parent, user, position: board.lists.label.size) + + service.execute(planning) + + expect(current_list_positions).to eq [0, 1, 2, 3] + end + + it 'keeps position of lists when new positon is greater than number of labels lists' do + service = described_class.new(parent, user, position: board.lists.label.size + 1) + + service.execute(planning) + + expect(current_list_positions).to eq [0, 1, 2, 3] + end + + it 'increments position of intermediate lists when new positon is equal to first position' do + service = described_class.new(parent, user, position: 0) + + service.execute(staging) + + expect(current_list_positions).to eq [1, 2, 3, 0] + end + + it 'decrements position of intermediate lists when new positon is equal to last position' do + service = described_class.new(parent, user, position: board.lists.label.last.position) + + service.execute(planning) + + expect(current_list_positions).to eq [3, 0, 1, 2] + end + + it 'decrements position of intermediate lists when new position is greater than old position' do + service = described_class.new(parent, user, position: 2) + + service.execute(planning) + + expect(current_list_positions).to eq [2, 0, 1, 3] + end + + it 'increments position of intermediate lists when new position is lower than old position' do + service = described_class.new(parent, user, position: 1) + + service.execute(staging) + + expect(current_list_positions).to eq [0, 2, 3, 1] + end + end + + it 'keeps position of lists when list type is closed' do + service = described_class.new(parent, user, position: 2) + + service.execute(closed) + + expect(current_list_positions).to eq [0, 1, 2, 3] + end + + def current_list_positions + [planning, development, review, staging].map { |list| list.reload.position } + end +end diff --git a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb index c5f455b8948..f28bf430f02 100644 --- a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb +++ b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb @@ -12,7 +12,7 @@ describe 'layouts/nav/sidebar/_project' do end describe 'issue boards' do - it 'has boards tab when multiple issue boards available' do + it 'has board tab' do render expect(rendered).to have_css('a[title="Board"]') |