diff options
Diffstat (limited to 'spec/services/issues')
| -rw-r--r-- | spec/services/issues/close_service_spec.rb | 8 | ||||
| -rw-r--r-- | spec/services/issues/create_service_spec.rb | 86 | ||||
| -rw-r--r-- | spec/services/issues/move_service_spec.rb | 8 | ||||
| -rw-r--r-- | spec/services/issues/reopen_service_spec.rb | 4 | ||||
| -rw-r--r-- | spec/services/issues/update_service_spec.rb | 162 |
5 files changed, 183 insertions, 85 deletions
diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 0b315422be8..9a70de80123 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -81,7 +81,7 @@ RSpec.describe Issues::CloseService do describe '#close_issue' do context 'with external issue' do context 'with an active external issue tracker supporting close_issue' do - let!(:external_issue_tracker) { create(:jira_service, project: project) } + let!(:external_issue_tracker) { create(:jira_integration, project: project) } it 'closes the issue on the external issue tracker' do project.reload @@ -92,7 +92,7 @@ RSpec.describe Issues::CloseService do end context 'with inactive external issue tracker supporting close_issue' do - let!(:external_issue_tracker) { create(:jira_service, project: project, active: false) } + let!(:external_issue_tracker) { create(:jira_integration, project: project, active: false) } it 'does not close the issue on the external issue tracker' do project.reload @@ -323,7 +323,7 @@ RSpec.describe Issues::CloseService do context 'when issue is not confidential' do it 'executes issue hooks' do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) - expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) + expect(project).to receive(:execute_integrations).with(an_instance_of(Hash), :issue_hooks) described_class.new(project: project, current_user: user).close_issue(issue) end @@ -334,7 +334,7 @@ RSpec.describe Issues::CloseService do issue = create(:issue, :confidential, project: project) expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) - expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) + expect(project).to receive(:execute_integrations).with(an_instance_of(Hash), :confidential_issue_hooks) described_class.new(project: project, current_user: user).close_issue(issue) end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 94810d6134a..b073ffd291f 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -8,11 +8,17 @@ RSpec.describe Issues::CreateService do let_it_be_with_reload(:project) { create(:project) } let_it_be(:user) { create(:user) } + let(:spam_params) { double } + describe '#execute' do let_it_be(:assignee) { create(:user) } let_it_be(:milestone) { create(:milestone, project: project) } - let(:issue) { described_class.new(project: project, current_user: user, params: opts).execute } + let(:issue) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute } + + before do + stub_spam_services + end context 'when params are valid' do let_it_be(:labels) { create_pair(:label, project: project) } @@ -44,7 +50,7 @@ RSpec.describe Issues::CreateService do end context 'when skip_system_notes is true' do - let(:issue) { described_class.new(project: project, current_user: user, params: opts).execute(skip_system_notes: true) } + let(:issue) { described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute(skip_system_notes: true) } it 'does not call Issuable::CommonSystemNotesService' do expect(Issuable::CommonSystemNotesService).not_to receive(:new) @@ -92,7 +98,7 @@ RSpec.describe Issues::CreateService do let_it_be(:non_member) { create(:user) } it 'filters out params that cannot be set without the :set_issue_metadata permission' do - issue = described_class.new(project: project, current_user: non_member, params: opts).execute + issue = described_class.new(project: project, current_user: non_member, params: opts, spam_params: spam_params).execute expect(issue).to be_persisted expect(issue.title).to eq('Awesome issue') @@ -104,7 +110,7 @@ RSpec.describe Issues::CreateService do end it 'can create confidential issues' do - issue = described_class.new(project: project, current_user: non_member, params: { confidential: true }).execute + issue = described_class.new(project: project, current_user: non_member, params: { confidential: true }, spam_params: spam_params).execute expect(issue.confidential).to be_truthy end @@ -113,7 +119,7 @@ RSpec.describe Issues::CreateService do it 'moves the issue to the end, in an asynchronous worker' do expect(IssuePlacementWorker).to receive(:perform_async).with(be_nil, Integer) - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute end context 'when label belongs to project group' do @@ -200,7 +206,7 @@ RSpec.describe Issues::CreateService do it 'invalidates open issues counter for assignees when issue is assigned' do project.add_maintainer(assignee) - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(assignee.assigned_open_issues_count).to eq 1 end @@ -224,18 +230,18 @@ RSpec.describe Issues::CreateService do opts = { title: 'Title', description: 'Description', confidential: false } expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) - expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) + expect(project).to receive(:execute_integrations).with(an_instance_of(Hash), :issue_hooks) - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute end it 'executes confidential issue hooks when issue is confidential' do opts = { title: 'Title', description: 'Description', confidential: true } expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) - expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) + expect(project).to receive(:execute_integrations).with(an_instance_of(Hash), :confidential_issue_hooks) - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute end context 'after_save callback to store_mentions' do @@ -279,7 +285,7 @@ RSpec.describe Issues::CreateService do it 'removes assignee when user id is invalid' do opts = { title: 'Title', description: 'Description', assignee_ids: [-1] } - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.assignees).to be_empty end @@ -287,7 +293,7 @@ RSpec.describe Issues::CreateService do it 'removes assignee when user id is 0' do opts = { title: 'Title', description: 'Description', assignee_ids: [0] } - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.assignees).to be_empty end @@ -296,7 +302,7 @@ RSpec.describe Issues::CreateService do project.add_maintainer(assignee) opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.assignees).to eq([assignee]) end @@ -314,7 +320,7 @@ RSpec.describe Issues::CreateService do project.update!(visibility_level: level) opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.assignees).to be_empty end @@ -324,7 +330,7 @@ RSpec.describe Issues::CreateService do end it_behaves_like 'issuable record that supports quick actions' do - let(:issuable) { described_class.new(project: project, current_user: user, params: params).execute } + let(:issuable) { described_class.new(project: project, current_user: user, params: params, spam_params: spam_params).execute } end context 'Quick actions' do @@ -364,14 +370,14 @@ RSpec.describe Issues::CreateService do let(:opts) { { discussion_to_resolve: discussion.id, merge_request_to_resolve_discussions_of: merge_request.iid } } it 'resolves the discussion' do - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute discussion.first_note.reload expect(discussion.resolved?).to be(true) end it 'added a system note to the discussion' do - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first @@ -379,7 +385,7 @@ RSpec.describe Issues::CreateService do end it 'assigns the title and description for the issue' do - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.title).not_to be_nil expect(issue.description).not_to be_nil @@ -391,7 +397,8 @@ RSpec.describe Issues::CreateService do merge_request_to_resolve_discussions_of: merge_request, description: nil, title: nil - }).execute + }, + spam_params: spam_params).execute expect(issue.description).to be_nil expect(issue.title).to be_nil @@ -402,14 +409,14 @@ RSpec.describe Issues::CreateService do let(:opts) { { merge_request_to_resolve_discussions_of: merge_request.iid } } it 'resolves the discussion' do - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute discussion.first_note.reload expect(discussion.resolved?).to be(true) end it 'added a system note to the discussion' do - described_class.new(project: project, current_user: user, params: opts).execute + described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first @@ -417,7 +424,7 @@ RSpec.describe Issues::CreateService do end it 'assigns the title and description for the issue' do - issue = described_class.new(project: project, current_user: user, params: opts).execute + issue = described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute expect(issue.title).not_to be_nil expect(issue.description).not_to be_nil @@ -429,7 +436,8 @@ RSpec.describe Issues::CreateService do merge_request_to_resolve_discussions_of: merge_request, description: nil, title: nil - }).execute + }, + spam_params: spam_params).execute expect(issue.description).to be_nil expect(issue.title).to be_nil @@ -438,47 +446,27 @@ RSpec.describe Issues::CreateService do end context 'checking spam' do - let(:request) { double(:request, headers: nil) } - let(:api) { true } - let(:captcha_response) { 'abc123' } - let(:spam_log_id) { 1 } - let(:params) do { - title: 'Spam issue', - request: request, - api: api, - captcha_response: captcha_response, - spam_log_id: spam_log_id + title: 'Spam issue' } end subject do - described_class.new(project: project, current_user: user, params: params) - end - - before do - allow_next_instance_of(UserAgentDetailService) do |instance| - allow(instance).to receive(:create) - end + described_class.new(project: project, current_user: user, params: params, spam_params: spam_params) end it 'executes SpamActionService' do - spam_params = Spam::SpamParams.new( - api: api, - captcha_response: captcha_response, - spam_log_id: spam_log_id - ) expect_next_instance_of( Spam::SpamActionService, { - spammable: an_instance_of(Issue), - request: request, - user: user, + spammable: kind_of(Issue), + spam_params: spam_params, + user: an_instance_of(User), action: :create } ) do |instance| - expect(instance).to receive(:execute).with(spam_params: spam_params) + expect(instance).to receive(:execute) end subject.execute diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 76588860957..36af38aef18 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -38,6 +38,10 @@ RSpec.describe Issues::MoveService do context 'issue movable' do include_context 'user can move issue' + it 'creates resource state event' do + expect { move_service.execute(old_issue, new_project) }.to change(ResourceStateEvent.where(issue_id: old_issue), :count).by(1) + end + context 'generic issue' do include_context 'issue move executed' @@ -87,6 +91,10 @@ RSpec.describe Issues::MoveService do expect(old_issue.moved_to).to eq new_issue end + it 'marks issue as closed' do + expect(old_issue.closed?).to eq true + end + it 'preserves create time' do expect(old_issue.created_at).to eq new_issue.created_at end diff --git a/spec/services/issues/reopen_service_spec.rb b/spec/services/issues/reopen_service_spec.rb index 746a9105531..d58c27289c2 100644 --- a/spec/services/issues/reopen_service_spec.rb +++ b/spec/services/issues/reopen_service_spec.rb @@ -65,7 +65,7 @@ RSpec.describe Issues::ReopenService do context 'when issue is not confidential' do it 'executes issue hooks' do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) - expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) + expect(project).to receive(:execute_integrations).with(an_instance_of(Hash), :issue_hooks) described_class.new(project: project, current_user: user).execute(issue) end @@ -76,7 +76,7 @@ RSpec.describe Issues::ReopenService do issue = create(:issue, :confidential, :closed, project: project) expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) - expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) + expect(project).to receive(:execute_integrations).with(an_instance_of(Hash), :confidential_issue_hooks) described_class.new(project: project, current_user: user).execute(issue) end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index b95d94e3784..70c3c2a0f5d 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -83,16 +83,16 @@ RSpec.describe Issues::UpdateService, :mailer do end context 'when issue type is not incident' do - it 'returns default severity' do + before do update_issue(opts) - - expect(issue.severity).to eq(IssuableSeverity::DEFAULT) end - it_behaves_like 'not an incident issue' do - before do - update_issue(opts) - end + it_behaves_like 'not an incident issue' + + context 'when confidentiality is changed' do + subject { update_issue(confidential: true) } + + it_behaves_like 'does not track incident management event' end end @@ -105,12 +105,16 @@ RSpec.describe Issues::UpdateService, :mailer do it_behaves_like 'incident issue' - it 'changes updates the severity' do - expect(issue.severity).to eq('low') + it 'does not add an incident label' do + expect(issue.labels).to match_array [label] end - it 'does not apply incident labels' do - expect(issue.labels).to match_array [label] + context 'when confidentiality is changed' do + let(:current_user) { user } + + subject { update_issue(confidential: true) } + + it_behaves_like 'an incident management tracked event', :incident_management_incident_change_confidential end end @@ -140,24 +144,6 @@ RSpec.describe Issues::UpdateService, :mailer do expect(issue.confidential).to be_falsey end - context 'issue in incident type' do - let(:current_user) { user } - - before do - opts.merge!(issue_type: 'incident', confidential: true) - end - - subject { update_issue(opts) } - - it_behaves_like 'an incident management tracked event', :incident_management_incident_change_confidential - - it_behaves_like 'incident issue' do - before do - subject - end - end - end - context 'changing issue_type' do let!(:label_1) { create(:label, project: project, title: 'incident') } let!(:label_2) { create(:label, project: project, title: 'missed-sla') } @@ -167,6 +153,12 @@ RSpec.describe Issues::UpdateService, :mailer do end context 'from issue to incident' do + it_behaves_like 'incident issue' do + before do + update_issue(**opts, issue_type: 'incident') + end + end + it 'adds a `incident` label if one does not exist' do expect { update_issue(issue_type: 'incident') }.to change(issue.labels, :count).by(1) expect(issue.labels.pluck(:title)).to eq(['incident']) @@ -488,6 +480,21 @@ RSpec.describe Issues::UpdateService, :mailer do end end end + + it 'verifies the number of queries' do + update_issue(description: "- [ ] Task 1 #{user.to_reference}") + + baseline = ActiveRecord::QueryRecorder.new do + update_issue(description: "- [x] Task 1 #{user.to_reference}") + end + + recorded = ActiveRecord::QueryRecorder.new do + update_issue(description: "- [x] Task 1 #{user.to_reference}\n- [ ] Task 2 #{user.to_reference}") + end + + expect(recorded.count).to eq(baseline.count - 1) + expect(recorded.cached_count).to eq(0) + end end context 'when description changed' do @@ -522,7 +529,7 @@ RSpec.describe Issues::UpdateService, :mailer do it 'executes confidential issue hooks' do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) - expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) + expect(project).to receive(:execute_integrations).with(an_instance_of(Hash), :confidential_issue_hooks) update_issue(confidential: true) end @@ -1005,6 +1012,101 @@ RSpec.describe Issues::UpdateService, :mailer do include_examples 'updating mentions', described_class end + context 'updating severity' do + let(:opts) { { severity: 'low' } } + + shared_examples 'updates the severity' do |expected_severity| + it 'has correct value' do + update_issue(opts) + + expect(issue.severity).to eq(expected_severity) + end + + it 'creates a system note' do + expect(::IncidentManagement::AddSeveritySystemNoteWorker).to receive(:perform_async).with(issue.id, user.id) + + update_issue(opts) + end + + it 'triggers webhooks' do + expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) + expect(project).to receive(:execute_integrations).with(an_instance_of(Hash), :issue_hooks) + + update_issue(opts) + end + end + + shared_examples 'does not change the severity' do + it 'retains the original value' do + expected_severity = issue.severity + + update_issue(opts) + + expect(issue.severity).to eq(expected_severity) + end + + it 'does not trigger side-effects' do + expect(::IncidentManagement::AddSeveritySystemNoteWorker).not_to receive(:perform_async) + expect(project).not_to receive(:execute_hooks) + expect(project).not_to receive(:execute_integrations) + + expect { update_issue(opts) }.not_to change(IssuableSeverity, :count) + end + end + + context 'on incidents' do + let(:issue) { create(:incident, project: project) } + + context 'when severity has not been set previously' do + it_behaves_like 'updates the severity', 'low' + + it 'creates a new record' do + expect { update_issue(opts) }.to change(IssuableSeverity, :count).by(1) + end + + context 'with unsupported severity value' do + let(:opts) { { severity: 'unsupported-severity' } } + + it_behaves_like 'does not change the severity' + end + + context 'with severity value defined but unchanged' do + let(:opts) { { severity: IssuableSeverity::DEFAULT } } + + it_behaves_like 'does not change the severity' + end + end + + context 'when severity has been set before' do + before do + create(:issuable_severity, issue: issue, severity: 'high') + end + + it_behaves_like 'updates the severity', 'low' + + it 'does not create a new record' do + expect { update_issue(opts) }.not_to change(IssuableSeverity, :count) + end + + context 'with unsupported severity value' do + let(:opts) { { severity: 'unsupported-severity' } } + + it_behaves_like 'updates the severity', IssuableSeverity::DEFAULT + end + + context 'with severity value defined but unchanged' do + let(:opts) { { severity: issue.severity } } + + it_behaves_like 'does not change the severity' + end + end + end + + context 'when issue type is not incident' do + it_behaves_like 'does not change the severity' + end + end + context 'duplicate issue' do let(:canonical_issue) { create(:issue, project: project) } |