From 2a8db218239433a8798b96d9513ef60a2fc29b37 Mon Sep 17 00:00:00 2001 From: Toon Willems Date: Mon, 16 Dec 2024 15:29:09 +0100 Subject: [PATCH] take created_at into account when running billing in the past --- app/services/subscriptions/billing_service.rb | 6 +- .../subscriptions/billing_service_spec.rb | 325 ++++++++---------- 2 files changed, 155 insertions(+), 176 deletions(-) diff --git a/app/services/subscriptions/billing_service.rb b/app/services/subscriptions/billing_service.rb index 2948ae0827b..7d0ecff9e56 100644 --- a/app/services/subscriptions/billing_service.rb +++ b/app/services/subscriptions/billing_service.rb @@ -78,6 +78,8 @@ def billable_subscriptions -- Do not bill subscriptions that have started _after_ :today (excludes subscriptions starting today! and also importantly invoices that might have started after this service is run) AND DATE(subscriptions.started_at#{at_time_zone}) < DATE(:today#{at_time_zone}) + -- Do not bill subscriptions that were not created yet + and DATE(subscriptions.created_at) <= Date(:today) AND ( subscriptions.ending_at IS NULL OR DATE(subscriptions.ending_at#{at_time_zone}) != DATE(:today#{at_time_zone}) @@ -95,9 +97,7 @@ def base_subscription_scope(billing_time: nil, interval: nil, conditions: nil) INNER JOIN plans ON plans.id = subscriptions.plan_id INNER JOIN customers ON customers.id = subscriptions.customer_id INNER JOIN organizations ON organizations.id = customers.organization_id - WHERE subscriptions.status IN (#{Subscription.statuses[:active]}, #{Subscription.statuses[:terminated]}) - -- Because this job might be run for the past, we need to "revert" the past and if the subscription was not yet terminated, it should be billed. - AND (subscriptions.terminated_at is NULL OR subscriptions.terminated_at >= :today#{at_time_zone}) + WHERE subscriptions.status = #{Subscription.statuses[:active]} AND subscriptions.billing_time = #{Subscription.billing_times[billing_time]} AND plans.interval = #{Plan.intervals[interval]} AND #{conditions.join(" AND ")} diff --git a/spec/services/subscriptions/billing_service_spec.rb b/spec/services/subscriptions/billing_service_spec.rb index 229ffb536d4..ab832c3f90b 100644 --- a/spec/services/subscriptions/billing_service_spec.rb +++ b/spec/services/subscriptions/billing_service_spec.rb @@ -3,11 +3,12 @@ require 'rails_helper' RSpec.describe Subscriptions::BillingService, type: :service do - subject(:billing_service) { described_class.new } + subject(:billing_service) { described_class.new(billing_at:) } describe '.call' do let(:plan) { create(:plan, interval:, bill_charges_monthly:) } let(:bill_charges_monthly) { false } + let(:created_at) { DateTime.parse('20 Feb 2020') } let(:subscription_at) { DateTime.parse('20 Feb 2021') } let(:customer) { create(:customer) } @@ -17,11 +18,13 @@ plan:, subscription_at:, started_at: current_date - 10.days, - billing_time: + billing_time:, + created_at: ) end let(:current_date) { DateTime.parse('20 Jun 2022') } + let(:billing_at) { current_date } before { subscription } @@ -35,7 +38,8 @@ customer:, plan:, subscription_at:, - started_at: current_date - 10.days + started_at: current_date - 10.days, + created_at: ) end @@ -45,7 +49,8 @@ customer:, plan:, subscription_at:, - started_at: current_date - 10.days + started_at: current_date - 10.days, + created_at: ) end @@ -54,7 +59,8 @@ :subscription, plan:, subscription_at:, - started_at: current_date - 10.days + started_at: current_date - 10.days, + created_at: ) end @@ -65,33 +71,31 @@ end it 'enqueues a job on billing day' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with( - contain_exactly(subscription1, subscription2), - current_date.to_i, - invoicing_reason: :subscription_periodic - ) + expect(BillSubscriptionJob).to have_been_enqueued + .with( + contain_exactly(subscription1, subscription2), + current_date.to_i, + invoicing_reason: :subscription_periodic + ) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with( - contain_exactly(subscription1, subscription2), - current_date - ) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with( + contain_exactly(subscription1, subscription2), + current_date + ) - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription3], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription3], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription3], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription3], current_date) end - it 'does not enqueue a job on other day' do - current_date = DateTime.parse('21 Jun 2022') + context "when billing_at is another day" do + let(:billing_at) { DateTime.parse('21 Jun 2022') } - travel_to(current_date) do + it 'does not enqueue a job on other day' do expect { billing_service.call }.not_to have_enqueued_job end end @@ -103,20 +107,18 @@ let(:current_date) { DateTime.parse('01 Feb 2022') } it 'enqueues a job on billing day' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued.with([subscription], current_date) - end + expect(BillNonInvoiceableFeesJob).to have_been_enqueued.with([subscription], current_date) end - it 'does not enqueue a job on other day' do - current_date = DateTime.parse('02 Feb 2022') + context 'when billing_at is different' do + let(:billing_at) { DateTime.parse('02 Feb 2022') } - travel_to(current_date) do + it 'does not enqueue a job on other day' do expect { billing_service.call }.not_to have_enqueued_job end end @@ -128,21 +130,19 @@ :subscription, plan:, subscription_at:, - started_at: Time.zone.now, + started_at: subscription_at, billing_time:, ending_at: billing_date ) end it 'does not enqueue a job on billing day' do - travel_to(billing_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).not_to have_been_enqueued - .with([subscription], billing_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).not_to have_been_enqueued - .with([subscription], billing_date) - end + expect(BillSubscriptionJob).not_to have_been_enqueued + .with([subscription], billing_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).not_to have_been_enqueued + .with([subscription], billing_date) end end end @@ -153,27 +153,23 @@ let(:current_date) { DateTime.parse('01 Apr 2022') } it 'enqueues a job on billing day' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end it 'does not enqueue a job on other day' do current_date = DateTime.parse('01 May 2022') - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).not_to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).not_to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).not_to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).not_to have_been_enqueued + .with([subscription], current_date) end end @@ -184,38 +180,33 @@ let(:current_date) { DateTime.parse('01 Jan 2022') } it 'enqueues a job on billing day' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end - it 'does not enqueue a job on other day' do - current_date = DateTime.parse('02 Jan 2022') + context "when billing at is not the billing day" do + let(:billing_at) { DateTime.parse('02 Jan 2022') } - travel_to(current_date) do + it 'does not enqueue a job on other day' do expect { billing_service.call }.not_to have_enqueued_job end end context 'when charges are billed monthly' do let(:bill_charges_monthly) { true } + let(:billing_at) { DateTime.parse('01 Feb 2022') } it 'enqueues a job on billing day' do - current_date = DateTime.parse('01 Feb 2022') - - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], billing_at.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], billing_at) end end end @@ -229,18 +220,18 @@ end it 'enqueues a job on billing day' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end - it 'does not enqueue a job on other day' do - travel_to(current_date + 1.day) do + context "when billing_at is a different day" do + let(:billing_at) { current_date + 1.day } + + it 'does not enqueue a job on other day' do expect { billing_service.call }.not_to have_enqueued_job end end @@ -252,18 +243,18 @@ let(:current_date) { subscription_at.next_month } it 'enqueues a job on billing day' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end - it 'does not enqueue a job on other day' do - travel_to(current_date + 1.day) do + context "when billing_at is a different day" do + let(:billing_at) { current_date + 1.day } + + it 'does not enqueue a job on other day' do expect { billing_service.call }.not_to have_enqueued_job end end @@ -273,14 +264,12 @@ let(:current_date) { DateTime.parse('28 Feb 2022') } it 'enqueues a job if the month count less than 31 days' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end end end @@ -291,18 +280,18 @@ let(:current_date) { subscription_at + 3.months } it 'enqueues a job on billing day' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end - it 'does not enqueue a job on other day' do - travel_to(current_date + 1.day) do + context "when billing_at is a different day" do + let(:billing_at) { current_date + 1.day } + + it 'does not enqueue a job on other day' do expect { billing_service.call }.not_to have_enqueued_job end end @@ -312,14 +301,12 @@ let(:current_date) { DateTime.parse('15 Sep 2022') } it 'enqueues a job' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end end @@ -328,14 +315,12 @@ let(:current_date) { DateTime.parse('30 Jun 2022') } it 'enqueues a job if the month count less than 31 days' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end end end @@ -347,18 +332,18 @@ let(:current_date) { subscription_at.next_year } it 'enqueues a job on billing day' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end - it 'does not enqueue a job on other day' do - travel_to(current_date + 1.day) do + context "when billing_at is a different day" do + let(:billing_at) { current_date + 1.day } + + it 'does not enqueue a job on other day' do expect { billing_service.call }.not_to have_enqueued_job end end @@ -368,14 +353,12 @@ let(:current_date) { DateTime.parse('28 Feb 2022') } it 'enqueues a job on 28th of february when year is not a leap year' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end end @@ -383,14 +366,16 @@ let(:bill_charges_monthly) { true } let(:current_date) { subscription_at.next_month } - it 'enqueues a job on billing day' do - travel_to(current_date.next_month) do + context "when billing_at is the next month" do + let(:billing_at) { current_date.next_month } + + it 'enqueues a job on billing day' do billing_service.call expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.next_month.to_i, invoicing_reason: :subscription_periodic) + .with([subscription], billing_at.to_i, invoicing_reason: :subscription_periodic) expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date.next_month) + .with([subscription], billing_at) end end @@ -399,14 +384,12 @@ let(:current_date) { DateTime.parse('28 Feb 2022') } it 'enqueues a job if the month count less than 31 days' do - travel_to(current_date) do - billing_service.call - - expect(BillSubscriptionJob).to have_been_enqueued - .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) - expect(BillNonInvoiceableFeesJob).to have_been_enqueued - .with([subscription], current_date) - end + billing_service.call + + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], current_date.to_i, invoicing_reason: :subscription_periodic) + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + .with([subscription], current_date) end end end @@ -422,7 +405,8 @@ subscription_at:, started_at: current_date - 10.days, previous_subscription:, - status: :pending + status: :pending, + created_at: ) end @@ -432,19 +416,18 @@ customer:, subscription_at:, started_at: current_date - 10.days, - billing_time: :anniversary + billing_time: :anniversary, + created_at: ) end before { subscription } it 'enqueues a job on billing day' do - travel_to(current_date) do - billing_service.call + billing_service.call - expect(Subscriptions::TerminateJob).to have_been_enqueued - .with(previous_subscription, current_date.to_i) - end + expect(Subscriptions::TerminateJob).to have_been_enqueued + .with(previous_subscription, current_date.to_i) end end @@ -468,19 +451,18 @@ let(:customer) { create(:customer, organization: plan.organization, timezone:) } let(:timezone) { nil } + let(:billing_at) { subscription_at } + it 'does not enqueue a job' do - travel_to(subscription_at) do - expect { billing_service.call }.not_to have_enqueued_job - end + expect { billing_service.call }.not_to have_enqueued_job end context 'with customer timezone' do let(:timezone) { 'Pacific/Noumea' } + let(:billing_at) { subscription_at + 10.hours } it 'does not enqueue a job' do - travel_to(subscription_at + 10.hours) do - expect { billing_service.call }.not_to have_enqueued_job - end + expect { billing_service.call }.not_to have_enqueued_job end end end @@ -488,14 +470,14 @@ context 'when subscription was already automatically billed today' do let(:interval) { :monthly } let(:billing_time) { :anniversary } - let(:subscription_at) { DateTime.parse('20 Feb 2021T12:00:00') } + let(:billing_at) { DateTime.parse('20 Jul 2021T12:00:00') } let(:invoice_subscription) do create( :invoice_subscription, subscription:, invoicing_reason: :subscription_periodic, - timestamp: subscription_at - 1.hour, + timestamp: billing_at - 1.hour, recurring: true ) end @@ -503,18 +485,15 @@ before { invoice_subscription } it 'does not enqueue a job' do - travel_to(subscription_at) do - expect { billing_service.call }.not_to have_enqueued_job - end + expect { billing_service.call }.not_to have_enqueued_job end context 'with customer timezone' do let(:timezone) { 'Pacific/Noumea' } + let(:billing_at) { DateTime.parse('20 Jul 2021T22:00:00') } it 'does not enqueue a job' do - travel_to(subscription_at + 10.hours) do - expect { billing_service.call }.not_to have_enqueued_job - end + expect { billing_service.call }.not_to have_enqueued_job end end end