pay icon indicating copy to clipboard operation
pay copied to clipboard

Fix trial_ends_at being set to the subscriptions cancellation time when the cancellation happens after the end of the trial

Open feliperaul opened this issue 1 year ago • 0 comments

While debugging some bugs in our app, we arrived this piece of code when syncing a subscription:

https://github.com/pay-rails/pay/blob/4c60f16da2a446ecf0bc40b025d2a8f9a4dc7057/lib/pay/stripe/subscription.rb#L46-L49

This is a somewhat recent addition from https://github.com/pay-rails/pay/commit/ed10e864503a364f1e0f17355a7094f89579dea2, due to fix issue https://github.com/pay-rails/pay/issues/601.

However, I'd like to propose this to be reconsidered; I think the correct logic here should be:

  1. If a subscription is being canceled BEFORE trial_ends_at, then yes, the current behavior is desired (trial_ends_at should be set to match the subscription's ends_at); afterall, if you cancel a subscription during the trial period, the trial period is ending at that same instant as well; so, we're persisting accurate information in the database;
  2. However, if the subscription is being canceled AFTER trial_ends_at, there's simply no point in updating trial_ends_at; not only there's no need to do so to fix the bug at #601 (because trial_ends_at will NOT be at the future in this case), but we're also persisting inaccurate information in the database, because the trial DID NOT end at that time.

The trial_ends_at field in the Subscription model should hold the true timestamp of when that subscriptions trial ended. IF the subscription was canceled before the trial_ends_at, it should be updated to match the subscription's cancellation (like it is being now, because the trial is ending prematurely with the subscription's ending).

But this should not happen if the subscription was canceled AFTER the trial ended, because then we're persisting misleading information of when that trial actually ended (it didn't end with the subscriptions cancelletion; it ended before).

Imagine if a user trials for 7 days, then subscribes regularly for 2 years, and then cancels; I don't think we want to have subscription#trial_ends_at reporting that the user just ended his trial (his trial actually ended 2 years ago). If we code a report "show me users that canceled their subscription on their last day of trial", this user would be reported incorrectly.

This PR fixes this, and changes an existing test that asserted test sync stripe subscription set trial_ends_at when subscription canceled after trial end to test the exact opposite.

feliperaul avatar Jul 18 '22 23:07 feliperaul