Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[Form] [FormElementManager] don't overwrite form factory if already set #3735

Merged
merged 1 commit into from Feb 19, 2013

Conversation

Projects
None yet
3 participants
Contributor

radnan commented Feb 8, 2013

Don't overwrite factory in case other initializers are called beforehand that set the form factory, or attach an input filter factory to it.

Member

ralphschindler commented Feb 15, 2013

Unit test?

@ghost ghost assigned ralphschindler Feb 15, 2013

@weierophinney weierophinney commented on the diff Feb 19, 2013

library/Zend/Form/FormElementManager.php
@@ -84,7 +84,7 @@ public function __construct(ConfigInterface $configuration = null)
public function injectFactory($element)
{
if ($element instanceof FormFactoryAwareInterface) {
- $element->setFormFactory(new Factory($this));
+ $element->getFormFactory()->setFormElementManager($this);
@weierophinney

weierophinney Feb 19, 2013

Owner

These are actually synonymous -- getFormFactory() will create a form factory instance internally. The old code is actually more performant than your change as it bypasses the getter and immediately sets a new instance that has the element manager composed. (The reason no unit tests fail currently is because we actually have a test that verifies that the form element manager is identical in the two objects.)

Can you indicate why you would want to alter this? A unit test detailing the use case would make the most sense, as right now, I'm unclear what the purpose of the change is.

@weierophinney

weierophinney Feb 19, 2013

Owner

Actually, your subject line details it: you want to inject the element manager into an existing factory, if present.

@ghost ghost assigned weierophinney Feb 19, 2013

weierophinney added a commit that referenced this pull request Feb 19, 2013

[#3735] Added unit test
- Test demonstrates that factory composed by object implementing
  FormFactoryAwareInterface is not overwritten, but *is* injected with
  FormElementManager instance.

weierophinney added a commit that referenced this pull request Feb 19, 2013

@weierophinney weierophinney merged commit 6ce846e into zendframework:develop Feb 19, 2013

1 check passed

default The Travis build passed
Details

weierophinney added a commit that referenced this pull request Feb 19, 2013

Owner

weierophinney commented Feb 19, 2013

Added a unit test on merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment