Skip to content

PHPORM-211 Fix unsetting property in embedded model #3052

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Jul 19, 2024
Merged

PHPORM-211 Fix unsetting property in embedded model #3052

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
38d89e7
PHPORM-211 Fix unsetting property in embedded model
GromNaN Jul 18, 2024
1c1c06f
Apply suggestions from code review
GromNaN Jul 19, 2024
fc2c251
Merge branch '4.7' into PHPORM-211
GromNaN Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.
* Add `Schema\Builder::getTables()` and `getTableListing()` by @GromNaN in [#3044](https://github.com/mongodb/laravel-mongodb/pull/3044)
* Add `Schema\Builder::getColumns()` and `getIndexes()` by @GromNaN in [#3045](https://github.com/mongodb/laravel-mongodb/pull/3045)
* Add `Schema\Builder::hasColumn` and `hasColumns` method by @Alex-Belyi in [#3001](https://github.com/mongodb/laravel-mongodb/pull/3001)
* Fix unsetting a field in an embedded model by @GromNaN in [#3052](https://github.com/mongodb/laravel-mongodb/pull/3052)

## [4.6.0] - 2024-07-09

9 changes: 8 additions & 1 deletion src/Relations/EmbedsOneOrMany.php
View file
Original file line number Diff line number Diff line change
@@ -15,8 +15,10 @@
use Throwable;

use function array_merge;
use function assert;
use function count;
use function is_array;
use function str_starts_with;
use function throw_if;

abstract class EmbedsOneOrMany extends Relation
@@ -392,7 +394,12 @@ public static function getUpdateValues($array, $prepend = '')
$results = [];

foreach ($array as $key => $value) {
$results[$prepend . $key] = $value;
if (str_starts_with($key, '$')) {
assert(is_array($value), 'Update operator value must be an array.');
$results[$key] = static::getUpdateValues($value, $prepend);
} else {
$results[$prepend . $key] = $value;
}
Comment on lines +397 to +402
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before, the incorrect update was:

{
  $set: {
    "address.$.updated_at": Date(...),
    "address.$.$unset": {
      "city": true,
    }
   }
}

After, it becomes:

{
  "$unset": {
    "address.$.city": true,
  },
  "$set": {
    "address.$.updated_at": Date(...),
  }
}

Field names starting with a $ are not supported, that's why we are sure there will not be 2 levels of recursion.

}

return $results;
45 changes: 32 additions & 13 deletions tests/EmbeddedRelationsTest.php
View file
Original file line number Diff line number Diff line change
@@ -10,12 +10,6 @@
use Mockery;
use MongoDB\BSON\ObjectId;
use MongoDB\Laravel\Tests\Models\Address;
use MongoDB\Laravel\Tests\Models\Book;
use MongoDB\Laravel\Tests\Models\Client;
use MongoDB\Laravel\Tests\Models\Group;
use MongoDB\Laravel\Tests\Models\Item;
use MongoDB\Laravel\Tests\Models\Photo;
use MongoDB\Laravel\Tests\Models\Role;
use MongoDB\Laravel\Tests\Models\User;

use function array_merge;
@@ -25,14 +19,7 @@ class EmbeddedRelationsTest extends TestCase
public function tearDown(): void
{
Mockery::close();

User::truncate();
Book::truncate();
Item::truncate();
Role::truncate();
Client::truncate();
Group::truncate();
Photo::truncate();
Comment on lines -30 to -35
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side change: This models are not used in this test class.

}

public function testEmbedsManySave()
@@ -951,4 +938,36 @@ public function testGetQueueableRelationsEmbedsOne()
$this->assertEquals(['father'], $user->getQueueableRelations());
$this->assertEquals([], $user->father->getQueueableRelations());
}

public function testUnsetPropertyOnEmbed()
{
$user = User::create(['name' => 'John Doe']);
$user->addresses()->save(new Address(['city' => 'New York']));
$user->addresses()->save(new Address(['city' => 'Tokyo']));

// Set property
$user->addresses->first()->city = 'Paris';
$user->addresses->first()->save();

$user = User::where('name', 'John Doe')->first();
$this->assertSame('Paris', $user->addresses->get(0)->city);
$this->assertSame('Tokyo', $user->addresses->get(1)->city);

// Unset property
unset($user->addresses->first()->city);
$user->addresses->first()->save();

$user = User::where('name', 'John Doe')->first();
$this->assertNull($user->addresses->get(0)->city);
$this->assertSame('Tokyo', $user->addresses->get(1)->city);

// Unset and reset property
unset($user->addresses->get(1)->city);
$user->addresses->get(1)->city = 'Kyoto';
$user->addresses->get(1)->save();

$user = User::where('name', 'John Doe')->first();
$this->assertNull($user->addresses->get(0)->city);
$this->assertSame('Kyoto', $user->addresses->get(1)->city);
}
}
Loading