diff options
Diffstat (limited to 'spec')
25 files changed, 716 insertions, 137 deletions
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 84bbbac39b0..0b3833e6515 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -641,24 +641,32 @@ describe ApplicationController do end end - it 'does not set a custom header' do + it 'sets a custom header' do get :index, format: :json - expect(response.headers['X-GitLab-Custom-Error']).to be_nil + expect(response.headers['X-GitLab-Custom-Error']).to eq '1' end - end - context 'given a json response for an html request' do - controller do - def index - render json: {}, status: :unprocessable_entity + context 'for html request' do + it 'sets a custom header' do + get :index + + expect(response.headers['X-GitLab-Custom-Error']).to eq '1' end end - it 'does not set a custom header' do - get :index + context 'for 200 response' do + controller do + def index + render json: {}, status: :ok + end + end - expect(response.headers['X-GitLab-Custom-Error']).to be_nil + it 'does not set a custom header' do + get :index, format: :json + + expect(response.headers['X-GitLab-Custom-Error']).to be_nil + end end end end diff --git a/spec/controllers/projects/git_http_controller_spec.rb b/spec/controllers/projects/git_http_controller_spec.rb index bf099e8deeb..88fa2236e33 100644 --- a/spec/controllers/projects/git_http_controller_spec.rb +++ b/spec/controllers/projects/git_http_controller_spec.rb @@ -12,4 +12,15 @@ describe Projects::GitHttpController do expect(response.status).to eq(403) end end + + describe 'GET #info_refs' do + it 'returns 401 for unauthenticated requests to public repositories when http protocol is disabled' do + stub_application_setting(enabled_git_access_protocol: 'ssh') + project = create(:project, :public, :repository) + + get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } + + expect(response.status).to eq(401) + end + end end diff --git a/spec/factories/services.rb b/spec/factories/services.rb index f3e662ad4f5..b2d6ada91fa 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -16,6 +16,19 @@ FactoryBot.define do ) end + factory :emails_on_push_service do + project + type 'EmailsOnPushService' + active true + push_events true + tag_push_events true + properties( + recipients: 'test@example.com', + disable_diffs: true, + send_from_committer_email: true + ) + end + factory :mock_deployment_service do project type 'MockDeploymentService' diff --git a/spec/features/markdown/metrics_spec.rb b/spec/features/markdown/metrics_spec.rb index aa53ac50c78..4de67cfcdbe 100644 --- a/spec/features/markdown/metrics_spec.rb +++ b/spec/features/markdown/metrics_spec.rb @@ -26,13 +26,31 @@ describe 'Metrics rendering', :js, :use_clean_rails_memory_store_caching do restore_host end - context 'with deployments and related deployable present' do - it 'shows embedded metrics' do + it 'shows embedded metrics' do + visit project_issue_path(project, issue) + + expect(page).to have_css('div.prometheus-graph') + expect(page).to have_text('Memory Usage (Total)') + expect(page).to have_text('Core Usage (Total)') + end + + context 'when dashboard params are in included the url' do + let(:metrics_url) { metrics_project_environment_url(project, environment, **chart_params) } + + let(:chart_params) do + { + group: 'System metrics (Kubernetes)', + title: 'Memory Usage (Pod average)', + y_label: 'Memory Used per Pod (MB)' + } + end + + it 'shows embedded metrics for the specifiec chart' do visit project_issue_path(project, issue) expect(page).to have_css('div.prometheus-graph') - expect(page).to have_text('Memory Usage (Total)') - expect(page).to have_text('Core Usage (Total)') + expect(page).to have_text(chart_params[:title]) + expect(page).to have_text(chart_params[:y_label]) end end diff --git a/spec/lib/banzai/filter/inline_metrics_filter_spec.rb b/spec/lib/banzai/filter/inline_metrics_filter_spec.rb index 542a9ced6d7..66bbcbf7292 100644 --- a/spec/lib/banzai/filter/inline_metrics_filter_spec.rb +++ b/spec/lib/banzai/filter/inline_metrics_filter_spec.rb @@ -12,7 +12,7 @@ describe Banzai::Filter::InlineMetricsFilter do let(:url) { 'https://foo.com' } it 'leaves regular non-metrics links unchanged' do - expect(doc.to_s).to eq input + expect(doc.to_s).to eq(input) end end @@ -21,7 +21,7 @@ describe Banzai::Filter::InlineMetricsFilter do let(:url) { urls.metrics_namespace_project_environment_url(*params) } it 'leaves the original link unchanged' do - expect(doc.at_css('a').to_s).to eq input + expect(doc.at_css('a').to_s).to eq(input) end it 'appends a metrics charts placeholder with dashboard url after metrics links' do @@ -29,7 +29,7 @@ describe Banzai::Filter::InlineMetricsFilter do expect(node).to be_present dashboard_url = urls.metrics_dashboard_namespace_project_environment_url(*params, embedded: true) - expect(node.attribute('data-dashboard-url').to_s).to eq dashboard_url + expect(node.attribute('data-dashboard-url').to_s).to eq(dashboard_url) end context 'when the metrics dashboard link is part of a paragraph' do @@ -37,9 +37,34 @@ describe Banzai::Filter::InlineMetricsFilter do let(:input) { %(<p>#{paragraph}</p>) } it 'appends the charts placeholder after the enclosing paragraph' do - expect(doc.at_css('p').to_s).to include paragraph + expect(doc.at_css('p').to_s).to include(paragraph) expect(doc.at_css('.js-render-metrics')).to be_present end end + + context 'with dashboard params specified' do + let(:params) do + [ + 'foo', + 'bar', + 12, + { + embedded: true, + dashboard: 'config/prometheus/common_metrics.yml', + group: 'System metrics (Kubernetes)', + title: 'Core Usage (Pod Average)', + y_label: 'Cores per Pod' + } + ] + end + + it 'appends a metrics charts placeholder with dashboard url after metrics links' do + node = doc.at_css('.js-render-metrics') + expect(node).to be_present + + dashboard_url = urls.metrics_dashboard_namespace_project_environment_url(*params) + expect(node.attribute('data-dashboard-url').to_s).to eq(dashboard_url) + end + end end end diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index f11f68ab3c2..2990594c538 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -101,13 +101,13 @@ describe Gitlab::Danger::Helper do describe '#changes_by_category' do it 'categorizes changed files' do - expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/foo lib/gitlab/database/foo.rb qa/foo ee/changelogs/foo.yml] } + expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/migrate/foo lib/gitlab/database/foo.rb qa/foo ee/changelogs/foo.yml] } allow(fake_git).to receive(:modified_files) { [] } allow(fake_git).to receive(:renamed_files) { [] } expect(helper.changes_by_category).to eq( backend: %w[foo.rb], - database: %w[db/foo lib/gitlab/database/foo.rb], + database: %w[db/migrate/foo lib/gitlab/database/foo.rb], frontend: %w[foo.js], none: %w[ee/changelogs/foo.yml foo.md], qa: %w[qa/foo], @@ -173,8 +173,13 @@ describe Gitlab::Danger::Helper do 'ee/FOO_VERSION' | :unknown - 'db/foo' | :database - 'ee/db/foo' | :database + 'db/schema.rb' | :database + 'db/migrate/foo' | :database + 'db/post_migrate/foo' | :database + 'ee/db/migrate/foo' | :database + 'ee/db/post_migrate/foo' | :database + 'ee/db/geo/migrate/foo' | :database + 'ee/db/geo/post_migrate/foo' | :database 'app/models/project_authorization.rb' | :database 'app/services/users/refresh_authorized_projects_service.rb' | :database 'lib/gitlab/background_migration.rb' | :database @@ -188,6 +193,9 @@ describe Gitlab::Danger::Helper do 'lib/gitlab/sql/foo' | :database 'rubocop/cop/migration/foo' | :database + 'db/fixtures/foo.rb' | :backend + 'ee/db/fixtures/foo.rb' | :backend + 'qa/foo' | :qa 'ee/qa/foo' | :qa diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index fddb5066d6f..3c6b17c10ec 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -242,6 +242,7 @@ project: - cluster_project - cluster_ingresses - creator +- cycle_analytics_stages - group - namespace - boards diff --git a/spec/lib/gitlab/metrics/dashboard/url_spec.rb b/spec/lib/gitlab/metrics/dashboard/url_spec.rb index 34bc6359414..e0dc6d98efc 100644 --- a/spec/lib/gitlab/metrics/dashboard/url_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/url_spec.rb @@ -9,14 +9,22 @@ describe Gitlab::Metrics::Dashboard::Url do end it 'matches a metrics dashboard link with named params' do - url = Gitlab::Routing.url_helpers.metrics_namespace_project_environment_url('foo', 'bar', 1, start: 123345456, anchor: 'title') + url = Gitlab::Routing.url_helpers.metrics_namespace_project_environment_url( + 'foo', + 'bar', + 1, + start: '2019-08-02T05:43:09.000Z', + dashboard: 'config/prometheus/common_metrics.yml', + group: 'awesome group', + anchor: 'title' + ) expected_params = { 'url' => url, 'namespace' => 'foo', 'project' => 'bar', 'environment' => '1', - 'query' => '?start=123345456', + 'query' => '?dashboard=config%2Fprometheus%2Fcommon_metrics.yml&group=awesome+group&start=2019-08-02T05%3A43%3A09.000Z', 'anchor' => '#title' } diff --git a/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb index f4a6e1fc7d9..b8add3c1324 100644 --- a/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb @@ -46,8 +46,6 @@ describe Gitlab::Metrics::Samplers::PumaSampler do expect(subject.metrics[:puma_workers]).to receive(:set).with(labels, 2) expect(subject.metrics[:puma_running_workers]).to receive(:set).with(labels, 2) expect(subject.metrics[:puma_stale_workers]).to receive(:set).with(labels, 0) - expect(subject.metrics[:puma_phase]).to receive(:set).once.with(labels, 2) - expect(subject.metrics[:puma_phase]).to receive(:set).once.with({ worker: 'worker_0' }, 1) subject.sample end diff --git a/spec/lib/gitlab/project_template_spec.rb b/spec/lib/gitlab/project_template_spec.rb index 8b82ea7faa5..c7c82d07508 100644 --- a/spec/lib/gitlab/project_template_spec.rb +++ b/spec/lib/gitlab/project_template_spec.rb @@ -28,6 +28,18 @@ describe Gitlab::ProjectTemplate do end end + describe '#project_path' do + subject { described_class.new('name', 'title', 'description', 'https://gitlab.com/some/project/path').project_path } + + it { is_expected.to eq 'some/project/path' } + end + + describe '#uri_encoded_project_path' do + subject { described_class.new('name', 'title', 'description', 'https://gitlab.com/some/project/path').uri_encoded_project_path } + + it { is_expected.to eq 'some%2Fproject%2Fpath' } + end + describe '.find' do subject { described_class.find(query) } diff --git a/spec/models/analytics/cycle_analytics/project_stage_spec.rb b/spec/models/analytics/cycle_analytics/project_stage_spec.rb new file mode 100644 index 00000000000..4e3923e82b1 --- /dev/null +++ b/spec/models/analytics/cycle_analytics/project_stage_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Analytics::CycleAnalytics::ProjectStage do + describe 'associations' do + it { is_expected.to belong_to(:project) } + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 53424204db7..d344a6d0f0d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3015,9 +3015,6 @@ describe MergeRequest do subject { merge_request.rebase_in_progress? } it do - # Stub out the legacy gitaly implementation - allow(merge_request).to receive(:gitaly_rebase_in_progress?) { false } - allow(Gitlab::SidekiqStatus).to receive(:running?).with(rebase_jid) { jid_valid } merge_request.rebase_jid = rebase_jid @@ -3027,42 +3024,6 @@ describe MergeRequest do end end - describe '#gitaly_rebase_in_progress?' do - let(:repo_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - subject.source_project.repository.path - end - end - let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") } - - before do - system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{rebase_path} master)) - end - - it 'returns true when there is a current rebase directory' do - expect(subject.rebase_in_progress?).to be_truthy - end - - it 'returns false when there is no rebase directory' do - FileUtils.rm_rf(rebase_path) - - expect(subject.rebase_in_progress?).to be_falsey - end - - it 'returns false when the rebase directory has expired' do - time = 20.minutes.ago.to_time - File.utime(time, time, rebase_path) - - expect(subject.rebase_in_progress?).to be_falsey - end - - it 'returns false when the source project has been removed' do - allow(subject).to receive(:source_project).and_return(nil) - - expect(subject.rebase_in_progress?).to be_falsey - end - end - describe '#allow_collaboration' do let(:merge_request) do build(:merge_request, source_branch: 'fixes', allow_collaboration: true) diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 2b9c3c43af9..972f26ac745 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -853,4 +853,64 @@ describe Namespace do it { is_expected.to be_falsy } end end + + describe '#emails_disabled?' do + context 'when not a subgroup' do + it 'returns false' do + group = create(:group, emails_disabled: false) + + expect(group.emails_disabled?).to be_falsey + end + + it 'returns true' do + group = create(:group, emails_disabled: true) + + expect(group.emails_disabled?).to be_truthy + end + end + + context 'when a subgroup' do + let(:grandparent) { create(:group) } + let(:parent) { create(:group, parent: grandparent) } + let(:group) { create(:group, parent: parent) } + + it 'returns false' do + expect(group.emails_disabled?).to be_falsey + end + + context 'when ancestor emails are disabled' do + it 'returns true' do + grandparent.update_attribute(:emails_disabled, true) + + expect(group.emails_disabled?).to be_truthy + end + end + end + + context 'when :emails_disabled feature flag is off' do + before do + stub_feature_flags(emails_disabled: false) + end + + context 'when not a subgroup' do + it 'returns false' do + group = create(:group, emails_disabled: true) + + expect(group.emails_disabled?).to be_falsey + end + end + + context 'when a subgroup and ancestor emails are disabled' do + let(:grandparent) { create(:group) } + let(:parent) { create(:group, parent: grandparent) } + let(:group) { create(:group, parent: parent) } + + it 'returns false' do + grandparent.update_attribute(:emails_disabled, true) + + expect(group.emails_disabled?).to be_falsey + end + end + end + end end diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb index 4122736c148..2ba53818e54 100644 --- a/spec/models/notification_recipient_spec.rb +++ b/spec/models/notification_recipient_spec.rb @@ -9,6 +9,38 @@ describe NotificationRecipient do subject(:recipient) { described_class.new(user, :watch, target: target, project: project) } + describe '#notifiable?' do + let(:recipient) { described_class.new(user, :mention, target: target, project: project) } + + context 'when emails are disabled' do + it 'returns false if group disabled' do + expect(project.namespace).to receive(:emails_disabled?).and_return(true) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq false + end + + it 'returns false if project disabled' do + expect(project).to receive(:emails_disabled?).and_return(true) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq false + end + end + + context 'when emails are enabled' do + it 'returns true if group enabled' do + expect(project.namespace).to receive(:emails_disabled?).and_return(false) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq true + end + + it 'returns true if project enabled' do + expect(project).to receive(:emails_disabled?).and_return(false) + expect(recipient).to receive(:emails_disabled?).and_call_original + expect(recipient.notifiable?).to eq true + end + end + end + describe '#has_access?' do before do allow(user).to receive(:can?).and_call_original diff --git a/spec/models/project_services/emails_on_push_service_spec.rb b/spec/models/project_services/emails_on_push_service_spec.rb index 0a58eb367e3..ffe241aa880 100644 --- a/spec/models/project_services/emails_on_push_service_spec.rb +++ b/spec/models/project_services/emails_on_push_service_spec.rb @@ -20,4 +20,24 @@ describe EmailsOnPushService do it { is_expected.not_to validate_presence_of(:recipients) } end end + + context 'project emails' do + let(:push_data) { { object_kind: 'push' } } + let(:project) { create(:project, :repository) } + let(:service) { create(:emails_on_push_service, project: project) } + + it 'does not send emails when disabled' do + expect(project).to receive(:emails_disabled?).and_return(true) + expect(EmailsOnPushWorker).not_to receive(:perform_async) + + service.execute(push_data) + end + + it 'does send emails when enabled' do + expect(project).to receive(:emails_disabled?).and_return(false) + expect(EmailsOnPushWorker).to receive(:perform_async) + + service.execute(push_data) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2afe1253e29..ff9e94afc12 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -98,6 +98,7 @@ describe Project do it { is_expected.to have_many(:lfs_file_locks) } it { is_expected.to have_many(:project_deploy_tokens) } it { is_expected.to have_many(:deploy_tokens).through(:project_deploy_tokens) } + it { is_expected.to have_many(:cycle_analytics_stages) } it 'has an inverse relationship with merge requests' do expect(described_class.reflect_on_association(:merge_requests).has_inverse?).to eq(:target_project) @@ -2315,6 +2316,57 @@ describe Project do end end + describe '#emails_disabled?' do + let(:project) { create(:project, emails_disabled: false) } + + context 'emails disabled in group' do + it 'returns true' do + allow(project.namespace).to receive(:emails_disabled?) { true } + + expect(project.emails_disabled?).to be_truthy + end + end + + context 'emails enabled in group' do + before do + allow(project.namespace).to receive(:emails_disabled?) { false } + end + + it 'returns false' do + expect(project.emails_disabled?).to be_falsey + end + + it 'returns true' do + project.update_attribute(:emails_disabled, true) + + expect(project.emails_disabled?).to be_truthy + end + end + + context 'when :emails_disabled feature flag is off' do + before do + stub_feature_flags(emails_disabled: false) + end + + context 'emails disabled in group' do + it 'returns false' do + allow(project.namespace).to receive(:emails_disabled?) { true } + + expect(project.emails_disabled?).to be_falsey + end + end + + context 'emails enabled in group' do + it 'returns false' do + allow(project.namespace).to receive(:emails_disabled?) { false } + project.update_attribute(:emails_disabled, true) + + expect(project.emails_disabled?).to be_falsey + end + end + end + end + describe '#lfs_enabled?' do let(:project) { create(:project) } diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb index ca1ffe3c524..ef09c6effbb 100644 --- a/spec/requests/api/discussions_spec.rb +++ b/spec/requests/api/discussions_spec.rb @@ -9,6 +9,61 @@ describe API::Discussions do project.add_developer(user) end + context 'with cross-reference system notes', :request_store do + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let(:new_merge_request) { create(:merge_request) } + let(:commit) { new_merge_request.project.commit } + let!(:note) { create(:system_note, noteable: merge_request, project: project, note: cross_reference) } + let!(:note_metadata) { create(:system_note_metadata, note: note, action: 'cross_reference') } + let(:cross_reference) { "test commit #{commit.to_reference(project)}" } + let(:pat) { create(:personal_access_token, user: user) } + + let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/discussions" } + + before do + project.add_developer(user) + new_merge_request.project.add_developer(user) + end + + it 'returns only the note that the user should see' do + hidden_merge_request = create(:merge_request) + new_cross_reference = "test commit #{hidden_merge_request.project.commit}" + new_note = create(:system_note, noteable: merge_request, project: project, note: new_cross_reference) + create(:system_note_metadata, note: new_note, action: 'cross_reference') + + get api(url, user, personal_access_token: pat) + expect(response).to have_gitlab_http_status(200) + expect(json_response.count).to eq(1) + expect(json_response.first['notes'].count).to eq(1) + + parsed_note = json_response.first['notes'].first + expect(parsed_note['id']).to eq(note.id) + expect(parsed_note['body']).to eq(cross_reference) + expect(parsed_note['system']).to be true + end + + it 'avoids Git calls and N+1 SQL queries' do + expect_any_instance_of(Repository).not_to receive(:find_commit).with(commit.id) + + control = ActiveRecord::QueryRecorder.new do + get api(url, user, personal_access_token: pat) + end + + expect(response).to have_gitlab_http_status(200) + + RequestStore.clear! + + new_note = create(:system_note, noteable: merge_request, project: project, note: cross_reference) + create(:system_note_metadata, note: new_note, action: 'cross_reference') + + RequestStore.clear! + + expect { get api(url, user, personal_access_token: pat) }.not_to exceed_query_limit(control) + expect(response).to have_gitlab_http_status(200) + end + end + context 'when noteable is an Issue' do let!(:issue) { create(:issue, project: project, author: user) } let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) } diff --git a/spec/services/groups/update_service_spec.rb b/spec/services/groups/update_service_spec.rb index 5d4576139f7..12e9c2b2f3a 100644 --- a/spec/services/groups/update_service_spec.rb +++ b/spec/services/groups/update_service_spec.rb @@ -86,6 +86,7 @@ describe Groups::UpdateService do context "unauthorized visibility_level validation" do let!(:service) { described_class.new(internal_group, user, visibility_level: 99) } + before do internal_group.add_user(user, Gitlab::Access::MAINTAINER) end @@ -96,6 +97,20 @@ describe Groups::UpdateService do end end + context 'when updating #emails_disabled' do + let(:service) { described_class.new(internal_group, user, emails_disabled: true) } + + it 'updates the attribute' do + internal_group.add_user(user, Gitlab::Access::OWNER) + + expect { service.execute }.to change { internal_group.emails_disabled }.to(true) + end + + it 'does not update when not group owner' do + expect { service.execute }.not_to change { internal_group.emails_disabled } + end + end + context 'rename group' do let!(:service) { described_class.new(internal_group, user, path: SecureRandom.hex) } diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb index ee9caaf2f47..7b8c94c86fe 100644 --- a/spec/services/merge_requests/rebase_service_spec.rb +++ b/spec/services/merge_requests/rebase_service_spec.rb @@ -25,7 +25,7 @@ describe MergeRequests::RebaseService do describe '#execute' do context 'when another rebase is already in progress' do before do - allow(merge_request).to receive(:gitaly_rebase_in_progress?).and_return(true) + allow(repository).to receive(:rebase_in_progress?).with(merge_request.id).and_return(true) end it 'saves the error message' do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 1dcade1de0d..d925aa2b6c3 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -240,45 +240,50 @@ describe NotificationService, :mailer do end describe '#new_note' do - it do - add_users(project) - add_user_subscriptions(issue) - reset_delivered_emails! + context do + before do + add_users(project) + add_user_subscriptions(issue) + reset_delivered_emails! + end - expect(SentNotification).to receive(:record).with(issue, any_args).exactly(10).times + it do + expect(SentNotification).to receive(:record).with(issue, any_args).exactly(10).times - notification.new_note(note) + notification.new_note(note) - should_email(@u_watcher) - should_email(note.noteable.author) - should_email(note.noteable.assignees.first) - should_email(@u_custom_global) - should_email(@u_mentioned) - should_email(@subscriber) - should_email(@watcher_and_subscriber) - should_email(@subscribed_participant) - should_email(@u_custom_off) - should_email(@unsubscribed_mentioned) - should_not_email(@u_guest_custom) - should_not_email(@u_guest_watcher) - should_not_email(note.author) - should_not_email(@u_participating) - should_not_email(@u_disabled) - should_not_email(@unsubscriber) - should_not_email(@u_outsider_mentioned) - should_not_email(@u_lazy_participant) - end + should_email(@u_watcher) + should_email(note.noteable.author) + should_email(note.noteable.assignees.first) + should_email(@u_custom_global) + should_email(@u_mentioned) + should_email(@subscriber) + should_email(@watcher_and_subscriber) + should_email(@subscribed_participant) + should_email(@u_custom_off) + should_email(@unsubscribed_mentioned) + should_not_email(@u_guest_custom) + should_not_email(@u_guest_watcher) + should_not_email(note.author) + should_not_email(@u_participating) + should_not_email(@u_disabled) + should_not_email(@unsubscriber) + should_not_email(@u_outsider_mentioned) + should_not_email(@u_lazy_participant) + end - it "emails the note author if they've opted into notifications about their activity" do - add_users(project) - add_user_subscriptions(issue) - reset_delivered_emails! + it "emails the note author if they've opted into notifications about their activity" do + note.author.notified_of_own_activity = true - note.author.notified_of_own_activity = true + notification.new_note(note) - notification.new_note(note) + should_email(note.author) + end - should_email(note.author) + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end end it 'filters out "mentioned in" notes' do @@ -337,6 +342,11 @@ describe NotificationService, :mailer do it_behaves_like 'new note notifications' + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end + context 'which is a subgroup' do let!(:parent) { create(:group) } let!(:group) { create(:group, parent: parent) } @@ -472,6 +482,11 @@ describe NotificationService, :mailer do expect(Notify).not_to receive(:note_issue_email) notification.new_note(mentioned_note) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end end end @@ -619,6 +634,11 @@ describe NotificationService, :mailer do notification.new_note(note) should_not_email(@u_committer) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end end end @@ -645,6 +665,11 @@ describe NotificationService, :mailer do .to contain_exactly(*merge_request.assignees.pluck(:id), merge_request.author.id, @u_watcher.id) expect(SentNotification.last.in_reply_to_discussion_id).to eq(note.discussion_id) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { note } + let(:notification_trigger) { notification.new_note(note) } + end end end end @@ -819,6 +844,11 @@ describe NotificationService, :mailer do should_email(user_4) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.new_issue(issue, @u_disabled) } + end + context 'confidential issues' do let(:author) { create(:user) } let(:assignee) { create(:user) } @@ -861,6 +891,11 @@ describe NotificationService, :mailer do let(:mentionable) { issue } include_examples 'notifications for new mentions' + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned) } + end end describe '#reassigned_issue' do @@ -969,6 +1004,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled, [assignee]) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled, [assignee]) } + end end describe '#relabeled_issue' do @@ -1028,6 +1068,11 @@ describe NotificationService, :mailer do should_email(subscriber_to_both) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.relabeled_issue(issue, [group_label_2, label_2], @u_disabled) } + end + context 'confidential issues' do let(:author) { create(:user) } let(:assignee) { create(:user) } @@ -1065,12 +1110,19 @@ describe NotificationService, :mailer do end describe '#removed_milestone_issue' do - it_behaves_like 'altered milestone notification on issue' do + context do let(:milestone) { create(:milestone, project: project, issues: [issue]) } let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } } - before do - notification.removed_milestone_issue(issue, issue.author) + it_behaves_like 'altered milestone notification on issue' do + before do + notification.removed_milestone_issue(issue, issue.author) + end + end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.removed_milestone_issue(issue, issue.author) } end end @@ -1110,12 +1162,19 @@ describe NotificationService, :mailer do end describe '#changed_milestone_issue' do - it_behaves_like 'altered milestone notification on issue' do + context do let(:new_milestone) { create(:milestone, project: project, issues: [issue]) } let!(:subscriber_to_new_milestone) { create(:user) { |u| issue.toggle_subscription(u, project) } } - before do - notification.changed_milestone_issue(issue, new_milestone, issue.author) + it_behaves_like 'altered milestone notification on issue' do + before do + notification.changed_milestone_issue(issue, new_milestone, issue.author) + end + end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.changed_milestone_issue(issue, new_milestone, issue.author) } end end @@ -1183,6 +1242,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.close_issue(issue, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.close_issue(issue, @u_disabled) } + end end describe '#reopen_issue' do @@ -1214,6 +1278,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.reopen_issue(issue, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.reopen_issue(issue, @u_disabled) } + end end describe '#issue_moved' do @@ -1240,6 +1309,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.issue_moved(issue, new_issue, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.issue_moved(issue, new_issue, @u_disabled) } + end end describe '#issue_due' do @@ -1280,6 +1354,11 @@ describe NotificationService, :mailer do let(:issuable) { issue } let(:notification_trigger) { notification.issue_due(issue) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { issue } + let(:notification_trigger) { notification.issue_due(issue) } + end end end @@ -1374,6 +1453,11 @@ describe NotificationService, :mailer do should_email(user_4) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.new_merge_request(merge_request, @u_disabled) } + end + context 'participating' do it_should_behave_like 'participating by assignee notification' do let(:participant) { create(:user, username: 'user-participant')} @@ -1406,6 +1490,11 @@ describe NotificationService, :mailer do let(:mentionable) { merge_request } include_examples 'notifications for new mentions' + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned) } + end end describe '#reassigned_merge_request' do @@ -1449,6 +1538,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.reassigned_merge_request(merge_request, current_user, [assignee]) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.reassigned_merge_request(merge_request, current_user, [assignee]) } + end end describe '#push_to_merge_request' do @@ -1479,6 +1573,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.push_to_merge_request(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.push_to_merge_request(merge_request, @u_disabled) } + end end describe '#relabel_merge_request' do @@ -1512,28 +1611,43 @@ describe NotificationService, :mailer do should_not_email(@u_participating) should_not_email(@u_lazy_participant) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.relabeled_merge_request(merge_request, [group_label_2, label_2], @u_disabled) } + end end describe '#removed_milestone_merge_request' do - it_behaves_like 'altered milestone notification on merge request' do - let(:milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } - let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + let(:milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } + let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + it_behaves_like 'altered milestone notification on merge request' do before do notification.removed_milestone_merge_request(merge_request, merge_request.author) end end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.removed_milestone_merge_request(merge_request, merge_request.author) } + end end describe '#changed_milestone_merge_request' do - it_behaves_like 'altered milestone notification on merge request' do - let(:new_milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } - let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + let(:new_milestone) { create(:milestone, project: project, merge_requests: [merge_request]) } + let!(:subscriber_to_new_milestone) { create(:user) { |u| merge_request.toggle_subscription(u, project) } } + it_behaves_like 'altered milestone notification on merge request' do before do notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author) end end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.changed_milestone_merge_request(merge_request, new_milestone, merge_request.author) } + end end describe '#merge_request_unmergeable' do @@ -1544,6 +1658,11 @@ describe NotificationService, :mailer do expect(email_recipients.size).to eq(1) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.merge_request_unmergeable(merge_request) } + end + describe 'when merge_when_pipeline_succeeds is true' do before do merge_request.update( @@ -1590,6 +1709,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.close_mr(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.close_mr(merge_request, @u_disabled) } + end end describe '#merged_merge_request' do @@ -1642,6 +1766,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.merge_mr(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.merge_mr(merge_request, @u_disabled) } + end end describe '#reopen_merge_request' do @@ -1672,6 +1801,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.reopen_mr(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.reopen_mr(merge_request, @u_disabled) } + end end describe "#resolve_all_discussions" do @@ -1695,6 +1829,11 @@ describe NotificationService, :mailer do let(:issuable) { merge_request } let(:notification_trigger) { notification.resolve_all_discussions(merge_request, @u_disabled) } end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { merge_request } + let(:notification_trigger) { notification.resolve_all_discussions(merge_request, @u_disabled) } + end end end @@ -1719,6 +1858,11 @@ describe NotificationService, :mailer do should_not_email(@u_disabled) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.project_was_moved(project, "gitlab/gitlab") } + end + context 'users not having access to the new location' do it 'does not send email' do old_user = create(:user) @@ -1762,6 +1906,11 @@ describe NotificationService, :mailer do should_only_email(@u_participating) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.project_exported(project, @u_participating) } + end end describe '#project_not_exported' do @@ -1770,6 +1919,11 @@ describe NotificationService, :mailer do should_only_email(@u_participating) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.project_not_exported(project, @u_participating, ['error']) } + end end end end @@ -1800,6 +1954,11 @@ describe NotificationService, :mailer do should_email(maintainer) should_not_email(developer) end + + it_behaves_like 'group emails are disabled' do + let(:notification_target) { group } + let(:notification_trigger) { group.request_access(added_user) } + end end describe '#decline_group_invite' do @@ -1839,6 +1998,11 @@ describe NotificationService, :mailer do should_not_email_anyone end end + + it_behaves_like 'group emails are disabled' do + let(:notification_target) { group } + let(:notification_trigger) { group.add_guest(added_user) } + end end end @@ -1859,6 +2023,11 @@ describe NotificationService, :mailer do should_only_email(project.owner) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { project.request_access(added_user) } + end end context 'for a project in a group' do @@ -1878,7 +2047,7 @@ describe NotificationService, :mailer do end end - describe '#decline_group_invite' do + describe '#decline_project_invite' do let(:member) { create(:user) } before do @@ -1900,6 +2069,11 @@ describe NotificationService, :mailer do should_only_email(added_user) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { create_member! } + end + context 'when notifications are disabled' do before do create_global_setting_for(added_user, :disabled) @@ -2071,6 +2245,11 @@ describe NotificationService, :mailer do should_only_email(u_custom_notification_enabled, kind: :bcc) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { pipeline } + let(:notification_trigger) { notification.pipeline_finished(pipeline) } + end + context 'when the creator has group notification email set' do let(:group_notification_email) { 'user+group@example.com' } @@ -2100,6 +2279,11 @@ describe NotificationService, :mailer do should_only_email(u_member, kind: :bcc) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { pipeline } + let(:notification_trigger) { notification.pipeline_finished(pipeline) } + end + context 'when the creator has group notification email set' do let(:group_notification_email) { 'user+group@example.com' } @@ -2215,6 +2399,11 @@ describe NotificationService, :mailer do should_only_email(u_maintainer1, u_maintainer2, u_owner) end + it_behaves_like 'project emails are disabled' do + let(:notification_target) { domain } + let(:notification_trigger) { notify! } + end + it 'emails nobody if the project is missing' do domain.project = nil @@ -2224,30 +2413,6 @@ describe NotificationService, :mailer do end end end - - describe '#pages_domain_verification_failed' do - it 'emails current watching maintainers' do - notification.pages_domain_verification_failed(domain) - - should_only_email(u_maintainer1, u_maintainer2, u_owner) - end - end - - describe '#pages_domain_enabled' do - it 'emails current watching maintainers' do - notification.pages_domain_enabled(domain) - - should_only_email(u_maintainer1, u_maintainer2, u_owner) - end - end - - describe '#pages_domain_disabled' do - it 'emails current watching maintainers' do - notification.pages_domain_disabled(domain) - - should_only_email(u_maintainer1, u_maintainer2, u_owner) - end - end end context 'Auto DevOps notifications' do @@ -2266,6 +2431,11 @@ describe NotificationService, :mailer do should_email(owner, times: 1) # Once for the disable pipeline. should_email(pipeline_user, times: 2) # Once for the new permission, once for the disable. end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.autodevops_disabled(pipeline, [owner.email, pipeline_user.email]) } + end end end @@ -2279,6 +2449,11 @@ describe NotificationService, :mailer do should_email(user) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.repository_cleanup_success(project, user) } + end end describe '#repository_cleanup_failure' do @@ -2287,6 +2462,11 @@ describe NotificationService, :mailer do should_email(user) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.repository_cleanup_failure(project, user, 'Some error') } + end end end @@ -2320,6 +2500,11 @@ describe NotificationService, :mailer do should_only_email(u_maintainer1, u_maintainer2, u_owner) end + + it_behaves_like 'project emails are disabled' do + let(:notification_target) { project } + let(:notification_trigger) { notification.remote_mirror_update_failed(remote_mirror) } + end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 82010dd283c..31bd0f0f836 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -369,9 +369,28 @@ describe Projects::UpdateService do end end + context 'when updating #emails_disabled' do + it 'updates the attribute for the project owner' do + expect { update_project(project, user, emails_disabled: true) } + .to change { project.emails_disabled } + .to(true) + end + + it 'does not update when not project owner' do + maintainer = create(:user) + project.add_user(maintainer, :maintainer) + + expect { update_project(project, maintainer, emails_disabled: true) } + .not_to change { project.emails_disabled } + end + end + context 'with external authorization enabled' do before do enable_external_authorization_service_check + + allow(::Gitlab::ExternalAuthorization) + .to receive(:access_allowed?).with(user, 'default_label', project.full_path).and_call_original end it 'does not save the project with an error if the service denies access' do @@ -402,8 +421,7 @@ describe Projects::UpdateService do end it 'does not check the label when it does not change' do - expect(::Gitlab::ExternalAuthorization) - .not_to receive(:access_allowed?) + expect(::Gitlab::ExternalAuthorization).to receive(:access_allowed?).once update_project(project, user, { name: 'New name' }) end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index bcc133790d1..bd504f1553b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -48,6 +48,9 @@ Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } quality_level = Quality::TestLevel.new RSpec.configure do |config| + config.filter_run focus: true + config.run_all_when_everything_filtered = true + config.use_transactional_fixtures = true config.use_instantiated_fixtures = false config.fixture_path = Rails.root diff --git a/spec/support/helpers/email_helpers.rb b/spec/support/helpers/email_helpers.rb index 83ba654fab3..024340310a1 100644 --- a/spec/support/helpers/email_helpers.rb +++ b/spec/support/helpers/email_helpers.rb @@ -31,6 +31,10 @@ module EmailHelpers expect(ActionMailer::Base.deliveries).to be_empty end + def should_email_anyone + expect(ActionMailer::Base.deliveries).not_to be_empty + end + def email_recipients(kind: :to) ActionMailer::Base.deliveries.flat_map(&kind) end diff --git a/spec/support/shared_examples/services/notification_service_shared_examples.rb b/spec/support/shared_examples/services/notification_service_shared_examples.rb new file mode 100644 index 00000000000..dd338ea47c7 --- /dev/null +++ b/spec/support/shared_examples/services/notification_service_shared_examples.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +# Note that we actually update the attribute on the target_project/group, rather than +# using `allow`. This is because there are some specs where, based on how the notification +# is done, using an `allow` doesn't change the correct object. +shared_examples 'project emails are disabled' do + let(:target_project) { notification_target.is_a?(Project) ? notification_target : notification_target.project } + + before do + reset_delivered_emails! + target_project.clear_memoization(:emails_disabled) + end + + it 'sends no emails with project emails disabled' do + target_project.update_attribute(:emails_disabled, true) + + notification_trigger + + should_not_email_anyone + end + + it 'sends emails to someone' do + target_project.update_attribute(:emails_disabled, false) + + notification_trigger + + should_email_anyone + end +end + +shared_examples 'group emails are disabled' do + let(:target_group) { notification_target.is_a?(Group) ? notification_target : notification_target.project.group } + + before do + reset_delivered_emails! + target_group.clear_memoization(:emails_disabled) + end + + it 'sends no emails with group emails disabled' do + target_group.update_attribute(:emails_disabled, true) + + notification_trigger + + should_not_email_anyone + end + + it 'sends emails to someone' do + target_group.update_attribute(:emails_disabled, false) + + notification_trigger + + should_email_anyone + end +end diff --git a/spec/tasks/gitlab/update_templates_rake_spec.rb b/spec/tasks/gitlab/update_templates_rake_spec.rb index 7b17549b8c7..14b4ad5e3d8 100644 --- a/spec/tasks/gitlab/update_templates_rake_spec.rb +++ b/spec/tasks/gitlab/update_templates_rake_spec.rb @@ -8,9 +8,18 @@ describe 'gitlab:update_project_templates rake task' do before do Rake.application.rake_require 'tasks/gitlab/update_templates' create(:admin) + allow(Gitlab::ProjectTemplate) .to receive(:archive_directory) .and_return(Pathname.new(tmpdir)) + + # Gitlab::HTTP resolves the domain to an IP prior to WebMock taking effect, hence the wildcard + stub_request(:get, %r{^https://.*/api/v4/projects/gitlab-org%2Fproject-templates%2Frails/repository/commits\?page=1&per_page=1}) + .to_return( + status: 200, + body: [{ id: '67812735b83cb42710f22dc98d73d42c8bf4d907' }].to_json, + headers: { 'Content-Type' => 'application/json' } + ) end after do |