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

[WIP] Add Subscription Bundle #385

Closed

Conversation

Richtermeister
Copy link
Contributor

This goal of this PR is to add subscription features to Sylius, to support recurring orders, club-memberships, etc.

sylius_subscriptions1

sylius_subscriptions2

sylius_subscriptions3

The model revolves around subscriptions and subscription items, which on the application level can be linked specific products/variants. The scheduling mechanism is decoupled so that complex scheduling scenarios can be supported. A simple interval-based scheduler is included. The bundle also comes with a command and a processor harness to process subscriptions that are due. Actual processing logic is delegated to the application layer.

I also added a new section to the customer panel to allow customers to manage their subscriptions.

Feedback pleez!
Thanks! :)

Todos:

  • Make Interval Units a dropdown and configurable
  • Add twig extension to format interval unit + frequency, i.e. "Every 10 days / Every Month"
  • Add Behat Features
  • Add ability to create new subscriptions (from cart / during checkout)
  • Add ability to add items to existing subscription (how to select product variant?)
  • Add admin capabilities
  • Add log / retry / audit capabilities (optional, could be separate bundle)

@Richtermeister
Copy link
Contributor Author

Hi guys, thanks for checking this out. I hope this feature is in everybody's interest and not too opinionated.

I'm looking for feedback on whether this is the right approach, flexible enough, "Sylius worthy", etc.

With regards to the implementation, I'm not 100% sure about the Processor integration because it really only throws events, and I am also thinking about better interval handling which supports more flexible intervals. The current implementation works with only one unit (days), and it would be nice to support other intervals: First of every month, every Monday, each year on my Birthday.. so I'm wondering if the complex scheduling support should be delegated to a separate bundle which could also be used for other activities within Sylius (triggering emails, running batch operations, etc).

Thank you for feedback!

@pjedrzejewski
Copy link
Member

wow

I implemented similar system into client's Sylius app, but it could not be open sourced / extracted easily. I cried hard about that, but here you come and boom, awesome!

I hope to review this tomorrow. Sorry, today, it's 1AM here, snap...

Thanks Daniel, that look really great!

{
$output->writeln('Processing Subscriptions...');

$counter = new \stdClass();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using a simple array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You got it. Forgot about the "by reference" trick..

@winzou
Copy link
Contributor

winzou commented Oct 9, 2013

I had a very quick look, but awesome work so far!

*/
public function getProduct()
{
return $this->variant->getProduct();
Copy link
Contributor

Choose a reason for hiding this comment

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

What if variant was not yet set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I took inspiration from here: https://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/CoreBundle/Model/OrderItem.php#L34 Guess we should fix it there as well.

@Richtermeister
Copy link
Contributor Author

Hey guys, I just pushed a refactoring of the scheduling/processing code, and renamed a few interfaces/classes. (I took some inspiration from Magento).

This version should start to be pretty solid:

  • added Recurring/Schedulable interfaces so scheduling is more flexible and independent from entities
  • added lots more specs
  • recurring feature can be toggled with single configuration parameter

Also, thanks everybody for the feedback so far - I will make sure it will all be addressed.
Let me know if squashing the commits helps review this..

(todo list moved up top)

@pjedrzejewski
Copy link
Member

@Richtermeister Please do not squash these commits, they're fine and reflect the dev process. Usually I prefer to squash some CS fix commits, simple typos or 5 commits changing 1 line. :)

@amenophis
Copy link
Contributor

Oh i love this idea !!! 👍
I will use it as soon as it can be used :)

@Richtermeister
Copy link
Contributor Author

@pjedrzejewski Thanks man, and thanks for the tweet! :)

Glad you like the idea, hope this implementation covers the features you had in your project.
Any other proprietary features you'd like opensourced? ;)

@Richtermeister
Copy link
Contributor Author

@amenophis Thanks man. This should already be usable and I think it's starting to stabilize, you can give it a shot and let me know if it works for you or if you have unaddressed use-cases. Thanks! :)

{
if ($this->variant) {
return $this->variant->getProduct();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO you should do:

return null;

After that if just to have all things clear, and not guess.

@pjedrzejewski
Copy link
Member

Would be nice to add some Behat scenarios for the account panel. Also, I think we want to create and list subscriptions in backend, but I'm in favor of adding this functionality after redesign PR which I hope to merge tonight / tomorrow in worst case... (I need to make the login page prettier and working) Thoughts?

@Richtermeister
Copy link
Contributor Author

@stloyd Very good feedback man, can't sneak anything past you! ;) I hope I addressed it all. Thanks!

@Richtermeister
Copy link
Contributor Author

@pjedrzejewski Absolutely! What I am not so sure about is how to select a product(variant) when adding a new subscription item, but I guess I can use the same form / resolving logic used in the front-end?

Also, great to hear about the Bootstrap3 update. I am currently running BS3 in the front-end and the global form-theming is messing with the admin area. I guess the demo site will have the reverse problem until the front-end is upgraded, too? Maybe forms should not be globally themed, only in the template where they are used? /offtopic

@pjedrzejewski
Copy link
Member

Yeah, @stloyd's reviews are awesome, thanks for that man! :)

@Richtermeister I'll think about simplest way to select variant, we'll need the same thing for order creation... I think we should have a product selection autocomplete field, and then dynamically load options / variants selection fields. It can become complex, but I think we can do it in next iteration, it's cool as is. In my use case, subscriptions were created "in code", not via ui.

Regarding the B3 issue, I will take a look hard it will be to switch frontend to BS3, not changing the look, just updating the forms etc. I think I will be able to do it pretty easily so we avoid the conflicts.

Thanks for your work, I could give you a hand with Behat part, hm?

@Richtermeister
Copy link
Contributor Author

@pjedrzejewski Re: Behat, if you're offering, I'm taking! Thanks! :) I'm sure I'll get another chance to learn it better..
Overall, are you happy with how this bundle is structured (all the interfaces, scheduler, processor, etc.., or should I rework anything?)

@@ -0,0 +1,72 @@
<?php

namespace Sylius\Bundle\CoreBundle\DataFixtures\ORM;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing license header.

@amenophis
Copy link
Contributor

Any chance to see this PR merged soon ?

@pjedrzejewski
Copy link
Member

@amenophis I'd love to have that in core, but it requires review and this functionality is not a priority at this moment. I greatly appreciate all work done here and this will land in master, but can't estimate when... When we are done with current things. (search, api, new frontend/backend theme, multi-store)

@MichaelMackus
Copy link
Contributor

@amenophis an alternative would be to use the 2 repos I linked to above... you should be able to add those to your composer.json and implement the necessary forms (from CoreBundle) in your application.. this is how we're doing it in one of our sites since they need this feature now

There's still things to be done, though... the payment processing & storage still needs to be implemented, for example. But I think this should be a good start, and the subscription frontend seems to be working well.

@pjedrzejewski if there's anything I can do to speed up the merge let me know

@arnolanglade
Copy link
Contributor

@pjedrzejewski search, api, new frontend/backend theme, multi-store and the doc ;)

@adremasu
Copy link

any news about this issue?

@drewmach
Copy link

I just want to give a shout out that this is functionality I am looking for in Sylius as well. I will give a shot at trying to implement the repo's linked above and add the missing functionality, but would be awesome if this would be implemented in the core.

@pjedrzejewski pjedrzejewski removed this from the v1.0.0-ALPHA1 milestone May 11, 2015
@pjedrzejewski pjedrzejewski added New Feature and removed Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). labels Dec 14, 2015
@kzap
Copy link

kzap commented Mar 15, 2016

@pjedrzejewski
so subscriptions were never merged in?

is there any core component for this even?

or is subscriptions considered an archetype or a special pricing calculator on a product?

whats the easiest way one would extend the core to have subscriptions?

@MichaelMackus is this still being developed here or where should PR's be submitted?

@MichaelMackus
Copy link
Contributor

@kzap development for this is being continued in the repos linked above.. the PR is on a hiatus until dev team is interested in merging

@michalmarcinkowski
Copy link
Contributor

@MichaelMackus @kzap unfortunately this feature will not land on master in Sylius 1.0 release. We're in stabilization phase and as @pjedrzejewski comment here we have other more important features to polish. We will be interested to resume this work later on ;)

@kzap
Copy link

kzap commented Mar 16, 2016

Thanks for keeping a updated repo..ill use yours then
On Mar 16, 2016 12:07 AM, "Michał Marcinkowski" notifications@github.com
wrote:

@MichaelMackus https://github.com/MichaelMackus @kzap
https://github.com/kzap unfortunately this feature will not land on
master in Sylius 1.0 release. We're in stabilization phase and as
@pjedrzejewski https://github.com/pjedrzejewski comment here
#385 (comment) we have
other more important features to polish. We will be interested to resume
this work later on ;)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#385 (comment)

pamil pushed a commit to pamil/Sylius that referenced this pull request Mar 21, 2016
@pamil pamil added Feature New feature proposals. and removed Feature New feature proposals. Future labels Oct 26, 2017
@xrow
Copy link

xrow commented Nov 23, 2017

Hi, I have an requirement on this feature. The last update is already long ago. I am wondering, if one can provide an update on the roadmap / timings. So far is looks like that this will never get merged since it is so old. Please don't mind asking. I am just curious. Thx.

@lchrusciel
Copy link
Member

I'm not sure if there is any point in keeping this one open. Too many things have changed. Thus it is a good plugin proposal :)

@lchrusciel lchrusciel closed this Nov 28, 2017
@gadelkareem
Copy link

hmm, any plugin already available providing this feature?

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

Successfully merging this pull request may close these issues.

None yet