Canceled subscriptions are now always inactive

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ### Unreleased
 
+* Subscriptions with `status: :canceled` and `ends_at: future` are now considered canceled. Previously, these were considered active to accomodate canceling a Braintree subscription during trial (and allowing the user to continue using until the end of the trial).
+
 ### 6.8.1
 
 * [Stripe] Skip sync if object is not attached to a customer. Fixes #842

app/models/pay/subscription.rb

@@ -8,10 +8,11 @@ class Subscription < Pay::ApplicationRecord
 
     # Scopes
     scope :for_name, ->(name) { where(name: name) }
-    scope :on_trial, -> { where.not(trial_ends_at: nil).where("#{table_name}.trial_ends_at > ?", Time.current) }
-    scope :cancelled, -> { where.not(ends_at: nil) }
-    scope :on_grace_period, -> { cancelled.where("#{table_name}.ends_at > ?", Time.current) }
-    scope :active, -> { where(status: ["trialing", "active", "canceled"], ends_at: nil).pause_not_started.or(on_grace_period).or(on_trial) }
+    scope :on_trial, -> { where("trial_ends_at > ?", Time.current) }
+    scope :canceled, -> { where.not(ends_at: nil) }
+    scope :cancelled, -> { canceled }
+    scope :on_grace_period, -> { where("#{table_name}.ends_at IS NOT NULL AND #{table_name}.ends_at > ?", Time.current) }
+    scope :active, -> { where(status: ["trialing", "active"]).pause_not_started.where("#{table_name}.ends_at IS NULL OR #{table_name}.ends_at > ?", Time.current).where("trial_ends_at IS NULL OR trial_ends_at > ?", Time.current) }
     scope :paused, -> { where(status: "paused").or(where("pause_starts_at <= ?", Time.current)) }
     scope :pause_not_started, -> { where("pause_starts_at IS NULL OR pause_starts_at > ?", Time.current) }
     scope :active_or_paused, -> { active.or(paused) }
@@ -106,9 +107,9 @@ def ended?
 
     # If you cancel during a trial, you should still retain access until the end of the trial
     # Otherwise a subscription is active unless it has ended or is currently paused
-    # Check the subscription status so we don't accidentally consider "incomplete", "past_due", or other statuses as active
+    # Check the subscription status so we don't accidentally consider "incomplete", "unpaid", or other statuses as active
     def active?
-      ["trialing", "active", "canceled"].include?(status) &&
+      ["trialing", "active"].include?(status) &&
         (!(canceled? || paused?) || on_trial? || on_grace_period?)
     end
 
@@ -160,9 +161,7 @@ def latest_payment
     private
 
     def cancel_if_active
-      if active?
-        cancel_now!
-      end
+      cancel_now! if active?
     rescue => e
       Rails.logger.info "[Pay] Unable to automatically cancel subscription `#{customer.processor} #{id}`: #{e.message}"
     end

lib/pay/braintree/subscription.rb

@@ -77,6 +77,9 @@ def subscription(**options)
       end
 
       def cancel(**options)
+        return if canceled?
+
+        # Braintree doesn't allow canceling at period end while on trial, so trials are canceled immediately
         result = if on_trial?
           gateway.subscription.cancel(processor_id)
         else
@@ -90,6 +93,8 @@ def cancel(**options)
       end
 
       def cancel_now!(**options)
+        return if canceled?
+
         result = gateway.subscription.cancel(processor_id)
         pay_subscription.sync!(object: result.subscription)
       rescue ::Braintree::BraintreeError => e

lib/pay/fake_processor/subscription.rb

@@ -29,6 +29,8 @@ def subscription(**options)
       # With trial, sets end to trial end (mimicing Stripe)
       # Without trial, sets can ends_at to end of month
       def cancel(**options)
+        return if canceled?
+
         if pay_subscription.on_trial?
           pay_subscription.update(ends_at: pay_subscription.trial_ends_at)
         else
@@ -37,6 +39,8 @@ def cancel(**options)
       end
 
       def cancel_now!(**options)
+        return if canceled?
+
         ends_at = Time.current
         pay_subscription.update(
           status: :canceled,

lib/pay/paddle/subscription.rb

@@ -82,16 +82,9 @@ def subscription
       end
 
       def cancel(**options)
-        if on_trial?
-          trial_ends_at
-        elsif paused?
-          pause_starts_at
-        else
-          processor_subscription.next_payment&.fetch(:date) || Time.current
-        end
+        return if canceled?
 
         response = ::Paddle::Subscription.cancel(id: processor_id, effective_from: "next_billing_period")
-
         pay_subscription.update(status: :canceled, ends_at: response.scheduled_change.effective_at)
 
         # Remove payment methods since customer cannot be reused after cancelling
@@ -101,6 +94,15 @@ def cancel(**options)
       end
 
       def cancel_now!(**options)
+        return if canceled?
+
+        PaddlePay::Subscription::User.cancel(processor_id)
+        pay_subscription.update(status: :canceled, ends_at: Time.current)
+
+        # Remove payment methods since customer cannot be reused after cancelling
+        Pay::PaymentMethod.where(customer_id: pay_subscription.customer_id).destroy_all
+      rescue ::PaddlePay::PaddlePayError => e
+        raise Pay::Paddle::Error, e
       end
 
       def change_quantity(quantity, **options)

lib/pay/paddle_classic/subscription.rb

@@ -77,6 +77,8 @@ def subscription(**options)
       end
 
       def cancel(**options)
+        return if canceled?
+
         ends_at = if on_trial?
           trial_ends_at
         elsif paused?
@@ -95,6 +97,8 @@ def cancel(**options)
       end
 
       def cancel_now!(**options)
+        return if canceled?
+
         PaddlePay::Subscription::User.cancel(processor_id)
         pay_subscription.update(status: :canceled, ends_at: Time.current)
 

lib/pay/stripe/subscription.rb

@@ -159,6 +159,8 @@ def client_secret
       end
 
       def cancel(**options)
+        return if canceled?
+
         @stripe_subscription = ::Stripe::Subscription.update(processor_id, {cancel_at_period_end: true}.merge(expand_options), stripe_options)
         pay_subscription.update(ends_at: (on_trial? ? trial_ends_at : Time.at(@stripe_subscription.current_period_end)))
       rescue ::Stripe::StripeError => e
@@ -170,6 +172,8 @@ def cancel(**options)
       # cancel_now!(prorate: true)
       # cancel_now!(invoice_now: true)
       def cancel_now!(**options)
+        return if canceled?
+
         @stripe_subscription = ::Stripe::Subscription.cancel(processor_id, options.merge(expand_options), stripe_options)
         pay_subscription.update(ends_at: Time.current, status: :canceled)
       rescue ::Stripe::StripeError => e

test/models/pay/subscription_test.rb

@@ -305,6 +305,24 @@ class Pay::Subscription::Test < ActiveSupport::TestCase
     refute @subscription.active?
   end
 
+  test "past_due" do
+    @subscription.update!(status: :past_due)
+    refute @subscription.active?
+    assert_not_includes @pay_customer.subscriptions.active, @subscription
+  end
+
+  test "unpaid" do
+    @subscription.update!(status: :unpaid)
+    refute @subscription.active?
+    assert_not_includes @pay_customer.subscriptions.active, @subscription
+  end
+
+  test "canceled subscriptions with a future ends_at are inactive" do
+    @subscription.update(status: :canceled, ends_at: 1.hour.from_now)
+    refute @subscription.active?
+    assert_not_includes @pay_customer.subscriptions.active, @subscription
+  end
+
   test "cancel" do
     @subscription.cancel
     assert @subscription.ends_at?

test/pay/attributes_test.rb

@@ -27,9 +27,29 @@ class Pay::AttributesTest < ActiveSupport::TestCase
   end
 
   test "deleting user doesn't remove pay customers" do
-    Pay::Subscription.any_instance.expects(:cancel_now!)
-    assert_no_difference "Pay::Customer.count" do
-      users(:stripe).destroy
+    user = users(:stripe)
+    ::Stripe::Subscription.stub(:cancel, user.payment_processor.subscription) do
+      assert_no_difference "Pay::Customer.count" do
+        user.destroy
+      end
+    end
+  end
+
+  test "deleting user cancels subscriptions" do
+    user = users(:stripe)
+    ::Stripe::Subscription.stub(:cancel, user.payment_processor.subscription) do
+      assert user.payment_processor.subscription.active?
+      user.destroy
+      refute user.payment_processor.subscription.active?
+    end
+  end
+
+  test "deleting user ignores canceled subscriptions" do
+    user = users(:stripe)
+    ::Stripe::Subscription.stub(:cancel, user.payment_processor.subscription) do
+      user.payment_processor.subscription.cancel_now!
+      refute user.payment_processor.subscription.active?
+      user.destroy
     end
   end
 

test/pay/billable_test.rb

@@ -83,7 +83,7 @@ class Pay::Billable::Test < ActiveSupport::TestCase
 
   test "subscribed? with canceled subscription on grace period" do
     @user.payment_processor.subscription.update(status: :canceled, ends_at: 1.day.from_now)
-    assert @user.payment_processor.subscribed?
+    refute @user.payment_processor.subscribed?
   end
 
   test "subscribed? with different plan" do

test/pay/braintree/subscription_test.rb

@@ -49,9 +49,11 @@ class Pay::Braintree::SubscriptionTest < ActiveSupport::TestCase
       pay_subscription.cancel_now!
 
       # Canceling during a trial ends the subscription, but continues to give access during the trial period
-      assert pay_subscription.active?
+      assert_equal "canceled", pay_subscription.status
+      refute pay_subscription.active?
       assert pay_subscription.on_trial?
       assert pay_subscription.ended?
+      assert_not_includes @pay_customer.subscriptions.active, pay_subscription
     end
   end
 
@@ -130,10 +132,12 @@ class Pay::Braintree::SubscriptionTest < ActiveSupport::TestCase
 
       pay_subscription = Pay::Braintree::Subscription.sync(processor_id)
 
-      # Canceling during a trial ends the subscription, but continues to give access during the trial period
-      assert pay_subscription.active?
+      # Canceling during a trial ends the subscription, but continues to give acess during the trial period
+      assert_equal "canceled", pay_subscription.status
+      refute pay_subscription.active?
       assert pay_subscription.on_trial?
       assert pay_subscription.ended?
+      assert_not_includes @pay_customer.subscriptions.active, pay_subscription
     end
   end
 end