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

Fix for DC-797 #7

Closed
wants to merge 6 commits into from
Closed

Fix for DC-797 #7

wants to merge 6 commits into from

Conversation

dominics
Copy link
Contributor

The Record's _isValueModified method was considering a change from null to Doctrine_Null as a modification. But the hydrator sets null fields to Doctrine_Null on objects that have an empty one-to-one relation. The end result was records being hydrated as dirty, before any modifications by the user.

More details, and a test case, in the bug report.

Don't consider a change from null to Doctrine_Null to be a value
modification
Setting Translation properties doesn't seem to mark a record as
modified. So, this test case previously marked the Place as TDIRTY, so
that it would be saved. However, this test case didn't set the Sura to
TDIRTY.

Now that DC-797 is fixed, and the Sura is no longer marked as TDIRTY
by setting a related Place, the state must be set on both the Sura
and the Place.
@dominics
Copy link
Contributor Author

The failing test turns out to be an interesting case where it looks like the user is required to set the state of the record to dirty themselves before calling save(). And they do on one of the records in the test, but not the other. So one of the records isn't saved, now that the record isn't dirty to start with.

I've fixed that test, and also added another to test the bug these changes fix.

Currently, count(new Doctrine_Null()) returns 1. This means
Doctrine_Hydrator_RecordDriver::setLastElement() will try to call
getLast() on instances of Doctrine_Null and fail.

Compare this to count(null), which gives 0: if Doctrine_Null is
to represent null it should give the same count.
@dominics
Copy link
Contributor Author

Going to do this on a topic branch.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant