Framework: Syncing currently attached relationships updates the updated_at timestamp even if the data has not changed

Created on 12 Nov 2019  路  10Comments  路  Source: laravel/framework

  • Laravel Version: 6.5.1

Description:

When syncing a BelongsToMany (using ->sync()) with a custom pivot class, the pivot table updated_at timestamp is being updated even if the data has not changed.

I believe the issue may come from the following lines, within Illuminate\Database\Eloquent\Relations\Concerns\InteractsWithPivotTable, and the updateExistingPivotUsingCustomClass method.

https://github.com/laravel/framework/blob/6.x/src/Illuminate/Database/Eloquent/Relations/Concerns/InteractsWithPivotTable.php#L221-L244

$updated = $pivot ? $pivot->fill($attributes)->isDirty() : false;

$pivot = $this->newPivot([
    $this->foreignPivotKey => $this->parent->{$this->parentKey},
    $this->relatedPivotKey => $this->parseId($id),
], true);

$pivot->timestamps = in_array($this->updatedAt(), $this->pivotColumns);

Specifically $pivot->timestamps, which is detecting that the pivot uses timestamps and therefore adjusting the updated_at column value. Should it not take into account if the currently attached pivot has been modified?

This could be achieved by changing:

$pivot->timestamps = in_array($this->updatedAt(), $this->pivotColumns);

to:

$pivot->timestamps = $updated && in_array($this->updatedAt(), $this->pivotColumns);
needs more info

Most helpful comment

It makes sense to me that the association is updated after calling sync() method.
As documentation states:

Any IDs that are not in the given array will be removed from the intermediate table.

If you need to know when the association has been created you can probably use created_at instead of updated_at.

All 10 comments

I'm actually not sure tbh. Maybe it's best that we gather some more opinions before marking this as a bug.

@staudenmeir do you have any idea maybe?

Sure yeah. I've written a short example to try and help out better understand.

An Item model has many Fields, with the relationship $item->fields(). A pivot table field_item holds the field_id, item_id and value (as well as timestamps updated_at and created_at). A user can then assign fields to items and hold a record of when those individual fields against each item were last modified. A custom pivot model FieldValue is used:

public function fields() : BelongsToMany
{
          return $this->belongsToMany(Field::class)
                              ->withTimestamps()
                              ->withPivot('value')
                              ->using(FieldValue::class);
}

An item and field are created:

$item = Item::create();

$nameField = Field::create([
    'name' => 'name',
    'type' => 'string',
]);

$item->fields()->sync([
    $nameField->id => ['value' => 'Foo bar item'],
]);

This would create an item and a field, and then associate that field to the item, with a value of "Foo bar item" as the "name".

If the item was then updated (let's say through an API or CRUD style backend), the ->sync() would run again - say other data attached to the item has prompted the update but the field name value hasn't changed (but is still posted with the update), i.e.

$item->fields()->sync([
    1 => ['value' => 'Foo bar item'],
]);

As it currently stands, this appears to increment the updated_at timestamp on the pivot entry for the item/field combination, despite the value not actually changing.

It should only update the updated_at timestamp on the pivot, if the name was changed - i.e. say the following was ran:

$item->fields()->sync([
    1 => ['value' => 'A different name for the item'],
]);

Hope that is of some use 馃槉

/cc @ralphschindler

It makes sense to me that the association is updated after calling sync() method.
As documentation states:

Any IDs that are not in the given array will be removed from the intermediate table.

If you need to know when the association has been created you can probably use created_at instead of updated_at.

@ralphschindler can you maybe weigh in here if you have a minute? thanks :)

I just ran into this issue after upgrading from 6.4 to 6.16. I have a table of events that only uses created_at (events are never modified, thus no need for updated_at). Events belong to users so they are add via attach. When attaching users to events, this property on the Event model is seemingly ignored in the corresponding query:

const UPDATED_AT = null;

I'm using this workaround until the issue is resolved or there is further clarification on how this should be more appropriately handled:

    public function setUpdatedAt($value)
    {
       // do nothing
    }

Is it better practice to set $timestamps = false; and just handle created_at without the convenience of Laravel's automatic timestamp management?

@moraleslevi yes:

Screenshot 2020-04-09 at 15 18 16

PR for this was merged.

@timrspratt btw this should also already be fixed in 7.x so can you try it out on Laravel 7?

Thanks @driesvints , I can confirm it's working 馃槉

Was this page helpful?
0 / 5 - 0 ratings

Related issues

felixsanz picture felixsanz  路  3Comments

kerbylav picture kerbylav  路  3Comments

Fuzzyma picture Fuzzyma  路  3Comments

JamborJan picture JamborJan  路  3Comments

shopblocks picture shopblocks  路  3Comments