[10.x] Set timestamps on Model::insert()

[cosmastech]

Problem

Assume we have a model (MyModel) with timestamps. The following code inserts the data to the DB, but never sets the timestamps.

for($i = 0; $i < 100; $i++) {
    $toInsert[] = ['name' => faker()->name()];
}

MyModel::insert($toInsert);

assert(MyModel::query()->latest()->first()->created_at instanceof \DateTime); // ❌  AssertionError

The way around this is to explicitly set the created_at and updated_at columns.

for($i = 0; $i < 100; $i++) {
    $toInsert[] = [
        'name' => faker()->name(),
        'created_at' => now(),
        'updated_at' => now(),
    ];
}

MyModel::insert($toInsert);

assert(MyModel::query()->latest()->first()->created_at instanceof \DateTime); // 👍 

Solution

With this PR, if the Model uses timestamps, we'll set the created_at and/or updated_at columns to the current time upon calling Eloquent\Builder@insert()

Changes made to tests

For a number of Eloquent tests (especially those relying on Pivot models), no $timestamps property was ever set. I have updated those. I fear that this very necessity could be seen as the functionality being a breaking change. I personally don't think that it is, and was more or less oversight within the tests.

Final thoughts

It would be really nice to have a distinct method that casts all properties when doing an insert, rather than just the timestamps. Builder::insertWithCasts() could handle type juggling as well as timestamps. That way if client code needs to use an insert where the table has a json column, they don't have to remember to json_encode() the data. I would not tie it into the insert() method as it's computational waste to retrieve a model and cast properties.

[ankurk91]

How it is different from create method?

[cosmastech]

How it is different from create method?

Builder@insert() runs 1 query to insert many records.

Builder@create() runs 1 query per model that is being inserted.

[taylorotwell]

Insert is a low-level query builder method. You would need to use create to get full Eloquent features.

[cosmastech]

Insert is a low-level query builder method. You would need to use create to get full Eloquent features.

@taylorotwell What do you think about a createMany() method on the Builder class itself? This function exists on HasOneOrMany & BelongsToMany (though it's looping through and saving them each one-by-one).

Model isn't Macroable and neither is Eloquent\Builder, right? Which I think is usually the project-level solution to this. Otherwise, it's extend the Model class and make sure everything within a project extends from it or add a trait to every model. Both are not ideal solutions.

[bert-w]

Insert is a low-level query builder method. You would need to use create to get full Eloquent features.

@taylorotwell What do you think about a createMany() method on the Builder class itself? This function exists on HasOneOrMany & BelongsToMany (though it's looping through and saving them each one-by-one).

Model isn't Macroable and neither is Eloquent\Builder, right? Which I think is usually the project-level solution to this. Otherwise, it's extend the Model class and make sure everything within a project extends from it or add a trait to every model. Both are not ideal solutions.

The thing is when you want to make something like this and attach Eloquent features like created_at/updated_at, people also expect it to fire events like updated/created/saved/.... These events require a hydrated model in order to be usable, which is not available through one UPDATE statement so it requires additional queries.

The existing createMany function is in that sense inefficient since as you noted it saves each model separately (there is room for improvement here since you could potentially send it all as one update-statement, preferably in a transaction). It would be nice too if this was a main EloquentBuilder method instead of a relation only.

Also I believe both EloquentBuilder and QueryBuilder are both macroable but you cant macro/overwrite existing functions afaik.