diff options
author | Jacob Schatz <jschatz@gitlab.com> | 2016-06-04 10:55:29 +0000 |
---|---|---|
committer | Jacob Schatz <jschatz@gitlab.com> | 2016-06-04 10:55:29 +0000 |
commit | 83b8d4e63634a0d590ceea8bf4f90cdeb6d777e4 (patch) | |
tree | 32e11423467945c0fa6c7557569a75dd70b525bc | |
parent | 9aca0a1f96d0480ff61c92603acb2d6a987dd271 (diff) | |
parent | 94d9efaa8e59a93d7350e2e1f7a4f99ef93a5ab7 (diff) | |
download | gitlab-ce-83b8d4e63634a0d590ceea8bf4f90cdeb6d777e4.tar.gz |
Merge branch 'issue_13499' into 'master'
Allow bulk-assign labels to issues
- [x] Allow bulk-assignment labels.
- [x] Show indeterminate state for labels present on selection.
- [x] Remove existing labels from selected items if label gets unmarked.
- [x] Fix conflicting tests.
- [x] Write tests.
Closes #13499 and #15489
See merge request !3917
21 files changed, 776 insertions, 133 deletions
diff --git a/CHANGELOG b/CHANGELOG index d54cac7f93b..e2ba6bc79dd 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.9.0 (unreleased) + - Bulk assign/unassign labels to issues. - Allow enabling wiki page events from Webhook management UI - Make EmailsOnPushWorker use Sidekiq mailers queue - Fix wiki page events' webhook to point to the wiki repository diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index cd418ad244a..bae67a2ebaf 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -17,6 +17,7 @@ class Dispatcher switch page when 'projects:issues:index' Issuable.init() + new IssuableBulkActions() shortcut_handler = new ShortcutsNavigation() when 'projects:issues:show' new Issue() diff --git a/app/assets/javascripts/flash.js.coffee b/app/assets/javascripts/flash.js.coffee index 5de012e409f..4f73d215b85 100644 --- a/app/assets/javascripts/flash.js.coffee +++ b/app/assets/javascripts/flash.js.coffee @@ -1,5 +1,5 @@ class @Flash - constructor: (message, type)-> + constructor: (message, type = 'alert')-> @flash = $(".flash-container") @flash.html("") diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index d263faa287a..7c7334e9e40 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -11,6 +11,8 @@ class GitLabDropdownFilter $inputContainer = @input.parent() $clearButton = $inputContainer.find('.js-dropdown-input-clear') + @indeterminateIds = [] + # Clear click $clearButton.on 'click', (e) => e.preventDefault() @@ -35,20 +37,20 @@ class GitLabDropdownFilter if keyCode is 13 return false - clearTimeout timeout - timeout = setTimeout => - blur_field = @shouldBlur keyCode - search_text = @input.val() + # Only filter asynchronously only if option remote is set + if @options.remote + clearTimeout timeout + timeout = setTimeout => + blur_field = @shouldBlur keyCode - if blur_field and @filterInputBlur - @input.blur() + if blur_field and @filterInputBlur + @input.blur() - if @options.remote - @options.query search_text, (data) => + @options.query @input.val(), (data) => @options.callback(data) - else - @filter search_text - , 250 + , 250 + else + @filter @input.val() shouldBlur: (keyCode) -> return BLUR_KEYCODES.indexOf(keyCode) >= 0 @@ -142,6 +144,7 @@ class GitLabDropdown LOADING_CLASS = "is-loading" PAGE_TWO_CLASS = "is-page-two" ACTIVE_CLASS = "is-active" + INDETERMINATE_CLASS = "is-indeterminate" currentIndex = -1 FILTER_INPUT = '.dropdown-input .dropdown-input-field' @@ -182,9 +185,6 @@ class GitLabDropdown @fullData = data @parseData @fullData - - if @options.filterable - @filterInput.trigger 'keyup' } # Init filterable @@ -298,6 +298,13 @@ class GitLabDropdown opened: => @addArrowKeyEvent() + if @options.setIndeterminateIds + @options.setIndeterminateIds.call(@) + + # Makes indeterminate items effective + if @fullData and @dropdown.find('.dropdown-menu-toggle').hasClass('js-filter-bulk-update') + @parseData @fullData + contentHtml = $('.dropdown-content', @dropdown).html() if @remote && contentHtml is "" @remote.execute() @@ -309,12 +316,18 @@ class GitLabDropdown hidden: (e) => @removeArrayKeyEvent() + + $input = @dropdown.find(".dropdown-input-field") + if @options.filterable - @dropdown - .find(".dropdown-input-field") + $input .blur() .val("") - .trigger("keyup") + + # Triggering 'keyup' will re-render the dropdown which is not always required + # specially if we want to keep the state of the dropdown needed for bulk-assignment + if not @options.persistWhenHide + $input.trigger("keyup") if @dropdown.find(".dropdown-toggle-page").length $('.dropdown-menu', @dropdown).removeClass PAGE_TWO_CLASS @@ -358,7 +371,7 @@ class GitLabDropdown if @options.renderRow # Call the render function - html = @options.renderRow(data) + html = @options.renderRow.call(@options, data, @) else if not selected value = if @options.id then @options.id(data) else data.id @@ -443,6 +456,17 @@ class GitLabDropdown $(@el).find(".dropdown-toggle-text").text @options.toggleLabel else selectedObject + else if el.hasClass(INDETERMINATE_CLASS) + el.addClass ACTIVE_CLASS + el.removeClass INDETERMINATE_CLASS + + if not value? + field.remove() + + if not field.length and fieldName + @addInput(fieldName, value) + + return selectedObject else if not @options.multiSelect or el.hasClass('dropdown-clear-active') @dropdown.find(".#{ACTIVE_CLASS}").removeClass ACTIVE_CLASS @@ -459,17 +483,23 @@ class GitLabDropdown $(@el).find(".dropdown-toggle-text").text @options.toggleLabel(selectedObject, el) if value? if !field.length and fieldName - # Create hidden input for form - input = "<input type='hidden' name='#{fieldName}' value='#{value}' />" - if @options.inputId? - input = $(input) - .attr('id', @options.inputId) - @dropdown.before input + @addInput(fieldName, value) else field.val value return selectedObject + addInput: (fieldName, value)-> + # Create hidden input for form + $input = $('<input>').attr('type', 'hidden') + .attr('name', fieldName) + .val(value) + + if @options.inputId? + $input.attr('id', @options.inputId) + + @dropdown.before $input + selectRowAtIndex: (e, index) -> selector = ".dropdown-content li:not(.divider,.dropdown-header,.separator):eq(#{index}) a" diff --git a/app/assets/javascripts/issues-bulk-assignment.js.coffee b/app/assets/javascripts/issues-bulk-assignment.js.coffee new file mode 100644 index 00000000000..16d023dd391 --- /dev/null +++ b/app/assets/javascripts/issues-bulk-assignment.js.coffee @@ -0,0 +1,109 @@ +class @IssuableBulkActions + constructor: (opts = {}) -> + # Set defaults + { + @container = $('.content') + @form = @getElement('.bulk-update') + @issues = @getElement('.issues-list .issue') + } = opts + + @bindEvents() + + getElement: (selector) -> + @container.find selector + + bindEvents: -> + @form.off('submit').on('submit', @onFormSubmit.bind(@)) + + onFormSubmit: (e) -> + e.preventDefault() + @submit() + + submit: -> + _this = @ + + xhr = $.ajax + url: @form.attr 'action' + method: @form.attr 'method' + dataType: 'JSON', + data: @getFormDataAsObject() + + xhr.done (response, status, xhr) -> + location.reload() + + xhr.fail -> + new Flash("Issue update failed") + + xhr.always @onFormSubmitAlways.bind(@) + + onFormSubmitAlways: -> + @form.find('[type="submit"]').enable() + + getSelectedIssues: -> + @issues.has('.selected_issue:checked') + + getLabelsFromSelection: -> + labels = [] + + @getSelectedIssues().map -> + _labels = $(@).data('labels') + if _labels + _labels.map (labelId) -> + labels.push(labelId) if labels.indexOf(labelId) is -1 + + labels + + ###* + * Will return only labels that were marked previously and the user has unmarked + * @return {Array} Label IDs + ### + getUnmarkedIndeterminedLabels: -> + result = [] + labelsToKeep = [] + + for el in @getElement('.labels-filter .is-indeterminate') + labelsToKeep.push $(el).data('labelId') + + for id in @getLabelsFromSelection() + # Only the ones that we are not going to keep + result.push(id) if labelsToKeep.indexOf(id) is -1 + + result + + ###* + * Simple form serialization, it will return just what we need + * Returns key/value pairs from form data + ### + getFormDataAsObject: -> + formData = + update: + state_event : @form.find('input[name="update[state_event]"]').val() + assignee_id : @form.find('input[name="update[assignee_id]"]').val() + milestone_id : @form.find('input[name="update[milestone_id]"]').val() + issues_ids : @form.find('input[name="update[issues_ids]"]').val() + add_label_ids : [] + remove_label_ids : [] + + @getLabelsToApply().map (id) -> + formData.update.add_label_ids.push id + + @getLabelsToRemove().map (id) -> + formData.update.remove_label_ids.push id + + formData + + getLabelsToApply: -> + labelIds = [] + $labels = @form.find('.labels-filter input[name="update[label_ids][]"]') + + $labels.each (k, label) -> + labelIds.push $(label).val() if label + + labelIds + + ###* + * Just an alias of @getUnmarkedIndeterminedLabels + * @return {Array} Array of labels + ### + getLabelsToRemove: -> + @getUnmarkedIndeterminedLabels() diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 995fd768603..ec74dfaae1a 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -1,5 +1,7 @@ class @LabelsSelect constructor: -> + _this = @ + $('.js-label-select').each (i, dropdown) -> $dropdown = $(dropdown) projectId = $dropdown.data('project-id') @@ -196,10 +198,18 @@ class @LabelsSelect callback data - renderRow: (label) -> - removesAll = label.id is 0 or not label.id? + renderRow: (label, instance) -> + $li = $('<li>') + $a = $('<a href="#">') selectedClass = [] + removesAll = label.id is 0 or not label.id? + + if $dropdown.hasClass('js-filter-bulk-update') + indeterminate = instance.indeterminateIds + if indeterminate.indexOf(label.id) isnt -1 + selectedClass.push 'is-indeterminate' + if $form.find("input[type='hidden']\ [name='#{$dropdown.data('fieldName')}']\ [value='#{this.id(label)}']").length @@ -230,13 +240,17 @@ class @LabelsSelect else colorEl = '' - "<li> - <a href='#' class='#{selectedClass.join(' ')}'> - #{colorEl} - #{_.escape(label.title)} - </a> - </li>" - filterable: true + # We need to identify which items are actually labels + if label.id + selectedClass.push('label-item') + $a.attr('data-label-id', label.id) + + $a.addClass(selectedClass.join(' ')) + .html("#{colorEl} #{_.escape(label.title)}") + + # Return generated html + $li.html($a).prop('outerHTML') + persistWhenHide: $dropdown.data('persistWhenHide') search: fields: ['title'] selectable: true @@ -280,10 +294,19 @@ class @LabelsSelect else if $dropdown.hasClass('js-filter-submit') $dropdown.closest('form').submit() else - saveLabelData() + if not $dropdown.hasClass 'js-filter-bulk-update' + saveLabelData() + + if $dropdown.hasClass('js-filter-bulk-update') + # If we are persisting state we need the classes + if not @options.persistWhenHide + $dropdown.parent().find('.is-active, .is-indeterminate').removeClass() multiSelect: $dropdown.hasClass 'js-multiselect' clicked: (label) -> + if $dropdown.hasClass('js-filter-bulk-update') + return + page = $('body').data 'page' isIssueIndex = page is 'projects:issues:index' isMRIndex = page is 'projects:merge_requests:index' @@ -298,4 +321,31 @@ class @LabelsSelect return else saveLabelData() + + setIndeterminateIds: -> + if @dropdown.find('.dropdown-menu-toggle').hasClass('js-filter-bulk-update') + @indeterminateIds = _this.getIndeterminateIds() ) + + @bindEvents() + + bindEvents: -> + $('body').on 'change', '.selected_issue', @onSelectCheckboxIssue + + onSelectCheckboxIssue: -> + return if $('.selected_issue:checked').length + + # Remove inputs + $('.issues_bulk_update .labels-filter input[type="hidden"]').remove() + + # Also restore button text + $('.issues_bulk_update .labels-filter .dropdown-toggle-text').text('Label') + + getIndeterminateIds: -> + label_ids = [] + + $('.selected_issue:checked').each (i, el) -> + issue_id = $(el).data('id') + label_ids.push $("#issue_#{issue_id}").data('labels') + + _.flatten(label_ids) diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index 93c63c69843..28634d0c59f 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -232,9 +232,8 @@ a { padding-left: 25px; - &.is-active { + &.is-indeterminate, &.is-active { &::before { - content: "\f00c"; position: absolute; left: 5px; top: 50%; @@ -246,6 +245,14 @@ -moz-osx-font-smoothing: grayscale; } } + + &.is-indeterminate::before { + content: "\f068"; + } + + &.is-active::before { + content: "\f00c"; + } } } diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 5f3745d94b9..4e2d3bebb2e 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -156,7 +156,12 @@ class Projects::IssuesController < Projects::ApplicationController def bulk_update result = Issues::BulkUpdateService.new(project, current_user, bulk_update_params).execute - redirect_back_or_default(default: { action: 'index' }, options: { notice: "#{result[:count]} issues updated" }) + + respond_to do |format| + format.json do + render json: { notice: "#{result[:count]} issues updated" } + end + end end protected @@ -216,7 +221,10 @@ class Projects::IssuesController < Projects::ApplicationController :issues_ids, :assignee_id, :milestone_id, - :state_event + :state_event, + label_ids: [], + add_label_ids: [], + remove_label_ids: [] ) end end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index fe84ee3de44..37b93f63145 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -96,5 +96,4 @@ module IssuablesHelper issuable.open? ? :opened : :closed end end - end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 2b16089df1b..e3dc569152c 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -45,6 +45,8 @@ class IssuableBaseService < BaseService unless can?(current_user, ability, project) params.delete(:milestone_id) + params.delete(:add_label_ids) + params.delete(:remove_label_ids) params.delete(:label_ids) params.delete(:assignee_id) end @@ -67,10 +69,34 @@ class IssuableBaseService < BaseService end def filter_labels - return if params[:label_ids].to_a.empty? + if params[:add_label_ids].present? || params[:remove_label_ids].present? + params.delete(:label_ids) + + filter_labels_in_param(:add_label_ids) + filter_labels_in_param(:remove_label_ids) + else + filter_labels_in_param(:label_ids) + end + end + + def filter_labels_in_param(key) + return if params[key].to_a.empty? - params[:label_ids] = - project.labels.where(id: params[:label_ids]).pluck(:id) + params[key] = project.labels.where(id: params[key]).pluck(:id) + end + + def update_issuable(issuable, attributes) + issuable.with_transaction_returning_status do + add_label_ids = attributes.delete(:add_label_ids) + remove_label_ids = attributes.delete(:remove_label_ids) + + issuable.label_ids |= add_label_ids if add_label_ids + issuable.label_ids -= remove_label_ids if remove_label_ids + + issuable.assign_attributes(attributes.merge(updated_by: current_user)) + + issuable.save + end end def update(issuable) @@ -78,7 +104,7 @@ class IssuableBaseService < BaseService filter_params old_labels = issuable.labels.to_a - if params.present? && issuable.update_attributes(params.merge(updated_by: current_user)) + if params.present? && update_issuable(issuable, params) issuable.reset_events_cache handle_common_system_notes(issuable, old_labels: old_labels) handle_changes(issuable, old_labels: old_labels) diff --git a/app/services/issues/bulk_update_service.rb b/app/services/issues/bulk_update_service.rb index de8387c4900..15825b81685 100644 --- a/app/services/issues/bulk_update_service.rb +++ b/app/services/issues/bulk_update_service.rb @@ -4,9 +4,9 @@ module Issues issues_ids = params.delete(:issues_ids).split(",") issue_params = params - issue_params.delete(:state_event) unless issue_params[:state_event].present? - issue_params.delete(:milestone_id) unless issue_params[:milestone_id].present? - issue_params.delete(:assignee_id) unless issue_params[:assignee_id].present? + %i(state_event milestone_id assignee_id add_label_ids remove_label_ids).each do |key| + issue_params.delete(key) unless issue_params[key].present? + end issues = Issue.where(id: issues_ids) issues.each do |issue| diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 4701429215b..79b14819865 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -1,4 +1,4 @@ -%li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue) } +%li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue), data: { labels: issue.label_ids, id: issue.id } } - if controller.controller_name == 'issues' && can?(current_user, :admin_issue, @project) .issue-check = check_box_tag dom_id(issue,"selected"), nil, false, 'data-id' => issue.id, class: "selected_issue" diff --git a/app/views/projects/labels/_label.html.haml b/app/views/projects/labels/_label.html.haml index 8bf544b8371..294fec422c5 100644 --- a/app/views/projects/labels/_label.html.haml +++ b/app/views/projects/labels/_label.html.haml @@ -1,6 +1,5 @@ -%li{id: dom_id(label)} +%li{ id: dom_id(label), data: { id: label.id } } = render "shared/label_row", label: label - .pull-info-right %span.append-right-20 = link_to_label(label, type: :merge_request) do diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index cedff4af2e0..380ab465bf4 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -31,7 +31,7 @@ - if controller.controller_name == 'issues' .issues_bulk_update.hide - = form_tag bulk_update_namespace_project_issues_path(@project.namespace, @project), method: :post do + = form_tag bulk_update_namespace_project_issues_path(@project.namespace, @project), method: :post, class: 'bulk-update' do .filter-item.inline = dropdown_tag("Status", options: { toggle_class: "js-issue-status", title: "Change status", dropdown_class: "dropdown-menu-status dropdown-menu-selectable", data: { field_name: "update[state_event]" } } ) do %ul @@ -44,6 +44,10 @@ placeholder: "Search authors", data: { first_user: (current_user.username if current_user), null_user: true, current_user: true, project_id: @project.id, field_name: "update[assignee_id]" } }) .filter-item.inline = dropdown_tag("Milestone", options: { title: "Assign milestone", toggle_class: 'js-milestone-select js-extra-options js-filter-submit js-filter-bulk-update', filter: true, dropdown_class: "dropdown-menu-selectable dropdown-menu-milestone", placeholder: "Search milestones", data: { show_no: true, field_name: "update[milestone_id]", project_id: @project.id, milestones: namespace_project_milestones_path(@project.namespace, @project, :json), use_id: true } }) + + .filter-item.inline.labels-filter + = render "shared/issuable/label_dropdown", classes: ['js-filter-bulk-update', 'js-multiselect'], show_create: false, show_footer: false, extra_options: false, filter_submit: false, show_footer: false, data_options: { persist_when_hide: "true", field_name: "update[label_ids][]", show_no: false, show_any: false, use_id: true } + = hidden_field_tag 'update[issues_ids]', [] = hidden_field_tag :state_event, params[:state_event] .filter-item.inline diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index 61fd1e9c335..d34d28f6736 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -1,14 +1,25 @@ +- show_create = local_assigns.fetch(:show_create, true) +- extra_options = local_assigns.fetch(:extra_options, true) +- filter_submit = local_assigns.fetch(:filter_submit, true) +- show_footer = local_assigns.fetch(:show_footer, true) +- data_options = local_assigns.fetch(:data_options, {}) +- classes = local_assigns.fetch(:classes, []) +- dropdown_data = {toggle: 'dropdown', field_name: 'label_name[]', show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"} +- dropdown_data.merge!(data_options) +- classes << 'js-extra-options' if extra_options +- classes << 'js-filter-submit' if filter_submit + - if params[:label_name].present? - if params[:label_name].respond_to?('any?') - params[:label_name].each do |label| = hidden_field_tag "label_name[]", label, id: nil .dropdown - %button.dropdown-menu-toggle.js-label-select.js-filter-submit.js-multiselect.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "label_name[]", show_no: "true", show_any: "true", selected: params[:label_name], project_id: @project.try(:id), labels: labels_filter_path, default_label: "Label"}} + %button.dropdown-menu-toggle.js-label-select.js-multiselect{class: classes.join(' '), type: "button", data: dropdown_data} %span.dropdown-toggle-text = h(multi_label_name(params[:label_name], "Label")) = icon('chevron-down') .dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable - = render partial: "shared/issuable/label_page_default", locals: { title: "Filter by label" } - - if can? current_user, :admin_label, @project and @project + = render partial: "shared/issuable/label_page_default", locals: { title: "Filter by label", show_footer: show_footer, show_create: show_create } + - if show_create and @project and can?(current_user, :admin_label, @project) = render partial: "shared/issuable/label_page_create" = dropdown_loading diff --git a/app/views/shared/issuable/_label_page_default.html.haml b/app/views/shared/issuable/_label_page_default.html.haml index 7f4867417f7..4e280c371ac 100644 --- a/app/views/shared/issuable/_label_page_default.html.haml +++ b/app/views/shared/issuable/_label_page_default.html.haml @@ -1,20 +1,22 @@ - title = local_assigns.fetch(:title, 'Assign labels') +- show_create = local_assigns.fetch(:show_create, true) +- show_footer = local_assigns.fetch(:show_footer, true) - filter_placeholder = local_assigns.fetch(:filter_placeholder, 'Search labels') .dropdown-page-one = dropdown_title(title) = dropdown_filter(filter_placeholder) = dropdown_content - - if @project + - if @project && show_footer = dropdown_footer do %ul.dropdown-footer-list - - if can? current_user, :admin_label, @project + - if can?(current_user, :admin_label, @project) %li %a.dropdown-toggle-page{href: "#"} Create new %li = link_to namespace_project_labels_path(@project.namespace, @project), :"data-is-link" => true do - - if can? current_user, :admin_label, @project + - if show_create && @project && can?(current_user, :admin_label, @project) Manage labels - else View labels - = dropdown_loading
\ No newline at end of file + = dropdown_loading diff --git a/features/steps/project/issues/filter_labels.rb b/features/steps/project/issues/filter_labels.rb index d82c6856918..d34fa694789 100644 --- a/features/steps/project/issues/filter_labels.rb +++ b/features/steps/project/issues/filter_labels.rb @@ -29,7 +29,7 @@ class Spinach::Features::ProjectIssuesFilterLabels < Spinach::FeatureSteps end step 'I click link "bug"' do - page.find('.js-label-select').click + page.find('.js-label-select', visible: true).click sleep 0.5 execute_script("$('.dropdown-menu-labels li:contains(\"bug\") a').click()") end diff --git a/spec/features/issues/bulk_assigment_labels_spec.rb b/spec/features/issues/bulk_assigment_labels_spec.rb new file mode 100644 index 00000000000..c58b87281a3 --- /dev/null +++ b/spec/features/issues/bulk_assigment_labels_spec.rb @@ -0,0 +1,196 @@ +require 'rails_helper' + +feature 'Issues > Labels bulk assignment', feature: true do + include WaitForAjax + + let(:user) { create(:user) } + let!(:project) { create(:project) } + let!(:issue1) { create(:issue, project: project, title: "Issue 1") } + let!(:issue2) { create(:issue, project: project, title: "Issue 2") } + let!(:bug) { create(:label, project: project, title: 'bug') } + let!(:feature) { create(:label, project: project, title: 'feature') } + + context 'as a allowed user', js: true do + before do + project.team << [user, :master] + + login_as user + end + + context 'can bulk assign' do + before do + visit namespace_project_issues_path(project.namespace, project) + end + + context 'a label' do + context 'to all issues' do + before do + check 'check_all_issues' + open_labels_dropdown ['bug'] + update_issues + end + + it do + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue2.id}")).to have_content 'bug' + end + end + + context 'to a issue' do + before do + check "selected_issue_#{issue1.id}" + open_labels_dropdown ['bug'] + update_issues + end + + it do + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue2.id}")).not_to have_content 'bug' + end + end + end + + context 'multiple labels' do + context 'to all issues' do + before do + check 'check_all_issues' + open_labels_dropdown ['bug', 'feature'] + update_issues + end + + it do + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue1.id}")).to have_content 'feature' + expect(find("#issue_#{issue2.id}")).to have_content 'bug' + expect(find("#issue_#{issue2.id}")).to have_content 'feature' + end + end + + context 'to a issue' do + before do + check "selected_issue_#{issue1.id}" + open_labels_dropdown ['bug', 'feature'] + update_issues + end + + it do + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue1.id}")).to have_content 'feature' + expect(find("#issue_#{issue2.id}")).not_to have_content 'bug' + expect(find("#issue_#{issue2.id}")).not_to have_content 'feature' + end + end + end + end + + context 'can bulk un-assign' do + context 'all labels to all issues' do + before do + issue1.labels << bug + issue1.labels << feature + issue2.labels << bug + issue2.labels << feature + + visit namespace_project_issues_path(project.namespace, project) + + check 'check_all_issues' + unmark_labels_in_dropdown ['bug', 'feature'] + update_issues + end + + it do + expect(find("#issue_#{issue1.id}")).not_to have_content 'bug' + expect(find("#issue_#{issue1.id}")).not_to have_content 'feature' + expect(find("#issue_#{issue2.id}")).not_to have_content 'bug' + expect(find("#issue_#{issue2.id}")).not_to have_content 'feature' + end + end + + context 'a label to a issue' do + before do + issue1.labels << bug + issue2.labels << feature + + visit namespace_project_issues_path(project.namespace, project) + + check_issue issue1 + unmark_labels_in_dropdown ['bug'] + update_issues + end + + it do + expect(find("#issue_#{issue1.id}")).not_to have_content 'bug' + expect(find("#issue_#{issue2.id}")).to have_content 'feature' + end + end + + context 'a label and keep the others label' do + before do + issue1.labels << bug + issue1.labels << feature + issue2.labels << bug + issue2.labels << feature + + visit namespace_project_issues_path(project.namespace, project) + + check_issue issue1 + check_issue issue2 + unmark_labels_in_dropdown ['bug'] + update_issues + end + + it do + expect(find("#issue_#{issue1.id}")).not_to have_content 'bug' + expect(find("#issue_#{issue1.id}")).to have_content 'feature' + expect(find("#issue_#{issue2.id}")).not_to have_content 'bug' + expect(find("#issue_#{issue2.id}")).to have_content 'feature' + end + end + end + end + + context 'as a guest' do + before do + login_as user + + visit namespace_project_issues_path(project.namespace, project) + end + + context 'cannot bulk assign labels' do + it do + expect(page).not_to have_css '.check_all_issues' + expect(page).not_to have_css '.issue-check' + end + end + end + + def open_labels_dropdown(items = [], unmark = false) + page.within('.issues_bulk_update') do + click_button 'Label' + wait_for_ajax + items.map do |item| + click_link item + end + if unmark + items.map do |item| + click_link item + end + end + end + end + + def unmark_labels_in_dropdown(items = []) + open_labels_dropdown(items, true) + end + + def check_issue(issue) + page.within('.issues-list') do + check "selected_issue_#{issue.id}" + end + end + + def update_issues + click_button 'Update issues' + wait_for_ajax + end +end diff --git a/spec/features/issues/update_issues_spec.rb b/spec/features/issues/update_issues_spec.rb index 466a6f7dfa7..ddbd69b2891 100644 --- a/spec/features/issues/update_issues_spec.rb +++ b/spec/features/issues/update_issues_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' feature 'Multiple issue updating from issues#index', feature: true do + include WaitForAjax + let!(:project) { create(:project) } let!(:issue) { create(:issue, project: project) } let!(:user) { create(:user)} @@ -24,9 +26,7 @@ feature 'Multiple issue updating from issues#index', feature: true do it 'should be set to open' do create_closed - visit namespace_project_issues_path(project.namespace, project) - - find('.issues-state-filters a', text: 'Closed').click + visit namespace_project_issues_path(project.namespace, project, state: 'closed') find('#check_all_issues').click find('.js-issue-status').click @@ -42,7 +42,7 @@ feature 'Multiple issue updating from issues#index', feature: true do visit namespace_project_issues_path(project.namespace, project) find('#check_all_issues').click - find('.js-update-assignee').click + click_update_assignee_button find('.dropdown-menu-user-link', text: user.username).click click_update_issues_button @@ -57,14 +57,11 @@ feature 'Multiple issue updating from issues#index', feature: true do visit namespace_project_issues_path(project.namespace, project) find('#check_all_issues').click - find('.js-update-assignee').click + click_update_assignee_button click_link 'Unassigned' click_update_issues_button - - within first('.issue .controls') do - expect(page).to have_no_selector('.author_link') - end + expect(find('.issue:first-child .controls')).not_to have_css('.author_link') end end @@ -95,7 +92,7 @@ feature 'Multiple issue updating from issues#index', feature: true do find('.dropdown-menu-milestone a', text: "No Milestone").click click_update_issues_button - expect(first('.issue')).not_to have_content milestone.title + expect(find('.issue:first-child')).not_to have_content milestone.title end end @@ -111,7 +108,13 @@ feature 'Multiple issue updating from issues#index', feature: true do create(:issue, project: project, milestone: milestone) end + def click_update_assignee_button + find('.js-update-assignee').click + wait_for_ajax + end + def click_update_issues_button find('.update_selected_issues').click + wait_for_ajax end end diff --git a/spec/services/issues/bulk_update_service_spec.rb b/spec/services/issues/bulk_update_service_spec.rb index 454d5849495..4a689e64dc5 100644 --- a/spec/services/issues/bulk_update_service_spec.rb +++ b/spec/services/issues/bulk_update_service_spec.rb @@ -1,114 +1,265 @@ require 'spec_helper' describe Issues::BulkUpdateService, services: true do - let(:issue) { create(:issue, project: @project) } - - before do - @user = create :user - opts = { - name: "GitLab", - namespace: @user.namespace - } - @project = Projects::CreateService.new(@user, opts).execute - end + let(:user) { create(:user) } + let(:project) { Projects::CreateService.new(user, namespace: user.namespace, name: 'test').execute } - describe :close_issue do + let!(:result) { Issues::BulkUpdateService.new(project, user, params).execute } - before do - @issues = create_list(:issue, 5, project: @project) - @params = { + describe :close_issue do + let(:issues) { create_list(:issue, 5, project: project) } + let(:params) do + { state_event: 'close', - issues_ids: @issues.map(&:id).join(",") + issues_ids: issues.map(&:id).join(',') } end - it do - result = Issues::BulkUpdateService.new(@project, @user, @params).execute + it 'succeeds and returns the correct number of issues updated' do expect(result[:success]).to be_truthy - expect(result[:count]).to eq(@issues.count) - - expect(@project.issues.opened).to be_empty - expect(@project.issues.closed).not_to be_empty + expect(result[:count]).to eq(issues.count) end + it 'closes all the issues passed' do + expect(project.issues.opened).to be_empty + expect(project.issues.closed).not_to be_empty + end end describe :reopen_issues do - before do - @issues = create_list(:closed_issue, 5, project: @project) - @params = { + let(:issues) { create_list(:closed_issue, 5, project: project) } + let(:params) do + { state_event: 'reopen', - issues_ids: @issues.map(&:id).join(",") + issues_ids: issues.map(&:id).join(',') } end - it do - result = Issues::BulkUpdateService.new(@project, @user, @params).execute + it 'succeeds and returns the correct number of issues updated' do expect(result[:success]).to be_truthy - expect(result[:count]).to eq(@issues.count) - - expect(@project.issues.closed).to be_empty - expect(@project.issues.opened).not_to be_empty + expect(result[:count]).to eq(issues.count) end + it 'reopens all the issues passed' do + expect(project.issues.closed).to be_empty + expect(project.issues.opened).not_to be_empty + end end - describe :update_assignee do + describe 'updating assignee' do + let(:issue) do + create(:issue, project: project) { |issue| issue.update_attributes(assignee: user) } + end - before do - @new_assignee = create :user - @params = { - issues_ids: issue.id.to_s, - assignee_id: @new_assignee.id + let(:params) do + { + assignee_id: assignee_id, + issues_ids: issue.id.to_s } end - it do - result = Issues::BulkUpdateService.new(@project, @user, @params).execute - expect(result[:success]).to be_truthy - expect(result[:count]).to eq(1) + context 'when the new assignee ID is a valid user' do + let(:new_assignee) { create(:user) } + let(:assignee_id) { new_assignee.id } - expect(@project.issues.first.assignee).to eq(@new_assignee) - end + it 'succeeds' do + expect(result[:success]).to be_truthy + expect(result[:count]).to eq(1) + end - it 'allows mass-unassigning' do - @project.issues.first.update_attribute(:assignee, @new_assignee) - expect(@project.issues.first.assignee).not_to be_nil + it 'updates the assignee to the use ID passed' do + expect(issue.reload.assignee).to eq(new_assignee) + end + end - @params[:assignee_id] = -1 + context 'when the new assignee ID is -1' do + let(:assignee_id) { -1 } - Issues::BulkUpdateService.new(@project, @user, @params).execute - expect(@project.issues.first.assignee).to be_nil + it 'unassigns the issues' do + expect(issue.reload.assignee).to be_nil + end end - it 'does not unassign when assignee_id is not present' do - @project.issues.first.update_attribute(:assignee, @new_assignee) - expect(@project.issues.first.assignee).not_to be_nil - - @params[:assignee_id] = '' + context 'when the new assignee ID is not present' do + let(:assignee_id) { nil } - Issues::BulkUpdateService.new(@project, @user, @params).execute - expect(@project.issues.first.assignee).not_to be_nil + it 'does not unassign' do + expect(issue.reload.assignee).to eq(user) + end end end - describe :update_milestone do + describe 'updating milestones' do + let(:issue) { create(:issue, project: project) } + let(:milestone) { create(:milestone, project: project) } - before do - @milestone = create(:milestone, project: @project) - @params = { + let(:params) do + { issues_ids: issue.id.to_s, - milestone_id: @milestone.id + milestone_id: milestone.id } end - it do - result = Issues::BulkUpdateService.new(@project, @user, @params).execute + it 'succeeds' do expect(result[:success]).to be_truthy expect(result[:count]).to eq(1) + end - expect(@project.issues.first.milestone).to eq(@milestone) + it 'updates the issue milestone' do + expect(project.issues.first.milestone).to eq(milestone) end end + describe 'updating labels' do + def create_issue_with_labels(labels) + create(:issue, project: project) { |issue| issue.update_attributes(labels: labels) } + end + + let(:bug) { create(:label, project: project) } + let(:regression) { create(:label, project: project) } + let(:merge_requests) { create(:label, project: project) } + + let(:issue_all_labels) { create_issue_with_labels([bug, regression, merge_requests]) } + let(:issue_bug_and_regression) { create_issue_with_labels([bug, regression]) } + let(:issue_bug_and_merge_requests) { create_issue_with_labels([bug, merge_requests]) } + let(:issue_no_labels) { create(:issue, project: project) } + let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests, issue_no_labels] } + + let(:labels) { [] } + let(:add_labels) { [] } + let(:remove_labels) { [] } + + let(:params) do + { + label_ids: labels.map(&:id), + add_label_ids: add_labels.map(&:id), + remove_label_ids: remove_labels.map(&:id), + issues_ids: issues.map(&:id).join(',') + } + end + + context 'when label_ids are passed' do + let(:issues) { [issue_all_labels, issue_no_labels] } + let(:labels) { [bug, regression] } + + it 'updates the labels of all issues passed to the labels passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(eq(labels.map(&:id))) + end + + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end + + context 'when those label IDs are empty' do + let(:labels) { [] } + + it 'updates the issues passed to have no labels' do + expect(issues.map(&:reload).map(&:label_ids)).to all(be_empty) + end + end + end + + context 'when add_label_ids are passed' do + let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] } + let(:add_labels) { [bug, regression, merge_requests] } + + it 'adds those label IDs to all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(*add_labels.map(&:id))) + end + + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end + end + + context 'when remove_label_ids are passed' do + let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] } + let(:remove_labels) { [bug, regression, merge_requests] } + + it 'removes those label IDs from all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(be_empty) + end + + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end + end + + context 'when add_label_ids and remove_label_ids are passed' do + let(:issues) { [issue_all_labels, issue_bug_and_merge_requests, issue_no_labels] } + let(:add_labels) { [bug] } + let(:remove_labels) { [merge_requests] } + + it 'adds the label IDs to all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id)) + end + + it 'removes the label IDs from all issues passed' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id) + end + + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end + end + + context 'when add_label_ids and label_ids are passed' do + let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests] } + let(:labels) { [merge_requests] } + let(:add_labels) { [regression] } + + it 'adds the label IDs to all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(regression.id)) + end + + it 'ignores the label IDs parameter' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id)) + end + + it 'does not update issues not passed in' do + expect(issue_no_labels.label_ids).to be_empty + end + end + + context 'when remove_label_ids and label_ids are passed' do + let(:issues) { [issue_no_labels, issue_bug_and_regression] } + let(:labels) { [merge_requests] } + let(:remove_labels) { [regression] } + + it 'remove the label IDs from all issues passed' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(regression.id) + end + + it 'ignores the label IDs parameter' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id) + end + + it 'does not update issues not passed in' do + expect(issue_all_labels.label_ids).to contain_exactly(bug.id, regression.id, merge_requests.id) + end + end + + context 'when add_label_ids, remove_label_ids, and label_ids are passed' do + let(:issues) { [issue_bug_and_merge_requests, issue_no_labels] } + let(:labels) { [regression] } + let(:add_labels) { [bug] } + let(:remove_labels) { [merge_requests] } + + it 'adds the label IDs to all issues passed' do + expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id)) + end + + it 'removes the label IDs from all issues passed' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(merge_requests.id) + end + + it 'ignores the label IDs parameter' do + expect(issues.map(&:reload).map(&:label_ids).flatten).not_to include(regression.id) + end + + it 'does not update issues not passed in' do + expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id) + end + end + end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index be19be17151..dacbcd8fb46 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -1,3 +1,4 @@ +# coding: utf-8 require 'spec_helper' describe Issues::UpdateService, services: true do @@ -273,5 +274,50 @@ describe Issues::UpdateService, services: true do end end end + + context 'updating labels' do + let(:label3) { create(:label, project: project) } + let(:result) { Issues::UpdateService.new(project, user, params).execute(issue).reload } + + context 'when add_label_ids and label_ids are passed' do + let(:params) { { label_ids: [label.id], add_label_ids: [label3.id] } } + + it 'ignores the label_ids parameter' do + expect(result.label_ids).not_to include(label.id) + end + + it 'adds the passed labels' do + expect(result.label_ids).to include(label3.id) + end + end + + context 'when remove_label_ids and label_ids are passed' do + let(:params) { { label_ids: [], remove_label_ids: [label.id] } } + + before { issue.update_attributes(labels: [label, label3]) } + + it 'ignores the label_ids parameter' do + expect(result.label_ids).not_to be_empty + end + + it 'removes the passed labels' do + expect(result.label_ids).not_to include(label.id) + end + end + + context 'when add_label_ids and remove_label_ids are passed' do + let(:params) { { add_label_ids: [label3.id], remove_label_ids: [label.id] } } + + before { issue.update_attributes(labels: [label]) } + + it 'adds the passed labels' do + expect(result.label_ids).to include(label3.id) + end + + it 'removes the passed labels' do + expect(result.label_ids).not_to include(label.id) + end + end + end end end |