From 9f46488805e86b1bc341ea1620b866016c2ce5ed Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 20 May 2020 14:34:42 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-0-stable-ee --- spec/services/issues/close_service_spec.rb | 2 +- spec/services/issues/create_service_spec.rb | 86 ++++++++++++++--- .../issues/related_branches_service_spec.rb | 102 +++++++++++++++++---- spec/services/issues/resolve_discussions_spec.rb | 19 ++-- spec/services/issues/update_service_spec.rb | 30 +++++- 5 files changed, 197 insertions(+), 42 deletions(-) (limited to 'spec/services/issues') diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 86377e054c1..6fc1928d47b 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -146,7 +146,7 @@ describe Issues::CloseService do context 'when `metrics.first_mentioned_in_commit_at` is already set' do before do - issue.metrics.update!(first_mentioned_in_commit_at: Time.now) + issue.metrics.update!(first_mentioned_in_commit_at: Time.current) end it 'does not update the metrics' do diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index bd50d6b1001..7a251e03e51 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -110,6 +110,31 @@ describe Issues::CreateService do end end + context 'when labels is nil' do + let(:opts) do + { title: 'Title', + description: 'Description', + labels: nil } + end + + it 'does not assign label' do + expect(issue.labels).to be_empty + end + end + + context 'when labels is nil and label_ids is present' do + let(:opts) do + { title: 'Title', + description: 'Description', + labels: nil, + label_ids: labels.map(&:id) } + end + + it 'assigns group labels' do + expect(issue.labels).to match_array labels + end + end + context 'when milestone belongs to different project' do let(:milestone) { create(:milestone) } @@ -368,6 +393,8 @@ describe Issues::CreateService do end context 'checking spam' do + include_context 'includes Spam constants' + let(:title) { 'Legit issue' } let(:description) { 'please fix' } let(:opts) do @@ -378,11 +405,13 @@ describe Issues::CreateService do } end + subject { described_class.new(project, user, opts) } + before do stub_feature_flags(allow_possible_spam: false) end - context 'when recaptcha was verified' do + context 'when reCAPTCHA was verified' do let(:log_user) { user } let(:spam_logs) { create_list(:spam_log, 2, user: log_user, title: title) } let(:target_spam_log) { spam_logs.last } @@ -391,7 +420,7 @@ describe Issues::CreateService do opts[:recaptcha_verified] = true opts[:spam_log_id] = target_spam_log.id - expect(Spam::AkismetService).not_to receive(:new) + expect(Spam::SpamVerdictService).not_to receive(:new) end it 'does not mark an issue as spam' do @@ -402,7 +431,7 @@ describe Issues::CreateService do expect(issue).to be_valid end - it 'does not assign a spam_log to an issue' do + it 'does not assign a spam_log to the issue' do expect(issue.spam_log).to be_nil end @@ -419,17 +448,42 @@ describe Issues::CreateService do end end - context 'when recaptcha was not verified' do + context 'when reCAPTCHA was not verified' do before do - expect_next_instance_of(Spam::SpamCheckService) do |spam_service| + expect_next_instance_of(Spam::SpamActionService) do |spam_service| expect(spam_service).to receive_messages(check_for_spam?: true) end end - context 'when akismet detects spam' do + context 'when SpamVerdictService requires reCAPTCHA' do before do - expect_next_instance_of(Spam::AkismetService) do |akismet_service| - expect(akismet_service).to receive_messages(spam?: true) + expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service| + expect(verdict_service).to receive(:execute).and_return(REQUIRE_RECAPTCHA) + end + end + + it 'does not mark the issue as spam' do + expect(issue).not_to be_spam + end + + it 'marks the issue as needing reCAPTCHA' do + expect(issue.needs_recaptcha?).to be_truthy + end + + it 'invalidates the issue' do + expect(issue).to be_invalid + end + + it 'creates a new spam_log' do + expect { issue } + .to have_spam_log(title: title, description: description, user_id: user.id, noteable_type: 'Issue') + end + end + + context 'when SpamVerdictService disallows creation' do + before do + expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service| + expect(verdict_service).to receive(:execute).and_return(DISALLOW) end end @@ -438,6 +492,10 @@ describe Issues::CreateService do expect(issue).to be_spam end + it 'does not mark the issue as needing reCAPTCHA' do + expect(issue.needs_recaptcha?).to be_falsey + end + it 'invalidates the issue' do expect(issue).to be_invalid end @@ -457,7 +515,11 @@ describe Issues::CreateService do expect(issue).not_to be_spam end - it '​creates a valid issue' do + it 'does not mark the issue as needing reCAPTCHA' do + expect(issue.needs_recaptcha?).to be_falsey + end + + it 'creates a valid issue' do expect(issue).to be_valid end @@ -468,10 +530,10 @@ describe Issues::CreateService do end end - context 'when akismet does not detect spam' do + context 'when the SpamVerdictService allows creation' do before do - expect_next_instance_of(Spam::AkismetService) do |akismet_service| - expect(akismet_service).to receive_messages(spam?: false) + expect_next_instance_of(Spam::SpamVerdictService) do |verdict_service| + expect(verdict_service).to receive(:execute).and_return(ALLOW) end end diff --git a/spec/services/issues/related_branches_service_spec.rb b/spec/services/issues/related_branches_service_spec.rb index eae35f12560..9f72e499414 100644 --- a/spec/services/issues/related_branches_service_spec.rb +++ b/spec/services/issues/related_branches_service_spec.rb @@ -3,39 +3,103 @@ require 'spec_helper' describe Issues::RelatedBranchesService do - let(:user) { create(:admin) } - let(:issue) { create(:issue) } + let_it_be(:developer) { create(:user) } + let_it_be(:issue) { create(:issue) } + let(:user) { developer } subject { described_class.new(issue.project, user) } + before do + issue.project.add_developer(developer) + end + describe '#execute' do + let(:sha) { 'abcdef' } + let(:repo) { issue.project.repository } + let(:project) { issue.project } + let(:branch_info) { subject.execute(issue) } + + def make_branch + double('Branch', dereferenced_target: double('Target', sha: sha)) + end + before do - allow(issue.project.repository).to receive(:branch_names).and_return(["mpempe", "#{issue.iid}mepmep", issue.to_branch_name, "#{issue.iid}-branch"]) + allow(repo).to receive(:branch_names).and_return(branch_names) end - it "selects the right branches when there are no referenced merge requests" do - expect(subject.execute(issue)).to eq([issue.to_branch_name, "#{issue.iid}-branch"]) + context 'no branches are available' do + let(:branch_names) { [] } + + it 'returns an empty array' do + expect(branch_info).to be_empty + end end - it "selects the right branches when there is a referenced merge request" do - merge_request = create(:merge_request, { description: "Closes ##{issue.iid}", - source_project: issue.project, - source_branch: "#{issue.iid}-branch" }) - merge_request.create_cross_references!(user) + context 'branches are available' do + let(:missing_branch) { "#{issue.to_branch_name}-missing" } + let(:unreadable_branch_name) { "#{issue.to_branch_name}-unreadable" } + let(:pipeline) { build(:ci_pipeline, :success, project: project) } + let(:unreadable_pipeline) { build(:ci_pipeline, :running) } + + let(:branch_names) do + [ + generate(:branch), + "#{issue.iid}doesnt-match", + issue.to_branch_name, + missing_branch, + unreadable_branch_name + ] + end + + before do + { + issue.to_branch_name => pipeline, + unreadable_branch_name => unreadable_pipeline + }.each do |name, pipeline| + allow(repo).to receive(:find_branch).with(name).and_return(make_branch) + allow(project).to receive(:pipeline_for).with(name, sha).and_return(pipeline) + end + + allow(repo).to receive(:find_branch).with(missing_branch).and_return(nil) + end + + it 'selects relevant branches, along with pipeline status where available' do + expect(branch_info).to contain_exactly( + { name: issue.to_branch_name, pipeline_status: an_instance_of(Gitlab::Ci::Status::Success) }, + { name: missing_branch, pipeline_status: be_nil }, + { name: unreadable_branch_name, pipeline_status: be_nil } + ) + end + + context 'the user has access to otherwise unreadable pipelines' do + let(:user) { create(:admin) } + + it 'returns info a developer could not see' do + expect(branch_info.pluck(:pipeline_status)).to include(an_instance_of(Gitlab::Ci::Status::Running)) + end + end + + it 'excludes branches referenced in merge requests' do + merge_request = create(:merge_request, { description: "Closes #{issue.to_reference}", + source_project: issue.project, + source_branch: issue.to_branch_name }) + merge_request.create_cross_references!(user) - referenced_merge_requests = Issues::ReferencedMergeRequestsService - .new(issue.project, user) - .referenced_merge_requests(issue) + referenced_merge_requests = Issues::ReferencedMergeRequestsService + .new(issue.project, user) + .referenced_merge_requests(issue) - expect(referenced_merge_requests).not_to be_empty - expect(subject.execute(issue)).to eq([issue.to_branch_name]) + expect(referenced_merge_requests).not_to be_empty + expect(branch_info.pluck(:name)).not_to include(merge_request.source_branch) + end end - it 'excludes stable branches from the related branches' do - allow(issue.project.repository).to receive(:branch_names) - .and_return(["#{issue.iid}-0-stable"]) + context 'one of the branches is stable' do + let(:branch_names) { ["#{issue.iid}-0-stable"] } - expect(subject.execute(issue)).to eq [] + it 'is excluded' do + expect(branch_info).to be_empty + end end end end diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index f12a3820b8d..ec6624db6fc 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -3,19 +3,20 @@ require 'spec_helper.rb' describe Issues::ResolveDiscussions do - class DummyService < Issues::BaseService - include ::Issues::ResolveDiscussions - - def initialize(*args) - super - filter_resolve_discussion_params - end - end - let(:project) { create(:project, :repository) } let(:user) { create(:user) } before do + stub_const('DummyService', Class.new(Issues::BaseService)) + DummyService.class_eval do + include ::Issues::ResolveDiscussions + + def initialize(*args) + super + filter_resolve_discussion_params + end + end + project.add_developer(user) end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index c32bef5a1a5..80039049bc3 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -510,7 +510,7 @@ describe Issues::UpdateService, :mailer do end it 'updates updated_at' do - expect(issue.reload.updated_at).to be > Time.now + expect(issue.reload.updated_at).to be > Time.current end end end @@ -842,5 +842,33 @@ describe Issues::UpdateService, :mailer do let(:open_issuable) { issue } let(:closed_issuable) { create(:closed_issue, project: project) } end + + context 'real-time updates' do + let(:update_params) { { assignee_ids: [user2.id] } } + + context 'when broadcast_issue_updates is enabled' do + before do + stub_feature_flags(broadcast_issue_updates: true) + end + + it 'broadcasts to the issues channel' do + expect(IssuesChannel).to receive(:broadcast_to).with(issue, event: 'updated') + + update_issue(update_params) + end + end + + context 'when broadcast_issue_updates is disabled' do + before do + stub_feature_flags(broadcast_issue_updates: false) + end + + it 'does not broadcast to the issues channel' do + expect(IssuesChannel).not_to receive(:broadcast_to) + + update_issue(update_params) + end + end + end end end -- cgit v1.2.3