Model and Web Forms

[klimov-paul]

This issue has been exposed at #213. Also similar problem was discussed for the Yii 1.1.x at yiisoft/yii#470.

Current Yii2 approach for the web forms breaks an MVC: it allows the model to specify its form name on its own.
“yii\base\Model::formName()” generates the key, which is used to create name for the form inputs and for the submitted data retrieval from “GET” or “POST”. You can call me paranoid, “holy var man” or else, but such functionality does NOT belong to the model layer in the MVC!
Although current implementation may seems harmless, it may (and, I think, will someday) lead to the problems, which do not reveal themselves at common situation.

First suggestion, which comes to my mind: introducing another class, which represents the web form at controller layer. Possible names are “web\Form”, “web\ModelHandler” etc. Such class can serve as wrapper around the model and should have methods “formName()” (currently - “yii\base\Model::formName()”) and “populate()” (currently – “yii\base\Controller”).
This class may perform:
– data sanitizing (like removing leading or trailing spaces)
– ensuring value collection from “difficult” HTML inputs, like “checkbox”, without any HTML hacks
– perform data format conversion from the model data format to the view data format (useful for the date/datetime inputs)

Possible controller code:

public function actionContact()
{
    $modelHandler = new ModelHandlerWeb(new ContactForm);
    if ($modelHandler->populate($_POST) && $modelHandler->model->contact(Yii::$app->params['adminEmail'])) {
        Yii::$app->session->setFlash('contactFormSubmitted');
        Yii::$app->response->refresh();
    } else {
        echo $this->render('contact', array(
            'model' => $modelHandler,
        ));
    }
}

“ActiveForm” widget can work with the model handler, just like “CListView” worked with data providers back in Yii 1.1.x.

Possible view code:

<?php $form = $this->beginWidget(ActiveForm::className(), array(
    'modelHandler' => $modelHandler,
    'options' => array('class' => 'form-horizontal'),
    'fieldConfig' => array('inputOptions' => array('class' => 'input-xlarge')),
)); ?>
    <?php echo $form->field('name')->textInput(); ?>
    <?php echo $form->field('email')->textInput(); ?>
    <?php echo $form->field('subject')->textInput(); ?>
    <?php echo $form->field('body')->textArea(array('rows' => 6)); ?>
    <div class="form-actions">
        <?php echo Html::submitButton('Submit', null, null, array('class' => 'btn btn-primary')); ?>
    </div>
<?php $this->endWidget(); ?> 

The approach I have proposed here is more flexible, although I admit it is a little heavier than the current one.

[qiangxue]

Ideally, model attributes and input field names should be decoupled, so do the so-called "form names". Theoretically, the decoupling could be done via an intermediate class (using some sort of name mapping) like you proposed.

Practically, the introduction of this additional class would confuse users and unnecessarily complicate the whole thing. So we are taking the practical approach by directly using attribute names as input names and model class names as the prefixes. In some uncommon cases when you want to use "hide" the class name and the attribute names from end users, you can do so by using formName() and customly defined model properties.

I don't see many drawbacks of having formName(). On the contrary, it solves several problems present in 1.1. I do see several drawbacks of introducing a separate class specifically for form processing purpose.

[yjeroen]

Can't you move the formName() functionality to the widget? This way, it's always set in the view file.

[creocoder]

@yjeroen What if you have 10 view files?

[klimov-paul]

Speaking about coupling, why do not move method “populate” from the controller to the model?
Method “yii\base\Controller::populate()” uses a lot of fields and methods from “yii\base\model” but does not use any internal field or method, this indicates method is not at his place. Currently it, of course, can process several models at once, but it is not a big deal.

[klimov-paul]

Can't you move the formName() functionality to the widget? This way, it's always set in the view file.

Form name can not belong to the widget. It is necessary for the data retrieve from GET or POST.

[qiangxue]

The populate() method is not finalized yet (it was the last addition before releasing public preview). Yes, will consider moving it to model.

[creocoder]

Vote against moving populate() from controller. From a practical point of view it would be clumsy using populate() from model!

@klimov-paul

The approach I have proposed here is more flexible, although I admit it is a little heavier than the current one.

A litte heavier??? No, it not only looks horrible it's also terribly impractical and long-winded.

[klimov-paul]

it would be clumsy using populate() from model!

Why? As far as we have "yii\base\Model::getFormName()", moving populate() inside the model looks logical.

No, it not only looks horrible it's also terribly impractical and long-winded.

I can see nothing horrible in my approach: it creates an entity, which represents the web form abstracting from model (or models) behind it inside the view. We are using Data Providers for the listings, why can not we use some providers for the forms?
Although, while there is none around, who consider it can be good idea, perhaps I am wrong indeed.

[creocoder]

@klimov-paul

Why? As far as we have "yii\base\Model::getFormName()", moving populate() inside the model looks logical.

Just compare usage on examples. Just compare. We always have controller instance and $this->populate() is very simple.

I can see nothing horrible in my approach

There is nothing horrible in approach, it just looks horribe in practice usage. But in principle, as always when choosing academic VS practical. My IMHO that academic is complex overcoding frameworks choose. Like zf2, sf2. When practice is Yii way.

[klimov-paul]

Comparing:

“populate” inside controller:

public function actionSome() {
    $model = new MyModel();
    $this->populate($_POST,  $model);
    ….
}

“populate” inside model:

public function actionSome() {
    $model = new MyModel();
    $model->populate($_POST);
    ….
}

I can not see benefit from the "Controller::populate" at this view.

[creocoder]

@klimov-paul In fact, I am now really with you to move populate() to Model since we use loop to create new models anyway. So seems we do not loose anything. Change my vote to move. Convienced. But in any case, need to think more about that. Maybe we loose when we already have $models array needed to populate.

[klimov-paul]

One final word about initial proposal:

To understand this issue and make a conclusion, you should decide what is “model”.

For me “model” is an entity, which holds the application business logic and nothing more. Model should be universal. It should be usable for any type of view or controller. Model should be able to serve both web application and console application.

I can’t see any purpose of “yii\base\Model::getFormName()” inside the console application. This method can serve only web application, web form, to be precise. That is why I do not like its presence there.
However, this method can not be moved to “yii\base\Controller”, because single controller can work with lot of different web forms.
So I suggest creation of an entity, which belongs “controller” layer and can consume such methods as “getFormName()” and “populate()”.

My opinion: model should be stripped from web, stripped from any view. Only then it can be considered as model, otherwise it is something else.

Yii model began to expand to controller layer once it introduced “safe/unsafe” attributes logic, deciding which data can be filled from web form and which is not. However, this feature still stays in a “grey” zone and can be considered as valid.
Now model moves forward to the controller layer, specifying the client/server data exchange format.

P.S. as I have already said, while there is none around, who consider it can be good idea, perhaps I am wrong with this.

[klimov-paul]

One more word about “yii\base\Model::getFormName()”: method name “getFormName” sounds confusing. For me words “form name” associates with “<form name=”form-name”>”.
I suppose this method should be named otherwise, like:

• getInputName()
• getInputGroupName()
• getRequestName()

[lucianobaraglia]

Talking about forms and populate values, here an excerpt of symfony controller doing the same thing:

  public function executeIndex($request)
  {
    $this->form = new ContactForm();

    if ($request->isMethod('post'))
    {
      $this->form->bind($request->getParameter('contact'));
      if ($this->form->isValid())
      {
        $this->redirect('contact/thankyou?'.http_build_query($this->form->getValues()));
      }
    }
  }

So the form populates itself...wich feels more "natural" in my opinion...

[psihius]

Using an intermidiate to populate model attributes from GET/POST is a good idea from the arhitectural point of view. Practically, I don't see the benefit of doing so. Why? Code will just get bloated and heavier, not straight forward as it is now.
And do you really use AR models in forms directly on the front-end? All my forms in Yii 1.1 are handled by enteties. extended from CFormModel - I rarely use AR models directly. And administrative CRUD stuff just does not care about how you name your fields as long as you can define labels for the fields (and all that stuff just gets auto-generated by gii).

You see, the idea behind Yii in the first place was the balance between architecture, design of the framework and real world everyday usage. As far as I'm concerned at this point, if you really need that kind of complexity, that your model data population needs an intermidiate to do that - just hop to Symphony/Zend Framework, Yii doesn't seems as a good choise at that point.
The strong case on Yii part is that it's straight forward, it's a tool that just does it's job without overcolmplicating things and giving you the ability to extend it as you need it to be. You don't have to override a whole bunch of stuff to achive a goal, because it's simple. Add a level of complexity and you now, sir, have to deal with built in functionality and wrap your head around on how to override it correctly or just plainly change it. You can find yourself just completely rewriting a component at that point and this rarely happens in Yii 1.1 (never happened to me, despite some really complex stuff going on in my projects). I prefer Yii 2 to stick to that moto and be simple, yet powerfull. So far it looks great and does not need to become more complex than it is.

My 0.02$.

[psihius]

Actually, I have a related question - what about handling multiple models in a single code? I have a page, where I handle data for 4 different models from the same form. Right now that code is repetative and you have to populate each model by hands.

[klimov-paul]

Proposal discarded.

[cebe]

@psihius check Controller::populate() method it is designed to work with all the models to populate them with the post data in one step.