Merge branch 'add-abuse-notification-email' into 'master'

Set abuse_notification_email instead of admin_notification_email

See merge request gitlab-org/gitlab!41319

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

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

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
 

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.
 

changelogs/unreleased/add-abuse-notification-email.yml

+---
+title: Set abuse_notification_email instead of admin_notification_email.
+merge_request: 41319
+author: Hiromi Nozawa
+type: deprecated

db/migrate/20200912152943_rename_admin_notification_email_application_setting.rb

+# 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

db/post_migrate/20200912153218_cleanup_admin_notification_email_application_setting_rename.rb

+# 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

db/schema_migrations/20200912152943

+b8d7a2ec9ecf51fd7cb9346e1484b45d5b472a85d90ad270d08c1cca1b44f039
\ No newline at end of file

db/schema_migrations/20200912153218

+fd632247f1588c537e83574edd7936c530e154091e3101d0404da3b7ef8b4bef
\ No newline at end of file

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)),

doc/api/settings.md

@@ -190,8 +190,9 @@ require other settings to be set in order to function properly. These requiremen
 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. |
+| --------- | ---- | :------: | ----------- |
+| `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. |

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

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)
 

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 }

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
 

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 }