diff --git a/db/post_migrate/20210813151908_replace_external_wiki_triggers.rb b/db/post_migrate/20210813151908_replace_external_wiki_triggers.rb new file mode 100644 index 00000000000..d11baae42e2 --- /dev/null +++ b/db/post_migrate/20210813151908_replace_external_wiki_triggers.rb @@ -0,0 +1,66 @@ +# 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 diff --git a/db/schema_migrations/20210813151908 b/db/schema_migrations/20210813151908 new file mode 100644 index 00000000000..b2d1602658b --- /dev/null +++ b/db/schema_migrations/20210813151908 @@ -0,0 +1 @@ +fdb6dd20c1cd5feaf0efd8eb94a4d61fc4812f1142572433ae397cd5f27bf603 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 2f34ab47f28..3c06ee0977e 100644 --- a/db/structure.sql +++ b/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(); diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb index c05a3610b99..89ccb0949fe 100644 --- a/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/revert/reverting_merge_request_spec.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true module QA - RSpec.describe 'Create' do + RSpec.describe 'Create', quarantine: { + only: { job: 'large-setup' }, + issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/338324', + type: :stale + } do describe 'Merged merge request' do let(:project) do Resource::Project.fabricate_via_api! do |project| diff --git a/spec/migrations/replace_external_wiki_triggers_spec.rb b/spec/migrations/replace_external_wiki_triggers_spec.rb new file mode 100644 index 00000000000..392ef76c5ba --- /dev/null +++ b/spec/migrations/replace_external_wiki_triggers_spec.rb @@ -0,0 +1,132 @@ +# 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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d8f3a63d221..501f36c5107 100644 --- a/spec/models/project_spec.rb +++ b/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) + integration.update_column(:active, false) + end.to change { has_external_wiki }.to(false) end - 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