[Enhancment] Allow model to change the name used on forms.

[ronaldfenner]

Currently when using the Active elements for a form generation they use the class name of the model. It would be nice if we could specify a name to be used rather than the class name. Reason for this is where you might have several models in different modules that come in through a generic ajax action. The action takes care of identifying the module and then calling the modules handler for the action. By being able to override the class name used on the form the generic action can perform various checks for the form data being there and not have to bother continuing to locate the module especially if the modules name is included as a hidden field. In my case i'm storing an id for a record in the database that actually contains the module the form belongs to.

Implementation would be adding a new function to CModel that by default returns the class name of the instance. Change CHtml's resolveName function to call the new function to get the name to use on the form. Subclasses of CModel would then be able to override the CModel's function to change the class name for the form.

From a security standpoint this allows us to also hide the exact name we use in the application for our models. It would also allow us if we have a really long model name to use a shorter name on the form.

[mdomba]

I would like to see this in the core, nice improvement.

@qiangxue , @samdark opinions? Maybe a better name for the method formName() ?

[qiangxue]

This is a good move. But I have some concerns.

• If we want to use the same type of model for two forms on the same page (e.g. two login forms), how should we differentiate their formName?
• How to deal with namespaced classes? If we choose to support formName, I suggest we do basename(get_class($this)) as default formName.
• There are possibly other places that need to be updated as well, e.g. code generated by Gii.

[mdomba]

Like @phpnode and @cebe commented on the pull request... this is more a "prefix" then a form name... any better name for this method?

and there are CForm::loadData() and CACtiveForm::validate that use get_class() currently, so this should be adjusted too.

Because of the problem that @qiangxue explained - "same model for two forms on same page" - maybe a set/get method would be better than overriding so that we can to something like this in the controller

$model1 = new Model; $model2 = new Model;

$model1->formName = 'FormPrefixForFirstForm"; $model2->formName = "FormPrefixForSecondForm";

[samdark]

From security standpoint this change will not add anything. Security through obscurity isn't useful.

[samdark]

Also I don't like the fact that form-specific method is added to the model.

[mdomba]

@samdark it's not about security but a way to differentiate the attribute names if the same model is used more than once or to just use something else as a prefix for the attribute name then the class name.

[mdomba]

This could be seen as something else then a formName... this can be a model ID or NAME or HASH

it can be seen as a way to differentiate two objects created by the same class... I see the usability of using it in the active forms... but maybe can be used elsewhere too

[phpnode]

it would also be nice to be able to set this value to false, so that prefixing is turned off completely. This is useful for e.g. nicer URLs when submitting search forms. I agree a getter / setter is a better way to go

[ronaldfenner]

Maybe the better way at least for the forms would be add a form name property to CForm that by default is NULL or empty and with this condition would cause the form to use the passed in model's class name. I originally went with this method as it was the less intrusive to the code path to get the desired form name to the methods that resolves the form's attributes names.

The above method would solve a lot of the issues raised. It being a prefix doesn't solve the problem I was needing this for. I trying to use multiple models with their own forms that are loaded based on a specific option selected by a drop down. The form is then submitted to a ajax handler to process the request. I wanted to be able to use a standard name for the forms so they all could go through one handler and and be able to test that the form data post was at least set.

Thinking about it now the one id i need to check so i can load the appropriate record that has the module name that will process the form data doesn't have to have the same name as the rest of the model data.

Still it would be nice to be able to have the option to name the form data with something other than the model name. While Alexander pointed out that Security through obscurity is not an answer the fact that the form data uses the models name gives some one a direct correlation to a file in your models directory should it be open with no way to change the form data name other than using the non active form elements to build your form.

And yes I realize the routes also can be used to directly correlate to files in your controllers but I believe ( I havet tried to) you can set up rules and aliases for routes to break that correlation if you wanted to. This of course all assumes you make the mistake of having the protected folder in a directory that the web server can serve up. I like to keep mine just outside of that public directory.

So to summarize a better method. Modify CForm to have say a attributeGroupName which is by default false that has a setter and getter to change and get it's name. By default with it set to false the getter returns the class name of the model. Then make the necessary change where class name is used to use the form's attribute for CHtml's active elements this means either adding another parameter to them or perhaps adding it to the html options array.

Ron

On Thu, Mar 8, 2012 at 8:39 AM, Charles Pick < reply@reply.github.com

wrote:

it would also be nice to be able to set this value to false, so that prefixing is turned off completely. This is useful for e.g. nicer URLs when submitting search forms. I agree a getter / setter is a better way to go

---

Reply to this email directly or view it on GitHub: https://github.com/yiisoft/yii/issues/470#issuecomment-4392273

[phpnode]

@ronaldfenner your suggestion means this will only work when using CForm, I think your original solution is better, it just needs to be a getter / setter than you can set per model instance, and it needs a better name.

@samdark I think in practice it's fine to put this in the model, it's something that relates to individual model instances, not form instances. I think the alternatives would be restrictive.

[samdark]

@phpnode yes, you're right. But I still think we should not name it like formName or formSomething.

[ronaldfenner]

I committed updated changes. After looking at what called resolve name, the model is the easiest way to be able to change active elements form name values. I changed it to be a getter/setter and changed it to modelName. By default the name is null which causes the getter to return the class name. Went in and modified what seem to the be the spots that the change was needed to get the model name right. Only one spot in CActiveForm I was unsure whether it needed to be changed so left it with a comment questioning it's change. The gii generators were updated as well.

I was unsure if the yiilite file needed to be changed. I believe it's auto generated right?

Ron

On Thu, Mar 8, 2012 at 11:40 AM, Alexander Makarov < reply@reply.github.com

wrote:

@phpnode yes, you're right. But I still think we should not name it like formName or formSomething.

---

Reply to this email directly or view it on GitHub: https://github.com/yiisoft/yii/issues/470#issuecomment-4396734

[samdark]

Yes, yiilite is generated automatically. No need to change it.

[mdomba]

As this is not the model name but a prefix... how about naming it modelPrefix?

How about adding one more helper method populateAttributes that would be a helper for easier code writing and reading

if(isset($_POST[get_class($model)]))
   $model->attributes=$_POST[get_class($model)];

So in the controller we would just have $model->populateAttributes(); for massive assignment

[cebe]

@mdomba I think that should be discussed in a separate issue. And why is it a prefix? It is the name for the model to be used instead of class name.

[ronaldfenner]

Yeah I agree it's not exactly the model's name more of it's form name but i wouldn't say it's a prefix. It's added to the active element to group it together with the rest of the model's attributes.

I do like you suggestion it makes the code a little more human readable and a very common chunk of code.

On Thu, Mar 8, 2012 at 3:44 PM, Maurizio Domba < reply@reply.github.com

wrote:

As this is not the model name but a prefix... how about naming it modelPrefix?

How about adding one more helper method populateAttributes that would be a helper for easier code writing and reading

if(isset($_POST[get_class($model)])) $model->attributes=$_POST[get_class($model)];

---

Reply to this email directly or view it on GitHub: https://github.com/yiisoft/yii/issues/470#issuecomment-4403349

[mdomba]

I'm just thinking to not confuse users... as class_name would give one value but modelName would give another one if set manually... that's why I would rather not call it name as we are not changing the model/class name (one model is one class)

If prefix is not good maybe call it as simple as tag?

$model->tag -> very simple, very short, has it's meaning

About the additional method, I don't see why not discuss it here as it's related to this addition, but I agree we can open a new discussion for that after we solve the initial implementation for this.

[phpnode]

@mdomba tag is really confusing imho, and will likely break quite a lot of people's code, i personally have models and behaviors with getTag() methods that refer to completely different functionality. Tag is also used a lot in other parts of the framework to refer to html tags, so it becomes even more confusing, especially as we'd call this in CHtml itself.

[ronaldfenner]

I originally thought formName perhaps modelFormName instead since tag might suggest to new users that it will generate an Html tag with the name.

On Thu, Mar 8, 2012 at 4:07 PM, Maurizio Domba < reply@reply.github.com

wrote:

I'm just thinking to not confuse users... as class_name would give one value but modelName would give another one if set manually... that's why I would rather not call it name as we are not changing the model/class name (one model is one class)

If prefix is not good maybe call it as simple as tag?

$model->tag -> very simple, very short, has it's meaning

About the additional method, I don't see why not discuss it here as it's related to this addition, but I agree we can open a new discussion for that after we solve the initial implementation for this.

---

Reply to this email directly or view it on GitHub: https://github.com/yiisoft/yii/issues/470#issuecomment-4403836

[mdomba]

right, tag is not a good choice as it means something else... I personally dislike to add the word "model" to this why write "model" two time like

$model->modelXXX

why not just

$model->XXX

formName is not good too we already wrote that above... so we just need to find a good name for this...

[qiangxue]

I think either modelName or modelID is fine. Note that this IS a property of a single model instance, not the whole model class. The name tag or any other simple names are not good because it may cause conflict with model attribute names.

[qiangxue]

Note that one of the main reasons for this enhancement is because we want to differentiate two instances of the same model class when they are used to build two forms.

[mdomba]

Just one more suggestion from my part, if not good then let's go with @qiangxue idea of modelName or modelID

My idea is to call it "identifier"

$model->identifier

[qiangxue]

Just found another place that may need to be changed: CActiveDataProvider line 82:

$this->setId($this->modelClass);

This probably should be changed to

$this->setId($this->model->getModelName());

[qiangxue]

How about uniqueId, like we are doing in CController? Let's brainstorm for a while to think out a good name. :)

[mdomba]

"uniqueId" is very appealing... it's just what this is meant for... I like it the most of all other suggestions

[ronaldfenner]

After looking it over it doesn't need to be changed as the model class needs to be the real model class as it's going to be providing records.

On Thu, Mar 8, 2012 at 4:22 PM, Qiang Xue < reply@reply.github.com

wrote:

Just found another place that may need to be changed: CActiveDataProvider line 82:

$this->setId($this->modelClass);

This probably should be changed to

$this->setId($this->model->getModelName());

---

Reply to this email directly or view it on GitHub: https://github.com/yiisoft/yii/issues/470#issuecomment-4404147

[cebe]

@mdomba we need an other issue to not mix discussion and not loose overview, I have some arguments to say about your new method but that would mess up this discussion, please open a new issue for that.

uniqueId stands in some conflict to the primaryKey when we use ActiveRecord, so I would not vote for that. It also is not really a name which we use to differentiate between two instances of a class, this is in case of AR done by primary key. We use this name to avoid conflicting, because we have the same thing in one scope. Crazy hard to describe in english, what I mean. hope you got it :-)

[mdomba]

not sure what you mean regarding the primary key... the controller unique id returns the controller ID prefixed with the module ID - http://www.yiiframework.com/doc/api/1.1/CController#getUniqueId-detail

[qiangxue]

I agree with what cebe said. How about reverting back to formName? Anyway, this is mainly used in form.

[mdomba]

we made a trip around the world to get to the starting point :D

At this point any name is good for me, just to implement this and document it good.

[klimov-paul]

As for me this implementation breaks the MVC. Any HTML code belongs to the “View” layer or at least to “Controller” layer, but not to the “Model”. Instead of allowing model to specify its HTML name, you should allow “CHtml”, “CForm” and “CActiveForm” to use internal parameter for this. I mean method “CHtml::resolveName()” , should have parameter, which specifies the model HTML name:

public static function resolveName($model,&$attribute,$modelHtmlName=null) {
    …
    return ($modelHtmlName===null) ? get_class($model).'['.substr($attribute,0,$pos).']'.substr($attribute,$pos) : $modelHtmlName;
}

So “CActiveForm”, “CForm” and so on should pass their internal setting to this method (and methods, which use this method).

So widget creation will look like:

beginWidget('CActiveForm', array(
    'modelHtmlName'=>'MyModelName',
…

Such solution will solve the issue, while keeping the MVC.

No offence, but if we will evolve current suggested solution, the next step will be removing “CActiveForm” and allowing “CModel” to generate HTML for its attribute inputs on its own:

$model->labelEx(‘some_attribute’);
$model->textField (‘some_attribute’);

[samdark]

@klimov-paul makes sense.

[mdomba]

@klimov-paul note that this is not about HTML code... it's about a unique model representation (formname, modelname, modelId, whatever we name it)...

This is not used only in the view for rendering the form, but even in the controller to validate the models... in your example if you set the modelHtmlName when calling the CActiveForm the controller would not know what is the model name to make for example the massive assignements...

[klimov-paul]

I am disagreed. This issue IS about an HTML, unless you can found an example of its usage inside the console application. This issue has risen because the poor MVC layers separation: you allowed view and controller be driven by the model, while the controller should drive the model and the view. When controller setup model attributes from the POST it MUST use the name, which is used inside the HTML form. Actually a controller should remember this name: name of the key inside the POST and name of the input inside HTML.

In my example you can create a field inside the controller, which should specify the name, which is used to client-server data transfer:

class MyController extends CController {
    public $itemModelHtmlName = ‘ItemHtmlForm’;

    public function actionAdd() {
        …
        $model->attributes = $_POST[$this->itemModelHtmlName];
    }

}

Than you can use this property inside the view file and pass it to the widget:

$form=$this->beginWidget('CActiveForm', array(
    'modelHtmlName'=>$this->itemModelHtmlName,

View and controller should work around the model, not through it!

The problem is the data transfer form should be an artifact at the controller layer, should have representation at the view layer and fill up the model.

class MyController extends CController {
    public $itemModelHtmlName = ‘ItemHtmlForm’;

    public function actionAdd() {
        …
        $form = $this->getItemForm();
        $model->attributes = $form->fetchDataFromPost();
    }

}

// View file
….
$form = $this->getItemForm();
$form->textField (‘some_attribute’,$model);

[mikehaertl]

Well, if we don't allow it in the model, then there should at least be something instead of no solution at all. I don't really agree to @klimov-paul: MVC is nice as a general rule of thumb but should not be taken to such an extreme that it's like the holy grail no one can ever touch: If there's a simple solution without any obvious harm and it breaks MVC, i'd go for it!

I would take this issue here even further: In some scenarios i want my input fields to be named query instead of SearchForm[query]. Simply because i may have a search form and want nice user friendly URLs after submission. It should be up to me as a developer if i want to change that - i'll know what i'm doing. Currently there's no way at all. Everything is hardwired into CHtml. I could live with a solution like $model->getInputName('query'), which i could override in some models if i wanted to.

I know, that this is not pure MVC - but it's a pragmatic solution. And similar to what we already do with form labels.

The only other solution i could think of, is to use some sort of helper class which i can feed into all CHtml::active* methods, which craetes form input names for me. But this sounds like overkill.

[klimov-paul]

@mikehaertl, you may wonder, but this entire issue has risen, because Yii has poor MVC. There is no explicit layer of "View", which should handle the form input names. "View" layer is mixed up with the controller and split around static helper classes. If the CHtml was an application component, which can be extendable and replacable, you would be able to override method "CHtml::resolveName()" in your project and design it as you like, even implementing the "Form name" approach.

At its born Yii framework has no care about the input names and this was harcoded in its architecture inside "CHtml". Now we can not just make framework to "care" without changing its architecture.

At the moment you can alwasy walk around your issue extending the "CActiveRecord" class, overridding its input creating methods using lower level of "CHtml" functions.

[mikehaertl]

@klimov-paul I agree that a View layer would make a lot of sense. As far as i know Yii 2.0 will implement such a view class. I also agree that the root of these problems lies in CHtml. In my opinion this class is way too complex - it should focus on HTML rendering only and not be interwoven with Yii's form handling so much (read: should not have all these active*() methods).

All form related mechanics could be moved to a HtmlForm widget or something. I once described it in a forum post here