Split CreditCardSale from Transaction

Christopher Vollick created

This is a refactor that involves pulling the Credit Card stuff (meaning
braintree) out of the Transaction stuff. This makes Transaction a more
generic implementation of our Transaction table.

This commit should maintain the status quo, though. The places that used
to call Transaction.sale now call CreditCardSale.create, and that got a
little easier because we now do the `.insert` inside the create, because
previously all the callsites just got the transaction out and then
inserted anyway.

So they got a little bit simpler, but the main value of this is that now
we can insert other kinds of transactions and not just credit card
transactions!

Change summary

lib/credit_card_sale.rb       |  96 +++++++++++++++++++++
lib/low_balance.rb            |   4 
lib/registration.rb           |  10 -
lib/transaction.rb            |  72 ++-------------
sgx_jmp.rb                    |   6 
test/test_credit_card_sale.rb | 165 +++++++++++++++++++++++-------------
test/test_helper.rb           |   4 
test/test_low_balance.rb      |  36 +++----
test/test_registration.rb     |  22 +---
test/test_web.rb              |  10 +-
10 files changed, 253 insertions(+), 172 deletions(-)

Detailed changes

lib/credit_card_sale.rb 🔗

@@ -0,0 +1,96 @@
+# frozen_string_literal: true
+
+require "bigdecimal/util"
+require "delegate"
+
+require_relative "transaction"
+require_relative "trust_level_repo"
+
+class CreditCardSale
+	def self.create(*args, transaction_class: Transaction, **kwargs)
+		new(*args, **kwargs).sale.then do |response|
+			tx = BraintreeTransaction.build(
+				response,
+				transaction_class: transaction_class
+			)
+			tx.insert.then { tx }
+		end
+	end
+
+	class BraintreeTransaction < SimpleDelegator
+		def self.build(braintree_transaction, transaction_class: Transaction)
+			new(braintree_transaction).to_transaction(transaction_class)
+		end
+
+		def to_transaction(transaction_class)
+			transaction_class.new(
+				customer_id: customer_details.id,
+				transaction_id: id,
+				created_at: created_at,
+				settled_after: created_at + (90 * 24 * 60 * 60),
+				amount: amount,
+				note: "Credit card payment"
+			)
+		end
+	end
+
+	def initialize(
+		customer, amount:, payment_method: nil,
+		trust_repo: TrustLevelRepo.new
+	)
+		@customer = customer
+		@amount = amount
+		@payment_method = payment_method
+		@trust_repo = trust_repo
+	end
+
+	def sale
+		EMPromise.all([validate!, resolve_payment_method]).then { |_, selected|
+			BRAINTREE.transaction.sale(
+				amount: @amount,
+				merchant_account_id: @customer.merchant_account,
+				options: { submit_for_settlement: true },
+				payment_method_token: selected.token
+			)
+		}.then { |response| decline_guard(response) }
+	end
+
+protected
+
+	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
+		end
+	end
+
+	def resolve_payment_method
+		EMPromise.all([
+			@payment_method ||
+				@customer.payment_methods.then(&:default_payment_method)
+		]).then do |(selected_method)|
+			raise "No valid payment method on file" unless selected_method
+
+			selected_method
+		end
+	end
+
+	def decline_guard(response)
+		if response.success?
+			REDIS.setex(
+				"jmp_customer_credit_card_lock-#{@customer.customer_id}",
+				60 * 60 * 24,
+				"1"
+			)
+			return response.transaction
+		end
+
+		@customer.mark_decline
+		raise response.message
+	end
+end

lib/low_balance.rb 🔗

@@ -97,9 +97,7 @@ class LowBalance
 		end
 
 		def sale
-			Transaction.sale(@customer, amount: top_up_amount).then do |tx|
-				tx.insert.then { tx }
-			end
+			CreditCardSale.create(@customer, amount: top_up_amount)
 		end
 
 		def failed(e)

lib/registration.rb 🔗

@@ -275,22 +275,20 @@ class Registration
 				end
 
 				def write
-					Transaction.sale(
+					CreditCardSale.create(
 						@customer,
 						amount: CONFIG[:activation_amount],
 						payment_method: @payment_method
 					).then(
-						method(:sold),
+						->(_) { sold },
 						->(_) { declined }
 					)
 				end
 
 			protected
 
-				def sold(tx)
-					tx.insert.then do
-						BillPlan.new(@customer, @tel, finish: @finish).write
-					end
+				def sold
+					BillPlan.new(@customer, @tel, finish: @finish).write
 				end
 
 				DECLINE_MESSAGE =

lib/transaction.rb 🔗

@@ -1,59 +1,17 @@
 # frozen_string_literal: true
 
 require "bigdecimal"
-require "bigdecimal/util"
-
-require_relative "trust_level_repo"
+require "time"
+require "value_semantics/monkey_patched"
 
 class Transaction
-	def self.sale(customer, amount:, payment_method: nil)
-		resolve_payment_method(customer, payment_method, amount).then do |selected|
-			BRAINTREE.transaction.sale(
-				amount: amount,
-				merchant_account_id: customer.merchant_account,
-				options: { submit_for_settlement: true },
-				payment_method_token: selected.token
-			).then do |response|
-				new(decline_guard(customer, response))
-			end
-		end
-	end
-
-	def self.resolve_payment_method(customer, payment_method, amount)
-		EMPromise.all([
-			REDIS.exists("jmp_customer_credit_card_lock-#{customer.customer_id}"),
-			TrustLevelRepo.new.find(customer), customer.declines,
-			payment_method || customer.payment_methods.then(&:default_payment_method)
-		]).then do |(lock, tl, declines, selected_method)|
-			raise "Declined" unless tl.credit_card_transaction?(amount.to_d, declines)
-			raise "Too many payments recently" if lock == 1
-			raise "No valid payment method on file" unless selected_method
-
-			selected_method
-		end
-	end
-
-	def self.decline_guard(customer, response)
-		if response.success?
-			REDIS.setex(
-				"jmp_customer_credit_card_lock-#{customer.customer_id}",
-				60 * 60 * 24,
-				"1"
-			)
-			return response.transaction
-		end
-
-		customer.mark_decline
-		raise response.message
-	end
-
-	attr_reader :amount
-
-	def initialize(braintree_transaction)
-		@customer_id = braintree_transaction.customer_details.id
-		@transaction_id = braintree_transaction.id
-		@created_at = braintree_transaction.created_at
-		@amount = BigDecimal(braintree_transaction.amount, 4)
+	value_semantics do
+		customer_id String
+		transaction_id String
+		created_at Time, coerce: ->(x) { Time.parse(x.to_s) }
+		settled_after Time, coerce: ->(x) { Time.parse(x.to_s) }
+		amount BigDecimal, coerce: ->(x) { BigDecimal(x, 4) }
+		note String
 	end
 
 	def insert
@@ -88,21 +46,17 @@ class Transaction
 		"$#{'%.2f' % amount}#{plus if bonus.positive?}"
 	end
 
-	def settled_after
-		@created_at + (90 * 24 * 60 * 60)
-	end
-
 protected
 
 	def insert_tx
 		params = [
-			@customer_id, @transaction_id, @created_at, settled_after, @amount
+			@customer_id, @transaction_id, @created_at, @settled_after, @amount, @note
 		]
 		DB.exec(<<~SQL, params)
 			INSERT INTO transactions
 				(customer_id, transaction_id, created_at, settled_after, amount, note)
 			VALUES
-				($1, $2, $3, $4, $5, 'Credit card payment')
+				($1, $2, $3, $4, $5, $6)
 		SQL
 	end
 
@@ -111,13 +65,13 @@ protected
 
 		params = [
 			@customer_id, "bonus_for_#{@transaction_id}", @created_at,
-			settled_after, bonus
+			@settled_after, bonus, "#{@note} bonus"
 		]
 		DB.exec(<<~SQL, params)
 			INSERT INTO transactions
 				(customer_id, transaction_id, created_at, settled_after, amount, note)
 			VALUES
-				($1, $2, $3, $4, $5, 'Credit card payment bonus')
+				($1, $2, $3, $4, $5, $6)
 		SQL
 	end
 end

sgx_jmp.rb 🔗

@@ -600,13 +600,11 @@ Command.new(
 				reply.allowed_actions = [:complete]
 				credit_form.add_to_form(reply.form)
 			}.then do |iq|
-				Transaction.sale(customer, **credit_form.parse(iq.form))
+				CreditCardSale.create(customer, **credit_form.parse(iq.form))
 			end
 		end
 	}.then { |transaction|
-		transaction.insert.then do
-			Command.finish("#{transaction} added to your account balance.")
-		end
+		Command.finish("#{transaction} added to your account balance.")
 	}.catch_only(BuyAccountCreditForm::AmountValidationError) do |e|
 		Command.finish(e.message, type: :error)
 	end

test/test_transaction.rb → test/test_credit_card_sale.rb 🔗

@@ -1,16 +1,16 @@
 # frozen_string_literal: true
 
 require "test_helper"
+require "credit_card_sale"
 require "customer"
 require "transaction"
 
-Transaction::DB = Minitest::Mock.new
-Transaction::BRAINTREE = Minitest::Mock.new
-Transaction::REDIS = Minitest::Mock.new
+CreditCardSale::BRAINTREE = Minitest::Mock.new
+CreditCardSale::REDIS = Minitest::Mock.new
 TrustLevelRepo::REDIS = Minitest::Mock.new
 TrustLevelRepo::DB = Minitest::Mock.new
 
-class TransactionTest < Minitest::Test
+class CreditCardSaleTest < Minitest::Test
 	FAKE_BRAINTREE_TRANSACTION =
 		OpenStruct.new(
 			customer_details: OpenStruct.new(id: "customer"),
@@ -20,7 +20,7 @@ class TransactionTest < Minitest::Test
 		)
 
 	def test_sale_fails
-		Transaction::REDIS.expect(
+		CreditCardSale::REDIS.expect(
 			:exists,
 			EMPromise.resolve(0),
 			["jmp_customer_credit_card_lock-test"]
@@ -51,7 +51,7 @@ class TransactionTest < Minitest::Test
 			["jmp_pay_decline-test", 60 * 60 * 24]
 		)
 		braintree_transaction = Minitest::Mock.new
-		Transaction::BRAINTREE.expect(:transaction, braintree_transaction)
+		CreditCardSale::BRAINTREE.expect(:transaction, braintree_transaction)
 		braintree_transaction.expect(
 			:sale,
 			EMPromise.resolve(
@@ -63,21 +63,21 @@ class TransactionTest < Minitest::Test
 			payment_method_token: "token"
 		)
 		assert_raises(RuntimeError) do
-			Transaction.sale(
+			CreditCardSale.new(
 				customer(plan_name: "test_usd"),
 				amount: 99,
 				payment_method: OpenStruct.new(token: "token")
-			).sync
+			).sale.sync
 		end
 		assert_mock CustomerFinancials::REDIS
-		assert_mock Transaction::REDIS
+		assert_mock CreditCardSale::REDIS
 		assert_mock TrustLevelRepo::REDIS
 		assert_mock TrustLevelRepo::DB
 	end
 	em :test_sale_fails
 
 	def test_sale_locked
-		Transaction::REDIS.expect(
+		CreditCardSale::REDIS.expect(
 			:exists,
 			EMPromise.resolve(1),
 			["jmp_customer_credit_card_lock-test"]
@@ -98,21 +98,21 @@ class TransactionTest < Minitest::Test
 			["jmp_pay_decline-test"]
 		)
 		assert_raises("locked") do
-			Transaction.sale(
+			CreditCardSale.new(
 				customer(plan_name: "test_usd"),
 				amount: 123,
 				payment_method: OpenStruct.new(token: "token")
-			).sync
+			).sale.sync
 		end
 		assert_mock CustomerFinancials::REDIS
-		assert_mock Transaction::REDIS
+		assert_mock CreditCardSale::REDIS
 		assert_mock TrustLevelRepo::REDIS
 		assert_mock TrustLevelRepo::DB
 	end
 	em :test_sale_locked
 
 	def test_sale
-		Transaction::REDIS.expect(
+		CreditCardSale::REDIS.expect(
 			:exists,
 			EMPromise.resolve(0),
 			["jmp_customer_credit_card_lock-test"]
@@ -133,7 +133,7 @@ class TransactionTest < Minitest::Test
 			["jmp_pay_decline-test"]
 		)
 		braintree_transaction = Minitest::Mock.new
-		Transaction::BRAINTREE.expect(:transaction, braintree_transaction)
+		CreditCardSale::BRAINTREE.expect(:transaction, braintree_transaction)
 		braintree_transaction.expect(
 			:sale,
 			EMPromise.resolve(
@@ -147,67 +147,112 @@ class TransactionTest < Minitest::Test
 			merchant_account_id: "merchant_usd",
 			options: { submit_for_settlement: true }
 		)
-		Transaction::REDIS.expect(
+		CreditCardSale::REDIS.expect(
 			:setex,
 			EMPromise.resolve(1),
 			["jmp_customer_credit_card_lock-test", 86400, "1"]
 		)
-		result = Transaction.sale(
+		result = CreditCardSale.new(
 			customer(plan_name: "test_usd"),
 			amount: 99,
 			payment_method: OpenStruct.new(token: "token")
-		).sync
-		assert_kind_of Transaction, result
+		).sale.sync
+		assert_equal FAKE_BRAINTREE_TRANSACTION, result
 		assert_mock CustomerFinancials::REDIS
-		assert_mock Transaction::REDIS
+		assert_mock CreditCardSale::REDIS
 		assert_mock TrustLevelRepo::REDIS
 		assert_mock TrustLevelRepo::DB
 	end
 	em :test_sale
 
-	def test_insert
-		Transaction::DB.expect(:transaction, []) do |&block|
-			block.call
-			true
-		end
-		Transaction::DB.expect(
-			:exec,
-			EMPromise.resolve(nil),
-			[
-				String,
-				["customer", "transaction", Time.at(0), Time.at(7776000), 12]
-			]
+	def test_builder
+		expected = Transaction.new(
+			customer_id: "customer",
+			transaction_id: "transaction",
+			created_at: Time.at(0),
+			settled_after: Time.at(7776000),
+			amount: 12,
+			note: "Credit card payment"
+		)
+
+		assert_equal(
+			expected,
+			CreditCardSale::BraintreeTransaction.build(FAKE_BRAINTREE_TRANSACTION)
 		)
-		Transaction.new(FAKE_BRAINTREE_TRANSACTION).insert.sync
-		Transaction::DB.verify
 	end
-	em :test_insert
 
-	def test_insert_with_bonus
-		Transaction::DB.expect(:transaction, []) do |&block|
-			block.call
-			true
-		end
-		Transaction::DB.expect(
-			:exec,
-			EMPromise.resolve(nil),
-			[
-				String,
-				["customer", "transaction", Time.at(0), Time.at(7776000), 100]
-			]
+	def test_create
+		CreditCardSale::REDIS.expect(
+			:exists,
+			EMPromise.resolve(0),
+			["jmp_customer_credit_card_lock-test"]
 		)
-		Transaction::DB.expect(
-			:exec,
-			EMPromise.resolve(nil),
-			[
-				String,
-				["customer", "bonus_for_transaction", Time.at(0), Time.at(7776000), 3]
-			]
-		)
-		tx = FAKE_BRAINTREE_TRANSACTION.dup
-		tx.amount = 100
-		Transaction.new(tx).insert.sync
-		Transaction::DB.verify
+		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("1"),
+			["jmp_pay_decline-test"]
+		)
+		braintree_transaction = Minitest::Mock.new
+		CreditCardSale::BRAINTREE.expect(:transaction, braintree_transaction)
+		response = EMPromise.resolve(
+			OpenStruct.new(
+				success?: true,
+				transaction: FAKE_BRAINTREE_TRANSACTION
+			)
+		)
+		braintree_transaction.expect(
+			:sale,
+			response,
+			amount: 99,
+			payment_method_token: "token",
+			merchant_account_id: "merchant_usd",
+			options: { submit_for_settlement: true }
+		)
+		CreditCardSale::REDIS.expect(
+			:setex,
+			EMPromise.resolve(1),
+			["jmp_customer_credit_card_lock-test", 86400, "1"]
+		)
+
+		transaction = PromiseMock.new
+		transaction.expect(:insert, EMPromise.resolve(nil))
+
+		transaction_class = Minitest::Mock.new
+		transaction_class.expect(
+			:new,
+			transaction,
+			customer_id: "customer",
+			transaction_id: "transaction",
+			created_at: Time.at(0),
+			settled_after: Time.at(7776000),
+			amount: 12,
+			note: "Credit card payment"
+		)
+
+		result = CreditCardSale.create(
+			customer(plan_name: "test_usd"),
+			amount: 99,
+			payment_method: OpenStruct.new(token: "token"),
+			transaction_class: transaction_class
+		).sync
+
+		assert_equal transaction.object_id, result.object_id
+		assert_mock transaction_class
+		assert_mock transaction
+		assert_mock CustomerFinancials::REDIS
+		assert_mock CreditCardSale::REDIS
+		assert_mock TrustLevelRepo::REDIS
+		assert_mock TrustLevelRepo::DB
 	end
-	em :test_insert_with_bonus
+	em :test_create
 end

test/test_helper.rb 🔗

@@ -168,6 +168,10 @@ class PromiseMock < Minitest::Mock
 			yield self
 		end
 	end
+
+	def is_a?(_klass)
+		false
+	end
 end
 
 class FakeTelSelections

test/test_low_balance.rb 🔗

@@ -155,7 +155,7 @@ class LowBalanceTest < Minitest::Test
 	em :test_for_auto_top_up_blocked
 
 	class AutoTopUpTest < Minitest::Test
-		LowBalance::AutoTopUp::Transaction = Minitest::Mock.new
+		LowBalance::AutoTopUp::CreditCardSale = Minitest::Mock.new
 
 		def setup
 			@customer = Minitest::Mock.new(customer(auto_top_up_amount: 100))
@@ -163,15 +163,13 @@ class LowBalanceTest < Minitest::Test
 		end
 
 		def test_notify!
-			tx = PromiseMock.new
-			tx.expect(:insert, EMPromise.resolve(nil))
-			LowBalance::AutoTopUp::Transaction.expect(
-				:sale,
-				tx,
+			tx = OpenStruct.new(total: 13)
+			LowBalance::AutoTopUp::CreditCardSale.expect(
+				:create,
+				EMPromise.resolve(tx),
 				[@customer], amount: 100
 			)
 			@auto_top_up.notify!
-			assert_mock tx
 		end
 		em :test_notify!
 
@@ -214,16 +212,14 @@ class LowBalanceTest < Minitest::Test
 				auto_top_up_amount: 15
 			))
 			auto_top_up = LowBalance::AutoTopUp.new(customer)
+			tx = OpenStruct.new(total: 13)
 
-			tx = PromiseMock.new
-			tx.expect(:insert, EMPromise.resolve(nil))
-			LowBalance::AutoTopUp::Transaction.expect(
-				:sale,
-				tx,
+			LowBalance::AutoTopUp::CreditCardSale.expect(
+				:create,
+				EMPromise.resolve(tx),
 				[customer], amount: 110
 			)
 			auto_top_up.notify!
-			assert_mock tx
 		end
 		em :test_very_low_balance_notify!
 
@@ -233,16 +229,14 @@ class LowBalanceTest < Minitest::Test
 				auto_top_up_amount: 15
 			))
 			auto_top_up = LowBalance::AutoTopUp.new(customer)
+			tx = OpenStruct.new(total: 13)
 
-			tx = PromiseMock.new
-			tx.expect(:insert, EMPromise.resolve(nil))
-			LowBalance::AutoTopUp::Transaction.expect(
-				:sale,
-				tx,
+			LowBalance::AutoTopUp::CreditCardSale.expect(
+				:create,
+				EMPromise.resolve(tx),
 				[customer], amount: 21
 			)
 			auto_top_up.notify!
-			assert_mock tx
 		end
 		em :test_border_low_balance_notify!
 
@@ -257,8 +251,8 @@ class LowBalanceTest < Minitest::Test
 					)
 				}]
 			)
-			LowBalance::AutoTopUp::Transaction.expect(
-				:sale,
+			LowBalance::AutoTopUp::CreditCardSale.expect(
+				:create,
 				EMPromise.reject(RuntimeError.new("test")),
 				[@customer], amount: 100
 			)

test/test_registration.rb 🔗

@@ -446,22 +446,17 @@ class RegistrationTest < Minitest::Test
 		class ActivateTest < Minitest::Test
 			Registration::Payment::CreditCard::Activate::Finish =
 				Minitest::Mock.new
-			Registration::Payment::CreditCard::Activate::Transaction =
+			Registration::Payment::CreditCard::Activate::CreditCardSale =
 				Minitest::Mock.new
 			Command::COMMAND_MANAGER = Minitest::Mock.new
 
 			def test_write
-				transaction = PromiseMock.new
-				transaction.expect(
-					:insert,
-					EMPromise.resolve(nil)
-				)
 				customer = Minitest::Mock.new(
 					customer(plan_name: "test_usd")
 				)
-				Registration::Payment::CreditCard::Activate::Transaction.expect(
-					:sale,
-					transaction
+				Registration::Payment::CreditCard::Activate::CreditCardSale.expect(
+					:create,
+					EMPromise.resolve(nil)
 				) do |acustomer, amount:, payment_method:|
 					assert_operator customer, :===, acustomer
 					assert_equal CONFIG[:activation_amount], amount
@@ -484,8 +479,7 @@ class RegistrationTest < Minitest::Test
 						"+15555550000"
 					).write
 				end
-				Registration::Payment::CreditCard::Activate::Transaction.verify
-				transaction.verify
+				Registration::Payment::CreditCard::Activate::CreditCardSale.verify
 				customer.verify
 				Registration::Payment::CreditCard::Activate::Finish.verify
 			end
@@ -510,8 +504,8 @@ class RegistrationTest < Minitest::Test
 					end]
 				)
 				result = execute_command do
-					Registration::Payment::CreditCard::Activate::Transaction.expect(
-						:sale,
+					Registration::Payment::CreditCard::Activate::CreditCardSale.expect(
+						:create,
 						EMPromise.reject("declined")
 					) do |acustomer, amount:, payment_method:|
 						assert_operator customer, :===, acustomer
@@ -526,7 +520,7 @@ class RegistrationTest < Minitest::Test
 					).write.catch { |e| e }
 				end
 				assert_equal :test_result, result
-				Registration::Payment::CreditCard::Activate::Transaction.verify
+				Registration::Payment::CreditCard::Activate::CreditCardSale.verify
 			end
 			em :test_write_declines
 		end

test/test_web.rb 🔗

@@ -10,7 +10,7 @@ ExpiringLock::REDIS = Minitest::Mock.new
 Customer::BLATHER = Minitest::Mock.new
 CustomerFwd::BANDWIDTH_VOICE = Minitest::Mock.new
 Web::BANDWIDTH_VOICE = Minitest::Mock.new
-LowBalance::AutoTopUp::Transaction = Minitest::Mock.new
+LowBalance::AutoTopUp::CreditCardSale = Minitest::Mock.new
 
 class WebTest < Minitest::Test
 	include Rack::Test::Methods
@@ -165,10 +165,10 @@ class WebTest < Minitest::Test
 	em :test_outbound_low_balance
 
 	def test_outbound_low_balance_top_up
-		LowBalance::AutoTopUp::Transaction.expect(
-			:sale,
+		LowBalance::AutoTopUp::CreditCardSale.expect(
+			:create,
 			EMPromise.resolve(
-				OpenStruct.new(insert: EMPromise.resolve(nil), total: 15)
+				OpenStruct.new(total: 15)
 			),
 			[Customer], amount: 15
 		)
@@ -225,7 +225,7 @@ class WebTest < Minitest::Test
 		)
 		assert_mock ExpiringLock::REDIS
 		assert_mock Customer::BLATHER
-		assert_mock LowBalance::AutoTopUp::Transaction
+		assert_mock LowBalance::AutoTopUp::CreditCardSale
 	end
 	em :test_outbound_low_balance_top_up