Permalink
Browse files

Fix for DC-797, 1-to-1 (or-none) hydrated dirty

Don't consider a change from null to Doctrine_Null to be a value
modification
  • Loading branch information...
1 parent 1ccee08 commit 93f01b7c81997b1e87100bfb4e0f7cebcce40ee8 @dominics dominics committed with jwage Sep 16, 2010
Showing with 4 additions and 0 deletions.
  1. +4 −0 lib/Doctrine/Record.php
View
@@ -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)) {

3 comments on commit 93f01b7

Owner

jwage replied Dec 8, 2010

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

Contributor

dominics replied Dec 10, 2010

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.

Contributor

dominics replied Dec 10, 2010

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.