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

Change plan and Cancel subscription have inconsistent subscription end behaviors #61

Closed
dollydagr opened this issue Nov 16, 2013 · 12 comments
Assignees
Labels
discussion The premise or details of this issue is open for discussion

Comments

@dollydagr
Copy link
Contributor

Hi @pydanny,

What I mean by that is that:

  • a) "Cancel subscription" ends the subscription at the period end,
  • b) whereas "Change plan" allows going from a plan to another right away while being credited in case of a downgrade (which is what I find inconsistent), but also prorating in case of an upgrade.

I don't know if this behavior was meant to be the way it is. To make things consistent, I believe either:

  • configuration a) should call cancel_subscription(at_period_end=False) (instead of True) in CancelSubscriptionView in views, or
  • configuration b) should assign prorate=False in cu.update_subscription in Customer.subscribe in models.

For my project, I opted for the latter.
What do you think?

@dollydagr
Copy link
Contributor Author

@ghost ghost assigned dollydagr Dec 5, 2013
@dollydagr
Copy link
Contributor Author

For the cancellation policy, and @pydanny mentioned this in a TODO comment in views.py, let's pass at_period_end in and control it from settings. And since we're there, we should similarly take care of prorate (which is set when a customer subscribes to a plan in models.py).

The thing is these two are mutually exclusive: if a provider allows proration on cancellation, then a subscription should end right away, i.e. if proration policy is True, then cancel at_period_end is False, and vice versa.

What is a clean way to parametrize this?

In settings, having the following and commenting it would do the job, but would be ugly:

PRORATION_POLICY = False
CANCELLATION_AT_PERIOD_END = not PRORATION_POLICY

Adding a self-guard method that checks that the two are indeed mutually exclusive and raises an exception otherwise could add some robustness.

Another suggestion?

@mlvn23
Copy link

mlvn23 commented Apr 29, 2014

Hi,

I've looked into this for the past few days, and if my interpretation is correct, most membership website have a policy like this:

  1. When a customer upgrades to a higher plan, set the benefits to the upgraded plan immediately and prorate the charge.
  2. When a customer downgrades to a lower plan or cancels, don't change the benefits until the next billing cycle, and don't refund any prorated charges either.

I think a policy like this solves two issues:

  1. It avoids money outflow while being fair with the customer
  2. It prevents a loophole in the current scheme wherein a customer could "upgrade" (without prorated charge) 1 day after the billing date, and "downgrade" 1 day before the billing date and repeat this over and over again. This assumes that as soon as the customer upgrades, they get the higher benefit right away (which is what's expected).

Right now, I can't implement this scheme. So I propose we make the prorate a parameter to the subscribe function with a default value of False to be compatible with the current code base.

Thanks,

Melvin

@dollydagr
Copy link
Contributor Author

Thank you @mlvn23 for looking into this.

The current scheme is already defaulted to False as for the proration policy and consequently makes a customer switch from their current plan to an upgraded or downgraded one while starting and being charged right away for the new plan.

The model you're describing makes better sense and is consistent with Stripe's model:

I was also wondering how Stripe manages the fees that it charges for each transaction and whether the provider were to be held liable when a customer upgrades their plan. In fact, no, as long as the customer's balance contains charges that were not already refunded. So crediting a customer by refunding those (or part of those) charges back implies also getting the provider refunded for Stripe's fees (https://stripe.com/docs/api#refund_charge).

@mlvn23
Copy link

mlvn23 commented May 1, 2014

Hi @dollydagr,

What I meant was to change the function prototype as follows so that when a customer upgrades, we can prorate it. As it is, we have no control over proration from an external app.

  • def subscribe(self, plan, quantity=1, trial_days=None, charge_immediately=True):
  • def subscribe(self, plan, quantity=1, trial_days=None, charge_immediately=True, prorate=False)

It's actually not fair to charge the customer when he upgrades the plan without proration. For example, on day 1, he buys silver plan which should be good for 30 days. On day 2, he upgrades to gold plan. He'll be charged for the gold plan without being compensated by the 29 days that he paid for the silver plan.

Thanks,

mlvn23

@dollydagr
Copy link
Contributor Author

Oh I see your point now.

So, at this point, we're pretty close to getting the upgrade scenario implementation specified: The one spot where I see we should verify if we are in an upgrade situation is in ChangePlanView class in views.py. If we are, then we call the subscribe method with prorate=True.
The downgrade scenario is more complicated to implement.

But now I wonder:

  • if is preferable to hardcode this policy in this way and impose it to everybody, or if is worth putting another parameter (similar to PRORATION_POLICY above) that can be controlled in settings? If the latter, what would you suggest as an understandable/fairly intuitive name? PRORATION_POLICY_FOR_UPGRADE?
  • would it be acceptable to integrate this change for upgrades in the code base and leave downgrades as is for now?

@mlvn23
Copy link

mlvn23 commented May 2, 2014

I think the upgrade / downgrade policies are tied to the company policy, so we definitely can't force it to everybody. Also, for backward compatibility, we might want to leave the current behavior as it is. I think the important thing right now is for the API to allow proration so that it's possible to implement the "standard" scheme. In the long run, it might be better to implement the policies as an external app (like django-billing and django-plans).

Actually, the downgrade API (from the model perspective... I'm using custom views) already supports it (prorate=False), so it's possible to implement the "standard" scheme. The complication happens on how to keep track of the "features / quota" when the "last paid subscription level" is not the same as the "next subscription level," but this is app-specific and I don't think it should be in djstripe. It's actually quite complex and I ended up implementing it with a state machine.

@pydanny
Copy link
Contributor

pydanny commented May 2, 2014

I've got a couple TODOs in the docs that are related to this issue: http://dj-stripe.readthedocs.org/en/latest/settings.html#djstripe-proration-policy-false

😉

@dollydagr
Copy link
Contributor Author

Nice! @pydanny, let me know if I can contribute to the docs.

@pydanny
Copy link
Contributor

pydanny commented May 2, 2014

Please do! I think you have a clearer understanding of what this does.

@kavdev kavdev added the discussion The premise or details of this issue is open for discussion label Apr 24, 2015
@kavdev
Copy link
Member

kavdev commented Apr 21, 2016

This seems to have been implemented according to @pydanny's link to the docs above. Point 2 of that setting now works correctly.

@kavdev kavdev closed this as completed Apr 21, 2016
@dsppatel
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion The premise or details of this issue is open for discussion
Projects
None yet
Development

No branches or pull requests

5 participants