From 618cf373ca5610d82e51b196cea113b9b7d03cc6 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 22 Jun 2016 14:58:38 -0500 Subject: Apply labels only if we picked a label from the dropdown --- .../javascripts/issues-bulk-assignment.js.coffee | 23 ++++++++++++++++++---- app/assets/javascripts/labels_select.js.coffee | 7 +++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/issues-bulk-assignment.js.coffee b/app/assets/javascripts/issues-bulk-assignment.js.coffee index b454f9389dd..8ca7409f5b4 100644 --- a/app/assets/javascripts/issues-bulk-assignment.js.coffee +++ b/app/assets/javascripts/issues-bulk-assignment.js.coffee @@ -7,11 +7,25 @@ class @IssuableBulkActions @issues = @getElement('.issues-list .issue') } = opts + # Save instance + @form.data 'bulkActions', @ + + @disableWillUpdate() + @bindEvents() # Fixes bulk-assign not working when navigating through pages Issuable.initChecks(); + enableWillUpdate: -> + @toggleWillUpdate(true) + + disableWillUpdate: -> + @toggleWillUpdate(false) + + toggleWillUpdate: (enable)-> + @willUpdateLabels = if enable? then enable else not @willUpdateLabels + getElement: (selector) -> @container.find selector @@ -87,11 +101,12 @@ class @IssuableBulkActions add_label_ids : [] remove_label_ids : [] - @getLabelsToApply().map (id) -> - formData.update.add_label_ids.push id + if @willUpdateLabels + @getLabelsToApply().map (id) -> + formData.update.add_label_ids.push id - @getLabelsToRemove().map (id) -> - formData.update.remove_label_ids.push id + @getLabelsToRemove().map (id) -> + formData.update.remove_label_ids.push id formData diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 6a10db10eb1..0729d5d91ce 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -319,6 +319,8 @@ class @LabelsSelect multiSelect: $dropdown.hasClass 'js-multiselect' clicked: (label) -> + _this.enableBulkLabelDropdown() + if $dropdown.hasClass('js-filter-bulk-update') return @@ -377,3 +379,8 @@ class @LabelsSelect label_ids.push $("#issue_#{issue_id}").data('labels') _.intersection.apply _, label_ids + + enableBulkLabelDropdown: -> + if $('.selected_issue:checked').length + issuableBulkActions = $('.bulk-update').data('bulkActions') + issuableBulkActions.enableWillUpdate() -- cgit v1.2.1 From 3d5172b5098afd8c1e8fa2eb970c5f6493ea9d0d Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 22 Jun 2016 17:05:45 -0500 Subject: Disable Label update when no issues are selected --- app/assets/javascripts/issuable.js.coffee | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee index d0901be1509..23c823942c7 100644 --- a/app/assets/javascripts/issuable.js.coffee +++ b/app/assets/javascripts/issuable.js.coffee @@ -68,12 +68,15 @@ issuable_created = false Turbolinks.visit(issuesUrl); initChecks: -> + @issuableBulkActions = $('.bulk-update').data('bulkActions') + $('.check_all_issues').off('click').on('click', -> $('.selected_issue').prop('checked', @checked) Issuable.checkChanged() ) - $('.selected_issue').off('change').on('change', Issuable.checkChanged) + $('.selected_issue').off('change').on('change', Issuable.checkChanged.bind(@)) + checkChanged: -> checked_issues = $('.selected_issue:checked') @@ -88,3 +91,6 @@ issuable_created = false $('#update_issues_ids').val [] $('.issues_bulk_update').hide() $('.issues-other-filters').show() + @issuableBulkActions.disableWillUpdate() + + return true -- cgit v1.2.1 From d18e407c409cf4b353797cb343a1c627e033e963 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 23 Jun 2016 06:57:30 -0500 Subject: Add tests to check fixes --- .../features/issues/bulk_assignment_labels_spec.rb | 151 ++++++++++++++++++++- 1 file changed, 148 insertions(+), 3 deletions(-) diff --git a/spec/features/issues/bulk_assignment_labels_spec.rb b/spec/features/issues/bulk_assignment_labels_spec.rb index 7143d0e40f3..afc093cc1f5 100644 --- a/spec/features/issues/bulk_assignment_labels_spec.rb +++ b/spec/features/issues/bulk_assignment_labels_spec.rb @@ -10,7 +10,7 @@ feature 'Issues > Labels bulk assignment', feature: true do let!(:bug) { create(:label, project: project, title: 'bug') } let!(:feature) { create(:label, project: project, title: 'feature') } - context 'as a allowed user', js: true do + context 'as an allowed user', js: true do before do project.team << [user, :master] @@ -164,6 +164,133 @@ feature 'Issues > Labels bulk assignment', feature: true do end end end + + context 'toggling a milestone' do + let!(:milestone) { create(:milestone, project: project, title: 'First Release') } + + context 'setting a milestone' do + before do + issue1.labels << bug + issue2.labels << feature + visit namespace_project_issues_path(project.namespace, project) + end + + it 'labels are kept' do + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue2.id}")).to have_content 'feature' + + check 'check_all_issues' + open_milestone_dropdown(['First Release']) + update_issues + + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue1.id}")).to have_content 'First Release' + expect(find("#issue_#{issue2.id}")).to have_content 'feature' + expect(find("#issue_#{issue2.id}")).to have_content 'First Release' + end + end + + context 'setting a milestone and adding another label' do + before do + issue1.labels << bug + + visit namespace_project_issues_path(project.namespace, project) + end + + it 'existing label is kept and new label is present' do + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + + check 'check_all_issues' + open_milestone_dropdown ['First Release'] + open_labels_dropdown ['feature'] + update_issues + + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue1.id}")).to have_content 'feature' + expect(find("#issue_#{issue1.id}")).to have_content 'First Release' + expect(find("#issue_#{issue2.id}")).to have_content 'feature' + expect(find("#issue_#{issue2.id}")).to have_content 'First Release' + end + end + + context 'setting a milestone and removing existing label' do + before do + issue1.labels << bug + issue1.labels << feature + issue2.labels << feature + + visit namespace_project_issues_path(project.namespace, project) + end + + it 'existing label is kept and new label is present' do + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue2.id}")).to have_content 'feature' + + check 'check_all_issues' + open_milestone_dropdown ['First Release'] + unmark_labels_in_dropdown ['feature'] + update_issues + + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue1.id}")).not_to have_content 'feature' + expect(find("#issue_#{issue1.id}")).to have_content 'First Release' + expect(find("#issue_#{issue2.id}")).not_to have_content 'feature' + expect(find("#issue_#{issue2.id}")).to have_content 'First Release' + end + end + + context 'unsetting a milestone' do + before do + issue1.milestone = milestone + issue2.milestone = milestone + issue1.save + issue2.save + issue1.labels << bug + issue2.labels << feature + + visit namespace_project_issues_path(project.namespace, project) + end + + it 'labels are kept' do + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue1.id}")).to have_content 'First Release' + expect(find("#issue_#{issue2.id}")).to have_content 'feature' + expect(find("#issue_#{issue2.id}")).to have_content 'First Release' + + check 'check_all_issues' + open_milestone_dropdown(['No Milestone']) + update_issues + + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue1.id}")).not_to have_content 'First Release' + expect(find("#issue_#{issue2.id}")).to have_content 'feature' + expect(find("#issue_#{issue2.id}")).not_to have_content 'First Release' + end + end + end + + context 'toggling checked issues' do + before do + issue1.labels << bug + + visit namespace_project_issues_path(project.namespace, project) + end + + it do + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + + check_issue issue1 + open_labels_dropdown ['feature'] + uncheck_issue issue1 + check_issue issue1 + update_issues + sleep 1 # needed + + expect(find("#issue_#{issue1.id}")).to have_content 'bug' + expect(find("#issue_#{issue1.id}")).not_to have_content 'feature' + end + end end context 'as a guest' do @@ -181,6 +308,16 @@ feature 'Issues > Labels bulk assignment', feature: true do end end + def open_milestone_dropdown(items = []) + page.within('.issues_bulk_update') do + click_button 'Milestone' + wait_for_ajax + items.map do |item| + click_link item + end + end + end + def open_labels_dropdown(items = [], unmark = false) page.within('.issues_bulk_update') do click_button 'Label' @@ -201,12 +338,20 @@ feature 'Issues > Labels bulk assignment', feature: true do open_labels_dropdown(items, true) end - def check_issue(issue) + def check_issue(issue, uncheck = false) page.within('.issues-list') do - check "selected_issue_#{issue.id}" + if uncheck + uncheck "selected_issue_#{issue.id}" + else + check "selected_issue_#{issue.id}" + end end end + def uncheck_issue(issue) + check_issue(issue, true) + end + def update_issues click_button 'Update issues' wait_for_ajax -- cgit v1.2.1 From 7c9909eb64ce346fc40a831493ffe8faef3d9d9d Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 23 Jun 2016 07:05:05 -0500 Subject: Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index fcfd0a21be3..5c87f86bf3a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 8.10.0 (unreleased) - Add Sidekiq queue duration to transaction metrics. - Fix MR-auto-close text added to description. !4836 - Fix pagination when sorting by columns with lots of ties (like priority) + - Fix unwanted label unassignment when doing bulk action on issues page - Implement Subresource Integrity for CSS and JavaScript assets. This prevents malicious assets from loading in the case of a CDN compromise. - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt) -- cgit v1.2.1 From 0e3208c40521000ad1b82ed80078b3ecc9dbaf76 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 23 Jun 2016 07:16:45 -0500 Subject: Use instance property to enable/disable label assignment --- app/assets/javascripts/issuable.js.coffee | 2 +- app/assets/javascripts/issues-bulk-assignment.js.coffee | 11 +---------- app/assets/javascripts/labels_select.js.coffee | 2 +- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee index 23c823942c7..6a108c033ea 100644 --- a/app/assets/javascripts/issuable.js.coffee +++ b/app/assets/javascripts/issuable.js.coffee @@ -91,6 +91,6 @@ issuable_created = false $('#update_issues_ids').val [] $('.issues_bulk_update').hide() $('.issues-other-filters').show() - @issuableBulkActions.disableWillUpdate() + @issuableBulkActions.willUpdateLabels = false return true diff --git a/app/assets/javascripts/issues-bulk-assignment.js.coffee b/app/assets/javascripts/issues-bulk-assignment.js.coffee index 8ca7409f5b4..6b0e69dbae7 100644 --- a/app/assets/javascripts/issues-bulk-assignment.js.coffee +++ b/app/assets/javascripts/issues-bulk-assignment.js.coffee @@ -10,22 +10,13 @@ class @IssuableBulkActions # Save instance @form.data 'bulkActions', @ - @disableWillUpdate() + @willUpdateLabels = false @bindEvents() # Fixes bulk-assign not working when navigating through pages Issuable.initChecks(); - enableWillUpdate: -> - @toggleWillUpdate(true) - - disableWillUpdate: -> - @toggleWillUpdate(false) - - toggleWillUpdate: (enable)-> - @willUpdateLabels = if enable? then enable else not @willUpdateLabels - getElement: (selector) -> @container.find selector diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 0729d5d91ce..e95fd96a83f 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -383,4 +383,4 @@ class @LabelsSelect enableBulkLabelDropdown: -> if $('.selected_issue:checked').length issuableBulkActions = $('.bulk-update').data('bulkActions') - issuableBulkActions.enableWillUpdate() + issuableBulkActions.willUpdateLabels = true -- cgit v1.2.1 From 56c36324a048c3e4c4668be9bc27bb92d6857cc9 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 23 Jun 2016 16:14:59 -0500 Subject: Update CHANGELOG --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 5c87f86bf3a..bb769087443 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,7 +5,6 @@ v 8.10.0 (unreleased) - Add Sidekiq queue duration to transaction metrics. - Fix MR-auto-close text added to description. !4836 - Fix pagination when sorting by columns with lots of ties (like priority) - - Fix unwanted label unassignment when doing bulk action on issues page - Implement Subresource Integrity for CSS and JavaScript assets. This prevents malicious assets from loading in the case of a CDN compromise. - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt) @@ -14,6 +13,7 @@ v 8.9.1 - Fix GitLab project import issues related to notes and builds - Improve performance of searching repository tags by name by using a memorized tag array - Fix false truncated warnings with ISO-8559 files + - Fix unwanted label unassignment when doing bulk action on issues page - Fix 404 when accessing pipelines as guest user on public projects v 8.9.0 -- cgit v1.2.1