Fixed generation of the HTML class attribute for the data model #2282

Closed
wants to merge 11 commits into
from

3 participants

Contributor

Original idea/code by LastDragon-ru

Updated files :

  • web/form/CForm
  • web/helpers/CHml
  • web/widgets/CActiveForm
  • web/CActiveDataProvider
  • CHANGELOG

Added

  • sample code in CHtml
  • credits to lastDragon-ru in the CHANGELOG
  • added shortcut to retrieve postdata
    instead of doing :

new methods added :

  • get html compliant name : CHtml::nameForModel($model)
  • get $_POST data :CHtml::postDataForModel($model)

new property added :

  • set the filtering method : CHtml::$modelNameFilter

For example (as explained by lastDragon-ru) you can shorten or obfuscate :

CHtml::$modelNameFilter = function($model) {
    return dechex(crc32(get_class($model)));
};
$htmlModelName = CHtml::nameForModel($model);
if(isset($_POST[htmlModelName]) {
  $model->attributes = $_POST[htmlModelName];
  // perform actions
  // ...
}

we can do :

$modelData = CHtml::postDataForModel($model);
if($modelData !== null) {
  $model->attributes = $modelData;
  // perform actions
  // ...
}
@klimov-paul klimov-paul commented on an outdated diff Apr 1, 2013
@@ -62,6 +62,7 @@ Version 1.1.14 work in progress
- New #575: Yii registering at Packagist, added composer info file (schmunk42)
- New #1785: Added CPasswordHelper (tom--)
- New #2178: Added Catalan Translation (ArnauAregall)
+- Enh #129: Change CHTML::resolvename() to allow the used of namespaced models without breaking HTML (lastdragon-ru, pgaultier)
klimov-paul
klimov-paul Apr 1, 2013 Member

Line should be moved above and placed at "Enh" section in numerical order, just before #1142.
Also fix camelCase at "CHtml::resolveName()".

@klimov-paul klimov-paul commented on an outdated diff Apr 1, 2013
framework/web/helpers/CHtml.php
@@ -87,6 +87,20 @@ class CHtml
public static $renderSpecialAttributesValue=true;
/**
+ * Namespaced class names can be shortened or obfuscated.
+ * Default method is to replace "\" with "_"
+ * <code>
+ * CHtml::$modelNameFilter = function($model) {
+ * return dechex(crc32(get_class($model)));
+ * };
+ * </code>
+ *
+ * @var callback method used to clean and/or shorten class name
klimov-paul
klimov-paul Apr 1, 2013 Member

Should be something like:
PHP callback used to compose input name from model instance.
If not set the model class name will be used. If class name is namespaced '' will be replaced by '_'.
You can setup this field intoducing your logic of the input name creation.
For example:
<code>
...
</code>

@var callback PHP callback used to compose input name from model instance.
@see nameForModel()

@klimov-paul klimov-paul and 1 other commented on an outdated diff Apr 1, 2013
framework/web/helpers/CHtml.php
@@ -2393,6 +2407,30 @@ public static function resolveNameID($model,&$attribute,&$htmlOptions)
}
/**
+ * Return HTML compliant name for selected model
+ * @param CModel $model the data model
+ * @return string the normalized model string
+ */
+ public static function nameForModel($model) {
klimov-paul
klimov-paul Apr 1, 2013 Member

Not sure this is a good name for such method.
Perhaps it would be better to use something like:

  • normalizeModelName()
  • composeModelName()
  • modelName()
pgaultier
pgaultier Apr 1, 2013 Contributor

Ok, I changed the name to normalizeModelName()

@klimov-paul klimov-paul and 1 other commented on an outdated diff Apr 1, 2013
framework/web/helpers/CHtml.php
@@ -2393,6 +2407,30 @@ public static function resolveNameID($model,&$attribute,&$htmlOptions)
}
/**
+ * Return HTML compliant name for selected model
+ * @param CModel $model the data model
+ * @return string the normalized model string
+ */
+ public static function nameForModel($model) {
+ if((self::$modelNameFilter !== null) && is_callable(self::$modelNameFilter)) {
klimov-paul
klimov-paul Apr 1, 2013 Member

It would be better to have fallback in the case "CHtml::$modelNameFilter" is not nul but also not callable.
In this case an exception shold be thrown informing developer he did something wrong.

pgaultier
pgaultier Apr 1, 2013 Contributor

I'm not sure about the fallback, CHtml feature no Exception. Why starting with this method ?

@klimov-paul klimov-paul and 1 other commented on an outdated diff Apr 1, 2013
framework/web/helpers/CHtml.php
@@ -87,6 +87,20 @@ class CHtml
public static $renderSpecialAttributesValue=true;
/**
+ * Namespaced class names can be shortened or obfuscated.
+ * Default method is to replace "\" with "_"
+ * <code>
+ * CHtml::$modelNameFilter = function($model) {
+ * return dechex(crc32(get_class($model)));
+ * };
+ * </code>
+ *
+ * @var callback method used to clean and/or shorten class name
+ * @since 1.1.14
+ */
+ public static $modelNameFilter;
klimov-paul
klimov-paul Apr 1, 2013 Member

Not sure about name.
But it defenetly should match the final name of the method "nameForModel".
Currently it is hard to make a connection between them from thier names.

pgaultier
pgaultier Apr 1, 2013 Contributor

I finally chose :
CHtml::normalizeModelName() and CHtml::$modelNameNormalizer

@klimov-paul klimov-paul and 1 other commented on an outdated diff Apr 1, 2013
framework/web/helpers/CHtml.php
+ * @return string the normalized model string
+ */
+ public static function nameForModel($model) {
+ if((self::$modelNameFilter !== null) && is_callable(self::$modelNameFilter)) {
+ $nameForModel = call_user_func(self::$modelNameFilter, $model);
+ } else {
+ $nameForModel = str_replace('\\', '_', get_class($model));
+ }
+ return $nameForModel;
+ }
+
+ /**
+ * Use html compliant model name to retrieve post data
+ * @param CModel $model the target model
+ * @return mixed the content of $_POST for current model or null if empty
+ */
klimov-paul
klimov-paul Apr 1, 2013 Member

This method should be removed.
Retrieving data from HTTP request belongs to the Controller layer, while "CHtml" belongs to the "View" layer.
Developer should perfrom such things on his own at his controller.
Besides, what about $_GET?

pgaultier
pgaultier Apr 1, 2013 Contributor

Ok, method has been removed

@klimov-paul klimov-paul commented on an outdated diff Apr 1, 2013
framework/web/helpers/CHtml.php
@@ -2393,6 +2407,30 @@ public static function resolveNameID($model,&$attribute,&$htmlOptions)
}
/**
+ * Return HTML compliant name for selected model
klimov-paul
klimov-paul Apr 1, 2013 Member

Should repeat some notes from doc comments of "$modelNameFilter".

@klimov-paul klimov-paul commented on an outdated diff Apr 2, 2013
framework/web/widgets/CActiveForm.php
@@ -886,8 +886,9 @@ public static function validate($models, $attributes=null, $loadInput=true)
$models=array($models);
foreach($models as $model)
{
- if($loadInput && isset($_POST[get_class($model)]))
- $model->attributes=$_POST[get_class($model)];
+ $normalizedModelClass = CHtml::normalizeModelName($model);
klimov-paul
klimov-paul Apr 2, 2013 Member

$normalizedModelClass => $normalizedModelName.
Also remove extra spaces around '='.

@klimov-paul klimov-paul commented on an outdated diff Apr 2, 2013
framework/web/widgets/CActiveForm.php
@@ -914,8 +915,9 @@ public static function validateTabular($models, $attributes=null, $loadInput=tru
$models=array($models);
foreach($models as $i=>$model)
{
- if($loadInput && isset($_POST[get_class($model)][$i]))
- $model->attributes=$_POST[get_class($model)][$i];
+ $normalizedModelName = CHtml::normalizeModelName($model);
klimov-paul
klimov-paul Apr 2, 2013 Member

Remove extra spaces around '='.

@klimov-paul klimov-paul and 1 other commented on an outdated diff Apr 2, 2013
framework/web/CActiveDataProvider.php
}
elseif($modelClass instanceof CActiveRecord)
{
- $this->modelClass=get_class($modelClass);
+ $this->modelClass=CHtml::normalizeModelName($modelClass);
klimov-paul
klimov-paul Apr 2, 2013 Member

"CActiveDataProvider::modelClass" should not be affected by "CHtml::normalizeModelName"!
It is passed to "CSort" at "CActiveDataProvider::getSort()".

pgaultier
pgaultier Apr 2, 2013 Contributor

My mistake, reverted to original get_class()

Member

First if all, we need to be sure every functionality affected by this fix is covered: need a files check list.
At the moment, I see at least the Gii Crub genertor templates also should be updated with the "CHtml::normalizeModelName()".

Also we need to decide what the default strategy for namespaced classes we shall use. At the moment we replace '' by '', but this means classes '\My\Space\Form\User' and '\My\Space\Form_User' will have same normalized names.
Perhaps we should replace '' with double underscrore '
_', this will make normalized names longer, but reduce the probability of thier duplicationg.

@yiisoft, opinions?

Owner
samdark commented May 20, 2013

Closed in favor of #2446

@samdark samdark closed this May 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment