Merge branch '388-warn-about-top-up-limits' of https://git.sr.ht/~amolith/sgx-jmp

Stephen Paul Weber created

* '388-warn-about-top-up-limits' of https://git.sr.ht/~amolith/sgx-jmp:
  feat(form): conditionally display max top-up amount
  test(credit-card-sale): add missing openstruct require
  style: wrap YARD comment lines
  refactor: remove duplicated logic from BuyAccountCreditForm
  refactor: set DeclinedError defaults to nil
  fix: raise and check for errors correctly
  refactor: protect max_declines
  fix: redo amount validation in the form
  docs: add Yard documentation to code we touched
  feat: add upper limit to top-ups based on trust level

Change summary

forms/top_up.rb                      |   5 +
lib/buy_account_credit_form.rb       |  47 ++++++++++--
lib/credit_card_sale.rb              |  74 +++++++++++++++++++-
lib/trust_level.rb                   | 108 ++++++++++++++++++++++++++---
sgx_jmp.rb                           |   5 +
test/test_buy_account_credit_form.rb |  40 ++++++++--
test/test_credit_card_sale.rb        |  76 +++++++++++++++++++++
7 files changed, 319 insertions(+), 36 deletions(-)

Detailed changes

forms/top_up.rb 🔗

@@ -1,6 +1,10 @@
 form!
 title "Buy Account Credit"
 
+if @max_top_up_amount & @max_top_up_amount.positive?
+	instructions "Max amount: $#{@max_top_up_amount}"
+end
+
 field(
 	type: "fixed",
 	label: "Current balance",
@@ -11,7 +15,6 @@ field(**@payment_methods.to_list_single)
 
 field(
 	datatype: "xs:decimal",
-	range: @range,
 	var: "amount",
 	label: "Amount of credit to buy",
 	prefix: "$",

lib/buy_account_credit_form.rb 🔗

@@ -1,35 +1,64 @@
 # frozen_string_literal: true
 
+require_relative "trust_level_repo"
+require_relative "credit_card_sale"
+
 class BuyAccountCreditForm
-	class AmountValidationError < StandardError
-		def initialize(amount)
-			super("amount #{amount} must be more than $15")
-		end
+	# Returns a TrustLevelRepo instance, allowing for dependency injection.
+	# Either creates a new instance given kwargs or returns the existing instance.
+	# @param kwargs [Hash] keyword arguments.
+	# @option kwargs [TrustLevelRepo] :trust_level_repo An existing TrustLevelRepo
+	# instance.
+	# @return [TrustLevelRepo] An instance of TrustLevelRepo.
+	def self.trust_level_repo(**kwargs)
+		kwargs[:trust_level_repo] || TrustLevelRepo.new(**kwargs)
 	end
 
+	# Factory method to create a BuyAccountCreditForm for a given customer.
+	# It fetches the customer's trust level to determine the maximum top-up
+	# amount.
+	# @param customer [Customer] The customer for whom the form is being created.
+	# @return [EMPromise<BuyAccountCreditForm>] A promise that resolves with the
+	# new form instance.
 	def self.for(customer)
-		customer.payment_methods.then do |payment_methods|
-			new(customer.balance, payment_methods)
+		trust_level_repo.find(customer).then do |trust_level|
+			customer.payment_methods.then do |payment_methods|
+				new(customer.balance, payment_methods, trust_level.max_top_up_amount)
+			end
 		end
 	end
 
-	def initialize(balance, payment_methods)
+	# Initializes a new BuyAccountCreditForm.
+	# @param balance [BigDecimal] The current balance of the customer.
+	# @param payment_methods [PaymentMethods] The available payment methods for
+	# the customer.
+	# @param max_top_up_amount [Numeric] The maximum amount the customer is
+	# allowed to top up, based on their trust level.
+	def initialize(balance, payment_methods, max_top_up_amount)
 		@balance = balance
 		@payment_methods = payment_methods
+		@max_top_up_amount = max_top_up_amount
 	end
 
+	# Generates the form template for topping up account credit.
+	# The form will include a range for the amount field, constrained by a minimum
+	# of $15
+	# and the customer's specific maximum top-up amount.
+	# @return [FormTemplate::OneRender] The rendered form template.
 	def form
 		FormTemplate.render(
 			:top_up,
 			balance: @balance,
 			payment_methods: @payment_methods,
-			range: [15, nil]
+			max_top_up_amount: @max_top_up_amount
 		)
 	end
 
 	def parse(form)
 		amount = form.field("amount")&.value&.to_s
-		raise AmountValidationError, amount unless amount.to_i >= 15
+		amount_value = amount.to_f
+
+		raise AmountTooLowError.new(amount_value, 15) if amount_value < 15
 
 		{
 			payment_method: @payment_methods.fetch(

lib/credit_card_sale.rb 🔗

@@ -6,6 +6,60 @@ require "delegate"
 require_relative "transaction"
 require_relative "trust_level_repo"
 
+class TransactionDeclinedError < StandardError; end
+
+# Error raised when a transaction amount exceeds the maximum allowed limit.
+class AmountTooHighError < TransactionDeclinedError
+	# @return [Numeric] The transaction amount that was too high.
+	attr_reader :amount
+	# @return [Numeric] The maximum amount allowed for the transaction.
+	attr_reader :max_amount
+
+	# Initializes a new AmountTooHighError.
+	# @param amount [Numeric] The transaction amount.
+	# @param max_amount [Numeric] The maximum allowed amount.
+	def initialize(amount, max_amount)
+		@amount = amount
+		@max_amount = max_amount
+		super("Amount $#{amount} exceeds maximum allowed amount of $#{max_amount}")
+	end
+end
+
+# Error raised when a transaction amount is below the minimum required limit.
+class AmountTooLowError < TransactionDeclinedError
+	# @return [Numeric] The transaction amount that was too low.
+	attr_reader :amount
+	# @return [Numeric] The minimum amount required for the transaction.
+	attr_reader :min_amount
+
+	# Initializes a new AmountTooLowError.
+	# @param amount [Numeric] The transaction amount.
+	# @param min_amount [Numeric] The minimum required amount.
+	def initialize(amount, min_amount)
+		@amount = amount
+		@min_amount = min_amount
+		super("Amount $#{amount} is below minimum amount of $#{min_amount}")
+	end
+end
+
+# Error raised when a transaction is declined, potentially due to exceeding
+# decline limits.
+class DeclinedError < TransactionDeclinedError
+	# @return [Integer, nil] The number of declines the customer has.
+	attr_reader :declines
+	# @return [Integer, nil] The maximum number of declines allowed.
+	attr_reader :max_declines
+
+	# Initializes a new DeclinedError.
+	# @param declines [Integer, nil] (nil) The current number of declines.
+	# @param max_declines [Integer, nil] (nil) The maximum allowed declines.
+	def initialize(declines=nil, max_declines=nil)
+		@declines = declines
+		@max_declines = max_declines
+		super("Transaction declined")
+	end
+end
+
 class CreditCardSale
 	def self.create(*args, transaction_class: Transaction, **kwargs)
 		new(*args, **kwargs).sale.then do |response|
@@ -57,15 +111,25 @@ class CreditCardSale
 
 protected
 
+	# Validates the transaction against customer locks, trust level, and decline
+	# history.
+	# @raise [TransactionDeclinedError] if the customer has made too many payments
+	# recently.
+	# @raise [AmountTooHighError] if the amount exceeds the trust level's maximum
+	# top-up amount.
+	# @raise [AmountTooLowError] if the amount is below any applicable minimum.
+	# @raise [DeclinedError] if the transaction is declined due to too many
+	# previous declines or other trust level restrictions.
+	# @return [EMPromise<void>] A promise that resolves if validation passes, or
+	# rejects with an error.
 	def validate!
 		EMPromise.all([
 			REDIS.exists("jmp_customer_credit_card_lock-#{@customer.customer_id}"),
 			@trust_repo.find(@customer), @customer.declines
 		]).then do |(lock, tl, declines)|
-			unless tl.credit_card_transaction?(@amount.to_d, declines)
-				raise "Declined"
-			end
-			raise "Too many payments recently" if lock == 1
+			raise TransactionDeclinedError, "Too many payments recently" if lock == 1
+
+			tl.validate_credit_card_transaction!(@amount.to_d, declines)
 		end
 	end
 
@@ -107,7 +171,7 @@ class BraintreeFailure < StandardError
 	attr_reader :response
 
 	def initialize(response)
-		super response.message
+		super(response.message)
 		@response = response
 	end
 end

lib/trust_level.rb 🔗

@@ -44,8 +44,14 @@ module TrustLevel
 			false
 		end
 
-		def credit_card_transaction?(*)
-			false
+		# Validates a credit card transaction for a Tomb trust level user.
+		# Users at this level cannot make credit card transactions.
+		# @param _amount [BigDecimal] The amount of the transaction (ignored).
+		# @param _declines [Integer] The number of recent declines (ignored).
+		# @raise [DeclinedError] Always raised to prevent transactions.
+		def validate_credit_card_transaction!(_amount, _declines)
+			# Give a more ambiguous error so they don't know they're tombed.
+			raise DeclinedError
 		end
 
 		def create_subaccount?(*)
@@ -55,6 +61,12 @@ module TrustLevel
 		def to_s
 			"Tomb"
 		end
+
+		# The maximum amount a user at Tomb trust level can top up.
+		# @return [Integer] Always 0 for Tomb level.
+		def max_top_up_amount
+			0
+		end
 	end
 
 	class Basement
@@ -74,8 +86,17 @@ module TrustLevel
 			messages_today < 40
 		end
 
-		def credit_card_transaction?(amount, declines)
-			amount <= 35 && declines <= 2
+		# Validates a credit card transaction for a Basement trust level user.
+		# @param amount [BigDecimal] The amount of the transaction.
+		# @param declines [Integer] The number of recent declines for the customer.
+		# @raise [DeclinedError] if the number of declines exceeds `max_declines`.
+		# @raise [AmountTooHighError] if the transaction amount exceeds
+		# `max_top_up_amount`.
+		def validate_credit_card_transaction!(amount, declines)
+			raise DeclinedError.new(declines, max_declines) if declines > max_declines
+			return unless amount > max_top_up_amount
+
+			raise AmountTooHighError.new(amount, max_top_up_amount)
 		end
 
 		def create_subaccount?(already_have)
@@ -85,6 +106,21 @@ module TrustLevel
 		def to_s
 			"Basement"
 		end
+
+		# The maximum amount a user at Basement trust level can top up.
+		# @return [Integer]
+		def max_top_up_amount
+			35
+		end
+
+	protected
+
+		# The maximum number of credit card declines allowed for a Basement user
+		# before further transactions are blocked.
+		# @return [Integer]
+		def max_declines
+			2
+		end
 	end
 
 	class Paragon
@@ -104,8 +140,17 @@ module TrustLevel
 			messages_today < 700
 		end
 
-		def credit_card_transaction?(amount, declines)
-			amount <= 500 && declines <= 3
+		# Validates a credit card transaction for a Paragon trust level user.
+		# @param amount [BigDecimal] The amount of the transaction.
+		# @param declines [Integer] The number of recent declines for the customer.
+		# @raise [DeclinedError] if the number of declines exceeds `max_declines`.
+		# @raise [AmountTooHighError] if the transaction amount exceeds
+		# `max_top_up_amount`.
+		def validate_credit_card_transaction!(amount, declines)
+			raise DeclinedError.new(declines, max_declines) if declines > max_declines
+			return unless amount > max_top_up_amount
+
+			raise AmountTooHighError.new(amount, max_top_up_amount)
 		end
 
 		def create_subaccount?(already_have)
@@ -115,6 +160,21 @@ module TrustLevel
 		def to_s
 			"Paragon"
 		end
+
+		# The maximum amount a user at Paragon trust level can top up.
+		# @return [Integer]
+		def max_top_up_amount
+			500
+		end
+
+	protected
+
+		# The maximum number of credit card declines allowed for a Paragon user
+		# before further transactions are blocked.
+		# @return [Integer]
+		def max_declines
+			3
+		end
 	end
 
 	class Olympias
@@ -134,9 +194,11 @@ module TrustLevel
 			true
 		end
 
-		def credit_card_transaction?(*)
-			true
-		end
+		# Validates a credit card transaction for an Olympias trust level user.
+		# Users at this level have no restrictions on credit card transactions
+		# through this method.
+		# @return [void]
+		def validate_credit_card_transaction!(*) end
 
 		def create_subaccount?(*)
 			true
@@ -179,8 +241,17 @@ module TrustLevel
 			messages_today < 500
 		end
 
-		def credit_card_transaction?(amount, declines)
-			amount <= 130 && declines <= 2
+		# Validates a credit card transaction for a Customer trust level user.
+		# @param amount [BigDecimal] The amount of the transaction.
+		# @param declines [Integer] The number of recent declines for the customer.
+		# @raise [DeclinedError] if the number of declines exceeds `max_declines`.
+		# @raise [AmountTooHighError] if the transaction amount exceeds
+		# `max_top_up_amount`.
+		def validate_credit_card_transaction!(amount, declines)
+			raise DeclinedError.new(declines, max_declines) if declines > max_declines
+			return unless amount > max_top_up_amount
+
+			raise AmountTooHighError.new(amount, max_top_up_amount)
 		end
 
 		def create_subaccount?(already_have)
@@ -190,5 +261,20 @@ module TrustLevel
 		def to_s
 			"Customer"
 		end
+
+		# The maximum amount a user at Customer trust level can top up.
+		# @return [Integer]
+		def max_top_up_amount
+			130
+		end
+
+	protected
+
+		# The maximum number of credit card declines allowed for a Customer user
+		# before further transactions are blocked.
+		# @return [Integer]
+		def max_declines
+			2
+		end
 	end
 end

sgx_jmp.rb 🔗

@@ -670,7 +670,10 @@ Command.new(
 		end
 	}.then { |transaction|
 		Command.finish("#{transaction} added to your account balance.")
-	}.catch_only(BuyAccountCreditForm::AmountValidationError) do |e|
+	}.catch_only(
+		BuyAccountCreditForm::AmountTooHighError,
+		BuyAccountCreditForm::AmountTooLowError
+	) do |e|
 		Command.finish(e.message, type: :error)
 	end
 }.register(self).then(&CommandList.method(:register))

test/test_buy_account_credit_form.rb 🔗

@@ -3,16 +3,21 @@
 require "test_helper"
 require "buy_account_credit_form"
 require "customer"
+require "credit_card_sale"
 
 CustomerFinancials::BRAINTREE = Minitest::Mock.new
 CustomerFinancials::REDIS = Minitest::Mock.new
+TrustLevelRepo::REDIS = Minitest::Mock.new
+TrustLevelRepo::DB = Minitest::Mock.new
 
 class BuyAccountCreditFormTest < Minitest::Test
 	def setup
 		@payment_method = OpenStruct.new(card_type: "Test", last_4: "1234")
+		@max_top_up_amount = 130
 		@form = BuyAccountCreditForm.new(
 			BigDecimal("15.1234"),
-			PaymentMethods.new([@payment_method])
+			PaymentMethods.new([@payment_method]),
+			@max_top_up_amount
 		)
 	end
 
@@ -26,10 +31,24 @@ class BuyAccountCreditFormTest < Minitest::Test
 			["test"]
 		)
 
+		TrustLevelRepo::REDIS.expect(
+			:get,
+			EMPromise.resolve("Customer"),
+			["jmp_customer_trust_level-test"]
+		)
+		TrustLevelRepo::DB.expect(
+			:query_one,
+			EMPromise.resolve({}),
+			[String, "test"], default: {}
+		)
+
 		assert_kind_of(
 			BuyAccountCreditForm,
 			BuyAccountCreditForm.for(customer).sync
 		)
+
+		assert_mock TrustLevelRepo::REDIS
+		assert_mock TrustLevelRepo::DB
 	end
 	em :test_for
 
@@ -67,14 +86,6 @@ class BuyAccountCreditFormTest < Minitest::Test
 		assert_equal "123", @form.parse(iq_form)[:amount]
 	end
 
-	def test_parse_bad_amount
-		iq_form = Blather::Stanza::X.new
-		iq_form.fields = [{ var: "amount", value: "1" }]
-		assert_raises(BuyAccountCreditForm::AmountValidationError) do
-			@form.parse(iq_form)[:amount]
-		end
-	end
-
 	def test_parse_payment_method
 		iq_form = Blather::Stanza::X.new
 		iq_form.fields = [
@@ -83,4 +94,15 @@ class BuyAccountCreditFormTest < Minitest::Test
 		]
 		assert_equal @payment_method, @form.parse(iq_form)[:payment_method]
 	end
+
+	def test_parse_amount_too_low
+		iq_form = Blather::Stanza::X.new
+		iq_form.fields = [
+			{ var: "payment_method", value: "0" },
+			{ var: "amount", value: "10" }
+		]
+		assert_raises(AmountTooLowError) do
+			@form.parse(iq_form)
+		end
+	end
 end

test/test_credit_card_sale.rb 🔗

@@ -4,11 +4,13 @@ require "test_helper"
 require "credit_card_sale"
 require "customer"
 require "transaction"
+require "ostruct"
 
 CreditCardSale::BRAINTREE = Minitest::Mock.new
 CreditCardSale::REDIS = Minitest::Mock.new
 TrustLevelRepo::REDIS = Minitest::Mock.new
 TrustLevelRepo::DB = Minitest::Mock.new
+CustomerFinancials::REDIS = Minitest::Mock.new
 
 class CreditCardSaleTest < Minitest::Test
 	FAKE_BRAINTREE_TRANSACTION =
@@ -111,6 +113,80 @@ class CreditCardSaleTest < Minitest::Test
 	end
 	em :test_sale_locked
 
+	def test_sale_amount_too_high
+		CreditCardSale::REDIS.expect(
+			:exists,
+			EMPromise.resolve(0),
+			["jmp_customer_credit_card_lock-test"]
+		)
+		TrustLevelRepo::REDIS.expect(
+			:get,
+			EMPromise.resolve("Customer"),
+			["jmp_customer_trust_level-test"]
+		)
+		TrustLevelRepo::DB.expect(
+			:query_one,
+			EMPromise.resolve({}),
+			[String, "test"], default: {}
+		)
+		CustomerFinancials::REDIS.expect(
+			:get,
+			EMPromise.resolve("0"),
+			["jmp_pay_decline-test"]
+		)
+
+		assert_raises(AmountTooHighError) do
+			CreditCardSale.new(
+				customer(plan_name: "test_usd"),
+				amount: 131,
+				payment_method: OpenStruct.new(token: "token")
+			).sale.sync
+		end
+
+		assert_mock CustomerFinancials::REDIS
+		assert_mock CreditCardSale::REDIS
+		assert_mock TrustLevelRepo::REDIS
+		assert_mock TrustLevelRepo::DB
+	end
+	em :test_sale_amount_too_high
+
+	def test_sale_too_many_declines
+		CreditCardSale::REDIS.expect(
+			:exists,
+			EMPromise.resolve(0),
+			["jmp_customer_credit_card_lock-test"]
+		)
+		TrustLevelRepo::REDIS.expect(
+			:get,
+			EMPromise.resolve("Customer"),
+			["jmp_customer_trust_level-test"]
+		)
+		TrustLevelRepo::DB.expect(
+			:query_one,
+			EMPromise.resolve({}),
+			[String, "test"], default: {}
+		)
+		CustomerFinancials::REDIS.expect(
+			:get,
+			EMPromise.resolve("3"),
+			["jmp_pay_decline-test"]
+		)
+
+		assert_raises(DeclinedError) do
+			CreditCardSale.new(
+				customer(plan_name: "test_usd"),
+				amount: 50,
+				payment_method: OpenStruct.new(token: "token")
+			).sale.sync
+		end
+
+		assert_mock CustomerFinancials::REDIS
+		assert_mock CreditCardSale::REDIS
+		assert_mock TrustLevelRepo::REDIS
+		assert_mock TrustLevelRepo::DB
+	end
+	em :test_sale_too_many_declines
+
 	def test_sale
 		req = stub_request(
 			:post,