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

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

Fixes changeset being empty when datetime change is sub second #2676

Merged
merged 7 commits into from
Sep 20, 2024

Conversation

magnetik
Copy link
Contributor

@magnetik magnetik commented Sep 17, 2024

Q A
Type fix
BC Break no
Fixed issues #2538

Summary

The ODM was ignoring changes when they are under a second (because of the cast as timestmap).

It might be a BC break because people can rely on the old behavior ?

@magnetik
Copy link
Contributor Author

magnetik commented Sep 18, 2024

(I do not have a local setup with mongo, so I'm sort of using the actions to test.. sorry about that)

edit : I just saw that I can enable them in my fork, i'll test there :)

@magnetik magnetik changed the title Update UnitOfWork.php Fixes changeset being empty when datetime change is sub second Sep 18, 2024
@magnetik magnetik marked this pull request as ready for review September 18, 2024 19:01
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.

LGTM with the PHPCS fix and suppression.

lib/Doctrine/ODM/MongoDB/UnitOfWork.php Show resolved Hide resolved
@magnetik
Copy link
Contributor Author

if you know how to fix the style issue, feel free to fix it directly there if you want to save some time to restart the checks later ^^

Thanks !

@malarzm
Copy link
Member

malarzm commented Sep 19, 2024

It goes like this:

// phpcs:ignore SlevomatCodingStandard.Operators.DisallowEqualOperators.DisallowedEqualOperator
if ($meta == unserialize(serialize($meta))) {

@alcaeus alcaeus added this to the 2.9.0 milestone Sep 20, 2024
@alcaeus alcaeus merged commit 3c9b1e8 into doctrine:2.9.x Sep 20, 2024
16 of 17 checks passed
@alcaeus
Copy link
Member

alcaeus commented Sep 20, 2024

Thanks @magnetik!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants