diff options
21 files changed, 485 insertions, 120 deletions
diff --git a/app/mailers/emails/in_product_marketing.rb b/app/mailers/emails/in_product_marketing.rb index a0d4f335cf4..ba94c0c8cac 100644 --- a/app/mailers/emails/in_product_marketing.rb +++ b/app/mailers/emails/in_product_marketing.rb @@ -21,6 +21,12 @@ module Emails mail_to(to: email, subject: @message.subject_line) end + def account_validation_email(pipeline, recipient_email) + @message = Gitlab::Email::Message::AccountValidation.new(pipeline) + + mail_to(to: recipient_email, subject: @message.subject_line) + end + private def mail_to(to:, subject:) diff --git a/app/views/layouts/in_product_marketing_mailer.html.haml b/app/views/layouts/in_product_marketing_mailer.html.haml index 4f8deb22eae..679a2d4b8b3 100644 --- a/app/views/layouts/in_product_marketing_mailer.html.haml +++ b/app/views/layouts/in_product_marketing_mailer.html.haml @@ -55,16 +55,23 @@ .cta_link a { font-size: 24px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif; - color: #ffffff; text-decoration: none; + display: inline-block; + } + + .cta_link_primary a { + color: #ffffff; border-radius: 5px; - -webkit-border-radius: 5px; background-color: #6e49cb; border-top: 15px solid #6e49cb; border-bottom: 15px solid #6e49cb; border-right: 40px solid #6e49cb; border-left: 40px solid #6e49cb; - display: inline-block; + } + + .cta_link_secondary a { + color: #6e49cb; + padding: 25px 40px 15px; } .footernav { diff --git a/app/views/notify/account_validation_email.html.haml b/app/views/notify/account_validation_email.html.haml new file mode 100644 index 00000000000..02256443430 --- /dev/null +++ b/app/views/notify/account_validation_email.html.haml @@ -0,0 +1,16 @@ +%tr + %td{ bgcolor: "#ffffff", height: "auto", style: "max-width: 600px; width: 100%; text-align: center; height: 200px; padding: 25px 15px; mso-line-height-rule: exactly; min-height: 40px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif;", valign: "middle", width: "100%" } + = inline_image_link(@message.logo_path, { width: '150', style: 'width: 150px;' }) + %h1{ style: "font-size: 40px; line-height: 46x; color: #000000; padding: 20px 0 0 0; font-weight: normal;" } + = @message.title +%tr + %td{ style: "padding: 10px 20px 30px 20px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif; color:#000000; font-size: 18px; line-height: 24px;" } + %p{ style: "margin: 0 0 20px 0;" } + = @message.body_line1.html_safe + - @message.body_line2&.tap do |line| + %p{ style: "margin: 0 0 20px 0;" } + = line.html_safe +%tr + %td{ align: "center", style: "padding: 20px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif;" } + .cta_link.cta_link_primary= @message.cta_link + .cta_link.cta_link_secondary= @message.cta2_link diff --git a/app/views/notify/account_validation_email.text.erb b/app/views/notify/account_validation_email.text.erb new file mode 100644 index 00000000000..c167eff9803 --- /dev/null +++ b/app/views/notify/account_validation_email.text.erb @@ -0,0 +1,15 @@ +<%= @message.title %> + +<%= @message.body_line1 %> + +<%= @message.body_line2 %> + +<%= @message.cta_link %> + +<%= @message.cta2_link %> + +<%= @message.footer_links %> + +<%= @message.address %> + +<%= @message.unsubscribe %> diff --git a/app/views/notify/in_product_marketing_email.html.haml b/app/views/notify/in_product_marketing_email.html.haml index 90f45c520cf..a88d581c5de 100644 --- a/app/views/notify/in_product_marketing_email.html.haml +++ b/app/views/notify/in_product_marketing_email.html.haml @@ -20,7 +20,7 @@ - if @message.cta_text %tr %td{ align: "center", style: "padding: 10px 20px 80px 20px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif;" } - .cta_link= @message.cta_link + .cta_link.cta_link_primary= @message.cta_link - else %tr %td{ style: "padding: 10px 20px 10px 20px; font-family: 'Source Sans Pro', helvetica, arial, sans-serif; color:#000000; font-size: 16px; line-height: 20px;" } diff --git a/db/post_migrate/20211118130836_drop_pages_deployments_builds_fk.rb b/db/post_migrate/20211118130836_drop_pages_deployments_builds_fk.rb new file mode 100644 index 00000000000..5eb532f2a00 --- /dev/null +++ b/db/post_migrate/20211118130836_drop_pages_deployments_builds_fk.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class DropPagesDeploymentsBuildsFk < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + FK_NAME = 'fk_rails_c3a90cf29b' + + def up + remove_foreign_key_if_exists(:pages_deployments, :ci_builds, name: FK_NAME) + end + + def down + add_concurrent_foreign_key( + :pages_deployments, + :ci_builds, + name: FK_NAME, + column: :ci_build_id, + target_column: :id, + on_delete: :nullify + ) + end +end diff --git a/db/schema_migrations/20211118130836 b/db/schema_migrations/20211118130836 new file mode 100644 index 00000000000..df16d8c5c5e --- /dev/null +++ b/db/schema_migrations/20211118130836 @@ -0,0 +1 @@ +2630b21c7134ac539a18798f2f2b99f468e171b79e30a184f7e8cdaccd11d465
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index d591bee6b21..cfe07cac82b 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -31056,9 +31056,6 @@ ALTER TABLE ONLY packages_nuget_dependency_link_metadata ALTER TABLE ONLY group_deploy_keys_groups ADD CONSTRAINT fk_rails_c3854f19f5 FOREIGN KEY (group_deploy_key_id) REFERENCES group_deploy_keys(id) ON DELETE CASCADE; -ALTER TABLE ONLY pages_deployments - ADD CONSTRAINT fk_rails_c3a90cf29b FOREIGN KEY (ci_build_id) REFERENCES ci_builds(id) ON DELETE SET NULL; - ALTER TABLE ONLY merge_request_user_mentions ADD CONSTRAINT fk_rails_c440b9ea31 FOREIGN KEY (note_id) REFERENCES notes(id) ON DELETE CASCADE; diff --git a/doc/api/graphql/removed_items.md b/doc/api/graphql/removed_items.md index 53f8872e74c..f70add0de45 100644 --- a/doc/api/graphql/removed_items.md +++ b/doc/api/graphql/removed_items.md @@ -4,7 +4,7 @@ group: Integrations info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# GraphQL API removed items +# GraphQL API removed items **(FREE)** GraphQL is a versionless API, unlike the REST API. Occasionally, items have to be updated or removed from the GraphQL API. diff --git a/doc/ci/jobs/ci_job_token.md b/doc/ci/jobs/ci_job_token.md index ab9f0b25bf2..e1a7af164b2 100644 --- a/doc/ci/jobs/ci_job_token.md +++ b/doc/ci/jobs/ci_job_token.md @@ -4,7 +4,7 @@ group: Pipeline Execution info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# GitLab CI/CD job token +# GitLab CI/CD job token **(FREE)** When a pipeline job is about to run, GitLab generates a unique token and injects it as the [`CI_JOB_TOKEN` predefined variable](../variables/predefined_variables.md). diff --git a/doc/integration/jira/connect-app.md b/doc/integration/jira/connect-app.md index 27f482ee2ba..597293ae5ca 100644 --- a/doc/integration/jira/connect-app.md +++ b/doc/integration/jira/connect-app.md @@ -76,7 +76,7 @@ If the app requires additional permissions, [the update must first be manually a ## Install the GitLab.com for Jira Cloud app for self-managed instances **(FREE SELF)** If your GitLab instance is self-managed, you must follow some -extra steps to install the GitLab.com for Jira Cloud app. +extra steps to install the GitLab.com for Jira Cloud app, and your GitLab instance must be accessible by Jira. Each Jira Cloud application must be installed from a single location. Jira fetches a [manifest file](https://developer.atlassian.com/cloud/jira/platform/connect-app-descriptor/) diff --git a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb index 73cfa02ce4b..651ed23eb25 100644 --- a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb +++ b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb @@ -34,7 +34,7 @@ module Gitlab end def file_name - "#{report_type.to_s.dasherize}-report-format.json" + report_type == :api_fuzzing ? "dast-report-format.json" : "#{report_type.to_s.dasherize}-report-format.json" end end diff --git a/lib/gitlab/database/gitlab_loose_foreign_keys.yml b/lib/gitlab/database/gitlab_loose_foreign_keys.yml index 61549aac746..aac1af4cd0f 100644 --- a/lib/gitlab/database/gitlab_loose_foreign_keys.yml +++ b/lib/gitlab/database/gitlab_loose_foreign_keys.yml @@ -37,3 +37,7 @@ packages_package_file_build_infos: - table: ci_pipelines column: pipeline_id on_delete: async_nullify +pages_deployments: + - table: ci_builds + column: ci_build_id + on_delete: async_nullify diff --git a/lib/gitlab/email/message/account_validation.rb b/lib/gitlab/email/message/account_validation.rb new file mode 100644 index 00000000000..92c3fc25059 --- /dev/null +++ b/lib/gitlab/email/message/account_validation.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +module Gitlab + module Email + module Message + class AccountValidation + include Gitlab::Email::Message::InProductMarketing::Helper + include Gitlab::Routing + + attr_accessor :pipeline, :format + + def initialize(pipeline, format: :html) + @pipeline = pipeline + @format = format + end + + def subject_line + s_('AccountValidation|Fix your pipelines by validating your account') + end + + def title + s_("AccountValidation|Looks like you’ll need to validate your account to use free pipeline minutes") + end + + def body_line1 + s_("AccountValidation|In order to use free pipeline minutes on shared runners, you'll need to validate your account with a credit or debit card. If you prefer not to provide one, you can run pipelines by bringing your own runners and disabling shared runners for your project.") + end + + def body_line2 + format_options = strong_options.merge({ learn_more_link: learn_more_link }) + s_("AccountValidation|This is required to discourage and reduce the abuse on GitLab infrastructure. %{strong_start}GitLab will not charge or store your card, it will only be used for validation.%{strong_end} %{learn_more_link}").html_safe % format_options + end + + def cta_text + s_('AccountValidation|Validate your account') + end + + def cta2_text + s_("AccountValidation|I'll bring my own runners") + end + + def logo_path + 'mailers/in_product_marketing/verify-2.png' + end + + def cta_link + url = project_pipeline_url(pipeline.project, pipeline) + + case format + when :html + ActionController::Base.helpers.link_to cta_text, url, target: '_blank', rel: 'noopener noreferrer' + else + [cta_text, url].join(' >> ') + end + end + + def cta2_link + url = 'https://docs.gitlab.com/runner/install/' + + case format + when :html + ActionController::Base.helpers.link_to cta2_text, url, target: '_blank', rel: 'noopener noreferrer' + else + [cta2_text, url].join(' >> ') + end + end + + def learn_more_link + link(s_('AccountValidation|Learn more.'), 'https://about.gitlab.com/blog/2021/05/17/prevent-crypto-mining-abuse/') + end + + def unsubscribe + parts = [ + s_('AccountValidation|If you no longer wish to receive marketing emails from us,'), + s_('AccountValidation|you may %{unsubscribe_link} at any time.') % { unsubscribe_link: mailgun_unsubscribe_link } + ] + + case format + when :html + parts.join(' ') + else + parts.join("\n" + ' ' * 16) + end + end + + private + + def mailgun_unsubscribe_link + mailgun_unsubscribe_url = '%tag_unsubscribe_url%' + + link(s_('AccountValidation|unsubscribe'), mailgun_unsubscribe_url) + end + end + end + end +end diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb index 7ce0e980f30..917c273d3f6 100644 --- a/lib/gitlab/usage_data.rb +++ b/lib/gitlab/usage_data.rb @@ -406,7 +406,8 @@ module Gitlab results[:projects_jira_cloud_active] = jira_integration_data_hash[:projects_jira_cloud_active] results - rescue ActiveRecord::StatementInvalid + rescue ActiveRecord::StatementInvalid => error + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error) { projects_jira_server_active: FALLBACK, projects_jira_cloud_active: FALLBACK } end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/gitlab/utils/usage_data.rb b/lib/gitlab/utils/usage_data.rb index aded61987db..e347168f419 100644 --- a/lib/gitlab/utils/usage_data.rb +++ b/lib/gitlab/utils/usage_data.rb @@ -56,7 +56,8 @@ module Gitlab else relation.count end - rescue ActiveRecord::StatementInvalid + rescue ActiveRecord::StatementInvalid => error + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error) FALLBACK end @@ -66,7 +67,8 @@ module Gitlab else relation.distinct_count_by(column) end - rescue ActiveRecord::StatementInvalid + rescue ActiveRecord::StatementInvalid => error + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error) FALLBACK end @@ -78,7 +80,8 @@ module Gitlab yield buckets if block_given? buckets.estimated_distinct_count - rescue ActiveRecord::StatementInvalid + rescue ActiveRecord::StatementInvalid => error + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error) FALLBACK # catch all rescue should be removed as a part of feature flag rollout issue # https://gitlab.com/gitlab-org/gitlab/-/issues/285485 @@ -89,7 +92,8 @@ module Gitlab def sum(relation, column, batch_size: nil, start: nil, finish: nil) Gitlab::Database::BatchCount.batch_sum(relation, column, batch_size: batch_size, start: start, finish: finish) - rescue ActiveRecord::StatementInvalid + rescue ActiveRecord::StatementInvalid => error + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(error) FALLBACK end @@ -155,7 +159,8 @@ module Gitlab query: query.to_sql, message: e.message ) - + # Raises error for dev env + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) HISTOGRAM_FALLBACK end # rubocop: enable CodeReuse/ActiveRecord diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 63aad6dfd66..20add87f831 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1877,6 +1877,36 @@ msgstr "" msgid "Account: %{account}" msgstr "" +msgid "AccountValidation|Fix your pipelines by validating your account" +msgstr "" + +msgid "AccountValidation|I'll bring my own runners" +msgstr "" + +msgid "AccountValidation|If you no longer wish to receive marketing emails from us," +msgstr "" + +msgid "AccountValidation|In order to use free pipeline minutes on shared runners, you'll need to validate your account with a credit or debit card. If you prefer not to provide one, you can run pipelines by bringing your own runners and disabling shared runners for your project." +msgstr "" + +msgid "AccountValidation|Learn more." +msgstr "" + +msgid "AccountValidation|Looks like you’ll need to validate your account to use free pipeline minutes" +msgstr "" + +msgid "AccountValidation|This is required to discourage and reduce the abuse on GitLab infrastructure. %{strong_start}GitLab will not charge or store your card, it will only be used for validation.%{strong_end} %{learn_more_link}" +msgstr "" + +msgid "AccountValidation|Validate your account" +msgstr "" + +msgid "AccountValidation|unsubscribe" +msgstr "" + +msgid "AccountValidation|you may %{unsubscribe_link} at any time." +msgstr "" + msgid "Action" msgstr "" @@ -5418,10 +5448,10 @@ msgstr "" msgid "Billings|Shared runners cannot be enabled until a valid credit card is on file." msgstr "" -msgid "Billings|To use free pipeline minutes on shared runners, you’ll need to validate your account with a credit or debit card. If you prefer not to provide one, you can run pipelines by bringing your own runners and disabling shared runners for your project. This is required to discourage and reduce abuse on GitLab infrastructure. %{strongStart}GitLab will not charge or store your card, it will only be used for validation.%{strongEnd} %{linkStart}Learn more%{linkEnd}." +msgid "Billings|To use free pipeline minutes on shared runners, you’ll need to validate your account with a credit or debit card. If you prefer not to provide one, you can run pipelines by bringing your own runners and disabling shared runners for your project. This is required to discourage and reduce abuse on GitLab infrastructure. %{strongStart}GitLab will not charge your card, it will only be used for validation.%{strongEnd} %{linkStart}Learn more%{linkEnd}." msgstr "" -msgid "Billings|To use free pipeline minutes on shared runners, you’ll need to validate your account with a credit or debit card. This is required to discourage and reduce abuse on GitLab infrastructure. %{strongStart}GitLab will not charge or store your card, it will only be used for validation.%{strongEnd}" +msgid "Billings|To use free pipeline minutes on shared runners, you’ll need to validate your account with a credit or debit card. This is required to discourage and reduce abuse on GitLab infrastructure. %{strongStart}GitLab will not charge your card, it will only be used for validation.%{strongEnd}" msgstr "" msgid "Billings|User successfully validated" diff --git a/spec/lib/gitlab/email/message/account_validation_spec.rb b/spec/lib/gitlab/email/message/account_validation_spec.rb new file mode 100644 index 00000000000..03d9414b914 --- /dev/null +++ b/spec/lib/gitlab/email/message/account_validation_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Email::Message::AccountValidation do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project) { create(:project, :repository, namespace: namespace) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + subject(:message) { described_class.new(pipeline) } + + it 'contains the correct message', :aggregate_failures do + expect(message.subject_line).to eq 'Fix your pipelines by validating your account' + expect(message.title).to eq "Looks like you’ll need to validate your account to use free pipeline minutes" + expect(message.body_line1).to eq "In order to use free pipeline minutes on shared runners, you'll need to validate your account with a credit or debit card. If you prefer not to provide one, you can run pipelines by bringing your own runners and disabling shared runners for your project." + expect(message.body_line2).to include( + 'This is required to discourage and reduce the abuse on GitLab infrastructure.', + '<b>GitLab will not charge or store your card, it will only be used for validation.</b>', + '<a href="https://about.gitlab.com/blog/2021/05/17/prevent-crypto-mining-abuse/">Learn more.</a>' + ) + expect(message.cta_text).to eq 'Validate your account' + expect(message.cta2_text).to eq "I'll bring my own runners" + expect(message.logo_path).to eq 'mailers/in_product_marketing/verify-2.png' + expect(message.unsubscribe).to include('%tag_unsubscribe_url%') + end +end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index 66cb95a59a6..015ecd1671e 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -684,23 +684,50 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do end end - it 'works when queries time out' do - allow_any_instance_of(ActiveRecord::Relation) - .to receive(:count).and_raise(ActiveRecord::StatementInvalid.new('')) + context 'when queries time out' do + let(:metric_method) { :count } + + before do + allow_any_instance_of(ActiveRecord::Relation).to receive(metric_method).and_raise(ActiveRecord::StatementInvalid) + allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(should_raise_for_dev) + end + + context 'with should_raise_for_dev? true' do + let(:should_raise_for_dev) { true } + + it 'raises an error' do + expect { subject }.to raise_error(ActiveRecord::StatementInvalid) + end + + context 'when metric calls find_in_batches' do + let(:metric_method) { :find_in_batches } + + it 'raises an error for jira_usage' do + expect { described_class.jira_usage }.to raise_error(ActiveRecord::StatementInvalid) + end + end + end + + context 'with should_raise_for_dev? false' do + let(:should_raise_for_dev) { false } + + it 'does not raise an error' do + expect { subject }.not_to raise_error + end - expect { subject }.not_to raise_error + context 'when metric calls find_in_batches' do + let(:metric_method) { :find_in_batches } + + it 'does not raise an error for jira_usage' do + expect { described_class.jira_usage }.not_to raise_error + end + end + end end it 'includes a recording_ce_finished_at timestamp' do expect(subject[:recording_ce_finished_at]).to be_a(Time) end - - it 'jira usage works when queries time out' do - allow_any_instance_of(ActiveRecord::Relation) - .to receive(:find_in_batches).and_raise(ActiveRecord::StatementInvalid.new('')) - - expect { described_class.jira_usage }.not_to raise_error - end end describe '.system_usage_data_monthly' do @@ -1355,46 +1382,58 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do context 'when queries time out' do before do - allow_any_instance_of(ActiveRecord::Relation) - .to receive(:count).and_raise(ActiveRecord::StatementInvalid.new('')) + allow_any_instance_of(ActiveRecord::Relation).to receive(:count).and_raise(ActiveRecord::StatementInvalid) + allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(should_raise_for_dev) end - it 'returns -1 for email campaign data' do - expected_data = { - "in_product_marketing_email_create_0_sent" => -1, - "in_product_marketing_email_create_0_cta_clicked" => -1, - "in_product_marketing_email_create_1_sent" => -1, - "in_product_marketing_email_create_1_cta_clicked" => -1, - "in_product_marketing_email_create_2_sent" => -1, - "in_product_marketing_email_create_2_cta_clicked" => -1, - "in_product_marketing_email_team_short_0_sent" => -1, - "in_product_marketing_email_team_short_0_cta_clicked" => -1, - "in_product_marketing_email_trial_short_0_sent" => -1, - "in_product_marketing_email_trial_short_0_cta_clicked" => -1, - "in_product_marketing_email_admin_verify_0_sent" => -1, - "in_product_marketing_email_admin_verify_0_cta_clicked" => -1, - "in_product_marketing_email_verify_0_sent" => -1, - "in_product_marketing_email_verify_0_cta_clicked" => -1, - "in_product_marketing_email_verify_1_sent" => -1, - "in_product_marketing_email_verify_1_cta_clicked" => -1, - "in_product_marketing_email_verify_2_sent" => -1, - "in_product_marketing_email_verify_2_cta_clicked" => -1, - "in_product_marketing_email_trial_0_sent" => -1, - "in_product_marketing_email_trial_0_cta_clicked" => -1, - "in_product_marketing_email_trial_1_sent" => -1, - "in_product_marketing_email_trial_1_cta_clicked" => -1, - "in_product_marketing_email_trial_2_sent" => -1, - "in_product_marketing_email_trial_2_cta_clicked" => -1, - "in_product_marketing_email_team_0_sent" => -1, - "in_product_marketing_email_team_0_cta_clicked" => -1, - "in_product_marketing_email_team_1_sent" => -1, - "in_product_marketing_email_team_1_cta_clicked" => -1, - "in_product_marketing_email_team_2_sent" => -1, - "in_product_marketing_email_team_2_cta_clicked" => -1, - "in_product_marketing_email_experience_0_sent" => -1 - } + context 'with should_raise_for_dev? true' do + let(:should_raise_for_dev) { true } - expect(subject).to eq(expected_data) + it 'raises an error' do + expect { subject }.to raise_error(ActiveRecord::StatementInvalid) + end + end + + context 'with should_raise_for_dev? false' do + let(:should_raise_for_dev) { false } + + it 'returns -1 for email campaign data' do + expected_data = { + "in_product_marketing_email_create_0_sent" => -1, + "in_product_marketing_email_create_0_cta_clicked" => -1, + "in_product_marketing_email_create_1_sent" => -1, + "in_product_marketing_email_create_1_cta_clicked" => -1, + "in_product_marketing_email_create_2_sent" => -1, + "in_product_marketing_email_create_2_cta_clicked" => -1, + "in_product_marketing_email_team_short_0_sent" => -1, + "in_product_marketing_email_team_short_0_cta_clicked" => -1, + "in_product_marketing_email_trial_short_0_sent" => -1, + "in_product_marketing_email_trial_short_0_cta_clicked" => -1, + "in_product_marketing_email_admin_verify_0_sent" => -1, + "in_product_marketing_email_admin_verify_0_cta_clicked" => -1, + "in_product_marketing_email_verify_0_sent" => -1, + "in_product_marketing_email_verify_0_cta_clicked" => -1, + "in_product_marketing_email_verify_1_sent" => -1, + "in_product_marketing_email_verify_1_cta_clicked" => -1, + "in_product_marketing_email_verify_2_sent" => -1, + "in_product_marketing_email_verify_2_cta_clicked" => -1, + "in_product_marketing_email_trial_0_sent" => -1, + "in_product_marketing_email_trial_0_cta_clicked" => -1, + "in_product_marketing_email_trial_1_sent" => -1, + "in_product_marketing_email_trial_1_cta_clicked" => -1, + "in_product_marketing_email_trial_2_sent" => -1, + "in_product_marketing_email_trial_2_cta_clicked" => -1, + "in_product_marketing_email_team_0_sent" => -1, + "in_product_marketing_email_team_0_cta_clicked" => -1, + "in_product_marketing_email_team_1_sent" => -1, + "in_product_marketing_email_team_1_cta_clicked" => -1, + "in_product_marketing_email_team_2_sent" => -1, + "in_product_marketing_email_team_2_cta_clicked" => -1, + "in_product_marketing_email_experience_0_sent" => -1 + } + + expect(subject).to eq(expected_data) + end end end diff --git a/spec/lib/gitlab/utils/usage_data_spec.rb b/spec/lib/gitlab/utils/usage_data_spec.rb index e5f3eaed3ad..a88ab9435f2 100644 --- a/spec/lib/gitlab/utils/usage_data_spec.rb +++ b/spec/lib/gitlab/utils/usage_data_spec.rb @@ -5,6 +5,30 @@ require 'spec_helper' RSpec.describe Gitlab::Utils::UsageData do include Database::DatabaseHelpers + shared_examples 'failing hardening method' do + before do + allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(should_raise_for_dev) + stub_const("Gitlab::Utils::UsageData::FALLBACK", fallback) + allow(failing_class).to receive(failing_method).and_raise(ActiveRecord::StatementInvalid) + end + + context 'with should_raise_for_dev? false' do + let(:should_raise_for_dev) { false } + + it 'returns the fallback' do + expect(subject).to eq(fallback) + end + end + + context 'with should_raise_for_dev? true' do + let(:should_raise_for_dev) { true } + + it 'raises an error' do + expect { subject }.to raise_error(ActiveRecord::StatementInvalid) + end + end + end + describe '#add_metric' do let(:metric) { 'UuidMetric'} @@ -22,11 +46,14 @@ RSpec.describe Gitlab::Utils::UsageData do expect(described_class.count(relation, batch: false)).to eq(1) end - it 'returns the fallback value when counting fails' do - stub_const("Gitlab::Utils::UsageData::FALLBACK", 15) - allow(relation).to receive(:count).and_raise(ActiveRecord::StatementInvalid.new('')) + context 'when counting fails' do + subject { described_class.count(relation, batch: false) } - expect(described_class.count(relation, batch: false)).to eq(15) + let(:fallback) { 15 } + let(:failing_class) { relation } + let(:failing_method) { :count } + + it_behaves_like 'failing hardening method' end end @@ -39,11 +66,14 @@ RSpec.describe Gitlab::Utils::UsageData do expect(described_class.distinct_count(relation, batch: false)).to eq(1) end - it 'returns the fallback value when counting fails' do - stub_const("Gitlab::Utils::UsageData::FALLBACK", 15) - allow(relation).to receive(:distinct_count_by).and_raise(ActiveRecord::StatementInvalid.new('')) + context 'when counting fails' do + subject { described_class.distinct_count(relation, batch: false) } + + let(:fallback) { 15 } + let(:failing_class) { relation } + let(:failing_method) { :distinct_count_by } - expect(described_class.distinct_count(relation, batch: false)).to eq(15) + it_behaves_like 'failing hardening method' end end @@ -155,14 +185,24 @@ RSpec.describe Gitlab::Utils::UsageData do stub_const("Gitlab::Utils::UsageData::DISTRIBUTED_HLL_FALLBACK", 4) end - it 'returns fallback if counter raises WRONG_CONFIGURATION_ERROR' do - expect(described_class.estimate_batch_distinct_count(relation, 'id', start: 1, finish: 0)).to eq 3 + context 'when counter raises WRONG_CONFIGURATION_ERROR' do + subject { described_class.estimate_batch_distinct_count(relation, 'id', start: 1, finish: 0) } + + let(:fallback) { 3 } + let(:failing_class) { Gitlab::Database::PostgresHll::BatchDistinctCounter } + let(:failing_method) { :new } + + it_behaves_like 'failing hardening method' end - it 'returns default fallback value when counting fails due to database error' do - allow(Gitlab::Database::PostgresHll::BatchDistinctCounter).to receive(:new).and_raise(ActiveRecord::StatementInvalid.new('')) + context 'when counting fails due to database error' do + subject { described_class.estimate_batch_distinct_count(relation) } - expect(described_class.estimate_batch_distinct_count(relation)).to eq(3) + let(:fallback) { 3 } + let(:failing_class) { Gitlab::Database::PostgresHll::BatchDistinctCounter } + let(:failing_method) { :new } + + it_behaves_like 'failing hardening method' end it 'logs error and returns DISTRIBUTED_HLL_FALLBACK value when counting raises any error', :aggregate_failures do @@ -187,13 +227,14 @@ RSpec.describe Gitlab::Utils::UsageData do expect(described_class.sum(relation, :column, batch_size: 100, start: 2, finish: 3)).to eq(1) end - it 'returns the fallback value when counting fails' do - stub_const("Gitlab::Utils::UsageData::FALLBACK", 15) - allow(Gitlab::Database::BatchCount) - .to receive(:batch_sum) - .and_raise(ActiveRecord::StatementInvalid.new('')) + context 'when counting fails' do + subject { described_class.sum(relation, :column) } + + let(:fallback) { 15 } + let(:failing_class) { Gitlab::Database::BatchCount } + let(:failing_method) { :batch_sum } - expect(described_class.sum(relation, :column)).to eq(15) + it_behaves_like 'failing hardening method' end end @@ -273,23 +314,45 @@ RSpec.describe Gitlab::Utils::UsageData do expect(histogram).to eq('2' => 1) end - it 'returns fallback and logs canceled queries' do - create(:alert_management_http_integration, :active, project: project1) + context 'when query timeout' do + subject do + with_statement_timeout(0.001) do + relation = AlertManagement::HttpIntegration.select('pg_sleep(0.002)') + described_class.histogram(relation, column, buckets: 1..100) + end + end - expect(Gitlab::AppJsonLogger).to receive(:error).with( - event: 'histogram', - relation: relation.table_name, - operation: 'histogram', - operation_args: [column, 1, 100, 99], - query: kind_of(String), - message: /PG::QueryCanceled/ - ) + before do + allow(Gitlab::ErrorTracking).to receive(:should_raise_for_dev?).and_return(should_raise_for_dev) + create(:alert_management_http_integration, :active, project: project1) + end + + context 'with should_raise_for_dev? false' do + let(:should_raise_for_dev) { false } + + it 'logs canceled queries' do + expect(Gitlab::AppJsonLogger).to receive(:error).with( + event: 'histogram', + relation: relation.table_name, + operation: 'histogram', + operation_args: [column, 1, 100, 99], + query: kind_of(String), + message: /PG::QueryCanceled/ + ) + subject + end - with_statement_timeout(0.001) do - relation = AlertManagement::HttpIntegration.select('pg_sleep(0.002)') - histogram = described_class.histogram(relation, column, buckets: 1..100) + it 'returns fallback' do + expect(subject).to eq(fallback) + end + end - expect(histogram).to eq(fallback) + context 'with should_raise_for_dev? true' do + let(:should_raise_for_dev) { true } + + it 'raises error' do + expect { subject }.to raise_error(ActiveRecord::QueryCanceled) + end end end end diff --git a/spec/mailers/emails/in_product_marketing_spec.rb b/spec/mailers/emails/in_product_marketing_spec.rb index a811b345c62..f6f8c187e80 100644 --- a/spec/mailers/emails/in_product_marketing_spec.rb +++ b/spec/mailers/emails/in_product_marketing_spec.rb @@ -5,29 +5,11 @@ require 'email_spec' RSpec.describe Emails::InProductMarketing do include EmailSpec::Matchers + include Gitlab::Routing.url_helpers let_it_be(:user) { create(:user) } - let_it_be(:group) { create(:group) } - - let!(:onboarding_progress) { create(:onboarding_progress, namespace: group) } - - describe '#in_product_marketing_email' do - using RSpec::Parameterized::TableSyntax - - let(:track) { :create } - let(:series) { 0 } - - subject { Notify.in_product_marketing_email(user.id, group.id, track, series) } - - include_context 'gitlab email notification' - - it 'sends to the right user with a link to unsubscribe' do - aggregate_failures do - expect(subject).to deliver_to(user.notification_email_or_default) - expect(subject).to have_body_text(profile_notifications_url) - end - end + shared_examples 'has custom headers when on gitlab.com' do context 'when on gitlab.com' do before do allow(Gitlab).to receive(:com?).and_return(true) @@ -45,6 +27,30 @@ RSpec.describe Emails::InProductMarketing do end end end + end + + describe '#in_product_marketing_email' do + let_it_be(:group) { create(:group) } + + let!(:onboarding_progress) { create(:onboarding_progress, namespace: group) } + + using RSpec::Parameterized::TableSyntax + + let(:track) { :create } + let(:series) { 0 } + + subject { Notify.in_product_marketing_email(user.id, group.id, track, series) } + + include_context 'gitlab email notification' + + it_behaves_like 'has custom headers when on gitlab.com' + + it 'sends to the right user with a link to unsubscribe' do + aggregate_failures do + expect(subject).to deliver_to(user.notification_email_or_default) + expect(subject).to have_body_text(profile_notifications_url) + end + end where(:track, :series) do :create | 0 @@ -102,4 +108,35 @@ RSpec.describe Emails::InProductMarketing do end end end + + describe '#account_validation_email' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project) { create(:project, :repository, namespace: namespace) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + + subject { Notify.account_validation_email(pipeline, user.notification_email_or_default) } + + it 'sends to the right user with a link to unsubscribe' do + expect(subject).to deliver_to(user.notification_email_or_default) + end + + it_behaves_like 'has custom headers when on gitlab.com' + + it 'has the correct subject and content' do + message = Gitlab::Email::Message::AccountValidation.new(pipeline) + cta_url = project_pipeline_url(pipeline.project, pipeline) + cta2_url = 'https://docs.gitlab.com/runner/install/' + + aggregate_failures do + is_expected.to have_subject(message.subject_line) + is_expected.to have_body_text(message.title) + is_expected.to have_body_text(message.body_line1) + is_expected.to have_body_text(CGI.unescapeHTML(message.body_line2)) + is_expected.to have_body_text(CGI.unescapeHTML(message.cta_link)) + is_expected.to have_body_text(CGI.unescapeHTML(message.cta2_link)) + is_expected.to have_body_text(cta_url) + is_expected.to have_body_text(cta2_url) + end + end + end end |