Skip to content

Fix incomplete fulfillment update on subscription plan upgrade #24

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.

Sign up for GitHub

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

Merged
rameerez merged 1 commit into from
Dec 29, 2025
Merged

Fix incomplete fulfillment update on subscription plan upgrade #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 53 additions & 2 deletions lib/usage_credits/models/concerns/pay_subscription_extension.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,16 @@ def handle_plan_change
fulfillment = UsageCredits::Fulfillment.find_by(source: self)
return unless fulfillment

# Debug logging to track plan changes and potential issues
Rails.logger.info "=" * 80
Rails.logger.info "[UsageCredits] Plan change detected for subscription #{id}"
Rails.logger.info " Processor plan changed: #{saved_change_to_processor_plan.inspect}"
Rails.logger.info " Subscription status: #{status}"
Rails.logger.info " Current period end: #{current_period_end}"
Rails.logger.info " Fulfillment metadata: #{fulfillment.metadata.inspect}"
Rails.logger.info " Fulfillment period: #{fulfillment.fulfillment_period}"
Rails.logger.info " Next fulfillment at: #{fulfillment.next_fulfillment_at}"

# Warn if current_period_end is nil for an active subscription - this is an edge case
# that could indicate incomplete data from the payment processor
if current_period_end.nil? && status == "active"
Expand All @@ -354,9 +364,15 @@ def handle_plan_change
current_plan_id = fulfillment.metadata["plan"]
new_plan_id = processor_plan

Rails.logger.info " Looking up current plan: #{current_plan_id}"
Rails.logger.info " Looking up new plan: #{new_plan_id}"

current_plan = UsageCredits.configuration.find_subscription_plan_by_processor_id(current_plan_id)
new_plan = UsageCredits.configuration.find_subscription_plan_by_processor_id(new_plan_id)

Rails.logger.info " Current plan found: #{current_plan&.name} (#{current_plan&.credits_per_period} credits)"
Rails.logger.info " New plan found: #{new_plan&.name} (#{new_plan&.credits_per_period} credits)"

# Handle downgrade to a non-credit plan: schedule fulfillment stop for end of period
if new_plan.nil? && current_plan.present?
handle_downgrade_to_non_credit_plan(fulfillment)
Expand All @@ -370,6 +386,7 @@ def handle_plan_change
# This must come first! Returning to current plan = no credits, just clear pending
# This matches Stripe's billing: no new charge means no new credits
if current_plan_id == new_plan_id
Rails.logger.info " Action: Returning to current plan (clearing pending change)"
clear_pending_plan_change(fulfillment)
return
end
Expand All @@ -378,29 +395,44 @@ def handle_plan_change
current_credits = current_plan&.credits_per_period || 0
new_credits = new_plan.credits_per_period

Rails.logger.info " Comparing credits: #{current_credits} → #{new_credits}"

if new_credits > current_credits
# UPGRADE: Grant new plan credits immediately
Rails.logger.info " Action: UPGRADE detected - awarding #{new_credits} credits immediately"
handle_plan_upgrade(new_plan, fulfillment)
elsif new_credits < current_credits
# DOWNGRADE: Schedule for end of period (overwrites any previous pending)
Rails.logger.info " Action: DOWNGRADE detected - scheduling for end of period"
handle_plan_downgrade(new_plan, fulfillment)
else
# Same credits amount, different plan - update metadata immediately
Rails.logger.info " Action: Same credits, different plan - updating metadata only"
update_fulfillment_plan_metadata(fulfillment, new_plan_id)
end
rescue => e
Rails.logger.error "Failed to handle plan change for subscription #{id}: #{e.message}"
Rails.logger.error e.backtrace.join("\n")
raise ActiveRecord::Rollback
end

Rails.logger.info " Plan change completed successfully"
Rails.logger.info "=" * 80
end

def handle_plan_upgrade(new_plan, fulfillment)
wallet = customer.owner.credit_wallet

Rails.logger.info " [UPGRADE] Starting upgrade process"
Rails.logger.info " [UPGRADE] Wallet ID: #{wallet.id}, Current balance: #{wallet.balance}"
Rails.logger.info " [UPGRADE] Credits to award: #{new_plan.credits_per_period}"
Rails.logger.info " [UPGRADE] New plan period: #{new_plan.fulfillment_period_display}"

# Calculate expiration using shared helper (uses current_period_end for upgrades)
credits_expire_at = calculate_credit_expiration(new_plan, current_period_end)

Rails.logger.info " [UPGRADE] Credits expire at: #{credits_expire_at || 'never (rollover enabled)'}"

# Grant full new plan credits immediately
# Use string keys consistently to avoid duplicates after JSON serialization
wallet.add_credits(
Expand All @@ -415,16 +447,35 @@ def handle_plan_upgrade(new_plan, fulfillment)
}
)

# Update fulfillment metadata AND clear any pending downgrade
# If user had scheduled a downgrade but then upgrades, the upgrade takes precedence
Rails.logger.info " [UPGRADE] Credits awarded successfully"
Rails.logger.info " [UPGRADE] New balance: #{wallet.reload.balance}"

# Calculate next fulfillment time based on the NEW plan's period
# This ensures the fulfillment schedule matches the new plan's cadence
next_fulfillment_at = Time.current + new_plan.parsed_fulfillment_period

Rails.logger.info " [UPGRADE] Updating fulfillment record"
Rails.logger.info " [UPGRADE] Old fulfillment_period: #{fulfillment.fulfillment_period}"
Rails.logger.info " [UPGRADE] New fulfillment_period: #{new_plan.fulfillment_period_display}"
Rails.logger.info " [UPGRADE] Old next_fulfillment_at: #{fulfillment.next_fulfillment_at}"
Rails.logger.info " [UPGRADE] New next_fulfillment_at: #{next_fulfillment_at}"

# Update fulfillment with ALL new plan properties
# This includes the period display string and the next fulfillment time
# to ensure future fulfillments happen on the correct schedule
# Use string keys consistently to avoid duplicates after JSON serialization
fulfillment.update!(
fulfillment_period: new_plan.fulfillment_period_display,
next_fulfillment_at: next_fulfillment_at,
metadata: fulfillment.metadata
.except("pending_plan_change", "plan_change_at")
.merge("plan" => processor_plan)
)

Rails.logger.info " [UPGRADE] Fulfillment updated successfully"
Rails.logger.info "Subscription #{id} upgraded to #{processor_plan}, granted #{new_plan.credits_per_period} credits"
Rails.logger.info " Fulfillment period updated to: #{new_plan.fulfillment_period_display}"
Rails.logger.info " Next fulfillment scheduled for: #{next_fulfillment_at}"
end

def handle_plan_downgrade(new_plan, fulfillment)
Expand Down
144 changes: 144 additions & 0 deletions test/models/concerns/pay_subscription_extension_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,150 @@ class PaySubscriptionExtensionTest < ActiveSupport::TestCase
assert_equal 2200, wallet.reload.credits
end

# ========================================
# REGRESSION: FULFILLMENT PERIOD UPDATE ON UPGRADE
# ========================================

test "REGRESSION: upgrade updates fulfillment_period and next_fulfillment_at" do
# This test ensures that when upgrading between plans with different fulfillment periods,
# the fulfillment record is properly updated with the new plan's schedule.
# Bug: Previously, only metadata was updated, leaving fulfillment_period and next_fulfillment_at
# on the old plan's schedule, causing delayed or incorrect future fulfillments.

# Create test plans with different fulfillment periods
# Note: We need to temporarily override min_fulfillment_period for this test
UsageCredits.configure do |config|
config.min_fulfillment_period = 1.second # Allow short periods for testing

config.subscription_plan :test_fast do
processor_plan(:fake_processor, "fast_plan")
gives 100.credits.every(1.day)
unused_credits :rollover
end

config.subscription_plan :test_slower do
processor_plan(:fake_processor, "slower_plan")
gives 500.credits.every(7.days)
unused_credits :rollover
end
end

user = User.create!(email: "period_upgrade@example.com", name: "Period Upgrade User")
wallet = user.credit_wallet

customer = Pay::Customer.create!(
owner: user,
processor: :fake_processor,
processor_id: "cus_period_upgrade"
)

# Start with fast plan (15 seconds)
subscription = Pay::Subscription.create!(
customer: customer,
name: "default",
processor_id: "sub_period_upgrade",
processor_plan: "fast_plan",
status: "active",
quantity: 1,
current_period_end: 30.days.from_now
)

fulfillment = UsageCredits::Fulfillment.find_by(source: subscription)
assert_equal "1 day", fulfillment.fulfillment_period
initial_next_fulfillment = fulfillment.next_fulfillment_at

# Verify initial next_fulfillment is ~1 day from now
assert_in_delta Time.current + 1.day, initial_next_fulfillment, 5.seconds

# Upgrade to slower plan (7 days)
travel_to 1.hour.from_now do
subscription.update!(processor_plan: "slower_plan")
end

# Verify fulfillment record was updated with new plan's period
fulfillment.reload
assert_equal "7 days", fulfillment.fulfillment_period,
"Fulfillment period should be updated to new plan's period"

# Verify next_fulfillment_at was recalculated based on new plan
# It should be ~7 days from the upgrade time (1 hour from initial time)
expected_next_fulfillment = Time.current + 1.hour + 7.days
assert_in_delta expected_next_fulfillment, fulfillment.next_fulfillment_at, 5.seconds,
"Next fulfillment should be rescheduled based on new plan's period"

# Verify it's significantly different from the old schedule
assert fulfillment.next_fulfillment_at > initial_next_fulfillment + 1.day,
"Next fulfillment should be much later than the old 1-day schedule"

# Verify metadata was also updated
assert_equal "slower_plan", fulfillment.metadata["plan"]

# Verify credits were awarded
assert_equal 600, wallet.reload.credits # 100 initial + 500 upgrade
end

test "REGRESSION: upgrade between plans with same period only updates metadata" do
# Ensure that upgrading between plans with the same fulfillment period
# doesn't unnecessarily reset the next_fulfillment_at schedule

UsageCredits.configure do |config|
config.subscription_plan :test_monthly_basic do
processor_plan(:fake_processor, "monthly_basic")
gives 100.credits.every(:month)
unused_credits :rollover
end

config.subscription_plan :test_monthly_premium do
processor_plan(:fake_processor, "monthly_premium")
gives 500.credits.every(:month)
unused_credits :rollover
end
end

user = User.create!(email: "same_period@example.com", name: "Same Period User")
wallet = user.credit_wallet

customer = Pay::Customer.create!(
owner: user,
processor: :fake_processor,
processor_id: "cus_same_period"
)

subscription = Pay::Subscription.create!(
customer: customer,
name: "default",
processor_id: "sub_same_period",
processor_plan: "monthly_basic",
status: "active",
quantity: 1,
current_period_end: 30.days.from_now
)

fulfillment = UsageCredits::Fulfillment.find_by(source: subscription)
initial_next_fulfillment = fulfillment.next_fulfillment_at

# Upgrade to premium (same period, different credits)
travel_to 5.seconds.from_now do
subscription.update!(processor_plan: "monthly_premium")
end

fulfillment.reload

# Period should be updated (even though it's the same value)
# Note: :month normalizes to "1 month", not "30 days"
assert_equal "1 month", fulfillment.fulfillment_period

# Next fulfillment should be recalculated from upgrade time
expected_next_fulfillment = Time.current + 5.seconds + 1.month
assert_in_delta expected_next_fulfillment, fulfillment.next_fulfillment_at, 5.seconds

# Metadata should be updated
assert_equal "monthly_premium", fulfillment.metadata["plan"]

# Credits should be awarded
assert_equal 600, wallet.reload.credits # 100 initial + 500 upgrade
end

# ========================================
# EDGE CASES: CREDIT EXPIRATION ON UPGRADE
# ========================================
Expand Down