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

After 2.3 improvements #230

Closed
4 tasks done
pjedrzejewski opened this issue Jul 19, 2013 · 18 comments
Closed
4 tasks done

After 2.3 improvements #230

pjedrzejewski opened this issue Jul 19, 2013 · 18 comments
Labels
Feature New feature proposals. RFC Discussions about potential changes or new features.

Comments

@pjedrzejewski
Copy link
Member

Hey,

Despite the delay, caused by various reasons (including both code and not code related stuff), I'm quite close to finishing the Symfony 2.3 upgrade.

I've made little mistake and did more than just sf version upgrade at once, thus I had to resolve much more issues than just compatibility. But with the upgrade, some new stuff is coming.

  1. Entity classes are gone! With Symfony 2.3, You extend Model classes directly, we removed bunch of code.

  2. Upgrade to latest phpspec! As you know, phpspec got a quite big update, with new mocking tool, new namespace and few BC breaks. All bundles used in main app are updated to use the latest version.

  3. New shipping engine.

a) cost calculators now do not require building the whole shipment, they use ShippingSubjectInterface, so any model which implements the interface can be used to calculate the costs.

b) New shipping method rules system. Similar to what we have for promotions. This gives the developer unlimited flexibility about filtering the shipping methods available for the user. Previously, only categories could be used to properly configure shipping. Now we have weight limits, order total limits, categories, item count limits, product dimensions + you can implement your own rules, just like for promotions.

c) These changes also affect the calculators, I'm implementing weight based cost calculator and so on... of course, like with anything else in Sylius, you can implement your own configurable cost calculator.

  1. AssortmentBundle got decoupled into ProductBundle and VariableProductBundle. In big shortcut, first one is about products and properties, second one works as a extension layer which adds options and variants.

  2. Validation groups. Overriding validation in Symfony2 isn't that easy as it may seem, especially that all our validation config was in "Default" group, which is always used during validation. This prevented you from changing the validation rules easily for Sylius models. From now on, thanks to work of @umpirsky and myself, all bundles use "sylius" group for mapping. And you can configure one or more groups directly in config. For example.

sylius_product:
    validation_groups:
        product:
            - acme

With this configuration, "acme" is used in product forms as the validation group. I hope it will solve some customization issues!

All bundles already are or will be tagged before 2.3 upgrades, so you can use them with 2.2 projects, but 2.2 will no longer be supported.

This is brief summary of Symfony 2.3 upgrade itself, more informations will come with a blog post, which I can't do now... you'll understand very soon why. :)

What after the upgrade? I'd like to list few things that require some work, to simplify our codebase.

  • Decoupling service registry pattern from all bundles. Shipping calculators, tax calculators, all kinds of checkers for the rules (shipping and promotions) as well as promotion actions uses their own registry classes and compiler passes. I believe we can remove this quite big duplication and move the related classes to Sylius\Component\DependencyInjection namespace. This should make few bundles a bit lighter and simpler.
  • Replacing the Doctrine RTEL compiler passes with usage of configuration prepend mechanisms (docs). Currently we use our own compiler pass which configures the doctrine listeners... so that we can define doctrine relations using interfaces, not concrete entities. Without that, to override any of the models, you would need to remap all related entities...
  • Abstracting common methods into base DI Extension class. As you may notice, all extension classes (DependencyInjection\SyliusXExtension where X is bundle name) contain some duplicated code for mapping classes to parameters, mapping validation groups, loading right driver file and so on. I believe we could abstract it to components too, as an abstract class - see 1st point.
  • Using listener to change the entity repository classes dynamically. We use repositories as services, which is my preferred method. But, we construct them manually (not via getRepository() method of ObjectManager). This allows us to easily override the repository class, from container, from configuration etc. But if someone uses the casual way, the getRepository method, he'll get the default repository class. What we can do is use a listener to change the class inside mapping. ("load metadata" event) and in service definition, use the factory method, to get the repository via object manager.

Let me know what do you think about these ideas!
Thank you for participating and being part of this awesome growing community, I'm more than happy I see so many smart people here. 👍

@silviuvoicu
Copy link

Is phpspec2 compatible with symfony 2.3 ?

@pjedrzejewski
Copy link
Member Author

@silviuvoicu I think that phpspec compatible with any 5.3.3+ library. If you think about Behat too, yes it is compatible.

@silviuvoicu
Copy link

I was trying to make a little project with symfony 2.3, but when I try to install with composer phpspec2, it complains about, and here https://packagist.org/packages/phpspec/phpspec2 it shows it's no compatible, yet. So, I have to downgrade to symfony 2.2.

@molchanoviv
Copy link

Let me know what do you think about these ideas!

I like them. And vote for them by two hands.

@docteurklein
Copy link
Contributor

@silviuvoicu you have to use phpspec/phpspec for 2.3. the precedent phpspec/phpspec2 is deprecated.

@pjedrzejewski
Copy link
Member Author

Ah, I wasn't aware of that. And when I said phpspec2, I was referring to phpspec/phpspec. :)

@pjedrzejewski
Copy link
Member Author

@molchanoviv I did not include other improvements which have their own issues on github, like the entities inheritance etc. Happy to hear it!

@docteurklein
Copy link
Contributor

@pjedrzejewski what about using the existing doctrine target resolver instead of yours ?
Also, wha'ts your idea behind prepend configs ?

@pjedrzejewski
Copy link
Member Author

@docteurklein Actually, we're using the existing doctrine target resolver, if you look here it is adding the proper listener tags and calls.

Of course, you can configure it in container configuration. The reason behind our own compiler pass to handle this, was that before "prepend configs" there was no way to configure this automatically. Every bundle classes/interface would need to be manually put in configuration by the developer during installation which is a bit tedious. Now, when we can use the "prepend" config mechanisms, we can configure the doctrine rtel automatically in the bundle container extension.

@dizda
Copy link

dizda commented Jul 19, 2013

Congrats :-)

@winzou
Copy link
Contributor

winzou commented Jul 19, 2013

This is great!
What about Sylius/SyliusResourceBundle#48? It would be awesome to have it integrated as well, so that we will all be able to use Sylius in our 2.3 projects ;)

@Eponymi
Copy link
Contributor

Eponymi commented Jul 19, 2013

But, what about payments/checkout process in the core...?

@jjanvier
Copy link
Contributor

Very good news. I like all the ideas. Two questions :

  1. How could we help to get these things done ?
  2. When will you publish the roadmap ? (I think this will answer Eponymi's question, al least I hope so).

@Francois-Francois
Copy link

Great !
Is multishop in Sylius's roadmap ?

@Eponymi
Copy link
Contributor

Eponymi commented Jul 20, 2013

@jjanvier I'm slowly realizing that the payments question will never be answered.

@jjanvier
Copy link
Contributor

@Eponymi keep faith, it will come !

@pjedrzejewski
Copy link
Member Author

@Eponymi @jjanvier Sorry guys, I just can't publish it yet. When it will be published, you'll understand why. Eventually, I can post it on github but I'm not sure I want to.

About your payments question, I already have an idea how we incorporate it into checkout without forcing Sylius to use any new/existing payments library and supporting different payment flows. I'll share the details of my idea (based on previous ideas, including yours RFC) as soon as I find time to write my thoughts down. I know that payments is big missing thing in main app. But please keep in mind, that while our bundles were started some time ago, this app is around 6 months old, where nobody was working on it full time, until now. :)

It's not the final roadmap I want to publish, I removed the dates for now and as you see some of the things are done already, but maybe it will give you some idea where stuff is. https://trello.com/b/X6tYxC24/roadmap

@empi89
Copy link

empi89 commented Aug 9, 2014

@pjedrzejewski the compiler pass is still there (https://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/ResourceBundle/DependencyInjection/Compiler/ResolveDoctrineTargetEntitiesPass.php), was the configuration prepend mechanism already implemented?

pamil pushed a commit to pamil/Sylius that referenced this issue Mar 21, 2016
Add an example for the variant generator and specify FQCNs used
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature proposals. RFC Discussions about potential changes or new features.
Projects
None yet
Development

No branches or pull requests

10 participants