FormHelper->inputs() does not work with form created with Plugin syntax,... #1096

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

@krolow
Contributor
krolow commented Jan 29, 2013

Ticket #3571

I`m not sure if the tests were well written, please let me know anything so I can improve...

@dereuromark
Member

You used spaces instead of strings as indentation (see the diff of files changed). You need to fix that :)

@krolow
Contributor
krolow commented Jan 29, 2013

Okay, I pass the phpcs and fixed the problem with the indentation.

@dereuromark dereuromark commented on the diff Jan 29, 2013
lib/Cake/View/Helper/FormHelper.php
@@ -329,6 +329,9 @@ public function create($model = null, $options = array()) {
$key = null;
if ($model !== false) {
+ if (strpos($model, '.') !== false) {
+ $model = str_replace('.', '', $model);
+ }
@dereuromark
dereuromark Jan 29, 2013 Member

Why just replacing the dot? and not pluginSplit() here? In your case it results in "TestPluginPost". But I think the model name usually is just "Post". If the name was "TestPluginPost", the full syntax would have to be "TestPlugin.TestPluginPost" as input for your form.
To clarify: If your model was "Post" in your "TestPlugin" plugin, then your code would probably fail then.

@krolow
krolow Jan 29, 2013 Contributor

Actually I was not sure about that... seems that pass the Plugin.Something, for creates no longer works because Helper::setEntity.

Creates pass the string directly for setEntity, and as far I could see, setEntity not handles plugins...

So I was not exactly sure if I would change setEntity, or the "create" method..

as setEntity is used often to generate all the inputs fields I thought would make sense to not involve setEntity in the changes.

So, I added just that condition in create, as far I could check in docs, the param $model is able to receive Plugin.Model, so I just made that conditional to check if it was a plugin + model that was been passed...

About your suggestion, do you think would make sense to change the Helper::setEntity?

@dereuromark
dereuromark Jan 29, 2013 Member

Well, I would probably want to hear other opinions on this. Maybe I read it wrong.

@markstory
markstory Jan 29, 2013 Member

This is odd, I would have thought to use pluginSplit(). Just doing a str_replace will result in TestPluginPost as the modelname. This is seldom the actual key name in the ClassRegistry.

@dereuromark
dereuromark Jan 29, 2013 Member

Exactly what I have been thinking :)

@markstory markstory commented on the diff Jan 29, 2013
lib/Cake/Test/Case/View/Helper/FormHelperTest.php
+ */
+ public function testFormInputsPlugin() {
+ $this->loadFixtures('Post');
+ App::build(array(
+ 'Plugin' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS)
+ ));
+ CakePlugin::load('TestPlugin');
+ $this->Form->request['models'] = array('TestPluginPost' => array('plugin' => 'TestPlugin', 'className' => 'TestPluginPost'));
+ $this->Form->create('TestPlugin.Post');
+ $result = $this->Form->inputs();
+ $expected = array(
+ 'fieldset' => array(),
+ 'legend' => array(),
+ 'New Test Plugin Post',
+ '/legend',
+ 'input' => array('type' => 'hidden', 'name' => 'data[TestPluginPost][id]', 'id' => 'TestPluginPostId'),
@markstory
markstory Jan 29, 2013 Member

Shouldn't these all be data[Post][id] etc? This doesn't really make sense given the model name.

@krolow
krolow Jan 29, 2013 Contributor

I could not found one test, or something in the book to get the properly output...

Just to make sure so the tests would looks like:

        App::build(array(
            'Plugin' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS)
        ));
        $this->loadFixtures('Post');
        CakePlugin::load('TestPlugin');
        $this->Form->request['models'] = array('Post' => array('plugin' => 'TestPlugin', 'className' => 'Post'));
        $this->Form->create('TestPlugin.Post');
        $result = $this->Form->inputs();
        $expected = array(
            'fieldset' => array(),
            'legend' => array(),
            'New Post',
            '/legend',
            'input' => array('type' => 'hidden', 'name' => 'data[Post][id]', 'id' => 'PostId'),
            array('div' => array('class' => 'input select')),
            '*/div',
            array('div' => array('class' => 'input text')),
            '*/div',
            array('div' => array('class' => 'input textarea')),
            '*/div',
            array('div' => array('class' => 'input text')),
            '*/div',
            array('div' => array('class' => 'input datetime')),
            '*/div',
            array('div' => array('class' => 'input datetime')),
            '*/div',
            '/fieldset'
        );
        $this->assertTags($result, $expected);
@markstory
Member

I fixed this in 78b23d8

@markstory markstory closed this Jan 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment