From 34c91613f7462f0929e660f0969fe9ab9fa5e448 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Fri, 27 Feb 2026 14:30:11 +1100 Subject: [PATCH] Customer account transaction, simplify balance calculation Lock the customer to ensure the balance calculation is correct. Much simpler than locking the first transaction. --- app/models/customer_account_transaction.rb | 39 +++++-------------- config/locales/en.yml | 2 - .../customer_account_transaction_spec.rb | 28 ------------- .../data_loader_service_spec.rb | 3 -- 4 files changed, 9 insertions(+), 63 deletions(-) diff --git a/app/models/customer_account_transaction.rb b/app/models/customer_account_transaction.rb index 97e25c0ba0..2ef97241ee 100644 --- a/app/models/customer_account_transaction.rb +++ b/app/models/customer_account_transaction.rb @@ -25,37 +25,16 @@ class CustomerAccountTransaction < ApplicationRecord end def update_balance - # We are creating the initial transaction, no need to calculate the balance - return if initial_transaction? - - first_transaction = CustomerAccountTransaction.where(customer: customer).first - if first_transaction.nil? - first_transaction = create_initial_transaction - end - - # The first transaction will always exists, so we lock it to ensure only one transaction - # is processed at the time to ensure the correct balance calculation. - first_transaction.with_lock("FOR UPDATE") do + # Locking the customer to prevent two transactions from behing created at the same time + # resulting in a potentially wrong balance calculation. + customer.with_lock(requires_new: true) do last_transaction = CustomerAccountTransaction.where(customer: customer).last - self.balance = last_transaction.balance + amount + + self.balance = if last_transaction.present? + last_transaction.balance + amount + else + amount + end end end - - # Creates the first transaction with a 0 amount - def create_initial_transaction - api_payment_method = customer.enterprise.payment_methods.internal.find_by!( - name: Rails.application.config.api_payment_method[:name] - ) - CustomerAccountTransaction.create!( - customer: customer, - amount: 0.00, - currency: CurrentConfig.get(:currency), - description: I18n.t("customer_account_transaction.account_creation"), - payment_method: api_payment_method - ) - end - - def initial_transaction? - description == I18n.t("customer_account_transaction.account_creation") && amount == 0.00 - end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 3dfbe85d2e..fa6a7d84a9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -5210,8 +5210,6 @@ en: credit_payment_method_missing: Credit payment method is missing no_credit_available: No credit available not_enough_credit_available: Not enough credit available - customer_account_transaction: - account_creation: Account creation orders: customer_credit_service: no_credit_owed: No credit owed diff --git a/spec/models/customer_account_transaction_spec.rb b/spec/models/customer_account_transaction_spec.rb index 0b5dc3a827..2829d8fbec 100644 --- a/spec/models/customer_account_transaction_spec.rb +++ b/spec/models/customer_account_transaction_spec.rb @@ -38,15 +38,6 @@ RSpec.describe CustomerAccountTransaction do end context "when no existing balance" do - it "creates an 'account creation' transaction" do - customer = create(:customer) - transaction = create(:customer_account_transaction, amount: 12.00, customer:) - - first_transaction = CustomerAccountTransaction.where(customer: customer).first - expect(first_transaction.amount).to eq(0.00) - expect(first_transaction.description).to eq("Account creation") - end - it "set the balance to the new transaction's amount" do transaction = create(:customer_account_transaction, amount: 12.00) @@ -54,24 +45,5 @@ RSpec.describe CustomerAccountTransaction do expect(res.balance).to eq(12.00) end end - - context "when the default payment method is missing" do - around do |example| - # A Customer account transaction is linked to a customer which is linked to an enterprise. - # That means FactoryBot will create an enterprise, so we disable the after create callback - # so that credit payment are not created. - Enterprise.skip_callback(:create, :after, :add_credit_payment_method) - example.run - Enterprise.set_callback(:create, :after, :add_credit_payment_method) - end - - it "raises an error" do - expect do - create( - :customer_account_transaction, amount: 12.00, payment_method: create(:payment_method) - ) - end.to raise_error(ActiveRecord::RecordNotFound) - end - end end end diff --git a/spec/services/customer_account_transactions/data_loader_service_spec.rb b/spec/services/customer_account_transactions/data_loader_service_spec.rb index 6b574ed2ae..753ddb2ccf 100644 --- a/spec/services/customer_account_transactions/data_loader_service_spec.rb +++ b/spec/services/customer_account_transactions/data_loader_service_spec.rb @@ -13,14 +13,11 @@ RSpec.describe CustomerAccountTransactions::DataLoaderService do customer = create(:customer, email: user.email, enterprise:) user.customers << customer customer_account_transactions = create_list(:customer_account_transaction, 3, customer:) - # This initial transaction created automatically by CustomerAccountTransaction - first_transaction = CustomerAccountTransaction.where(customer: customer).first expect(subject.customer_account_transactions).to eq([ customer_account_transactions.third, customer_account_transactions.second, customer_account_transactions.first, - first_transaction ]) end