yii2-gii
yii2-gii copied to clipboard
Implemented enum
| Q | A |
|---|---|
| Is bugfix? | no |
| New feature? | yes |
| Breaks BC? | no |
| Tests pass? | yes |
Generate:
- enum value constants
- rules type in
- opts
- get enum field label (mybe rename )
- is[EnumField][enumValue]
class Test extends \yii\db\ActiveRecord
{
const TYPE_PARTNER = 'PARTNER';
const TYPE_PUBLIC = 'PUBLIC';
const TYPE_INTERNAL = 'INTERNAL';
const TYPE_OWNER = 'OWNER';
public function rules()
{
return [
[['type'], 'string'],
[['position'], 'string', 'max' => 50],
['type', 'in', 'range' => [
self::TYPE_PARTNER,
self::TYPE_PUBLIC,
self::TYPE_INTERNAL,
self::TYPE_OWNER,
]
],
];
}
/**
* column type ENUM value labels
* @return array
*/
public static function optsType()
{
return [
self::TYPE_PARTNER => 'PARTNER',
self::TYPE_PUBLIC => 'PUBLIC',
self::TYPE_INTERNAL => 'INTERNAL',
self::TYPE_OWNER => 'OWNER',
];
}
/**
* @return string
*/
public function displayType()
{
return self::optsType()[$this->type];
}
/**
* @return bool
*/
public function isTypePARTNER()
{
return $this->type === self::TYPE_PARTNER;
}
/**
* @return bool
*/
public function isTypePUBLIC()
{
return $this->type === self::TYPE_PUBLIC;
}
/**
* @return bool
*/
public function isTypeINTERNAL()
{
return $this->type === self::TYPE_INTERNAL;
}
/**
* @return bool
*/
public function isTypeOWNER()
{
return $this->type === self::TYPE_OWNER;
}
}
Thank you for putting effort in the improvement of the Yii framework. We have reviewed your pull request.
In order for the framework and your solution to remain stable in the future, we have a unit test requirement in place. Therefore we can only accept your pull request if it is covered by unit tests.
Could you add these please?
Thanks!
P.S. If you have any questions about the creation of unit tests? Don't hesitate to ask for support. More information about unit tests
This is an automated comment, triggered by adding the label pr:request for unit tests.
Unit test done
I've got few concerns:
- How this will work with MSSQL? IIRC ENUM dbType is recognized but enumValues are left empty.
- Overall this needs some tests with real DB engines.
- Are we cool with that much code being generated here?
- What do you think about having these optional/configured?
I've got few concerns:
- How this will work with MSSQL? IIRC ENUM dbType is recognized but enumValues are left empty. This is a problem for MS SQL users
- Overall this needs some tests with real DB engines. On MySql and MariaDB enum use more two years
- Are we cool with that much code being generated here? If use enum, is very useful. Code generate, if model has enums. Setting like $model->setTypePublic() also would also be good
- What do you think about having these optional/configured? Actually gii model form is so big. No need.
In giiant enum use long time: https://github.com/schmunk42/yii2-giiant/blob/master/src/generators/model/Generator.php#L295
- How this will work with MSSQL? IIRC ENUM dbType is recognized but enumValues are left empty.
Please provide me MSSQL schema dump like https://github.com/yiisoft/yii2-gii/pull/434/files#diff-eef4be0410fa93517728588e0c838dd2f9afb86ff7148c7f7d7636de99df3b00R486
@samdark! Actual PR still waiting. @bizley added additional strange requirements. It work.
@bizley
How this will work with MSSQL? IIRC ENUM dbType is recognized but enumValues are left empty.
mssql don't have enum/set types and they not required in sql standards
What do you mean by "strange"? These are legitimate concerns. You are just adding the feature but we will have to maintain it later on - I would like to at least be sure it works in most of the cases. And I'm not at the moment. I would like to see it tested with PostgreSQL since there is schema already in the tests added. And still no one said anything about the amount of additional code generated here (that is apart from you two guys, it's nice that you support each other like that).
I woke up
Apparently he went back to sleep.
@samdark, what do you think about removing this PR from the 2.2.7 milestone and releasing the version?
Apparently he went back to sleep.
I define this initiative as illegal
I define this initiative as illegal
I prefer to believe that this was some kind of slang, as I don't understand English well.
But if you were bothered by my joke, I apologize.
My intention was just to clear the milestone so that a new version can be released.
I define this initiative as illegal
I prefer to believe that this was some kind of slang, as I don't understand English well.
But if you were bothered by my joke, I apologize.
My intention was just to clear the milestone so that a new version can be released.
Don't take it seriously. When should I wake up?
@uldisn time to wake up. Please add a line for CHANGELOG. Then I'll merge it.
@uldisn time to wake up. Please add a line for CHANGELOG. Then I'll merge it.
Done! Good night!
Thank you!