Skip to content
This repository has been archived by the owner on Nov 15, 2019. It is now read-only.

Commit

Permalink
Fix for DC-797, 1-to-1 (or-none) hydrated dirty
Browse files Browse the repository at this point in the history
Don't consider a change from null to Doctrine_Null to be a value
modification
  • Loading branch information
dominics authored and jwage committed Sep 17, 2010
1 parent 1ccee08 commit 93f01b7
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions lib/Doctrine/Record.php
Original file line number Diff line number Diff line change
Expand Up @@ -1537,6 +1537,10 @@ protected function _isValueModified($type, $old, $new)
return true;
}

if (($old === null || $old instanceof Doctrine_Null) && ($new == null || $new instanceof Doctrine_Null)) {
return false;
}

if ($type == 'boolean' && (is_bool($old) || is_numeric($old)) && (is_bool($new) || is_numeric($new)) && $old == $new) {
return false;
} else if (in_array($type, array('decimal', 'float')) && is_numeric($old) && is_numeric($new)) {
Expand Down

3 comments on commit 93f01b7

@jwage
Copy link
Member

@jwage jwage commented on 93f01b7 Dec 8, 2010

Choose a reason for hiding this comment

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

I have to revert this commit as it actually causes the test suite to fail.

@dominics
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like the rest of my pull request wasn't committed:
#8

One test is specifically addressed in the pull request:
https://github.com/dominics/doctrine1/commit/d6260c06283e61f14d2c3eb615ab98385bdddb05

And I wouldn't expect the whole suite to pass without this one too:
https://github.com/dominics/doctrine1/commit/754ca890faf76f87b5a3fe6707ba497099dfe314

The rationale for those changes is elaborated in their commit messages.

If any other tests are failing with those changes applied, I'd be happy to take another look.

@dominics
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, and more importantly, this commit is incorrect. This change is the correct one, which is the one attached to my topic branch pull request:
https://github.com/dominics/doctrine1/commit/9e08e27708c2e7d31f749a8731e656452ff3921c

NB: === versus == in the second half of the conditional

Please sign in to comment.