From 1a89944d24c3e62296a6b770c2c72f2ec83bd6f6 Mon Sep 17 00:00:00 2001 From: Toon Willems Date: Fri, 23 Aug 2024 18:34:41 +0200 Subject: [PATCH 1/5] Add CreditNotes::CreateFromProgressiveBillingInvoice --- app/models/invoice.rb | 2 + ...create_from_progressive_billing_invoice.rb | 60 +++++++++++++++++++ .../credits/progressive_billing_service.rb | 6 +- .../invoices/calculate_fees_service_spec.rb | 28 +++++++++ 4 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 app/services/credit_notes/create_from_progressive_billing_invoice.rb diff --git a/app/models/invoice.rb b/app/models/invoice.rb index a55d654b443..4429f99ebba 100644 --- a/app/models/invoice.rb +++ b/app/models/invoice.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Invoice < ApplicationRecord + self.ignored_columns += [:negative_amount_cents] # TODO: remove when negative_amount_cents is removed from the database + include AASM include PaperTrailTraceable include Sequenced diff --git a/app/services/credit_notes/create_from_progressive_billing_invoice.rb b/app/services/credit_notes/create_from_progressive_billing_invoice.rb new file mode 100644 index 00000000000..ff985e57467 --- /dev/null +++ b/app/services/credit_notes/create_from_progressive_billing_invoice.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module CreditNotes + class CreateFromProgressiveBillingInvoice < BaseService + def initialize(progressive_billing_invoice:, amount:, reason: :other) + @progressive_billing_invoice = progressive_billing_invoice + @amount = amount + @reason = reason + + super + end + + def call + return result unless amount.positive? + + # Important to call this method as it modifies @amount if needed + items = calculate_items! + + CreditNotes::CreateService.new( + invoice: progressive_billing_invoice, + credit_amount_cents: amount, + items:, + reason:, + automatic: true + ).call.raise_if_error! + end + + private + + attr_reader :progressive_billing_invoice, :amount, :reason + + def calculate_items! + items = [] + remaining = amount + + # The amount can be greater than a single fee amount. We'll keep on deducting until we've credited enough + progressive_billing_invoice.fees.each do |fee| + # no further credit remaining + break if remaining.zero? + + # take the lower value of remaining or maximum creditable for this fee. (whichever is the lowest) + fee_credit_amount = [remaining, fee.creditable_amount_cents].min + items << { + fee_id: fee.id, + amount_cents: fee_credit_amount.truncate(CreditNote::DB_PRECISION_SCALE) + } + + remaining -= fee_credit_amount + end + + # it could be that we have some amount remaining + # TODO: verify and check in v2 + if remaining.positive? + @amount -= remaining + end + + items + end + end +end diff --git a/app/services/credits/progressive_billing_service.rb b/app/services/credits/progressive_billing_service.rb index e5b5a9f94fe..553d2a302d7 100644 --- a/app/services/credits/progressive_billing_service.rb +++ b/app/services/credits/progressive_billing_service.rb @@ -28,8 +28,10 @@ def call amount_to_credit = progressive_billing_invoice.fees_amount_cents if amount_to_credit > remaining_to_credit - # TODO: create credit note for (amount_to_credit - remaining_credit) - invoice.negative_amount_cents -= (amount_to_credit - remaining_to_credit) + CreditNotes::CreateFromProgressiveBillingInvoice.call( + progressive_billing_invoice:, amount: amount_to_credit - remaining_to_credit + ).raise_if_error! + amount_to_credit = remaining_to_credit end diff --git a/spec/services/invoices/calculate_fees_service_spec.rb b/spec/services/invoices/calculate_fees_service_spec.rb index 31e6a6bdab8..684ecdbf635 100644 --- a/spec/services/invoices/calculate_fees_service_spec.rb +++ b/spec/services/invoices/calculate_fees_service_spec.rb @@ -120,6 +120,34 @@ expect(Credits::ProgressiveBillingService).to have_received(:call).with(invoice:) end + context "when a progressive_billing invoice is present" do + let(:progressive_invoice) do + create(:invoice, + customer:, + status: 'finalized', + invoice_type: :progressive_billing, + subscriptions: [subscription], + fees_amount_cents: 50, + issuing_date: timestamp - 5.days) + end + + let(:progressive_fee) do + create(:charge_fee, amount_cents: 50, invoice: progressive_invoice) + end + + before do + progressive_invoice + progressive_fee + end + + it "creates a credit note for the amount that was billed too much" do + expect { invoice_service.call }.to change(CreditNote, :count).by(1) + + credit_note = progressive_invoice.reload.credit_notes.sole + expect(credit_note.credit_amount_cents).to eq(50) + end + end + context 'when charge is pay_in_advance, not recurring and invoiceable' do let(:charge) do create( From a63a3db5991234a82a7c083a92c522e8b804fc8c Mon Sep 17 00:00:00 2001 From: Toon Willems Date: Fri, 23 Aug 2024 18:34:52 +0200 Subject: [PATCH 2/5] don't require debug and update it to the latest version --- Gemfile | 2 +- Gemfile.lock | 20 ++++++++++++------- .../progressive_billing_service_spec.rb | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/Gemfile b/Gemfile index a8fd978add9..e28aeedbf28 100644 --- a/Gemfile +++ b/Gemfile @@ -95,7 +95,7 @@ end group :development, :test do gem 'byebug' gem 'clockwork-test' - gem 'debug', platforms: %i[mri mingw x64_mingw] + gem 'debug', platforms: %i[mri mingw x64_mingw], require: false gem 'dotenv' gem 'i18n-tasks', git: 'https://github.com/glebm/i18n-tasks.git' gem 'rspec-rails' diff --git a/Gemfile.lock b/Gemfile.lock index ed791046735..e39970b58a3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -179,9 +179,9 @@ GEM database_cleaner-core (~> 2.0.0) database_cleaner-core (2.0.1) date (3.3.4) - debug (1.6.3) - irb (>= 1.3.6) - reline (>= 0.3.1) + debug (1.9.2) + irb (~> 1.10) + reline (>= 0.3.8) declarative (0.0.20) diff-lcs (1.5.0) digest-crc (0.6.4) @@ -295,9 +295,10 @@ GEM httpclient (2.8.3) i18n (1.14.5) concurrent-ruby (~> 1.0) - io-console (0.5.11) - irb (1.4.1) - reline (>= 0.3.0) + io-console (0.7.2) + irb (1.14.0) + rdoc (>= 4.0.0) + reline (>= 0.4.2) jmespath (1.6.1) json (2.7.1) jwt (2.7.0) @@ -607,6 +608,8 @@ GEM pry (0.14.2) coderay (~> 1.1) method_source (~> 1.0) + psych (5.1.2) + stringio public_suffix (5.0.1) puma (6.4.2) nio4r (~> 2.0) @@ -662,6 +665,8 @@ GEM rb-fsevent (0.11.2) rb-inotify (0.10.1) ffi (~> 1.0) + rdoc (6.7.0) + psych (>= 4.0.0) redis (5.1.0) redis-client (>= 0.17.0) redis-client (0.17.0) @@ -669,7 +674,7 @@ GEM redlock (2.0.6) redis-client (>= 0.14.1, < 1.0.0) regexp_parser (2.9.0) - reline (0.3.1) + reline (0.5.9) io-console (~> 0.5) representable (3.2.0) declarative (< 0.1.0) @@ -826,6 +831,7 @@ GEM standard-performance (1.3.1) lint_roller (~> 1.1) rubocop-performance (~> 1.20.2) + stringio (3.1.1) stripe (6.5.0) strong_migrations (2.0.0) activerecord (>= 6.1) diff --git a/spec/services/credits/progressive_billing_service_spec.rb b/spec/services/credits/progressive_billing_service_spec.rb index 731213b87a2..dd2348a4ca6 100644 --- a/spec/services/credits/progressive_billing_service_spec.rb +++ b/spec/services/credits/progressive_billing_service_spec.rb @@ -185,7 +185,7 @@ expect(first_credit.amount_cents).to eq(980) expect(invoice.progressive_billing_credit_amount_cents).to eq(1000) - expect(invoice.negative_amount_cents).to eq(-220) + # todo: validate that we've created a credit note here end end end From 6a68113e002a5be86a6547051760340c6c62804e Mon Sep 17 00:00:00 2001 From: Toon Willems Date: Mon, 26 Aug 2024 10:16:24 +0200 Subject: [PATCH 3/5] Add specs for credits behaviour --- ...create_from_progressive_billing_invoice.rb | 5 +- ...e_from_progressive_billing_invoice_spec.rb | 79 +++++++++++++++++++ .../progressive_billing_service_spec.rb | 17 +++- spec/spec_helper.rb | 7 ++ 4 files changed, 104 insertions(+), 4 deletions(-) create mode 100644 spec/services/credit_notes/create_from_progressive_billing_invoice_spec.rb diff --git a/app/services/credit_notes/create_from_progressive_billing_invoice.rb b/app/services/credit_notes/create_from_progressive_billing_invoice.rb index ff985e57467..2ff08ee2d79 100644 --- a/app/services/credit_notes/create_from_progressive_billing_invoice.rb +++ b/app/services/credit_notes/create_from_progressive_billing_invoice.rb @@ -12,6 +12,7 @@ def initialize(progressive_billing_invoice:, amount:, reason: :other) def call return result unless amount.positive? + return result.forbidden_failure! unless progressive_billing_invoice.progressive_billing? # Important to call this method as it modifies @amount if needed items = calculate_items! @@ -34,7 +35,7 @@ def calculate_items! remaining = amount # The amount can be greater than a single fee amount. We'll keep on deducting until we've credited enough - progressive_billing_invoice.fees.each do |fee| + progressive_billing_invoice.fees.order(amount_cents: :desc).each do |fee| # no further credit remaining break if remaining.zero? @@ -49,7 +50,7 @@ def calculate_items! end # it could be that we have some amount remaining - # TODO: verify and check in v2 + # TODO(ProgressiveBilling): verify and check in v2 if remaining.positive? @amount -= remaining end diff --git a/spec/services/credit_notes/create_from_progressive_billing_invoice_spec.rb b/spec/services/credit_notes/create_from_progressive_billing_invoice_spec.rb new file mode 100644 index 00000000000..a5cf2465537 --- /dev/null +++ b/spec/services/credit_notes/create_from_progressive_billing_invoice_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe CreditNotes::CreateFromProgressiveBillingInvoice, type: :service do + subject(:credit_service) { described_class.new(progressive_billing_invoice:, amount:, reason:) } + + let(:reason) { :other } + let(:amount) { 0 } + let(:invoice_type) { :progressive_billing } + let(:customer) { create(:customer) } + let(:organization) { customer.organization } + + let(:progressive_billing_invoice) do + create( + :invoice, + customer:, + organization:, + currency: 'EUR', + fees_amount_cents: 120, + total_amount_cents: 120, + invoice_type: + ) + end + + let(:fee1) do + create( + :fee, + invoice: progressive_billing_invoice, + amount_cents: 80 + ) + end + + let(:fee2) do + create( + :fee, + invoice: progressive_billing_invoice, + amount_cents: 40 + ) + end + + before do + progressive_billing_invoice + fee1 + fee2 + end + + describe "#call" do + it "does nothing when amount is zero" do + expect { credit_service.call }.not_to change(CreditNote, :count) + end + + context "with amount greater than zero" do + let(:amount) { 100 } + + context 'when called with a subscription invoice' do + let(:invoice_type) { :subscription } + + it "fails when the passed in invoice is not a progressive billing invoice" do + result = credit_service.call + expect(result).not_to be_success + end + end + + it "creates a credit note for all required fees" do + result = credit_service.call + credit_note = result.credit_note + + expect(credit_note.credit_amount_cents).to eq(amount) + expect(credit_note.items.size).to eq(2) + + credit_fee1 = credit_note.items.find { |i| i.fee == fee1 } + expect(credit_fee1.amount_cents).to eq(80) + credit_fee2 = credit_note.items.find { |i| i.fee == fee2 } + expect(credit_fee2.amount_cents).to eq(20) + end + end + end +end diff --git a/spec/services/credits/progressive_billing_service_spec.rb b/spec/services/credits/progressive_billing_service_spec.rb index dd2348a4ca6..1ad45691dfa 100644 --- a/spec/services/credits/progressive_billing_service_spec.rb +++ b/spec/services/credits/progressive_billing_service_spec.rb @@ -175,7 +175,7 @@ end describe "#call" do - it "applies one credit to the invoice" do + it "applies all the credits to the invoice" do result = credit_service.call expect(result.credits.size).to eq(2) first_credit = result.credits.find { |credit| credit.progressive_billing_invoice == progressive_billing_invoice } @@ -185,7 +185,20 @@ expect(first_credit.amount_cents).to eq(980) expect(invoice.progressive_billing_credit_amount_cents).to eq(1000) - # todo: validate that we've created a credit note here + end + + it "creates credit notes for the remainder of the progressive billed invoices" do + expect { credit_service.call }.to change(CreditNote, :count).by(2) + # we were able to credit 1000 from the invoice, this means we've got 20 and 200 remaining respectively + aggregate_failures do + expect(progressive_billing_invoice2.credit_notes.size).to eq(1) + expect(progressive_billing_invoice3.credit_notes.size).to eq(1) + + first = progressive_billing_invoice2.credit_notes.sole + expect(first.credit_amount_cents).to eq(20) + last = progressive_billing_invoice3.credit_notes.sole + expect(last.credit_amount_cents).to eq(200) + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 51a5959a1e1..fb95ca558cc 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,6 +2,13 @@ require 'webmock/rspec' +# Allow remote debugging when RUBY_DEBUG_PORT is set +if ENV['RUBY_DEBUG_PORT'] + require 'debug/open_nonstop' +else + require 'debug' +end + RSpec.configure do |config| config.expect_with :rspec do |expectations| expectations.include_chain_clauses_in_custom_matcher_descriptions = true From 406533ef86b7e607ab29535e9379a2c25c97986a Mon Sep 17 00:00:00 2001 From: Toon Willems Date: Mon, 26 Aug 2024 14:00:17 +0200 Subject: [PATCH 4/5] Calculate taxes for credit note --- .../create_from_progressive_billing_invoice.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/app/services/credit_notes/create_from_progressive_billing_invoice.rb b/app/services/credit_notes/create_from_progressive_billing_invoice.rb index 2ff08ee2d79..fcc6dcd13dc 100644 --- a/app/services/credit_notes/create_from_progressive_billing_invoice.rb +++ b/app/services/credit_notes/create_from_progressive_billing_invoice.rb @@ -19,7 +19,7 @@ def call CreditNotes::CreateService.new( invoice: progressive_billing_invoice, - credit_amount_cents: amount, + credit_amount_cents: creditable_amount_cents(amount, items), items:, reason:, automatic: true @@ -57,5 +57,18 @@ def calculate_items! items end + + def creditable_amount_cents(amount, items) + taxes_result = CreditNotes::ApplyTaxesService.call( + invoice: progressive_billing_invoice, + items: items.map { |item| CreditNoteItem.new(fee_id: item[:fee_id], precise_amount_cents: item[:amount_cents]) } + ) + + ( + amount.truncate(CreditNote::DB_PRECISION_SCALE) - + taxes_result.coupons_adjustment_amount_cents + + taxes_result.taxes_amount_cents + ).round + end end end From 1d2f48443fd533146ca86fca9e5f485fc3bb6e50 Mon Sep 17 00:00:00 2001 From: Toon Willems Date: Mon, 26 Aug 2024 15:03:11 +0200 Subject: [PATCH 5/5] Add tax calculation to spec --- ...eate_from_progressive_billing_invoice_spec.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/spec/services/credit_notes/create_from_progressive_billing_invoice_spec.rb b/spec/services/credit_notes/create_from_progressive_billing_invoice_spec.rb index a5cf2465537..07de9883cd6 100644 --- a/spec/services/credit_notes/create_from_progressive_billing_invoice_spec.rb +++ b/spec/services/credit_notes/create_from_progressive_billing_invoice_spec.rb @@ -10,6 +10,7 @@ let(:invoice_type) { :progressive_billing } let(:customer) { create(:customer) } let(:organization) { customer.organization } + let(:tax) { create(:tax, organization:, rate: 20) } let(:progressive_billing_invoice) do create( @@ -27,7 +28,9 @@ create( :fee, invoice: progressive_billing_invoice, - amount_cents: 80 + amount_cents: 80, + taxes_amount_cents: 16, + taxes_rate: 20 ) end @@ -35,14 +38,21 @@ create( :fee, invoice: progressive_billing_invoice, - amount_cents: 40 + amount_cents: 40, + taxes_amount_cents: 8, + taxes_rate: 20 ) end + let(:fee1_applied_tax) { create(:fee_applied_tax, tax:, fee: fee1) } + let(:fee2_applied_tax) { create(:fee_applied_tax, tax:, fee: fee2) } + before do progressive_billing_invoice fee1 fee2 + fee1_applied_tax + fee2_applied_tax end describe "#call" do @@ -66,7 +76,7 @@ result = credit_service.call credit_note = result.credit_note - expect(credit_note.credit_amount_cents).to eq(amount) + expect(credit_note.credit_amount_cents).to eq(120) expect(credit_note.items.size).to eq(2) credit_fee1 = credit_note.items.find { |i| i.fee == fee1 }