fix (invoices): group invoices upon downgrade and upgrade (#1712)

* group invoices upon downgrade and upgrade

* fix linter issues

* fix test

* add small refactor

app/services/subscriptions/create_service.rb

@@ -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
@@ -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

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

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

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

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,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

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