yiisoft / yii

Yii PHP Framework 1.1.x
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
4.85k stars 2.28k forks source link

Decimal field validation is shorter than the database can handle #4376

Closed joshbmarshall closed 3 years ago

joshbmarshall commented 3 years ago

What steps will reproduce the problem?

Create a decimal(6,2) field in the database. Try entering 1234.56 into a text field.

What is the expected result?

Validation is ok.

What do you get instead?

Validation will say it is too long.

Additional info

Fixed in PR #4375

Q A
Yii version 1.1.24
PHP version 7.4
Operating system Linux
marcovtwout commented 3 years ago

@joshbmarshall Could you explain or provide a code sample what you mean with "validation"?

joshbmarshall commented 3 years ago

My apologies. The number field that is created uses the validator's max field for the maxlength, so I can't actually type a number long enough to show the validator error in the CForm. e.g. in CHtml::activeInputField you can see that the maxlength is set to $validator->max

The max value is taken from the 6 of decimal(6,2) and so doesn't take the '.' into account and allow the field length to be 7.

Does that make sense?

marcovtwout commented 3 years ago

It sounds to me like the actual problem is somewhere else: I don't think the CDbColumnSchema $size is directly linked to any piece of validation code. Validation rules are typically configured in de Model rules() function, for example with https://www.yiiframework.com/doc/api/1.1/CNumberValidator#max-detail. Perhaps you generated some code with Gii that doesn't set the max size correctly?

joshbmarshall commented 3 years ago

Yes this is correct the code generated with Gii has max length set one char too short. The proposed fix is in #4375

marcovtwout commented 3 years ago

I thought so, but in that case CDbColumnSchema is not the correct place to fix this. You should locate the code in Gii that you think is wrong and apply the change there.

rob006 commented 3 years ago

@marcovtwout I think that purpose of size property is to keep length of the column, I don't think it is used by anything except Gii.

marcovtwout commented 3 years ago

I looked up the piece of code that I was talking about: gii's ModelCode function generateRules(): https://github.com/yiisoft/yii/blob/master/framework/gii/generators/model/ModelCode.php#L220 This converts the table schema metadata into Yii's validation rules and this is where you expect the length to be set to 7.

The CDbColumnSchema->$size of 6 is technically correct: with a DECIMAL(6,2) that first digit (the precision) is 6 and that is the number of digits that will be stored. This is the same in Yii2. Only when dealing with user input you want that extra character to allow a separator.

And even though Yii internally only uses $size in the Gii-module, the metadata is publicly exposed via methods such as ActiveRecord->getTableSchema() so we cannot change it for backwards compatibility.

So it should be fixed in generateRules() but that's not trivial, since Yii sees MySQL Decimal as a string and treats those all the same way: https://github.com/yiisoft/yii/blob/master/framework/gii/generators/model/ModelCode.php#L240

marcovtwout commented 3 years ago

I propose to close this as a wontfix - this issue is a trivial enhancement and not easy to improve.