Merge branch 'fix/bulk_update_integration_service/foreign_key' into 'master'

Fix bulk_update_integration_service foreign_key

See merge request gitlab-org/gitlab!73456

app/models/data_list.rb

@@ -1,22 +1,26 @@
 # frozen_string_literal: true
 
 class DataList
-  def initialize(batch, data_fields_hash, klass)
+  def initialize(batch, data_fields_hash, data_fields_klass)
     @batch = batch
     @data_fields_hash = data_fields_hash
-    @klass = klass
+    @data_fields_klass = data_fields_klass
   end
 
   def to_array
-    [klass, columns, values]
+    [data_fields_klass, columns, values]
   end
 
   private
 
-  attr_reader :batch, :data_fields_hash, :klass
+  attr_reader :batch, :data_fields_hash, :data_fields_klass
 
   def columns
-    data_fields_hash.keys << 'service_id'
+    data_fields_hash.keys << data_fields_foreign_key
+  end
+
+  def data_fields_foreign_key
+    data_fields_klass.reflections['integration'].foreign_key
   end
 
   def values

app/models/integration.rb

@@ -373,7 +373,7 @@ def to_integration_hash
   end
 
   def to_data_fields_hash
-    data_fields.as_json(only: data_fields.class.column_names).except('id', 'service_id')
+    data_fields.as_json(only: data_fields.class.column_names).except('id', 'service_id', 'integration_id')
   end
 
   def event_channel_names

app/services/bulk_update_integration_service.rb

@@ -12,7 +12,7 @@ def execute
       Integration.where(id: batch_ids).update_all(integration_hash)
 
       if integration.data_fields_present?
-        integration.data_fields.class.where(service_id: batch_ids).update_all(data_fields_hash)
+        integration.data_fields.class.where(data_fields_foreign_key => batch_ids).update_all(data_fields_hash)
       end
     end
   end
@@ -22,6 +22,11 @@ def execute
 
   attr_reader :integration, :batch
 
+  # service_id or integration_id
+  def data_fields_foreign_key
+    integration.data_fields.class.reflections['integration'].foreign_key
+  end
+
   def integration_hash
     integration.to_integration_hash.tap { |json| json['inherit_from_id'] = integration.inherit_from_id || integration.id }
   end

spec/models/data_list_spec.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DataList do
+  describe '#to_array' do
+    let(:jira_integration) { create(:jira_integration) }
+    let(:zentao_integration) { create(:zentao_integration) }
+    let(:cases) do
+      [
+        [jira_integration, 'Integrations::JiraTrackerData', 'service_id'],
+        [zentao_integration, 'Integrations::ZentaoTrackerData', 'integration_id']
+      ]
+    end
+
+    def data_list(integration)
+      DataList.new([integration], integration.to_data_fields_hash, integration.data_fields.class).to_array
+    end
+
+    it 'returns current data' do
+      cases.each do |integration, data_fields_class_name, foreign_key|
+        data_fields_klass, columns, values_items = data_list(integration)
+
+        expect(data_fields_klass.to_s).to eq data_fields_class_name
+        expect(columns.last).to eq foreign_key
+        values = values_items.first
+        expect(values.last).to eq integration.id
+      end
+    end
+  end
+end

spec/services/bulk_create_integration_service_spec.rb

@@ -25,7 +25,7 @@
     end
 
     context 'integration with data fields' do
-      let(:excluded_attributes) { %w[id service_id created_at updated_at] }
+      let(:excluded_attributes) { %w[id service_id integration_id created_at updated_at] }
 
       it 'updates the data fields from inherited integrations' do
         described_class.new(integration, batch, association).execute
@@ -82,6 +82,14 @@
 
       it_behaves_like 'creates integration from batch ids'
       it_behaves_like 'updates inherit_from_id'
+
+      context 'with different foreign key of data_fields' do
+        let(:integration) { create(:zentao_integration, group: group, project: nil) }
+        let(:created_integration) { project.zentao_integration }
+
+        it_behaves_like 'creates integration from batch ids'
+        it_behaves_like 'updates inherit_from_id'
+      end
     end
 
     context 'with a group association' do
@@ -94,6 +102,13 @@
 
       it_behaves_like 'creates integration from batch ids'
       it_behaves_like 'updates inherit_from_id'
+
+      context 'with different foreign key of data_fields' do
+        let(:integration) { create(:zentao_integration, group: group, project: nil, inherit_from_id: instance_integration.id) }
+
+        it_behaves_like 'creates integration from batch ids'
+        it_behaves_like 'updates inherit_from_id'
+      end
     end
   end
 end

spec/services/bulk_update_integration_service_spec.rb

@@ -88,4 +88,22 @@
       described_class.new(group_integration, [integration]).execute
     end.to change { integration.reload.url }.to(group_integration.url)
   end
+
+  context 'with different foreign key of data_fields' do
+    let(:integration) { create(:zentao_integration, project: create(:project, group: group)) }
+    let(:group_integration) do
+      Integrations::Zentao.create!(
+        group: group,
+        url: 'https://group.zentao.net',
+        api_token: 'GROUP_TOKEN',
+        zentao_product_xid: '1'
+      )
+    end
+
+    it 'works with batch as an array of ActiveRecord objects' do
+      expect do
+        described_class.new(group_integration, [integration]).execute
+      end.to change { integration.reload.url }.to(group_integration.url)
+    end
+  end
 end