summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacob Schatz <jschatz@gitlab.com>2016-06-23 21:46:06 +0000
committerRobert Speicher <rspeicher@gmail.com>2016-06-23 18:17:22 -0400
commit115aa6f75e2ffac61740af0b8bf0b13a5a74cc3e (patch)
tree13c42e93e9cd4e105fc41245f282d648f6f2854e
parent7f8faa80d7648df30d31e9fc200fd875e780901e (diff)
downloadgitlab-ce-115aa6f75e2ffac61740af0b8bf0b13a5a74cc3e.tar.gz
Merge branch 'fix-bulk-assign' into 'master'
Fix unwanted label unassignment ## What does this MR do? - When updating the milestone - [x] Do not remove labels when assigning a milestone - [x] Do not remove labels when unassigning a milestone - [x] Do not remove labels when assigning a milestone and adding another label - When toggling selected issues labels should be kept - [x] Select an issue with an assigned label -> pick another label from dropdown-> unselect the issue -> select the issue again -> submit the form: Existing label should not be removed. ## Are there points in the code the reviewer needs to double check? Labels should not be added or removed to issues when doing bulk actions unless we explicitly select a label from the dropdown ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [ ] API support added - Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !4863
-rw-r--r--CHANGELOG1
-rw-r--r--app/assets/javascripts/issuable.js.coffee8
-rw-r--r--app/assets/javascripts/issues-bulk-assignment.js.coffee14
-rw-r--r--app/assets/javascripts/labels_select.js.coffee7
-rw-r--r--spec/features/issues/bulk_assignment_labels_spec.rb151
5 files changed, 173 insertions, 8 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 797941fa5ad..7e5bf35be5d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -25,6 +25,7 @@ v 8.9.1 (unreleased)
- Use jQuery objects in ref dropdown. !4850
- Fix GitLab project import issues related to notes and builds. !4855
- Restrict header logo to 36px so it doesn't overflow. !4861
+ - Fix unwanted label unassignment. !4863
- Fix merge requests project settings help link anchor. !4873
- Fix 404 when accessing pipelines as guest user on public projects. !4881
diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee
index d0901be1509..6a108c033ea 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.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 b454f9389dd..6b0e69dbae7 100644
--- a/app/assets/javascripts/issues-bulk-assignment.js.coffee
+++ b/app/assets/javascripts/issues-bulk-assignment.js.coffee
@@ -7,6 +7,11 @@ class @IssuableBulkActions
@issues = @getElement('.issues-list .issue')
} = opts
+ # Save instance
+ @form.data 'bulkActions', @
+
+ @willUpdateLabels = false
+
@bindEvents()
# Fixes bulk-assign not working when navigating through pages
@@ -87,11 +92,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..e95fd96a83f 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.willUpdateLabels = true
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