diff options
Diffstat (limited to 'spec/services/issues')
| -rw-r--r-- | spec/services/issues/build_service_spec.rb | 96 | ||||
| -rw-r--r-- | spec/services/issues/close_service_spec.rb | 44 | ||||
| -rw-r--r-- | spec/services/issues/create_service_spec.rb | 79 | ||||
| -rw-r--r-- | spec/services/issues/set_crm_contacts_service_spec.rb | 162 | ||||
| -rw-r--r-- | spec/services/issues/update_service_spec.rb | 42 |
5 files changed, 339 insertions, 84 deletions
diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index b96dd981e0f..cf75efb5c57 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -7,12 +7,14 @@ RSpec.describe Issues::BuildService do let_it_be(:project) { create(:project, :repository) } let_it_be(:developer) { create(:user) } + let_it_be(:reporter) { create(:user) } let_it_be(:guest) { create(:user) } let(:user) { developer } before_all do project.add_developer(developer) + project.add_reporter(reporter) project.add_guest(guest) end @@ -140,76 +142,64 @@ RSpec.describe Issues::BuildService do end describe '#execute' do - context 'as developer' do - it 'builds a new issues with given params' do - milestone = create(:milestone, project: project) - issue = build_issue(milestone_id: milestone.id) - - expect(issue.milestone).to eq(milestone) - expect(issue.issue_type).to eq('issue') - expect(issue.work_item_type.base_type).to eq('issue') - end + describe 'setting milestone' do + context 'when developer' do + it 'builds a new issues with given params' do + milestone = create(:milestone, project: project) + issue = build_issue(milestone_id: milestone.id) + + expect(issue.milestone).to eq(milestone) + end - it 'sets milestone to nil if it is not available for the project' do - milestone = create(:milestone, project: create(:project)) - issue = build_issue(milestone_id: milestone.id) + it 'sets milestone to nil if it is not available for the project' do + milestone = create(:milestone, project: create(:project)) + issue = build_issue(milestone_id: milestone.id) - expect(issue.milestone).to be_nil + expect(issue.milestone).to be_nil + end end - context 'when issue_type is incident' do - it 'sets the correct issue type' do - issue = build_issue(issue_type: 'incident') + context 'when guest' do + let(:user) { guest } - expect(issue.issue_type).to eq('incident') - expect(issue.work_item_type.base_type).to eq('incident') + it 'cannot set milestone' do + milestone = create(:milestone, project: project) + issue = build_issue(milestone_id: milestone.id) + + expect(issue.milestone).to be_nil end end end - context 'as guest' do - let(:user) { guest } - - it 'cannot set milestone' do - milestone = create(:milestone, project: project) - issue = build_issue(milestone_id: milestone.id) + describe 'setting issue type' do + context 'with a corresponding WorkItem::Type' do + let_it_be(:type_issue_id) { WorkItem::Type.default_issue_type.id } + let_it_be(:type_incident_id) { WorkItem::Type.default_by_type(:incident).id } + + where(:issue_type, :current_user, :work_item_type_id, :resulting_issue_type) do + nil | ref(:guest) | ref(:type_issue_id) | 'issue' + 'issue' | ref(:guest) | ref(:type_issue_id) | 'issue' + 'incident' | ref(:guest) | ref(:type_issue_id) | 'issue' + 'incident' | ref(:reporter) | ref(:type_incident_id) | 'incident' + # update once support for test_case is enabled + 'test_case' | ref(:guest) | ref(:type_issue_id) | 'issue' + # update once support for requirement is enabled + 'requirement' | ref(:guest) | ref(:type_issue_id) | 'issue' + 'invalid' | ref(:guest) | ref(:type_issue_id) | 'issue' + # ensure that we don't set a value which has a permission check but is an invalid issue type + 'project' | ref(:guest) | ref(:type_issue_id) | 'issue' + end - expect(issue.milestone).to be_nil - end + with_them do + let(:user) { current_user } - context 'setting issue type' do - shared_examples 'builds an issue' do - specify do + it 'builds an issue' do issue = build_issue(issue_type: issue_type) expect(issue.issue_type).to eq(resulting_issue_type) expect(issue.work_item_type_id).to eq(work_item_type_id) end end - - it 'cannot set invalid issue type' do - issue = build_issue(issue_type: 'project') - - expect(issue).to be_issue - end - - context 'with a corresponding WorkItem::Type' do - let_it_be(:type_issue_id) { WorkItem::Type.default_issue_type.id } - let_it_be(:type_incident_id) { WorkItem::Type.default_by_type(:incident).id } - - where(:issue_type, :work_item_type_id, :resulting_issue_type) do - nil | ref(:type_issue_id) | 'issue' - 'issue' | ref(:type_issue_id) | 'issue' - 'incident' | ref(:type_incident_id) | 'incident' - 'test_case' | ref(:type_issue_id) | 'issue' # update once support for test_case is enabled - 'requirement' | ref(:type_issue_id) | 'issue' # update once support for requirement is enabled - 'invalid' | ref(:type_issue_id) | 'issue' - end - - with_them do - it_behaves_like 'builds an issue' - end - end end end end diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 93ef046a632..158f9dec83e 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -83,6 +83,14 @@ RSpec.describe Issues::CloseService do service.execute(issue) end + it 'does not change escalation status' do + resolved = IncidentManagement::Escalatable::STATUSES[:resolved] + + expect { service.execute(issue) } + .to not_change { IncidentManagement::IssuableEscalationStatus.where(issue: issue).count } + .and not_change { IncidentManagement::IssuableEscalationStatus.where(status: resolved).count } + end + context 'issue is incident type' do let(:issue) { create(:incident, project: project) } let(:current_user) { user } @@ -90,6 +98,40 @@ RSpec.describe Issues::CloseService do subject { service.execute(issue) } it_behaves_like 'an incident management tracked event', :incident_management_incident_closed + + it 'creates a new escalation resolved escalation status', :aggregate_failures do + expect { service.execute(issue) }.to change { IncidentManagement::IssuableEscalationStatus.where(issue: issue).count }.by(1) + + expect(issue.incident_management_issuable_escalation_status).to be_resolved + end + + context 'when there is an escalation status' do + before do + create(:incident_management_issuable_escalation_status, issue: issue) + end + + it 'changes escalations status to resolved' do + expect { service.execute(issue) }.to change { issue.incident_management_issuable_escalation_status.reload.resolved? }.to(true) + end + + it 'adds a system note', :aggregate_failures do + expect { service.execute(issue) }.to change { issue.notes.count }.by(1) + + new_note = issue.notes.last + expect(new_note.note).to eq('changed the status to **Resolved** by closing the incident') + expect(new_note.author).to eq(user) + end + + context 'when the escalation status did not change to resolved' do + let(:escalation_status) { instance_double('IncidentManagement::IssuableEscalationStatus', resolve: false) } + + it 'does not create a system note' do + allow(issue).to receive(:incident_management_issuable_escalation_status).and_return(escalation_status) + + expect { service.execute(issue) }.not_to change { issue.notes.count } + end + end + end end end @@ -237,7 +279,7 @@ RSpec.describe Issues::CloseService do it 'verifies the number of queries' do recorded = ActiveRecord::QueryRecorder.new { close_issue } - expected_queries = 27 + expected_queries = 32 expect(recorded.count).to be <= expected_queries expect(recorded.cached_count).to eq(0) diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 1887be4896e..18e03db11dc 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -15,8 +15,7 @@ RSpec.describe Issues::CreateService do expect(described_class.rate_limiter_scoped_and_keyed).to be_a(RateLimitedService::RateLimiterScopedAndKeyed) expect(described_class.rate_limiter_scoped_and_keyed.key).to eq(:issues_create) - expect(described_class.rate_limiter_scoped_and_keyed.opts[:scope]).to eq(%i[project current_user]) - expect(described_class.rate_limiter_scoped_and_keyed.opts[:users_allowlist].call).to eq(%w[support-bot]) + expect(described_class.rate_limiter_scoped_and_keyed.opts[:scope]).to eq(%i[project current_user external_author]) expect(described_class.rate_limiter_scoped_and_keyed.rate_limiter_klass).to eq(Gitlab::ApplicationRateLimiter) end end @@ -81,7 +80,7 @@ RSpec.describe Issues::CreateService do it_behaves_like 'not an incident issue' - context 'issue is incident type' do + context 'when issue is incident type' do before do opts.merge!(issue_type: 'incident') end @@ -91,23 +90,37 @@ RSpec.describe Issues::CreateService do subject { issue } - it_behaves_like 'incident issue' - it_behaves_like 'has incident label' + context 'as reporter' do + let_it_be(:reporter) { create(:user) } - it 'does create an incident label' do - expect { subject } - .to change { Label.where(incident_label_attributes).count }.by(1) - end + let(:user) { reporter } - context 'when invalid' do - before do - opts.merge!(title: '') + before_all do + project.add_reporter(reporter) end - it 'does not apply an incident label prematurely' do - expect { subject }.to not_change(LabelLink, :count).and not_change(Issue, :count) + it_behaves_like 'incident issue' + it_behaves_like 'has incident label' + + it 'does create an incident label' do + expect { subject } + .to change { Label.where(incident_label_attributes).count }.by(1) + end + + context 'when invalid' do + before do + opts.merge!(title: '') + end + + it 'does not apply an incident label prematurely' do + expect { subject }.to not_change(LabelLink, :count).and not_change(Issue, :count) + end end end + + context 'as guest' do + it_behaves_like 'not an incident issue' + end end it 'refreshes the number of open issues', :use_clean_rails_memory_store_caching do @@ -289,6 +302,44 @@ RSpec.describe Issues::CreateService do described_class.new(project: project, current_user: user, params: opts, spam_params: spam_params).execute end + context 'when rate limiting is in effect', :freeze_time, :clean_gitlab_redis_rate_limiting do + let(:user) { create(:user) } + + before do + stub_feature_flags(rate_limited_service_issues_create: true) + stub_application_setting(issues_create_limit: 1) + end + + subject do + 2.times { described_class.new(project: project, current_user: user, params: opts, spam_params: double).execute } + end + + context 'when too many requests are sent by one user' do + it 'raises an error' do + expect do + subject + end.to raise_error(RateLimitedService::RateLimitedError) + end + + it 'creates 1 issue' do + expect do + subject + rescue RateLimitedService::RateLimitedError + end.to change { Issue.count }.by(1) + end + end + + context 'when limit is higher than count of issues being created' do + before do + stub_application_setting(issues_create_limit: 2) + end + + it 'creates 2 issues' do + expect { subject }.to change { Issue.count }.by(2) + end + end + end + context 'after_save callback to store_mentions' do context 'when mentionable attributes change' do let(:opts) { { title: 'Title', description: "Description with #{user.to_reference}" } } diff --git a/spec/services/issues/set_crm_contacts_service_spec.rb b/spec/services/issues/set_crm_contacts_service_spec.rb new file mode 100644 index 00000000000..65b22fe3b35 --- /dev/null +++ b/spec/services/issues/set_crm_contacts_service_spec.rb @@ -0,0 +1,162 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Issues::SetCrmContactsService do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:contacts) { create_list(:contact, 4, group: group) } + + let(:issue) { create(:issue, project: project) } + let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" } + + before do + create(:issue_customer_relations_contact, issue: issue, contact: contacts[0]) + create(:issue_customer_relations_contact, issue: issue, contact: contacts[1]) + end + + subject(:set_crm_contacts) do + described_class.new(project: project, current_user: user, params: params).execute(issue) + end + + describe '#execute' do + context 'when the user has no permission' do + let(:params) { { crm_contact_ids: [contacts[1].id, contacts[2].id] } } + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array(['You have insufficient permissions to set customer relations contacts for this issue']) + end + end + + context 'when user has permission' do + before do + group.add_reporter(user) + end + + context 'when the contact does not exist' do + let(:params) { { crm_contact_ids: [non_existing_record_id] } } + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array(["Issue customer relations contacts #{non_existing_record_id}: #{does_not_exist_or_no_permission}"]) + end + end + + context 'when the contact belongs to a different group' do + let(:group2) { create(:group) } + let(:contact) { create(:contact, group: group2) } + let(:params) { { crm_contact_ids: [contact.id] } } + + before do + group2.add_reporter(user) + end + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array(["Issue customer relations contacts #{contact.id}: #{does_not_exist_or_no_permission}"]) + end + end + + context 'replace' do + let(:params) { { crm_contact_ids: [contacts[1].id, contacts[2].id] } } + + it 'updates the issue with correct contacts' do + response = set_crm_contacts + + expect(response).to be_success + expect(issue.customer_relations_contacts).to match_array([contacts[1], contacts[2]]) + end + end + + context 'add' do + let(:params) { { add_crm_contact_ids: [contacts[3].id] } } + + it 'updates the issue with correct contacts' do + response = set_crm_contacts + + expect(response).to be_success + expect(issue.customer_relations_contacts).to match_array([contacts[0], contacts[1], contacts[3]]) + end + end + + context 'remove' do + let(:params) { { remove_crm_contact_ids: [contacts[0].id] } } + + it 'updates the issue with correct contacts' do + response = set_crm_contacts + + expect(response).to be_success + expect(issue.customer_relations_contacts).to match_array([contacts[1]]) + end + end + + context 'when attempting to add more than 6' do + let(:id) { contacts[0].id } + let(:params) { { add_crm_contact_ids: [id, id, id, id, id, id, id] } } + + it 'returns expected error message' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array(['You can only add up to 6 contacts at one time']) + end + end + + context 'when trying to remove non-existent contact' do + let(:params) { { remove_crm_contact_ids: [non_existing_record_id] } } + + it 'returns expected error message' do + response = set_crm_contacts + + expect(response).to be_success + expect(response.message).to be_nil + end + end + + context 'when combining params' do + let(:error_invalid_params) { 'You cannot combine crm_contact_ids with add_crm_contact_ids or remove_crm_contact_ids' } + + context 'add and remove' do + let(:params) { { remove_crm_contact_ids: [contacts[1].id], add_crm_contact_ids: [contacts[3].id] } } + + it 'updates the issue with correct contacts' do + response = set_crm_contacts + + expect(response).to be_success + expect(issue.customer_relations_contacts).to match_array([contacts[0], contacts[3]]) + end + end + + context 'replace and remove' do + let(:params) { { crm_contact_ids: [contacts[3].id], remove_crm_contact_ids: [contacts[0].id] } } + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array([error_invalid_params]) + end + end + + context 'replace and add' do + let(:params) { { crm_contact_ids: [contacts[3].id], add_crm_contact_ids: [contacts[1].id] } } + + it 'returns expected error response' do + response = set_crm_contacts + + expect(response).to be_error + expect(response.message).to match_array([error_invalid_params]) + end + end + end + end + end +end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 83c17f051eb..85b8fef685e 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -1256,28 +1256,38 @@ RSpec.describe Issues::UpdateService, :mailer do let(:closed_issuable) { create(:closed_issue, project: project) } end - context 'real-time updates' do - using RSpec::Parameterized::TableSyntax - + context 'broadcasting issue assignee updates' do let(:update_params) { { assignee_ids: [user2.id] } } - where(:action_cable_in_app_enabled, :feature_flag_enabled, :should_broadcast) do - true | true | true - true | false | true - false | true | true - false | false | false - end + context 'when feature flag is enabled' do + before do + stub_feature_flags(broadcast_issue_updates: true) + end + + it 'triggers the GraphQL subscription' do + expect(GraphqlTriggers).to receive(:issuable_assignees_updated).with(issue) + + update_issue(update_params) + end - with_them do - it 'broadcasts to the issues channel based on ActionCable and feature flag values' do - allow(Gitlab::ActionCable::Config).to receive(:in_app?).and_return(action_cable_in_app_enabled) - stub_feature_flags(broadcast_issue_updates: feature_flag_enabled) + context 'when assignee is not updated' do + let(:update_params) { { title: 'Some other title' } } - if should_broadcast - expect(GraphqlTriggers).to receive(:issuable_assignees_updated).with(issue) - else + it 'does not trigger the GraphQL subscription' do expect(GraphqlTriggers).not_to receive(:issuable_assignees_updated).with(issue) + + update_issue(update_params) end + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(broadcast_issue_updates: false) + end + + it 'does not trigger the GraphQL subscription' do + expect(GraphqlTriggers).not_to receive(:issuable_assignees_updated).with(issue) update_issue(update_params) end |