Skip to content

PHPORM-211 Fix unsetting property in embedded model #3052

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 19, 2024
Merged

PHPORM-211 Fix unsetting property in embedded model #3052

Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ All notable changes to this project will be documented in this file.
* Add `Connection::getServerVersion()` by @GromNaN in [#3043](https://github.com/mongodb/laravel-mongodb/pull/3043)
* 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 [#3002](https://github.com/mongodb/laravel-mongodb/pull/3001)
* 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

Expand Down
9 changes: 8 additions & 1 deletion src/Relations/EmbedsOneOrMany.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
45 changes: 32 additions & 13 deletions tests/EmbeddedRelationsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()
Expand Down Expand Up @@ -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->assertSame(null, $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->assertSame(null, $user->addresses->get(0)->city);
$this->assertSame('Kyoto', $user->addresses->get(1)->city);
}
}
Loading