diff --git a/app/services/bulk_create_integration_service.rb b/app/services/bulk_create_integration_service.rb index 23b89b0d8a9db3407f4367e4154e4078752f4a7e..2cec7e719891e4656b14d1bc342a88942a691387 100644 --- a/app/services/bulk_create_integration_service.rb +++ b/app/services/bulk_create_integration_service.rb @@ -11,6 +11,8 @@ def execute 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 @@ def execute bulk_insert(*data_list) end - - run_callbacks(batch) if association == 'project' end end @@ -33,17 +33,15 @@ def bulk_insert(klass, columns, values_array) 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? diff --git a/changelogs/unreleased/268133-fix-propagate-integrations-for-project-callback.yml b/changelogs/unreleased/268133-fix-propagate-integrations-for-project-callback.yml new file mode 100644 index 0000000000000000000000000000000000000000..6af3e240d160debf8234b61ab97c5b1aef6a1545 --- /dev/null +++ b/changelogs/unreleased/268133-fix-propagate-integrations-for-project-callback.yml @@ -0,0 +1,5 @@ +--- +title: Fix project callbacks when propagating integrations +merge_request: 45781 +author: +type: fixed diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 87e4a8e355d306c4438c0cfbd1da3b53efaacbac..639fff06cecef610759d8198cbc0e676cb7b059c 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -10,8 +10,10 @@ 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 diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 139970808170c233452e43056ee98175b6715a9d..f4b73b6ed70b64c993127fec71ca909ef59539c9 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -139,6 +139,13 @@ 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 } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 2abcb39a1c8cd2ecaeca3cf5e073cd9186ad7fea..306478774c1a5cebb05f2ba1c495da94f4e4b1c8 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -890,7 +890,7 @@ 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 @@ 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 diff --git a/spec/services/bulk_create_integration_service_spec.rb b/spec/services/bulk_create_integration_service_spec.rb index 5d896f78b351e2c80c7bee15ce1e9af47cc17860..140b7a84d40fe187e6a9b7487fc730dd2a0c1a38 100644 --- a/spec/services/bulk_create_integration_service_spec.rb +++ b/spec/services/bulk_create_integration_service_spec.rb @@ -41,7 +41,7 @@ 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 @@ 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 @@ 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 @@ 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