feat(trial): Invoice subscription fee at the end of the trial period (#…

…1847)

## 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:
#1836
* Handle grace_period
* Handle minimum_commitment

---------

Co-authored-by: Romain Sempé <[email protected]>

.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

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

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

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)

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?

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,

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

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
 

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:)

app/services/subscriptions/activate_service.rb

@@ -14,16 +14,18 @@ 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|
           subscription.mark_as_active!(Time.zone.at(timestamp))
 
           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
 

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

app/services/subscriptions/free_trial_billing_service.rb

@@ -10,36 +10,57 @@ 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
 
     private
 
     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
           INNER JOIN plans ON subscriptions.plan_id = plans.id
           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

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