Database create many

[eserna27]

In briq.mx, we have this problem #137.

Here we have tried to solve it using insert_all to create all the notifications at once.

We have changed Noticed::Base#deliver to pass all recipients to run_delivery.

def run_delivery(recipients, enqueue: true)
  delivery_methods = self.class.delivery_methods.dup
  # Run database delivery inline first if it exists so other methods have access to the record
  if (index = delivery_methods.find_index { |m| m[:name] == :database })
      delivery_method = delivery_methods.delete_at(index)
      records = run_database_delivery_method(delivery_method, recipients: recipients, enqueue: false)
    end

    recipients.each do |recipient|
        record = Array.wrap(records).detect{|record| record.recipient == recipient}
        delivery_methods.each do |delivery_method|
        run_delivery_method(delivery_method, recipient: recipient, enqueue: enqueue, record: record)
    end
  end
end

[bhserna]

@eserna27 I have not tested it yet, but here are some things that maybe you can consider.

[bhserna]

I think that here records is not needed.

[bhserna]

You are not using this part any more... I think that you can delete it

[bhserna]

Maybe here you can use :database directly?

Suggested change

	method = delivery_method_for(delivery_method[:name], delivery_method[:options])
	method = delivery_method_for(:database, delivery_method[:options])

[bhserna]

Suggested change

	if bulk?
	ids = klass.insert_all!(notifications).rows
	records = klass.find(ids)
	else
	records = klass.create!(notifications)
	end
	records
	if bulk?
	ids = klass.insert_all!(notifications).rows
	klass.find(ids)
	else
	klass.create!(notifications)
	end

[bhserna]

Is it possible to do It like this?

Suggested change

	recipients.each do |recipient|
	record = Array.wrap(records).detect{|record| record.recipient == recipient}
	delivery_methods.each do |delivery_method|
	run_delivery_method(delivery_method, recipient: recipient, enqueue: enqueue, record: record)
	end
	end
	records.each do |recipient|
	delivery_methods.each do |delivery_method|
	run_delivery_method(delivery_method, recipient: record.recipient, enqueue: enqueue, record: record)
	end
	end

[bhserna]

Also, when you do the bulk insert, you are "finding" the notifications but you are not preloading the recipient (as far as I know)... I think that we should preload it.

[eserna27]

[bhserna]

Also... maybe instead of asking for the rails version and the current adapter, we can ask if klass responds to insert_all. Don't you think?

Suggested change

	if bulk?
	ids = klass.insert_all!(notifications).rows
	klass.preload(:recipient).find(ids)
	else
	klass.create!(notifications)
	end
	if klass.respond_to?(:insert_all!)
	ids = klass.insert_all!(notifications).rows
	klass.preload(:recipient).find(ids)
	else
	klass.create!(notifications)
	end

[excid3]

insert_all only seems to return the IDs on Postgres. This needs to be compatible with MySQL and SQLite too, so I'm not sure insert_all will make sense to use here.