Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[review] - migration fix to be consistent with model #4254

Merged

Conversation

nakashu
Copy link
Contributor

@nakashu nakashu commented Feb 22, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets #4248
License MIT

@@ -30,7 +30,7 @@ public function up(Schema $schema)
$this->addSql('CREATE TABLE sylius_product_review (id INT AUTO_INCREMENT NOT NULL, product_id INT NOT NULL, author_id INT DEFAULT NULL, title VARCHAR(255) NOT NULL, rating INT NOT NULL, comment LONGTEXT DEFAULT NULL, status VARCHAR(255) NOT NULL, created_at DATETIME NOT NULL, updated_at DATETIME DEFAULT NULL, INDEX IDX_C7056A994584665A (product_id), INDEX IDX_C7056A99F675F31B (author_id), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = InnoDB');
$this->addSql('ALTER TABLE sylius_product_review ADD CONSTRAINT FK_C7056A994584665A FOREIGN KEY (product_id) REFERENCES sylius_product (id) ON DELETE CASCADE');
$this->addSql('ALTER TABLE sylius_product_review ADD CONSTRAINT FK_C7056A99F675F31B FOREIGN KEY (author_id) REFERENCES sylius_customer (id)');
$this->addSql('ALTER TABLE sylius_product ADD average_rating DOUBLE PRECISION DEFAULT NULL');
$this->addSql('ALTER TABLE sylius_product ADD average_rating DOUBLE PRECISION NOT NULL');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about ...DOUBLE PRECISION NOT NULL DEFAULT 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can change it, but this is how doctrine would generate it on its own, and also how the mapping is set currently

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

@nakashu nakashu force-pushed the fix/average_rating-review-migration branch from 78d8e95 to fa61d83 Compare February 22, 2016 13:54
pjedrzejewski pushed a commit that referenced this pull request Feb 23, 2016
…tion

[review] - migration fix to be consistent with model
@pjedrzejewski pjedrzejewski merged commit fc7fd44 into Sylius:master Feb 23, 2016
@pjedrzejewski
Copy link
Member

Thanks @nakashu! I am not sure we should define default values in Doctrine mapping, but I haven not idea for a better fix atm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants