Commit da3c6029 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch '209346-bulk-insert-follow-ups' into 'master'

Bulk-insert follow-up fixes

See merge request gitlab-org/gitlab!26764
parents 5f30192d 96b6ecef
......@@ -111,8 +111,8 @@ module BulkInsertSafe
end
def _bulk_insert_reject_primary_key!(attributes, primary_key)
if attributes.delete(primary_key)
raise PrimaryKeySetError, "Primary key set: #{primary_key}:#{attributes[primary_key]}\n" \
if existing_pk = attributes.delete(primary_key)
raise PrimaryKeySetError, "Primary key set: #{primary_key}:#{existing_pk}\n" \
"Bulk-inserts are only supported for rows that don't already have PK set"
end
end
......
......@@ -93,11 +93,14 @@ module BulkInsertableAssociations
end
def _bulk_insert_configure_foreign_key(reflection, items)
primary_key = self[reflection.active_record_primary_key]
raise "Classes including `BulkInsertableAssociations` must define a `primary_key`" unless primary_key
primary_key_column = reflection.active_record_primary_key
raise "Classes including `BulkInsertableAssociations` must define a `primary_key`" unless primary_key_column
primary_key_value = self[primary_key_column]
raise "No value found for primary key `#{primary_key_column}`" unless primary_key_value
items.each do |item|
item[reflection.foreign_key] = primary_key
item[reflection.foreign_key] = primary_key_value
if reflection.type
item[reflection.type] = self.class.polymorphic_name
......
......@@ -57,16 +57,12 @@ describe BulkInsertableAssociations do
end
end
before do
ActiveRecord::Base.connection.execute('TRUNCATE bulk_foos RESTART IDENTITY')
end
context 'saving bulk insertable associations' do
let(:parent) { BulkParent.new(name: 'parent') }
context 'when items already have IDs' do
it 'stores nothing and raises an error' do
build_items(parent: parent) { |n, item| item.id = 100 + n }
build_items(parent: parent) { |n, item| item.id = n }
expect { save_with_bulk_inserts(parent) }.to raise_error(BulkInsertSafe::PrimaryKeySetError)
expect(BulkFoo.count).to eq(0)
......@@ -79,7 +75,7 @@ describe BulkInsertableAssociations do
expect(BulkFoo).to receive(:bulk_insert!).once.and_call_original
expect { save_with_bulk_inserts(parent) }.to change { BulkFoo.count }.from(0).to(items.size)
expect(parent.bulk_foos.pluck(:id)).to contain_exactly(*(1..10))
expect(parent.bulk_foos.pluck(:id)).to all(be_a Integer)
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment