Tidy up BulkUpdateService specs

1. Don't use instance variables, use `let` instead.
2. Add descriptions for all specs.
3. Share variables where possible.
4. Give labels more vivid names than 1, 2, and 3.
5. Remove deprecation warnings by passing issue IDs as '1,2,3' instead
   of an array, as that's how they're passed by the front-end. (The
   deprecation warning is for passing a nested array, which is what
   happens if an actual array is passed, as:
     `[1, 2, 3].split(',') == [[1, 2, 3]]`
This commit is contained in:
Sean McGivern 2016-05-02 17:36:25 +01:00 committed by Alfredo Sumaran
parent ac40843c94
commit 0fcf6e1094
1 changed files with 111 additions and 117 deletions

View File

@ -1,36 +1,28 @@
require 'spec_helper'
describe Issues::BulkUpdateService, services: true do
let(:issue) { create(:issue, project: @project) }
let(:user) { create(:user) }
let(:project) { Projects::CreateService.new(user, namespace: user.namespace, name: 'test').execute }
before do
@user = create :user
opts = {
name: "GitLab",
namespace: @user.namespace
}
@project = Projects::CreateService.new(@user, opts).execute
end
describe :close_issue do
let!(:result) { Issues::BulkUpdateService.new(project, user, params).execute }
before do
@issues = create_list(:issue, 5, project: @project)
@params = {
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
@ -38,95 +30,99 @@ describe Issues::BulkUpdateService, services: true do
@issues = create_list(:closed_issue, 5, project: @project)
@params = {
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
end
describe :update_assignee do
before do
@new_assignee = create :user
@params = {
issues_ids: issue.id.to_s,
assignee_id: @new_assignee.id
}
end
it do
result = Issues::BulkUpdateService.new(@project, @user, @params).execute
expect(result[:success]).to be_truthy
expect(result[:count]).to eq(1)
expect(@project.issues.first.assignee).to eq(@new_assignee)
end
it 'allows mass-unassigning' do
@project.issues.first.update_attribute(:assignee, @new_assignee)
expect(@project.issues.first.assignee).not_to be_nil
@params[:assignee_id] = -1
Issues::BulkUpdateService.new(@project, @user, @params).execute
expect(@project.issues.first.assignee).to be_nil
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] = ''
Issues::BulkUpdateService.new(@project, @user, @params).execute
expect(@project.issues.first.assignee).not_to be_nil
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_milestone do
describe 'updating assignee' do
let(:issue) do
create(:issue, project: project) { |issue| issue.update_attributes(assignee: user) }
end
before do
@milestone = create(:milestone, project: @project)
@params = {
issues_ids: issue.id.to_s,
milestone_id: @milestone.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
context 'when the new assignee ID is a valid user' do
let(:new_assignee) { create(:user) }
let(:assignee_id) { new_assignee.id }
it 'succeeds' do
expect(result[:success]).to be_truthy
expect(result[:count]).to eq(1)
end
it 'updates the assignee to the use ID passed' do
expect(issue.reload.assignee).to eq(new_assignee)
end
end
context 'when the new assignee ID is -1' do
let(:assignee_id) { -1 }
it 'unassigns the issues' do
expect(issue.reload.assignee).to be_nil
end
end
context 'when the new assignee ID is not present', focus: true do
let(:assignee_id) { nil }
it 'does not unassign' do
expect(issue.reload.assignee).to eq(user)
end
end
end
describe 'updating milestones' do
let(:issue) { create(:issue, project: project) }
let(:milestone) { create(:milestone, project: project) }
let(:params) do
{
issues_ids: issue.id.to_s,
milestone_id: milestone.id
}
end
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.labels = labels }
create(:issue, project: project) { |issue| issue.update_attributes(labels: labels) }
end
let(:user) { create(:user) }
let(:project) { Projects::CreateService.new(user, namespace: user.namespace, name: 'test').execute }
let(:label_1) { create(:label, project: project) }
let(:label_2) { create(:label, project: project) }
let(:label_3) { create(:label, project: project) }
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([label_1, label_2, label_3]) }
let(:issue_labels_1_and_2) { create_issue_with_labels([label_1, label_2]) }
let(:issue_labels_1_and_3) { create_issue_with_labels([label_1, label_3]) }
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_labels_1_and_2, issue_labels_1_and_3, issue_no_labels] }
let(:issues) { [issue_all_labels, issue_bug_and_regression, issue_bug_and_merge_requests, issue_no_labels] }
let(:labels) { [] }
let(:add_labels) { [] }
@ -141,76 +137,74 @@ describe Issues::BulkUpdateService, services: true do
}
end
before { Issues::BulkUpdateService.new(project, user, params).execute }
context 'when label_ids are passed' do
let(:issues) { [issue_all_labels, issue_no_labels] }
let(:labels) { [label_1, label_2] }
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_labels_1_and_2.label_ids).to contain_exactly(label_1.id, label_2.id)
expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
end
end
context 'when add_label_ids are passed' do
let(:issues) { [issue_all_labels, issue_labels_1_and_3, issue_no_labels] }
let(:add_labels) { [label_1, label_2, label_3] }
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_labels_1_and_2.label_ids).to contain_exactly(label_1.id, label_2.id)
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_labels_1_and_3, issue_no_labels] }
let(:remove_labels) { [label_1, label_2, label_3] }
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_labels_1_and_2.label_ids).to contain_exactly(label_1.id, label_2.id)
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_labels_1_and_3, issue_no_labels] }
let(:add_labels) { [label_1] }
let(:remove_labels) { [label_3] }
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(label_1.id))
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(label_3.id)
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_labels_1_and_2.label_ids).to contain_exactly(label_1.id, label_2.id)
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_labels_1_and_2, issue_labels_1_and_3] }
let(:labels) { [label_3] }
let(:add_labels) { [label_2] }
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(label_2.id))
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(label_1.id))
expect(issues.map(&:reload).map(&:label_ids)).to all(include(bug.id))
end
it 'does not update issues not passed in' do
@ -219,43 +213,43 @@ describe Issues::BulkUpdateService, services: true do
end
context 'when remove_label_ids and label_ids are passed' do
let(:issues) { [issue_no_labels, issue_labels_1_and_2] }
let(:labels) { [label_3] }
let(:remove_labels) { [label_2] }
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(label_2.id)
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(label_3.id)
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(label_1.id, label_2.id, label_3.id)
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_labels_1_and_3, issue_no_labels] }
let(:labels) { [label_2] }
let(:add_labels) { [label_1] }
let(:remove_labels) { [label_3] }
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(label_1.id))
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(label_3.id)
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(label_2.id)
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_labels_1_and_2.label_ids).to contain_exactly(label_1.id, label_2.id)
expect(issue_bug_and_regression.label_ids).to contain_exactly(bug.id, regression.id)
end
end
end