From 0c85feba3d97838ae9776c277da9f63521e98df9 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sat, 26 Sep 2020 00:09:35 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/helpers/application_settings_helper.rb | 8 +++++++- app/mailers/abuse_report_mailer.rb | 4 ++-- app/models/application_setting.rb | 2 +- .../admin/application_settings/_abuse.html.haml | 4 ++-- app/views/import/shared/_errors.html.haml | 8 +++++--- ...n-app-views-import-shared-_errors-html-h.yml | 5 +++++ .../unreleased/add-abuse-notification-email.yml | 5 +++++ ...in_notification_email_application_setting.rb | 17 +++++++++++++++++ ...fication_email_application_setting_rename.rb | 17 +++++++++++++++++ db/schema_migrations/20200912152943 | 1 + db/schema_migrations/20200912153218 | 1 + db/structure.sql | 2 +- doc/api/settings.md | 7 ++++--- lib/api/helpers/settings_helpers.rb | 1 + lib/api/settings.rb | 8 +++++++- spec/mailers/abuse_report_mailer_spec.rb | 10 +++++----- spec/models/application_setting_spec.rb | 2 +- spec/requests/api/settings_spec.rb | 8 ++++++++ 18 files changed, 90 insertions(+), 20 deletions(-) create mode 100644 changelogs/unreleased/233703-replace-bootstrap-alerts-in-app-views-import-shared-_errors-html-h.yml create mode 100644 changelogs/unreleased/add-abuse-notification-email.yml create mode 100644 db/migrate/20200912152943_rename_admin_notification_email_application_setting.rb create mode 100644 db/post_migrate/20200912153218_cleanup_admin_notification_email_application_setting_rename.rb create mode 100644 db/schema_migrations/20200912152943 create mode 100644 db/schema_migrations/20200912153218 diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 9245cc1cb1c..585d13279dd 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -168,7 +168,7 @@ module ApplicationSettingsHelper def visible_attributes [ - :admin_notification_email, + :abuse_notification_email, :after_sign_out_path, :after_sign_up_text, :akismet_api_key, @@ -345,6 +345,12 @@ module ApplicationSettingsHelper ] end + def deprecated_attributes + [ + :admin_notification_email # ok to remove in REST API v5 + ] + end + def expanded_by_default? Rails.env.test? end diff --git a/app/mailers/abuse_report_mailer.rb b/app/mailers/abuse_report_mailer.rb index 0f2f63b43f5..20aabb6fe58 100644 --- a/app/mailers/abuse_report_mailer.rb +++ b/app/mailers/abuse_report_mailer.rb @@ -11,7 +11,7 @@ class AbuseReportMailer < ApplicationMailer @abuse_report = AbuseReport.find(abuse_report_id) mail( - to: Gitlab::CurrentSettings.admin_notification_email, + to: Gitlab::CurrentSettings.abuse_notification_email, subject: "#{@abuse_report.user.name} (#{@abuse_report.user.username}) was reported for abuse" ) end @@ -19,6 +19,6 @@ class AbuseReportMailer < ApplicationMailer private def deliverable? - Gitlab::CurrentSettings.admin_notification_email.present? + Gitlab::CurrentSettings.abuse_notification_email.present? end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index e9a3dcf39df..2d26d5655ca 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -95,7 +95,7 @@ class ApplicationSetting < ApplicationRecord allow_blank: true, addressable_url: true - validates :admin_notification_email, + validates :abuse_notification_email, devise_email: true, allow_blank: true diff --git a/app/views/admin/application_settings/_abuse.html.haml b/app/views/admin/application_settings/_abuse.html.haml index ddffec32c41..6ffeea81f28 100644 --- a/app/views/admin/application_settings/_abuse.html.haml +++ b/app/views/admin/application_settings/_abuse.html.haml @@ -3,8 +3,8 @@ %fieldset .form-group - = f.label :admin_notification_email, 'Abuse reports notification email', class: 'label-bold' - = f.text_field :admin_notification_email, class: 'form-control' + = f.label :abuse_notification_email, 'Abuse reports notification email', class: 'label-bold' + = f.text_field :abuse_notification_email, class: 'form-control' .form-text.text-muted Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area. diff --git a/app/views/import/shared/_errors.html.haml b/app/views/import/shared/_errors.html.haml index de60c15351f..32b4a39924b 100644 --- a/app/views/import/shared/_errors.html.haml +++ b/app/views/import/shared/_errors.html.haml @@ -1,4 +1,6 @@ - if @errors.present? - .alert.alert-danger - - @errors.each do |error| - = error + .gl-alert.gl-alert-danger.gl-mb-5 + = sprite_icon('error', size: 16, css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title') + .gl-alert-body + - @errors.each do |error| + = error diff --git a/changelogs/unreleased/233703-replace-bootstrap-alerts-in-app-views-import-shared-_errors-html-h.yml b/changelogs/unreleased/233703-replace-bootstrap-alerts-in-app-views-import-shared-_errors-html-h.yml new file mode 100644 index 00000000000..37ce9481f51 --- /dev/null +++ b/changelogs/unreleased/233703-replace-bootstrap-alerts-in-app-views-import-shared-_errors-html-h.yml @@ -0,0 +1,5 @@ +--- +title: Replace bootstrap alerts in app/views/import/shared/_errors.html.haml +merge_request: 41288 +author: Gilang Gumilar +type: changed diff --git a/changelogs/unreleased/add-abuse-notification-email.yml b/changelogs/unreleased/add-abuse-notification-email.yml new file mode 100644 index 00000000000..1d8312d9cf7 --- /dev/null +++ b/changelogs/unreleased/add-abuse-notification-email.yml @@ -0,0 +1,5 @@ +--- +title: Set abuse_notification_email instead of admin_notification_email. +merge_request: 41319 +author: Hiromi Nozawa +type: deprecated diff --git a/db/migrate/20200912152943_rename_admin_notification_email_application_setting.rb b/db/migrate/20200912152943_rename_admin_notification_email_application_setting.rb new file mode 100644 index 00000000000..b469099014d --- /dev/null +++ b/db/migrate/20200912152943_rename_admin_notification_email_application_setting.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class RenameAdminNotificationEmailApplicationSetting < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + rename_column_concurrently :application_settings, :admin_notification_email, :abuse_notification_email + end + + def down + undo_rename_column_concurrently :application_settings, :admin_notification_email, :abuse_notification_email + end +end diff --git a/db/post_migrate/20200912153218_cleanup_admin_notification_email_application_setting_rename.rb b/db/post_migrate/20200912153218_cleanup_admin_notification_email_application_setting_rename.rb new file mode 100644 index 00000000000..35c54b64ddf --- /dev/null +++ b/db/post_migrate/20200912153218_cleanup_admin_notification_email_application_setting_rename.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CleanupAdminNotificationEmailApplicationSettingRename < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :application_settings, :admin_notification_email, :abuse_notification_email + end + + def down + undo_cleanup_concurrent_column_rename :application_settings, :admin_notification_email, :abuse_notification_email + end +end diff --git a/db/schema_migrations/20200912152943 b/db/schema_migrations/20200912152943 new file mode 100644 index 00000000000..4dd1ae708cd --- /dev/null +++ b/db/schema_migrations/20200912152943 @@ -0,0 +1 @@ +b8d7a2ec9ecf51fd7cb9346e1484b45d5b472a85d90ad270d08c1cca1b44f039 \ No newline at end of file diff --git a/db/schema_migrations/20200912153218 b/db/schema_migrations/20200912153218 new file mode 100644 index 00000000000..9c013281ea2 --- /dev/null +++ b/db/schema_migrations/20200912153218 @@ -0,0 +1 @@ +fd632247f1588c537e83574edd7936c530e154091e3101d0404da3b7ef8b4bef \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 56c8a9c3834..770f072bb94 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9031,7 +9031,6 @@ CREATE TABLE application_settings ( session_expire_delay integer DEFAULT 10080 NOT NULL, import_sources text, help_page_text text, - admin_notification_email character varying, shared_runners_enabled boolean DEFAULT true NOT NULL, max_artifacts_size integer DEFAULT 100 NOT NULL, runners_registration_token character varying, @@ -9270,6 +9269,7 @@ CREATE TABLE application_settings ( elasticsearch_client_request_timeout integer DEFAULT 0 NOT NULL, gitpod_enabled boolean DEFAULT false NOT NULL, gitpod_url text DEFAULT 'https://gitpod.io/'::text, + abuse_notification_email character varying, CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)), CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)), CONSTRAINT check_9c6c447a13 CHECK ((char_length(maintenance_mode_message) <= 255)), diff --git a/doc/api/settings.md b/doc/api/settings.md index c1852374061..d1346cf1fa0 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -189,9 +189,10 @@ In general, all settings are optional. Certain settings though, if enabled, will require other settings to be set in order to function properly. These requirements are listed in the descriptions of the relevant settings. -| Attribute | Type | Required | Description | -|:-----------------------------------------|:-----------------|:-------------------------------------|:------------| -| `admin_notification_email` | string | no | [Abuse reports](../user/admin_area/abuse_reports.md) are sent to this address if set. Abuse reports are always available in the Admin Area. | +| Attribute | Type | Required | Description | +| --------- | ---- | :------: | ----------- | +| `admin_notification_email` | string | no | Deprecated: Use `abuse_notification_email` instead. If set, [abuse reports](../user/admin_area/abuse_reports.md) are sent to this address. Abuse reports are always available in the Admin Area. | +| `abuse_notification_email` | string | no | If set, [abuse reports](../user/admin_area/abuse_reports.md) are sent to this address. Abuse reports are always available in the Admin Area. | | `after_sign_out_path` | string | no | Where to redirect users after logout. | | `after_sign_up_text` | string | no | Text shown to the user after signing up | | `akismet_api_key` | string | required by: `akismet_enabled` | API key for Akismet spam protection. | diff --git a/lib/api/helpers/settings_helpers.rb b/lib/api/helpers/settings_helpers.rb index 65aec6ae2e7..451e578fdd6 100644 --- a/lib/api/helpers/settings_helpers.rb +++ b/lib/api/helpers/settings_helpers.rb @@ -12,6 +12,7 @@ module API def self.optional_attributes [*::ApplicationSettingsHelper.visible_attributes, *::ApplicationSettingsHelper.external_authorization_service_attributes, + *::ApplicationSettingsHelper.deprecated_attributes, :performance_bar_allowed_group_id].freeze end end diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 6e5534d0c9a..4056d8602f3 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -29,7 +29,8 @@ module API success Entities::ApplicationSetting end params do - optional :admin_notification_email, type: String, desc: 'Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.' + optional :admin_notification_email, type: String, desc: 'Deprecated: Use :abuse_notification_email instead. Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.' + optional :abuse_notification_email, type: String, desc: 'Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.' optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' optional :after_sign_out_path, type: String, desc: 'We will redirect users to this page after they sign out' optional :akismet_enabled, type: Boolean, desc: 'Helps prevent bots from creating issues' @@ -194,6 +195,11 @@ module API attrs[:allow_local_requests_from_web_hooks_and_services] = attrs.delete(:allow_local_requests_from_hooks_and_services) end + # support legacy names, can be removed in v5 + if attrs.has_key?(:admin_notification_email) + attrs[:abuse_notification_email] = attrs.delete(:admin_notification_email) + end + # since 13.0 it's not possible to disable hashed storage - support can be removed in 14.0 attrs.delete(:hashed_storage_enabled) if attrs.has_key?(:hashed_storage_enabled) diff --git a/spec/mailers/abuse_report_mailer_spec.rb b/spec/mailers/abuse_report_mailer_spec.rb index 4eb616722ac..061f972fd35 100644 --- a/spec/mailers/abuse_report_mailer_spec.rb +++ b/spec/mailers/abuse_report_mailer_spec.rb @@ -7,7 +7,7 @@ RSpec.describe AbuseReportMailer do describe '.notify' do before do - stub_application_setting(admin_notification_email: 'admin@example.com') + stub_application_setting(abuse_notification_email: 'admin@example.com') end let(:report) { create(:abuse_report) } @@ -17,8 +17,8 @@ RSpec.describe AbuseReportMailer do it_behaves_like 'appearance header and footer enabled' it_behaves_like 'appearance header and footer not enabled' - context 'with admin_notification_email set' do - it 'sends to the admin_notification_email' do + context 'with abuse_notification_email set' do + it 'sends to the abuse_notification_email' do is_expected.to deliver_to 'admin@example.com' end @@ -27,9 +27,9 @@ RSpec.describe AbuseReportMailer do end end - context 'with no admin_notification_email set' do + context 'with no abuse_notification_email set' do it 'returns early' do - stub_application_setting(admin_notification_email: nil) + stub_application_setting(abuse_notification_email: nil) expect { described_class.notify(spy).deliver_now } .not_to change { ActionMailer::Base.deliveries.count } diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 9f76fb3330d..f572e6ffade 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -320,7 +320,7 @@ RSpec.describe ApplicationSetting do end end - it_behaves_like 'an object with email-formated attributes', :admin_notification_email do + it_behaves_like 'an object with email-formated attributes', :abuse_notification_email do subject { setting } end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index ef12f6dbed3..b0face6ec41 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -413,6 +413,14 @@ RSpec.describe API::Settings, 'Settings' do end end + it 'supports legacy admin_notification_email' do + put api('/application/settings', admin), + params: { admin_notification_email: 'test@example.com' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['abuse_notification_email']).to eq('test@example.com') + end + context "missing sourcegraph_url value when sourcegraph_enabled is true" do it "returns a blank parameter error message" do put api("/application/settings", admin), params: { sourcegraph_enabled: true }