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

Cleanup remaining alerts service code

See merge request gitlab-org/gitlab!53534

app/models/project.rb

@@ -1351,9 +1351,9 @@ class Project < ApplicationRecord
   end
 
   def disabled_services
-    return %w(datadog alerts) unless Feature.enabled?(:datadog_ci_integration, self)
+    return %w(datadog) 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
-
-# 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
-  before_save :prevent_save
-
-  def self.to_param
-    'alerts'
-  end
-
-  def self.supported_events
-    %w()
-  end
-
-  private
-
-  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')
-
-    # 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

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

+---
+title: Remove legacy alerts service data and table
+merge_request: 53534
+author:
+type: removed

db/migrate/20210205213915_remove_foreign_keys_from_alerts_service_data.rb

+# frozen_string_literal: true
+
+class RemoveForeignKeysFromAlertsServiceData < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      remove_foreign_key_if_exists :alerts_service_data, column: :service_id
+    end
+  end
+
+  def down
+    with_lock_retries do
+      add_foreign_key :alerts_service_data, :services, column: :service_id, on_delete: :cascade
+    end
+  end
+end

db/post_migrate/20210205213933_drop_alerts_service_data.rb

+# frozen_string_literal: true
+
+class DropAlertsServiceData < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      drop_table :alerts_service_data
+    end
+  end
+
+  # rubocop:disable Migration/PreventStrings
+  def down
+    with_lock_retries do
+      create_table :alerts_service_data do |t|
+        t.bigint :service_id, null: false
+        t.timestamps_with_timezone
+        t.string :encrypted_token, limit: 255
+        t.string :encrypted_token_iv, limit: 255
+      end
+    end
+  end
+  # rubocop:enable Migration/PreventStrings
+end

db/post_migrate/20210205214003_remove_alerts_service_records_again.rb

+# frozen_string_literal: true
+
+class RemoveAlertsServiceRecordsAgain < 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/20210205213915

+c7c2936062f4a7c764938453fb28dc2f461a06f0a21cc74b1750edbde9398fa1
\ No newline at end of file

db/schema_migrations/20210205213933

+caec7f6c66a0277561f650ae513fedaba581ab35bb238351eccccfef1132d118
\ No newline at end of file

db/schema_migrations/20210205214003

+6ca08c885fddccd3c82fc8651d20140655b65019e56f9c6136e92140401386d1
\ No newline at end of file

db/structure.sql

@@ -8885,24 +8885,6 @@ CREATE SEQUENCE alert_management_http_integrations_id_seq
 
 ALTER SEQUENCE alert_management_http_integrations_id_seq OWNED BY alert_management_http_integrations.id;
 
-CREATE TABLE alerts_service_data (
-    id bigint NOT NULL,
-    service_id integer NOT NULL,
-    created_at timestamp with time zone NOT NULL,
-    updated_at timestamp with time zone NOT NULL,
-    encrypted_token character varying(255),
-    encrypted_token_iv character varying(255)
-);
-
-CREATE SEQUENCE alerts_service_data_id_seq
-    START WITH 1
-    INCREMENT BY 1
-    NO MINVALUE
-    NO MAXVALUE
-    CACHE 1;
-
-ALTER SEQUENCE alerts_service_data_id_seq OWNED BY alerts_service_data.id;
-
 CREATE TABLE allowed_email_domains (
     id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
@@ -18635,8 +18617,6 @@ ALTER TABLE ONLY alert_management_alerts ALTER COLUMN id SET DEFAULT nextval('al
 
 ALTER TABLE ONLY alert_management_http_integrations ALTER COLUMN id SET DEFAULT nextval('alert_management_http_integrations_id_seq'::regclass);
 
-ALTER TABLE ONLY alerts_service_data ALTER COLUMN id SET DEFAULT nextval('alerts_service_data_id_seq'::regclass);
-
 ALTER TABLE ONLY allowed_email_domains ALTER COLUMN id SET DEFAULT nextval('allowed_email_domains_id_seq'::regclass);
 
 ALTER TABLE ONLY analytics_cycle_analytics_group_stages ALTER COLUMN id SET DEFAULT nextval('analytics_cycle_analytics_group_stages_id_seq'::regclass);
@@ -19661,9 +19641,6 @@ ALTER TABLE ONLY alert_management_alerts
 ALTER TABLE ONLY alert_management_http_integrations
     ADD CONSTRAINT alert_management_http_integrations_pkey PRIMARY KEY (id);
 
-ALTER TABLE ONLY alerts_service_data
-    ADD CONSTRAINT alerts_service_data_pkey PRIMARY KEY (id);
-
 ALTER TABLE ONLY allowed_email_domains
     ADD CONSTRAINT allowed_email_domains_pkey PRIMARY KEY (id);
 
@@ -21360,8 +21337,6 @@ CREATE UNIQUE INDEX index_alert_user_mentions_on_alert_id_and_note_id ON alert_m
 
 CREATE UNIQUE INDEX index_alert_user_mentions_on_note_id ON alert_management_alert_user_mentions USING btree (note_id) WHERE (note_id IS NOT NULL);
 
-CREATE INDEX index_alerts_service_data_on_service_id ON alerts_service_data USING btree (service_id);
-
 CREATE INDEX index_allowed_email_domains_on_group_id ON allowed_email_domains USING btree (group_id);
 
 CREATE INDEX index_analytics_ca_group_stages_on_end_event_label_id ON analytics_cycle_analytics_group_stages USING btree (end_event_label_id);
@@ -25813,9 +25788,6 @@ ALTER TABLE ONLY approval_project_rules_protected_branches
 ALTER TABLE ONLY packages_composer_cache_files
     ADD CONSTRAINT fk_rails_b82cea43a0 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE SET NULL;
 
-ALTER TABLE ONLY alerts_service_data
-    ADD CONSTRAINT fk_rails_b93215a42c FOREIGN KEY (service_id) REFERENCES services(id) ON DELETE CASCADE;
-
 ALTER TABLE ONLY merge_trains
     ADD CONSTRAINT fk_rails_b9d67af01d FOREIGN KEY (target_project_id) REFERENCES projects(id) ON DELETE CASCADE;
 

lib/gitlab/usage_data.rb

@@ -163,7 +163,7 @@ module Gitlab
             projects_with_repositories_enabled: count(ProjectFeature.where('repository_access_level > ?', ProjectFeature::DISABLED)),
             projects_with_tracing_enabled: count(ProjectTracingSetting),
             projects_with_error_tracking_enabled: count(::ErrorTracking::ProjectErrorTrackingSetting.where(enabled: true)),
-            projects_with_alerts_service_enabled: count(AlertsService.active),
+            projects_with_alerts_service_enabled: count(Service.active.where(type: 'AlertsService')),
             projects_with_alerts_created: distinct_count(::AlertManagement::Alert, :project_id),
             projects_with_enabled_alert_integrations: distinct_count(::AlertManagement::HttpIntegration.active, :project_id),
             projects_with_prometheus_alerts: distinct_count(PrometheusAlert, :project_id),

locale/gitlab.pot

@@ -2891,9 +2891,6 @@ msgstr ""
 msgid "Alerts"
 msgstr ""
 
-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"
 msgstr ""
 

spec/migrations/drop_alerts_service_data_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20210205213933_drop_alerts_service_data.rb')
+
+RSpec.describe DropAlertsServiceData do
+  let_it_be(:alerts_service_data) { table(:alerts_service_data) }
+
+  it 'correctly migrates up and down' do
+    reversible_migration do |migration|
+      migration.before -> {
+        expect(alerts_service_data.create!(service_id: 1)).to be_a alerts_service_data
+      }
+
+      migration.after -> {
+        expect { alerts_service_data.create!(service_id: 1) }
+          .to raise_error(ActiveRecord::StatementInvalid, /UndefinedTable/)
+      }
+    end
+  end
+end

spec/migrations/remove_alerts_service_records_again_spec.rb

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

spec/models/project_services/alerts_service_spec.rb

-# frozen_string_literal: true
-
-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) }
-  subject(:service) { described_class.new(project: project) }
-
-  it { is_expected.to be_valid }
-
-  describe '#to_param' do
-    subject { service.to_param }
-
-    it { is_expected.to eq('alerts') }
-  end
-
-  describe '#supported_events' do
-    subject { service.supported_events }
-
-    it { is_expected.to be_empty }
-  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')
-      )
-
-      expect(service.save).to be_falsey
-
-      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