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 (invoices): group invoices upon downgrade and upgrade #1712

Merged
merged 4 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
25 changes: 23 additions & 2 deletions app/services/subscriptions/create_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,25 @@ def upgrade_subscription

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 immediatly
# The previous one must be terminated
Subscriptions::TerminateService.call(subscription: current_subscription, upgrade: true)

new_subscription.mark_as_active!
perform_later(job_class: SendWebhookJob, arguments: ['subscription.started', new_subscription])

if plan.pay_in_advance?
# 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 commited before processing the job
# We do not set offset anymore but instead retry jobs
perform_later(job_class: BillSubscriptionJob, arguments: [[new_subscription], Time.zone.now.to_i + 1.second])
perform_later(
job_class: BillSubscriptionJob,
arguments: [billable_subscriptions, Time.zone.now.to_i + 1.second],
)
end

new_subscription
Expand Down Expand Up @@ -245,5 +252,19 @@ 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

return billable_subscriptions unless plan.pay_in_advance?

billable_subscriptions << new_subscription
end
end
end
16 changes: 11 additions & 5 deletions app/services/subscriptions/terminate_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ def call
credit_note_result.raise_if_error!
end

bill_subscription
# NOTE: We should bill subscription and generate invoice for all cases except for the upgrade
# For upgrade we will create only one invoice for termination charges and for in advance charges
# It is handled in subscriptions/create_service.rb
bill_subscription unless upgrade
end

# NOTE: Pending next subscription should be canceled as well
Expand Down Expand Up @@ -61,15 +64,18 @@ def terminate_and_start_next(timestamp:)
# NOTE: Create an invoice for the terminated subscription
# if it has not been billed yet
# or only for the charges if subscription was billed in advance
BillSubscriptionJob.perform_later([subscription], timestamp)
# Also, add new pay in advance plan inside if applicable
billable_subscriptions = if next_subscription.plan.pay_in_advance?
[subscription, next_subscription]
else
[subscription]
end
BillSubscriptionJob.perform_later(billable_subscriptions, timestamp)

SendWebhookJob.perform_later('subscription.terminated', subscription)
SendWebhookJob.perform_later('subscription.started', next_subscription)

result.subscription = next_subscription
return result unless next_subscription.plan.pay_in_advance?

BillSubscriptionJob.perform_later([next_subscription], timestamp)

result
rescue ActiveRecord::RecordInvalid => e
Expand Down
10 changes: 4 additions & 6 deletions spec/scenarios/invoices/invoices_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -538,15 +538,13 @@
)
perform_all_enqueued_jobs
}.to change { subscription.reload.status }.from('active').to('terminated')
.and change { customer.invoices.count }.from(1).to(3)
.and change { customer.invoices.count }.from(1).to(2)

terminated_invoice = subscription.invoices.order(created_at: :desc).first
new_subscription_invoice = customer.subscriptions.active.first.invoices.order(created_at: :desc).first
invoice = customer.subscriptions.active.first.invoices.order(created_at: :desc).first
credit_note = customer.credit_notes.first

expect(credit_note.credit_amount_cents).to eq(1_800)
expect(terminated_invoice.total_amount_cents).to eq(0) # 11/29 x 500 = 190 - 190(CN)
expect(new_subscription_invoice.total_amount_cents).to eq(18_000 - (1_800 - 190))
expect(invoice.total_amount_cents).to eq(18_000 + 190 - 1_800) # 11/29 x 500 = 190
end

travel_to(DateTime.new(2024, 3, 1, 12, 12)) do
Expand Down Expand Up @@ -1088,7 +1086,7 @@
},
)

expect(customer.invoices.draft.count).to eq(2)
expect(customer.invoices.draft.count).to eq(1)

pay_in_arrear_subscription = customer.subscriptions.terminated.first
pay_in_arrear_invoice = pay_in_arrear_subscription.invoices.first
Expand Down
14 changes: 7 additions & 7 deletions spec/scenarios/subscriptions/downgrade_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,7 @@
expect { perform_all_enqueued_jobs }.to change { subscription.reload.invoices.count }
expect(subscription.reload).to be_terminated
expect(subscription.invoices.count).to eq(5)

# Termination invoice. For in advance subscription without charges it should be equal to zero
invoice = subscription.invoices.order(created_at: :asc).last
expect(invoice.fees_amount_cents).to eq(0)
expect(customer.invoices.count).to eq(5)

new_subscription = subscription.reload.next_subscription

Expand All @@ -132,9 +129,12 @@

new_sub_invoice = new_subscription.invoices.order(created_at: :asc).last
# There are 243 days from new sub started_at until old subscription subscription_at. Also, 2024 is a leap year
expect(new_sub_invoice.fees_amount_cents).to eq((yearly_plan.amount_cents.fdiv(366) * 243).round)
expect(new_sub_invoice.invoice_subscriptions.first.from_datetime.iso8601).to eq('2023-11-19T00:00:00Z')
expect(new_sub_invoice.invoice_subscriptions.first.to_datetime.iso8601).to eq('2024-07-18T23:59:59Z')
# Also for old pay in advance plan there are no charges so total amount is zero
expect(new_sub_invoice.fees_amount_cents).to eq(0 + (yearly_plan.amount_cents.fdiv(366) * 243).round)
expect(new_subscription.invoice_subscriptions.order(created_at: :desc).first.from_datetime.iso8601)
.to eq('2023-11-19T00:00:00Z')
expect(new_subscription.invoice_subscriptions.order(created_at: :desc).first.to_datetime.iso8601)
.to eq('2024-07-18T23:59:59Z')
end
end
end
21 changes: 14 additions & 7 deletions spec/scenarios/subscriptions/upgrade_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

let(:subscription_at) { DateTime.new(2023, 6, 29, 12, 12) }

it 'upgrades and bill subscriptions on a regulat basis' do
it 'upgrades and bill subscriptions on a regular basis' do
subscription = nil

# NOTE: Jun 29th: create the subscription
Expand Down Expand Up @@ -85,7 +85,7 @@
end

# NOTE: On september 28th: Upgrade to the yearly plan
travel_to(DateTime.new(2023, 9, 28, 0, 0)) do
travel_to(DateTime.new(2023, 9, 28, 5, 0)) do
create_subscription(
{
external_customer_id: customer.external_id,
Expand All @@ -97,21 +97,28 @@

expect(subscription.reload).to be_terminated
expect(subscription.invoices.count).to eq(4)
expect(customer.invoices.count).to eq(4)

invoice = subscription.invoices.order(created_at: :asc).last
expect(invoice.fees_amount_cents).to eq(0)
# expect(invoice.invoice_subscriptions.first.from_datetime.iso8601).to eq('2023-08-29T00:00:00Z')
# expect(invoice.invoice_subscriptions.first.to_datetime.iso8601).to eq('2023-09-28T23:59:59Z')
expect(invoice.invoice_subscriptions.first.charges_from_datetime.iso8601).to eq('2023-08-29T00:00:00Z')
expect(invoice.invoice_subscriptions.first.charges_to_datetime.iso8601).to eq('2023-09-28T00:00:00Z')
expect(subscription.invoice_subscriptions.order(created_at: :desc).first.charges_from_datetime.iso8601)
.to eq('2023-08-29T00:00:00Z')
expect(subscription.invoice_subscriptions.order(created_at: :desc).first.charges_to_datetime.iso8601)
.to eq('2023-09-28T05:00:00Z')

new_subscription = customer.subscriptions.order(created_at: :asc).last
expect(new_subscription.plan.code).to eq(yearly_plan.code)
expect(new_subscription).to be_active
expect(new_subscription.invoices.count).to eq(1)

invoice = new_subscription.invoices.last
expect(invoice.fees_amount_cents).not_to eq(0)

expect(customer.credit_notes.first.credit_amount_cents).to eq(32) # 1000 / 31

number_of_days = (DateTime.new(2024, 6, 29, 0, 0) - DateTime.new(2023, 9, 28, 0, 0)).to_i
single_day_price = 12_000.fdiv(366)

expect(invoice.fees_amount_cents).to eq((number_of_days * single_day_price).round)
end
end
end
9 changes: 5 additions & 4 deletions spec/services/subscriptions/terminate_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,11 @@

before { subscription.plan.update!(pay_in_advance: true) }

it 'enqueues a job to bill the existing subscription' do
expect do
terminate_service.terminate_and_start_next(timestamp:)
end.to have_enqueued_job(BillSubscriptionJob).twice
it 'enqueues one job' do
terminate_service.terminate_and_start_next(timestamp:)

expect(BillSubscriptionJob).to have_been_enqueued
.with([subscription, next_subscription], timestamp)
end
end
end
Expand Down
Loading