Gii sets wrong model rule for integer field with default value
What steps will reproduce the problem?
create table parent (id serial primary key, code character varying(5));
insert into parent (code) values ('one');
insert into parent (code) values ('two');
Next, create child table with FK pointing to parent.id
create table child (
id serial primary key,
parent_id integer references parent(id) not null default 1,
code character varying(5)
);
insert into child (parent_id, code) values (1, 'c 1');
insert into child (parent_id, code) values (2, 'c 2');
insert into child (code) values ('c 3'); -- parent_id will be defaulted to `1`
Next, generate models via Gii/model
What is the expected result?
/**
* @inheritdoc
*/
public function rules()
{
return [
[['parent_id'], 'default', 'value' => 1], <--- this is what I expected
[['parent_id'], 'integer'],
[['code'], 'string', 'max' => 5],
[['parent_id'], 'exist', 'skipOnError' => true, 'targetClass' => Parent::className(), 'targetAttribute' => ['parent_id' => 'id']],
];
}
What do you get instead?
/**
* @inheritdoc
*/
public function rules()
{
return [
[['parent_id'], 'default', 'value' => null], <--- this is what I get
[['parent_id'], 'integer'],
[['code'], 'string', 'max' => 5],
[['parent_id'], 'exist', 'skipOnError' => true, 'targetClass' => Parent::className(), 'targetAttribute' => ['parent_id' => 'id']],
];
}
Additional info
| Q | A |
|---|---|
| Yii version | 2.0.13.1 |
| Gii | 2.0.6.0 |
| PHP version | 7.0 |
| PostgreSQL version | PostgreSQL 9.5.9 on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609, 64-bit |
| Operating system | Ubuntu 16.04 LTS |
This was added for sake of #224.
I submitted a PR, not sure how to test other than what I did below (manual tests):
My test DB tables:
create table brand (id serial primary key, code character varying(20));
create table country (id serial primary key, code character varying(20));
create table region (id serial primary key, code character varying(20));
create table state (id serial primary key, code character varying(20));
create table car (
id serial primary key,
brand_id integer references brand(id) not null default 1,
country_id integer references country(id) not null,
region_id integer references region(id),
state_id integer references region(id) default 2,
original_code character varying(20) not null,
country_specific_code character varying(20),
internal_code character varying(20) not null default 'Secret-code',
external_code character varying,
media_code character varying(20) not null
);
Current gii output:
public function rules()
{
return [
[['brand_id', 'country_id', 'region_id', 'state_id'], 'default', 'value' => null],
[['brand_id', 'country_id', 'region_id', 'state_id'], 'integer'],
[['country_id', 'original_code', 'media_code'], 'required'],
[['external_code'], 'string'],
[['original_code', 'country_specific_code', 'internal_code', 'media_code'], 'string', 'max' => 20],
[['brand_id'], 'exist', 'skipOnError' => true, 'targetClass' => Brand::className(), 'targetAttribute' => ['brand_id' => 'id']],
[['country_id'], 'exist', 'skipOnError' => true, 'targetClass' => Country::className(), 'targetAttribute' => ['country_id' => 'id']],
[['region_id'], 'exist', 'skipOnError' => true, 'targetClass' => Region::className(), 'targetAttribute' => ['region_id' => 'id']],
[['state_id'], 'exist', 'skipOnError' => true, 'targetClass' => Region::className(), 'targetAttribute' => ['state_id' => 'id']],
];
}
After gii code is updated:
public function rules()
{
return [
[['brand_id', 'country_id', 'region_id', 'state_id'], 'integer'],
[['brand_id'], 'default', 'value' => 1],
[['state_id'], 'default', 'value' => 2],
[['country_id', 'original_code', 'media_code'], 'required'],
[['external_code'], 'string'],
[['internal_code'], 'default', 'value' => 'Secret-code'],
[['original_code', 'country_specific_code', 'internal_code', 'media_code'], 'string', 'max' => 20],
[['brand_id', 'country_id', 'state_id'], 'unique', 'targetAttribute' => ['brand_id', 'country_id', 'state_id']],
[['brand_id'], 'exist', 'skipOnError' => true, 'targetClass' => Brand::className(), 'targetAttribute' => ['brand_id' => 'id']],
[['country_id'], 'exist', 'skipOnError' => true, 'targetClass' => Country::className(), 'targetAttribute' => ['country_id' => 'id']],
[['region_id'], 'exist', 'skipOnError' => true, 'targetClass' => Region::className(), 'targetAttribute' => ['region_id' => 'id']],
[['state_id'], 'exist', 'skipOnError' => true, 'targetClass' => Region::className(), 'targetAttribute' => ['state_id' => 'id']],
];
}
If default values are set in the database, why do you need it in your model?
@schmunk42 the issue was that
[['parent_id'], 'default', 'value' => null], <--- this is what I get
was generated while default value was actually present in DB.
I was referring to those two lines in the patched output
[['brand_id'], 'default', 'value' => 1],
[['state_id'], 'default', 'value' => 2],
But my view is mirroring full table requirements in model. That allow in validation rules trust on default values. Currently my know problem is with require, but any other problems can occur.
If there's a default value in the DB, I think it's debatable whether you want that in your model. If you "just" change it in the DB, you have to regenerate your code, otherwise you might end up in setting the wrong default value in your app.
Even more special is the case if you have a default value and NOT NULL - actually it does not have to be required in the model, since the DB takes care about setting a value.
But I also understand your point, that's why I mentioned making it optional in https://github.com/yiisoft/yii2-gii/pull/420#issuecomment-554980604.
We had the same argument (db would handle default values), and the conclusion was:
- db Vs model alignment
- how does it handle null Vs empty string
- if it's not required to set in the model, mind as well don't set null as default in the model
- you shouldn't just change in the dB, if there a change, there should be a migration as well make sure to regenerate your model