From 1a2640d6f9f1f1ea8cbbfc0c7b4c05e57b3b242e Mon Sep 17 00:00:00 2001 From: Osakpolor Obaseki Date: Tue, 3 Jan 2023 23:51:49 +0100 Subject: [PATCH 1/2] Add low balance/auto top up with target amount --- lib/customer.rb | 6 ++-- lib/low_balance.rb | 52 +++++++++++++++++++++-------- test/test_low_balance.rb | 71 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 15 deletions(-) diff --git a/lib/customer.rb b/lib/customer.rb index 05f3e22eb0e8c323fd9a3cec0d3eb7d49810a60b..bec541565c4c3d00a8ce17fa65500dba5224083b 100644 --- a/lib/customer.rb +++ b/lib/customer.rb @@ -33,6 +33,8 @@ class Customer :add_btc_address, :declines, :mark_decline, :transactions + TOP_UP_ADJUSTEMENT_THRESHOLD = 5 + def self.extract(customer_id, jid, **kwargs) klass, *keys = if kwargs[:parent_customer_id] [ChildCustomer, :parent_customer_id] @@ -87,8 +89,8 @@ class Customer def auto_top_up_amount if @plan.auto_top_up_amount.positive? && - balance < -@plan.auto_top_up_amount + 5 - -balance + @plan.auto_top_up_amount + balance + @plan.auto_top_up_amount < TOP_UP_ADJUSTEMENT_THRESHOLD + -balance + @plan.auto_top_up_amount # amount needed to set balance to @plan.auto_top_up_amount else @plan.auto_top_up_amount end diff --git a/lib/low_balance.rb b/lib/low_balance.rb index dd4cdefde60983c2c776fbd00a8e5028a929d4a6..1c1eee68630e8a941deeea14f84dd7b639b97234 100644 --- a/lib/low_balance.rb +++ b/lib/low_balance.rb @@ -4,30 +4,33 @@ require_relative "expiring_lock" require_relative "transaction" class LowBalance - def self.for(customer) + def self.for(customer, transaction_amount=0) return Locked.new unless customer.registered? ExpiringLock.new( "jmp_customer_low_balance-#{customer.billing_customer_id}", expiry: 60 * 60 * 24 * 7 ).with(-> { Locked.new }) do - customer.billing_customer.then(&method(:for_no_lock)) + customer.billing_customer.then do |customer| + self.for_no_lock(customer, transaction_amount) + end end end - def self.for_no_lock(customer, auto: true) - if auto && customer.auto_top_up_amount.positive? - AutoTopUp.for(customer) + def self.for_no_lock(customer, transaction_amount=0, auto: true) + if auto && (customer.auto_top_up_amount.positive? || transaction_amount.positive?) + AutoTopUp.for(customer, transaction_amount) else customer.btc_addresses.then do |btc_addresses| - new(customer, btc_addresses) + new(customer, btc_addresses, transaction_amount) end end end - def initialize(customer, btc_addresses) + def initialize(customer, btc_addresses, transaction_amount=0) @customer = customer @btc_addresses = btc_addresses + @transaction_amount = transaction_amount end def notify! @@ -35,11 +38,20 @@ class LowBalance m.from = CONFIG[:notify_from] m.body = "Your balance of $#{'%.4f' % @customer.balance} is low." \ + "#{pending_cost_for_notification}" \ "#{btc_addresses_for_notification}" @customer.stanza_to(m) EMPromise.resolve(0) end + def pending_cost_for_notification + return unless (@transaction_amount.positive? && @transaction_amount > @customer.balance) + + "\nYou tried to perform an activity that cost #{@transaction_amount}"\ + "You need an additional #{'%.4f' % (@transaction_amount - @customer.balance)} "\ + "to perform this activity." + end + def btc_addresses_for_notification return if @btc_addresses.empty? @@ -48,13 +60,13 @@ class LowBalance end class AutoTopUp - def self.for(customer) + def self.for(customer, target=0) customer.payment_methods.then(&:default_payment_method).then do |method| blocked?(method).then do |block| - next AutoTopUp.new(customer, method) if block.zero? + next AutoTopUp.new(customer, method, target) if block.zero? log.info("#{customer.customer_id} auto top up blocked") - LowBalance.for_no_lock(customer, auto: false) + LowBalance.for_no_lock(customer, target, auto: false) end end end @@ -67,17 +79,31 @@ class LowBalance ) end - def initialize(customer, method=nil) + + + def initialize(customer, method=nil, target=0, margin=10) @customer = customer @method = method + @target = target + @margin = margin @message = Blather::Stanza::Message.new @message.from = CONFIG[:notify_from] end + def top_up_amount() + expected_balance = @customer.balance + @customer.auto_top_up_amount # @cutomer.auto_top_up is the adjusted auto_to_up + if expected_balance < @target + deficit = @target - expected_balance + @customer.auto_top_up_amount + deficit + @margin + else + @customer.auto_top_up_amount + end + end + def sale Transaction.sale( @customer, - amount: @customer.auto_top_up_amount + amount: top_up_amount() ).then do |tx| tx.insert.then { tx } end @@ -91,7 +117,7 @@ class LowBalance ) @message.body = "Automatic top-up transaction for " \ - "$#{@customer.auto_top_up_amount} failed: #{e.message}" + "$#{top_up_amount()} failed: #{e.message}" 0 end diff --git a/test/test_low_balance.rb b/test/test_low_balance.rb index f31380635ccf0f6fa6199f06b413cfb0e12e2fb9..e6586518d4f0f82def8fc41837b63359b00e6815 100644 --- a/test/test_low_balance.rb +++ b/test/test_low_balance.rb @@ -38,6 +38,44 @@ class LowBalanceTest < Minitest::Test end em :test_for_no_auto_top_up + def test_for_auto_top_up_on_transaction_amount + ExpiringLock::REDIS.expect( + :set, + EMPromise.resolve("OK"), + ["jmp_customer_low_balance-test", Time, "EX", 604800, "NX"] + ) + CustomerFinancials::REDIS.expect( + :smembers, + EMPromise.resolve([]), + ["block_credit_cards"] + ) + LowBalance::AutoTopUp::REDIS.expect( + :exists, + 0, + ["jmp_auto_top_up_block-abcd"] + ) + braintree_customer = Minitest::Mock.new + CustomerFinancials::BRAINTREE.expect(:customer, braintree_customer) + payment_methods = OpenStruct.new(payment_methods: [ + OpenStruct.new(default?: true, unique_number_identifier: "abcd") + ]) + braintree_customer.expect( + :find, + EMPromise.resolve(payment_methods), + ["test"] + ) + assert_kind_of( + LowBalance::AutoTopUp, + LowBalance.for(customer(auto_top_up_amount: 0), transaction_amount = 15).sync + ) + assert_mock ExpiringLock::REDIS + assert_mock CustomerFinancials::REDIS + assert_mock CustomerFinancials::BRAINTREE + assert_mock braintree_customer + end + em :test_for_auto_top_up_on_transaction_amount + + def test_for_auto_top_up ExpiringLock::REDIS.expect( :set, @@ -138,6 +176,39 @@ class LowBalanceTest < Minitest::Test end em :test_notify! + def test_top_up_amount_when_target_greater_than_expected_balance + customer = Minitest::Mock.new(customer( + balance: 10, + auto_top_up_amount: 15 + )) + auto_top_up = LowBalance::AutoTopUp.new(customer, nil, 30, 5) + + assert_equal 25, auto_top_up.top_up_amount() + end + em :test_top_up_amount_when_target_greater_than_expected_balance + + def test_top_up_amount_when_target_less_than_expected_balance + customer = Minitest::Mock.new(customer( + balance: 10, + auto_top_up_amount: 15 + )) + auto_top_up = LowBalance::AutoTopUp.new(customer, nil, 12, 5) + + assert_equal 15, auto_top_up.top_up_amount() + end + em :test_top_up_amount_when_target_less_than_expected_balance + + def test_top_up_amount_when_balance_is_negative_and_target_less_than_expected_balance + customer = Minitest::Mock.new(customer( + balance: -11, + auto_top_up_amount: 15 + )) + auto_top_up = LowBalance::AutoTopUp.new(customer, nil, 35, 5) + + assert_equal 51, auto_top_up.top_up_amount() + end + em :test_top_up_amount_when_balance_is_negative_and_target_less_than_expected_balance + def test_very_low_balance_notify! customer = Minitest::Mock.new(customer( balance: -100, From cc8787a765b4bd29eecefca16702c5c779fd25c2 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Tue, 3 Jan 2023 22:22:31 -0500 Subject: [PATCH 2/2] Fix linter, integrate patch feedback --- lib/customer.rb | 13 +------------ lib/low_balance.rb | 39 ++++++++++++++++----------------------- test/test_low_balance.rb | 23 +++++++++++------------ 3 files changed, 28 insertions(+), 47 deletions(-) diff --git a/lib/customer.rb b/lib/customer.rb index bec541565c4c3d00a8ce17fa65500dba5224083b..83ef25007cf63d6abecdc8367840a8330b4e5f8d 100644 --- a/lib/customer.rb +++ b/lib/customer.rb @@ -24,7 +24,7 @@ class Customer def_delegators :@plan, :active?, :activate_plan_starting_now, :bill_plan, :currency, :merchant_account, :plan_name, :minute_limit, :message_limit, :monthly_overage_limit, :activation_date, - :expires_at, :monthly_price, :save_plan! + :expires_at, :monthly_price, :save_plan!, :auto_top_up_amount def_delegators :@sgx, :deregister!, :register!, :registered?, :set_ogm_url, :fwd, :transcription_enabled def_delegators :@usage, :usage_report, :message_usage, :incr_message_usage, @@ -33,8 +33,6 @@ class Customer :add_btc_address, :declines, :mark_decline, :transactions - TOP_UP_ADJUSTEMENT_THRESHOLD = 5 - def self.extract(customer_id, jid, **kwargs) klass, *keys = if kwargs[:parent_customer_id] [ChildCustomer, :parent_customer_id] @@ -87,15 +85,6 @@ class Customer EMPromise.resolve(self) end - def auto_top_up_amount - if @plan.auto_top_up_amount.positive? && - balance + @plan.auto_top_up_amount < TOP_UP_ADJUSTEMENT_THRESHOLD - -balance + @plan.auto_top_up_amount # amount needed to set balance to @plan.auto_top_up_amount - else - @plan.auto_top_up_amount - end - end - def unused_invites InvitesRepo.new(DB).unused_invites(customer_id) end diff --git a/lib/low_balance.rb b/lib/low_balance.rb index 1c1eee68630e8a941deeea14f84dd7b639b97234..d237cfe66680def6ad93ab929ddcf62d55e9a141 100644 --- a/lib/low_balance.rb +++ b/lib/low_balance.rb @@ -11,14 +11,14 @@ class LowBalance "jmp_customer_low_balance-#{customer.billing_customer_id}", expiry: 60 * 60 * 24 * 7 ).with(-> { Locked.new }) do - customer.billing_customer.then do |customer| - self.for_no_lock(customer, transaction_amount) + customer.billing_customer.then do |billing_customer| + for_no_lock(billing_customer, transaction_amount) end end end - def self.for_no_lock(customer, transaction_amount=0, auto: true) - if auto && (customer.auto_top_up_amount.positive? || transaction_amount.positive?) + def self.for_no_lock(customer, transaction_amount, auto: true) + if auto && customer.auto_top_up_amount.positive? AutoTopUp.for(customer, transaction_amount) else customer.btc_addresses.then do |btc_addresses| @@ -45,11 +45,12 @@ class LowBalance end def pending_cost_for_notification - return unless (@transaction_amount.positive? && @transaction_amount > @customer.balance) + return unless @transaction_amount&.positive? + return unless @transaction_amount > @customer.balance - "\nYou tried to perform an activity that cost #{@transaction_amount}"\ - "You need an additional #{'%.4f' % (@transaction_amount - @customer.balance)} "\ - "to perform this activity." + "You need an additional " \ + "$#{'%.2f' % (@transaction_amount - @customer.balance)} "\ + "to complete this transaction." end def btc_addresses_for_notification @@ -79,9 +80,7 @@ class LowBalance ) end - - - def initialize(customer, method=nil, target=0, margin=10) + def initialize(customer, method=nil, target=0, margin: 10) @customer = customer @method = method @target = target @@ -90,21 +89,15 @@ class LowBalance @message.from = CONFIG[:notify_from] end - def top_up_amount() - expected_balance = @customer.balance + @customer.auto_top_up_amount # @cutomer.auto_top_up is the adjusted auto_to_up - if expected_balance < @target - deficit = @target - expected_balance - @customer.auto_top_up_amount + deficit + @margin - else + def top_up_amount + [ + (@target + @margin) - @customer.balance, @customer.auto_top_up_amount - end + ].max end def sale - Transaction.sale( - @customer, - amount: top_up_amount() - ).then do |tx| + Transaction.sale(@customer, amount: top_up_amount).then do |tx| tx.insert.then { tx } end end @@ -117,7 +110,7 @@ class LowBalance ) @message.body = "Automatic top-up transaction for " \ - "$#{top_up_amount()} failed: #{e.message}" + "$#{top_up_amount} failed: #{e.message}" 0 end diff --git a/test/test_low_balance.rb b/test/test_low_balance.rb index e6586518d4f0f82def8fc41837b63359b00e6815..7b1a8792f5ddae5999eecdd7746e142d7e9a9214 100644 --- a/test/test_low_balance.rb +++ b/test/test_low_balance.rb @@ -66,7 +66,7 @@ class LowBalanceTest < Minitest::Test ) assert_kind_of( LowBalance::AutoTopUp, - LowBalance.for(customer(auto_top_up_amount: 0), transaction_amount = 15).sync + LowBalance.for(customer(auto_top_up_amount: 1), 15).sync ) assert_mock ExpiringLock::REDIS assert_mock CustomerFinancials::REDIS @@ -75,7 +75,6 @@ class LowBalanceTest < Minitest::Test end em :test_for_auto_top_up_on_transaction_amount - def test_for_auto_top_up ExpiringLock::REDIS.expect( :set, @@ -181,9 +180,9 @@ class LowBalanceTest < Minitest::Test balance: 10, auto_top_up_amount: 15 )) - auto_top_up = LowBalance::AutoTopUp.new(customer, nil, 30, 5) + auto_top_up = LowBalance::AutoTopUp.new(customer, nil, 30, margin: 5) - assert_equal 25, auto_top_up.top_up_amount() + assert_equal 25, auto_top_up.top_up_amount end em :test_top_up_amount_when_target_greater_than_expected_balance @@ -192,22 +191,22 @@ class LowBalanceTest < Minitest::Test balance: 10, auto_top_up_amount: 15 )) - auto_top_up = LowBalance::AutoTopUp.new(customer, nil, 12, 5) + auto_top_up = LowBalance::AutoTopUp.new(customer, nil, 12, margin: 5) - assert_equal 15, auto_top_up.top_up_amount() + assert_equal 15, auto_top_up.top_up_amount end em :test_top_up_amount_when_target_less_than_expected_balance - def test_top_up_amount_when_balance_is_negative_and_target_less_than_expected_balance + def test_negative_balance_target_less_than_expected_balance customer = Minitest::Mock.new(customer( balance: -11, auto_top_up_amount: 15 )) - auto_top_up = LowBalance::AutoTopUp.new(customer, nil, 35, 5) + auto_top_up = LowBalance::AutoTopUp.new(customer, nil, 35, margin: 5) - assert_equal 51, auto_top_up.top_up_amount() + assert_equal 51, auto_top_up.top_up_amount end - em :test_top_up_amount_when_balance_is_negative_and_target_less_than_expected_balance + em :test_negative_balance_target_less_than_expected_balance def test_very_low_balance_notify! customer = Minitest::Mock.new(customer( @@ -221,7 +220,7 @@ class LowBalanceTest < Minitest::Test LowBalance::AutoTopUp::Transaction.expect( :sale, tx, - [customer], amount: 115 + [customer], amount: 110 ) auto_top_up.notify! assert_mock tx @@ -240,7 +239,7 @@ class LowBalanceTest < Minitest::Test LowBalance::AutoTopUp::Transaction.expect( :sale, tx, - [customer], amount: 26 + [customer], amount: 21 ) auto_top_up.notify! assert_mock tx