diff options
Diffstat (limited to 'spec')
24 files changed, 777 insertions, 388 deletions
diff --git a/spec/controllers/sent_notifications_controller_spec.rb b/spec/controllers/sent_notifications_controller_spec.rb index 190c00092b6..3061697551e 100644 --- a/spec/controllers/sent_notifications_controller_spec.rb +++ b/spec/controllers/sent_notifications_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe SentNotificationsController do +RSpec.describe SentNotificationsController, feature_category: :shared do let(:user) { create(:user) } let(:project) { create(:project, :public) } let(:private_project) { create(:project, :private) } @@ -235,7 +235,9 @@ RSpec.describe SentNotificationsController do end end - let(:sent_notification) { create(:sent_notification, project: project, noteable: merge_request, recipient: user) } + let(:sent_notification) do + create(:sent_notification, project: project, noteable: merge_request, recipient: user) + end before do unsubscribe @@ -299,12 +301,36 @@ RSpec.describe SentNotificationsController do end context 'when support bot is the notification recipient' do - let(:sent_notification) { create(:sent_notification, project: target_project, noteable: noteable, recipient: Users::Internal.support_bot) } + let(:sent_notification) do + create(:sent_notification, + project: target_project, noteable: noteable, recipient: Users::Internal.support_bot) + end it 'deletes the external author on the issue' do expect { unsubscribe }.to change { issue.issue_email_participants.count }.by(-1) end + context 'when sent_notification contains issue_email_participant' do + let!(:other_issue_email_participant) do + create(:issue_email_participant, issue: issue, email: 'other@example.com') + end + + let(:sent_notification) do + create(:sent_notification, + project: target_project, + noteable: noteable, + recipient: Users::Internal.support_bot, + issue_email_participant: other_issue_email_participant + ) + end + + it 'deletes the connected issue email participant' do + expect { unsubscribe }.to change { issue.issue_email_participants.count }.by(-1) + # Ensure external author is still present + expect(issue.email_participants_emails).to contain_exactly(email) + end + end + context 'when noteable is not an issue' do let(:noteable) { merge_request } diff --git a/spec/frontend/content_editor/extensions/attachment_spec.js b/spec/frontend/content_editor/extensions/attachment_spec.js index f037ac520fe..18ce6c9ab59 100644 --- a/spec/frontend/content_editor/extensions/attachment_spec.js +++ b/spec/frontend/content_editor/extensions/attachment_spec.js @@ -126,12 +126,12 @@ describe('content_editor/extensions/attachment', () => { describe.each` nodeType | html | file | mediaType - ${'image (png)'} | ${PROJECT_WIKI_ATTACHMENT_IMAGE_HTML} | ${imageFile} | ${(attrs) => image(attrs)} - ${'image (svg)'} | ${PROJECT_WIKI_ATTACHMENT_IMAGE_SVG_HTML} | ${imageFileSvg} | ${(attrs) => image(attrs)} + ${'image'} | ${PROJECT_WIKI_ATTACHMENT_IMAGE_HTML} | ${imageFile} | ${(attrs) => image(attrs)} + ${'image'} | ${PROJECT_WIKI_ATTACHMENT_IMAGE_SVG_HTML} | ${imageFileSvg} | ${(attrs) => image(attrs)} ${'audio'} | ${PROJECT_WIKI_ATTACHMENT_AUDIO_HTML} | ${audioFile} | ${(attrs) => audio(attrs)} ${'video'} | ${PROJECT_WIKI_ATTACHMENT_VIDEO_HTML} | ${videoFile} | ${(attrs) => video(attrs)} ${'drawioDiagram'} | ${PROJECT_WIKI_ATTACHMENT_DRAWIO_DIAGRAM_HTML} | ${drawioDiagramFile} | ${(attrs) => drawioDiagram(attrs)} - `('when the file is $nodeType', ({ html, file, mediaType }) => { + `('when the file is $nodeType', ({ nodeType, html, file, mediaType }) => { beforeEach(() => { renderMarkdown.mockResolvedValue(html); }); @@ -149,7 +149,13 @@ describe('content_editor/extensions/attachment', () => { it('inserts a media content with src set to the encoded content and uploading=file_name', async () => { const expectedDoc = doc( - p(mediaType({ uploading: file.name, src: blobUrl, alt: file.name })), + p( + mediaType({ + uploading: expect.stringMatching(new RegExp(`${nodeType}[0-9]+`)), + src: blobUrl, + alt: file.name, + }), + ), ); await expectDocumentAfterTransaction({ @@ -248,7 +254,12 @@ describe('content_editor/extensions/attachment', () => { it('inserts a link with a blob url', async () => { const expectedDoc = doc( - p(link({ uploading: attachmentFile.name, href: blobUrl }, 'test-file.zip')), + p( + link( + { uploading: expect.stringMatching(/file[0-9]+/), href: blobUrl }, + 'test-file.zip', + ), + ), ); await expectDocumentAfterTransaction({ @@ -334,68 +345,228 @@ describe('content_editor/extensions/attachment', () => { }; it.each([ - [1, () => doc(p(link({ href: blobUrl, uploading: 'test-file.zip' }, 'test-file.zip')))], + [ + 1, + () => + doc( + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file.zip', + ), + ), + ), + ], [ 2, () => doc( - p(link({ href: blobUrl, uploading: 'test-file.zip' }, 'test-file.zip')), - p(image({ alt: 'test-file.png', src: blobUrl, uploading: 'test-file.png' })), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file.zip', + ), + ), + p( + image({ + alt: 'test-file.png', + src: blobUrl, + uploading: expect.stringMatching(/image[0-9]+/), + }), + ), ), ], [ 3, () => doc( - p(link({ href: blobUrl, uploading: 'test-file.zip' }, 'test-file.zip')), - p(image({ alt: 'test-file.png', src: blobUrl, uploading: 'test-file.png' })), - p(video({ alt: 'test-file.mp4', src: blobUrl, uploading: 'test-file.mp4' })), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file.zip', + ), + ), + p( + image({ + alt: 'test-file.png', + src: blobUrl, + uploading: expect.stringMatching(/image[0-9]+/), + }), + ), + p( + video({ + alt: 'test-file.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), ), ], [ 4, () => doc( - p(link({ href: blobUrl, uploading: 'test-file.zip' }, 'test-file.zip')), - p(image({ alt: 'test-file.png', src: blobUrl, uploading: 'test-file.png' })), - p(video({ alt: 'test-file.mp4', src: blobUrl, uploading: 'test-file.mp4' })), - p(link({ href: blobUrl, uploading: 'test-file1.zip' }, 'test-file1.zip')), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file.zip', + ), + ), + p( + image({ + alt: 'test-file.png', + src: blobUrl, + uploading: expect.stringMatching(/image[0-9]+/), + }), + ), + p( + video({ + alt: 'test-file.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file1.zip', + ), + ), ), ], [ 5, () => doc( - p(link({ href: blobUrl, uploading: 'test-file.zip' }, 'test-file.zip')), - p(image({ alt: 'test-file.png', src: blobUrl, uploading: 'test-file.png' })), - p(video({ alt: 'test-file.mp4', src: blobUrl, uploading: 'test-file.mp4' })), - p(link({ href: blobUrl, uploading: 'test-file1.zip' }, 'test-file1.zip')), - p(link({ href: blobUrl, uploading: 'test-file2.zip' }, 'test-file2.zip')), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file.zip', + ), + ), + p( + image({ + alt: 'test-file.png', + src: blobUrl, + uploading: expect.stringMatching(/image[0-9]+/), + }), + ), + p( + video({ + alt: 'test-file.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file1.zip', + ), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file2.zip', + ), + ), ), ], [ 6, () => doc( - p(link({ href: blobUrl, uploading: 'test-file.zip' }, 'test-file.zip')), - p(image({ alt: 'test-file.png', src: blobUrl, uploading: 'test-file.png' })), - p(video({ alt: 'test-file.mp4', src: blobUrl, uploading: 'test-file.mp4' })), - p(link({ href: blobUrl, uploading: 'test-file1.zip' }, 'test-file1.zip')), - p(link({ href: blobUrl, uploading: 'test-file2.zip' }, 'test-file2.zip')), - p(video({ alt: 'test-file1.mp4', src: blobUrl, uploading: 'test-file1.mp4' })), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file.zip', + ), + ), + p( + image({ + alt: 'test-file.png', + src: blobUrl, + uploading: expect.stringMatching(/image[0-9]+/), + }), + ), + p( + video({ + alt: 'test-file.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file1.zip', + ), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file2.zip', + ), + ), + p( + video({ + alt: 'test-file1.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), ), ], [ 7, () => doc( - p(link({ href: blobUrl, uploading: 'test-file.zip' }, 'test-file.zip')), - p(image({ alt: 'test-file.png', src: blobUrl, uploading: 'test-file.png' })), - p(video({ alt: 'test-file.mp4', src: blobUrl, uploading: 'test-file.mp4' })), - p(link({ href: blobUrl, uploading: 'test-file1.zip' }, 'test-file1.zip')), - p(link({ href: blobUrl, uploading: 'test-file2.zip' }, 'test-file2.zip')), - p(video({ alt: 'test-file1.mp4', src: blobUrl, uploading: 'test-file1.mp4' })), - p(audio({ alt: 'test-file.mp3', src: blobUrl, uploading: 'test-file.mp3' })), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file.zip', + ), + ), + p( + image({ + alt: 'test-file.png', + src: blobUrl, + uploading: expect.stringMatching(/image[0-9]+/), + }), + ), + p( + video({ + alt: 'test-file.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file1.zip', + ), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file2.zip', + ), + ), + p( + video({ + alt: 'test-file1.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + audio({ + alt: 'test-file.mp3', + src: blobUrl, + uploading: expect.stringMatching(/audio[0-9]+/), + }), + ), ), ], [ @@ -412,12 +583,46 @@ describe('content_editor/extensions/attachment', () => { 'test-file.zip', ), ), - p(image({ alt: 'test-file.png', src: blobUrl, uploading: 'test-file.png' })), - p(video({ alt: 'test-file.mp4', src: blobUrl, uploading: 'test-file.mp4' })), - p(link({ href: blobUrl, uploading: 'test-file1.zip' }, 'test-file1.zip')), - p(link({ href: blobUrl, uploading: 'test-file2.zip' }, 'test-file2.zip')), - p(video({ alt: 'test-file1.mp4', src: blobUrl, uploading: 'test-file1.mp4' })), - p(audio({ alt: 'test-file.mp3', src: blobUrl, uploading: 'test-file.mp3' })), + p( + image({ + alt: 'test-file.png', + src: blobUrl, + uploading: expect.stringMatching(/image[0-9]+/), + }), + ), + p( + video({ + alt: 'test-file.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file1.zip', + ), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file2.zip', + ), + ), + p( + video({ + alt: 'test-file1.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + audio({ + alt: 'test-file.mp3', + src: blobUrl, + uploading: expect.stringMatching(/audio[0-9]+/), + }), + ), ), ], [ @@ -442,11 +647,39 @@ describe('content_editor/extensions/attachment', () => { uploading: false, }), ), - p(video({ alt: 'test-file.mp4', src: blobUrl, uploading: 'test-file.mp4' })), - p(link({ href: blobUrl, uploading: 'test-file1.zip' }, 'test-file1.zip')), - p(link({ href: blobUrl, uploading: 'test-file2.zip' }, 'test-file2.zip')), - p(video({ alt: 'test-file1.mp4', src: blobUrl, uploading: 'test-file1.mp4' })), - p(audio({ alt: 'test-file.mp3', src: blobUrl, uploading: 'test-file.mp3' })), + p( + video({ + alt: 'test-file.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file1.zip', + ), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file2.zip', + ), + ), + p( + video({ + alt: 'test-file1.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + audio({ + alt: 'test-file.mp3', + src: blobUrl, + uploading: expect.stringMatching(/audio[0-9]+/), + }), + ), ), ], [ @@ -479,10 +712,32 @@ describe('content_editor/extensions/attachment', () => { uploading: false, }), ), - p(link({ href: blobUrl, uploading: 'test-file1.zip' }, 'test-file1.zip')), - p(link({ href: blobUrl, uploading: 'test-file2.zip' }, 'test-file2.zip')), - p(video({ alt: 'test-file1.mp4', src: blobUrl, uploading: 'test-file1.mp4' })), - p(audio({ alt: 'test-file.mp3', src: blobUrl, uploading: 'test-file.mp3' })), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file1.zip', + ), + ), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file2.zip', + ), + ), + p( + video({ + alt: 'test-file1.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + audio({ + alt: 'test-file.mp3', + src: blobUrl, + uploading: expect.stringMatching(/audio[0-9]+/), + }), + ), ), ], [ @@ -525,9 +780,26 @@ describe('content_editor/extensions/attachment', () => { 'test-file1.zip', ), ), - p(link({ href: blobUrl, uploading: 'test-file2.zip' }, 'test-file2.zip')), - p(video({ alt: 'test-file1.mp4', src: blobUrl, uploading: 'test-file1.mp4' })), - p(audio({ alt: 'test-file.mp3', src: blobUrl, uploading: 'test-file.mp3' })), + p( + link( + { href: blobUrl, uploading: expect.stringMatching(/file[0-9]+/) }, + 'test-file2.zip', + ), + ), + p( + video({ + alt: 'test-file1.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + audio({ + alt: 'test-file.mp3', + src: blobUrl, + uploading: expect.stringMatching(/audio[0-9]+/), + }), + ), ), ], [ @@ -580,8 +852,20 @@ describe('content_editor/extensions/attachment', () => { 'test-file2.zip', ), ), - p(video({ alt: 'test-file1.mp4', src: blobUrl, uploading: 'test-file1.mp4' })), - p(audio({ alt: 'test-file.mp3', src: blobUrl, uploading: 'test-file.mp3' })), + p( + video({ + alt: 'test-file1.mp4', + src: blobUrl, + uploading: expect.stringMatching(/video[0-9]+/), + }), + ), + p( + audio({ + alt: 'test-file.mp3', + src: blobUrl, + uploading: expect.stringMatching(/audio[0-9]+/), + }), + ), ), ], [ @@ -642,7 +926,13 @@ describe('content_editor/extensions/attachment', () => { uploading: false, }), ), - p(audio({ alt: 'test-file.mp3', src: blobUrl, uploading: 'test-file.mp3' })), + p( + audio({ + alt: 'test-file.mp3', + src: blobUrl, + uploading: expect.stringMatching(/audio[0-9]+/), + }), + ), ), ], [ diff --git a/spec/graphql/mutations/work_items/update_task_spec.rb b/spec/graphql/mutations/work_items/update_task_spec.rb deleted file mode 100644 index cb37a72bbdd..00000000000 --- a/spec/graphql/mutations/work_items/update_task_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Mutations::WorkItems::UpdateTask do - let_it_be(:project) { create(:project) } - let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } - let_it_be(:referenced_work_item, refind: true) { create(:work_item, project: project, title: 'REFERENCED') } - let_it_be(:parent_work_item) do - create(:work_item, project: project, description: "- [ ] #{referenced_work_item.to_reference}+") - end - - let(:task_params) { { title: 'UPDATED' } } - let(:task_input) { { id: referenced_work_item.to_global_id }.merge(task_params) } - let(:input) { { id: parent_work_item.to_global_id, task_data: task_input } } - let(:mutation) { described_class.new(object: nil, context: { current_user: current_user }, field: nil) } - - describe '#resolve' do - subject(:resolve) do - mutation.resolve(**input) - end - - context 'when user has sufficient permissions' do - let(:current_user) { developer } - - it 'expires etag cache for parent work item' do - allow(WorkItem).to receive(:find).and_call_original - allow(WorkItem).to receive(:find).with(parent_work_item.id.to_s).and_return(parent_work_item) - - expect(parent_work_item).to receive(:expire_etag_cache) - - resolve - end - end - end -end diff --git a/spec/lib/api/entities/group_detail_spec.rb b/spec/lib/api/entities/group_detail_spec.rb index 8fcb120c809..f3200b28c4d 100644 --- a/spec/lib/api/entities/group_detail_spec.rb +++ b/spec/lib/api/entities/group_detail_spec.rb @@ -2,18 +2,50 @@ require 'spec_helper' -RSpec.describe API::Entities::GroupDetail do +RSpec.describe API::Entities::GroupDetail, feature_category: :groups_and_projects do describe '#as_json' do - it 'includes prevent_sharing_groups_outside_hierarchy for a root group' do - group = create(:group) + subject { described_class.new(group, options).as_json } - expect(described_class.new(group).as_json).to include(prevent_sharing_groups_outside_hierarchy: false) + let_it_be(:root_group) { create(:group) } + let_it_be(:subgroup) { create(:group, :nested) } + + let(:options) { {} } + + describe '#prevent_sharing_groups_outside_hierarchy' do + context 'for a root group' do + let(:group) { root_group } + + it { is_expected.to include(:prevent_sharing_groups_outside_hierarchy) } + end + + context 'for a subgroup' do + let(:group) { subgroup } + + it { is_expected.not_to include(:prevent_sharing_groups_outside_hierarchy) } + end end - it 'excludes prevent_sharing_groups_outside_hierarchy for a subgroup' do - subgroup = build(:group, :nested) + describe '#enabled_git_access_protocol' do + using RSpec::Parameterized::TableSyntax + + where(:group, :can_admin_group, :includes_field) do + ref(:root_group) | false | false + ref(:root_group) | true | true + ref(:subgroup) | false | false + ref(:subgroup) | true | false + end + + with_them do + let(:options) { { user_can_admin_group: can_admin_group } } - expect(described_class.new(subgroup).as_json.keys).not_to include(:prevent_sharing_groups_outside_hierarchy) + it 'verifies presence of the field' do + if includes_field + is_expected.to include(:enabled_git_access_protocol) + else + is_expected.not_to include(:enabled_git_access_protocol) + end + end + end end end end diff --git a/spec/lib/gitlab/click_house_spec.rb b/spec/lib/gitlab/click_house_spec.rb new file mode 100644 index 00000000000..3241eca34ae --- /dev/null +++ b/spec/lib/gitlab/click_house_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::ClickHouse, feature_category: :database do + context 'when ClickHouse is not configured' do + it 'returns false' do + expect(described_class).not_to be_configured + end + end + + context 'when ClickHouse is configured', :click_house do + it 'returns false' do + expect(described_class).to be_configured + end + end +end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 796fe75521a..5ff3bc20bc1 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -365,6 +365,20 @@ RSpec.describe Gitlab::GitalyClient, feature_category: :gitaly do it 'returns the result of the allow_n_plus_1_calls block' do expect(described_class.allow_n_plus_1_calls { "result" }).to eq("result") end + + context 'when the `gitaly_call_count_exception_block_depth` key is not present' do + before do + allow(Gitlab::SafeRequestStore).to receive(:[]).with(:gitaly_call_count_exception_block_depth).and_return(0, 1, nil) + allow(Gitlab::SafeRequestStore).to receive(:+) + allow(Gitlab::SafeRequestStore).to receive(:-) + end + + it 'does not decrement call count' do + expect(Gitlab::SafeRequestStore).not_to have_received(:-) + + described_class.allow_n_plus_1_calls { "result" } + end + end end context 'when RequestStore is not active' do diff --git a/spec/lib/gitlab/http_connection_adapter_spec.rb b/spec/lib/gitlab/http_connection_adapter_spec.rb deleted file mode 100644 index fac0c1a2a9f..00000000000 --- a/spec/lib/gitlab/http_connection_adapter_spec.rb +++ /dev/null @@ -1,161 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::HTTPConnectionAdapter, feature_category: :shared do - include StubRequests - - let(:uri) { URI('https://example.org') } - let(:options) { {} } - - subject(:connection) { described_class.new(uri, options).connection } - - describe '#connection' do - before do - stub_all_dns('https://example.org', ip_address: '93.184.216.34') - end - - context 'when local requests are allowed' do - let(:options) { { allow_local_requests: true } } - - it 'sets up the connection' do - expect(connection).to be_a(Gitlab::NetHttpAdapter) - expect(connection.address).to eq('93.184.216.34') - expect(connection.hostname_override).to eq('example.org') - expect(connection.addr_port).to eq('example.org') - expect(connection.port).to eq(443) - end - end - - context 'when local requests are not allowed' do - let(:options) { { allow_local_requests: false } } - - it 'sets up the connection' do - expect(connection).to be_a(Gitlab::NetHttpAdapter) - expect(connection.address).to eq('93.184.216.34') - expect(connection.hostname_override).to eq('example.org') - expect(connection.addr_port).to eq('example.org') - expect(connection.port).to eq(443) - end - - context 'when it is a request to local network' do - let(:uri) { URI('http://172.16.0.0/12') } - - it 'raises error' do - expect { subject }.to raise_error( - Gitlab::HTTP::BlockedUrlError, - "URL is blocked: Requests to the local network are not allowed" - ) - end - - context 'when local request allowed' do - let(:options) { { allow_local_requests: true } } - - it 'sets up the connection' do - expect(connection).to be_a(Gitlab::NetHttpAdapter) - expect(connection.address).to eq('172.16.0.0') - expect(connection.hostname_override).to be(nil) - expect(connection.addr_port).to eq('172.16.0.0') - expect(connection.port).to eq(80) - end - end - end - - context 'when it is a request to local address' do - let(:uri) { URI('http://127.0.0.1') } - - it 'raises error' do - expect { subject }.to raise_error( - Gitlab::HTTP::BlockedUrlError, - "URL is blocked: Requests to localhost are not allowed" - ) - end - - context 'when local request allowed' do - let(:options) { { allow_local_requests: true } } - - it 'sets up the connection' do - expect(connection).to be_a(Gitlab::NetHttpAdapter) - expect(connection.address).to eq('127.0.0.1') - expect(connection.hostname_override).to be(nil) - expect(connection.addr_port).to eq('127.0.0.1') - expect(connection.port).to eq(80) - end - end - end - - context 'when port different from URL scheme is used' do - let(:uri) { URI('https://example.org:8080') } - - it 'sets up the addr_port accordingly' do - expect(connection).to be_a(Gitlab::NetHttpAdapter) - expect(connection.address).to eq('93.184.216.34') - expect(connection.hostname_override).to eq('example.org') - expect(connection.addr_port).to eq('example.org:8080') - expect(connection.port).to eq(8080) - end - end - end - - context 'when DNS rebinding protection is disabled' do - before do - stub_application_setting(dns_rebinding_protection_enabled: false) - end - - it 'sets up the connection' do - expect(connection).to be_a(Gitlab::NetHttpAdapter) - expect(connection.address).to eq('example.org') - expect(connection.hostname_override).to eq(nil) - expect(connection.addr_port).to eq('example.org') - expect(connection.port).to eq(443) - end - end - - context 'when proxy is enabled' do - before do - stub_env('http_proxy', 'http://proxy.example.com') - end - - it 'proxy stays configured' do - expect(connection.proxy?).to be true - expect(connection.proxy_from_env?).to be true - expect(connection.proxy_address).to eq('proxy.example.com') - end - - context 'when no_proxy matches the request' do - before do - stub_env('no_proxy', 'example.org') - end - - it 'proxy is disabled' do - expect(connection.proxy?).to be false - expect(connection.proxy_from_env?).to be false - expect(connection.proxy_address).to be nil - end - end - - context 'when no_proxy does not match the request' do - before do - stub_env('no_proxy', 'example.com') - end - - it 'proxy stays configured' do - expect(connection.proxy?).to be true - expect(connection.proxy_from_env?).to be true - expect(connection.proxy_address).to eq('proxy.example.com') - end - end - end - - context 'when URL scheme is not HTTP/HTTPS' do - let(:uri) { URI('ssh://example.org') } - - it 'raises error' do - expect { subject }.to raise_error( - Gitlab::HTTP::BlockedUrlError, - "URL is blocked: Only allowed schemes are http, https" - ) - end - end - end -end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 6d5c17176dc..9e83b7d85e9 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -1010,6 +1010,7 @@ epic: - user_note_authors - boards_epic_user_preferences - epic_board_positions +- work_item epic_issue: - epic - issue diff --git a/spec/lib/gitlab/import_export/remote_stream_upload_spec.rb b/spec/lib/gitlab/import_export/remote_stream_upload_spec.rb index 3d9d6e1b96b..c27bc7b5fe1 100644 --- a/spec/lib/gitlab/import_export/remote_stream_upload_spec.rb +++ b/spec/lib/gitlab/import_export/remote_stream_upload_spec.rb @@ -23,10 +23,14 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do describe '#execute' do context 'when download request and upload request return 200' do - it 'uploads the downloaded content' do + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: true) + stub_application_setting(dns_rebinding_protection_enabled: false) stub_request(:get, download_url).to_return(status: 200, body: 'ABC', headers: { 'Content-Length' => 3 }) stub_request(:post, upload_url) + end + it 'uploads the downloaded content' do subject.execute expect( @@ -35,6 +39,16 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do ) ).to have_been_made end + + it 'calls the connection adapter twice with required args' do + expect(Gitlab::HTTP_V2::NewConnectionAdapter) + .to receive(:new).twice.with(instance_of(URI::HTTP), { + allow_local_requests: true, + dns_rebind_protection: false + }).and_call_original + + subject.execute + end end context 'when upload method is put' do @@ -87,7 +101,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do it 'raises error' do expect { subject.execute }.to raise_error( - Gitlab::HTTP::BlockedUrlError, + Gitlab::HTTP_V2::BlockedUrlError, "URL is blocked: Requests to localhost are not allowed" ) end @@ -113,7 +127,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do it 'raises error' do expect { subject.execute }.to raise_error( - Gitlab::HTTP::BlockedUrlError, + Gitlab::HTTP_V2::BlockedUrlError, "URL is blocked: Requests to the local network are not allowed" ) end @@ -141,7 +155,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do stub_request(:get, download_url) expect { subject.execute }.to raise_error( - Gitlab::HTTP::BlockedUrlError, + Gitlab::HTTP_V2::BlockedUrlError, "URL is blocked: Requests to localhost are not allowed" ) end @@ -167,7 +181,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do it 'raises error' do expect { subject.execute }.to raise_error( - Gitlab::HTTP::BlockedUrlError, + Gitlab::HTTP_V2::BlockedUrlError, "URL is blocked: Requests to the local network are not allowed" ) end @@ -191,7 +205,7 @@ RSpec.describe Gitlab::ImportExport::RemoteStreamUpload do stub_full_request(upload_url, ip_address: '127.0.0.1', method: upload_method) expect { subject.execute }.to raise_error( - Gitlab::HTTP::BlockedUrlError, + Gitlab::HTTP_V2::BlockedUrlError, "URL is blocked: Requests to localhost are not allowed" ) end diff --git a/spec/lib/gitlab/metrics/global_search_slis_spec.rb b/spec/lib/gitlab/metrics/global_search_slis_spec.rb index 68793db6e41..9aa4f3b106e 100644 --- a/spec/lib/gitlab/metrics/global_search_slis_spec.rb +++ b/spec/lib/gitlab/metrics/global_search_slis_spec.rb @@ -87,6 +87,10 @@ RSpec.describe Gitlab::Metrics::GlobalSearchSlis, feature_category: :global_sear end describe '#record_apdex' do + before do + allow(::Gitlab::ApplicationContext).to receive(:current_context_attribute).with(:caller_id).and_return('end') + end + where(:search_type, :code_search, :duration_target) do 'basic' | false | 8.812 'basic' | true | 27.538 @@ -96,10 +100,6 @@ RSpec.describe Gitlab::Metrics::GlobalSearchSlis, feature_category: :global_sear end with_them do - before do - allow(::Gitlab::ApplicationContext).to receive(:current_context_attribute).with(:caller_id).and_return('end') - end - let(:search_scope) { code_search ? 'blobs' : 'issues' } it 'increments the global_search SLI as a success if the elapsed time is within the target' do @@ -144,6 +144,46 @@ RSpec.describe Gitlab::Metrics::GlobalSearchSlis, feature_category: :global_sear ) end end + + context 'when the search scope is merge_requests and the search type is basic' do + it 'increments the global_search SLI as a success if the elapsed time is within the target' do + expect(Gitlab::Metrics::Sli::Apdex[:global_search]).to receive(:increment).with( + labels: { + search_type: 'basic', + search_level: 'global', + search_scope: 'merge_requests', + endpoint_id: 'end' + }, + success: true + ) + + described_class.record_apdex( + elapsed: 14, + search_type: 'basic', + search_level: 'global', + search_scope: 'merge_requests' + ) + end + + it 'increments the global_search SLI as a failure if the elapsed time is not within the target' do + expect(Gitlab::Metrics::Sli::Apdex[:global_search]).to receive(:increment).with( + labels: { + search_type: 'basic', + search_level: 'global', + search_scope: 'merge_requests', + endpoint_id: 'end' + }, + success: false + ) + + described_class.record_apdex( + elapsed: 16, + search_type: 'basic', + search_level: 'global', + search_scope: 'merge_requests' + ) + end + end end describe '#record_error_rate' do diff --git a/spec/mailers/emails/service_desk_spec.rb b/spec/mailers/emails/service_desk_spec.rb index 3ed531a16bc..6653a599602 100644 --- a/spec/mailers/emails/service_desk_spec.rb +++ b/spec/mailers/emails/service_desk_spec.rb @@ -19,13 +19,10 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do let_it_be(:credential) { create(:service_desk_custom_email_credential, project: project) } let_it_be(:verification) { create(:service_desk_custom_email_verification, project: project) } let_it_be(:service_desk_setting) { create(:service_desk_setting, project: project, custom_email: 'user@example.com') } + let_it_be(:issue_email_participant) { create(:issue_email_participant, issue: issue, email: email) } let(:template) { double(content: template_content) } - before_all do - issue.issue_email_participants.create!(email: email) - end - before do # Because we use global project and custom email instances, make sure # custom email is disabled in all regular cases to avoid flakiness. @@ -50,6 +47,9 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do expect(subject.parts[1].body.to_s).to include(expected_html) expect(subject.parts[1].content_type).to include('text/html') + + # Sets issue email participant in sent notification + expect(issue.reset.sent_notifications.first.issue_email_participant).to eq(issue_email_participant) end end @@ -209,6 +209,10 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do let_it_be(:expected_html) { expected_text } + before do + issue.update!(external_author: email) + end + subject { ServiceEmailClass.service_desk_thank_you_email(issue.id) } it_behaves_like 'a service desk notification email' @@ -272,9 +276,9 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do let(:other_issue) { create(:issue, project: project, description: full_issue_reference) } let(:template_content) { '%{ISSUE_DESCRIPTION}' } - let(:expected_template_html) { "<p data-sourcepos=\"1:1-1:22\" dir=\"auto\">#{full_issue_reference}</p>" } + let(:expected_template_html) { full_issue_reference } - subject { ServiceEmailClass.service_desk_thank_you_email(other_issue.id) } + subject(:message) { ServiceEmailClass.service_desk_thank_you_email(other_issue.id) } before do expect(Gitlab::Template::ServiceDeskTemplate).to receive(:find) @@ -285,7 +289,7 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do end it 'does not render GitLab-specific-reference links with title attribute' do - is_expected.to have_body_text(expected_template_html) + expect(message.body.to_s).to include(expected_template_html) end end end @@ -326,7 +330,7 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do let_it_be(:expected_html) { 'My <strong>note</strong>' } let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: expected_text) } - subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, email) } + subject { ServiceEmailClass.service_desk_new_note_email(issue.id, note.id, issue_email_participant) } it_behaves_like 'a service desk notification email' it_behaves_like 'read template from repository', 'new_note' @@ -466,7 +470,7 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do let_it_be(:upload_path_1) { "/uploads/#{path_1}" } let_it_be(:note) { create(:note_on_issue, noteable: issue, project: project, note: "a new comment with [#{filename}](#{upload_path}) [#{filename_1}](#{upload_path_1})") } - context 'when all uploads processed correct' do + context 'when all uploads processed correct' do # rubocop:disable RSpec/MultipleMemoizedHelpers -- Avoid duplication with heavy use of helpers before do allow_next_instance_of(FileUploader) do |instance| allow(instance).to receive(:size).and_return(5.megabytes) @@ -521,7 +525,7 @@ RSpec.describe Emails::ServiceDesk, feature_category: :service_desk do end context 'when custom email is enabled' do - subject { Notify.service_desk_new_note_email(issue.id, note.id, email) } + subject { Notify.service_desk_new_note_email(issue.id, note.id, issue_email_participant) } it_behaves_like 'a service desk notification email that uses custom email' end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 34311a8ae22..618717a61df 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -1538,9 +1538,12 @@ RSpec.describe Notify, feature_category: :code_review_workflow do end context 'for service desk issues' do + let_it_be(:issue_email_participant) do + create(:issue_email_participant, issue: issue, email: 'service.desk@example.com') + end + before do issue.update!(external_author: 'service.desk@example.com') - issue.issue_email_participants.create!(email: 'service.desk@example.com') end describe 'thank you email', feature_category: :service_desk do @@ -1615,7 +1618,7 @@ RSpec.describe Notify, feature_category: :code_review_workflow do describe 'new note email', feature_category: :service_desk do let_it_be(:first_note) { create(:discussion_note_on_issue, note: 'Hello world') } - subject { described_class.service_desk_new_note_email(issue.id, first_note.id, 'service.desk@example.com') } + subject { described_class.service_desk_new_note_email(issue.id, first_note.id, issue_email_participant) } it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' diff --git a/spec/models/sent_notification_spec.rb b/spec/models/sent_notification_spec.rb index 5b31e8e5e3c..8da87c2349a 100644 --- a/spec/models/sent_notification_spec.rb +++ b/spec/models/sent_notification_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe SentNotification, :request_store do +RSpec.describe SentNotification, :request_store, feature_category: :shared do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } @@ -40,6 +40,12 @@ RSpec.describe SentNotification, :request_store do end end + describe ' associations' do + subject { build(:sent_notification) } + + it { is_expected.to belong_to(:issue_email_participant) } + end + shared_examples 'a successful sent notification' do it 'creates a new SentNotification' do expect { subject }.to change { described_class.count }.by(1) @@ -61,6 +67,20 @@ RSpec.describe SentNotification, :request_store do it_behaves_like 'a successful sent notification' it_behaves_like 'a non-sticky write' + + context 'with issue email participant' do + let!(:issue_email_participant) { create(:issue_email_participant, issue: issue) } + + subject(:sent_notification) do + described_class.record(issue, user.id, described_class.reply_key, { + issue_email_participant: issue_email_participant + }) + end + + it 'saves the issue_email_participant' do + expect(sent_notification.issue_email_participant).to eq(issue_email_participant) + end + end end describe '.record_note' do diff --git a/spec/requests/api/graphql/mutations/work_items/update_task_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_task_spec.rb deleted file mode 100644 index 717de983871..00000000000 --- a/spec/requests/api/graphql/mutations/work_items/update_task_spec.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Update a work item task', feature_category: :team_planning do - include GraphqlHelpers - - let_it_be(:project) { create(:project) } - let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } - let_it_be(:unauthorized_work_item) { create(:work_item) } - let_it_be(:referenced_work_item, refind: true) { create(:work_item, project: project, title: 'REFERENCED') } - let_it_be(:parent_work_item) do - create(:work_item, project: project, description: "- [ ] #{referenced_work_item.to_reference}+") - end - - let(:task) { referenced_work_item } - let(:work_item) { parent_work_item } - let(:task_params) { { 'title' => 'UPDATED' } } - let(:task_input) { { 'id' => task.to_global_id.to_s }.merge(task_params) } - let(:input) { { 'id' => work_item.to_global_id.to_s, 'taskData' => task_input } } - let(:mutation) { graphql_mutation(:workItemUpdateTask, input, nil, ['productAnalyticsState']) } - let(:mutation_response) { graphql_mutation_response(:work_item_update_task) } - - context 'the user is not allowed to read a work item' do - let(:current_user) { create(:user) } - - it_behaves_like 'a mutation that returns a top-level access error' - end - - context 'when user has permissions to update a work item' do - let(:current_user) { developer } - - it 'updates the work item and invalidates markdown cache on the original work item' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - work_item.reload - referenced_work_item.reload - end.to change(referenced_work_item, :title).from(referenced_work_item.title).to('UPDATED') - - expect(response).to have_gitlab_http_status(:success) - expect(mutation_response).to include( - 'workItem' => hash_including( - 'title' => work_item.title, - 'descriptionHtml' => a_string_including('UPDATED') - ), - 'task' => hash_including( - 'title' => 'UPDATED' - ) - ) - end - - context 'when providing invalid task params' do - let(:task_params) { { 'title' => '' } } - - it 'makes no changes to the DB and returns an error message' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - work_item.reload - task.reload - end.to not_change(task, :title).and( - not_change(work_item, :description_html) - ) - - expect(mutation_response['errors']).to contain_exactly("Title can't be blank") - end - end - - context 'when user cannot update the task' do - let(:task) { unauthorized_work_item } - - it_behaves_like 'a mutation that returns a top-level access error' - end - - it_behaves_like 'has spam protection' do - let(:mutation_class) { ::Mutations::WorkItems::UpdateTask } - end - end - - context 'when user does not have permissions to update a work item' do - let(:current_user) { developer } - let(:work_item) { unauthorized_work_item } - - it_behaves_like 'a mutation that returns a top-level access error' - end -end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 6b949962e53..f2876ddee1b 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -991,6 +991,23 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do end end + context 'updating the `enabled_git_access_protocol` attribute' do + %w[ssh http all].each do |protocol| + context "with #{protocol}" do + subject do + put api("/groups/#{group1.id}", user1), params: { enabled_git_access_protocol: protocol } + end + + it 'updates the attribute', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['enabled_git_access_protocol']).to eq(protocol) + end + end + end + end + context 'malicious group name' do subject { put api("/groups/#{group1.id}", user1), params: { name: "<SCRIPT>alert('DOUBLE-ATTACK!')</SCRIPT>" } } @@ -2058,6 +2075,19 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do end end + context 'when creating a group with `enabled_git_access_protocol' do + let(:params) { attributes_for_group_api enabled_git_access_protocol: 'all' } + + subject { post api("/groups", user3), params: params } + + it 'creates group with the specified Git access protocol', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['enabled_git_access_protocol']).to eq(nil) + end + end + it "does not create group, duplicate", :aggregate_failures do post api("/groups", user3), params: { name: 'Duplicate Test', path: group2.path } diff --git a/spec/requests/api/usage_data_spec.rb b/spec/requests/api/usage_data_spec.rb index 37fa75a812c..eb0192d773b 100644 --- a/spec/requests/api/usage_data_spec.rb +++ b/spec/requests/api/usage_data_spec.rb @@ -5,6 +5,64 @@ require 'spec_helper' RSpec.describe API::UsageData, feature_category: :service_ping do let_it_be(:user) { create(:user) } + describe 'GET /usage_data/service_ping' do + let(:endpoint) { '/usage_data/service_ping' } + + context 'when feature flag is disabled' do + before do + stub_feature_flags(usage_data_api: false) + end + + it 'returns not_found' do + get api(endpoint) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'without authentication' do + it 'returns 401 response' do + get api(endpoint) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when authenticated as non-admin' do + let(:user) { create(:user) } + + it 'returns 403' do + get api(endpoint, user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when authenticated as an admin using read_service_ping access token' do + let(:scopes) { [Gitlab::Auth::READ_SERVICE_PING_SCOPE] } + let(:personal_access_token) { create(:personal_access_token, user: user, scopes: scopes) } + + before do + allow(Ability).to receive(:allowed?).and_return(true) + end + + it 'returns 200' do + get api(endpoint, personal_access_token: personal_access_token) + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'returns service ping payload' do + usage_data = { 'key' => 'value' } + allow(Rails.cache).to receive(:fetch).and_return(usage_data) + + get api(endpoint, personal_access_token: personal_access_token) + + expect(response.body).to eq(usage_data.to_json) + end + end + end + describe 'POST /usage_data/increment_counter' do let(:endpoint) { '/usage_data/increment_counter' } let(:known_event) { "diff_searches" } diff --git a/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb b/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb index eb9324fd24b..af1785a7eb7 100644 --- a/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb +++ b/spec/services/click_house/sync_strategies/base_sync_strategy_spec.rb @@ -128,7 +128,7 @@ RSpec.describe ClickHouse::SyncStrategies::BaseSyncStrategy, feature_category: : context 'when clickhouse is not configured' do before do - allow(ClickHouse::Client.configuration).to receive(:databases).and_return({}) + allow(Gitlab::ClickHouse).to receive(:configured?).and_return(false) end it 'skips execution' do @@ -138,7 +138,7 @@ RSpec.describe ClickHouse::SyncStrategies::BaseSyncStrategy, feature_category: : context 'when exclusive lease error happens' do it 'skips execution' do - allow(ClickHouse::Client.configuration).to receive(:databases).and_return({ main: :some_db }) + allow(Gitlab::ClickHouse).to receive(:configured?).and_return(true) expect(strategy).to receive(:in_lock).and_raise(Gitlab::ExclusiveLeaseHelpers::FailedToObtainLockError) expect(execute).to eq({ status: :skipped }) diff --git a/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb b/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb index 05fcf6ddeb3..80f5b7b30f7 100644 --- a/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb +++ b/spec/services/click_house/sync_strategies/event_sync_strategy_spec.rb @@ -95,7 +95,7 @@ RSpec.describe ClickHouse::SyncStrategies::EventSyncStrategy, feature_category: describe '#enabled?' do context 'when the clickhouse database is configured the feature flag is enabled' do before do - allow(ClickHouse::Client.configuration).to receive(:databases).and_return({ main: :some_db }) + allow(Gitlab::ClickHouse).to receive(:configured?).and_return(true) stub_feature_flags(event_sync_worker_for_click_house: true) end @@ -106,7 +106,7 @@ RSpec.describe ClickHouse::SyncStrategies::EventSyncStrategy, feature_category: context 'when the clickhouse database is not configured' do before do - allow(ClickHouse::Client.configuration).to receive(:databases).and_return({}) + allow(Gitlab::ClickHouse).to receive(:configured?).and_return(false) end it 'returns false' do @@ -116,7 +116,7 @@ RSpec.describe ClickHouse::SyncStrategies::EventSyncStrategy, feature_category: context 'when the feature flag is disabled' do before do - allow(ClickHouse::Client.configuration).to receive(:databases).and_return({ main: :some_db }) + allow(Gitlab::ClickHouse).to receive(:configured?).and_return(true) stub_feature_flags(event_sync_worker_for_click_house: false) end diff --git a/spec/services/namespace_settings/update_service_spec.rb b/spec/services/namespace_settings/update_service_spec.rb index 413a551ca0c..9473b958957 100644 --- a/spec/services/namespace_settings/update_service_spec.rb +++ b/spec/services/namespace_settings/update_service_spec.rb @@ -145,6 +145,7 @@ RSpec.describe NamespaceSettings::UpdateService, feature_category: :groups_and_p where(:setting_key, :setting_changes_from, :setting_changes_to) do :prevent_sharing_groups_outside_hierarchy | false | true :new_user_signups_cap | nil | 100 + :enabled_git_access_protocol | 'all' | 'ssh' end with_them do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 15e7f794795..587e5ed25d5 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -581,7 +581,7 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do end describe 'Notes' do - context 'issue note' do + describe 'issue note' do let_it_be(:project) { create(:project, :private) } let_it_be_with_reload(:issue) { create(:issue, project: project, assignees: [assignee]) } let_it_be(:mentioned_issue) { create(:issue, assignees: issue.assignees) } @@ -591,26 +591,28 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do subject { notification.new_note(note) } - context 'issue_email_participants' do + describe 'issue_email_participants' do before do allow(Notify).to receive(:service_desk_new_note_email) - .with(Integer, Integer, String).and_return(mailer) + .with(Integer, Integer, IssueEmailParticipant).and_return(mailer) - allow(::Gitlab::Email::IncomingEmail).to receive(:enabled?) { true } - allow(::Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?) { true } + allow(::Gitlab::Email::IncomingEmail).to receive(:enabled?).and_return(true) + allow(::Gitlab::Email::IncomingEmail).to receive(:supports_wildcard?).and_return(true) end - let(:subject) { described_class.new } let(:mailer) { double(deliver_later: true) } let(:issue) { create(:issue, author: Users::Internal.support_bot) } let(:project) { issue.project } let(:note) { create(:note, noteable: issue, project: project) } + subject(:notification_service) { described_class.new } + shared_examples 'notification with exact metric events' do |number_of_events| it 'adds metric event' do metric_transaction = double('Gitlab::Metrics::WebTransaction', increment: true, observe: true) allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction) - expect(metric_transaction).to receive(:add_event).with(:service_desk_new_note_email).exactly(number_of_events).times + expect(metric_transaction).to receive(:add_event) + .with(:service_desk_new_note_email).exactly(number_of_events).times subject.new_note(note) end @@ -638,9 +640,9 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do it 'sends the email' do expect(Notify).to receive(:service_desk_new_note_email) - .with(issue.id, note.id, issue.external_author) + .with(issue.id, note.id, issue_email_participant) - subject.new_note(note) + notification_service.new_note(note) end it_behaves_like 'notification with exact metric events', 1 @@ -652,6 +654,71 @@ RSpec.describe NotificationService, :mailer, feature_category: :team_planning do it_behaves_like 'no participants are notified' end + + context 'with multiple external participants' do + let!(:other_external_participant) { issue.issue_email_participants.create!(email: 'user@example.com') } + + it 'sends emails' do + expect(Notify).to receive(:service_desk_new_note_email) + .with(issue.id, note.id, IssueEmailParticipant).twice + + notification_service.new_note(note) + end + + context 'when note is from an external participant' do + shared_examples 'only sends one Service Desk notification email' do + it 'sends one email' do + expect(Notify).not_to receive(:service_desk_new_note_email) + .with(issue.id, note.id, non_recipient) + + expect(Notify).to receive(:service_desk_new_note_email) + .with(issue.id, note.id, recipient) + + notification_service.new_note(note) + end + end + + let!(:note) do + create( + :note_on_issue, + author: Users::Internal.support_bot, + noteable: issue, + project_id: issue.project_id, + note: '@mention referenced, @unsubscribed_mentioned and @outsider also' + ) + end + + context 'and the note is from the external issue author' do + let(:non_recipient) { issue_email_participant } + let(:recipient) { other_external_participant } + let!(:note_metadata) do + create(:note_metadata, note: note, email_participant: issue_email_participant.email) + end + + it_behaves_like 'only sends one Service Desk notification email' + end + + context 'and the note is from another external participant' do + let(:non_recipient) { other_external_participant } + let(:recipient) { issue_email_participant } + let!(:note_metadata) do + create(:note_metadata, note: note, email_participant: other_external_participant.email) + end + + it_behaves_like 'only sends one Service Desk notification email' + + context 'and the external note auhor email has different format' do + let(:non_recipient) { other_external_participant } + let(:recipient) { issue_email_participant } + let!(:note_metadata) do + create(:note_metadata, note: note, email_participant: 'USER@example.com') + end + + it_behaves_like 'only sends one Service Desk notification email' + end + end + end + end end context 'do exist and note is confidential' do diff --git a/spec/services/work_items/widgets/labels_service/create_service_spec.rb b/spec/services/work_items/widgets/labels_service/create_service_spec.rb new file mode 100644 index 00000000000..70b185b9301 --- /dev/null +++ b/spec/services/work_items/widgets/labels_service/create_service_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::Widgets::LabelsService::CreateService, feature_category: :team_planning do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:label1) { create(:label, project: project) } + let_it_be(:label2) { create(:label, project: project) } + let_it_be(:label3) { create(:label, project: project) } + let_it_be(:current_user) { create(:user).tap { |user| project.add_reporter(user) } } + + let(:work_item) { create(:work_item, project: project, labels: [label1, label2]) } + let(:widget) { work_item.widgets.find { |widget| widget.is_a?(WorkItems::Widgets::Labels) } } + let(:service) { described_class.new(widget: widget, current_user: current_user) } + + describe '#prepare_create_params' do + context 'when params are set' do + let(:params) { { add_label_ids: [label1.id], label_ids: [label2.id] } } + + it "sets params correctly" do + expect(service.prepare_create_params(params: params)).to include( + { + add_label_ids: match_array([label1.id]), + label_ids: match_array([label2.id]) + } + ) + end + + context "and user doesn't have permissions to update labels" do + let_it_be(:current_user) { create(:user) } + + it 'removes label params' do + expect(service.prepare_create_params(params: params)).to be_nil + end + end + end + + context 'when widget does not exist in new type' do + let(:params) { {} } + + before do + allow(service).to receive(:new_type_excludes_widget?).and_return(true) + end + + it "sets label params as empty" do + expect(service.prepare_create_params(params: params)).to include( + { + add_label_ids: [], + label_ids: [] + } + ) + end + end + end +end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 66730340f84..91d423fcd67 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -4340,7 +4340,6 @@ - './spec/graphql/mutations/todos/restore_many_spec.rb' - './spec/graphql/mutations/todos/restore_spec.rb' - './spec/graphql/mutations/user_callouts/create_spec.rb' -- './spec/graphql/mutations/work_items/update_task_spec.rb' - './spec/graphql/resolvers/admin/analytics/usage_trends/measurements_resolver_spec.rb' - './spec/graphql/resolvers/alert_management/alert_resolver_spec.rb' - './spec/graphql/resolvers/alert_management/alert_status_counts_resolver_spec.rb' @@ -6221,7 +6220,6 @@ - './spec/lib/gitlab/hook_data/subgroup_builder_spec.rb' - './spec/lib/gitlab/hook_data/user_builder_spec.rb' - './spec/lib/gitlab/hotlinking_detector_spec.rb' -- './spec/lib/gitlab/http_connection_adapter_spec.rb' - './spec/lib/gitlab/http_io_spec.rb' - './spec/lib/gitlab/http_spec.rb' - './spec/lib/gitlab/i18n/metadata_entry_spec.rb' @@ -7924,7 +7922,6 @@ - './spec/requests/api/graphql/mutations/work_items/create_spec.rb' - './spec/requests/api/graphql/mutations/work_items/delete_spec.rb' - './spec/requests/api/graphql/mutations/work_items/update_spec.rb' -- './spec/requests/api/graphql/mutations/work_items/update_task_spec.rb' - './spec/requests/api/graphql/namespace/package_settings_spec.rb' - './spec/requests/api/graphql/namespace/projects_spec.rb' - './spec/requests/api/graphql/namespace_query_spec.rb' diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index c6ea665a160..ec0d1e1a976 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -78,6 +78,7 @@ RSpec.shared_context 'GroupPolicy context' do read_billing edit_billing admin_member_access_request + update_git_access_protocol ] end diff --git a/spec/workers/click_house/event_authors_consistency_cron_worker_spec.rb b/spec/workers/click_house/event_authors_consistency_cron_worker_spec.rb index 4d7e0e138e9..a66657c639f 100644 --- a/spec/workers/click_house/event_authors_consistency_cron_worker_spec.rb +++ b/spec/workers/click_house/event_authors_consistency_cron_worker_spec.rb @@ -7,7 +7,7 @@ RSpec.describe ClickHouse::EventAuthorsConsistencyCronWorker, feature_category: context 'when ClickHouse is disabled' do it 'does nothing' do - allow(ClickHouse::Client).to receive(:database_configured?).and_return(false) + allow(Gitlab::ClickHouse).to receive(:configured?).and_return(false) expect(worker).not_to receive(:log_extra_metadata_on_done) @@ -17,7 +17,7 @@ RSpec.describe ClickHouse::EventAuthorsConsistencyCronWorker, feature_category: context 'when the event_sync_worker_for_click_house feature flag is off' do it 'does nothing' do - allow(ClickHouse::Client).to receive(:database_configured?).and_return(true) + allow(Gitlab::ClickHouse).to receive(:configured?).and_return(true) stub_feature_flags(event_sync_worker_for_click_house: false) expect(worker).not_to receive(:log_extra_metadata_on_done) |