diff --git a/app/services/subscriptions/create_service.rb b/app/services/subscriptions/create_service.rb index 6803fd899f7..67cc7eb9b0b 100644 --- a/app/services/subscriptions/create_service.rb +++ b/app/services/subscriptions/create_service.rb @@ -146,6 +146,9 @@ 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) @@ -153,11 +156,15 @@ def upgrade_subscription 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 @@ -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 diff --git a/app/services/subscriptions/terminate_service.rb b/app/services/subscriptions/terminate_service.rb index d9f52ce2474..bc6afce4219 100644 --- a/app/services/subscriptions/terminate_service.rb +++ b/app/services/subscriptions/terminate_service.rb @@ -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 @@ -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 diff --git a/spec/scenarios/invoices/invoices_spec.rb b/spec/scenarios/invoices/invoices_spec.rb index 17a8cf03760..0aff82a95ad 100644 --- a/spec/scenarios/invoices/invoices_spec.rb +++ b/spec/scenarios/invoices/invoices_spec.rb @@ -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 @@ -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 diff --git a/spec/scenarios/subscriptions/downgrade_spec.rb b/spec/scenarios/subscriptions/downgrade_spec.rb index 2a007c8e290..491053fcfd8 100644 --- a/spec/scenarios/subscriptions/downgrade_spec.rb +++ b/spec/scenarios/subscriptions/downgrade_spec.rb @@ -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 @@ -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 diff --git a/spec/scenarios/subscriptions/upgrade_spec.rb b/spec/scenarios/subscriptions/upgrade_spec.rb index a1f2548ed44..b58f5507f66 100644 --- a/spec/scenarios/subscriptions/upgrade_spec.rb +++ b/spec/scenarios/subscriptions/upgrade_spec.rb @@ -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 @@ -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, @@ -97,13 +97,14 @@ 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) @@ -111,7 +112,13 @@ 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 diff --git a/spec/services/subscriptions/terminate_service_spec.rb b/spec/services/subscriptions/terminate_service_spec.rb index d4229057ed8..6608ee8de5f 100644 --- a/spec/services/subscriptions/terminate_service_spec.rb +++ b/spec/services/subscriptions/terminate_service_spec.rb @@ -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