Refactor BuyAccountCreditForm

Stephen Paul Weber created 

add_to_form no longer needs a promise or a network call, instead we use
a factory to get that data up front and inject the dependency.

New BuyAccountCreditForm#parse to get the relevant data back out of an
XMPP form.

Some changes to Transaction and sgx_jmp.rb to use the new method
semantics.

Change summary

lib/buy_account_credit_form.rb       | 38 ++++++++++++++++++++---------
lib/registration.rb                  |  4 +-
lib/transaction.rb                   |  2 
sgx_jmp.rb                           | 19 ++++----------
test/test_buy_account_credit_form.rb | 30 +++++++++++++----------
test/test_registration.rb            | 26 +++++++++-----------
test/test_transaction.rb             |  8 +++---
7 files changed, 68 insertions(+), 59 deletions(-)

Detailed changes

lib/buy_account_credit_form.rb 🔗 

@@ -3,8 +3,15 @@
 require_relative "./xep0122_field"
 
 class BuyAccountCreditForm
-	def initialize(customer)
-		@customer = customer
+	def self.for(customer)
+		@customer.payment_methods.then do |payment_methods|
+			new(customer.balance, payment_methods)
+		end
+	end
+
+	def initialize(balance, payment_methods)
+		@balance = balance
+		@payment_methods = payment_methods
 	end
 
 	AMOUNT_FIELD =
@@ -19,19 +26,26 @@ class BuyAccountCreditForm
 	def balance
 		{
 			type: "fixed",
-			value: "Current balance: $#{'%.2f' % @customer.balance}"
+			value: "Current balance: $#{'%.2f' % @balance}"
 		}
 	end
 
 	def add_to_form(form)
-		@customer.payment_methods.then do |payment_methods|
-			form.type = :form
-			form.title = "Buy Account Credit"
-			form.fields = [
-				balance,
-				payment_methods.to_list_single,
-				AMOUNT_FIELD
-			]
-		end
+		form.type = :form
+		form.title = "Buy Account Credit"
+		form.fields = [
+			balance,
+			@payment_methods.to_list_single,
+			AMOUNT_FIELD
+		]
+	end
+
+	def parse(form)
+		{
+			payment_method: @payment_methods.fetch(
+				form.field("payment_method")&.value.to_i
+			),
+			amount: form.field("amount")&.value&.to_s
+		}
 	end
 end

lib/registration.rb 🔗 

@@ -258,8 +258,8 @@ class Registration
 				def write
 					Transaction.sale(
 						@customer,
-						CONFIG[:activation_amount],
-						@payment_method
+						amount: CONFIG[:activation_amount],
+						payment_method: @payment_method
 					).then(
 						method(:sold),
 						->(_) { declined }

lib/transaction.rb 🔗 

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Transaction
-	def self.sale(customer, amount, payment_method=nil)
+	def self.sale(customer, amount:, payment_method: nil)
 		REDIS.get("jmp_pay_decline-#{customer.customer_id}").then do |declines|
 			raise "too many declines" if declines.to_i >= 2

sgx_jmp.rb 🔗 

@@ -253,20 +253,13 @@ command :execute?, node: "buy-credit", sessionid: nil do |iq|
 	reply.allowed_actions = [:complete]
 
 	Customer.for_jid(iq.from.stripped).then { |customer|
-		BuyAccountCreditForm.new(customer).add_to_form(reply.form).then { customer }
-	}.then { |customer|
-		EMPromise.all([
-			customer,
-			customer.payment_methods,
-			COMMAND_MANAGER.write(reply)
-		])
-	}.then { |(customer, payment_methods, iq2)|
+		BuyAccountCreditForm.for(customer).then do |credit_form|
+			credit_form.add_to_form(reply.form)
+			COMMAND_MANAGER.write(reply).then { |iq2| [customer, credit_form, iq2] }
+		end
+	}.then { |(customer, credit_form, iq2)|
 		iq = iq2 # This allows the catch to use it also
-		payment_method = payment_methods.fetch(
-			iq.form.field("payment_method")&.value.to_i
-		)
-		amount = iq.form.field("amount").value.to_s
-		Transaction.sale(customer, amount, payment_method)
+		Transaction.sale(customer, **credit_form.parse(iq2.form))
 	}.then { |transaction|
 		transaction.insert.then { transaction.amount }
 	}.then { |amount|

test/test_buy_account_credit_form.rb 🔗 

@@ -6,18 +6,11 @@ require "customer"
 
 class BuyAccountCreditFormTest < Minitest::Test
 	def setup
-		@customer = Minitest::Mock.new(Customer.new(
-			1,
-			plan_name: "test_usd",
-			balance: BigDecimal.new("12.1234")
-		))
-		@customer.expect(
-			:payment_methods,
-			EMPromise.resolve(PaymentMethods.new([
-				OpenStruct.new(card_type: "Test", last_4: "1234")
-			]))
+		@payment_method = OpenStruct.new(card_type: "Test", last_4: "1234")
+		@form = BuyAccountCreditForm.new(
+			BigDecimal.new("12.1234"),
+			PaymentMethods.new([@payment_method])
 		)
-		@form = BuyAccountCreditForm.new(@customer)
 	end
 
 	def test_balance
@@ -29,7 +22,7 @@ class BuyAccountCreditFormTest < Minitest::Test
 
 	def test_add_to_form
 		iq_form = Blather::Stanza::X.new
-		@form.add_to_form(iq_form).sync
+		@form.add_to_form(iq_form)
 		assert_equal :form, iq_form.type
 		assert_equal "Buy Account Credit", iq_form.title
 		assert_equal(
@@ -50,5 +43,16 @@ class BuyAccountCreditFormTest < Minitest::Test
 			iq_form.fields
 		)
 	end
-	em :test_add_to_form
+
+	def test_parse_amount
+		iq_form = Blather::Stanza::X.new
+		iq_form.fields = [{ var: "amount", value: "123" }]
+		assert_equal "123", @form.parse(iq_form)[:amount]
+	end
+
+	def test_parse_payment_method
+		iq_form = Blather::Stanza::X.new
+		iq_form.fields = [{ var: "payment_method", value: "0" }]
+		assert_equal @payment_method, @form.parse(iq_form)[:payment_method]
+	end
 end

test/test_registration.rb 🔗 

@@ -264,13 +264,12 @@ class RegistrationTest < Minitest::Test
 				)
 				Registration::Payment::CreditCard::Activate::Transaction.expect(
 					:sale,
-					transaction,
-					[
-						customer,
-						CONFIG[:activation_amount],
-						:test_default_method
-					]
-				)
+					transaction
+				) do |acustomer, amount:, payment_method:|
+					assert_operator customer, :===, acustomer
+					assert_equal CONFIG[:activation_amount], amount
+					assert_equal :test_default_method, payment_method
+				end
 				iq = Blather::Stanza::Iq::Command.new
 				customer.expect(:bill_plan, nil)
 				Registration::Payment::CreditCard::Activate::Finish.expect(
@@ -297,13 +296,12 @@ class RegistrationTest < Minitest::Test
 				)
 				Registration::Payment::CreditCard::Activate::Transaction.expect(
 					:sale,
-					EMPromise.reject("declined"),
-					[
-						customer,
-						CONFIG[:activation_amount],
-						:test_default_method
-					]
-				)
+					EMPromise.reject("declined")
+				) do |acustomer, amount:, payment_method:|
+					assert_operator customer, :===, acustomer
+					assert_equal CONFIG[:activation_amount], amount
+					assert_equal :test_default_method, payment_method
+				end
 				iq = Blather::Stanza::Iq::Command.new
 				iq.from = "test@example.com"
 				result = Minitest::Mock.new

test/test_transaction.rb 🔗 

@@ -44,8 +44,8 @@ class TransactionTest < Minitest::Test
 		assert_raises("declined") do
 			Transaction.sale(
 				Customer.new("test", plan_name: "test_usd"),
-				123,
-				OpenStruct.new(token: "token")
+				amount: 123,
+				payment_method: OpenStruct.new(token: "token")
 			).sync
 		end
 	end
@@ -76,8 +76,8 @@ class TransactionTest < Minitest::Test
 		)
 		result = Transaction.sale(
 			Customer.new("test", plan_name: "test_usd"),
-			123,
-			OpenStruct.new(token: "token")
+			amount: 123,
+			payment_method: OpenStruct.new(token: "token")
 		).sync
 		assert_kind_of Transaction, result
 	end