Skip to content

Commit

Permalink
feat(charges): Add regroup_paid_fees to Charge (#2232)
Browse files Browse the repository at this point in the history
## Context

When your charge is pay in advance but not invoiceable, the fees are
created but not attached to any invoice.
This adds an options so at the end of the period, you get a new invoice
showing all PAID fees. unpaid fees are ignored.

See also #2171

## Description

We're dropping the idea of `invoice_strategy` because it turned out to
be a bad idea. Instead we're adding `regroup_paid_fees` attribute that
takes only one value today `invoice`.
  • Loading branch information
julienbourdeau authored Jul 4, 2024
1 parent d90692e commit 340e9e6
Show file tree
Hide file tree
Showing 23 changed files with 226 additions and 31 deletions.
1 change: 1 addition & 0 deletions app/controllers/api/v1/plans_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ def input_params
:pay_in_advance,
:prorated,
:invoiceable,
:regroup_paid_fees,
:min_amount_cents,
{
properties: {}
Expand Down
1 change: 1 addition & 0 deletions app/graphql/types/charges/object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class Object < Types::BaseObject
field :pay_in_advance, Boolean, null: false
field :properties, Types::Charges::Properties, null: true
field :prorated, Boolean, null: false
field :regroup_paid_fees, Types::Charges::RegroupPaidFeesEnum, null: true

field :filters, [Types::ChargeFilters::Object], null: true

Expand Down
11 changes: 11 additions & 0 deletions app/graphql/types/charges/regroup_paid_fees_enum.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

module Types
module Charges
class RegroupPaidFeesEnum < Types::BaseEnum
Charge::REGROUPING_PAID_FEES_OPTIONS.each do |type|
value type
end
end
end
end
12 changes: 12 additions & 0 deletions app/models/charge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ class Charge < ApplicationRecord
custom
].freeze

REGROUPING_PAID_FEES_OPTIONS = %i[invoice].freeze

enum charge_model: CHARGE_MODELS
enum regroup_paid_fees: REGROUPING_PAID_FEES_OPTIONS

validate :validate_amount, if: -> { standard? && group_properties.empty? }
validate :validate_graduated, if: -> { graduated? && group_properties.empty? }
Expand All @@ -40,6 +43,7 @@ class Charge < ApplicationRecord
validates :charge_model, presence: true

validate :validate_pay_in_advance
validate :validate_regroup_paid_fees
validate :validate_prorated
validate :validate_min_amount_cents
validate :validate_uniqueness_group_properties
Expand Down Expand Up @@ -98,6 +102,14 @@ def validate_pay_in_advance
end
end

# NOTE: regroup_paid_fees only works with pay_in_advance and non-invoiceable charges
def validate_regroup_paid_fees
return if regroup_paid_fees.nil?
return if pay_in_advance? && !invoiceable?

errors.add(:regroup_paid_fees, :only_compatible_with_pay_in_advance_and_non_invoiceable)
end

def validate_min_amount_cents
return unless pay_in_advance? && min_amount_cents.positive?

Expand Down
1 change: 1 addition & 0 deletions app/serializers/v1/charge_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def serialize
created_at: model.created_at.iso8601,
charge_model: model.charge_model,
invoiceable: model.invoiceable,
regroup_paid_fees: model.regroup_paid_fees,
pay_in_advance: model.pay_in_advance,
prorated: model.prorated,
min_amount_cents: model.min_amount_cents,
Expand Down
8 changes: 6 additions & 2 deletions app/services/invoices/advance_charges_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ def initialize(subscriptions:, billing_at:)
end

def call
# TODO: check if related charges have correct `invoicing_strategy`
# return result unless has_charges_to_invoice?
return result unless has_charges_with_statement?

return result if subscriptions.empty?

Expand All @@ -40,6 +39,11 @@ def call

attr_accessor :subscriptions, :billing_at, :customer, :organization, :currency

def has_charges_with_statement?
plan_ids = subscriptions.pluck(:plan_id)
Charge.where(plan_id: plan_ids, pay_in_advance: true, invoiceable: false, regroup_paid_fees: :invoice).any?
end

def create_group_invoice
invoice = nil

Expand Down
1 change: 1 addition & 0 deletions app/services/plans/create_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ def create_charge(plan, args)

if License.premium?
charge.invoiceable = args[:invoiceable] unless args[:invoiceable].nil?
charge.regroup_paid_fees = args[:regroup_paid_fees] if args.has_key?(:regroup_paid_fees)
charge.min_amount_cents = args[:min_amount_cents] || 0
end

Expand Down
1 change: 1 addition & 0 deletions app/services/plans/update_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def create_charge(plan, params)

if License.premium?
charge.invoiceable = params[:invoiceable] unless params[:invoiceable].nil?
charge.regroup_paid_fees = params[:regroup_paid_fees] if params.has_key?(:regroup_paid_fees)
charge.min_amount_cents = params[:min_amount_cents] || 0
end

Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ en:
missing_volume_ranges: missing_volume_ranges
not_compatible_with_aggregation_type: not_compatible_with_aggregation_type
not_compatible_with_pay_in_advance: not_compatible_with_pay_in_advance
only_compatible_with_pay_in_advance_and_non_invoiceable: only_compatible_with_pay_in_advance_and_non_invoiceable
required: relation_must_exist
taken: value_already_exist
too_long: value_is_too_long
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
class UpdatePropertiesOnPercentageCharges < ActiveRecord::Migration[7.0]
def change
LagoApi::Application.load_tasks
Rake::Task['charges:update_properties_for_free_units'].invoke
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

class AddRateToPercentageAmountDetails < ActiveRecord::Migration[7.0]
class Fee < ApplicationRecord
belongs_to :charge, -> { with_discarded }, optional: true
belongs_to :charge, optional: true
end

class Charge < ApplicationRecord
enum charge_model: %i[percentage graduated_percentage].freeze
end

def up
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# frozen_string_literal: true

class Charge
attribute :regroup_paid_fees, :integer, default: nil
end

class AddCustomAggregationToOrganizations < ActiveRecord::Migration[7.0]
def change
add_column :organizations, :custom_aggregation, :boolean, default: false
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20240702081109_add_regroup_paid_fees_to_charges.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddRegroupPaidFeesToCharges < ActiveRecord::Migration[7.1]
def change
add_column :charges, :regroup_paid_fees, :integer, default: nil
end
end
3 changes: 2 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 0 additions & 14 deletions lib/tasks/charges.rake
Original file line number Diff line number Diff line change
@@ -1,20 +1,6 @@
# frozen_string_literal: true

namespace :charges do
desc 'Update Properties for Fixed Fee and Free Units'
task update_properties_for_free_units: :environment do
# Notes: We consider that we don’t have any clients with a percentage charge
# created containing a fixed_amount. All existing charges have fixed_amount
# and fixed_amount_target with a null value.

Charge.unscoped.percentage.where("properties -> 'fixed_amount_target' IS NOT NULL").find_each do |charge|
charge.properties.delete('fixed_amount_target')
charge.properties['free_units_per_events'] = nil
charge.properties['free_units_per_total_aggregation'] = nil
charge.save!
end
end

desc 'Set graduated properties to hash and rename volume ranges'
task update_graduated_properties_to_hash: :environment do
# Rename existing volume ranges from `ranges: []` to `volume_ranges: []`
Expand Down
5 changes: 5 additions & 0 deletions schema.graphql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions spec/factories/charges.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,11 @@
trait :pay_in_advance do
pay_in_advance { true }
end

trait :regroup_paid_fees do
pay_in_advance { true }
invoiceable { false }
regroup_paid_fees { 'invoice' }
end
end
end
1 change: 1 addition & 0 deletions spec/graphql/types/charges/object_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
it { is_expected.to have_field(:invoice_display_name).of_type('String') }
it { is_expected.to have_field(:billable_metric).of_type('BillableMetric!') }
it { is_expected.to have_field(:charge_model).of_type('ChargeModelEnum!') }
it { is_expected.to have_field(:regroup_paid_fees).of_type('RegroupPaidFeesEnum') }
it { is_expected.to have_field(:invoiceable).of_type('Boolean!') }
it { is_expected.to have_field(:min_amount_cents).of_type('BigInt!') }
it { is_expected.to have_field(:pay_in_advance).of_type('Boolean!') }
Expand Down
30 changes: 30 additions & 0 deletions spec/models/charge_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,36 @@
end
end

describe '#validate_regroup_paid_fees' do
context 'when regroup_paid_fees is nil' do
it 'does not return an error when' do
expect(build(:standard_charge, pay_in_advance: true, invoiceable: true, regroup_paid_fees: nil)).to be_valid
expect(build(:standard_charge, pay_in_advance: true, invoiceable: false, regroup_paid_fees: nil)).to be_valid
expect(build(:standard_charge, pay_in_advance: false, invoiceable: true, regroup_paid_fees: nil)).to be_valid
expect(build(:standard_charge, pay_in_advance: false, invoiceable: false, regroup_paid_fees: nil)).to be_valid
end
end

context 'when regroup_paid_fees is `invoice`' do
it 'requires charge to be pay_in_advance and non invoiceable' do
expect(build(:standard_charge, pay_in_advance: true, invoiceable: false, regroup_paid_fees: 'invoice')).to be_valid

[
{pay_in_advance: true, invoiceable: true},
{pay_in_advance: false, invoiceable: true},
{pay_in_advance: false, invoiceable: false}
].each do |params|
charge = build(:standard_charge, regroup_paid_fees: 'invoice', **params)

aggregate_failures do
expect(charge).not_to be_valid
expect(charge.errors.messages[:regroup_paid_fees]).to include('only_compatible_with_pay_in_advance_and_non_invoiceable')
end
end
end
end
end

describe '#validate_min_amount_cents' do
it 'does not return an error' do
expect(build(:standard_charge)).to be_valid
Expand Down
Loading

0 comments on commit 340e9e6

Please sign in to comment.