[WIP] Use Order model

[winzou]

This PR refers to: https://github.com/Sylius/SyliusSalesBundle/issues/30 The aim is to delete the duplication between Cart and Sales bundle.

First step is updating Order model to be able to deal with Cart system. This is done by Sylius/SyliusSalesBundle#31 and this current PR depends on it.

Second step is using Order model from CartBundle. This is what this PR does.

Todo:

• I just modified all Cart and CartItem instances to Order and OrderItem
• Re-add Cart and CartItem models to keep Cart specific logic in this bundle and leave SalesBundle clean
• Use dbu's solution to remove Cart entity and solve the model extending issue

[pjedrzejewski]

I restarted the build as it errored cause of some github random error.

[pjedrzejewski]

You also need to add the sales bundle as dependency to composer.json. Thanks for your work! That's huge help as I assigned this task to me, so I can do other important Sylius tasks! Few things I'm thinking about before we merge this.

• Tagging the bundles.
• Updating cart bundle documentation.

And most important one - maybe we could keep the CartInterface, which would extend the OrderInterface - if you wonder why - it simply would allow us to keep sales bundle clearly separated from cart, so in the end we'd get the ideal result we wanted - cart bundle using sales bundle as a base to avoid duplication! What do you think? It's also very important to have all specs passing, I can help you with that once we get all things right. :)

[winzou]

Uhm, I can make a CartInterface extending OrderInterface without adding any method (for now). But the thing is that the Order model can't implement this interface (otherwise CartBundle will also become a dependency for SalesBundle). What do you have in mind?

[pjedrzejewski]

Hm... I was thinking this way - sales bundle remains only about orders, so no cart specific stuff (like expiration time). And cart bundle provides a bit "smarter" model implementing CartInterface, and in core, we would have Order which extends the model from cart bundle. I just want to keep things in places they belong/separated. Moving cart specific stuff to proper bundle, also allows us to use CartInterface as typehint in cart services, which feels a bit more natural. There might be a problem I don't see... my goal is to keep sales bundle separated from cart, and make cart bundle using sales bundle as the base.

[winzou]

You're totally right, I missed that we can extend the Cart model from CoreBundle:Order. I'm fixing this ;)

[winzou]

I have a problem in entities inheritance: Core/Entity/Order (entity) extends Cart/Entity/Cart (mapped superclass) extends Cart/Model/Cart extends Sales/Model/Order.

Problem is that entity Core/Entity/Order doesn't extends mapped superclass Sales/Entity/Order so that Doctrine wants to delete all persisted attributes included in it.

Cart/Model/Cart extends Sales/Entity/Order (instead of Sales/Model/Order) would solve the problem but this is of course not a solution.

I may be blind but I don't see how to solve this issue. Idea?

[pjedrzejewski]

Currently, what I'd do (if you agree) is to extend only the models, and just copy the mapping... The ultimate solution for this annoying problem is here - FriendsOfSymfony/FOSUserBundle#1081. Thoughts?

[dbu]

that is pretty ready now, orm and symfony 2.3 already merged it.
question is what we do for "legacy" (like symfony 2.2 and older, or any not bleeding-edge doctrine bundle), see also doctrine/DoctrineBundle#186 - if you want i can dump the legacy support code in a little repo in dbu namespace so you could use it right away.

[dbu]

or you can copy-paste the code from fos user bundle but that ends up in that code copy-pasted to a lot of places.

[pjedrzejewski]

We're doing quite significant changes here so I'll tag proper versions with 2.2 support. And our target is 2.3 - there is last beta now so I think we'll stick to 2.3. I also want to remove the entity classes, not deprecate them.

Thanks @dbu for clarifying - that's great feature - thanks for your work! I was looking forward for this solution for pretty long time. :)

@winzou What are your thoughts on this?

[dbu]

cool. if you have any worries, ping me. and feel free to improve the
cookbook entry on how to use this all
http://symfony.com/doc/master/cookbook/doctrine/mapping_model_classes.html

[winzou]

@dbu thanks a lot for that!

@pjedrzejewski I think we shouldn't care about 2.2, so I'm totally in favor of removing entity classes and use dbu's solution. Just tag the last 2.2 compatible versions, and we can move forward.

[pjedrzejewski]

@winzou Agree, I'm almost done with refactoring of assortment bundle, where I can try this compiler pass, and work out good way to include it in other bundles. I should have this finished today's evening, sounds good? Then we can do the same with cart & sales bundle... and then with other bundles.

[winzou]

There are several Composer issues to use Symfony 2.3, could you please remove all <2.3-dev in Sylius' composer.json files? FOSUB 1.3.1 is obsolete too, we have to use 2.*@dev. And phpspec 2 has moved to phpspec/phpspec instead of the current weird repo phpspec/phpspec2 (which includes a <2.3-dev constraint).

I think this is the moment we tag 2.2 and switch to 2.3 ;) We will then move forward faster!

[winzou]

Github is stupid in the diff view: it says 23 additions and 23 deletions in Entity/Cart.php, whereas in the end I haven't touched this file. Is there a way to make it understand that and simplify the diff?

[pjedrzejewski]

I started from resource bundle, v0.2.0 is compatible with Symfony <2.3, now bundles should depend on 0.3.* version of it. I'm updating other bundles too.

[winzou]

I've seen, this is great. I let you update all bundles.

Then there is just this compiler pass stuff to add and it will be good to merge. Already working like a charm in my app.

[albanR]

Wow, so much going on, leaving a week and it's hard to follow all the changes...
About this tag about symfony 2.2 compatibility, how does it work? Sorry if it's a dumb question but I've never been confronted to a bundle with such intense dev happening!

[pjedrzejewski]

@albanR The idea is to tag a Symfony2.2 compatible version and switch to 2.3. I don't think we can afford maintaining separate branch to support 2.2 for longer time as there are a lot of deprecated code and this would be really expensive to do it, especially when Symfony2.3 is around the corner.

So you'll be able to use all bundles with 2.2, but to get latest patches you'll need to upgrade your project to 2.3, which seems to be much easier than upgrading from 2.0 to 2.1 for example. (I remember form BC breaks :D)

[pjedrzejewski]

And welcome back @albanR! :)

[albanR]

Thanks! So I have nothing to do and just move to 2.3 when I want to. That's the kind of answer I like :)

[pjedrzejewski]

I'll take it from here, will merge your branch into 2.3 update. Thanks!