Automatic merge of gitlab-org/gitlab master

app/models/project_services/alerts_service.rb

@@ -14,6 +14,8 @@ class AlertsService < Service
   before_validation :prevent_token_assignment
   before_validation :ensure_token, if: :activated?
 
+  after_save :update_http_integration
+
   def url
     return if instance? || template?
 
@@ -77,6 +79,14 @@ class AlertsService < Service
   def url_helpers
     Gitlab::Routing.url_helpers
   end
+
+  def update_http_integration
+    return unless project_id && type == 'AlertsService'
+
+    AlertManagement::SyncAlertServiceDataService # rubocop: disable CodeReuse/ServiceClass
+      .new(self)
+      .execute
+  end
 end
 
 AlertsService.prepend_if_ee('EE::AlertsService')

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

changelogs/unreleased/255519-migrate-existing-generic-alert-endpoints-to-http-integrations-form.yml

+---
+title: Migrate Alert Service to HTTP Integrations model
+merge_request: 46188
+author:
+type: added

db/migrate/20201027002551_migrate_services_to_http_integrations.rb

+# frozen_string_literal: true
+
+class MigrateServicesToHttpIntegrations < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  ALERT_SERVICE_TYPE = 'AlertsService'
+  SERVICE_NAMES_IDENTIFIER = {
+    name: 'HTTP endpoint',
+    identifier: 'legacy'
+  }
+
+  class HttpIntegration < ActiveRecord::Base
+    self.table_name = 'alert_management_http_integrations'
+  end
+
+  # For each Alerts service,
+  # Create the matching HttpIntegration
+  def up
+    HttpIntegration.reset_column_information
+
+    sql = <<~SQL
+      SELECT * FROM services
+      JOIN alerts_service_data
+      ON (services.id = alerts_service_data.service_id)
+      WHERE type = '#{ALERT_SERVICE_TYPE}'
+      AND active = TRUE
+    SQL
+
+    current_time = Time.current
+
+    values = select_all(sql).map do |alerts_service|
+      {
+        project_id: alerts_service['project_id'],
+        name: SERVICE_NAMES_IDENTIFIER[:name],
+        endpoint_identifier: SERVICE_NAMES_IDENTIFIER[:identifier],
+        encrypted_token: alerts_service['encrypted_token'],
+        encrypted_token_iv: alerts_service['encrypted_token_iv'],
+        active: alerts_service['active'],
+        updated_at: current_time,
+        created_at: current_time
+      }
+    end
+
+    HttpIntegration.insert_all(values) if values.present?
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20201027002551

+34e357b739235d627e543e1bc7202e0cf26f5b1ca8021bb25357e29e883f785b
\ No newline at end of file

ee/app/models/ee/application_setting.rb

@@ -95,11 +95,15 @@ module EE
       validate :allowed_frameworks, if: :compliance_frameworks_changed?
 
       validates :new_user_signups_cap,
-                numericality: { only_integer: true,
-                                allow_nil: true,
-                                greater_than: 0 }
+                allow_blank: true,
+                numericality: { only_integer: true, greater_than: 0 }
       validates :new_user_signups_cap,
-                numericality: { less_than_or_equal_to: proc { License.current&.restricted_user_count } },
+                allow_blank: true,
+                numericality: {
+                  only_integer: true,
+                  greater_than: 0,
+                  less_than_or_equal_to: proc { License.current&.restricted_user_count }
+                },
                 if: proc { License.current&.restricted_user_count? }
 
       after_commit :update_personal_access_tokens_lifetime, if: :saved_change_to_max_personal_access_token_lifetime?

ee/spec/models/application_setting_spec.rb

@@ -88,6 +88,8 @@ RSpec.describe ApplicationSetting do
     it { is_expected.to allow_value(nil).for(:new_user_signups_cap) }
     it { is_expected.to allow_value(1).for(:new_user_signups_cap) }
     it { is_expected.to allow_value(10).for(:new_user_signups_cap) }
+    it { is_expected.to allow_value("").for(:new_user_signups_cap) }
+    it { is_expected.not_to allow_value("value").for(:new_user_signups_cap) }
     it { is_expected.not_to allow_value(-1).for(:new_user_signups_cap) }
     it { is_expected.not_to allow_value(2.5).for(:new_user_signups_cap) }
 
@@ -175,6 +177,7 @@ RSpec.describe ApplicationSetting do
 
       it { is_expected.to allow_value(max_active_user_count - 1).for(:new_user_signups_cap) }
       it { is_expected.to allow_value(max_active_user_count).for(:new_user_signups_cap) }
+      it { is_expected.to allow_value(nil).for(:new_user_signups_cap) }
       it { is_expected.not_to allow_value(max_active_user_count + 1).for(:new_user_signups_cap) }
     end
   end

qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb

@@ -19,7 +19,7 @@ module QA
         end
       end
 
-      it 'closes an issue', quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/225303', type: :bug }, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/852' do
+      it 'closes an issue', quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/225303', type: :bug }, testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1085' do
         closed_issue.visit!
 
         Page::Project::Issue::Show.perform do |issue_page|

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

@@ -56,6 +56,10 @@ FactoryBot.define do
     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

spec/migrations/20201027002551_migrate_services_to_http_integrations_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20201027002551_migrate_services_to_http_integrations.rb')
+
+RSpec.describe MigrateServicesToHttpIntegrations do
+  let!(:namespace) { table(:namespaces).create!(name: 'namespace', path: 'namespace') }
+  let!(:project) { table(:projects).create!(id: 1, namespace_id: namespace.id) }
+  let!(:alert_service) { table(:services).create!(type: 'AlertsService', project_id: project.id, active: true) }
+  let!(:alert_service_data) { table(:alerts_service_data).create!(service_id: alert_service.id, encrypted_token: 'test', encrypted_token_iv: 'test')}
+  let(:http_integrations) { table(:alert_management_http_integrations) }
+
+  describe '#up' do
+    it 'creates the http integrations from the alert services', :aggregate_failures do
+      expect { migrate! }.to change { http_integrations.count }.by(1)
+
+      http_integration = http_integrations.last
+      expect(http_integration.project_id).to eq(alert_service.project_id)
+      expect(http_integration.encrypted_token).to eq(alert_service_data.encrypted_token)
+      expect(http_integration.encrypted_token_iv).to eq(alert_service_data.encrypted_token_iv)
+      expect(http_integration.active).to eq(alert_service.active)
+      expect(http_integration.name).to eq(described_class::SERVICE_NAMES_IDENTIFIER[:name])
+      expect(http_integration.endpoint_identifier).to eq(described_class::SERVICE_NAMES_IDENTIFIER[:identifier])
+    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