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

fix(subscription upgrade): Upgrade subscriptions previously considered downgrade on amount increase #2823

Merged
merged 5 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
23 changes: 16 additions & 7 deletions app/services/plans/update_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ def call
plan.description = params[:description] if params.key?(:description)
plan.amount_cents = params[:amount_cents] if params.key?(:amount_cents)

# NOTE: Only name and description are editable if plan
# is attached to subscriptions
# NOTE: If plan is attached to subscriptions the editable attributes are:
# name, invoice_display_name, description, amount_cents
unless plan.attached_to_subscriptions?
plan.code = params[:code] if params.key?(:code)
plan.interval = params[:interval].to_sym if params.key?(:interval)
Expand Down Expand Up @@ -268,12 +268,21 @@ def process_downgraded_subscriptions
end
end

# NOTE: We should remove pending subscriptions
# if plan has been downgraded but amount cents of pending plan became higher than original plan.
# This pending subscription is not relevant in this case and downgrade should be ignored
# NOTE: If new plan yearly amount is higher than its value before the update
# and there are pending subscriptions for the plan,
# this is a plan upgrade, old subscription must be terminated and billed
# new subscription with updated plan must be activated inmediately.
def process_pending_subscriptions
Subscription.where(plan:, status: :pending).find_each do |sub|
sub.mark_as_canceled! if plan.amount_cents > sub.previous_subscription.plan.amount_cents
Subscription.where(plan:, status: :pending).find_each do |subscription|
next unless subscription.previous_subscription

if plan.yearly_amount_cents >= subscription.previous_subscription.plan.yearly_amount_cents
Subscriptions::PlanUpgradeService.call(
current_subscription: subscription.previous_subscription,
plan: plan,
params: {name: subscription.name}
).raise_if_error!
end
end
end
end
Expand Down
59 changes: 3 additions & 56 deletions app/services/subscriptions/create_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,48 +136,9 @@ def create_subscription
end

def upgrade_subscription
if current_subscription.starting_in_the_future?
update_pending_subscription

return current_subscription
end

new_subscription = Subscription.new(
customer:,
plan: params.key?(:plan_overrides) ? override_plan(plan) : plan,
name:,
external_id: current_subscription.external_id,
previous_subscription_id: current_subscription.id,
subscription_at: current_subscription.subscription_at,
billing_time: current_subscription.billing_time,
ending_at: params.key?(:ending_at) ? params[:ending_at] : current_subscription.ending_at
)

cancel_pending_subscription if pending_subscription?

# Collection that groups all billable subscriptions for an invoice
billable_subscriptions = billable_subscriptions(new_subscription)

# NOTE: When upgrading, the new subscription becomes active immediately
# The previous one must be terminated
Subscriptions::TerminateService.call(subscription: current_subscription, upgrade: true)

new_subscription.mark_as_active!
after_commit { SendWebhookJob.perform_later('subscription.started', new_subscription) }

# NOTE: If plan is in advance we should create only one invoice for termination fees and for new plan fees
if billable_subscriptions.any?
# NOTE: Since job is launched from inside a db transaction
# we must wait for it to be committed before processing the job
# We do not set offset anymore but instead retry jobs
after_commit do
billing_at = Time.current + 1.second
BillSubscriptionJob.perform_later(billable_subscriptions, billing_at.to_i, invoicing_reason: :upgrading)
BillNonInvoiceableFeesJob.perform_later(billable_subscriptions, billing_at)
end
end

new_subscription
PlanUpgradeService.call(current_subscription:, plan:, params:).tap do |result|
result.raise_if_error!
end.subscription
end

def downgrade_subscription
Expand Down Expand Up @@ -266,19 +227,5 @@ def editable_subscriptions
def override_plan(plan)
Plans::OverrideService.call(plan:, params: params[:plan_overrides].to_h.with_indifferent_access).plan
end

def billable_subscriptions(new_subscription)
billable_subscriptions = if current_subscription.starting_in_the_future?
[]
elsif current_subscription.pending?
[]
elsif !current_subscription.terminated?
[current_subscription]
end.to_a

billable_subscriptions << new_subscription if plan.pay_in_advance? && !new_subscription.in_trial_period?

billable_subscriptions
end
end
end
113 changes: 113 additions & 0 deletions app/services/subscriptions/plan_upgrade_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
module Subscriptions
class PlanUpgradeService < BaseService
def initialize(current_subscription:, plan:, params:)
@current_subscription = current_subscription
@plan = plan

@params = params
@name = params[:name].to_s.strip
super
end

def call
if current_subscription.starting_in_the_future?
update_pending_subscription

result.subscription = current_subscription
return result
end

new_subscription = new_subcription_with_overrides

ActiveRecord::Base.transaction do
cancel_pending_subscription if pending_subscription?

# Group subscriptions for billing
billable_subscriptions = billable_subscriptions(new_subscription)

# Terminate current subscription as part of the upgrade process
Subscriptions::TerminateService.call(
subscription: current_subscription,
upgrade: true
)

new_subscription.mark_as_active!
after_commit do
SendWebhookJob.perform_later("subscription.started", new_subscription)
end

bill_subscriptions(billable_subscriptions) if billable_subscriptions.any?
end

result.subscription = new_subscription
result
rescue ActiveRecord::RecordInvalid => e
result.record_validation_failure!(record: e.record)
rescue BaseService::FailedResult => e
result.fail_with_error!(e)
end

private

attr_reader :current_subscription, :plan, :params, :name

def new_subcription_with_overrides
Subscription.new(
customer: current_subscription.customer,
plan: params.key?(:plan_overrides) ? override_plan : plan,
name:,
external_id: current_subscription.external_id,
previous_subscription_id: current_subscription.id,
subscription_at: current_subscription.subscription_at,
billing_time: current_subscription.billing_time,
ending_at: params.key?(:ending_at) ? params[:ending_at] : current_subscription.ending_at
)
end

def update_pending_subscription
current_subscription.plan = plan
current_subscription.name = name if name.present?
current_subscription.save!

if current_subscription.should_sync_crm_subscription?
Integrations::Aggregator::Subscriptions::Crm::UpdateJob.perform_later(subscription: current_subscription)
end
end

def override_plan
Plans::OverrideService.call(plan:, params: params[:plan_overrides].to_h.with_indifferent_access).plan
end

def cancel_pending_subscription
current_subscription.next_subscription.mark_as_canceled!
end

def pending_subscription?
return false unless current_subscription.next_subscription

current_subscription.next_subscription.pending?
end

def billable_subscriptions(new_subscription)
billable_subscriptions = if current_subscription.starting_in_the_future?
[]
elsif current_subscription.pending?
[]
elsif !current_subscription.terminated?
[current_subscription]
end.to_a

billable_subscriptions << new_subscription if plan.pay_in_advance? && !new_subscription.in_trial_period?

billable_subscriptions
end

def bill_subscriptions(billable_subscriptions)
after_commit do
billing_at = Time.current + 1.second
BillSubscriptionJob.perform_later(billable_subscriptions, billing_at.to_i, invoicing_reason: :upgrading)
BillNonInvoiceableFeesJob.perform_later(billable_subscriptions, billing_at)
end
end
end
end
51 changes: 44 additions & 7 deletions spec/services/plans/update_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -402,17 +402,54 @@
amount_currency: 'EUR'
}
end
let(:plan_upgrade_result) { BaseService::Result.new }

before { pending_subscription }
before do
allow(Subscriptions::PlanUpgradeService)
.to receive(:call)
.and_return(plan_upgrade_result)

it 'correctly cancels pending subscriptions' do
pending_subscription
end

it "upgrades subscription plan" do
plans_service.call

expect(Subscriptions::PlanUpgradeService).to have_received(:call)
end

it 'updates the plan', :aggregate_failures do
result = plans_service.call

updated_plan = result.plan
aggregate_failures do
expect(updated_plan.name).to eq('Updated plan name')
expect(updated_plan.amount_cents).to eq(200)
expect(Subscription.find_by(id: pending_subscription.id).status).to eq('canceled')
expect(result.plan.name).to eq('Updated plan name')
expect(result.plan.amount_cents).to eq(200)
end

context "when pending subscription does not have a previous one" do
let(:pending_subscription) do
create(:subscription, plan:, status: :pending, previous_subscription_id: nil)
end

it "does not upgrade it" do
plans_service.call

expect(Subscriptions::PlanUpgradeService).not_to have_received(:call)
end
end

context "when subscription upgrade fails" do
let(:plan_upgrade_result) do
BaseService::Result.new.validation_failure!(
errors: {billing_time: ['value_is_invalid']}
)
end

it "returns an error", :aggregate_failures do
result = plans_service.call

expect(result).not_to be_success
expect(result.error).to be_a(BaseService::ValidationFailure)
expect(result.error.messages).to eq({ billing_time: ["value_is_invalid"] })
end
end
end
Expand Down
22 changes: 22 additions & 0 deletions spec/services/subscriptions/create_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,28 @@
end
end

context "when subscription upgrade fails" do
let(:result_failure) do
BaseService::Result.new.validation_failure!(
errors: {billing_time: ['value_is_invalid']}
)
end

before do
allow(Subscriptions::PlanUpgradeService)
.to receive(:call)
.and_return(result_failure)
end

it "returns an error", :aggregate_failures do
result = create_service.call

expect(result).not_to be_success
expect(result.error).to be_a(BaseService::ValidationFailure)
expect(result.error.messages).to eq({ billing_time: ["value_is_invalid"] })
end
end

context 'when current subscription is pending' do
before { subscription.pending! }

Expand Down
Loading
Loading