Merge branch '268133-fix-propagate-integrations-for-project-callback' into 'master'

Fix project callbacks when propagating integrations

See merge request gitlab-org/gitlab!45781

app/services/bulk_create_integration_service.rb

@@ -11,6 +11,8 @@ class BulkCreateIntegrationService
     service_list = ServiceList.new(batch, service_hash, association).to_array
 
     Service.transaction do
+      run_callbacks(batch) if association == 'project'
+
       results = bulk_insert(*service_list)
 
       if integration.data_fields_present?
@@ -18,8 +20,6 @@ class BulkCreateIntegrationService
 
         bulk_insert(*data_list)
       end
-
-      run_callbacks(batch) if association == 'project'
     end
   end
 
@@ -33,17 +33,15 @@ class BulkCreateIntegrationService
     klass.insert_all(items_to_insert, returning: [:id])
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def run_callbacks(batch)
-    if integration.issue_tracker?
-      Project.where(id: batch.select(:id)).update_all(has_external_issue_tracker: true)
+    if integration.issue_tracker? && integration.active?
+      batch.update_all(has_external_issue_tracker: true)
     end
 
-    if integration.type == 'ExternalWikiService'
-      Project.where(id: batch.select(:id)).update_all(has_external_wiki: true)
+    if integration.type == 'ExternalWikiService' && integration.active?
+      batch.update_all(has_external_wiki: true)
     end
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   def service_hash
     if integration.template?

changelogs/unreleased/268133-fix-propagate-integrations-for-project-callback.yml

+---
+title: Fix project callbacks when propagating integrations
+merge_request: 45781
+author:
+type: fixed

spec/factories/projects.rb

@@ -10,8 +10,10 @@ FactoryBot.define do
   factory :project, class: 'Project' do
     sequence(:name) { |n| "project#{n}" }
     path { name.downcase.gsub(/\s/, '_') }
-    # Behaves differently to nil due to cache_has_external_issue_tracker
+
+    # Behaves differently to nil due to cache_has_external_* methods.
     has_external_issue_tracker { false }
+    has_external_wiki { false }
 
     # Associations
     namespace

spec/factories/services.rb

@@ -139,6 +139,13 @@ FactoryBot.define do
     end
   end
 
+  factory :external_wiki_service do
+    project
+    type { ExternalWikiService }
+    active { true }
+    external_wiki_url { 'http://external-wiki-url.com' }
+  end
+
   factory :open_project_service do
     project
     active { true }

spec/requests/api/projects_spec.rb

@@ -890,7 +890,7 @@ RSpec.describe API::Projects do
       expect(response).to have_gitlab_http_status(:created)
 
       project.each_pair do |k, v|
-        next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled storage_version].include?(k)
+        next if %i[has_external_issue_tracker has_external_wiki issues_enabled merge_requests_enabled wiki_enabled storage_version].include?(k)
 
         expect(json_response[k.to_s]).to eq(v)
       end
@@ -1309,7 +1309,7 @@ RSpec.describe API::Projects do
       expect(response).to have_gitlab_http_status(:created)
 
       project.each_pair do |k, v|
-        next if %i[has_external_issue_tracker path storage_version].include?(k)
+        next if %i[has_external_issue_tracker has_external_wiki path storage_version].include?(k)
 
         expect(json_response[k.to_s]).to eq(v)
       end

spec/services/bulk_create_integration_service_spec.rb

@@ -41,7 +41,7 @@ RSpec.describe BulkCreateIntegrationService do
     end
   end
 
-  shared_examples 'runs project callbacks' do
+  shared_examples 'updates project callbacks' do
     it 'updates projects#has_external_issue_tracker for issue tracker services' do
       described_class.new(integration, batch, association).execute
 
@@ -49,14 +49,7 @@ RSpec.describe BulkCreateIntegrationService do
     end
 
     context 'with an external wiki integration' do
-      let(:integration) do
-        ExternalWikiService.create!(
-          instance: true,
-          active: true,
-          push_events: false,
-          external_wiki_url: 'http://external-wiki-url.com'
-        )
-      end
+      let(:integration) { create(:external_wiki_service, :instance) }
 
       it 'updates projects#has_external_wiki for external wiki services' do
         described_class.new(integration, batch, association).execute
@@ -66,18 +59,44 @@ RSpec.describe BulkCreateIntegrationService do
     end
   end
 
+  shared_examples 'does not update project callbacks' do
+    it 'does not update projects#has_external_issue_tracker for issue tracker services' do
+      described_class.new(integration, batch, association).execute
+
+      expect(project.reload.has_external_issue_tracker).to eq(false)
+    end
+
+    context 'with an inactive external wiki integration' do
+      let(:integration) { create(:external_wiki_service, :instance, active: false) }
+
+      it 'does not update projects#has_external_wiki for external wiki services' do
+        described_class.new(integration, batch, association).execute
+
+        expect(project.reload.has_external_wiki).to eq(false)
+      end
+    end
+  end
+
   context 'with an instance-level integration' do
     let(:integration) { instance_integration }
 
     context 'with a project association' do
       let!(:project) { create(:project) }
       let(:created_integration) { project.jira_service }
-      let(:batch) { Project.all }
+      let(:batch) { Project.without_integration(integration) }
       let(:association) { 'project' }
 
       it_behaves_like 'creates integration from batch ids'
       it_behaves_like 'updates inherit_from_id'
-      it_behaves_like 'runs project callbacks'
+      it_behaves_like 'updates project callbacks'
+
+      context 'when integration is not active' do
+        before do
+          integration.update!(active: false)
+        end
+
+        it_behaves_like 'does not update project callbacks'
+      end
     end
 
     context 'with a group association' do
@@ -101,7 +120,7 @@ RSpec.describe BulkCreateIntegrationService do
       let(:association) { 'project' }
 
       it_behaves_like 'creates integration from batch ids'
-      it_behaves_like 'runs project callbacks'
+      it_behaves_like 'updates project callbacks'
     end
   end
 end