Hotfix/5640 for bug in nested Zend\Form\Element\Collection::extract() recursion #5890

Closed
wants to merge 7 commits into
from

4 participants

@leogr

This PR fixes a problem in "Recursively extract and populate values for nested fieldsets" for the use case of nested collections as described by #5640

Maybe this is not the definitive solution, but it includes a test for nested collections and a fix that solves the problem without breaking any other tested behaviours.

@samsonasik samsonasik commented on an outdated diff Mar 1, 2014
tests/ZendTest/Form/TestAsset/Entity/Phone.php
@@ -0,0 +1,36 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
@samsonasik
samsonasik added a line comment Mar 1, 2014

now 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik commented on an outdated diff Mar 1, 2014
tests/ZendTest/Form/TestAsset/PhoneFieldset.php
@@ -0,0 +1,35 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
@samsonasik
samsonasik added a line comment Mar 1, 2014

now 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@samsonasik samsonasik and 2 others commented on an outdated diff Mar 1, 2014
tests/ZendTest/Form/TestAsset/Entity/Address.php
@@ -57,4 +62,22 @@ public function getCity()
{
return $this->city;
}
+
+ /**
+ * @param array $phones
+ * @return Address
@samsonasik
samsonasik added a line comment Mar 1, 2014

@return self

@leogr
leogr added a line comment Mar 1, 2014

Are you sure about that?
I didn't find other @return self in codebase

@samsonasik
samsonasik added a line comment Mar 1, 2014

please find in files

@Maks3w
Zend Framework member
Maks3w added a line comment Mar 1, 2014

@samsonasik Instead return self is better to say return $this. However this is not an issue need to fix. Address is as good type as any other so there is no need to fix this things

@leogr
leogr added a line comment Mar 1, 2014

@Maks3w I already changed the doc: leogr/zf2@6f6f80d
Let me know if I've to revert to @return Address

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@weierophinney weierophinney added this to the 2.2.6 milestone Mar 3, 2014
@Ocramius
Zend Framework member

This PR requires a rebase

@Ocramius Ocramius added the Form label Mar 4, 2014
@leogr

Rebase done.

I noticed that now my fix is no more needed, so I left just the test case as described in #5640

@Ocramius : is it ok?

@danizord

👍

@weierophinney weierophinney self-assigned this Mar 4, 2014
@weierophinney weierophinney added a commit that closed this pull request Mar 4, 2014
@weierophinney weierophinney Merge branch 'hotfix/5890'
Close #5890
71c0bbf
@weierophinney weierophinney added a commit that referenced this pull request Mar 4, 2014
@weierophinney weierophinney Merge branch 'hotfix/5890' into develop
Forward port #5890
dcd99ba
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment