laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 31 forks source link

Upsert for related models #2666

Open kohenkatz opened 2 years ago

kohenkatz commented 2 years ago

The documentation for Inserting & Updating Related Models includes multiple ways to insert related models, including save, saveMany, create, createMany, findOrNew, firstOrNew, firstOrCreate, and updateOrCreate. It then includes a link to the documentation about Upserts. However, upserts are not actually properly handled, as in this example:

class Feature extends Model
{
    public function statuses()
    {
        return $this->hasMany(Status::class);
    }
}

class Status extends Model
{
    public function feature()
    {
        return $this->belongsTo(Feature::class);
    }
}
// Create features and then insert their relevant statuses
<?php

$features = [
    ['name' => 'feature_a', 'friendly_name' => 'Feature A', 'description' => 'User can do A'],
    ['name' => 'feature_b', 'friendly_name' => 'Feature B', 'description' => 'User can do B'],
    ['name' => 'feature_c', 'friendly_name' => 'Feature C', 'description' => 'User can do C'],
];

Feature::upsert($features, ['name'], ['friendly_name', 'description']);

$statuses = [
    'feature_a' => [
        ['name' => 's_a1', 'friendly_name' => 'Status A-1', 'is_emergency' => false, 'is_for_location' => true, 'is_for_user' => false],
        ['name' => 's_a2', 'friendly_name' => 'Status A-2', 'is_emergency' => false, 'is_for_location' => true, 'is_for_user' => false],
    ],
    'feature_b' => [
        ['name' => 's_b1', 'friendly_name' => 'Status B-1', 'is_emergency' => false, 'is_for_location' => true, 'is_for_user' => false],
        ['name' => 's_b2', 'friendly_name' => 'Status B-2', 'is_emergency' => true,  'is_for_location' => true, 'is_for_user' => false],
    ],
    'feature_c' => [
        ['name' => 's_c', 'friendly_name' => 'Status C', 'is_emergency' => true, 'is_for_location' => false, 'is_for_user' => true],
    ],
];

$features = Feature::all();

foreach ($features as $feature) {
    $feature->statuses()->upsert($statuses[$feature->name], ['name'], ['friendly_name', 'is_emergency', 'is_for_location', 'is_for_user']);
}

When this code runs, it inserts all the rows into the statuses table with the feature_id set to null instead of being set to the correct ID value of the Feature.

Here is my current workaround:

foreach ($features as $feature) {
    $statusList = $statuses[$feature->name];
    array_walk($statusList, fn(&$s) => $s['feature_id'] = $feature->id);
    Status::upsert($statusList, ['name'], ['feature_id', 'friendly_name', 'is_emergency', 'is_for_location', 'is_for_user']);
}

I have a number of places where I want to upsert related models, and this pattern gets ugly fast.

I think I can do a PR to add this feature to upsert similar to how it is already done for updateOrCreate (using $this->getForeignKeyName() and $this->getParentKey()), but I wanted to get feedback before I start to see if that will be appreciated.