diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 7d588e8eee6..1ec950494ff 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -74,7 +74,9 @@ case 'projects:merge_requests:index': case 'projects:issues:index': Issuable.init(); - new gl.IssuableBulkActions(); + new gl.IssuableBulkActions({ + prefixId: page === 'projects:merge_requests:index' ? 'merge_request_' : 'issue_' + }); shortcut_handler = new ShortcutsNavigation(); break; case 'projects:issues:show': @@ -144,10 +146,6 @@ new ZenMode(); new MergedButtons(); break; - case 'projects:merge_requests:index': - shortcut_handler = new ShortcutsNavigation(); - Issuable.init(); - break; case 'dashboard:activity': new gl.Activities(); break; diff --git a/app/assets/javascripts/gl_dropdown.js b/app/assets/javascripts/gl_dropdown.js index 68a345d83f9..57dabfe05e4 100644 --- a/app/assets/javascripts/gl_dropdown.js +++ b/app/assets/javascripts/gl_dropdown.js @@ -23,7 +23,6 @@ this.filterInputBlur = (ref = this.options.filterInputBlur) != null ? ref : true; $inputContainer = this.input.parent(); $clearButton = $inputContainer.find('.js-dropdown-input-clear'); - this.indeterminateIds = []; $clearButton.on('click', (function(_this) { // Clear click return function(e) { @@ -348,12 +347,12 @@ $el = $(this); selected = self.rowClicked($el); if (self.options.clicked) { - self.options.clicked(selected, $el, e); + self.options.clicked(selected[0], $el, e, selected[1]); } // Update label right after all modifications in dropdown has been done if (self.options.toggleLabel) { - self.updateLabel(selected, $el, self); + self.updateLabel(selected[0], $el, self); } $el.trigger('blur'); @@ -444,12 +443,6 @@ this.resetRows(); this.addArrowKeyEvent(); - if (this.options.setIndeterminateIds) { - this.options.setIndeterminateIds.call(this); - } - if (this.options.setActiveIds) { - this.options.setActiveIds.call(this); - } // Makes indeterminate items effective if (this.fullData && this.dropdown.find('.dropdown-menu-toggle').hasClass('js-filter-bulk-update')) { this.parseData(this.fullData); @@ -483,11 +476,6 @@ if (this.options.filterable) { $input.blur().val(""); } - // 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 (!this.options.persistWhenHide) { - $input.trigger("input"); - } if (this.dropdown.find(".dropdown-toggle-page").length) { $('.dropdown-menu', this.dropdown).removeClass(PAGE_TWO_CLASS); } @@ -620,7 +608,8 @@ }; GitLabDropdown.prototype.rowClicked = function(el) { - var field, fieldName, groupName, isInput, selectedIndex, selectedObject, value; + var field, fieldName, groupName, isInput, selectedIndex, selectedObject, value, isMarking; + fieldName = this.options.fieldName; isInput = $(this.el).is('input'); if (this.renderedData) { @@ -641,7 +630,7 @@ el.addClass(ACTIVE_CLASS); } - return selectedObject; + return [selectedObject]; } field = []; @@ -659,6 +648,7 @@ } if (el.hasClass(ACTIVE_CLASS)) { + isMarking = false; el.removeClass(ACTIVE_CLASS); if (field && field.length) { if (isInput) { @@ -668,6 +658,7 @@ } } } else if (el.hasClass(INDETERMINATE_CLASS)) { + isMarking = true; el.addClass(ACTIVE_CLASS); el.removeClass(INDETERMINATE_CLASS); if (field && field.length && value == null) { @@ -677,6 +668,7 @@ this.addInput(fieldName, value, selectedObject); } } else { + isMarking = true; if (!this.options.multiSelect || el.hasClass('dropdown-clear-active')) { this.dropdown.find("." + ACTIVE_CLASS).removeClass(ACTIVE_CLASS); if (!isInput) { @@ -697,7 +689,7 @@ } } - return selectedObject; + return [selectedObject, isMarking]; }; GitLabDropdown.prototype.focusTextInput = function() { diff --git a/app/assets/javascripts/issuable.js.es6 b/app/assets/javascripts/issuable.js.es6 index b174eb2ff96..1c10a7445bb 100644 --- a/app/assets/javascripts/issuable.js.es6 +++ b/app/assets/javascripts/issuable.js.es6 @@ -144,6 +144,9 @@ const $issuesOtherFilters = $('.issues-other-filters'); const $issuesBulkUpdate = $('.issues_bulk_update'); + this.issuableBulkActions.willUpdateLabels = false; + this.issuableBulkActions.setOriginalDropdownData(); + if ($checkedIssues.length > 0) { let ids = $.map($checkedIssues, function(value) { return $(value).data('id'); @@ -155,7 +158,6 @@ $updateIssuesIds.val([]); $issuesBulkUpdate.hide(); $issuesOtherFilters.show(); - this.issuableBulkActions.willUpdateLabels = false; } return true; }, diff --git a/app/assets/javascripts/issues_bulk_assignment.js.es6 b/app/assets/javascripts/issues_bulk_assignment.js.es6 index ad25104152c..1c8e5dede6f 100644 --- a/app/assets/javascripts/issues_bulk_assignment.js.es6 +++ b/app/assets/javascripts/issues_bulk_assignment.js.es6 @@ -5,9 +5,10 @@ ((global) => { class IssuableBulkActions { - constructor({ container, form, issues } = {}) { - this.container = container || $('.content'), + constructor({ container, form, issues, prefixId } = {}) { + this.prefixId = prefixId || 'issue_'; this.form = form || this.getElement('.bulk-update'); + this.$labelDropdown = this.form.find('.js-label-select'); this.issues = issues || this.getElement('.issues-list .issue'); this.form.data('bulkActions', this); this.willUpdateLabels = false; @@ -16,10 +17,6 @@ Issuable.initChecks(); } - getElement(selector) { - return this.container.find(selector); - } - bindEvents() { return this.form.off('submit').on('submit', this.onFormSubmit.bind(this)); } @@ -73,10 +70,7 @@ getUnmarkedIndeterminedLabels() { const result = []; - const labelsToKeep = []; - - this.getElement('.labels-filter .is-indeterminate') - .each((i, el) => labelsToKeep.push($(el).data('labelId'))); + const labelsToKeep = this.$labelDropdown.data('indeterminate'); this.getLabelsFromSelection().forEach((id) => { if (labelsToKeep.indexOf(id) === -1) { @@ -106,45 +100,65 @@ } }; if (this.willUpdateLabels) { - this.getLabelsToApply().map(function(id) { - return formData.update.add_label_ids.push(id); - }); - this.getLabelsToRemove().map(function(id) { - return formData.update.remove_label_ids.push(id); - }); + formData.update.add_label_ids = this.$labelDropdown.data('marked'); + formData.update.remove_label_ids = this.$labelDropdown.data('unmarked'); } return formData; } - getLabelsToApply() { - const labelIds = []; - const $labels = this.form.find('.labels-filter input[name="update[label_ids][]"]'); - $labels.each(function(k, label) { - if (label) { - return labelIds.push(parseInt($(label).val())); - } - }); - return labelIds; + setOriginalDropdownData() { + let $labelSelect = $('.bulk-update .js-label-select'); + $labelSelect.data('common', this.getOriginalCommonIds()); + $labelSelect.data('marked', this.getOriginalMarkedIds()); + $labelSelect.data('indeterminate', this.getOriginalIndeterminateIds()); } + // From issuable's initial bulk selection + getOriginalCommonIds() { + let labelIds = []; - /** - * Returns Label IDs that will be removed from issue selection - * @return {Array} Array of labels IDs - */ - - getLabelsToRemove() { - const result = []; - const indeterminatedLabels = this.getUnmarkedIndeterminedLabels(); - const labelsToApply = this.getLabelsToApply(); - indeterminatedLabels.map(function(id) { - // We need to exclude label IDs that will be applied - // By not doing this will cause issues from selection to not add labels at all - if (labelsToApply.indexOf(id) === -1) { - return result.push(id); - } + this.getElement('.selected_issue:checked').each((i, el) => { + labelIds.push(this.getElement(`#${this.prefixId}${el.dataset.id}`).data('labels')); }); - return result; + return _.intersection.apply(this, labelIds); + } + + // From issuable's initial bulk selection + getOriginalMarkedIds() { + var labelIds = []; + this.getElement('.selected_issue:checked').each((i, el) => { + labelIds.push(this.getElement(`#${this.prefixId}${el.dataset.id}`).data('labels')); + }); + return _.intersection.apply(_, labelIds); + } + + // From issuable's initial bulk selection + getOriginalIndeterminateIds() { + let uniqueIds = []; + let labelIds = []; + let issuableLabels = []; + + // Collect unique label IDs for all checked issues + this.getElement('.selected_issue:checked').each((i, el) => { + issuableLabels = this.getElement(`#${this.prefixId}${el.dataset.id}`).data('labels'); + issuableLabels.forEach((labelId) => { + // Store unique IDs + if (uniqueIds.indexOf(labelId) === -1) { + uniqueIds.push(labelId); + } + }); + // Store array of IDs per issuable + labelIds.push(issuableLabels); + }); + // Add uniqueIds to add it as argument for _.intersection + labelIds.unshift(uniqueIds); + // Return IDs that are present but not in all selected issueables + return _.difference(uniqueIds, _.intersection.apply(this, labelIds)); + } + + getElement(selector) { + this.scopeEl = this.scopeEl || $('.content'); + return this.scopeEl.find(selector); } } diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index d29a5edb9ad..6853d6b9db2 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -8,8 +8,9 @@ var _this; _this = this; $('.js-label-select').each(function(i, dropdown) { - var $block, $colorPreview, $dropdown, $form, $loading, $selectbox, $sidebarCollapsedValue, $value, abilityName, defaultLabel, enableLabelCreateButton, issueURLSplit, issueUpdateURL, labelHTMLTemplate, labelNoneHTMLTemplate, labelUrl, namespacePath, projectPath, saveLabelData, selectedLabel, showAny, showNo, $sidebarLabelTooltip, initialSelected, $toggleText, fieldName, useId, propertyName, showMenuAbove; + var $block, $colorPreview, $dropdown, $form, $loading, $selectbox, $sidebarCollapsedValue, $value, abilityName, defaultLabel, enableLabelCreateButton, issueURLSplit, issueUpdateURL, labelHTMLTemplate, labelNoneHTMLTemplate, labelUrl, namespacePath, projectPath, saveLabelData, selectedLabel, showAny, showNo, $sidebarLabelTooltip, initialSelected, $toggleText, fieldName, useId, propertyName, showMenuAbove, $container; $dropdown = $(dropdown); + $dropdownContainer = $dropdown.closest('.labels-filter'); $toggleText = $dropdown.find('.dropdown-toggle-text'); namespacePath = $dropdown.data('namespace-path'); projectPath = $dropdown.data('project-path'); @@ -125,7 +126,7 @@ }); }); }; - return $dropdown.glDropdown({ + $dropdown.glDropdown({ showMenuAbove: showMenuAbove, data: function(term, callback) { return $.ajax({ @@ -172,33 +173,40 @@ }); }, renderRow: function(label, instance) { - var $a, $li, active, color, colorEl, indeterminate, removesAll, selectedClass, spacing; + var $a, $li, color, colorEl, indeterminate, removesAll, selectedClass, spacing, i, marked, dropdownName, dropdownValue; $li = $('
  • '); $a = $(''); selectedClass = []; removesAll = label.id <= 0 || (label.id == null); if ($dropdown.hasClass('js-filter-bulk-update')) { - indeterminate = instance.indeterminateIds; - active = instance.activeIds; + indeterminate = $dropdown.data('indeterminate') || []; + marked = $dropdown.data('marked') || []; + if (indeterminate.indexOf(label.id) !== -1) { selectedClass.push('is-indeterminate'); } - if (active.indexOf(label.id) !== -1) { + + if (marked.indexOf(label.id) !== -1) { // Remove is-indeterminate class if the item will be marked as active i = selectedClass.indexOf('is-indeterminate'); if (i !== -1) { selectedClass.splice(i, 1); } selectedClass.push('is-active'); - // Add input manually - instance.addInput(this.fieldName, label.id); } - } - if (this.id(label) && $form.find("input[type='hidden'][name='" + ($dropdown.data('fieldName')) + "'][value='" + this.id(label).toString().replace(/'/g, '\\\'') + "']").length) { - selectedClass.push('is-active'); - } - if ($dropdown.hasClass('js-multiselect') && removesAll) { - selectedClass.push('dropdown-clear-active'); + } else { + if (this.id(label)) { + dropdownName = $dropdown.data('fieldName'); + dropdownValue = this.id(label).toString().replace(/'/g, '\\\''); + + if ($form.find("input[type='hidden'][name='" + dropdownName + "'][value='" + dropdownValue + "']").length) { + selectedClass.push('is-active'); + } + } + + if ($dropdown.hasClass('js-multiselect') && removesAll) { + selectedClass.push('dropdown-clear-active'); + } } if (label.duplicate) { spacing = 100 / label.color.length; @@ -234,7 +242,6 @@ // Return generated html return $li.html($a).prop('outerHTML'); }, - persistWhenHide: $dropdown.data('persistWhenHide'), search: { fields: ['title'] }, @@ -313,18 +320,15 @@ } } } - if ($dropdown.hasClass('js-filter-bulk-update')) { - // If we are persisting state we need the classes - if (!this.options.persistWhenHide) { - return $dropdown.parent().find('.is-active, .is-indeterminate').removeClass(); - } - } }, multiSelect: $dropdown.hasClass('js-multiselect'), vue: $dropdown.hasClass('js-issue-board-sidebar'), - clicked: function(label, $el, e) { + clicked: function(label, $el, e, isMarking) { var isIssueIndex, isMRIndex, page; - _this.enableBulkLabelDropdown(); + + page = $('body').data('page'); + isIssueIndex = page === 'projects:issues:index'; + isMRIndex = page === 'projects:merge_requests:index'; if ($dropdown.parent().find('.is-active:not(.dropdown-clear-active)').length) { $dropdown.parent() @@ -333,12 +337,11 @@ } if ($dropdown.hasClass('js-filter-bulk-update') || $dropdown.hasClass('js-issuable-form-dropdown')) { + _this.enableBulkLabelDropdown(); + _this.setDropdownData($dropdown, isMarking, this.id(label)); return; } - page = $('body').data('page'); - isIssueIndex = page === 'projects:issues:index'; - isMRIndex = page === 'projects:merge_requests:index'; if ($('html').hasClass('issue-boards-page') && !$dropdown.hasClass('js-issue-board-sidebar')) { if (label.isAny) { gl.issueBoards.BoardsStore.state.filters['label_name'] = []; @@ -400,17 +403,10 @@ } } }, - setIndeterminateIds: function() { - if (this.dropdown.find('.dropdown-menu-toggle').hasClass('js-filter-bulk-update')) { - return this.indeterminateIds = _this.getIndeterminateIds(); - } - }, - setActiveIds: function() { - if (this.dropdown.find('.dropdown-menu-toggle').hasClass('js-filter-bulk-update')) { - return this.activeIds = _this.getActiveIds(); - } - } }); + + // Set dropdown data + _this.setOriginalDropdownData($dropdownContainer, $dropdown); }); this.bindEvents(); } @@ -423,34 +419,9 @@ if ($('.selected_issue:checked').length) { return; } - // Remove inputs - $('.issues_bulk_update .labels-filter input[type="hidden"]').remove(); - // Also restore button text return $('.issues_bulk_update .labels-filter .dropdown-toggle-text').text('Label'); }; - LabelsSelect.prototype.getIndeterminateIds = function() { - var label_ids; - label_ids = []; - $('.selected_issue:checked').each(function(i, el) { - var issue_id; - issue_id = $(el).data('id'); - return label_ids.push($("#issue_" + issue_id).data('labels')); - }); - return _.flatten(label_ids); - }; - - LabelsSelect.prototype.getActiveIds = function() { - var label_ids; - label_ids = []; - $('.selected_issue:checked').each(function(i, el) { - var issue_id; - issue_id = $(el).data('id'); - return label_ids.push($("#issue_" + issue_id).data('labels')); - }); - return _.intersection.apply(_, label_ids); - }; - LabelsSelect.prototype.enableBulkLabelDropdown = function() { var issuableBulkActions; if ($('.selected_issue:checked').length) { @@ -459,8 +430,59 @@ } }; - return LabelsSelect; + LabelsSelect.prototype.setDropdownData = function($dropdown, isMarking, value) { + var i, markedIds, unmarkedIds, indeterminateIds; + var issuableBulkActions = $('.bulk-update').data('bulkActions'); + markedIds = $dropdown.data('marked') || []; + unmarkedIds = $dropdown.data('unmarked') || []; + indeterminateIds = $dropdown.data('indeterminate') || []; + + if (isMarking) { + markedIds.push(value); + + i = indeterminateIds.indexOf(value); + if (i > -1) { + indeterminateIds.splice(i, 1); + } + + i = unmarkedIds.indexOf(value); + if (i > -1) { + unmarkedIds.splice(i, 1); + } + } else { + // If marked item (not common) is unmarked + i = markedIds.indexOf(value); + if (i > -1) { + markedIds.splice(i, 1); + } + + // If an indeterminate item is being unmarked + if (issuableBulkActions.getOriginalIndeterminateIds().indexOf(value) > -1) { + unmarkedIds.push(value); + } + + // If a marked item is being unmarked + // (a marked item could also be a label that is present in all selection) + if (issuableBulkActions.getOriginalCommonIds().indexOf(value) > -1) { + unmarkedIds.push(value); + } + } + + $dropdown.data('marked', markedIds); + $dropdown.data('unmarked', unmarkedIds); + $dropdown.data('indeterminate', indeterminateIds); + }; + + LabelsSelect.prototype.setOriginalDropdownData = function($container, $dropdown) { + var labels = []; + $container.find('[name="label_name[]"]').map(function() { + return labels.push(this.value); + }); + $dropdown.data('marked', labels); + }; + + return LabelsSelect; })(); }).call(this); diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index 959c796ecec..b3c43286a50 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -1,4 +1,4 @@ -%li{ class: mr_css_classes(merge_request) } +%li{ id: dom_id(merge_request), class: mr_css_classes(merge_request), data: { labels: merge_request.label_ids, id: merge_request.id } } - if @bulk_edit .issue-check = check_box_tag dom_id(merge_request, "selected"), nil, false, 'data-id' => merge_request.id, class: "selected_issue" diff --git a/changelogs/unreleased/24877-bulk-edit-only-keeps-common-labels-when-searching.yml b/changelogs/unreleased/24877-bulk-edit-only-keeps-common-labels-when-searching.yml new file mode 100644 index 00000000000..cc7c2604824 --- /dev/null +++ b/changelogs/unreleased/24877-bulk-edit-only-keeps-common-labels-when-searching.yml @@ -0,0 +1,4 @@ +--- +title: Improve bulk assignment for issuables +merge_request: +author: diff --git a/spec/features/issues/bulk_assignment_labels_spec.rb b/spec/features/issues/bulk_assignment_labels_spec.rb index bc2c087c9b9..832757b24d4 100644 --- a/spec/features/issues/bulk_assignment_labels_spec.rb +++ b/spec/features/issues/bulk_assignment_labels_spec.rb @@ -9,6 +9,7 @@ feature 'Issues > Labels bulk assignment', feature: true do 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') } + let!(:wontfix) { create(:label, project: project, title: 'wontfix') } context 'as an allowed user', js: true do before do @@ -291,6 +292,45 @@ feature 'Issues > Labels bulk assignment', feature: true do expect(find("#issue_#{issue1.id}")).not_to have_content 'feature' end end + + # Special case https://gitlab.com/gitlab-org/gitlab-ce/issues/24877 + context 'unmarking common label' do + before do + issue1.labels << bug + issue1.labels << feature + issue2.labels << bug + + visit namespace_project_issues_path(project.namespace, project) + end + + it 'applies label from filtered results' do + check 'check_all_issues' + + page.within('.issues_bulk_update') do + click_button 'Labels' + wait_for_ajax + + expect(find('.dropdown-menu-labels li', text: 'bug')).to have_css('.is-active') + expect(find('.dropdown-menu-labels li', text: 'feature')).to have_css('.is-indeterminate') + + click_link 'bug' + find('.dropdown-input-field', visible: true).set('wontfix') + click_link 'wontfix' + end + + update_issues + + page.within '.issues-holder' do + expect(find("#issue_#{issue1.id}")).not_to have_content 'bug' + expect(find("#issue_#{issue1.id}")).to have_content 'feature' + expect(find("#issue_#{issue1.id}")).to have_content 'wontfix' + + expect(find("#issue_#{issue2.id}")).not_to have_content 'bug' + expect(find("#issue_#{issue2.id}")).not_to have_content 'feature' + expect(find("#issue_#{issue2.id}")).to have_content 'wontfix' + end + end + end end context 'as a guest' do @@ -320,7 +360,7 @@ feature 'Issues > Labels bulk assignment', feature: true do def open_labels_dropdown(items = [], unmark = false) page.within('.issues_bulk_update') do - click_button 'Label' + click_button 'Labels' wait_for_ajax items.map do |item| click_link item