Merge branch '334127-integrations-change-wiki-trigger' into 'master'

Change external_wiki maintainance triggers

See merge request gitlab-org/gitlab!68172

db/post_migrate/20210813151908_replace_external_wiki_triggers.rb

+# frozen_string_literal: true
+
+class ReplaceExternalWikiTriggers < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::SchemaHelpers
+
+  def up
+    replace_triggers('type_new', 'Integrations::ExternalWiki')
+
+    # we need an extra trigger to handle when type_new is updated by the
+    # `integrations_set_type_new` trigger.
+    # This can be removed when this trigger has been removed.
+    execute(<<~SQL.squish)
+      CREATE TRIGGER #{trigger_name(:type_new_updated)}
+        AFTER UPDATE OF type_new ON integrations FOR EACH ROW
+        WHEN ((new.type_new)::text = 'Integrations::ExternalWiki'::text AND new.project_id IS NOT NULL)
+        EXECUTE FUNCTION set_has_external_wiki();
+    SQL
+  end
+
+  def down
+    execute("DROP TRIGGER IF EXISTS #{trigger_name(:type_new_updated)} ON integrations;")
+    replace_triggers('type', 'ExternalWikiService')
+  end
+
+  private
+
+  def replace_triggers(column_name, value)
+    triggers(column_name, value).each do |event, condition|
+      trigger = trigger_name(event)
+
+      # create duplicate trigger, using the defined condition
+      execute(<<~SQL.squish)
+      CREATE TRIGGER #{trigger}_new AFTER #{event.upcase} ON integrations FOR EACH ROW
+        WHEN (#{condition})
+        EXECUTE FUNCTION set_has_external_wiki();
+      SQL
+
+      # Swap the triggers in place, so that the new trigger has the canonical name
+      execute("ALTER TRIGGER #{trigger} ON integrations RENAME TO #{trigger}_old;")
+      execute("ALTER TRIGGER #{trigger}_new ON integrations RENAME TO #{trigger};")
+
+      # remove the old, now redundant trigger
+      execute("DROP TRIGGER IF EXISTS #{trigger}_old ON integrations;")
+    end
+  end
+
+  def trigger_name(event)
+    "trigger_has_external_wiki_on_#{event}"
+  end
+
+  def triggers(column_name, value)
+    {
+      delete: "#{matches_value('old', column_name, value)} AND #{project_not_null('old')}",
+      insert: "(new.active = true) AND #{matches_value('new', column_name, value)} AND #{project_not_null('new')}",
+      update: "#{matches_value('new', column_name, value)} AND (old.active <> new.active) AND #{project_not_null('new')}"
+    }
+  end
+
+  def project_not_null(row)
+    "(#{row}.project_id IS NOT NULL)"
+  end
+
+  def matches_value(row, column_name, value)
+    "((#{row}.#{column_name})::text = '#{value}'::text)"
+  end
+end

db/schema_migrations/20210813151908

+fdb6dd20c1cd5feaf0efd8eb94a4d61fc4812f1142572433ae397cd5f27bf603
\ No newline at end of file

db/structure.sql

@@ -26151,11 +26151,13 @@ CREATE TRIGGER trigger_has_external_issue_tracker_on_insert AFTER INSERT ON inte
 
 CREATE TRIGGER trigger_has_external_issue_tracker_on_update AFTER UPDATE ON integrations FOR EACH ROW WHEN ((((new.category)::text = 'issue_tracker'::text) AND (old.active <> new.active) AND (new.project_id IS NOT NULL))) EXECUTE FUNCTION set_has_external_issue_tracker();
 
-CREATE TRIGGER trigger_has_external_wiki_on_delete AFTER DELETE ON integrations FOR EACH ROW WHEN ((((old.type)::text = 'ExternalWikiService'::text) AND (old.project_id IS NOT NULL))) EXECUTE FUNCTION set_has_external_wiki();
+CREATE TRIGGER trigger_has_external_wiki_on_delete AFTER DELETE ON integrations FOR EACH ROW WHEN (((old.type_new = 'Integrations::ExternalWiki'::text) AND (old.project_id IS NOT NULL))) EXECUTE FUNCTION set_has_external_wiki();
 
-CREATE TRIGGER trigger_has_external_wiki_on_insert AFTER INSERT ON integrations FOR EACH ROW WHEN (((new.active = true) AND ((new.type)::text = 'ExternalWikiService'::text) AND (new.project_id IS NOT NULL))) EXECUTE FUNCTION set_has_external_wiki();
+CREATE TRIGGER trigger_has_external_wiki_on_insert AFTER INSERT ON integrations FOR EACH ROW WHEN (((new.active = true) AND (new.type_new = 'Integrations::ExternalWiki'::text) AND (new.project_id IS NOT NULL))) EXECUTE FUNCTION set_has_external_wiki();
 
-CREATE TRIGGER trigger_has_external_wiki_on_update AFTER UPDATE ON integrations FOR EACH ROW WHEN ((((new.type)::text = 'ExternalWikiService'::text) AND (old.active <> new.active) AND (new.project_id IS NOT NULL))) EXECUTE FUNCTION set_has_external_wiki();
+CREATE TRIGGER trigger_has_external_wiki_on_type_new_updated AFTER UPDATE OF type_new ON integrations FOR EACH ROW WHEN (((new.type_new = 'Integrations::ExternalWiki'::text) AND (new.project_id IS NOT NULL))) EXECUTE FUNCTION set_has_external_wiki();
+
+CREATE TRIGGER trigger_has_external_wiki_on_update AFTER UPDATE ON integrations FOR EACH ROW WHEN (((new.type_new = 'Integrations::ExternalWiki'::text) AND (old.active <> new.active) AND (new.project_id IS NOT NULL))) EXECUTE FUNCTION set_has_external_wiki();
 
 CREATE TRIGGER trigger_type_new_on_insert AFTER INSERT ON integrations FOR EACH ROW EXECUTE FUNCTION integrations_set_type_new();
 

spec/migrations/replace_external_wiki_triggers_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require_migration!
+
+RSpec.describe ReplaceExternalWikiTriggers do
+  let(:migration) { described_class.new }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:integrations) { table(:integrations) }
+
+  before do
+    @namespace = namespaces.create!(name: 'foo', path: 'foo')
+    @project = projects.create!(namespace_id: @namespace.id)
+  end
+
+  def create_external_wiki_integration(**attrs)
+    attrs.merge!(type_info)
+
+    integrations.create!(**attrs)
+  end
+
+  def has_external_wiki
+    !!@project.reload.has_external_wiki
+  end
+
+  shared_examples 'external wiki triggers' do
+    describe 'INSERT trigger' do
+      it 'sets `has_external_wiki` to true when active external wiki integration is inserted' do
+        expect do
+          create_external_wiki_integration(active: true, project_id: @project.id)
+        end.to change { has_external_wiki }.to(true)
+      end
+
+      it 'does not set `has_external_wiki` to true when integration is for a different project' do
+        different_project = projects.create!(namespace_id: @namespace.id)
+
+        expect do
+          create_external_wiki_integration(active: true, project_id: different_project.id)
+        end.not_to change { has_external_wiki }
+      end
+
+      it 'does not set `has_external_wiki` to true when inactive external wiki integration is inserted' do
+        expect do
+          create_external_wiki_integration(active: false, project_id: @project.id)
+        end.not_to change { has_external_wiki }
+      end
+
+      it 'does not set `has_external_wiki` to true when active other service is inserted' do
+        expect do
+          integrations.create!(type_new: 'Integrations::MyService', type: 'MyService', active: true, project_id: @project.id)
+        end.not_to change { has_external_wiki }
+      end
+    end
+
+    describe 'UPDATE trigger' do
+      it 'sets `has_external_wiki` to true when `ExternalWikiService` is made active' do
+        service = create_external_wiki_integration(active: false, project_id: @project.id)
+
+        expect do
+          service.update!(active: true)
+        end.to change { has_external_wiki }.to(true)
+      end
+
+      it 'sets `has_external_wiki` to false when integration is made inactive' do
+        service = create_external_wiki_integration(active: true, project_id: @project.id)
+
+        expect do
+          service.update!(active: false)
+        end.to change { has_external_wiki }.to(false)
+      end
+
+      it 'does not change `has_external_wiki` when integration is for a different project' do
+        different_project = projects.create!(namespace_id: @namespace.id)
+        service = create_external_wiki_integration(active: false, project_id: different_project.id)
+
+        expect do
+          service.update!(active: true)
+        end.not_to change { has_external_wiki }
+      end
+    end
+
+    describe 'DELETE trigger' do
+      it 'sets `has_external_wiki` to false when integration is deleted' do
+        service = create_external_wiki_integration(active: true, project_id: @project.id)
+
+        expect do
+          service.delete
+        end.to change { has_external_wiki }.to(false)
+      end
+
+      it 'does not change `has_external_wiki` when integration is for a different project' do
+        different_project = projects.create!(namespace_id: @namespace.id)
+        service = create_external_wiki_integration(active: true, project_id: different_project.id)
+
+        expect do
+          service.delete
+        end.not_to change { has_external_wiki }
+      end
+    end
+  end
+
+  describe '#up' do
+    before do
+      migrate!
+    end
+
+    context 'when integrations are created with the new STI value' do
+      let(:type_info) { { type_new: 'Integrations::ExternalWiki' } }
+
+      it_behaves_like 'external wiki triggers'
+    end
+
+    context 'when integrations are created with the old STI value' do
+      let(:type_info) { { type: 'ExternalWikiService' } }
+
+      it_behaves_like 'external wiki triggers'
+    end
+  end
+
+  describe '#down' do
+    before do
+      migration.up
+      migration.down
+    end
+
+    let(:type_info) { { type: 'ExternalWikiService' } }
+
+    it_behaves_like 'external wiki triggers'
+  end
+end

spec/models/project_spec.rb

@@ -1257,19 +1257,19 @@ RSpec.describe Project, factory_default: :keep do
     end
 
     it 'returns an active external wiki' do
-      create(:service, project: project, type: 'ExternalWikiService', active: true)
+      create(:external_wiki_integration, project: project, active: true)
 
       is_expected.to be_kind_of(Integrations::ExternalWiki)
     end
 
     it 'does not return an inactive external wiki' do
-      create(:service, project: project, type: 'ExternalWikiService', active: false)
+      create(:external_wiki_integration, project: project, active: false)
 
       is_expected.to eq(nil)
     end
 
     it 'sets Project#has_external_wiki when it is nil' do
-      create(:service, project: project, type: 'ExternalWikiService', active: true)
+      create(:external_wiki_integration, project: project, active: true)
       project.update_column(:has_external_wiki, nil)
 
       expect { subject }.to change { project.has_external_wiki }.from(nil).to(true)
@@ -1279,36 +1279,40 @@ RSpec.describe Project, factory_default: :keep do
   describe '#has_external_wiki' do
     let_it_be(:project) { create(:project) }
 
-    def subject
+    def has_external_wiki
       project.reload.has_external_wiki
     end
 
-    specify { is_expected.to eq(false) }
+    specify { expect(has_external_wiki).to eq(false) }
 
-    context 'when there is an active external wiki service' do
-      let!(:service) do
-        create(:service, project: project, type: 'ExternalWikiService', active: true)
+    context 'when there is an active external wiki integration' do
+      let(:active) { true }
+
+      let!(:integration) do
+        create(:external_wiki_integration, project: project, active: active)
       end
 
-      specify { is_expected.to eq(true) }
+      specify { expect(has_external_wiki).to eq(true) }
 
       it 'becomes false if the external wiki service is destroyed' do
         expect do
-          Integration.find(service.id).delete
-        end.to change { subject }.to(false)
+          Integration.find(integration.id).delete
+        end.to change { has_external_wiki }.to(false)
       end
 
       it 'becomes false if the external wiki service becomes inactive' do
         expect do
-          service.update_column(:active, false)
-        end.to change { subject }.to(false)
-      end
+          integration.update_column(:active, false)
+        end.to change { has_external_wiki }.to(false)
       end
 
-    it 'is false when external wiki service is not active' do
-      create(:service, project: project, type: 'ExternalWikiService', active: false)
+      context 'when created as inactive' do
+        let(:active) { false }
 
-      is_expected.to eq(false)
+        it 'is false' do
+          expect(has_external_wiki).to eq(false)
+        end
+      end
     end
   end