-
-
Notifications
You must be signed in to change notification settings - Fork 261
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
Fix "diff" result for dates and other objects #599
Fix "diff" result for dates and other objects #599
Conversation
… currently fail, because the strict comparison is used.
This is necessary because the strict comparison operator (===) will return false if the objects are not the same instance.
1678e06
to
283df18
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @befresh-mweimerskirch.
Could you please update the description telling how the method compareObjects()
is different from the loose comparison (==
) between objects?
{ | ||
$diff = new ArrayDiff(); | ||
|
||
$dateInstance1 = new \DateTimeImmutable('2014-01-01 00:00:00.000000'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, we should also add a test setting different time zones in the compared objects, in order to expose how the comparison behaves with different time zones using the same UTC offset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add that test case on Monday.
If I understood correctly, the compareObject is pretty the same than |
Co-authored-by: Javier Spagnoletti <[email protected]>
Added to the todo section of the PR. I'll do that on Monday.
No. When using the loose comparison operator on objects, the property values are also compared loosely. Imagine a class that has only a property "name". One object with name "TEST" and one object with name "test" as their respective value. Those two instances would be considered equal using the "==" operator. |
As promised, I added a test case with two dates that have different timezones. |
Thanks |
Subject
This pull request fixes an issue with the ArrayDiff class and consequentially the AuditReader.
AuditReader::diff currently shows dates (instances of DateTime or DateTimeImmutable) as changed even though they are the same.
This issue was introduced when the loose comparator ("==") was changed to the strict comparison ("===") in commit bf02fd4
This PR reverts the old behaviour for objects, while keeping the strict comparison for other types.
I also added tests. Some of the tests fail without the change, so I hope this illustrates the issue.
I am targeting this branch, because this is a fix that restores previous behaviour (and slightly improves it).
Changelog