Commit 0710560c authored by Jacob Schatz's avatar Jacob Schatz

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
parents d61c69f4 56c36324
...@@ -14,6 +14,7 @@ v 8.9.1 ...@@ -14,6 +14,7 @@ v 8.9.1
- Fix GitLab project import issues related to notes and builds - Fix GitLab project import issues related to notes and builds
- Improve performance of searching repository tags by name by using a memorized tag array - Improve performance of searching repository tags by name by using a memorized tag array
- Fix false truncated warnings with ISO-8559 files - 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 - Fix 404 when accessing pipelines as guest user on public projects
v 8.9.0 v 8.9.0
......
...@@ -68,12 +68,15 @@ issuable_created = false ...@@ -68,12 +68,15 @@ issuable_created = false
Turbolinks.visit(issuesUrl); Turbolinks.visit(issuesUrl);
initChecks: -> initChecks: ->
@issuableBulkActions = $('.bulk-update').data('bulkActions')
$('.check_all_issues').off('click').on('click', -> $('.check_all_issues').off('click').on('click', ->
$('.selected_issue').prop('checked', @checked) $('.selected_issue').prop('checked', @checked)
Issuable.checkChanged() Issuable.checkChanged()
) )
$('.selected_issue').off('change').on('change', Issuable.checkChanged) $('.selected_issue').off('change').on('change', Issuable.checkChanged.bind(@))
checkChanged: -> checkChanged: ->
checked_issues = $('.selected_issue:checked') checked_issues = $('.selected_issue:checked')
...@@ -88,3 +91,6 @@ issuable_created = false ...@@ -88,3 +91,6 @@ issuable_created = false
$('#update_issues_ids').val [] $('#update_issues_ids').val []
$('.issues_bulk_update').hide() $('.issues_bulk_update').hide()
$('.issues-other-filters').show() $('.issues-other-filters').show()
@issuableBulkActions.willUpdateLabels = false
return true
...@@ -7,6 +7,11 @@ class @IssuableBulkActions ...@@ -7,6 +7,11 @@ class @IssuableBulkActions
@issues = @getElement('.issues-list .issue') @issues = @getElement('.issues-list .issue')
} = opts } = opts
# Save instance
@form.data 'bulkActions', @
@willUpdateLabels = false
@bindEvents() @bindEvents()
# Fixes bulk-assign not working when navigating through pages # Fixes bulk-assign not working when navigating through pages
...@@ -87,11 +92,12 @@ class @IssuableBulkActions ...@@ -87,11 +92,12 @@ class @IssuableBulkActions
add_label_ids : [] add_label_ids : []
remove_label_ids : [] remove_label_ids : []
@getLabelsToApply().map (id) -> if @willUpdateLabels
formData.update.add_label_ids.push id @getLabelsToApply().map (id) ->
formData.update.add_label_ids.push id
@getLabelsToRemove().map (id) -> @getLabelsToRemove().map (id) ->
formData.update.remove_label_ids.push id formData.update.remove_label_ids.push id
formData formData
......
...@@ -319,6 +319,8 @@ class @LabelsSelect ...@@ -319,6 +319,8 @@ class @LabelsSelect
multiSelect: $dropdown.hasClass 'js-multiselect' multiSelect: $dropdown.hasClass 'js-multiselect'
clicked: (label) -> clicked: (label) ->
_this.enableBulkLabelDropdown()
if $dropdown.hasClass('js-filter-bulk-update') if $dropdown.hasClass('js-filter-bulk-update')
return return
...@@ -377,3 +379,8 @@ class @LabelsSelect ...@@ -377,3 +379,8 @@ class @LabelsSelect
label_ids.push $("#issue_#{issue_id}").data('labels') label_ids.push $("#issue_#{issue_id}").data('labels')
_.intersection.apply _, label_ids _.intersection.apply _, label_ids
enableBulkLabelDropdown: ->
if $('.selected_issue:checked').length
issuableBulkActions = $('.bulk-update').data('bulkActions')
issuableBulkActions.willUpdateLabels = true
...@@ -10,7 +10,7 @@ feature 'Issues > Labels bulk assignment', feature: true do ...@@ -10,7 +10,7 @@ feature 'Issues > Labels bulk assignment', feature: true do
let!(:bug) { create(:label, project: project, title: 'bug') } let!(:bug) { create(:label, project: project, title: 'bug') }
let!(:feature) { create(:label, project: project, title: 'feature') } 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 before do
project.team << [user, :master] project.team << [user, :master]
...@@ -164,6 +164,133 @@ feature 'Issues > Labels bulk assignment', feature: true do ...@@ -164,6 +164,133 @@ feature 'Issues > Labels bulk assignment', feature: true do
end end
end 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 end
context 'as a guest' do context 'as a guest' do
...@@ -181,6 +308,16 @@ feature 'Issues > Labels bulk assignment', feature: true do ...@@ -181,6 +308,16 @@ feature 'Issues > Labels bulk assignment', feature: true do
end end
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) def open_labels_dropdown(items = [], unmark = false)
page.within('.issues_bulk_update') do page.within('.issues_bulk_update') do
click_button 'Label' click_button 'Label'
...@@ -201,12 +338,20 @@ feature 'Issues > Labels bulk assignment', feature: true do ...@@ -201,12 +338,20 @@ feature 'Issues > Labels bulk assignment', feature: true do
open_labels_dropdown(items, true) open_labels_dropdown(items, true)
end end
def check_issue(issue) def check_issue(issue, uncheck = false)
page.within('.issues-list') do 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
end end
def uncheck_issue(issue)
check_issue(issue, true)
end
def update_issues def update_issues
click_button 'Update issues' click_button 'Update issues'
wait_for_ajax wait_for_ajax
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment