From 329cb475a652e1f9cce140014516499709a37076 Mon Sep 17 00:00:00 2001 From: Julien Bourdeau Date: Mon, 15 Apr 2024 10:23:51 +0200 Subject: [PATCH] feat(trial): Invoice subscription fee at the end of the trial period (#1847) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context When a subscription has a trial period, we want to invoice the customer at the end of the trial, not at the beginning. Following #1820 and #1821 ## Description * Disable billing when creating a new subscription if there is a trial * Disable billing when activating a pending subscription if there is a trial * Run the `Subscriptions::FreeTrialBillingService` introduced in #1821 every hour * Migrate data for existing subscription with trial already ended * Ensure we don't bill customers if an invoice was created when the subscription started (for sub created before this feature) ### Free trial reminder Keep in mind that if you're trial ends on day X, this day is *NOT FREE*. If the 10 days free trial start on the 10th at 14:00, it will end on the 20th at 14:00. You get the 10th to the 19th for free. Day 20 is your first pay day even if you're still in trial for a few hours. This was not changed in this PR or other related PRs. ## Note * Introduction of `invoice.skip_charge` * Refactor of charge fees for first invoice, see: https://github.com/getlago/lago-api/pull/1836 * Handle grace_period * Handle minimum_commitment --------- Co-authored-by: Romain Sempé --- .github/workflows/pronto.yml | 4 +- app/jobs/bill_subscription_job.rb | 4 +- .../free_trial_subscriptions_biller_job.rb | 11 + app/jobs/send_webhook_job.rb | 1 + app/models/subscription.rb | 1 - app/serializers/v1/subscription_serializer.rb | 1 + .../invoices/calculate_fees_service.rb | 15 +- .../invoices/create_generating_service.rb | 6 +- app/services/invoices/subscription_service.rb | 6 +- .../subscriptions/activate_service.rb | 8 +- app/services/subscriptions/create_service.rb | 25 +- .../free_trial_billing_service.rb | 59 +- app/services/utils/transactional_jobs.rb | 6 +- .../subscriptions/trial_ended_service.rb | 27 + clock.rb | 4 + ...12415_fill_subscriptions_trial_ended_at.rb | 41 ++ ...23257_add_skip_charges_bool_in_invoices.rb | 7 + db/schema.rb | 3 +- spec/jobs/bill_subscription_job_spec.rb | 4 +- spec/scenarios/invoices/invoices_spec.rb | 16 +- .../subscriptions/free_trial_billing_spec.rb | 526 ++++++++++++++++++ .../v1/subscription_serializer_spec.rb | 1 + .../invoices/calculate_fees_service_spec.rb | 31 +- .../subscriptions/activate_service_spec.rb | 58 +- .../subscriptions/create_service_spec.rb | 8 + .../free_trial_billing_service_spec.rb | 28 +- .../subscriptions/terminated_service_spec.rb | 2 +- .../subscriptions/trial_ended_service_spec.rb | 32 ++ .../matchers/have_empty_charge_fees.rb | 17 + spec/support/scenarios_helper.rb | 1 + 30 files changed, 862 insertions(+), 91 deletions(-) create mode 100644 app/jobs/clock/free_trial_subscriptions_biller_job.rb create mode 100644 app/services/webhooks/subscriptions/trial_ended_service.rb create mode 100644 db/migrate/20240329112415_fill_subscriptions_trial_ended_at.rb create mode 100644 db/migrate/20240404123257_add_skip_charges_bool_in_invoices.rb create mode 100644 spec/scenarios/subscriptions/free_trial_billing_spec.rb create mode 100644 spec/services/webhooks/subscriptions/trial_ended_service_spec.rb create mode 100644 spec/support/matchers/have_empty_charge_fees.rb diff --git a/.github/workflows/pronto.yml b/.github/workflows/pronto.yml index 3c3775b9abb..3b0aaac0b88 100644 --- a/.github/workflows/pronto.yml +++ b/.github/workflows/pronto.yml @@ -19,8 +19,10 @@ jobs: run: | bundle install - name: Run Rubocop + # We must pass the list of files because for now, rubocop on the entire project throws too many errors. + # We exclude the db/schema.rb file explicitly because passing a list of files will override the `AllCops.Exclude` config in .rubocop.yml run: | - FILES=$(git diff --diff-filter=d --name-only origin/${{ github.base_ref }}...HEAD -- '*.rb') + FILES=$(git diff --diff-filter=d --name-only origin/${{ github.base_ref }}...HEAD -- '*.rb' ':!db/*schema.rb') if [ -z "$FILES" ]; then echo "No Ruby files to lint" exit 0 diff --git a/app/jobs/bill_subscription_job.rb b/app/jobs/bill_subscription_job.rb index 05a496c69a9..4efef9be0f1 100644 --- a/app/jobs/bill_subscription_job.rb +++ b/app/jobs/bill_subscription_job.rb @@ -5,12 +5,13 @@ class BillSubscriptionJob < ApplicationJob retry_on Sequenced::SequenceError, ActiveJob::DeserializationError - def perform(subscriptions, timestamp, recurring: false, invoice: nil) + def perform(subscriptions, timestamp, recurring: false, invoice: nil, skip_charges: false) result = Invoices::SubscriptionService.call( subscriptions:, timestamp:, recurring:, invoice:, + skip_charges:, ) return if result.success? @@ -22,6 +23,7 @@ def perform(subscriptions, timestamp, recurring: false, invoice: nil) timestamp, recurring:, invoice: result.invoice, + skip_charges:, ) end end diff --git a/app/jobs/clock/free_trial_subscriptions_biller_job.rb b/app/jobs/clock/free_trial_subscriptions_biller_job.rb new file mode 100644 index 00000000000..f2a8806bc38 --- /dev/null +++ b/app/jobs/clock/free_trial_subscriptions_biller_job.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Clock + class FreeTrialSubscriptionsBillerJob < ApplicationJob + queue_as 'clock' + + def perform + Subscriptions::FreeTrialBillingService.call + end + end +end diff --git a/app/jobs/send_webhook_job.rb b/app/jobs/send_webhook_job.rb index 303aa54483a..e3bddd712e9 100644 --- a/app/jobs/send_webhook_job.rb +++ b/app/jobs/send_webhook_job.rb @@ -31,6 +31,7 @@ class SendWebhookJob < ApplicationJob 'subscription.terminated' => Webhooks::Subscriptions::TerminatedService, 'subscription.started' => Webhooks::Subscriptions::StartedService, 'subscription.termination_alert' => Webhooks::Subscriptions::TerminationAlertService, + 'subscription.trial_ended' => Webhooks::Subscriptions::TrialEndedService, }.freeze def perform(webhook_type, object, options = {}, webhook_id = nil) diff --git a/app/models/subscription.rb b/app/models/subscription.rb index 4254c29d6b1..cba81701121 100644 --- a/app/models/subscription.rb +++ b/app/models/subscription.rb @@ -91,7 +91,6 @@ def trial_end_datetime def in_trial_period? return false if trial_ended_at - return false unless active? return false if initial_started_at.future? trial_end_datetime.present? && trial_end_datetime.future? diff --git a/app/serializers/v1/subscription_serializer.rb b/app/serializers/v1/subscription_serializer.rb index 93bd00ae8f0..35e1c7feddc 100644 --- a/app/serializers/v1/subscription_serializer.rb +++ b/app/serializers/v1/subscription_serializer.rb @@ -14,6 +14,7 @@ def serialize billing_time: model.billing_time, subscription_at: model.subscription_at&.iso8601, started_at: model.started_at&.iso8601, + trial_ended_at: model.trial_ended_at&.iso8601, ending_at: model.ending_at&.iso8601, terminated_at: model.terminated_at&.iso8601, canceled_at: model.canceled_at&.iso8601, diff --git a/app/services/invoices/calculate_fees_service.rb b/app/services/invoices/calculate_fees_service.rb index a6c211b5ea2..80ea3035911 100644 --- a/app/services/invoices/calculate_fees_service.rb +++ b/app/services/invoices/calculate_fees_service.rb @@ -169,9 +169,10 @@ def should_create_subscription_fee?(subscription) return false if subscription.plan.pay_in_advance? && fee_exists return false unless should_create_yearly_subscription_fee?(subscription) + return false if subscription.in_trial_period? && !subscription.trial_end_datetime&.today? # NOTE: When a subscription is terminated we still need to charge the subscription - # fee if the plan is in pay in arrear, otherwise this fee will never + # fee if the plan is in pay in arrears, otherwise this fee will never # be created. subscription.active? || (subscription.terminated? && subscription.plan.pay_in_arrear?) || @@ -192,14 +193,12 @@ def should_create_yearly_subscription_fee?(subscription) end def should_create_charge_fees?(subscription) - # We should take a look at charges if subscription is created in the past and if it is not upgrade - if subscription.plan.pay_in_advance? && subscription.started_in_past? && subscription.previous_subscription.nil? - return true - end + return false if invoice.skip_charges - # NOTE: Charges should not be billed in advance when we are just upgrading to a new - # pay_in_advance subscription - return false if subscription.plan.pay_in_advance? && subscription.invoices.created_before(invoice).count.zero? + # We should take a look at charges if subscription is created in the past and if it is not upgrade + return true if subscription.plan.pay_in_advance? && + subscription.started_in_past? && + subscription.previous_subscription.nil? true end diff --git a/app/services/invoices/create_generating_service.rb b/app/services/invoices/create_generating_service.rb index 9da6dfc9486..a1ddcedbc6a 100644 --- a/app/services/invoices/create_generating_service.rb +++ b/app/services/invoices/create_generating_service.rb @@ -2,12 +2,13 @@ module Invoices class CreateGeneratingService < BaseService - def initialize(customer:, invoice_type:, datetime:, currency:, charge_in_advance: false) + def initialize(customer:, invoice_type:, datetime:, currency:, charge_in_advance: false, skip_charges: false) # rubocop:disable Metrics/ParameterLists @customer = customer @invoice_type = invoice_type @currency = currency @datetime = datetime @charge_in_advance = charge_in_advance + @skip_charges = skip_charges super end @@ -24,6 +25,7 @@ def call issuing_date:, payment_due_date:, net_payment_term: customer.applicable_net_payment_term, + skip_charges:, ) result.invoice = invoice @@ -35,7 +37,7 @@ def call private - attr_accessor :customer, :invoice_type, :currency, :datetime, :charge_in_advance + attr_accessor :customer, :invoice_type, :currency, :datetime, :charge_in_advance, :skip_charges delegate :organization, to: :customer diff --git a/app/services/invoices/subscription_service.rb b/app/services/invoices/subscription_service.rb index 9faa7ffba7b..d39835acf9c 100644 --- a/app/services/invoices/subscription_service.rb +++ b/app/services/invoices/subscription_service.rb @@ -2,7 +2,7 @@ module Invoices class SubscriptionService < BaseService - def initialize(subscriptions:, timestamp:, recurring:, invoice: nil) + def initialize(subscriptions:, timestamp:, recurring:, invoice: nil, skip_charges: false) @subscriptions = subscriptions @timestamp = timestamp @@ -17,6 +17,7 @@ def initialize(subscriptions:, timestamp:, recurring:, invoice: nil) # and if the generating invoice was persisted, # the process can be retried without creating a new invoice @invoice = invoice + @skip_charges = skip_charges super end @@ -60,7 +61,7 @@ def call private - attr_accessor :subscriptions, :timestamp, :recurring, :customer, :currency, :invoice + attr_accessor :subscriptions, :timestamp, :recurring, :customer, :currency, :invoice, :skip_charges def active_subscriptions @active_subscriptions ||= subscriptions.select(&:active?) @@ -72,6 +73,7 @@ def create_generating_invoice invoice_type: :subscription, currency:, datetime: Time.zone.at(timestamp), + skip_charges:, ) do |invoice| Invoices::CreateInvoiceSubscriptionService .call(invoice:, subscriptions:, timestamp:, recurring:) diff --git a/app/services/subscriptions/activate_service.rb b/app/services/subscriptions/activate_service.rb index 62712c5d881..bf72adeba1c 100644 --- a/app/services/subscriptions/activate_service.rb +++ b/app/services/subscriptions/activate_service.rb @@ -14,8 +14,8 @@ def activate_all_pending .pending .where(previous_subscription: nil) .where( - "DATE(subscriptions.subscription_at#{Utils::Timezone.at_time_zone_sql}) <= " \ - "DATE(?#{Utils::Timezone.at_time_zone_sql})", + "DATE(subscriptions.subscription_at#{at_time_zone}) <= " \ + "DATE(?#{at_time_zone})", Time.zone.at(timestamp), ) .find_each do |subscription| @@ -23,7 +23,9 @@ def activate_all_pending SendWebhookJob.perform_later('subscription.started', subscription) - BillSubscriptionJob.perform_later([subscription], timestamp) if subscription.plan.pay_in_advance? + if subscription.plan.pay_in_advance? && !subscription.in_trial_period? + BillSubscriptionJob.perform_later([subscription], timestamp) + end end end diff --git a/app/services/subscriptions/create_service.rb b/app/services/subscriptions/create_service.rb index 67cc7eb9b0b..30998770dcd 100644 --- a/app/services/subscriptions/create_service.rb +++ b/app/services/subscriptions/create_service.rb @@ -39,6 +39,7 @@ def call customer:, currency: plan.amount_currency, ) + return currency_result unless currency_result.success? result.subscription = handle_subscription @@ -93,6 +94,10 @@ def downgrade? plan.yearly_amount_cents < current_subscription.plan.yearly_amount_cents end + def should_be_billed_today?(sub) + sub.active? && sub.subscription_at.today? && plan.pay_in_advance? && !sub.in_trial_period? + end + def create_subscription new_subscription = Subscription.new( customer:, @@ -112,11 +117,15 @@ def create_subscription new_subscription.mark_as_active! end - if new_subscription.active? && new_subscription.subscription_at.today? && plan.pay_in_advance? - # NOTE: Since job is laucnhed from inside a db transaction - # we must wait for it to be commited before processing the job. + if should_be_billed_today?(new_subscription) + # 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 - perform_later(job_class: BillSubscriptionJob, arguments: [[new_subscription], Time.zone.now.to_i]) + perform_later( + job_class: BillSubscriptionJob, + arguments: [[new_subscription], Time.zone.now.to_i], + skip_charges: true, + ) end if new_subscription.active? @@ -149,7 +158,7 @@ def upgrade_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 + # NOTE: When upgrading, the new subscription becomes active immediately # The previous one must be terminated Subscriptions::TerminateService.call(subscription: current_subscription, upgrade: true) @@ -159,7 +168,7 @@ def upgrade_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 commited before processing the job + # we must wait for it to be committed before processing the job # We do not set offset anymore but instead retry jobs perform_later( job_class: BillSubscriptionJob, @@ -262,9 +271,9 @@ def billable_subscriptions(new_subscription) [current_subscription] end.to_a - return billable_subscriptions unless plan.pay_in_advance? + billable_subscriptions << new_subscription if plan.pay_in_advance? && !new_subscription.in_trial_period? - billable_subscriptions << new_subscription + billable_subscriptions end end end diff --git a/app/services/subscriptions/free_trial_billing_service.rb b/app/services/subscriptions/free_trial_billing_service.rb index 51a5ef492f6..97c0ad290d3 100644 --- a/app/services/subscriptions/free_trial_billing_service.rb +++ b/app/services/subscriptions/free_trial_billing_service.rb @@ -10,12 +10,15 @@ def initialize(timestamp: Time.current) def call ending_trial_subscriptions.each do |subscription| - # if subscription.plan_pay_in_advance - # BillSubscriptionJob.perform_later([subscription], timestamp) - # end + if subscription.plan_pay_in_advance && + !subscription.was_already_billed_today && + !already_billed_on_day_one?(subscription) + BillSubscriptionJob.perform_later([subscription], timestamp, recurring: false, skip_charges: true) + end - # TODO: Send webhook - subscription.update!(trial_ended_at: timestamp) + subscription.update!(trial_ended_at: subscription.trial_end_utc_date_from_query) + + SendWebhookJob.perform_later('subscription.trial_ended', subscription) end end @@ -23,12 +26,29 @@ def call attr_reader :timestamp + # This is to avoid billing at the end of the trial if the customer was billed at the beginning + # It's only for users who started billing customer AND upgraded their lago with this feature + # during the customer trial period + # Unfortunately, this introduces an N+1 query + def already_billed_on_day_one?(subscription) + Fee.subscription_kind.where( + invoice_id: subscription.invoice_subscriptions.select('invoices.id').joins(:invoice).where( + 'invoices.invoice_type' => :subscription, + 'invoices.status' => %i[draft finalized], + timestamp: subscription.started_at.all_day, + ), + ).any? + end + def ending_trial_subscriptions sql = <<-SQL WITH - initial_started_at AS (#{initial_started_at}) + initial_started_at AS (#{initial_started_at}), + already_billed_today AS (#{already_billed_today}) SELECT DISTINCT plans.pay_in_advance AS plan_pay_in_advance, + already_billed_today.invoiced_count > 0 AS was_already_billed_today, + #{trial_end_date} as trial_end_utc_date_from_query, subscriptions.* FROM subscriptions @@ -36,10 +56,11 @@ def ending_trial_subscriptions INNER JOIN initial_started_at ON initial_started_at.external_id = subscriptions.external_id INNER JOIN customers ON subscriptions.customer_id = customers.id INNER JOIN organizations ON customers.organization_id = organizations.id + LEFT JOIN already_billed_today ON already_billed_today.subscription_id = subscriptions.id WHERE subscriptions.status = 1 AND subscriptions.trial_ended_at IS NULL - AND DATE(initial_started_at#{at_time_zone} + plans.trial_period * INTERVAL '1 day') = DATE('#{timestamp}'#{at_time_zone}) + AND #{trial_end_date + at_time_zone} <= '#{timestamp}'#{at_time_zone} SQL Subscription.find_by_sql([sql, { timestamp: }]) @@ -54,5 +75,29 @@ def initial_started_at subscriptions SQL end + + def trial_end_date + <<-SQL + (initial_started_at + plans.trial_period * INTERVAL '1 day') + SQL + end + + def already_billed_today + <<-SQL + SELECT + invoice_subscriptions.subscription_id, + COUNT(invoice_subscriptions.id) AS invoiced_count + FROM invoice_subscriptions + INNER JOIN subscriptions AS sub ON invoice_subscriptions.subscription_id = sub.id + INNER JOIN customers AS cus ON sub.customer_id = cus.id + INNER JOIN organizations AS org ON cus.organization_id = org.id + WHERE invoice_subscriptions.recurring = 't' + AND invoice_subscriptions.timestamp IS NOT NULL + AND DATE( + (invoice_subscriptions.timestamp)#{at_time_zone(customer: 'cus', organization: 'org')} + ) = DATE('#{timestamp}'#{at_time_zone(customer: 'cus', organization: 'org')}) + GROUP BY invoice_subscriptions.subscription_id + SQL + end end end diff --git a/app/services/utils/transactional_jobs.rb b/app/services/utils/transactional_jobs.rb index 516a8263f98..b764334080e 100644 --- a/app/services/utils/transactional_jobs.rb +++ b/app/services/utils/transactional_jobs.rb @@ -9,15 +9,15 @@ def pending_jobs @pending_jobs ||= [] end - def perform_later(job_class:, arguments:) + def perform_later(job_class:, arguments:, **optional_arguments) return job_class.perform_later(*arguments) unless ActiveRecord::Base.connection.transaction_open? - pending_jobs << { job_class:, arguments: } + pending_jobs << { job_class:, arguments:, optional_arguments: } end def perform_pending_jobs pending_jobs.each do |job| - job[:job_class].perform_later(*job[:arguments]) + job[:job_class].perform_later(*job[:arguments], **job[:optional_arguments]) end end end diff --git a/app/services/webhooks/subscriptions/trial_ended_service.rb b/app/services/webhooks/subscriptions/trial_ended_service.rb new file mode 100644 index 00000000000..86e46fb7f9e --- /dev/null +++ b/app/services/webhooks/subscriptions/trial_ended_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Webhooks + module Subscriptions + class TrialEndedService < Webhooks::BaseService + def current_organization + @current_organization ||= object.organization + end + + def object_serializer + ::V1::SubscriptionSerializer.new( + object, + root_name: 'subscription', + includes: %i[plan customer], + ) + end + + def webhook_type + 'subscription.trial_ended' + end + + def object_type + 'subscription' + end + end + end +end diff --git a/clock.rb b/clock.rb index 1cdf7e2f320..1d016c1ca29 100644 --- a/clock.rb +++ b/clock.rb @@ -46,6 +46,10 @@ module Clockwork Clock::TerminateCouponsJob.perform_later end + every(1.hour, 'schedule:bill_ended_trial_subscriptions', at: '*:35') do + Clock::FreeTrialSubscriptionsBillerJob.perform_later + end + every(1.hour, 'schedule:terminate_wallets', at: '*:45') do Clock::TerminateWalletsJob.perform_later end diff --git a/db/migrate/20240329112415_fill_subscriptions_trial_ended_at.rb b/db/migrate/20240329112415_fill_subscriptions_trial_ended_at.rb new file mode 100644 index 00000000000..456142ac6fb --- /dev/null +++ b/db/migrate/20240329112415_fill_subscriptions_trial_ended_at.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +class FillSubscriptionsTrialEndedAt < ActiveRecord::Migration[7.0] + class Subscription < ApplicationRecord + belongs_to :customer + belongs_to :plan + + # NOTE: We reimplement the logic from Subscription#initial_started_at differently to avoid N+1 queries + # eager loading subscription.customer.subscriptions is not enough. + def initial_started_at + customer.subscriptions.select do |s| + s.external_id == external_id && s.started_at.present? + end.min_by(&:started_at)&.started_at || subscription_at + end + end + + class Customer < ApplicationRecord + has_many :subscriptions + end + + class Plan < ApplicationRecord + has_many :subscriptions + end + + def up + Subscription + .joins(:plan) + .where(trial_ended_at: nil) + .where.not(plans: { trial_period: nil }) + .includes(:plan, customer: :subscriptions) + .find_each do |subscription| + trial_ended_at = subscription.initial_started_at + subscription.plan.trial_period.days + + next if trial_ended_at.to_date >= Time.zone.today + + subscription.update(trial_ended_at:) + end + end + + def down; end +end diff --git a/db/migrate/20240404123257_add_skip_charges_bool_in_invoices.rb b/db/migrate/20240404123257_add_skip_charges_bool_in_invoices.rb new file mode 100644 index 00000000000..106b3d490a3 --- /dev/null +++ b/db/migrate/20240404123257_add_skip_charges_bool_in_invoices.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddSkipChargesBoolInInvoices < ActiveRecord::Migration[7.0] + def change + add_column :invoices, :skip_charges, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 11726de18df..a76802e9073 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_04_03_084644) do +ActiveRecord::Schema[7.0].define(version: 2024_04_04_123257) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -629,6 +629,7 @@ t.integer "organization_sequential_id", default: 0, null: false t.boolean "ready_to_be_refreshed", default: false, null: false t.datetime "payment_dispute_lost_at" + t.boolean "skip_charges", default: false, null: false t.index ["customer_id", "sequential_id"], name: "index_invoices_on_customer_id_and_sequential_id", unique: true t.index ["customer_id"], name: "index_invoices_on_customer_id" t.index ["organization_id"], name: "index_invoices_on_organization_id" diff --git a/spec/jobs/bill_subscription_job_spec.rb b/spec/jobs/bill_subscription_job_spec.rb index 5cdb8b03816..c326dfb393c 100644 --- a/spec/jobs/bill_subscription_job_spec.rb +++ b/spec/jobs/bill_subscription_job_spec.rb @@ -12,7 +12,7 @@ before do allow(Invoices::SubscriptionService).to receive(:call) - .with(subscriptions:, timestamp:, recurring:, invoice:) + .with(subscriptions:, timestamp:, recurring:, invoice:, skip_charges: false) .and_return(result) end @@ -58,7 +58,7 @@ expect(Invoices::SubscriptionService).to have_received(:call) expect(described_class).to have_been_enqueued - .with(subscriptions, timestamp, recurring: false, invoice: result_invoice) + .with(subscriptions, timestamp, recurring: false, invoice: result_invoice, skip_charges: false) end end diff --git a/spec/scenarios/invoices/invoices_spec.rb b/spec/scenarios/invoices/invoices_spec.rb index b412885a5df..fe31f6a6952 100644 --- a/spec/scenarios/invoices/invoices_spec.rb +++ b/spec/scenarios/invoices/invoices_spec.rb @@ -36,10 +36,16 @@ end subscription = customer.subscriptions.first + expect(subscription.invoices.count).to eq(0) + + travel_to(DateTime.new(2024, 3, 11, 22)) do + perform_billing + end + invoice = subscription.invoices.first expect(invoice.total_amount_cents).to eq(2371) # (31 - 3 - 7) * 35 / 31 - travel_to(DateTime.new(2024, 3, 5, 3)) do + travel_to(DateTime.new(2024, 3, 11, 23)) do terminate_subscription(subscription) end @@ -49,7 +55,7 @@ expect(invoice.reload.credit_notes.count).to eq(1) expect(invoice.credit_notes.first.total_amount_cents).to eq(2371) - travel_to(DateTime.new(2024, 3, 5, 4)) do + travel_to(DateTime.new(2024, 3, 11, 23, 5)) do create_subscription( { external_customer_id: customer.external_id, @@ -1447,7 +1453,7 @@ end subscription_invoice = Invoice.draft.first - subscription = subscription_invoice.subscriptions.first + subscription = subscription_invoice.subscriptions.sole expect(subscription_invoice.total_amount_cents).to eq(658) # 17 days - From 15th Dec. to 31st Dec. ### 17 Dec: Create event + refresh. @@ -1475,13 +1481,15 @@ .and change { subscription.invoices.count }.from(1).to(2) # Credit note is created (31 - 20) * 548 / 17.0 * 1.2 = 425.5 rounded at 426 - credit_note = subscription_invoice.credit_notes.first + credit_note = subscription_invoice.reload.credit_notes.first expect(credit_note.credit_amount_cents).to eq(426) expect(credit_note.balance_amount_cents).to eq(426) # Invoice for termination is created termination_invoice = subscription.invoices.order(created_at: :desc).first + pp subscription.invoices.count + # Total amount does not reflect the credit note as it's not finalized. expect(termination_invoice.total_amount_cents).to eq(120) expect(termination_invoice.credits.count).to eq(0) diff --git a/spec/scenarios/subscriptions/free_trial_billing_spec.rb b/spec/scenarios/subscriptions/free_trial_billing_spec.rb new file mode 100644 index 00000000000..ea15c455782 --- /dev/null +++ b/spec/scenarios/subscriptions/free_trial_billing_spec.rb @@ -0,0 +1,526 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'Free Trial Billing Subscriptions Scenario', :scenarios, type: :request do + let(:timezone) { 'UTC' } + let(:organization) { create(:organization, webhook_url: nil) } + let(:billable_metric) { create(:billable_metric, organization:) } + let(:customer) { create(:customer, organization:, timezone:) } + let(:plan) do + create( + :plan, + organization:, + trial_period:, + amount_cents: 5_000_000, + pay_in_advance: true, + ) + end + + def create_customer_subscription! + create(:standard_charge, plan:, billable_metric:, properties: { amount: '10' }) + create_subscription( + { + external_customer_id: customer.external_id, + external_id: customer.external_id, + plan_code: plan.code, + }, + ) + end + + def create_usage_event! + create_event( + { code: billable_metric.code, transaction_id: SecureRandom.uuid, external_customer_id: customer.external_id }, + ) + end + + context 'without free trial' do + let(:trial_period) { 0 } + + it 'bills the customer at the beginning of the subscription' do + travel_to(Time.zone.parse('2024-03-05T12:12:00')) do + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(1) + expect(customer.invoices.first.fees.subscription).to exist + end + end + end + + context 'with free trial' do + let(:trial_period) { 10 } + + it 'bills the customer at the end of the free trial' do + travel_to(Time.zone.parse('2024-03-05T12:12:00')) do + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + end + subscription = customer.subscriptions.sole + + # Ensure nothing happened + travel_to(Time.zone.parse('2024-03-10T12:12:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(0) + end + + # NOTE: The subscription was started at 12:12:00, so the trial period ends exactly at 12:12:00 + # This ensure that Subscriptions::FreeTrialBillingService grabs subscriptions that + # ended in the last hour. + travel_to(Time.zone.parse('2024-03-15T12:02:00')) do + expect(subscription).to be_in_trial_period + perform_billing + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-15T13:02:00')) do + expect(subscription).not_to be_in_trial_period + perform_billing + expect(customer.reload.invoices.count).to eq(1) + invoice = customer.reload.invoices.sole + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(2_741_935) # (31 - 4 - 10) / 31 * 5000000 = 2741935 + end + end + + # NOTE: This only happens if the customer was billed at the beginning of the free trial + # BEFORE the feature to bill at the end of the free trial was implemented + it 'does not bill the customer if it was already billed at the beginning of the trial' do + travel_to(Time.zone.parse('2024-03-05T12:12:00')) do + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + + plan.update! trial_period: 0 # disable trial to force billing + BillSubscriptionJob.perform_now(customer.subscriptions, Time.current) + + expect(customer.reload.invoices.count).to eq(1) + + plan.update! trial_period: 10 + end + + # Ensure nothing happened + travel_to(Time.zone.parse('2024-03-10T12:12:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(1) + end + + travel_to(Time.zone.parse('2024-03-15T15:00:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(1) + end + + travel_to(Time.zone.parse('2024-03-20T12:12:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(1) + end + end + end + + context 'with a plan upgrade during the trial' do + let(:trial_period) { 10 } + + it 'bills the subscription of the upgraded plan at the end of the trial' do + travel_to(Time.zone.parse('2024-03-05T12:12:00')) do + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + perform_billing + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-08')) { create_usage_event! } + + # Upgrade to a new plan + # It create an invoice with the old plan because there was some usage + travel_to(Time.zone.parse('2024-03-10T12:12:00')) do + upgrade_plan = create(:plan, organization:, trial_period: 13, amount_cents: 10_000_000, pay_in_advance: true) + create(:standard_charge, plan: upgrade_plan, billable_metric:, properties: { amount: '12' }) + create_subscription( + { + external_customer_id: customer.external_id, + external_id: customer.external_id, + plan_code: upgrade_plan.code, + }, + ) + perform_billing + expect(customer.reload.invoices.count).to eq(1) + invoice = customer.invoices.sole + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + end + + travel_to(Time.zone.parse('2024-03-11')) { create_usage_event! } + + # After plan.trial_period days, nothing happens + travel_to(Time.zone.parse('2024-03-15T13:00:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(1) + end + + # Using plan.started_at + upgrade_plan.trial_period days, the trial ends + travel_to(Time.zone.parse('2024-03-18T13:00:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(2) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(4_516_129) # (31 - 4 - 13) / 31 * 10000000 + end + + travel_to(Time.zone.parse('2024-04-01T12:12:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(3) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(2) + expect(invoice.fees.charge.first.amount_cents).to eq(1200) + expect(invoice.fees.subscription.first.amount_cents).to eq(10_000_000) + end + end + + context 'when the upgrade happens on day one' do + it 'bills the usage instantly and the subscription of the upgraded plan at the end of trial period' do + travel_to(Time.zone.parse('2024-03-05T12:12:00')) do + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + perform_billing + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-05T13:00:00')) { create_usage_event! } + + # Upgrade to a new plan + # It create an invoice with the old plan because there was some usage + travel_to(Time.zone.parse('2024-03-05T13:15:00')) do + upgrade_plan = create(:plan, organization:, trial_period: 13, amount_cents: 10_000_000, pay_in_advance: true) + create(:standard_charge, plan: upgrade_plan, billable_metric:, properties: { amount: '12' }) + create_subscription( + { + external_customer_id: customer.external_id, + external_id: customer.external_id, + plan_code: upgrade_plan.code, + }, + ) + perform_billing + expect(customer.reload.invoices.count).to eq(1) + invoice = customer.invoices.sole + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + end + + travel_to(Time.zone.parse('2024-03-05T15:00:00')) { create_usage_event! } + travel_to(Time.zone.parse('2024-03-05T15:01:00')) { create_usage_event! } + + # Using plan.started_at + upgrade_plan.trial_period days, the trial ends + travel_to(Time.zone.parse('2024-03-18T13:00:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(2) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(4_516_129) # (31 - 4 - 13) / 31 * 10000000 + end + + travel_to(Time.zone.parse('2024-04-01T12:12:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(3) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(2) + expect(invoice.fees.charge.first.amount_cents).to eq(2400) + expect(invoice.fees.subscription.first.amount_cents).to eq(10_000_000) + end + end + end + + context 'with a grace period' do + it 'bills the usage instantly and the subscription of the upgraded plan at the end of trial period' do + travel_to(Time.zone.parse('2024-03-05T12:12:00')) do + customer.update! invoice_grace_period: 2 + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + perform_billing + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-05T13:00:00')) { create_usage_event! } + + # Upgrade to a new plan + # It create an invoice with the old plan because there was some usage + travel_to(Time.zone.parse('2024-03-05T13:15:00')) do + upgrade_plan = create(:plan, organization:, trial_period: 13, amount_cents: 10_000_000, pay_in_advance: true) + create(:standard_charge, plan: upgrade_plan, billable_metric:, properties: { amount: '12' }) + create_subscription( + { + external_customer_id: customer.external_id, + external_id: customer.external_id, + plan_code: upgrade_plan.code, + }, + ) + perform_billing + expect(customer.reload.invoices.count).to eq(1) + invoice = customer.invoices.sole + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + expect(invoice.status).to eq('draft') + end + + travel_to(Time.zone.parse('2024-03-07T18:00:00')) do + Clock::FinalizeInvoicesJob.perform_later + perform_all_enqueued_jobs + + invoice = customer.invoices.reload.sole + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + expect(invoice.status).to eq('finalized') + end + + travel_to(Time.zone.parse('2024-03-05T15:00:00')) { create_usage_event! } + travel_to(Time.zone.parse('2024-03-05T15:01:00')) { create_usage_event! } + + # Using plan.started_at + upgrade_plan.trial_period days, the trial ends + travel_to(Time.zone.parse('2024-03-18T13:00:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(2) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(4_516_129) # (31 - 4 - 13) / 31 * 10000000 + end + + travel_to(Time.zone.parse('2024-04-01T12:12:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(3) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(2) + expect(invoice.fees.charge.first.amount_cents).to eq(2400) + expect(invoice.fees.subscription.first.amount_cents).to eq(10_000_000) + end + end + end + end + + context 'with free trial > billing period' do + let(:trial_period) { 45 } + + it 'bills subscription at the end of the free trial' do + travel_to(Time.zone.parse('2024-03-05T12:12:00')) do + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-10')) { create_usage_event! } + + travel_to(Time.zone.parse('2024-04-01')) do + perform_billing + expect(customer.reload.invoices.count).to eq(1) + invoice = customer.invoices.sole + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + end + + travel_to(Time.zone.parse('2024-04-19T13:01:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(2) + free_trial_invoice = customer.invoices.order(created_at: :desc).first + expect(free_trial_invoice.fees.count).to eq(1) + expect(free_trial_invoice.fees.subscription.first.amount_cents).to eq(2_000_000) # 5_000_000 * 12 / 30 + end + end + + context 'with a grace period' do + it 'bills the customer at the end of the free trial but finalize after grace period' do + travel_to(Time.zone.parse('2024-03-05T12:12:00')) do + customer.update! invoice_grace_period: 2 + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-10')) { create_usage_event! } + + travel_to(Time.zone.parse('2024-04-01')) do + perform_billing + expect(customer.reload.invoices.count).to eq(1) + invoice = customer.invoices.sole + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + end + + travel_to(Time.zone.parse('2024-04-19T13:01:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(2) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(2_000_000) # 5_000_000 * 12 / 30 + expect(invoice.status).to eq('draft') + end + + # Ensure charge fees are not added when refreshing the invoice + travel_to(Time.zone.parse('2024-04-21T13:22:00')) do + invoice = customer.invoices.order(created_at: :desc).first + Invoices::RefreshDraftJob.perform_later(invoice) + perform_all_enqueued_jobs + expect(customer.reload.invoices.count).to eq(2) + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(2_000_000) + expect(invoice.status).to eq('draft') + + Clock::FinalizeInvoicesJob.perform_later + perform_all_enqueued_jobs + + expect(customer.reload.invoices.count).to eq(2) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(2_000_000) + expect(invoice.status).to eq('finalized') + end + end + end + + context 'with a plan with minimum commitment' do + it 'bills minimum commitment on billing day, despite being in trial' do + travel_to(Time.zone.parse('2024-03-05T12:12:00')) do + create(:commitment, :minimum_commitment, plan:, amount_cents: 10_000_000) + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-10')) { create_usage_event! } + + travel_to(Time.zone.parse('2024-04-01')) do + perform_billing + expect(customer.reload.invoices.count).to eq(1) + invoice = customer.invoices.sole + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + end + + travel_to(Time.zone.parse('2024-04-10')) { create_usage_event! } + + travel_to(Time.zone.parse('2024-04-19T13:01:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(2) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(2_000_000) # 5_000_000 * 12 / 30 + end + + travel_to(Time.zone.parse('2024-05-01')) do + perform_billing + expect(customer.reload.invoices.count).to eq(3) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(3) + expect(invoice.fees.subscription.first.amount_cents).to eq(5_000_000) + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + # The minimum commitment true up look at usage in previous month, + # when the trial ended and the customer paid only 2_000_000 in subscription fee + expect(invoice.fees.commitment.first.amount_cents).to eq(10_000_000 - 2_000_000 - 1000) + end + end + end + end + + context 'with free trial ending on billing day' do + let(:trial_period) { 10 } + + it 'bills subscription and usage-based charges' do + start_time = Time.zone.parse('2024-03-22T12:12:00') + travel_to(start_time) do + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-23')) { create_usage_event! } + + expect(customer.reload.invoices.count).to eq(0) + + # NOTE: Subscriptions::BillingService will bill the subscription because it's billing day + # Subscriptions::FreeTrialBillingService will ignore it because the trial ends at 12:12:00 + travel_to(Time.zone.parse('2024-04-01')) do + perform_billing + invoice = customer.invoices.order(created_at: :desc).sole + expect(invoice.fees.subscription.first.amount_cents).to eq(5_000_000) # full fee, trial is over + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + end + + # NOTE: After the trial ends, we don't invoice again because it was done above + # but we terminate the trial and send the webhook + travel_to(Time.zone.parse('2024-04-01T13:11:00')) do + perform_billing + expect(customer.reload.invoices.count).to eq(1) + expect(customer.subscriptions.sole.trial_ended_at).to match_datetime(start_time + trial_period.days) + end + end + + context 'with customer with a timezone' do + let(:trial_period) { 10 } + let(:timezone) { 'Asia/Tokyo' } + + it 'follows customer timezone for billing' do + # Trial ends on April 1st, 2024 in UTC + # but April 2nd, 2024 in Asia/Tokyo + start_time = Time.parse('2024-03-22T18:12:00 UTC').in_time_zone(timezone) + travel_to(start_time) do + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-28')) { create_usage_event! } + + expect(customer.reload.invoices.count).to eq(0) + + # NOTE: Billing day in Asia/Tokyo + travel_to(Time.parse('2024-03-31T15:10:00 UTC')) do # 2024-04-01T00:10:00 Asia/Tokyo + perform_billing + invoice = customer.invoices.order(created_at: :desc).sole + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.charge.first.amount_cents).to eq(1000) + end + + # April 1st in both timezone, nothing should happen + travel_to(Time.parse('2024-04-01T13:11:00 UTC')) do + perform_billing + expect(customer.reload.invoices.count).to eq(1) + end + + travel_to(Time.parse('2024-04-01T19:11:00 UTC')) do # April 2nd, 2024 04:11:00 Asia/Tokyo, trial ended + perform_billing + expect(customer.reload.invoices.count).to eq(2) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(4_833_333) # Trial ends on the 2nd in customer tz + end + end + end + + context 'with SubscriptionsBillerJob running after FreeTrialSubscriptionsBillerJob' do + it 'bills subscription and usage-based charges' do + start_time = Time.zone.parse('2024-03-22T12:12:00') + travel_to(start_time) do + create_customer_subscription! + expect(customer.reload.invoices.count).to eq(0) + end + + travel_to(Time.zone.parse('2024-03-23')) { create_usage_event! } + + expect(customer.reload.invoices.count).to eq(0) + + travel_to(Time.zone.parse('2024-04-01T13:01:00')) do + Clock::FreeTrialSubscriptionsBillerJob.perform_later + perform_all_enqueued_jobs + + invoice = customer.invoices.order(created_at: :desc).sole + expect(customer.subscriptions.sole.trial_ended_at).to match_datetime(start_time + trial_period.days) + # NOTE: The charge are not billed because FreeTrialBillingService use `skip_charges: true` + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.subscription.first.amount_cents).to eq(5_000_000) # full fee, trial is over + end + + # NOTE: A new invoice is created because the end of trial invoice is created with `recurring: false` + # Only the usage-based is charged because subscription was already billed + # see Invoices::CalculateFeesService.should_create_subscription_fee? + travel_to(Time.zone.parse('2024-04-01T15:11:00')) do + Clock::SubscriptionsBillerJob.perform_later + perform_all_enqueued_jobs + + expect(customer.reload.invoices.count).to eq(2) + invoice = customer.invoices.order(created_at: :desc).first + expect(invoice.fees.count).to eq(1) + expect(invoice.fees.charge.sole.amount_cents).to eq(1000) + end + end + end + end +end diff --git a/spec/serializers/v1/subscription_serializer_spec.rb b/spec/serializers/v1/subscription_serializer_spec.rb index cf766851014..9d57eff1e92 100644 --- a/spec/serializers/v1/subscription_serializer_spec.rb +++ b/spec/serializers/v1/subscription_serializer_spec.rb @@ -27,6 +27,7 @@ 'billing_time' => subscription.billing_time, 'created_at' => subscription.created_at.iso8601, 'ending_at' => subscription.ending_at.iso8601, + 'trial_ended_at' => nil, ) expect(result['subscription']['customer']['lago_id']).to be_present diff --git a/spec/services/invoices/calculate_fees_service_spec.rb b/spec/services/invoices/calculate_fees_service_spec.rb index 79d1ea5dfd7..e695162d918 100644 --- a/spec/services/invoices/calculate_fees_service_spec.rb +++ b/spec/services/invoices/calculate_fees_service_spec.rb @@ -69,10 +69,11 @@ let(:terminated_at) { nil } let(:status) { :active } - let(:plan) { create(:plan, organization:, interval:, pay_in_advance:) } + let(:plan) { create(:plan, organization:, interval:, pay_in_advance:, trial_period:) } let(:pay_in_advance) { false } let(:billing_time) { :calendar } let(:interval) { 'monthly' } + let(:trial_period) { 0 } let(:charge) { create(:standard_charge, plan: subscription.plan, charge_model: 'standard') } @@ -679,7 +680,7 @@ expect(invoice.subscriptions.first).to eq(subscription) expect(invoice.payment_status).to eq('pending') expect(invoice.fees.subscription_kind.count).to eq(1) - expect(invoice.fees.charge_kind.count).to eq(0) + expect(invoice).to have_empty_charge_fees invoice_subscription = invoice.invoice_subscriptions.first expect(invoice_subscription).to have_attributes( @@ -713,7 +714,7 @@ expect(invoice.subscriptions.first).to eq(subscription) expect(invoice.payment_status).to eq('pending') expect(invoice.fees.subscription_kind.count).to eq(1) - expect(invoice.fees.charge_kind.count).to eq(0) + expect(invoice).to have_empty_charge_fees invoice_subscription = invoice.invoice_subscriptions.first expect(invoice_subscription).to have_attributes( @@ -735,6 +736,22 @@ end end + context 'when plan is in trial period' do + let(:trial_period) { 45 } + let(:started_at) { 40.days.ago } + + it 'does not create a subscription fee' do + subscription.created_at + result = invoice_service.call + + aggregate_failures do + expect(result).to be_success + + expect(invoice.fees.subscription_kind.count).to eq(0) + end + end + end + context 'when subscription was already billed earlier the same day' do let(:timestamp) { Time.current } @@ -814,7 +831,7 @@ aggregate_failures do expect(result).to be_success - expect(invoice.fees.charge_kind.count).to eq(0) + expect(invoice).to have_empty_charge_fees end end end @@ -856,7 +873,7 @@ expect(invoice).to be_pending expect(invoice.fees.subscription_kind.count).to eq(1) - expect(invoice.fees.charge_kind.count).to eq(0) + expect(invoice).to have_empty_charge_fees invoice_subscription = invoice.invoice_subscriptions.first expect(invoice_subscription).to have_attributes( @@ -890,7 +907,7 @@ expect(invoice).to be_pending expect(invoice.fees.subscription_kind.count).to eq(1) - expect(invoice.fees.charge_kind.count).to eq(0) + expect(invoice).to have_empty_charge_fees invoice_subscription = invoice.invoice_subscriptions.first expect(invoice_subscription).to have_attributes( @@ -1069,7 +1086,7 @@ expect(invoice.subscriptions.first).to eq(subscription) expect(invoice.fees.subscription_kind.count).to eq(1) - expect(invoice.fees.charge_kind.count).to eq(0) + expect(invoice).to have_empty_charge_fees # Because we didn't fake usage events invoice_subscription = invoice.invoice_subscriptions.first expect(invoice_subscription).to have_attributes( diff --git a/spec/services/subscriptions/activate_service_spec.rb b/spec/services/subscriptions/activate_service_spec.rb index 9ddc9334afd..47bd29d460f 100644 --- a/spec/services/subscriptions/activate_service_spec.rb +++ b/spec/services/subscriptions/activate_service_spec.rb @@ -7,45 +7,51 @@ let(:timestamp) { Time.current } - describe 'activate_all_expired' do - let(:active_subscription) { create(:subscription) } - let(:pending_subscriptions) { create_list(:subscription, 3, :pending, subscription_at: timestamp) } - - let(:future_pending_subscriptions) do + describe '.activate_all_expired' do + it 'activates all pending subscriptions with subscription date set to today' do + create(:subscription) + create_list(:subscription, 2, :pending, subscription_at: timestamp) + create(:subscription, :pending, subscription_at: timestamp, plan: create(:plan, pay_in_advance: true)) create_list(:subscription, 2, :pending, subscription_at: (timestamp + 10.days)) - end - - before do - active_subscription - pending_subscriptions - future_pending_subscriptions - end - it 'activates all pending subscriptions with subscription date set to today' do expect { activate_service.activate_all_pending } .to change(Subscription.pending, :count).by(-3) .and change(Subscription.active, :count).by(3) - end - - it 'enqueues a SendWebhookJob' do - expect do - activate_service.activate_all_pending - end.to have_enqueued_job(SendWebhookJob).at_least(1).times + .and have_enqueued_job(SendWebhookJob).exactly(3).times + .and have_enqueued_job(BillSubscriptionJob).once end context 'with customer timezone' do let(:timestamp) { DateTime.parse('2023-08-24 00:07:00') } - let(:pending_subscription) { pending_subscriptions.first } - - before do - pending_subscription.customer.update!(timezone: 'America/Bogota') - pending_subscription.update!(subscription_at: DateTime.parse('2023-08-24 04:17:00')) + let!(:pending_subscription) do + create( + :subscription, + :pending, + { subscription_at: timestamp, customer: create(:customer, timezone: 'America/Bogota') }, + ) end it 'takes timezone into account' do + activate_service.activate_all_pending + expect(pending_subscription.reload).to be_active + end + end + + context 'with a subscription in trial' do + it do + create(:subscription, :pending, subscription_at: timestamp, plan: create(:plan, pay_in_advance: true)) + create( + :subscription, + :pending, + subscription_at: timestamp, + plan: create(:plan, pay_in_advance: true, trial_period: 10), + ) + expect { activate_service.activate_all_pending } - .to change(Subscription.pending, :count).by(-3) - .and change(Subscription.active, :count).by(3) + .to change(Subscription.pending, :count).by(-2) + .and change(Subscription.active, :count).by(2) + .and have_enqueued_job(SendWebhookJob).exactly(2).times + .and have_enqueued_job(BillSubscriptionJob).once end end end diff --git a/spec/services/subscriptions/create_service_spec.rb b/spec/services/subscriptions/create_service_spec.rb index 1e78257642d..ab07557827c 100644 --- a/spec/services/subscriptions/create_service_spec.rb +++ b/spec/services/subscriptions/create_service_spec.rb @@ -227,6 +227,14 @@ end end + context 'when plan is pay_in_advance and subscription_at is current date but there is a trial period' do + let(:plan) { create(:plan, amount_cents: 100, organization:, pay_in_advance: true, trial_period: 10) } + + it 'does not enqueue a job to bill the subscription' do + expect { create_service.call }.not_to have_enqueued_job(BillSubscriptionJob) + end + end + context 'when customer is missing' do let(:customer) { nil } let(:external_customer_id) { nil } diff --git a/spec/services/subscriptions/free_trial_billing_service_spec.rb b/spec/services/subscriptions/free_trial_billing_service_spec.rb index aebf04cd9c1..180c1420359 100644 --- a/spec/services/subscriptions/free_trial_billing_service_spec.rb +++ b/spec/services/subscriptions/free_trial_billing_service_spec.rb @@ -6,23 +6,24 @@ subject(:service) { described_class.new(timestamp:) } describe '#call' do - let(:timestamp) { Time.current.change(usec: 0) } + let(:timestamp) { Time.zone.parse('2024-04-15T13:00:00') } let(:plan) { create(:plan, trial_period: 10, pay_in_advance: true) } context 'without any ending trial subscriptions' do it 'does not set trial_ended_at', :aggregate_failures do sub1 = create(:subscription, plan:, started_at: 2.days.ago) - sub2 = create(:subscription, plan:, started_at: 15.days.ago) expect { service.call }.not_to change { sub1.reload.trial_ended_at }.from(nil) - expect { service.call }.not_to change { sub2.reload.trial_ended_at }.from(nil) end end context 'with ending trial subscriptions' do - it 'sets trial_ended_at to current time' do - sub = create(:subscription, plan:, started_at: 10.days.ago) - expect { service.call }.to change { sub.reload.trial_ended_at }.from(nil).to(timestamp) + it 'sets trial_ended_at to trial end date' do + sub = create(:subscription, plan:, started_at: Time.zone.parse('2024-04-05T12:12:00')) + sub2 = create(:subscription, plan:, started_at: 15.days.ago) + service.call + expect(sub.reload.trial_ended_at).to match_datetime(sub.trial_end_datetime) + expect(sub2.reload.trial_ended_at).to match_datetime(sub2.trial_end_datetime) end end @@ -31,23 +32,22 @@ customer = create(:customer) attr = { customer:, plan:, external_id: 'abc123' } sub = create(:subscription, started_at: 6.days.ago, **attr) - create(:subscription, started_at: 10.days.ago, terminated_at: 6.days.ago, status: :terminated, **attr) + started_at = (10.days + 1.hour).ago + create(:subscription, started_at:, terminated_at: 6.days.ago, status: :terminated, **attr) - expect { service.call }.to change { sub.reload.trial_ended_at }.from(nil).to(timestamp) + expect { service.call }.to change { sub.reload.trial_ended_at }.from(nil).to(sub.trial_end_datetime) end end context 'with customer timezone' do - let(:timestamp) { DateTime.parse('2024-03-12 01:00:00 UTC') } + let(:timestamp) { DateTime.parse('2024-03-11 13:03:00 UTC') } - it 'sets trial_ended_at to the expected subscription', :aggregate_failures do + it 'sets trial_ended_at to the expected subscription (timezone is irrelevant)', :aggregate_failures do started_at = DateTime.parse('2024-03-01 12:00:00 UTC') customer = create(:customer, timezone: 'America/Los_Angeles') sub = create(:subscription, plan:, customer:, started_at:) - utc_sub = create(:subscription, plan:, started_at:) - - expect { service.call }.to change { sub.reload.trial_ended_at }.from(nil).to(timestamp) - expect { service.call }.not_to change { utc_sub.reload.trial_ended_at }.from(nil) + service.call + expect(sub.reload.trial_ended_at).to match_datetime(sub.trial_end_datetime) end end end diff --git a/spec/services/webhooks/subscriptions/terminated_service_spec.rb b/spec/services/webhooks/subscriptions/terminated_service_spec.rb index d88d56ad991..0ccb6c7913e 100644 --- a/spec/services/webhooks/subscriptions/terminated_service_spec.rb +++ b/spec/services/webhooks/subscriptions/terminated_service_spec.rb @@ -18,7 +18,7 @@ allow(lago_client).to receive(:post_with_response) end - it 'builds payload with subscription.started webhook type' do + it 'builds payload with subscription.terminated webhook type' do webhook_service.call expect(LagoHttpClient::Client).to have_received(:new) diff --git a/spec/services/webhooks/subscriptions/trial_ended_service_spec.rb b/spec/services/webhooks/subscriptions/trial_ended_service_spec.rb new file mode 100644 index 00000000000..aefd8630b56 --- /dev/null +++ b/spec/services/webhooks/subscriptions/trial_ended_service_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Webhooks::Subscriptions::TrialEndedService do + subject(:webhook_service) { described_class.new(object: subscription) } + + let(:subscription) { create(:subscription, plan: create(:plan, trial_period: 1)) } + let(:organization) { subscription.organization } + + describe '.call' do + let(:lago_client) { instance_double(LagoHttpClient::Client) } + + before do + allow(LagoHttpClient::Client).to receive(:new) + .with(organization.webhook_endpoints.first.webhook_url) + .and_return(lago_client) + allow(lago_client).to receive(:post_with_response) + end + + it 'builds payload with subscription.trial_ended webhook type' do + webhook_service.call + + expect(LagoHttpClient::Client).to have_received(:new) + .with(organization.webhook_endpoints.first.webhook_url) + expect(lago_client).to have_received(:post_with_response) do |payload| + expect(payload[:webhook_type]).to eq('subscription.trial_ended') + expect(payload[:object_type]).to eq('subscription') + end + end + end +end diff --git a/spec/support/matchers/have_empty_charge_fees.rb b/spec/support/matchers/have_empty_charge_fees.rb new file mode 100644 index 00000000000..2f3a53049f1 --- /dev/null +++ b/spec/support/matchers/have_empty_charge_fees.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +RSpec::Matchers.define :have_empty_charge_fees do + match do |invoice| + invoice.fees.charge.all? { |fee| fee.total_amount_cents.zero? } + end + + failure_message do |invoice| + "expected that #{invoice} would have empty charge fees but fees were found.\n" \ + "Fees: #{invoice.fees.charge.all.map(&:total_amount_cents)}" + end + + failure_message_when_negated do |invoice| + "expected that #{invoice} would have some charge fees but none were found.\n" \ + "Fees: #{invoice.fees.charge.all.map(&:total_amount_cents)}" + end +end diff --git a/spec/support/scenarios_helper.rb b/spec/support/scenarios_helper.rb index fbad1450138..0c4238c23b2 100644 --- a/spec/support/scenarios_helper.rb +++ b/spec/support/scenarios_helper.rb @@ -121,6 +121,7 @@ def perform_all_enqueued_jobs def perform_billing Clock::SubscriptionsBillerJob.perform_later + Clock::FreeTrialSubscriptionsBillerJob.perform_later perform_all_enqueued_jobs end