Framework: [7.x] Bug: BelongsTo relation returns old relation when associate() is called with model ID

[mpyw]

• Laravel Version: 6.6
• PHP Version: 7.3

Description:

• BelongsTo relation returns old relation when associate() is called with model ID · Issue #26401 · laravel/framework

This issue is almost the same as previously submitted #26401 (@joostbaptist), but reports incomplete bug fixes.

The argument can be checked with isDirty() when the relation corresponding to the synced key is loaded, but it will fail if the relation corresponding to the dirty key is loaded.

Steps To Reproduce:

$user = User::find(1);

$post = new Post();
$post->user()->associate(2);
$post->save();

$post->user()->associate($user); // dirty
$post->user()->associate(2); // clean

assert($user !== $post->user); // fails

Patch Proposal:

     /**
      * Associate the model instance to the given parent.
      *
      * @param  \Illuminate\Database\Eloquent\Model|int|string  $model
      * @return \Illuminate\Database\Eloquent\Model
      */
     public function associate($model)
     {
         $ownerKey = $model instanceof Model ? $model->getAttribute($this->ownerKey) : $model;

         $this->child->setAttribute($this->foreignKey, $ownerKey);

         if ($model instanceof Model) {
             $this->child->setRelation($this->relationName, $model);
-        } elseif ($this->child->isDirty($this->foreignKey)) {
+        } elseif (
+            $this->child->relationLoaded($this->relationName)
+            && optional($this->child->getRelationValue($this->relationName))->getKey() !== $ownerKey
+        ) {
             $this->child->unsetRelation($this->relationName);
         }

         return $this->child;
     }

[driesvints]

Thanks for reporting. Feel free to send in a PR with a test.

[mpyw]

@driesvints

optional($this->child->getRelationValue($this->relationName))->getKey()

I'm pretty anxious whether the statement is really appropriate. How do you think?

(CC: @taylorotwell @joostbaptist @staudenmeir)

[staudenmeir]

I think it would be easier to remove the key comparison and just always unset the relation when associate() is called with an ID.

[mpyw]

@staudenmeir Good idea

[mpyw]

@driesvints Should we target 7.x?