Components, II attempt

[pjedrzejewski]

• tag all bundles
• review all bundles' composer.json
• review all components' composer.json
• review all bundles' CHANGELOG
• review all components' CHANGELOG
• rename Payments to Payment on packagist and github
• rename Promotions to Promotion on packagist and github
• rename Taxonomies to Taxonomy on packagist and github

[jjanvier]

@pjedrzejewski need help ? What's left to do ?

[jjanvier]

@pjedrzejewski should we provide a way to migrate the database ?

[pjedrzejewski]

@jjanvier I'll generate a migration and see how it works. :) Right now I'm adding UPGRADE file. @Sylius What do you think about releasing 0.9.0 tomorrow and merging this? I think we should do it tomorrow and move forward + do what is possible to help people upgrading. (it is not that hard... just boring changes)

[jjanvier]

@pjedrzejewski as long as you tag everything I guess you can merge. My migration was impossible. I had to drop my database, but I didn't look at what failed.

Until now, I've encountered only one bug : if you create a percentage attribute, you'll not be able to create a product. I'm looking at it. A few labels are broken also, but nothing serious. FIXED

[jjanvier]

Bug when I try to edit a choice attribute.

[jjanvier]

Bug: no validation constraint on the attribute type when you add a new attribute to the product (everything's OK when you edit an existing attribute)

[jjanvier]

@pjedrzejewski 1000 stargazers tomorrow \o/

[winzou]

I haven't checked anything on this branch, but if you consider it's quite OK then yes we should merge it (after 0.9 tag of course). We'll then work on master to polish everything, it will be clearer.

@jjanvier can you tell more about this migration that was impossible? This is the most annoying point...

[jjanvier]

@winzou I got this when I update a database schema that is up-to-date with the master branch :

  [Doctrine\DBAL\DBALException]                                                                         
  An exception occurred while executing 'ALTER TABLE sylius_product_prototype_option ADD CONSTRAINT FK  
  _1AD7AAC5A7C41D6F FOREIGN KEY (option_id) REFERENCES sylius_product_option (id)':                     

  SQLSTATE[HY000]: General error: 1005 Can't create table 'sylius_dev.#sql-a88_35' (errno: 150)    

[jjanvier]

@winzou That's because doctrine:schema:update generates a stupid batch. It tries to add a FK to sylius_product_prototype_option refering an attribute id than does not exist yet in the table sylius_product_option.

ALTER TABLE sylius_product_prototype_option ADD CONSTRAINT FK_1AD7AAC5A7C41D6F FOREIGN KEY (option_id) REFERENCES sylius_product_option (id);
// ...
// other updates
// ...
ALTER TABLE sylius_product_option ADD id INT AUTO_INCREMENT NOT NULL, ADD name 

[jjanvier]

Maybe we should consider using doctrine:migration from now on...

[pjedrzejewski]

@jjanvier That's what I meant with migrations. :)

[jjanvier]

Ah.. Maybe I should delete my comment so :)

[pjedrzejewski]

@jjanvier I added DoctrineMigrationsBundle and auto-generated migration. I'll try to modify it to preserve the data and just rename the tables.

[pjedrzejewski]

Heads up - I want to be absolutely sure that both Sylius 0.9.0 and Sylius-Standard 0.9.0 works properly, so I'll verify this and squash some minor issues during the weekend and merge this on Sunday. Blog post on Monday. :)

[jjanvier]

@pjedrzejewski need help for the migration script ? I can at least test a real migration process once this script will be finished

[pjedrzejewski]

@jjanvier If you have some time to play with it - yes, definitely!

[pjedrzejewski]

Github is showing weird diff stats after rebase...

[jjanvier]

Like what ?

[pjedrzejewski]

Should be more files and higher addition/deletion stats. Weird, it was showing correctly before.

[jjanvier]

It seems new components and bundles are gone...

[jjanvier]

That's why you should work on a fork ><

[pjedrzejewski]

I'll switch to fork yeah, but still this is not the problem - the bundles and components are there - https://github.com/Sylius/Sylius/tree/components-2/src/Sylius/Bundle/AttributeBundle.

[pjedrzejewski]

If you browse the branch and commits, everything is in place. It looks like PR is wrong.

[jjanvier]

Is it possible to open another PR from this branch ? Github can handle that ?

[jjanvier]

@pjedrzejewski maybe we should add another warning : "Please, test this migration in your stage environment before going to production."

I didn't have the time to test your upgrade procedure. Did someone have the time ?

[winzou]

The Doctrine Migration itself worked properly.

[jjanvier]

yes !

[winzou]

I'm not sure about the class Sylius\Component\Product\Model\Variable\VariableProduct being in the file src/Component/Variation/Model/Variable.php :)

[pjedrzejewski]

Done.

[makasim]

That's great work guys! Big step forward.

P.S. I waited with some good stuff till it is merged, lets do it now.

[pjedrzejewski]

@makasim Great! :)

[pjedrzejewski]

Thanks for the help guys!

[arnolanglade]

Good job guys !!

[pjedrzejewski]

I have a little issue with the subtree splits of renamed bundles, hopefully I'll fix this soon.

[winzou]

Awesome! Congrats guys!
Let's resume the business now =)

[rvanlaak]

Did read about decoupling MoneyBundle from PromotionsBundle in #1191 , is there a status update on that? 👍

Isn't it a matter about an extra $options key in ItemTotalConfigurationType to check or the MoneyBundle is available?