Model rules for default values #420

uldisn · 2019-11-16T18:25:05Z

Q	A
Is bugfix?	yes
New feature?	yes
Breaks BC?	no
Tests pass?	yes
Fixed issues	-

Bug

CREATE TABLE `contract` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `contract_number` varchar(50)  NOT NULL DEFAULT 'NoNumber' COMMENT 'Number',
  `contract_date` date DEFAULT NULL COMMENT 'Contract date',
  `contract_place` varchar(250) DEFAULT NULL COMMENT 'Contract place',
  `notes` text COMMENT 'Notes',
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8

rules

    /**
     * @inheritdoc
     */
    public function rules()
    {
        return [
            [['contract_number'], 'required'],
            [['contract_date'], 'safe'],
            [['notes'], 'string'],
            [['contract_number'], 'string', 'max' => 50],
            [['contract_place'], 'string', 'max' => 250],
        ];
    }

If in rules no defined default value for field contract_number and not initiated, validation fired on require rule.

Example of generated rules:

    /**
     * {@inheritdoc}
     */
    public function rules()
    {
        return [
            [['name', 'address', 'profile_id'], 'default', 'value' => null],
	    [['status'], 'integer', 'default', 'value' => 0],
            [['email'], 'required'],
            [['address'], 'string'],
            [['status', 'profile_id'], 'integer'],
            [['email', 'name'], 'string', 'max' => 128],
        ];

Signed-off-by: uldisn <uldisnelsons@gmail.com>

Fix yiisoft#416: Improved generation of model attributes and type annotations

…sDefaultValues

Rules default values

Signed-off-by: uldisn <uldisnelsons@gmail.com>

yii-bot · 2019-11-18T11:16:44Z

Thank you for putting effort in the improvement of the Yii framework.
We have reviewed your pull request.

Unfortunately a use case is missing. It is required to get a better understanding of the pull request and helps us to determine the necessity and applicability of the suggested change to the framework.

Could you supply us with a use case please? Please be as detailed as possible and show some code!

Thanks!

This is an automated comment, triggered by adding the label pr:missing usecase.

schmunk42 · 2019-11-18T11:43:21Z

I think this should either be enabled by a parameter or done in a 2.2.x release.

uldisn · 2019-11-18T17:06:46Z

Realy is bug! See description.

samdark

👍

src/generators/model/Generator.php

…NT_TIMESTAMP Signed-off-by: uldisn <uldisnelsons@gmail.com>

src/generators/model/Generator.php

schmunk42 · 2019-11-19T09:47:22Z

I don't get it from the description, but apparently @samdark does, so it's fine 😄

Signed-off-by: uldisn <uldisnelsons@gmail.com>

samdark · 2019-11-19T10:17:05Z

Thinking more about it... @uldisn what's the use case for having explicit default rules if they match database ones? As far as I know, database silently uses default when value is not provided anyway...

uldisn · 2019-11-19T10:37:10Z

Sometimes rules is fired, if not set default values. For example, if field required ( not null), in database is defined default value and in model not set. Then rule required fired.
For optimal indexing not null is very actual, but usually using default CURRENT_TIMESTAMP fired on require rule.

samdark · 2019-11-19T10:53:48Z

Isn't it better not to generate required rule in this case?

uldisn · 2019-11-19T11:13:19Z

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 other problems can occur.

For tuning masters yes. They can do it ( not to generate required rule in this case).

Additionally i see follow improvements:

default value define as model constant
create method "loadDefaultValues()" - useful for new record before showing in form.

samdark · 2019-11-19T11:21:10Z

Currently my know problem is with require, but other problems can occur.

Any examples?

create method "loadDefaultValues()" - useful for new record before showing in form.

https://www.yiiframework.com/doc/api/2.0/yii-db-activerecord#loadDefaultValues()-detail is already there and works without any extra rules.

uldisn · 2019-11-20T14:37:15Z

On issue #335 is problem

samdark · 2019-11-20T14:39:46Z

#335 seems to have another pull request https://github.com/yiisoft/yii2-gii/pull/421/files

uldisn · 2019-11-20T19:09:39Z

Solutions:

database fields with not null and default value not include in require rule
- fired only on like Gii sets wrong model rule for integer field with default value #335 issue described situation
- minimal and optimal code
create full list rules for default values
- work correctly in any situation
- huge code
create full list rules for default values excluding null default value
- work correctly in any situation
- no so huge code

Signed-off-by: uldisn <uldisnelsons@gmail.com>

This reverts commit c83843f Signed-off-by: uldisn <uldisnelsons@gmail.com>

uldisn · 2020-01-17T16:46:10Z

Generation default value rules cover follow problems:

Gii sets wrong model rule for integer field with default value Gii sets wrong model rule for integer field with default value #335 Fix for issue #335 #421
The generator makes all integer columns as […, 'default', 'value' => null] even column set as not nul The generator makes all integer columns as […, 'default', 'value' => null] even column set as not null #402

Only need adapt for PostgrSQL

WinterSilence · 2021-03-08T17:48:46Z

that's magic in yii confuse me - why not define properties with defaults from db table?

class Contact extends ActiveRecord
{
    /**
     * @var int
     */
    public $id;

    /**
     * @var string Number
     */
    public $contract_number = 'NoNumber';

    ...
}

/**
 * @var {type} {comment}
 */
public ${column} ={default};

WinterSilence · 2021-04-10T13:49:10Z

src/generators/model/Generator.php

+                        case Schema::TYPE_DATETIME:
+                        case Schema::TYPE_TIMESTAMP:
+                            if(strtoupper($column->defaultValue) === 'CURRENT_TIMESTAMP'){
+                                $defaultValue = 'date(\'Y-m-d H:i:s\')';


mssql/sql server:

DEFAULT definitions cannot be created on columns with a timestamp data type or columns with an IDENTITY property.

https://docs.microsoft.com/ru-ru/sql/t-sql/statements/create-table-transact-sql?view=sql-server-ver15

also it must be $defaultValue = 'time()';

For timestamp column, if default value not set, fired on require rule.

@uldisn

if (strtoupper($column->defaultValue) === 'CURRENT_TIMESTAMP') { if ($driverName === 'mssql') { $defaultValue = $this->generateString($column->defaultValue); } else { $defaultValue = 'time()'; } }

On MSSQL fired rule, who validate actual field value - time format.

@uldisn yes, my fail

if (strtoupper($column->defaultValue) === 'CURRENT_TIMESTAMP') { if ($driverName === 'mysql' && $column->type === Schema::TYPE_TIMESTAMP) { $defaultValue = 'time()'; } else { $defaultValue = "date('Y-m-d H:i:s')"; } }

uldisn · 2021-04-12T06:23:16Z

that's magic in yii confuse me - why not define properties with defaults from db table?

class Contact extends ActiveRecord
{
    /**
     * @var int
     */
    public $id;

    /**
     * @var string Number
     */
    public $contract_number = 'NoNumber';

    ...
}

/**
 * @var {type} {comment}
 */
public ${column} ={default};

For example, if default value is timestamp, it cannot set to property.

WinterSilence · 2021-04-12T08:12:30Z

@uldisn why? timestamp is integer

WinterSilence · 2021-04-12T08:20:22Z

src/generators/model/Generator.php

+        $rules = [];
+        $driverName = $this->getDbDriverName();
+
+        if (in_array($driverName, ['mysql', 'sqlite'], true)) {


why only that's drivers?

I know only MySql.

I'm sure just about MySql

@uldisn that's types normalized check DB subfolders, also:

/** * @var string abstract type of this column. Possible abstract types include: * char, string, text, boolean, smallint, integer, bigint, float, decimal, datetime, * timestamp, time, date, binary, and money. */ public $type; /** * @var string the PHP type of this column. Possible PHP types include: * `string`, `boolean`, `integer`, `double`, `array`. */ public $phpType;

as you can see, $phpType use only 5 types, you can detect numeric as if (in_array($column->phpType, ['integer', 'double', 'float'], true)) 'float' added because php dont have type 'double', i think it's used to set validation rule

WinterSilence · 2021-04-12T08:32:41Z

src/generators/model/Generator.php

+
+            foreach ($table->columns as $column) {
+
+                if ($column->defaultValue !== null) {


if ($column->defaultValue === null && !$column->allowNull) { continue; }

Not need "&& !$column->allowNull"

@uldisn no need $columnsDefaultNull set if ($column->allowNull) {$defaultValue = 'null';}

by default all attributes has null value. Not need.

@uldisn null !== 'null', echo null or echo false don't print null or false, in my packages I use <?=json_encode($var)?>

I checked MySql schema. If in MySql field no default value, then $column->defaultValue is null.
$column->allowNull use for rules "required"
Correct:

if ($column->defaultValue === null) { continue; }

@uldisn $column->allowNull is @param type|null $foo, function setFoo(?type $value) and function getFoo(): ?type.

If in MySql field no default value, then $column->defaultValue is null.

CREATE TABLE `demo` (`a` VARCHAR(255) NULL, `b` TEXT NULL); INSERT INTO `demo` (`a`) VALUES (NULL); // (NULL, NULL)

CREATE TABLE ` demo` ( `id` INT(11) UNSIGNED NOT NULL AUTO_INCREMENT, `a` TEXT NULL DEFAULT NULL, `b` VARCHAR(50) NULL DEFAULT 'is default' PRIMARY KEY (`id`) USING BTREE ) ENGINE=InnoDB; INSERT INTO ` demo` (`a`, `b`) VALUES ('...', NULL); INSERT INTO ` demo` (`a`, `b`) VALUES (NULL, '...'); INSERT INTO `demo` (`b`) VALUES (NULL); INSERT INTO `demo` (`a`) VALUES ('...');

uldisn and others added 14 commits November 11, 2019 11:56

improved generation annotation of properties

a13b973

Signed-off-by: uldisn <uldisnelsons@gmail.com>

Rolled back qualifiers

f9adb4c

Update Generator.php

10f8d33

Update ModelGeneratorTest.php

2b69aa0

Added changes to changelog.md

e0a20b9

Signed-off-by: uldisn <uldisnelsons@gmail.com>

Update CHANGELOG.md

f4cb847

To model generator added rules for default values

6676dae

Signed-off-by: uldisn <uldisnelsons@gmail.com>

Reformated code

32f9c95

Signed-off-by: uldisn <uldisnelsons@gmail.com>

Merge pull request #2 from yiisoft/master

05b0b09

Fix yiisoft#416: Improved generation of model attributes and type annotations

Merge branch 'master' of https://github.com/uldisn/yii2-gii into Rule…

1e4d743

…sDefaultValues

Merge pull request #3 from uldisn/RulesDefaultValues

57f2d72

Rules default values

Fixes of fixes

73ff305

Signed-off-by: uldisn <uldisnelsons@gmail.com>

Fix/fix/fix

23b4a3a

Signed-off-by: uldisn <uldisnelsons@gmail.com>

Added quoting for text default values

e534105

Signed-off-by: uldisn <uldisnelsons@gmail.com>

samdark added the pr:missing usecase It is not clear what is the use case for the pull request. label Nov 18, 2019

samdark added type:bug Bug and removed pr:missing usecase It is not clear what is the use case for the pull request. labels Nov 18, 2019

uldisn changed the title ~~To model rules add default values~~ Model rules for default values Nov 18, 2019

samdark requested a review from a team November 19, 2019 08:36

samdark requested changes Nov 19, 2019

View reviewed changes

src/generators/model/Generator.php Outdated Show resolved Hide resolved

src/generators/model/Generator.php Outdated Show resolved Hide resolved

src/generators/model/Generator.php Outdated Show resolved Hide resolved

src/generators/model/Generator.php Outdated Show resolved Hide resolved

samdark added the status:under development Someone is working on a pull request. label Nov 19, 2019

Done requested changes and added special case for default value CURRE…

0399a42

…NT_TIMESTAMP Signed-off-by: uldisn <uldisnelsons@gmail.com>

samdark added status:code review The pull request needs review. and removed status:under development Someone is working on a pull request. labels Nov 19, 2019

samdark added this to the 2.1.2 milestone Nov 19, 2019

samdark reviewed Nov 19, 2019

View reviewed changes

src/generators/model/Generator.php Outdated Show resolved Hide resolved

samdark removed the status:code review The pull request needs review. label Nov 19, 2019

samdark added the status:under development Someone is working on a pull request. label Nov 19, 2019

For default value escaping used $db->getSchema()->quoteValue()

7f51d73

Signed-off-by: uldisn <uldisnelsons@gmail.com>

samdark added status:under discussion and removed status:under development Someone is working on a pull request. labels Nov 19, 2019

uldisn requested review from samdark and removed request for a team November 19, 2019 11:18

samdark removed this from the 2.1.2 milestone Nov 19, 2019

schmunk42 mentioned this pull request Nov 20, 2019

Gii sets wrong model rule for integer field with default value #335

Open

samdark added this to the 2.1.4 milestone Nov 20, 2019

uldisn added 2 commits November 22, 2019 13:00

Uz Yii 2.0.30

c83843f

Signed-off-by: uldisn <uldisnelsons@gmail.com>

Revert "Uz Yii 2.0.30"

a648b93

This reverts commit c83843f Signed-off-by: uldisn <uldisnelsons@gmail.com>

samdark removed this from the 2.1.4 milestone Jan 17, 2020

WinterSilence reviewed Apr 10, 2021

View reviewed changes

WinterSilence reviewed Apr 12, 2021

View reviewed changes

uldisn added 2 commits April 12, 2021 23:04

Update Generator.php

5cda9d9

Update Generator.php

cb3d572

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Model rules for default values #420

Model rules for default values #420

uldisn commented Nov 16, 2019 •

edited

Loading

yii-bot commented Nov 18, 2019

schmunk42 commented Nov 18, 2019

uldisn commented Nov 18, 2019

samdark left a comment

schmunk42 commented Nov 19, 2019

samdark commented Nov 19, 2019

uldisn commented Nov 19, 2019

samdark commented Nov 19, 2019

uldisn commented Nov 19, 2019 •

edited

Loading

samdark commented Nov 19, 2019

uldisn commented Nov 20, 2019

samdark commented Nov 20, 2019

uldisn commented Nov 20, 2019 •

edited

Loading

uldisn commented Jan 17, 2020

WinterSilence commented Mar 8, 2021 •

edited

Loading

WinterSilence Apr 10, 2021 •

edited

Loading

uldisn Apr 12, 2021

WinterSilence Apr 12, 2021

uldisn Apr 12, 2021

WinterSilence Apr 12, 2021

uldisn commented Apr 12, 2021 •

edited

Loading

WinterSilence commented Apr 12, 2021

WinterSilence Apr 12, 2021

uldisn Apr 12, 2021

uldisn Apr 12, 2021

WinterSilence Apr 12, 2021 •

edited

Loading

WinterSilence Apr 12, 2021

uldisn Apr 12, 2021

uldisn Apr 12, 2021

WinterSilence Apr 12, 2021

uldisn Apr 12, 2021

WinterSilence Apr 12, 2021 •

edited

Loading

uldisn Apr 12, 2021

WinterSilence Apr 12, 2021 •

edited

Loading


		foreach ($table->columns as $column) {

		if ($column->defaultValue !== null) {

Model rules for default values #420

Are you sure you want to change the base?

Model rules for default values #420

Conversation

uldisn commented Nov 16, 2019 • edited Loading

Bug

Example of generated rules:

yii-bot commented Nov 18, 2019

schmunk42 commented Nov 18, 2019

uldisn commented Nov 18, 2019

samdark left a comment

Choose a reason for hiding this comment

schmunk42 commented Nov 19, 2019

samdark commented Nov 19, 2019

uldisn commented Nov 19, 2019

samdark commented Nov 19, 2019

uldisn commented Nov 19, 2019 • edited Loading

samdark commented Nov 19, 2019

uldisn commented Nov 20, 2019

samdark commented Nov 20, 2019

uldisn commented Nov 20, 2019 • edited Loading

uldisn commented Jan 17, 2020

WinterSilence commented Mar 8, 2021 • edited Loading

WinterSilence Apr 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uldisn commented Apr 12, 2021 • edited Loading

WinterSilence commented Apr 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WinterSilence Apr 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WinterSilence Apr 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WinterSilence Apr 12, 2021 • edited Loading

Choose a reason for hiding this comment

uldisn commented Nov 16, 2019 •

edited

Loading

uldisn commented Nov 19, 2019 •

edited

Loading

uldisn commented Nov 20, 2019 •

edited

Loading

WinterSilence commented Mar 8, 2021 •

edited

Loading

WinterSilence Apr 10, 2021 •

edited

Loading

uldisn commented Apr 12, 2021 •

edited

Loading

WinterSilence Apr 12, 2021 •

edited

Loading

WinterSilence Apr 12, 2021 •

edited

Loading

WinterSilence Apr 12, 2021 •

edited

Loading