diff --git a/app/models/integration.rb b/app/models/integration.rb index 32aaee4b49ea371940017d9422a301dc9409b649..3101d257d9efc1b5178695fb61749c5c01b7094c 100644 --- a/app/models/integration.rb +++ b/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) } diff --git a/app/models/integrations/base_third_party_wiki.rb b/app/models/integrations/base_third_party_wiki.rb new file mode 100644 index 0000000000000000000000000000000000000000..24f5bec93cfe11de7368cdddd7dc7977cdcda333 --- /dev/null +++ b/app/models/integrations/base_third_party_wiki.rb @@ -0,0 +1,39 @@ +# 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 diff --git a/app/models/integrations/confluence.rb b/app/models/integrations/confluence.rb index 65adce7a8d698079e56b9d0cfb5824c20aa851ba..4e1d1993d026e4bd3d4e95ad6ab8f9bbbd94d96a 100644 --- a/app/models/integrations/confluence.rb +++ b/app/models/integrations/confluence.rb @@ -1,7 +1,7 @@ # 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 @@ class Confluence < Integration 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 @@ def confluence_uri_valid? 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 diff --git a/app/models/integrations/shimo.rb b/app/models/integrations/shimo.rb index 0e1023bb7a7b8030082e381369ee60734ee90843..dd25a0bc558ac5ce83f17a678c679ab06104c9f3 100644 --- a/app/models/integrations/shimo.rb +++ b/app/models/integrations/shimo.rb @@ -1,12 +1,10 @@ # 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 @@ def execute(_data) nil end - def self.supported_events - %w() - end - def fields [ { @@ -47,14 +41,5 @@ def fields } ] 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 diff --git a/db/post_migrate/20220324032250_migrate_shimo_confluence_service_category.rb b/db/post_migrate/20220324032250_migrate_shimo_confluence_service_category.rb new file mode 100644 index 0000000000000000000000000000000000000000..d341cc508744c785419de028e96a2cb68c012589 --- /dev/null +++ b/db/post_migrate/20220324032250_migrate_shimo_confluence_service_category.rb @@ -0,0 +1,21 @@ +# 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 diff --git a/db/schema_migrations/20220324032250 b/db/schema_migrations/20220324032250 new file mode 100644 index 0000000000000000000000000000000000000000..ef37a8ff2234f239d55c11f8a41a6475468959ac --- /dev/null +++ b/db/schema_migrations/20220324032250 @@ -0,0 +1 @@ +fcfead40cfa1d75303bd0d392c09b5b0399ce5163e7ad6f8650360fe3adbe85d \ No newline at end of file diff --git a/lib/gitlab/background_migration/migrate_shimo_confluence_integration_category.rb b/lib/gitlab/background_migration/migrate_shimo_confluence_integration_category.rb new file mode 100644 index 0000000000000000000000000000000000000000..ec4631d1e34c4435e95b53d53de26f5757749057 --- /dev/null +++ b/lib/gitlab/background_migration/migrate_shimo_confluence_integration_category.rb @@ -0,0 +1,27 @@ +# 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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 906cf5654f4953df9e3d94e64c0be19ab5aea5b3..b2ffce3aeeeddb3b7da4879c740f8ab932011eb1 100644 --- a/locale/gitlab.pot +++ b/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 "" diff --git a/spec/lib/gitlab/background_migration/migrate_shimo_confluence_integration_category_spec.rb b/spec/lib/gitlab/background_migration/migrate_shimo_confluence_integration_category_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c7f0607fc884dc137eb65724fa4819ccb85eaeeb --- /dev/null +++ b/spec/lib/gitlab/background_migration/migrate_shimo_confluence_integration_category_spec.rb @@ -0,0 +1,28 @@ +# 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 diff --git a/spec/lib/sidebars/projects/panel_spec.rb b/spec/lib/sidebars/projects/panel_spec.rb index 7e69a2dfe52eb0258f8587bc4509ee18701b6913..ff253eedd0818c1a6f1a0ae36e2d4cb7c6849360 100644 --- a/spec/lib/sidebars/projects/panel_spec.rb +++ b/spec/lib/sidebars/projects/panel_spec.rb @@ -17,16 +17,40 @@ 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 diff --git a/spec/migrations/20220324032250_migrate_shimo_confluence_service_category_spec.rb b/spec/migrations/20220324032250_migrate_shimo_confluence_service_category_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..38db6d51e7e396ea2ddfe978ed0240aa8a6e0bbb --- /dev/null +++ b/spec/migrations/20220324032250_migrate_shimo_confluence_service_category_spec.rb @@ -0,0 +1,34 @@ +# 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 diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index a1638cdc6afb39f0d4b12296b00e4c2528f7b9ca..21f6d320d71a2613e76a8cef9dc5f6677ce77801 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -60,6 +60,17 @@ 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) diff --git a/spec/models/integrations/base_third_party_wiki_spec.rb b/spec/models/integrations/base_third_party_wiki_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..11e044c2a18b71d69933fd7e84caf8a233b78fa1 --- /dev/null +++ b/spec/models/integrations/base_third_party_wiki_spec.rb @@ -0,0 +1,42 @@ +# 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