Don't modify trial_ends_at in the cancel_now method unless the subscription is actually on trial

[mpvosseller]

Pull Request

Summary:

This updates Pay::Stripe::Subscription#cancel_now! so it will not set (modify) the trial_ends_at property unless the subscription is currently on_trial?

We had been using trial_ends_at to indicate whether a subscription was a trial but found this was not working on subscriptions where cancel_now! was called.

Related Commit
5a719ad

Related Issue:

Description:

Testing:

I'm not quite sure if / how I can add a new VCR cassette? Is that something you would do or can you provide me a reference / pointer to do so?

Screenshots (if applicable):

Checklist:

• Code follows the project's coding standards
• Tests have been added or updated to cover the changes
• Documentation has been updated (if applicable)
• All existing tests pass
• Conforms to the contributing guidelines

Additional Notes:

[excid3]

Hmm, why are you cancelling the subscription immediately instead of on period end?

[mpvosseller]

Hmm, why are you cancelling the subscription immediately instead of on period end?

I guess the most common general use case would be when a customer "forgets to cancel" and customer support graciously decides to "cancel_now" and issue a partial refund.

I assumed setting trial_ends_at in cancel_now! was just an oversight. If you don't think we should be relying on trial_ends_at as an indication of the subscription having been a trial we could probably use the Stripe object's trial_end property instead now that the full Stripe object is in the database.

[excid3]

I'm not 100% sure I understand the situation but cancel now's intention is to completely cancel trials or active subscriptions.

[mpvosseller]

Sorry if I'm not being clear. Let me rephrase the problem.

Let's say a user creates a regular (non-trial) subscription and at some point we call subscription.cancel (to cancel at end of billing period) the trial_ends_at property stays nil forever.

Now let's say another user creates a regular (non-trial) subscription and at some point we call subscription.cancel_now! (to cancel immediately) the trial_ends_at gets set to the current time.

Why does the pay gem set trial_ends_at when canceling with one method (cancel_now!) but not with the other (cancel)? And when neither subscription was a trial?

[excid3]

Ah yep, I get you now. I agree we shouldn't set trial_ends_at if there wasn't one.

[excid3]

The current implementation is also incorrect. The values set should match what's returned from the API since the webhooks will update the values anyways. Currently this will set different times for each.

[mpvosseller]

Thank you @excid3

[excid3]

Thanks for catching that @mpvosseller! I meant to give you credit in the commit, sorry! 🙃