Commit 97c4a1dc authored by Robert Speicher's avatar Robert Speicher

Refactor Issues::BulkUpdateService spec

- Create fewer Issue objects; 2 is as good as 5 for these cases. This
  gives us a nice little speed improvement.
- Don't `describe` Symbols.
- Simplify object creation.
- Lessen "mystery guest" antipattern
parent 9de37726
...@@ -18,5 +18,15 @@ FactoryGirl.define do ...@@ -18,5 +18,15 @@ FactoryGirl.define do
factory :closed_issue, traits: [:closed] factory :closed_issue, traits: [:closed]
factory :reopened_issue, traits: [:reopened] factory :reopened_issue, traits: [:reopened]
factory :labeled_issue do
transient do
labels []
end
after(:create) do |issue, evaluator|
issue.update_attributes(labels: evaluator.labels)
end
end
end end
end end
...@@ -2,89 +2,81 @@ require 'spec_helper' ...@@ -2,89 +2,81 @@ require 'spec_helper'
describe Issues::BulkUpdateService, services: true do describe Issues::BulkUpdateService, services: true do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { Projects::CreateService.new(user, namespace: user.namespace, name: 'test').execute } let(:project) { create(:empty_project, namespace: user.namespace) }
let!(:result) { Issues::BulkUpdateService.new(project, user, params).execute } def bulk_update(issues, extra_params = {})
bulk_update_params = extra_params
.reverse_merge(issues_ids: Array(issues).map(&:id).join(','))
describe :close_issue do Issues::BulkUpdateService.new(project, user, bulk_update_params).execute
let(:issues) { create_list(:issue, 5, project: project) }
let(:params) do
{
state_event: 'close',
issues_ids: issues.map(&:id).join(',')
}
end end
describe 'close issues' do
let(:issues) { create_list(:issue, 2, project: project) }
it 'succeeds and returns the correct number of issues updated' do it 'succeeds and returns the correct number of issues updated' do
result = bulk_update(issues, state_event: 'close')
expect(result[:success]).to be_truthy expect(result[:success]).to be_truthy
expect(result[:count]).to eq(issues.count) expect(result[:count]).to eq(issues.count)
end end
it 'closes all the issues passed' do it 'closes all the issues passed' do
bulk_update(issues, state_event: 'close')
expect(project.issues.opened).to be_empty expect(project.issues.opened).to be_empty
expect(project.issues.closed).not_to be_empty expect(project.issues.closed).not_to be_empty
end end
end end
describe :reopen_issues do describe 'reopen issues' do
let(:issues) { create_list(:closed_issue, 5, project: project) } let(:issues) { create_list(:closed_issue, 2, project: project) }
let(:params) do
{
state_event: 'reopen',
issues_ids: issues.map(&:id).join(',')
}
end
it 'succeeds and returns the correct number of issues updated' do it 'succeeds and returns the correct number of issues updated' do
result = bulk_update(issues, state_event: 'reopen')
expect(result[:success]).to be_truthy expect(result[:success]).to be_truthy
expect(result[:count]).to eq(issues.count) expect(result[:count]).to eq(issues.count)
end end
it 'reopens all the issues passed' do it 'reopens all the issues passed' do
bulk_update(issues, state_event: 'reopen')
expect(project.issues.closed).to be_empty expect(project.issues.closed).to be_empty
expect(project.issues.opened).not_to be_empty expect(project.issues.opened).not_to be_empty
end end
end end
describe 'updating assignee' do describe 'updating assignee' do
let(:issue) do let(:issue) { create(:issue, project: project, assignee: user) }
create(:issue, project: project) { |issue| issue.update_attributes(assignee: user) }
end
let(:params) do
{
assignee_id: assignee_id,
issues_ids: issue.id.to_s
}
end
context 'when the new assignee ID is a valid user' do 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 it 'succeeds' do
result = bulk_update(issue, assignee_id: create(:user).id)
expect(result[:success]).to be_truthy expect(result[:success]).to be_truthy
expect(result[:count]).to eq(1) expect(result[:count]).to eq(1)
end end
it 'updates the assignee to the use ID passed' do it 'updates the assignee to the use ID passed' do
expect(issue.reload.assignee).to eq(new_assignee) assignee = create(:user)
expect { bulk_update(issue, assignee_id: assignee.id) }
.to change { issue.reload.assignee }.from(user).to(assignee)
end end
end end
context 'when the new assignee ID is -1' do context 'when the new assignee ID is -1' do
let(:assignee_id) { -1 }
it 'unassigns the issues' do it 'unassigns the issues' do
expect(issue.reload.assignee).to be_nil expect { bulk_update(issue, assignee_id: -1) }
.to change { issue.reload.assignee }.to(nil)
end end
end end
context 'when the new assignee ID is not present' do context 'when the new assignee ID is not present' do
let(:assignee_id) { nil }
it 'does not unassign' do it 'does not unassign' do
expect(issue.reload.assignee).to eq(user) expect { bulk_update(issue, assignee_id: nil) }
.not_to change { issue.reload.assignee }
end end
end end
end end
...@@ -93,26 +85,22 @@ describe Issues::BulkUpdateService, services: true do ...@@ -93,26 +85,22 @@ describe Issues::BulkUpdateService, services: true do
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:milestone) { create(:milestone, 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 it 'succeeds' do
result = bulk_update(issue, milestone_id: milestone.id)
expect(result[:success]).to be_truthy expect(result[:success]).to be_truthy
expect(result[:count]).to eq(1) expect(result[:count]).to eq(1)
end end
it 'updates the issue milestone' do it 'updates the issue milestone' do
expect(project.issues.first.milestone).to eq(milestone) expect { bulk_update(issue, milestone_id: milestone.id) }
.to change { issue.reload.milestone }.from(nil).to(milestone)
end end
end end
describe 'updating labels' do describe 'updating labels' do
def create_issue_with_labels(labels) def create_issue_with_labels(labels)
create(:issue, project: project) { |issue| issue.update_attributes(labels: labels) } create(:labeled_issue, project: project, labels: labels)
end end
let(:bug) { create(:label, project: project) } let(:bug) { create(:label, project: project) }
...@@ -129,15 +117,18 @@ describe Issues::BulkUpdateService, services: true do ...@@ -129,15 +117,18 @@ describe Issues::BulkUpdateService, services: true do
let(:add_labels) { [] } let(:add_labels) { [] }
let(:remove_labels) { [] } let(:remove_labels) { [] }
let(:params) do let(:bulk_update_params) do
{ {
label_ids: labels.map(&:id), label_ids: labels.map(&:id),
add_label_ids: add_labels.map(&:id), add_label_ids: add_labels.map(&:id),
remove_label_ids: remove_labels.map(&:id), remove_label_ids: remove_labels.map(&:id),
issues_ids: issues.map(&:id).join(',')
} }
end end
before do
bulk_update(issues, bulk_update_params)
end
context 'when label_ids are passed' do context 'when label_ids are passed' do
let(:issues) { [issue_all_labels, issue_no_labels] } let(:issues) { [issue_all_labels, issue_no_labels] }
let(:labels) { [bug, regression] } let(:labels) { [bug, regression] }
...@@ -263,40 +254,28 @@ describe Issues::BulkUpdateService, services: true do ...@@ -263,40 +254,28 @@ describe Issues::BulkUpdateService, services: true do
end end
end end
describe :subscribe_issues do describe 'subscribe to issues' do
let(:issues) { create_list(:issue, 5, project: project) } let(:issues) { create_list(:issue, 2, project: project) }
let(:params) do
{
subscription_event: 'subscribe',
issues_ids: issues.map(&:id).join(',')
}
end
it 'subscribes the given user' do it 'subscribes the given user' do
issues.each do |issue| bulk_update(issues, subscription_event: 'subscribe')
expect(issue.subscribed?(user)).to be_truthy
end
end
end
describe :unsubscribe_issues do expect(issues).to all(be_subscribed(user))
let(:issues) { create_list(:closed_issue, 5, project: project) } end
let(:params) do
{
subscription_event: 'unsubscribe',
issues_ids: issues.map(&:id).join(',')
}
end end
before do describe 'unsubscribe from issues' do
issues.each do |issue| let(:issues) do
create_list(:closed_issue, 2, project: project) do |issue|
issue.subscriptions.create(user: user, subscribed: true) issue.subscriptions.create(user: user, subscribed: true)
end end
end end
it 'unsubscribes the given user' do it 'unsubscribes the given user' do
bulk_update(issues, subscription_event: 'unsubscribe')
issues.each do |issue| issues.each do |issue|
expect(issue.subscribed?(user)).to be_falsey expect(issue).not_to be_subscribed(user)
end end
end end
end end
......
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