Fix linter, integrate patch feedback

Stephen Paul Weber created

Change summary

lib/customer.rb          | 13 +------------
lib/low_balance.rb       | 39 ++++++++++++++++-----------------------
test/test_low_balance.rb | 23 +++++++++++------------
3 files changed, 28 insertions(+), 47 deletions(-)

Detailed changes

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

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
 

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