Merge branch 'sy-remove-alerts-service' into 'master'

Remove references to alerts service

See merge request gitlab-org/gitlab!50913

app/models/project.rb

@@ -147,7 +147,6 @@ class Project < ApplicationRecord
   has_many :boards
 
   # Project services
-  has_one :alerts_service
   has_one :campfire_service
   has_one :datadog_service
   has_one :discord_service
@@ -1357,9 +1356,9 @@ class Project < ApplicationRecord
   end
 
   def disabled_services
-    return ['datadog'] unless Feature.enabled?(:datadog_ci_integration, self)
+    return %w(datadog alerts) unless Feature.enabled?(:datadog_ci_integration, self)
 
-    []
+    %w(alerts)
   end
 
   def find_or_initialize_service(name)

app/models/project_services/alerts_service.rb

 # frozen_string_literal: true
 
-require 'securerandom'
-
+# This service is scheduled for removal. All records must
+# be deleted before the class can be removed.
+# https://gitlab.com/groups/gitlab-org/-/epics/5056
 class AlertsService < Service
-  has_one :data, class_name: 'AlertsServiceData', autosave: true,
-    inverse_of: :service, foreign_key: :service_id
-
-  attribute :token, :string
-  delegate :token, :token=, :token_changed?, :token_was, to: :data
-
-  validates :token, presence: true, if: :activated?
-
-  before_validation :prevent_token_assignment
-  before_validation :ensure_token, if: :activated?
-
-  after_save :update_http_integration
-
-  def url
-    return if instance? || template?
-
-    url_helpers.project_alerts_notify_url(project, format: :json)
-  end
-
-  def json_fields
-    super + %w(token)
-  end
-
-  def editable?
-    false
-  end
-
-  def show_active_box?
-    false
-  end
-
-  def can_test?
-    false
-  end
-
-  def title
-    _('Alerts endpoint')
-  end
-
-  def description
-    _('Authorize external services to send alerts to GitLab')
-  end
-
-  def detailed_description
-    description
-  end
+  before_save :prevent_save
 
   def self.to_param
     'alerts'
@@ -58,33 +14,15 @@ class AlertsService < Service
     %w()
   end
 
-  def data
-    super || build_data
-  end
-
   private
 
-  def prevent_token_assignment
-    self.token = token_was if token.present? && token_changed?
-  end
-
-  def ensure_token
-    self.token = generate_token if token.blank?
-  end
-
-  def generate_token
-    SecureRandom.hex
-  end
-
-  def url_helpers
-    Gitlab::Routing.url_helpers
-  end
-
-  def update_http_integration
-    return unless project_id && type == 'AlertsService'
+  def prevent_save
+    errors.add(:base, _('Alerts endpoint is deprecated and should not be created or modified. Use HTTP Integrations instead.'))
+    log_error('Prevented attempt to save or update deprecated AlertsService')
 
-    AlertManagement::SyncAlertServiceDataService # rubocop: disable CodeReuse/ServiceClass
-      .new(self)
-      .execute
+    # Stops execution of callbacks and database operation while
+    # preserving expectations of #save (will not raise) & #save! (raises)
+    # https://guides.rubyonrails.org/active_record_callbacks.html#halting-execution
+    throw :abort # rubocop:disable Cop/BanCatchThrow
   end
 end

app/models/project_services/alerts_service_data.rb

-# frozen_string_literal: true
-
-require 'securerandom'
-
-class AlertsServiceData < ApplicationRecord
-  belongs_to :service, class_name: 'AlertsService'
-
-  validates :service, presence: true
-
-  attr_encrypted :token,
-    mode: :per_attribute_iv,
-    key: Settings.attr_encrypted_db_key_base_truncated,
-    algorithm: 'aes-256-gcm'
-
-  def token_changed?
-    attribute_changed?(:token)
-  end
-end

app/models/service.rb

@@ -19,7 +19,6 @@ class Service < ApplicationRecord
 
   PROJECT_SPECIFIC_SERVICE_NAMES = %w[
     jenkins
-    alerts
   ].freeze
 
   # Fake services to help with local development.

app/services/alert_management/sync_alert_service_data_service.rb

-# frozen_string_literal: true
-
-module AlertManagement
-  class SyncAlertServiceDataService
-    # @param alert_service [AlertsService]
-    def initialize(alert_service)
-      @alert_service = alert_service
-    end
-
-    def execute
-      http_integration = find_http_integration
-
-      result = if http_integration
-                 update_integration_data(http_integration)
-               else
-                 create_integration
-               end
-
-      result ? ServiceResponse.success : ServiceResponse.error(message: 'Update failed')
-    end
-
-    private
-
-    attr_reader :alert_service
-
-    def find_http_integration
-      AlertManagement::HttpIntegrationsFinder.new(
-        alert_service.project,
-        endpoint_identifier: ::AlertManagement::HttpIntegration::LEGACY_IDENTIFIER
-      )
-      .execute
-      .first
-    end
-
-    def create_integration
-      new_integration = AlertManagement::HttpIntegration.create(
-        project_id: alert_service.project_id,
-        name: 'HTTP endpoint',
-        endpoint_identifier: AlertManagement::HttpIntegration::LEGACY_IDENTIFIER,
-        active: alert_service.active,
-        encrypted_token: alert_service.data.encrypted_token,
-        encrypted_token_iv: alert_service.data.encrypted_token_iv
-      )
-
-      new_integration.persisted?
-    end
-
-    def update_integration_data(http_integration)
-      http_integration.update(
-        active: alert_service.active,
-        encrypted_token: alert_service.data.encrypted_token,
-        encrypted_token_iv: alert_service.data.encrypted_token_iv
-      )
-    end
-  end
-end

app/views/projects/services/alerts/_help.html.haml

-.js-alerts-service-settings{ data: alerts_settings_data(disabled: true) }

app/views/projects/services/alerts/_top.html.haml

-.row
-  .col-lg-12
-    .gl-alert.gl-alert-info{ role: 'alert' }
-      = sprite_icon('information-o', css_class: 'gl-icon gl-alert-icon gl-alert-icon-no-title')
-      .gl-alert-body
-        = _('You can now manage alert endpoint configuration in the Alerts section on the Operations settings page. Fields on this page have been deprecated.')
-      .gl-alert-actions
-        = link_to _('Visit settings page'), project_settings_operations_path(@project, anchor: 'js-alert-management-settings'), class: 'btn gl-alert-action btn-info new-gl-button'

changelogs/unreleased/sy-remove-alerts-service.yml

+---
+title: Remove deprecated generic alert integration in favor of HTTP Integrations
+merge_request: 50913
+author:
+type: removed

db/post_migrate/20210107194543_remove_alerts_service_records.rb

+# frozen_string_literal: true
+
+class RemoveAlertsServiceRecords < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class Service < ActiveRecord::Base
+    self.table_name = 'services'
+  end
+
+  def up
+    Service.delete_by(type: 'AlertsService')
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20210107194543

+d72cf1c88a060ccadd9f90cbef5ae7d4ea6a4416a6263d11a870e01b02d1f935
\ No newline at end of file

doc/api/graphql/reference/gitlab_schema.graphql

@@ -22981,7 +22981,6 @@ type ServiceEdge {
 }
 
 enum ServiceType {
-  ALERTS_SERVICE
   ASANA_SERVICE
   ASSEMBLA_SERVICE
   BAMBOO_SERVICE

doc/api/graphql/reference/gitlab_schema.json

@@ -66648,12 +66648,6 @@
           "inputFields": null,
           "interfaces": null,
           "enumValues": [
-            {
-              "name": "ALERTS_SERVICE",
-              "description": null,
-              "isDeprecated": false,
-              "deprecationReason": null
-            },
             {
               "name": "ASANA_SERVICE",
               "description": null,

doc/api/graphql/reference/index.md

@@ -4921,7 +4921,6 @@ State of a Sentry error.
 
 | Value | Description |
 | ----- | ----------- |
-| `ALERTS_SERVICE` |  |
 | `ASANA_SERVICE` |  |
 | `ASSEMBLA_SERVICE` |  |
 | `BAMBOO_SERVICE` |  |

ee/app/helpers/ee/operations_helper.rb

@@ -47,12 +47,6 @@ module EE
 
     private
 
-    def alerts_service
-      strong_memoize(:alerts_service) do
-        @project.find_or_initialize_service(::AlertsService.to_param)
-      end
-    end
-
     def incident_sla_data
       setting = project_incident_management_setting
 

lib/api/helpers/services_helpers.rb

@@ -161,7 +161,6 @@ module API
 
       def self.services
         {
-          'alerts' => [],
           'asana' => [
             {
               required: true,
@@ -807,7 +806,6 @@ module API
 
       def self.service_classes
         [
-          ::AlertsService,
           ::AsanaService,
           ::AssemblaService,
           ::BambooService,

locale/gitlab.pot

@@ -2760,7 +2760,7 @@ msgstr ""
 msgid "Alerts"
 msgstr ""
 
-msgid "Alerts endpoint"
+msgid "Alerts endpoint is deprecated and should not be created or modified. Use HTTP Integrations instead."
 msgstr ""
 
 msgid "AlertsIntegrations|Alerts will be created through this integration"
@@ -4151,9 +4151,6 @@ msgstr ""
 msgid "Authorize %{user} to use your account?"
 msgstr ""
 
-msgid "Authorize external services to send alerts to GitLab"
-msgstr ""
-
 msgid "Authorized %{new_chat_name}"
 msgstr ""
 
@@ -32306,9 +32303,6 @@ msgstr ""
 msgid "You can now export your security dashboard to a CSV report."
 msgstr ""
 
-msgid "You can now manage alert endpoint configuration in the Alerts section on the Operations settings page. Fields on this page have been deprecated."
-msgstr ""
-
 msgid "You can now submit a merge request to get this change into the original branch."
 msgstr ""
 

spec/factories/alerts_service_data.rb

-# frozen_string_literal: true
-
-FactoryBot.define do
-  factory :alerts_service_data do
-    service { association(:alerts_service) }
-    token { SecureRandom.hex }
-  end
-end

spec/factories/services.rb

@@ -38,24 +38,6 @@ FactoryBot.define do
     end
   end
 
-  factory :alerts_service do
-    active
-    project
-    type { 'AlertsService' }
-
-    trait :active do
-      active { true }
-    end
-
-    trait :inactive do
-      active { false }
-    end
-
-    before(:create) do |service|
-      service.data = build(:alerts_service_data, service: service)
-    end
-  end
-
   factory :drone_ci_service do
     project
     active { true }

spec/factories/usage_data.rb

@@ -32,8 +32,8 @@ FactoryBot.define do
       create(:service, project: projects[2], type: 'CustomIssueTrackerService', active: true)
       create(:project_error_tracking_setting, project: projects[0])
       create(:project_error_tracking_setting, project: projects[1], enabled: false)
-      create(:alerts_service, project: projects[0])
-      create(:alerts_service, :inactive, project: projects[1])
+      create(:service, project: projects[0], type: 'AlertsService', active: true)
+      create(:service, project: projects[1], type: 'AlertsService', active: false)
       alert_bot_issues = create_list(:incident, 2, project: projects[0], author: User.alert_bot)
       create_list(:incident, 2, project: projects[1], author: User.alert_bot)
       issues = create_list(:issue, 4, project: projects[0])

spec/features/projects/services/user_activates_alerts_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe 'User activates Alerts', :js do
-  let_it_be(:project) { create(:project) }
-  let_it_be(:user) { create(:user) }
-
-  let(:service_name) { 'alerts' }
-  let(:service_title) { 'Alerts endpoint' }
-
-  before do
-    sign_in(user)
-    project.add_maintainer(user)
-  end
-
-  context 'when service is deactivated' do
-    it 'user cannot activate service' do
-      visit_project_services
-
-      expect(page).to have_link(service_title)
-      click_link(service_title)
-
-      expect(page).to have_callout_message
-      expect(page).to have_toggle_active_disabled
-    end
-  end
-
-  context 'when service is activated' do
-    let_it_be(:activated_alerts_service) do
-      create(:alerts_service, :active, project: project)
-    end
-
-    before do
-      visit_alerts_service
-    end
-
-    it 'user cannot change settings' do
-      expect(page).to have_callout_message
-      expect(page).to have_toggle_active_disabled
-      expect(page).to have_button_reset_key_disabled
-    end
-  end
-
-  private
-
-  def visit_project_services
-    visit(project_settings_integrations_path(project))
-  end
-
-  def visit_alerts_service
-    visit(edit_project_service_path(project, service_name))
-  end
-
-  def have_callout_message
-    within('.gl-alert') do
-      have_content('You can now manage alert endpoint configuration in the Alerts section on the Operations settings page.')
-    end
-  end
-
-  def have_toggle_active_disabled
-    have_selector('#activated .project-feature-toggle.is-disabled')
-  end
-
-  def have_button_reset_key_disabled
-    have_button('Reset key', disabled: true)
-  end
-end

spec/lib/gitlab/import_export/all_models.yml

@@ -525,7 +525,6 @@ project:
 - designs
 - project_aliases
 - external_pull_requests
-- alerts_service
 - grafana_integration
 - remove_source_branch_after_merge
 - deleting_user

spec/migrations/remove_alerts_service_records_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20210107194543_remove_alerts_service_records.rb')
+
+RSpec.describe RemoveAlertsServiceRecords do
+  let(:services) { table(:services) }
+  let(:alerts_service_data) { table(:alerts_service_data) }
+
+  before do
+    5.times do
+      service = services.create!(type: 'AlertsService')
+      alerts_service_data.create!(service_id: service.id)
+    end
+
+    services.create!(type: 'SomeOtherType')
+  end
+
+  it 'removes services records of type AlertsService and corresponding data', :aggregate_failures do
+    expect(services.count).to eq(6)
+    expect(alerts_service_data.count).to eq(5)
+
+    migrate!
+
+    expect(services.count).to eq(1)
+    expect(services.first.type).to eq('SomeOtherType')
+    expect(services.where(type: 'AlertsService')).to be_empty
+    expect(alerts_service_data.all).to be_empty
+  end
+end

spec/models/project_services/alerts_service_spec.rb

@@ -2,108 +2,38 @@
 
 require 'spec_helper'
 
+# AlertsService is stripped down to only required methods
+# to avoid errors loading integration-related pages if
+# records are present.
 RSpec.describe AlertsService do
   let_it_be(:project) { create(:project) }
-  let(:service_params) { { project: project, active: active } }
-  let(:active) { true }
-  let(:service) { described_class.new(service_params) }
+  subject(:service) { described_class.new(project: project) }
 
-  shared_context 'when active' do
-    let(:active) { true }
-  end
-
-  shared_context 'when inactive' do
-    let(:active) { false }
-  end
-
-  shared_context 'when persisted' do
-    before do
-      service.save!
-      service.reload
-    end
-  end
-
-  describe '#url' do
-    include Gitlab::Routing
-
-    subject { service.url }
+  it { is_expected.to be_valid }
 
-    it { is_expected.to eq(project_alerts_notify_url(project, format: :json)) }
-  end
-
-  describe '#json_fields' do
-    subject { service.json_fields }
+  describe '#to_param' do
+    subject { service.to_param }
 
-    it { is_expected.to eq(%w(active token)) }
+    it { is_expected.to eq('alerts') }
   end
 
-  describe '#as_json' do
-    subject { service.as_json(only: service.json_fields) }
+  describe '#supported_events' do
+    subject { service.supported_events }
 
-    it { is_expected.to eq('active' => true, 'token' => nil) }
+    it { is_expected.to be_empty }
   end
 
-  describe '#token' do
-    shared_context 'reset token' do
-      before do
-        service.token = ''
-        service.valid?
-      end
-    end
-
-    shared_context 'assign token' do |token|
-      before do
-        service.token = token
-        service.valid?
-      end
-    end
-
-    shared_examples 'valid token' do
-      it { is_expected.to match(/\A\h{32}\z/) }
-    end
-
-    shared_examples 'no token' do
-      it { is_expected.to be_blank }
-    end
-
-    subject { service.token }
-
-    context 'when active' do
-      include_context 'when active'
-
-      context 'when resetting' do
-        let!(:previous_token) { service.token }
-
-        include_context 'reset token'
-
-        it_behaves_like 'valid token'
-
-        it { is_expected.not_to eq(previous_token) }
-      end
-
-      context 'when assigning' do
-        include_context 'assign token', 'random token'
-
-        it_behaves_like 'valid token'
-      end
-    end
-
-    context 'when inactive' do
-      include_context 'when inactive'
-
-      context 'when resetting' do
-        let!(:previous_token) { service.token }
-
-        include_context 'reset token'
-
-        it_behaves_like 'no token'
-      end
-    end
+  describe '#save' do
+    it 'prevents records from being created or updated' do
+      expect(Gitlab::ProjectServiceLogger).to receive(:error).with(
+        hash_including(message: 'Prevented attempt to save or update deprecated AlertsService')
+      )
 
-    context 'when persisted' do
-      include_context 'when persisted'
+      expect(service.save).to be_falsey
 
-      it_behaves_like 'valid token'
+      expect(service.errors.full_messages).to include(
+        'Alerts endpoint is deprecated and should not be created or modified. Use HTTP Integrations instead.'
+      )
     end
   end
 end

spec/services/alert_management/sync_alert_service_data_service_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe AlertManagement::SyncAlertServiceDataService do
-  let_it_be(:alerts_service) do
-    AlertsService.skip_callback(:save, :after, :update_http_integration)
-    service = create(:alerts_service, :active)
-    AlertsService.set_callback(:save, :after, :update_http_integration)
-
-    service
-  end
-
-  describe '#execute' do
-    subject(:execute) { described_class.new(alerts_service).execute }
-
-    context 'without http integration' do
-      it 'creates the integration' do
-        expect { execute }
-          .to change { AlertManagement::HttpIntegration.count }.by(1)
-      end
-
-      it 'returns a success' do
-        expect(subject.success?).to eq(true)
-      end
-    end
-
-    context 'existing legacy http integration' do
-      let_it_be(:integration) { create(:alert_management_http_integration, :legacy, project: alerts_service.project) }
-
-      it 'updates the integration' do
-        expect { execute }
-          .to change { integration.reload.encrypted_token }.to(alerts_service.data.encrypted_token)
-          .and change { integration.encrypted_token_iv }.to(alerts_service.data.encrypted_token_iv)
-      end
-
-      it 'returns a success' do
-        expect(subject.success?).to eq(true)
-      end
-    end
-
-    context 'existing other http integration' do
-      let_it_be(:integration) { create(:alert_management_http_integration, project: alerts_service.project) }
-
-      it 'creates the integration' do
-        expect { execute }
-          .to change { AlertManagement::HttpIntegration.count }.by(1)
-      end
-
-      it 'returns a success' do
-        expect(subject.success?).to eq(true)
-      end
-    end
-  end
-end