Skip to content
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

PHPORM-211 Fix unsetting property in embedded model #3052

Merged
merged 3 commits into from
Jul 19, 2024

Conversation

GromNaN
Copy link
Member

@GromNaN GromNaN commented Jul 18, 2024

Fix PHPORM-211

Checklist

  • Add tests and ensure they pass
  • Add an entry to the CHANGELOG.md file
  • Update documentation for new features

Comment on lines +397 to +402
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;
}
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.

Comment on lines -30 to -35
Book::truncate();
Item::truncate();
Role::truncate();
Client::truncate();
Group::truncate();
Photo::truncate();
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.

@GromNaN GromNaN marked this pull request as ready for review July 18, 2024 09:04
@GromNaN GromNaN requested a review from a team as a code owner July 18, 2024 09:04
@GromNaN GromNaN requested a review from alcaeus July 18, 2024 09:04
@GromNaN GromNaN added the fixed label Jul 18, 2024
Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

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

Nit about the assertion method, but LGTM other than that!

Co-authored-by: Andreas Braun <git@alcaeus.org>
@GromNaN GromNaN requested a review from alcaeus July 19, 2024 08:04
@GromNaN GromNaN merged commit f0716ab into mongodb:4.7 Jul 19, 2024
26 checks passed
@GromNaN GromNaN deleted the PHPORM-211 branch July 19, 2024 08:43
rustagir pushed a commit to rustagir/laravel-mongodb that referenced this pull request Jul 23, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants