From d8d34e5007ee38a8c66e77debfb6e847713fe44a Mon Sep 17 00:00:00 2001 From: wakqasahmed Date: Fri, 4 Sep 2026 17:19:01 +0200 Subject: [PATCH 1/2] Fire model.relation.beforeAdd/afterAdd events from AttachOneOrMany::create() create() built and persisted the related model directly via parent::create(), bypassing add() entirely whenever no sessionKey was given (the immediate, non-deferred-binding path). add() is what fires model.relation.beforeAdd/afterAdd and handles the single-attachment sibling deletion for AttachOne, so both silently never ran when using $model->relation()->create(...) directly instead of ->add(). Route create() through add() in both cases: build an unsaved model via newInstance() when sessionKey is null so add()'s own $model->save() is the only persist, keeping parent::create() for the deferred-binding path where the model must exist immediately. --- .../Relations/Concerns/AttachOneOrMany.php | 13 ++---- tests/Database/Relations/AttachManyTest.php | 41 ++++++++++++++++++ tests/Database/Relations/AttachOneTest.php | 42 +++++++++++++++++++ 3 files changed, 87 insertions(+), 9 deletions(-) diff --git a/src/Database/Relations/Concerns/AttachOneOrMany.php b/src/Database/Relations/Concerns/AttachOneOrMany.php index ff07ae3be..be5f4f61e 100644 --- a/src/Database/Relations/Concerns/AttachOneOrMany.php +++ b/src/Database/Relations/Concerns/AttachOneOrMany.php @@ -150,22 +150,17 @@ public function save(Model $model, $sessionKey = null) */ public function create(array $attributes = [], $sessionKey = null) { - // Delete siblings for single attachments - if ($sessionKey === null && $this instanceof AttachOne) { - $this->delete(); - } - if (!array_key_exists('is_public', $attributes)) { $attributes = array_merge(['is_public' => $this->isPublic()], $attributes); } $attributes['field'] = $this->fieldName; - $model = parent::create($attributes); + $model = $sessionKey === null + ? $this->related->newInstance($attributes) + : parent::create($attributes); - if ($sessionKey !== null) { - $this->add($model, $sessionKey); - } + $this->add($model, $sessionKey); return $model; } diff --git a/tests/Database/Relations/AttachManyTest.php b/tests/Database/Relations/AttachManyTest.php index ae3e75923..701c10484 100644 --- a/tests/Database/Relations/AttachManyTest.php +++ b/tests/Database/Relations/AttachManyTest.php @@ -82,4 +82,45 @@ public function testDeleteFlagDeleteModelLaravelRelation() $user->delete(); $this->assertNull(File::find($photoId)); } + + public function testCreateFiresRelationEvents() + { + Model::unguard(); + $user = User::create(['name' => 'Stevie', 'email' => 'stevie@example.com']); + Model::reguard(); + + $beforeAddCalls = []; + $afterAddCalls = []; + $user->bindEvent('model.relation.beforeAdd', function ($relationName, $relatedModel) use (&$beforeAddCalls) { + $beforeAddCalls[] = [$relationName, $relatedModel]; + }); + $user->bindEvent('model.relation.afterAdd', function ($relationName, $relatedModel) use (&$afterAddCalls) { + $afterAddCalls[] = [$relationName, $relatedModel]; + }); + + $photo = $user->photos()->create(['data' => dirname(dirname(__DIR__)) . '/fixtures/attach/avatar.png']); + + $this->assertCount(1, $beforeAddCalls); + $this->assertSame('photos', $beforeAddCalls[0][0]); + $this->assertTrue($photo->is($beforeAddCalls[0][1])); + + $this->assertCount(1, $afterAddCalls); + $this->assertSame('photos', $afterAddCalls[0][0]); + $this->assertTrue($photo->is($afterAddCalls[0][1])); + } + + public function testCreateDoesNotReplaceExistingAttachments() + { + Model::unguard(); + $user = User::create(['name' => 'Stevie', 'email' => 'stevie@example.com']); + Model::reguard(); + + $first = $user->photos()->create(['data' => dirname(dirname(__DIR__)) . '/fixtures/attach/avatar.png']); + $second = $user->photos()->create(['data' => dirname(dirname(__DIR__)) . '/fixtures/attach/avatar.png']); + + $this->assertNotNull(File::find($first->id)); + $this->assertNotNull(File::find($second->id)); + $user->reloadRelations(); + $this->assertCount(2, $user->photos); + } } diff --git a/tests/Database/Relations/AttachOneTest.php b/tests/Database/Relations/AttachOneTest.php index 2ed5e47d0..9e5bf3afc 100644 --- a/tests/Database/Relations/AttachOneTest.php +++ b/tests/Database/Relations/AttachOneTest.php @@ -176,4 +176,46 @@ public function testDeleteFlagSoftDeleteModel() $user->delete(); $this->assertNotNull(File::find($avatarId)); } + + public function testCreateFiresRelationEvents() + { + Model::unguard(); + $user = User::create(['name' => 'Stevie', 'email' => 'stevie@example.com']); + Model::reguard(); + + $beforeAddCalls = []; + $afterAddCalls = []; + $user->bindEvent('model.relation.beforeAdd', function ($relationName, $relatedModel) use (&$beforeAddCalls) { + $beforeAddCalls[] = [$relationName, $relatedModel]; + }); + $user->bindEvent('model.relation.afterAdd', function ($relationName, $relatedModel) use (&$afterAddCalls) { + $afterAddCalls[] = [$relationName, $relatedModel]; + }); + + $avatar = $user->avatar()->create(['data' => dirname(dirname(__DIR__)) . '/fixtures/attach/avatar.png']); + + $this->assertCount(1, $beforeAddCalls); + $this->assertSame('avatar', $beforeAddCalls[0][0]); + $this->assertTrue($avatar->is($beforeAddCalls[0][1])); + + $this->assertCount(1, $afterAddCalls); + $this->assertSame('avatar', $afterAddCalls[0][0]); + $this->assertTrue($avatar->is($afterAddCalls[0][1])); + } + + public function testCreateReplacesExistingSingleAttachment() + { + Model::unguard(); + $user = User::create(['name' => 'Stevie', 'email' => 'stevie@example.com']); + Model::reguard(); + + $first = $user->avatar()->create(['data' => dirname(dirname(__DIR__)) . '/fixtures/attach/avatar.png']); + $firstId = $first->id; + + $second = $user->avatar()->create(['data' => dirname(dirname(__DIR__)) . '/fixtures/attach/avatar.png']); + + $this->assertNull(File::find($firstId)); + $this->assertNotNull(File::find($second->id)); + $this->assertNotSame($firstId, $second->id); + } } From 347e6f5b1ff96ccb3a3e2e014daf392e7c163093 Mon Sep 17 00:00:00 2001 From: wakqasahmed Date: Fri, 4 Sep 2026 20:40:26 +0200 Subject: [PATCH 2/2] phpstan: correct expected-occurrence count for AttachOne::delete() to 2 create() no longer has its own direct call to the parent's private delete() (per the prior commit, it now routes through add(), which already had one) -- the baseline's expected-count for this message was never updated to match, so PHPStan silently over-tolerated one fewer occurrence than actually exists. Verified via a clean --memory-limit=1G analyse run with zero errors at count: 2. --- phpstan-baseline.neon | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 0e626dc55..89be28d34 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -177,7 +177,7 @@ parameters: - message: "#^Call to private method delete\\(\\) of parent class Illuminate\\\\Database\\\\Eloquent\\\\Relations\\\\MorphOne\\\\.$#" - count: 3 + count: 2 path: src/Database/Relations/AttachOne.php -