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

Fix #603 #604

Closed
wants to merge 9 commits into from
Closed

Fix #603 #604

wants to merge 9 commits into from

Conversation

marcj
Copy link
Member

@marcj marcj commented Feb 15, 2013

Anything is explained in #603

MArcJ added 3 commits February 15, 2013 00:47
TestSuite output:

    $ phpunit test/testsuite/misc/MoreRelationTest.php
    PHPUnit 3.7.13 by Sebastian Bergmann.

    Configuration read from /Users/marc/Propel/phpunit.xml.dist

    F

    Time: 0 seconds, Memory: 18.75Mb

    There was 1 failure:

    1) MoreRelationTest::testContentsDeletion
    We assigned a collection of only one item.
    Failed asserting that 4 matches expected 1.

    /Users/marc/Propel/test/testsuite/misc/MoreRelationTest.php:97

    FAILURES!
    Tests: 1, Assertions: 2, Failures: 1.
Added a test set that proves the bug.
@marcj
Copy link
Member Author

marcj commented Feb 15, 2013

Wait, please not merge yet. Have had a dream with another scenario where this fails too. Am trying to reproduce it later on the pc.

@@ -4090,11 +4104,13 @@ protected function addRefFkScheduledForDeletion(&$script, $refFK)
$queryClassName = $this->getNewStubQueryBuilder($refFK->getTable())->getClassname();

$localColumn = $refFK->getLocalColumn();
$localColumn->isNotNull();
Copy link
Member

Choose a reason for hiding this comment

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

value is never assigned

…ryKeys` at class ForeignKey

and use it in the PHP5ObjectBuilder to check whether we have to backup the data of the relation entries
to be able to manipulate it later.
Added these methods because `isForeignPrimaryKey` returns only true if _all_ foreign columns are pks,
which is not useful in this case.
$this->assertEquals(1, $count, 'We assigned a collection of only one item.');

}

Copy link
Contributor

Choose a reason for hiding this comment

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

useless blank lines

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, not done yet.

@willdurand
Copy link
Contributor

let me know when it's ok.

MArcJ added 2 commits February 15, 2013 20:56
@marcj
Copy link
Member Author

marcj commented Feb 15, 2013

Done.

@willdurand
Copy link
Contributor

Could you rebase?

@willdurand
Copy link
Contributor

Hum, yeah, I meant "squash", not rebase...

@marcj marcj closed this Feb 15, 2013
@marcj
Copy link
Member Author

marcj commented Feb 15, 2013

Sorry, destroyed my branch. Git rebase hates me ;o) Created a new PR.

*/
public function __clone()
{
foreach ($this as $key => $obj) {
Copy link
Member

Choose a reason for hiding this comment

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

Will this also make sense for the onDemand collections, or should this be overwritten and implemented differently for those? I would suppose a ondemand collection would hydrate all rows and therefore would no longer be lazy loaded,..?

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