Refactor: Introduce BaseThirdPartyWiki

We extract BaseThirdPartyWiki from Shimo and Confluence, reduce
duplicate code. Also add post migration to add `third_party_wiki`
category for Shimo and Confluence.

Changelog: other

app/models/integration.rb

@@ -96,6 +96,9 @@ class Integration < ApplicationRecord
   validate :validate_belongs_to_project_or_group
 
   scope :external_issue_trackers, -> { where(category: 'issue_tracker').active }
+  # TODO: Will be modified in 15.0
+  # Details: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74501#note_744393645
+  scope :third_party_wikis, -> { where(type: %w[Integrations::Confluence Integrations::Shimo]).active }
   scope :by_name, ->(name) { by_type(integration_name_to_type(name)) }
   scope :external_wikis, -> { by_name(:external_wiki).active }
   scope :active, -> { where(active: true) }

app/models/integrations/base_third_party_wiki.rb

+# frozen_string_literal: true
+
+module Integrations
+  class BaseThirdPartyWiki < Integration
+    default_value_for :category, 'third_party_wiki'
+
+    validate :only_one_third_party_wiki, if: :activated?, on: :manual_change
+
+    after_commit :cache_project_has_integration
+
+    def self.supported_events
+      %w()
+    end
+
+    private
+
+    def only_one_third_party_wiki
+      return unless project_level?
+
+      if project.integrations.third_party_wikis.id_not_in(id).any?
+        errors.add(:base, _('Another third-party wiki is already in use. '\
+                            'Only one third-party wiki integration can be active at a time'))
+      end
+    end
+
+    def cache_project_has_integration
+      return unless project && !project.destroyed?
+
+      project_setting = project.project_setting
+
+      project_setting.public_send("#{project_settings_cache_key}=", active?) # rubocop:disable GitlabSecurity/PublicSend
+      project_setting.save!
+    end
+
+    def project_settings_cache_key
+      "has_#{self.class.to_param}"
+    end
+  end
+end

app/models/integrations/confluence.rb

 # frozen_string_literal: true
 
 module Integrations
-  class Confluence < Integration
+  class Confluence < BaseThirdPartyWiki
     VALID_SCHEME_MATCH = %r{\Ahttps?\Z}.freeze
     VALID_HOST_MATCH = %r{\A.+\.atlassian\.net\Z}.freeze
     VALID_PATH_MATCH = %r{\A/wiki(/|\Z)}.freeze
@@ -11,16 +11,10 @@ module Integrations
     validates :confluence_url, presence: true, if: :activated?
     validate :validate_confluence_url_is_cloud, if: :activated?
 
-    after_commit :cache_project_has_confluence
-
     def self.to_param
       'confluence'
     end
 
-    def self.supported_events
-      %w()
-    end
-
     def title
       s_('ConfluenceService|Confluence Workspace')
     end
@@ -80,12 +74,5 @@ module Integrations
     rescue URI::InvalidURIError
       false
     end
-
-    def cache_project_has_confluence
-      return unless project && !project.destroyed?
-
-      project.project_setting.save! unless project.project_setting.persisted?
-      project.project_setting.update_column(:has_confluence, active?)
-    end
   end
 end

app/models/integrations/shimo.rb

 # frozen_string_literal: true
 
 module Integrations
-  class Shimo < Integration
+  class Shimo < BaseThirdPartyWiki
     prop_accessor :external_wiki_url
     validates :external_wiki_url, presence: true, public_url: true, if: :activated?
 
-    after_commit :cache_project_has_shimo
-
     def render?
       return false unless Feature.enabled?(:shimo_integration, project)
 
@@ -33,10 +31,6 @@ module Integrations
       nil
     end
 
-    def self.supported_events
-      %w()
-    end
-
     def fields
       [
         {
@@ -47,14 +41,5 @@ module Integrations
         }
       ]
     end
-
-    private
-
-    def cache_project_has_shimo
-      return unless project && !project.destroyed?
-
-      project.project_setting.save! unless project.project_setting.persisted?
-      project.project_setting.update_column(:has_shimo, activated?)
-    end
   end
 end

db/post_migrate/20220324032250_migrate_shimo_confluence_service_category.rb

+# frozen_string_literal: true
+
+class MigrateShimoConfluenceServiceCategory < Gitlab::Database::Migration[1.0]
+  MIGRATION = 'MigrateShimoConfluenceIntegrationCategory'
+  DELAY_INTERVAL = 2.minutes
+  BATCH_SIZE = 10_000
+
+  disable_ddl_transaction!
+
+  def up
+    queue_background_migration_jobs_by_range_at_intervals(
+      define_batchable_model('integrations').where(type_new: %w[Integrations::Confluence Integrations::Shimo]),
+      MIGRATION,
+      DELAY_INTERVAL,
+      batch_size: BATCH_SIZE,
+      track_jobs: true)
+  end
+
+  def down
+  end
+end

db/schema_migrations/20220324032250

+fcfead40cfa1d75303bd0d392c09b5b0399ce5163e7ad6f8650360fe3adbe85d
\ No newline at end of file

lib/gitlab/background_migration/migrate_shimo_confluence_integration_category.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # The class to migrate category of integrations to third_party_wiki for confluence and shimo
+    class MigrateShimoConfluenceIntegrationCategory
+      include Gitlab::Database::DynamicModelHelpers
+
+      def perform(start_id, end_id)
+        define_batchable_model('integrations', connection: ::ActiveRecord::Base.connection)
+          .where(id: start_id..end_id, type_new: %w[Integrations::Confluence Integrations::Shimo])
+          .update_all(category: 'third_party_wiki')
+
+        mark_job_as_succeeded(start_id, end_id)
+      end
+
+      private
+
+      def mark_job_as_succeeded(*arguments)
+        Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded(
+          self.class.name.demodulize,
+          arguments
+        )
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -4259,6 +4259,9 @@ msgstr ""
 msgid "Another issue tracker is already in use. Only one issue tracker service can be active at a time"
 msgstr ""
 
+msgid "Another third-party wiki is already in use. Only one third-party wiki integration can be active at a time"
+msgstr ""
+
 msgid "Anti-spam verification"
 msgstr ""
 

spec/lib/gitlab/background_migration/migrate_shimo_confluence_integration_category_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::MigrateShimoConfluenceIntegrationCategory do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:integrations) { table(:integrations) }
+  let(:perform) { described_class.new.perform(1, 5) }
+
+  before do
+    namespace = namespaces.create!(name: 'test', path: 'test')
+    projects.create!(id: 1, namespace_id: namespace.id, name: 'gitlab', path: 'gitlab')
+    integrations.create!(id: 1, active: true, type_new: "Integrations::SlackSlashCommands",
+                         category: 'chat', project_id: 1)
+    integrations.create!(id: 3, active: true, type_new: "Integrations::Confluence", category: 'common', project_id: 1)
+    integrations.create!(id: 5, active: true, type_new: "Integrations::Shimo", category: 'common', project_id: 1)
+  end
+
+  describe '#up' do
+    it 'updates category to third_party_wiki for Shimo and Confluence' do
+      perform
+
+      expect(integrations.where(category: 'third_party_wiki').count).to eq(2)
+      expect(integrations.where(category: 'chat').count).to eq(1)
+    end
+  end
+end

spec/lib/sidebars/projects/panel_spec.rb

@@ -17,16 +17,40 @@ RSpec.describe Sidebars::Projects::Panel do
     subject { described_class.new(context).instance_variable_get(:@menus) }
 
     context 'when integration is present and active' do
-      let_it_be(:confluence) { create(:confluence_integration, active: true) }
+      context 'confluence only' do
+        let_it_be(:confluence) { create(:confluence_integration, active: true) }
 
-      let(:project) { confluence.project }
+        let(:project) { confluence.project }
 
-      it 'contains Confluence menu item' do
-        expect(subject.index { |i| i.is_a?(Sidebars::Projects::Menus::ConfluenceMenu) }).not_to be_nil
+        it 'contains Confluence menu item' do
+          expect(subject.index { |i| i.is_a?(Sidebars::Projects::Menus::ConfluenceMenu) }).not_to be_nil
+        end
+
+        it 'does not contain Wiki menu item' do
+          expect(subject.index { |i| i.is_a?(Sidebars::Projects::Menus::WikiMenu) }).to be_nil
+        end
       end
 
-      it 'does not contain Wiki menu item' do
-        expect(subject.index { |i| i.is_a?(Sidebars::Projects::Menus::WikiMenu) }).to be_nil
+      context 'shimo only' do
+        let_it_be(:shimo) { create(:shimo_integration, active: true) }
+
+        let(:project) { shimo.project }
+
+        it 'contains Shimo menu item' do
+          expect(subject.index { |i| i.is_a?(Sidebars::Projects::Menus::ShimoMenu) }).not_to be_nil
+        end
+      end
+
+      context 'confluence & shimo' do
+        let_it_be(:confluence) { create(:confluence_integration, active: true) }
+        let_it_be(:shimo) { create(:shimo_integration, active: true) }
+
+        let(:project) { confluence.project }
+
+        it 'contains Confluence menu item, not Shimo' do
+          expect(subject.index { |i| i.is_a?(Sidebars::Projects::Menus::ConfluenceMenu) }).not_to be_nil
+          expect(subject.index { |i| i.is_a?(Sidebars::Projects::Menus::ShimoMenu) }).to be_nil
+        end
       end
     end
 

spec/migrations/20220324032250_migrate_shimo_confluence_service_category_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe MigrateShimoConfluenceServiceCategory, :migration do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:integrations) { table(:integrations) }
+
+  before do
+    namespace = namespaces.create!(name: 'test', path: 'test')
+    projects.create!(id: 1, namespace_id: namespace.id, name: 'gitlab', path: 'gitlab')
+    integrations.create!(id: 1, active: true, type_new: "Integrations::SlackSlashCommands",
+                         category: 'chat', project_id: 1)
+    integrations.create!(id: 3, active: true, type_new: "Integrations::Confluence", category: 'common', project_id: 1)
+    integrations.create!(id: 5, active: true, type_new: "Integrations::Shimo", category: 'common', project_id: 1)
+  end
+
+  describe '#up' do
+    it 'correctly schedules background migrations', :aggregate_failures do
+      stub_const("#{described_class.name}::BATCH_SIZE", 2)
+
+      Sidekiq::Testing.fake! do
+        freeze_time do
+          migrate!
+
+          expect(described_class::MIGRATION).to be_scheduled_migration(3, 5)
+          expect(BackgroundMigrationWorker.jobs.size).to eq(1)
+        end
+      end
+    end
+  end
+end

spec/models/integration_spec.rb

@@ -60,6 +60,17 @@ RSpec.describe Integration do
   end
 
   describe 'Scopes' do
+    describe '.third_party_wikis' do
+      let!(:integration1) { create(:jira_integration) }
+      let!(:integration2) { create(:redmine_integration) }
+      let!(:integration3) { create(:confluence_integration) }
+      let!(:integration4) { create(:shimo_integration) }
+
+      it 'returns the right group integration' do
+        expect(described_class.third_party_wikis).to contain_exactly(integration3, integration4)
+      end
+    end
+
     describe '.with_default_settings' do
       it 'returns the correct integrations' do
         instance_integration = create(:integration, :instance)

spec/models/integrations/base_third_party_wiki_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Integrations::BaseThirdPartyWiki do
+  describe 'Validations' do
+    let_it_be_with_reload(:project) { create(:project) }
+
+    describe 'only one third party wiki per project' do
+      subject(:integration) { create(:shimo_integration, project: project, active: true) }
+
+      before_all do
+        create(:confluence_integration, project: project, active: true)
+      end
+
+      context 'when integration is changed manually by user' do
+        it 'executes the validation' do
+          valid = integration.valid?(:manual_change)
+
+          expect(valid).to be_falsey
+          error_message = 'Another third-party wiki is already in use. '\
+                          'Only one third-party wiki integration can be active at a time'
+          expect(integration.errors[:base]).to include _(error_message)
+        end
+      end
+
+      context 'when integration is changed internally' do
+        it 'does not execute the validation' do
+          expect(integration.valid?).to be_truthy
+        end
+      end
+
+      context 'when integration is not on the project level' do
+        subject(:integration) { create(:shimo_integration, :instance, active: true) }
+
+        it 'executes the validation' do
+          expect(integration.valid?(:manual_change)).to be_truthy
+        end
+      end
+    end
+  end
+end