From 90c60138db4e1f86026aac5760febe4ba066ca30 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 14 Aug 2017 02:26:19 -0500 Subject: Move "Move to different project" to sidebar Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/34261 --- app/assets/javascripts/gl_dropdown.js | 8 +- app/assets/javascripts/issuable_form.js | 52 ----- .../javascripts/issue_show/components/app.vue | 21 --- .../issue_show/components/fields/project_move.vue | 83 -------- .../javascripts/issue_show/components/form.vue | 14 -- app/assets/javascripts/issue_show/index.js | 2 - app/assets/javascripts/issue_show/stores/index.js | 1 - app/assets/javascripts/right_sidebar.js | 9 +- .../sidebar/components/assignees/assignee_title.js | 2 +- .../javascripts/sidebar/lib/sidebar_move_issue.js | 85 +++++++++ .../sidebar/services/sidebar_service.js | 20 +- app/assets/javascripts/sidebar/sidebar_bundle.js | 7 + app/assets/javascripts/sidebar/sidebar_mediator.js | 29 ++- .../javascripts/sidebar/stores/sidebar_store.js | 10 + app/assets/stylesheets/framework/dropdowns.scss | 7 +- app/assets/stylesheets/pages/issuable.scss | 20 +- app/controllers/autocomplete_controller.rb | 6 - app/controllers/projects/issues_controller.rb | 40 ++-- app/helpers/dropdowns_helper.rb | 6 +- app/helpers/issuables_helper.rb | 4 +- .../boards/components/sidebar/_due_date.html.haml | 2 +- .../boards/components/sidebar/_labels.html.haml | 2 +- .../boards/components/sidebar/_milestone.html.haml | 2 +- app/views/shared/icons/_icon_arrow_right.svg.erb | 1 + app/views/shared/issuable/_form.html.haml | 12 -- app/views/shared/issuable/_sidebar.html.haml | 23 ++- .../shared/issuable/_sidebar_assignees.html.haml | 2 +- .../shared/issuable/form/_issue_assignee.html.haml | 2 +- .../form/_merge_request_assignee.html.haml | 2 +- app/views/shared/milestones/_sidebar.html.haml | 4 +- .../unreleased/34261-move-move-to-sidebar.yml | 5 + config/routes/project.rb | 1 + doc/user/project/issues/img/sidebar_move_issue.png | Bin 0 -> 54511 bytes doc/user/project/issues/index.md | 4 + doc/user/project/issues/moving_issues.md | 10 + spec/controllers/autocomplete_controller_spec.rb | 28 ++- .../controllers/projects/issues_controller_spec.rb | 209 +++++++++++---------- spec/features/issues/move_spec.rb | 32 ++-- spec/javascripts/issue_show/components/app_spec.js | 21 +-- .../components/fields/project_move_spec.js | 38 ---- .../javascripts/issue_show/components/form_spec.js | 2 - spec/javascripts/sidebar/mock_data.js | 41 ++++ spec/javascripts/sidebar/sidebar_mediator_spec.js | 40 +++- .../javascripts/sidebar/sidebar_move_issue_spec.js | 142 ++++++++++++++ spec/javascripts/sidebar/sidebar_service_spec.js | 28 ++- spec/javascripts/sidebar/sidebar_store_spec.js | 14 ++ 46 files changed, 664 insertions(+), 429 deletions(-) delete mode 100644 app/assets/javascripts/issue_show/components/fields/project_move.vue create mode 100644 app/assets/javascripts/sidebar/lib/sidebar_move_issue.js create mode 100644 app/views/shared/icons/_icon_arrow_right.svg.erb create mode 100644 changelogs/unreleased/34261-move-move-to-sidebar.yml create mode 100644 doc/user/project/issues/img/sidebar_move_issue.png create mode 100644 doc/user/project/issues/moving_issues.md delete mode 100644 spec/javascripts/issue_show/components/fields/project_move_spec.js create mode 100644 spec/javascripts/sidebar/sidebar_move_issue_spec.js diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index b62acfcd445..d65bbc0d808 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -486,7 +486,7 @@ GitLabDropdown = (function() { GitLabDropdown.prototype.shouldPropagate = function(e) { var $target; - if (this.options.multiSelect) { + if (this.options.multiSelect || this.options.shouldPropagate === false) { $target = $(e.target); if ($target && !$target.hasClass('dropdown-menu-close') && !$target.hasClass('dropdown-menu-close-icon') && @@ -546,10 +546,10 @@ GitLabDropdown = (function() { }; GitLabDropdown.prototype.positionMenuAbove = function() { - var $button = $(this.el); var $menu = this.dropdown.find('.dropdown-menu'); - $menu.css('top', ($button.height() + $menu.height()) * -1); + $menu.css('top', 'initial'); + $menu.css('bottom', '100%'); }; GitLabDropdown.prototype.hidden = function(e) { @@ -698,7 +698,7 @@ GitLabDropdown = (function() { GitLabDropdown.prototype.noResults = function() { var html; - return html = ""; + return html = ''; }; GitLabDropdown.prototype.rowClicked = function(el) { diff --git a/app/assets/javascripts/issuable_form.js b/app/assets/javascripts/issuable_form.js index 3f848e0859b..470c39c6f76 100644 --- a/app/assets/javascripts/issuable_form.js +++ b/app/assets/javascripts/issuable_form.js @@ -10,8 +10,6 @@ import ZenMode from './zen_mode'; (function() { this.IssuableForm = (function() { - IssuableForm.prototype.issueMoveConfirmMsg = 'Are you sure you want to move this issue to another project?'; - IssuableForm.prototype.wipRegex = /^\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i; function IssuableForm(form) { @@ -26,7 +24,6 @@ import ZenMode from './zen_mode'; new ZenMode(); this.titleField = this.form.find("input[name*='[title]']"); this.descriptionField = this.form.find("textarea[name*='[description]']"); - this.issueMoveField = this.form.find("#move_to_project_id"); if (!(this.titleField.length && this.descriptionField.length)) { return; } @@ -34,7 +31,6 @@ import ZenMode from './zen_mode'; this.form.on("submit", this.handleSubmit); this.form.on("click", ".btn-cancel", this.resetAutosave); this.initWip(); - this.initMoveDropdown(); $issuableDueDate = $('#issuable-due-date'); if ($issuableDueDate.length) { calendar = new Pikaday({ @@ -56,12 +52,6 @@ import ZenMode from './zen_mode'; }; IssuableForm.prototype.handleSubmit = function() { - var fieldId = (this.issueMoveField != null) ? this.issueMoveField.val() : null; - if ((parseInt(fieldId, 10) || 0) > 0) { - if (!confirm(this.issueMoveConfirmMsg)) { - return false; - } - } return this.resetAutosave(); }; @@ -113,48 +103,6 @@ import ZenMode from './zen_mode'; return this.titleField.val("WIP: " + (this.titleField.val())); }; - IssuableForm.prototype.initMoveDropdown = function() { - var $moveDropdown, pageSize; - $moveDropdown = $('.js-move-dropdown'); - if ($moveDropdown.length) { - pageSize = $moveDropdown.data('page-size'); - return $('.js-move-dropdown').select2({ - ajax: { - url: $moveDropdown.data('projects-url'), - quietMillis: 125, - data: function(term, page, context) { - return { - search: term, - offset_id: context - }; - }, - results: function(data) { - var context, - more; - - if (data.length >= pageSize) - more = true; - - if (data[data.length - 1]) - context = data[data.length - 1].id; - - return { - results: data, - more: more, - context: context - }; - } - }, - formatResult: function(project) { - return project.name_with_namespace; - }, - formatSelection: function(project) { - return project.name_with_namespace; - } - }); - } - }; - return IssuableForm; })(); }).call(window); diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index eaaafd4c149..e115ee40219 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -17,10 +17,6 @@ export default { required: true, type: String, }, - canMove: { - required: true, - type: Boolean, - }, canUpdate: { required: true, type: Boolean, @@ -96,10 +92,6 @@ export default { type: String, required: true, }, - projectsAutocompletePath: { - type: String, - required: true, - }, }, data() { const store = new Store({ @@ -142,7 +134,6 @@ export default { confidential: this.isConfidential, description: this.state.descriptionText, lockedWarningVisible: false, - move_to_project_id: 0, updateLoading: false, }); } @@ -151,16 +142,6 @@ export default { this.showForm = false; }, updateIssuable() { - const canPostUpdate = this.store.formState.move_to_project_id !== 0 ? - confirm('Are you sure you want to move this issue to another project?') : true; // eslint-disable-line no-alert - - if (!canPostUpdate) { - this.store.setFormState({ - updateLoading: false, - }); - return; - } - this.service.updateIssuable(this.store.formState) .then(res => res.json()) .then((data) => { @@ -239,14 +220,12 @@ export default {
- import tooltip from '../../../vue_shared/directives/tooltip'; - - export default { - directives: { - tooltip, - }, - props: { - formState: { - type: Object, - required: true, - }, - projectsAutocompletePath: { - type: String, - required: true, - }, - }, - mounted() { - const $moveDropdown = $(this.$refs['move-dropdown']); - - $moveDropdown.select2({ - ajax: { - url: this.projectsAutocompletePath, - quietMillis: 125, - data(term, page, context) { - return { - search: term, - offset_id: context, - }; - }, - results(data) { - const more = data.length >= 50; - const context = data[data.length - 1] ? data[data.length - 1].id : null; - - return { - results: data, - more, - context, - }; - }, - }, - formatResult(project) { - return project.name_with_namespace; - }, - formatSelection(project) { - return project.name_with_namespace; - }, - }) - .on('change', (e) => { - this.formState.move_to_project_id = parseInt(e.target.value, 10); - }); - }, - beforeDestroy() { - $(this.$refs['move-dropdown']).select2('destroy'); - }, - }; - - - diff --git a/app/assets/javascripts/issue_show/components/form.vue b/app/assets/javascripts/issue_show/components/form.vue index d9b53bc55cf..6a2dd502fe2 100644 --- a/app/assets/javascripts/issue_show/components/form.vue +++ b/app/assets/javascripts/issue_show/components/form.vue @@ -4,15 +4,10 @@ import descriptionField from './fields/description.vue'; import editActions from './edit_actions.vue'; import descriptionTemplate from './fields/description_template.vue'; - import projectMove from './fields/project_move.vue'; import confidentialCheckbox from './fields/confidential_checkbox.vue'; export default { props: { - canMove: { - type: Boolean, - required: true, - }, canDestroy: { type: Boolean, required: true, @@ -42,10 +37,6 @@ type: String, required: true, }, - projectsAutocompletePath: { - type: String, - required: true, - }, }, components: { lockedWarning, @@ -53,7 +44,6 @@ descriptionField, descriptionTemplate, editActions, - projectMove, confidentialCheckbox, }, computed: { @@ -93,10 +83,6 @@ :markdown-docs-path="markdownDocsPath" /> - diff --git a/app/assets/javascripts/issue_show/index.js b/app/assets/javascripts/issue_show/index.js index 60b69b300fd..8053ef57e6c 100644 --- a/app/assets/javascripts/issue_show/index.js +++ b/app/assets/javascripts/issue_show/index.js @@ -28,7 +28,6 @@ document.addEventListener('DOMContentLoaded', () => { props: { canUpdate: this.canUpdate, canDestroy: this.canDestroy, - canMove: this.canMove, endpoint: this.endpoint, issuableRef: this.issuableRef, initialTitleHtml: this.initialTitleHtml, @@ -41,7 +40,6 @@ document.addEventListener('DOMContentLoaded', () => { markdownDocsPath: this.markdownDocsPath, projectPath: this.projectPath, projectNamespace: this.projectNamespace, - projectsAutocompletePath: this.projectsAutocompletePath, updatedAt: this.updatedAt, updatedByName: this.updatedByName, updatedByPath: this.updatedByPath, diff --git a/app/assets/javascripts/issue_show/stores/index.js b/app/assets/javascripts/issue_show/stores/index.js index 0c8bd6f1cc3..f4639e9ed2a 100644 --- a/app/assets/javascripts/issue_show/stores/index.js +++ b/app/assets/javascripts/issue_show/stores/index.js @@ -6,7 +6,6 @@ export default class Store { confidential: false, description: '', lockedWarningVisible: false, - move_to_project_id: 0, updateLoading: false, }; } diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index fa958d75fa4..4c87d46c96e 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -157,11 +157,16 @@ import SidebarHeightManager from './sidebar_height_manager'; Sidebar.prototype.openDropdown = function(blockOrName) { var $block; $block = _.isString(blockOrName) ? this.getBlock(blockOrName) : blockOrName; - $block.find('.edit-link').trigger('click'); if (!this.isOpen()) { this.setCollapseAfterUpdate($block); - return this.toggleSidebar('open'); + this.toggleSidebar('open'); } + + // Wait for the sidebar to trigger('click') open + // so it doesn't cause our dropdown to close preemptively + setTimeout(() => { + $block.find('.js-sidebar-dropdown-toggle').trigger('click'); + }); }; Sidebar.prototype.setCollapseAfterUpdate = function($block) { diff --git a/app/assets/javascripts/sidebar/components/assignees/assignee_title.js b/app/assets/javascripts/sidebar/components/assignees/assignee_title.js index 5a6e47e566e..77f070d48cc 100644 --- a/app/assets/javascripts/sidebar/components/assignees/assignee_title.js +++ b/app/assets/javascripts/sidebar/components/assignees/assignee_title.js @@ -36,7 +36,7 @@ export default { /> Edit diff --git a/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js b/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js new file mode 100644 index 00000000000..1c15a1b877a --- /dev/null +++ b/app/assets/javascripts/sidebar/lib/sidebar_move_issue.js @@ -0,0 +1,85 @@ +/* global Flash */ + +function isValidProjectId(id) { + return id > 0; +} + +class SidebarMoveIssue { + constructor(mediator, dropdownToggle, confirmButton) { + this.mediator = mediator; + + this.$dropdownToggle = $(dropdownToggle); + this.$confirmButton = $(confirmButton); + + this.onConfirmClickedWrapper = this.onConfirmClicked.bind(this); + } + + init() { + this.initDropdown(); + this.addEventListeners(); + } + + destroy() { + this.removeEventListeners(); + } + + initDropdown() { + this.$dropdownToggle.glDropdown({ + search: { + fields: ['name_with_namespace'], + }, + showMenuAbove: true, + selectable: true, + filterable: true, + filterRemote: true, + multiSelect: false, + // Keep the dropdown open after selecting an option + shouldPropagate: false, + data: (searchTerm, callback) => { + this.mediator.fetchAutocompleteProjects(searchTerm) + .then(callback) + .catch(() => new Flash('An error occured while fetching projects autocomplete.')); + }, + renderRow: project => ` +
  • + + ${project.name_with_namespace} + +
  • + `, + clicked: (options) => { + const project = options.selectedObj; + const selectedProjectId = options.isMarking ? project.id : 0; + this.mediator.setMoveToProjectId(selectedProjectId); + + this.$confirmButton.attr('disabled', !isValidProjectId(selectedProjectId)); + }, + }); + } + + addEventListeners() { + this.$confirmButton.on('click', this.onConfirmClickedWrapper); + } + + removeEventListeners() { + this.$confirmButton.off('click', this.onConfirmClickedWrapper); + } + + onConfirmClicked() { + if (isValidProjectId(this.mediator.store.moveToProjectId)) { + this.$confirmButton + .disable() + .addClass('is-loading'); + + this.mediator.moveIssue() + .catch(() => { + Flash('An error occured while moving the issue.'); + this.$confirmButton + .enable() + .removeClass('is-loading'); + }); + } + } +} + +export default SidebarMoveIssue; diff --git a/app/assets/javascripts/sidebar/services/sidebar_service.js b/app/assets/javascripts/sidebar/services/sidebar_service.js index 5a82d01dc41..604648407a4 100644 --- a/app/assets/javascripts/sidebar/services/sidebar_service.js +++ b/app/assets/javascripts/sidebar/services/sidebar_service.js @@ -4,9 +4,11 @@ import VueResource from 'vue-resource'; Vue.use(VueResource); export default class SidebarService { - constructor(endpoint) { + constructor(endpointMap) { if (!SidebarService.singleton) { - this.endpoint = endpoint; + this.endpoint = endpointMap.endpoint; + this.moveIssueEndpoint = endpointMap.moveIssueEndpoint; + this.projectsAutocompleteEndpoint = endpointMap.projectsAutocompleteEndpoint; SidebarService.singleton = this; } @@ -25,4 +27,18 @@ export default class SidebarService { emulateJSON: true, }); } + + getProjectsAutocomplete(searchTerm) { + return Vue.http.get(this.projectsAutocompleteEndpoint, { + params: { + search: searchTerm, + }, + }); + } + + moveIssue(moveToProjectId) { + return Vue.http.post(this.moveIssueEndpoint, { + move_to_project_id: moveToProjectId, + }); + } } diff --git a/app/assets/javascripts/sidebar/sidebar_bundle.js b/app/assets/javascripts/sidebar/sidebar_bundle.js index 9edded3ead6..3d8972050a9 100644 --- a/app/assets/javascripts/sidebar/sidebar_bundle.js +++ b/app/assets/javascripts/sidebar/sidebar_bundle.js @@ -2,6 +2,7 @@ import Vue from 'vue'; import sidebarTimeTracking from './components/time_tracking/sidebar_time_tracking'; import sidebarAssignees from './components/assignees/sidebar_assignees'; import confidential from './components/confidential/confidential_issue_sidebar.vue'; +import SidebarMoveIssue from './lib/sidebar_move_issue'; import Mediator from './sidebar_mediator'; @@ -31,6 +32,12 @@ function domContentLoaded() { service: mediator.service, }, }).$mount(confidentialEl); + + new SidebarMoveIssue( + mediator, + $('.js-move-issue'), + $('.js-move-issue-confirmation-button'), + ).init(); } new Vue(sidebarTimeTracking).$mount('#issuable-time-tracker'); diff --git a/app/assets/javascripts/sidebar/sidebar_mediator.js b/app/assets/javascripts/sidebar/sidebar_mediator.js index 721e92221cf..e38a8db4cc5 100644 --- a/app/assets/javascripts/sidebar/sidebar_mediator.js +++ b/app/assets/javascripts/sidebar/sidebar_mediator.js @@ -7,7 +7,11 @@ export default class SidebarMediator { constructor(options) { if (!SidebarMediator.singleton) { this.store = new Store(options); - this.service = new Service(options.endpoint); + this.service = new Service({ + endpoint: options.endpoint, + moveIssueEndpoint: options.moveIssueEndpoint, + projectsAutocompleteEndpoint: options.projectsAutocompleteEndpoint, + }); SidebarMediator.singleton = this; } @@ -26,6 +30,10 @@ export default class SidebarMediator { return this.service.update(field, selected.length === 0 ? [0] : selected); } + setMoveToProjectId(projectId) { + this.store.setMoveToProjectId(projectId); + } + fetch() { this.service.get() .then(response => response.json()) @@ -35,4 +43,23 @@ export default class SidebarMediator { }) .catch(() => new Flash('Error occured when fetching sidebar data')); } + + fetchAutocompleteProjects(searchTerm) { + return this.service.getProjectsAutocomplete(searchTerm) + .then(response => response.json()) + .then((data) => { + this.store.setAutocompleteProjects(data); + return this.store.autocompleteProjects; + }); + } + + moveIssue() { + return this.service.moveIssue(this.store.moveToProjectId) + .then(response => response.json()) + .then((data) => { + if (location.pathname !== data.web_url) { + gl.utils.visitUrl(data.web_url); + } + }); + } } diff --git a/app/assets/javascripts/sidebar/stores/sidebar_store.js b/app/assets/javascripts/sidebar/stores/sidebar_store.js index 3356dd0191f..cc04a2a3fcf 100644 --- a/app/assets/javascripts/sidebar/stores/sidebar_store.js +++ b/app/assets/javascripts/sidebar/stores/sidebar_store.js @@ -13,6 +13,8 @@ export default class SidebarStore { this.isFetching = { assignees: true, }; + this.autocompleteProjects = []; + this.moveToProjectId = 0; SidebarStore.singleton = this; } @@ -53,4 +55,12 @@ export default class SidebarStore { removeAllAssignees() { this.assignees = []; } + + setAutocompleteProjects(projects) { + this.autocompleteProjects = projects; + } + + setMoveToProjectId(moveToProjectId) { + this.moveToProjectId = moveToProjectId; + } } diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 110b171676a..487b3148b14 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -193,7 +193,7 @@ min-width: 240px; max-width: 500px; margin-top: 2px; - margin-bottom: 0; + margin-bottom: 2px; font-size: 14px; font-weight: $gl-font-weight-normal; padding: 8px 0; @@ -622,6 +622,11 @@ border-top: 1px solid $dropdown-divider-color; } +.dropdown-footer-content { + padding-left: 10px; + padding-right: 10px; +} + .dropdown-due-date-footer { padding-top: 0; margin-left: 10px; diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 4b0b238a767..6523376ccc3 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -473,7 +473,7 @@ padding-top: 6px; } - .open .dropdown-menu { + .dropdown-menu { width: 100%; } } @@ -486,6 +486,24 @@ } } +.sidebar-move-issue-dropdown { + @include new-style-dropdown; +} + +.sidebar-move-issue-confirmation-button { + width: 100%; + + &.is-loading { + .sidebar-move-issue-confirmation-loading-icon { + display: inline-block; + } + } +} + +.sidebar-move-issue-confirmation-loading-icon { + display: none; +} + .detail-page-description { padding: 16px 0; diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 59be955599d..dfc8bd0ba81 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -41,12 +41,6 @@ class AutocompleteController < ApplicationController project = Project.find_by_id(params[:project_id]) projects = projects_finder.execute(project, search: params[:search], offset_id: params[:offset_id]) - no_project = { - id: 0, - name_with_namespace: 'No project' - } - projects.unshift(no_project) unless params[:offset_id].present? - render json: projects.to_json(only: [:id, :name_with_namespace], methods: :name_with_namespace) end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 349b19f72e2..0d4266f0899 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -15,7 +15,7 @@ class Projects::IssuesController < Projects::ApplicationController before_action :authorize_create_issue!, only: [:new, :create] # Allow modify issue - before_action :authorize_update_issue!, only: [:edit, :update] + before_action :authorize_update_issue!, only: [:edit, :update, :move] # Allow create a new branch and empty WIP merge request from current issue before_action :authorize_create_merge_request!, only: [:create_merge_request] @@ -142,25 +142,33 @@ class Projects::IssuesController < Projects::ApplicationController @issue = Issues::UpdateService.new(project, current_user, update_params).execute(issue) + respond_to do |format| + format.html do + recaptcha_check_with_fallback { render :edit } + end + + format.json do + render_issue_json + end + end + + rescue ActiveRecord::StaleObjectError + render_conflict_response + end + + def move + params.require(:move_to_project_id) + if params[:move_to_project_id].to_i > 0 new_project = Project.find(params[:move_to_project_id]) return render_404 unless issue.can_move?(current_user, new_project) - move_service = Issues::MoveService.new(project, current_user) - @issue = move_service.execute(@issue, new_project) + @issue = Issues::UpdateService.new(project, current_user, target_project: new_project).execute(issue) end respond_to do |format| - format.html do - recaptcha_check_with_fallback { render :edit } - end - format.json do - if @issue.valid? - render json: serializer.represent(@issue) - else - render json: { errors: @issue.errors.full_messages }, status: :unprocessable_entity - end + render_issue_json end end @@ -271,6 +279,14 @@ class Projects::IssuesController < Projects::ApplicationController return render_404 unless @project.feature_available?(:issues, current_user) end + def render_issue_json + if @issue.valid? + render json: serializer.represent(@issue) + else + render json: { errors: @issue.errors.full_messages }, status: :unprocessable_entity + end + end + def issue_params params.require(:issue).permit(*issue_params_attributes) end diff --git a/app/helpers/dropdowns_helper.rb b/app/helpers/dropdowns_helper.rb index ff305fa39b4..5089da519df 100644 --- a/app/helpers/dropdowns_helper.rb +++ b/app/helpers/dropdowns_helper.rb @@ -97,9 +97,11 @@ module DropdownsHelper end end - def dropdown_footer(&block) + def dropdown_footer(add_content_class: false, &block) content_tag(:div, class: "dropdown-footer") do - if block + if add_content_class + content_tag(:div, capture(&block), class: "dropdown-footer-content") + else capture(&block) end end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 0fcd3347095..d81ba2c06eb 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -207,12 +207,10 @@ module IssuablesHelper endpoint: project_issue_path(@project, issuable), canUpdate: can?(current_user, :update_issue, issuable), canDestroy: can?(current_user, :destroy_issue, issuable), - canMove: current_user ? issuable.can_move?(current_user) : false, issuableRef: issuable.to_reference, isConfidential: issuable.confidential, markdownPreviewPath: preview_markdown_path(@project), markdownDocsPath: help_page_path('user/markdown'), - projectsAutocompletePath: autocomplete_projects_path(project_id: @project.id), issuableTemplates: issuable_templates(issuable), projectPath: ref_project.path, projectNamespace: ref_project.namespace.full_path, @@ -354,6 +352,8 @@ module IssuablesHelper def issuable_sidebar_options(issuable, can_edit_issuable) { endpoint: "#{issuable_json_path(issuable)}?basic=true", + moveIssueEndpoint: move_namespace_project_issue_path(namespace_id: issuable.project.namespace.to_param, project_id: issuable.project, id: issuable), + projectsAutocompleteEndpoint: autocomplete_projects_path(project_id: @project.id), editable: can_edit_issuable, currentUser: current_user.as_json(only: [:username, :id, :name], methods: :avatar_url), rootPath: root_path, diff --git a/app/views/projects/boards/components/sidebar/_due_date.html.haml b/app/views/projects/boards/components/sidebar/_due_date.html.haml index f44a9d49a54..e8394eab213 100644 --- a/app/views/projects/boards/components/sidebar/_due_date.html.haml +++ b/app/views/projects/boards/components/sidebar/_due_date.html.haml @@ -3,7 +3,7 @@ Due date - if can?(current_user, :admin_issue, @project) = icon("spinner spin", class: "block-loading") - = link_to "Edit", "#", class: "edit-link pull-right" + = link_to "Edit", "#", class: "js-sidebar-dropdown-toggle edit-link pull-right" .value .value-content %span.no-value{ "v-if" => "!issue.dueDate" } diff --git a/app/views/projects/boards/components/sidebar/_labels.html.haml b/app/views/projects/boards/components/sidebar/_labels.html.haml index 7d0c35fe183..6b389736e8b 100644 --- a/app/views/projects/boards/components/sidebar/_labels.html.haml +++ b/app/views/projects/boards/components/sidebar/_labels.html.haml @@ -3,7 +3,7 @@ Labels - if can?(current_user, :admin_issue, @project) = icon("spinner spin", class: "block-loading") - = link_to "Edit", "#", class: "edit-link pull-right" + = link_to "Edit", "#", class: "js-sidebar-dropdown-toggle edit-link pull-right" .value.issuable-show-labels %span.no-value{ "v-if" => "issue.labels && issue.labels.length === 0" } None diff --git a/app/views/projects/boards/components/sidebar/_milestone.html.haml b/app/views/projects/boards/components/sidebar/_milestone.html.haml index 002e9994ee0..a1ddb261ea3 100644 --- a/app/views/projects/boards/components/sidebar/_milestone.html.haml +++ b/app/views/projects/boards/components/sidebar/_milestone.html.haml @@ -3,7 +3,7 @@ Milestone - if can?(current_user, :admin_issue, @project) = icon("spinner spin", class: "block-loading") - = link_to "Edit", "#", class: "edit-link pull-right" + = link_to "Edit", "#", class: "js-sidebar-dropdown-toggle edit-link pull-right" .value %span.no-value{ "v-if" => "!issue.milestone" } None diff --git a/app/views/shared/icons/_icon_arrow_right.svg.erb b/app/views/shared/icons/_icon_arrow_right.svg.erb new file mode 100644 index 00000000000..24d64eb73bd --- /dev/null +++ b/app/views/shared/icons/_icon_arrow_right.svg.erb @@ -0,0 +1 @@ + diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index c016aa2abcd..bb02dfa0d3a 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -29,18 +29,6 @@ = render 'shared/issuable/form/metadata', issuable: issuable, form: form -- if issuable.can_move?(current_user) - %hr - .form-group - = label_tag :move_to_project_id, 'Move', class: 'control-label' - .col-sm-10 - .issuable-form-select-holder - = hidden_field_tag :move_to_project_id, nil, class: 'js-move-dropdown', data: { placeholder: 'Select project', projects_url: autocomplete_projects_path(project_id: @project.id), page_size: MoveToProjectFinder::PAGE_SIZE } -   - %span{ data: { toggle: 'tooltip', placement: 'auto top' }, style: 'cursor: default', - title: 'Moving an issue will copy the discussion to a different project and close it here. All participants will be notified of the new location.' } - = icon('question-circle') - = render 'shared/issuable/form/branch_chooser', issuable: issuable, form: form = render 'shared/issuable/form/merge_params', issuable: issuable diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index c3f25c9d255..b07bc45512f 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -34,7 +34,7 @@ Milestone = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable - = link_to 'Edit', '#', class: 'edit-link pull-right' + = link_to 'Edit', '#', class: 'js-sidebar-dropdown-toggle edit-link pull-right' .value.hide-collapsed - if issuable.milestone = link_to issuable.milestone.title, milestone_path(issuable.milestone), class: "bold has-tooltip", title: milestone_remaining_days(issuable.milestone), data: { container: "body", html: 1 } @@ -60,7 +60,7 @@ Due date = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project) - = link_to 'Edit', '#', class: 'edit-link pull-right' + = link_to 'Edit', '#', class: 'js-sidebar-dropdown-toggle edit-link pull-right' .value.hide-collapsed %span.value-content - if issuable.due_date @@ -95,7 +95,7 @@ Labels = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable - = link_to 'Edit', '#', class: 'edit-link pull-right' + = link_to 'Edit', '#', class: 'js-sidebar-dropdown-toggle edit-link pull-right' .value.issuable-show-labels.hide-collapsed{ class: ("has-labels" if selected_labels.any?) } - if selected_labels.any? - selected_labels.each do |label| @@ -141,5 +141,22 @@ %cite{ title: project_ref } = project_ref = clipboard_button(text: project_ref, title: "Copy reference to clipboard", placement: "left") + - if current_user && issuable.can_move?(current_user) + .block.js-sidebar-move-issue-block + .sidebar-collapsed-icon{ data: { toggle: 'tooltip', placement: 'left', container: 'body' }, title: 'Move issue' } + = custom_icon('icon_arrow_right') + .dropdown.sidebar-move-issue-dropdown.hide-collapsed + %button.btn.btn-default.btn-block.js-sidebar-dropdown-toggle.js-move-issue{ type: 'button', + data: { toggle: 'dropdown' } } + Move issue + .dropdown-menu.dropdown-menu-selectable + = dropdown_title('Move issue') + = dropdown_filter('Search project', search_id: 'sidebar-move-issue-dropdown-search') + = dropdown_content + = dropdown_loading + = dropdown_footer add_content_class: true do + %button.btn.btn-new.sidebar-move-issue-confirmation-button.js-move-issue-confirmation-button{ disabled: true } + Move + = icon('spinner spin', class: 'sidebar-move-issue-confirmation-loading-icon') %script.js-sidebar-options{ type: "application/json" }= issuable_sidebar_options(issuable, can_edit_issuable).to_json.html_safe diff --git a/app/views/shared/issuable/_sidebar_assignees.html.haml b/app/views/shared/issuable/_sidebar_assignees.html.haml index 57392cd7fbb..58782fa5f58 100644 --- a/app/views/shared/issuable/_sidebar_assignees.html.haml +++ b/app/views/shared/issuable/_sidebar_assignees.html.haml @@ -13,7 +13,7 @@ Assignee = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable - = link_to 'Edit', '#', class: 'edit-link pull-right' + = link_to 'Edit', '#', class: 'js-sidebar-dropdown-toggle edit-link pull-right' - if !signed_in %a.gutter-toggle.pull-right.js-sidebar-toggle{ role: "button", href: "#", "aria-label" => "Toggle sidebar" } = sidebar_gutter_toggle_icon diff --git a/app/views/shared/issuable/form/_issue_assignee.html.haml b/app/views/shared/issuable/form/_issue_assignee.html.haml index 66091d95a91..9b2b6e572e7 100644 --- a/app/views/shared/issuable/form/_issue_assignee.html.haml +++ b/app/views/shared/issuable/form/_issue_assignee.html.haml @@ -11,7 +11,7 @@ Assignee = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable - = link_to 'Edit', '#', class: 'edit-link pull-right' + = link_to 'Edit', '#', class: 'js-sidebar-dropdown-toggle edit-link pull-right' .value.hide-collapsed - if assignees.any? - assignees.each do |assignee| diff --git a/app/views/shared/issuable/form/_merge_request_assignee.html.haml b/app/views/shared/issuable/form/_merge_request_assignee.html.haml index 18011d528a0..bf8613b0f0d 100644 --- a/app/views/shared/issuable/form/_merge_request_assignee.html.haml +++ b/app/views/shared/issuable/form/_merge_request_assignee.html.haml @@ -9,7 +9,7 @@ Assignee = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable - = link_to 'Edit', '#', class: 'edit-link pull-right' + = link_to 'Edit', '#', class: 'js-sidebar-dropdown-toggle edit-link pull-right' .value.hide-collapsed - if merge_request.assignee = link_to_member(@project, merge_request.assignee, size: 32, extra_class: 'bold') do diff --git a/app/views/shared/milestones/_sidebar.html.haml b/app/views/shared/milestones/_sidebar.html.haml index 40379f48393..2ae0145727c 100644 --- a/app/views/shared/milestones/_sidebar.html.haml +++ b/app/views/shared/milestones/_sidebar.html.haml @@ -21,7 +21,7 @@ .title Start date - if @project && can?(current_user, :admin_milestone, @project) - = link_to 'Edit', edit_project_milestone_path(@project, @milestone), class: 'edit-link pull-right' + = link_to 'Edit', edit_project_milestone_path(@project, @milestone), class: 'js-sidebar-dropdown-toggle edit-link pull-right' .value %span.value-content - if milestone.start_date @@ -51,7 +51,7 @@ .title.hide-collapsed Due date - if @project && can?(current_user, :admin_milestone, @project) - = link_to 'Edit', edit_project_milestone_path(@project, @milestone), class: 'edit-link pull-right' + = link_to 'Edit', edit_project_milestone_path(@project, @milestone), class: 'js-sidebar-dropdown-toggle edit-link pull-right' .value.hide-collapsed %span.value-content - if milestone.due_date diff --git a/changelogs/unreleased/34261-move-move-to-sidebar.yml b/changelogs/unreleased/34261-move-move-to-sidebar.yml new file mode 100644 index 00000000000..59fa1d4c221 --- /dev/null +++ b/changelogs/unreleased/34261-move-move-to-sidebar.yml @@ -0,0 +1,5 @@ +--- +title: Move "Move issue" controls to right-sidebar +merge_request: +author: +type: changed diff --git a/config/routes/project.rb b/config/routes/project.rb index c703a7294ed..a15e7f8a344 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -303,6 +303,7 @@ constraints(ProjectUrlConstrainer.new) do member do post :toggle_subscription post :mark_as_spam + post :move get :referenced_merge_requests get :related_branches get :can_create_branch diff --git a/doc/user/project/issues/img/sidebar_move_issue.png b/doc/user/project/issues/img/sidebar_move_issue.png new file mode 100644 index 00000000000..111f7861364 Binary files /dev/null and b/doc/user/project/issues/img/sidebar_move_issue.png differ diff --git a/doc/user/project/issues/index.md b/doc/user/project/issues/index.md index 20901e01f6e..0f187946a4a 100644 --- a/doc/user/project/issues/index.md +++ b/doc/user/project/issues/index.md @@ -86,6 +86,10 @@ Read through the [documentation on creating issues](create_new_issue.md). Learn distinct ways to [close issues](closing_issues.md) in GitLab. +## Moving issues + +Read through the [documentation on moving issues](moving_issues.md). + ## Create a merge request from an issue Learn more about it on the [GitLab Issues Functionalities documentation](issues_functionalities.md#18-new-merge-request). diff --git a/doc/user/project/issues/moving_issues.md b/doc/user/project/issues/moving_issues.md new file mode 100644 index 00000000000..211a651b89e --- /dev/null +++ b/doc/user/project/issues/moving_issues.md @@ -0,0 +1,10 @@ +# Moving Issues + +Please read through the [GitLab Issue Documentation](index.md) for an overview on GitLab Issues. + +Moving an issue will close it and duplicate it on the specified project. +There will also be a system note added to both issues indicating where it came from or went to. + +You can move an issue with the "Move issue" button at the bottom of the right-sidebar when viewing the issue. + +![move issue - button](img/sidebar_move_issue.png) diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 572b567cddf..be27bbb4283 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -241,13 +241,10 @@ describe AutocompleteController do it 'returns projects' do expect(json_response).to be_kind_of(Array) - expect(json_response.size).to eq(2) - - expect(json_response.first['id']).to eq(0) - expect(json_response.first['name_with_namespace']).to eq 'No project' + expect(json_response.size).to eq(1) - expect(json_response.last['id']).to eq authorized_project.id - expect(json_response.last['name_with_namespace']).to eq authorized_project.name_with_namespace + expect(json_response.first['id']).to eq authorized_project.id + expect(json_response.first['name_with_namespace']).to eq authorized_project.name_with_namespace end end end @@ -265,10 +262,10 @@ describe AutocompleteController do it 'returns projects' do expect(json_response).to be_kind_of(Array) - expect(json_response.size).to eq(2) + expect(json_response.size).to eq(1) - expect(json_response.last['id']).to eq authorized_search_project.id - expect(json_response.last['name_with_namespace']).to eq authorized_search_project.name_with_namespace + expect(json_response.first['id']).to eq authorized_search_project.id + expect(json_response.first['name_with_namespace']).to eq authorized_search_project.name_with_namespace end end end @@ -292,7 +289,7 @@ describe AutocompleteController do it 'returns projects' do expect(json_response).to be_kind_of(Array) - expect(json_response.size).to eq 3 # Of a total of 4 + expect(json_response.size).to eq 2 # Of a total of 3 end end end @@ -312,9 +309,9 @@ describe AutocompleteController do get(:projects, project_id: project.id, offset_id: authorized_project.id) end - it 'returns "No project"' do - expect(json_response.detect { |item| item['id'] == 0 }).to be_nil # 'No project' is not there - expect(json_response.detect { |item| item['id'] == authorized_project.id }).to be_nil # Offset project is not there either + it 'returns projects' do + expect(json_response).to be_kind_of(Array) + expect(json_response.size).to eq 2 # Of a total of 3 end end end @@ -331,10 +328,9 @@ describe AutocompleteController do get(:projects, project_id: project.id) end - it 'returns a single "No project"' do + it 'returns no projects' do expect(json_response).to be_kind_of(Array) - expect(json_response.size).to eq(1) # 'No project' - expect(json_response.first['id']).to eq 0 + expect(json_response.size).to eq(0) end end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 65f4d09cfce..5d9403c23ac 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -233,144 +233,119 @@ describe Projects::IssuesController do end end - context 'when moving issue to another private project' do - let(:another_project) { create(:project, :private) } - - context 'when user has access to move issue' do - before do - another_project.team << [user, :reporter] - end - - it 'moves issue to another project' do - move_issue + context 'Akismet is enabled' do + let(:project) { create(:project_empty_repo, :public) } - expect(response).to have_http_status :found - expect(another_project.issues).not_to be_empty - end + before do + stub_application_setting(recaptcha_enabled: true) + allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) end - context 'when user does not have access to move issue' do - it 'responds with 404' do - move_issue + context 'when an issue is not identified as spam' do + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) + end - expect(response).to have_http_status :not_found + it 'normally updates the issue' do + expect { update_issue(title: 'Foo') }.to change { issue.reload.title }.to('Foo') end end - context 'Akismet is enabled' do - let(:project) { create(:project_empty_repo, :public) } - + context 'when an issue is identified as spam' do before do - stub_application_setting(recaptcha_enabled: true) - allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end - context 'when an issue is not identified as spam' do - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) + context 'when captcha is not verified' do + def update_spam_issue + update_issue(title: 'Spam Title', description: 'Spam lives here') end - it 'normally updates the issue' do - expect { update_issue(title: 'Foo') }.to change { issue.reload.title }.to('Foo') + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) end - end - context 'when an issue is identified as spam' do - before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + it 'rejects an issue recognized as a spam' do + expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true) + expect { update_spam_issue }.not_to change { issue.reload.title } end - context 'when captcha is not verified' do - def update_spam_issue - update_issue(title: 'Spam Title', description: 'Spam lives here') - end + it 'rejects an issue recognized as a spam when recaptcha disabled' do + stub_application_setting(recaptcha_enabled: false) - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - end + expect { update_spam_issue }.not_to change { issue.reload.title } + end - it 'rejects an issue recognized as a spam' do - expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true) - expect { update_spam_issue }.not_to change { issue.reload.title } - end + it 'creates a spam log' do + update_spam_issue - it 'rejects an issue recognized as a spam when recaptcha disabled' do - stub_application_setting(recaptcha_enabled: false) + spam_logs = SpamLog.all - expect { update_spam_issue }.not_to change { issue.reload.title } - end + expect(spam_logs.count).to eq(1) + expect(spam_logs.first.title).to eq('Spam Title') + expect(spam_logs.first.recaptcha_verified).to be_falsey + end - it 'creates a spam log' do + context 'as HTML' do + it 'renders verify template' do update_spam_issue - spam_logs = SpamLog.all - - expect(spam_logs.count).to eq(1) - expect(spam_logs.first.title).to eq('Spam Title') - expect(spam_logs.first.recaptcha_verified).to be_falsey + expect(response).to render_template(:verify) end + end - context 'as HTML' do - it 'renders verify template' do - update_spam_issue - - expect(response).to render_template(:verify) - end + context 'as JSON' do + before do + update_issue({ title: 'Spam Title', description: 'Spam lives here' }, format: :json) end - context 'as JSON' do - before do - update_issue({ title: 'Spam Title', description: 'Spam lives here' }, format: :json) - end - - it 'renders json errors' do - expect(json_response) - .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."]) - end + it 'renders json errors' do + expect(json_response) + .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."]) + end - it 'returns 422 status' do - expect(response).to have_http_status(422) - end + it 'returns 422 status' do + expect(response).to have_http_status(422) end end + end - context 'when captcha is verified' do - let(:spammy_title) { 'Whatever' } - let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) } + context 'when captcha is verified' do + let(:spammy_title) { 'Whatever' } + let!(:spam_logs) { create_list(:spam_log, 2, user: user, title: spammy_title) } - def update_verified_issue - update_issue({ title: spammy_title }, - { spam_log_id: spam_logs.last.id, - recaptcha_verification: true }) - end + def update_verified_issue + update_issue({ title: spammy_title }, + { spam_log_id: spam_logs.last.id, + recaptcha_verification: true }) + end - before do - allow_any_instance_of(described_class).to receive(:verify_recaptcha) - .and_return(true) - end + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha) + .and_return(true) + end - it 'redirect to issue page' do - update_verified_issue + it 'redirect to issue page' do + update_verified_issue - expect(response) - .to redirect_to(project_issue_path(project, issue)) - end + expect(response) + .to redirect_to(project_issue_path(project, issue)) + end - it 'accepts an issue after recaptcha is verified' do - expect { update_verified_issue }.to change { issue.reload.title }.to(spammy_title) - end + it 'accepts an issue after recaptcha is verified' do + expect { update_verified_issue }.to change { issue.reload.title }.to(spammy_title) + end - it 'marks spam log as recaptcha_verified' do - expect { update_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true) - end + it 'marks spam log as recaptcha_verified' do + expect { update_verified_issue }.to change { SpamLog.last.recaptcha_verified }.from(false).to(true) + end - it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do - spam_log = create(:spam_log) + it 'does not mark spam log as recaptcha_verified when it does not belong to current_user' do + spam_log = create(:spam_log) - expect { update_issue(spam_log_id: spam_log.id, recaptcha_verification: true) } - .not_to change { SpamLog.last.recaptcha_verified } - end + expect { update_issue(spam_log_id: spam_log.id, recaptcha_verification: true) } + .not_to change { SpamLog.last.recaptcha_verified } end end end @@ -385,13 +360,45 @@ describe Projects::IssuesController do put :update, params end + end + end + + describe 'POST #move' do + before do + sign_in(user) + project.add_developer(user) + end + + context 'when moving issue to another private project' do + let(:another_project) { create(:project, :private) } + + context 'when user has access to move issue' do + before do + another_project.add_reporter(user) + end + + it 'moves issue to another project' do + move_issue + + expect(response).to have_http_status :ok + expect(another_project.issues).not_to be_empty + end + end + + context 'when user does not have access to move issue' do + it 'responds with 404' do + move_issue + + expect(response).to have_http_status :not_found + end + end def move_issue - put :update, + post :move, + format: :json, namespace_id: project.namespace.to_param, project_id: project, id: issue.iid, - issue: { title: 'New title' }, move_to_project_id: another_project.id end end diff --git a/spec/features/issues/move_spec.rb b/spec/features/issues/move_spec.rb index 494c309c9ea..b2724945da4 100644 --- a/spec/features/issues/move_spec.rb +++ b/spec/features/issues/move_spec.rb @@ -15,11 +15,11 @@ feature 'issue move to another project' do background do old_project.team << [user, :guest] - edit_issue(issue) + visit issue_path(issue) end scenario 'moving issue to another project not allowed' do - expect(page).to have_no_selector('#move_to_project_id') + expect(page).to have_no_selector('.js-sidebar-move-issue-block') end end @@ -34,12 +34,14 @@ feature 'issue move to another project' do old_project.team << [user, :reporter] new_project.team << [user, :reporter] - edit_issue(issue) + visit issue_path(issue) end scenario 'moving issue to another project', js: true do - find('#issuable-move', visible: false).set(new_project.id) - click_button('Save changes') + find('.js-move-issue').trigger('click') + wait_for_requests + all('.js-move-issue-dropdown-item')[0].click + find('.js-move-issue-confirmation-button').click expect(page).to have_content("Text with #{cross_reference}#{mr.to_reference}") expect(page).to have_content("moved from #{cross_reference}#{issue.to_reference}") @@ -50,13 +52,12 @@ feature 'issue move to another project' do scenario 'searching project dropdown', js: true do new_project_search.team << [user, :reporter] - page.within '.detail-page-description' do - first('.select2-choice').click - end + find('.js-move-issue').trigger('click') + wait_for_requests - fill_in('s2id_autogen1_search', with: new_project_search.name) + page.within '.js-sidebar-move-issue-block' do + fill_in('sidebar-move-issue-dropdown-search', with: new_project_search.name) - page.within '.select2-drop' do expect(page).to have_content(new_project_search.name) expect(page).not_to have_content(new_project.name) end @@ -68,10 +69,10 @@ feature 'issue move to another project' do background { another_project.team << [user, :guest] } scenario 'browsing projects in projects select' do - click_link 'Move to a different project' + find('.js-move-issue').trigger('click') + wait_for_requests - page.within '.select2-results' do - expect(page).to have_content 'No project' + page.within '.js-sidebar-move-issue-block' do expect(page).to have_content new_project.name_with_namespace end end @@ -89,11 +90,6 @@ feature 'issue move to another project' do end end - def edit_issue(issue) - visit issue_path(issue) - page.within('.issuable-actions') { first(:link, 'Edit').click } - end - def issue_path(issue) project_issue_path(issue.project, issue) end diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index 3af26e2f28f..39065814bc2 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -34,7 +34,6 @@ describe('Issuable output', () => { propsData: { canUpdate: true, canDestroy: true, - canMove: true, endpoint: '/gitlab-org/gitlab-shell/issues/9/realtime_changes', issuableRef: '#1', initialTitleHtml: '', @@ -43,7 +42,6 @@ describe('Issuable output', () => { initialDescriptionText: '', markdownPreviewPath: '/', markdownDocsPath: '/', - projectsAutocompletePath: '/', isConfidential: false, projectNamespace: '/', projectPath: '/', @@ -226,7 +224,7 @@ describe('Issuable output', () => { }); }); - it('redirects if issue is moved', (done) => { + it('redirects if returned web_url has changed', (done) => { spyOn(gl.utils, 'visitUrl'); spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve) => { resolve({ @@ -250,23 +248,6 @@ describe('Issuable output', () => { }); }); - it('does not update issuable if project move confirm is false', (done) => { - spyOn(window, 'confirm').and.returnValue(false); - spyOn(vm.service, 'updateIssuable'); - - vm.store.formState.move_to_project_id = 1; - - vm.updateIssuable(); - - setTimeout(() => { - expect( - vm.service.updateIssuable, - ).not.toHaveBeenCalled(); - - done(); - }); - }); - it('closes form on error', (done) => { spyOn(window, 'Flash').and.callThrough(); spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve, reject) => { diff --git a/spec/javascripts/issue_show/components/fields/project_move_spec.js b/spec/javascripts/issue_show/components/fields/project_move_spec.js deleted file mode 100644 index 8b6ed6a03a9..00000000000 --- a/spec/javascripts/issue_show/components/fields/project_move_spec.js +++ /dev/null @@ -1,38 +0,0 @@ -import Vue from 'vue'; -import projectMove from '~/issue_show/components/fields/project_move.vue'; - -describe('Project move field component', () => { - let vm; - let formState; - - beforeEach((done) => { - const Component = Vue.extend(projectMove); - - formState = { - move_to_project_id: 0, - }; - - vm = new Component({ - propsData: { - formState, - projectsAutocompletePath: '/autocomplete', - }, - }).$mount(); - - Vue.nextTick(done); - }); - - it('mounts select2 element', () => { - expect( - vm.$el.querySelector('.select2-container'), - ).not.toBeNull(); - }); - - it('updates formState on change', () => { - $(vm.$refs['move-dropdown']).val(2).trigger('change'); - - expect( - formState.move_to_project_id, - ).toBe(2); - }); -}); diff --git a/spec/javascripts/issue_show/components/form_spec.js b/spec/javascripts/issue_show/components/form_spec.js index d8af5287431..6e89528a3ea 100644 --- a/spec/javascripts/issue_show/components/form_spec.js +++ b/spec/javascripts/issue_show/components/form_spec.js @@ -12,7 +12,6 @@ describe('Inline edit form component', () => { vm = new Component({ propsData: { canDestroy: true, - canMove: true, formState: { title: 'b', description: 'a', @@ -20,7 +19,6 @@ describe('Inline edit form component', () => { }, markdownPreviewPath: '/', markdownDocsPath: '/', - projectsAutocompletePath: '/', projectPath: '/', projectNamespace: '/', }, diff --git a/spec/javascripts/sidebar/mock_data.js b/spec/javascripts/sidebar/mock_data.js index 9fc8667ecc9..e2b6bcabc98 100644 --- a/spec/javascripts/sidebar/mock_data.js +++ b/spec/javascripts/sidebar/mock_data.js @@ -66,17 +66,57 @@ const sidebarMockData = { }, labels: [], }, + '/autocomplete/projects?project_id=15': [ + { + 'id': 0, + 'name_with_namespace': 'No project', + }, { + 'id': 20, + 'name_with_namespace': 'foo / bar', + }, + ], }, 'PUT': { '/gitlab-org/gitlab-shell/issues/5.json': { data: {}, }, }, + 'POST': { + '/gitlab-org/gitlab-shell/issues/5/move': { + id: 123, + iid: 5, + author_id: 1, + description: 'some description', + lock_version: 5, + milestone_id: null, + state: 'opened', + title: 'some title', + updated_by_id: 1, + created_at: '2017-06-27T19:54:42.437Z', + updated_at: '2017-08-18T03:39:49.222Z', + deleted_at: null, + time_estimate: 0, + total_time_spent: 0, + human_time_estimate: null, + human_total_time_spent: null, + branch_name: null, + confidential: false, + assignees: [], + due_date: null, + moved_to_id: null, + project_id: 7, + milestone: null, + labels: [], + web_url: '/root/some-project/issues/5', + }, + }, }; export default { mediator: { endpoint: '/gitlab-org/gitlab-shell/issues/5.json', + moveIssueEndpoint: '/gitlab-org/gitlab-shell/issues/5/move', + projectsAutocompleteEndpoint: '/autocomplete/projects?project_id=15', editable: true, currentUser: { id: 1, @@ -85,6 +125,7 @@ export default { avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', }, rootPath: '/', + fullPath: '/gitlab-org/gitlab-shell', }, time: { time_estimate: 3600, diff --git a/spec/javascripts/sidebar/sidebar_mediator_spec.js b/spec/javascripts/sidebar/sidebar_mediator_spec.js index e246f41ee82..3aa8ca5db0d 100644 --- a/spec/javascripts/sidebar/sidebar_mediator_spec.js +++ b/spec/javascripts/sidebar/sidebar_mediator_spec.js @@ -30,7 +30,7 @@ describe('Sidebar mediator', () => { expect(resp.status).toEqual(200); done(); }) - .catch(() => {}); + .catch(done.fail); }); it('fetches the data', () => { @@ -38,4 +38,42 @@ describe('Sidebar mediator', () => { this.mediator.fetch(); expect(this.mediator.service.get).toHaveBeenCalled(); }); + + it('sets moveToProjectId', () => { + const projectId = 7; + spyOn(this.mediator.store, 'setMoveToProjectId').and.callThrough(); + + this.mediator.setMoveToProjectId(projectId); + + expect(this.mediator.store.setMoveToProjectId).toHaveBeenCalledWith(projectId); + }); + + it('fetches autocomplete projects', (done) => { + const searchTerm = 'foo'; + spyOn(this.mediator.service, 'getProjectsAutocomplete').and.callThrough(); + spyOn(this.mediator.store, 'setAutocompleteProjects').and.callThrough(); + + this.mediator.fetchAutocompleteProjects(searchTerm) + .then(() => { + expect(this.mediator.service.getProjectsAutocomplete).toHaveBeenCalledWith(searchTerm); + expect(this.mediator.store.setAutocompleteProjects).toHaveBeenCalled(); + done(); + }) + .catch(done.fail); + }); + + it('moves issue', (done) => { + const moveToProjectId = 7; + this.mediator.store.setMoveToProjectId(moveToProjectId); + spyOn(this.mediator.service, 'moveIssue').and.callThrough(); + spyOn(gl.utils, 'visitUrl'); + + this.mediator.moveIssue() + .then(() => { + expect(this.mediator.service.moveIssue).toHaveBeenCalledWith(moveToProjectId); + expect(gl.utils.visitUrl).toHaveBeenCalledWith('/root/some-project/issues/5'); + done(); + }) + .catch(done.fail); + }); }); diff --git a/spec/javascripts/sidebar/sidebar_move_issue_spec.js b/spec/javascripts/sidebar/sidebar_move_issue_spec.js new file mode 100644 index 00000000000..8b0d51bbcc8 --- /dev/null +++ b/spec/javascripts/sidebar/sidebar_move_issue_spec.js @@ -0,0 +1,142 @@ +import Vue from 'vue'; +import SidebarMediator from '~/sidebar/sidebar_mediator'; +import SidebarStore from '~/sidebar/stores/sidebar_store'; +import SidebarService from '~/sidebar/services/sidebar_service'; +import SidebarMoveIssue from '~/sidebar/lib/sidebar_move_issue'; +import Mock from './mock_data'; + +describe('SidebarMoveIssue', () => { + beforeEach(() => { + Vue.http.interceptors.push(Mock.sidebarMockInterceptor); + this.mediator = new SidebarMediator(Mock.mediator); + this.$content = $(` + + `); + this.$toggleButton = this.$content.find('.js-toggle'); + this.$confirmButton = this.$content.find('.js-confirm-button'); + + this.sidebarMoveIssue = new SidebarMoveIssue( + this.mediator, + this.$toggleButton, + this.$confirmButton, + ); + this.sidebarMoveIssue.init(); + }); + + afterEach(() => { + SidebarService.singleton = null; + SidebarStore.singleton = null; + SidebarMediator.singleton = null; + + this.sidebarMoveIssue.destroy(); + + Vue.http.interceptors = _.without(Vue.http.interceptors, Mock.sidebarMockInterceptor); + }); + + describe('init', () => { + it('should initialize the dropdown and listeners', () => { + spyOn(this.sidebarMoveIssue, 'initDropdown'); + spyOn(this.sidebarMoveIssue, 'addEventListeners'); + + this.sidebarMoveIssue.init(); + + expect(this.sidebarMoveIssue.initDropdown).toHaveBeenCalled(); + expect(this.sidebarMoveIssue.addEventListeners).toHaveBeenCalled(); + }); + }); + + describe('destroy', () => { + it('should remove the listeners', () => { + spyOn(this.sidebarMoveIssue, 'removeEventListeners'); + + this.sidebarMoveIssue.destroy(); + + expect(this.sidebarMoveIssue.removeEventListeners).toHaveBeenCalled(); + }); + }); + + describe('initDropdown', () => { + it('should initialize the gl_dropdown', () => { + spyOn($.fn, 'glDropdown'); + + this.sidebarMoveIssue.initDropdown(); + + expect($.fn.glDropdown).toHaveBeenCalled(); + }); + }); + + describe('onConfirmClicked', () => { + it('should move the issue with valid project ID', () => { + spyOn(this.mediator, 'moveIssue').and.returnValue(Promise.resolve()); + this.mediator.setMoveToProjectId(7); + + this.sidebarMoveIssue.onConfirmClicked(); + + expect(this.mediator.moveIssue).toHaveBeenCalled(); + expect(this.$confirmButton.attr('disabled')).toBe('disabled'); + expect(this.$confirmButton.hasClass('is-loading')).toBe(true); + }); + + it('should remove loading state from confirm button on failure', (done) => { + spyOn(window, 'Flash'); + spyOn(this.mediator, 'moveIssue').and.returnValue(Promise.reject()); + this.mediator.setMoveToProjectId(7); + + this.sidebarMoveIssue.onConfirmClicked(); + + expect(this.mediator.moveIssue).toHaveBeenCalled(); + // Wait for the move issue request to fail + setTimeout(() => { + expect(window.Flash).toHaveBeenCalled(); + expect(this.$confirmButton.attr('disabled')).toBe(undefined); + expect(this.$confirmButton.hasClass('is-loading')).toBe(false); + done(); + }); + }); + + it('should not move the issue with id=0', () => { + spyOn(this.mediator, 'moveIssue'); + this.mediator.setMoveToProjectId(0); + + this.sidebarMoveIssue.onConfirmClicked(); + + expect(this.mediator.moveIssue).not.toHaveBeenCalled(); + }); + }); + + it('should set moveToProjectId on dropdown item "No project" click', (done) => { + spyOn(this.mediator, 'setMoveToProjectId'); + + // Open the dropdown + this.$toggleButton.dropdown('toggle'); + + // Wait for the autocomplete request to finish + setTimeout(() => { + this.$content.find('.js-move-issue-dropdown-item').eq(0).trigger('click'); + + expect(this.mediator.setMoveToProjectId).toHaveBeenCalledWith(0); + expect(this.$confirmButton.attr('disabled')).toBe('disabled'); + done(); + }, 0); + }); + + it('should set moveToProjectId on dropdown item click', (done) => { + spyOn(this.mediator, 'setMoveToProjectId'); + + // Open the dropdown + this.$toggleButton.dropdown('toggle'); + + // Wait for the autocomplete request to finish + setTimeout(() => { + this.$content.find('.js-move-issue-dropdown-item').eq(1).trigger('click'); + + expect(this.mediator.setMoveToProjectId).toHaveBeenCalledWith(20); + expect(this.$confirmButton.attr('disabled')).toBe(undefined); + done(); + }, 0); + }); +}); diff --git a/spec/javascripts/sidebar/sidebar_service_spec.js b/spec/javascripts/sidebar/sidebar_service_spec.js index 91a4dd669a7..a4bd8ba8d88 100644 --- a/spec/javascripts/sidebar/sidebar_service_spec.js +++ b/spec/javascripts/sidebar/sidebar_service_spec.js @@ -5,7 +5,11 @@ import Mock from './mock_data'; describe('Sidebar service', () => { beforeEach(() => { Vue.http.interceptors.push(Mock.sidebarMockInterceptor); - this.service = new SidebarService('/gitlab-org/gitlab-shell/issues/5.json'); + this.service = new SidebarService({ + endpoint: '/gitlab-org/gitlab-shell/issues/5.json', + moveIssueEndpoint: '/gitlab-org/gitlab-shell/issues/5/move', + projectsAutocompleteEndpoint: '/autocomplete/projects?project_id=15', + }); }); afterEach(() => { @@ -19,7 +23,7 @@ describe('Sidebar service', () => { expect(resp).toBeDefined(); done(); }) - .catch(() => {}); + .catch(done.fail); }); it('updates the data', (done) => { @@ -28,6 +32,24 @@ describe('Sidebar service', () => { expect(resp).toBeDefined(); done(); }) - .catch(() => {}); + .catch(done.fail); + }); + + it('gets projects for autocomplete', (done) => { + this.service.getProjectsAutocomplete() + .then((resp) => { + expect(resp).toBeDefined(); + done(); + }) + .catch(done.fail); + }); + + it('moves the issue to another project', (done) => { + this.service.moveIssue(123) + .then((resp) => { + expect(resp).toBeDefined(); + done(); + }) + .catch(done.fail); }); }); diff --git a/spec/javascripts/sidebar/sidebar_store_spec.js b/spec/javascripts/sidebar/sidebar_store_spec.js index b3fa156eb64..69eb3839d67 100644 --- a/spec/javascripts/sidebar/sidebar_store_spec.js +++ b/spec/javascripts/sidebar/sidebar_store_spec.js @@ -82,4 +82,18 @@ describe('Sidebar store', () => { expect(this.store.humanTimeEstimate).toEqual(Mock.time.human_time_estimate); expect(this.store.humanTotalTimeSpent).toEqual(Mock.time.human_total_time_spent); }); + + it('set autocomplete projects', () => { + const projects = [{ id: 0 }]; + this.store.setAutocompleteProjects(projects); + + expect(this.store.autocompleteProjects).toEqual(projects); + }); + + it('set move to project ID', () => { + const projectId = 7; + this.store.setMoveToProjectId(projectId); + + expect(this.store.moveToProjectId).toEqual(projectId); + }); }); -- cgit v1.2.1