Unify more invite code handling

Stephen Paul Weber created

Using the optional field on Activation or the InviteCode flow now use
the same code and do the same things. Always check balance and proceed
to bill if we've used a parent code, otherwise never do that.

Fixes the subaccount-from-onboarding guard as well.

Change summary

lib/invites_repo.rb       |   4 
lib/parent_code_repo.rb   |  17 ++++
lib/registration.rb       | 115 +++++++++++--------------------
test/test_helper.rb       |   2 
test/test_registration.rb | 147 +++++++++++++++++++++++++++++-----------
5 files changed, 165 insertions(+), 120 deletions(-)

Detailed changes

lib/invites_repo.rb 🔗

@@ -133,7 +133,9 @@ protected
 	end
 
 	def invalid_code(customer_id, code)
-		@redis.incr("jmp_invite_tries-#{customer_id}").then {
+		stash_code(customer_id, code).then {
+			@redis.incr("jmp_invite_tries-#{customer_id}")
+		}.then {
 			@redis.expire("jmp_invite_tries-#{customer_id}", 60 * 60)
 		}.then {
 			@redis.hexists("jmp_group_codes", code)

lib/parent_code_repo.rb 🔗

@@ -7,6 +7,8 @@ require_relative "customer"
 require_relative "trust_level_repo"
 
 class ParentCodeRepo
+	class Invalid < StandardError; end
+
 	def initialize(
 		redis: REDIS,
 		db: DB,
@@ -17,6 +19,21 @@ class ParentCodeRepo
 		@trust_level_repo = trust_level_repo
 	end
 
+	def claim_code(customer, code)
+		customer_domain = ProxiedJID.new(customer.jid).unproxied.domain
+		find(code).then do |parent|
+			raise Invalid, "Not a valid code" unless parent
+
+			if parent && customer_domain == CONFIG[:onboarding_domain]
+				raise "Please create a new Jabber ID before creating a subaccount."
+			end
+
+			plan_name = customer.plan_name
+			customer = customer.with_plan(plan_name, parent_customer_id: parent)
+			customer.save_plan!.then { block_given? ? yield(customer) : customer }
+		end
+	end
+
 	def find(code)
 		@redis.hget("jmp_parent_codes", code).then do |parent_id|
 			trust_level_guard(parent_id).then { parent_id }

lib/registration.rb 🔗

@@ -41,13 +41,6 @@ class Registration
 		end
 	end
 
-	def self.guard_onboarding_subaccounts(customer)
-		customer_domain = ProxiedJID.new(customer.jid).domain
-		return unless customer_domain == CONFIG[:onboarding_domain]
-
-		raise "Please create a new Jabber ID before creating a subaccount."
-	end
-
 	class Registered
 		def self.for(customer, tel)
 			jid = ProxiedJID.new(customer.jid).unproxied
@@ -108,7 +101,6 @@ class Registration
 		def initialize(customer, tel)
 			@customer = customer
 			@tel = tel
-			@invites = InvitesRepo.new(DB, REDIS)
 		end
 
 		attr_reader :customer, :tel
@@ -125,33 +117,15 @@ class Registration
 		end
 
 		def next_step(iq)
-			code = iq.form.field("code")&.value&.to_s
-			save_customer_plan(iq, code).then {
-				finish_if_valid_invite(code)
-			}.catch_only(InvitesRepo::Invalid) do
-				@invites.stash_code(customer.customer_id, code).then do
-					Payment.for(iq, @customer, @tel).then(&:write)
-				end
-			end
-		end
-
-	protected
-
-		def finish_if_valid_invite(code)
-			@invites.claim_code(@customer.customer_id, code) {
-				@customer.activate_plan_starting_now
-			}.then do
-				Finish.new(@customer, @tel).write
-			end
-		end
-
-		def save_customer_plan(iq, code)
-			Registration.guard_onboarding_subaccounts(@customer)
-
-			ParentCodeRepo.new(redis: REDIS, db: DB).find(code).then do |parent|
+			EMPromise.resolve(nil).then do
 				plan = Plan.for_registration(iq.form.field("plan_name").value.to_s)
-				@customer = @customer.with_plan(plan.name, parent_customer_id: parent)
-				@customer.save_plan!
+				@customer = @customer.with_plan(plan.name)
+				Registration::Payment::InviteCode.new(
+					@customer, @tel, finish: Finish, db: DB, redis: REDIS
+				).parse(iq, force_save_plan: true)
+					.catch_only(InvitesRepo::Invalid) do
+						Payment.for(iq, @customer, @tel).then(&:write)
+					end
 			end
 		end
 
@@ -440,24 +414,7 @@ class Registration
 		end
 
 		class InviteCode
-			Payment.kinds[:code] = ->(*args, **kw) { self.for(*args, **kw) }
-
-			def self.for(in_customer, tel, finish: Finish, **)
-				reload_customer(in_customer).then do |customer|
-					if customer.balance >= CONFIG[:activation_amount_accept]
-						next BillPlan.new(customer, tel, finish: finish)
-					end
-
-					msg = if customer.balance.positive?
-						"Account balance not enough to cover the activation"
-					end
-					new(customer, tel, error: msg, finish: Finish)
-				end
-			end
-
-			def self.reload_customer(customer)
-				Command.execution.customer_repo.find(customer.customer_id)
-			end
+			Payment.kinds[:code] = method(:new)
 
 			FIELDS = [{
 				var: "code",
@@ -466,12 +423,16 @@ class Registration
 				required: true
 			}].freeze
 
-			def initialize(customer, tel, error: nil, finish: Finish, **)
+			def initialize(
+				customer, tel,
+				error: nil, finish: Finish, db: DB, redis: REDIS, **
+			)
 				@customer = customer
 				@tel = tel
 				@error = error
 				@finish = finish
-				@parent_code_repo = ParentCodeRepo.new(redis: REDIS, db: DB)
+				@invites_repo = InvitesRepo.new(db, redis)
+				@parent_code_repo = ParentCodeRepo.new(db: db, redis: redis)
 			end
 
 			def add_form(reply)
@@ -486,48 +447,52 @@ class Registration
 				Command.reply { |reply|
 					reply.allowed_actions = [:next, :prev]
 					add_form(reply)
-				}.then(&method(:parse))
+				}.then(&method(:parse)).catch_only(InvitesRepo::Invalid) { |e|
+					invalid_code(e).write
+				}
 			end
 
-			def parse(iq)
+			def parse(iq, force_save_plan: false)
 				return Activation.for(@customer, nil, @tel).then(&:write) if iq.prev?
 
-				verify(iq.form.field("code")&.value&.to_s)
-					.catch_only(InvitesRepo::Invalid, &method(:invalid_code))
+				verify(iq.form.field("code")&.value&.to_s, force_save_plan)
 					.then(&:write)
 			end
 
 		protected
 
 			def invalid_code(e)
-				InviteCode.new(@customer, @tel, error: e.message)
+				self.class.new(@customer, @tel, error: e.message, finish: @finish)
 			end
 
 			def customer_id
 				@customer.customer_id
 			end
 
-			def verify(code)
-				@parent_code_repo.find(code).then do |parent_customer_id|
-					if parent_customer_id
-						set_parent(parent_customer_id)
-					else
-						InvitesRepo.new(DB, REDIS).claim_code(customer_id, code) {
+			def verify(code, force_save_plan)
+				@parent_code_repo.claim_code(@customer, code) {
+					check_parent_balance
+				}.catch_only(ParentCodeRepo::Invalid) {
+					(@customer.save_plan! if force_save_plan).then do
+						@invites_repo.claim_code(customer_id, code) {
 							@customer.activate_plan_starting_now
-						}.then { Finish.new(@customer, @tel) }
+						}.then { @finish.new(@customer, @tel) }
 					end
-				end
+				}
 			end
 
-			def set_parent(parent_customer_id)
-				Registration.guard_onboarding_subaccounts(@customer)
+			def reload_customer
+				Command.execution.customer_repo.find(@customer.customer_id)
+			end
 
-				@customer = @customer.with_plan(
-					@customer.plan_name,
-					parent_customer_id: parent_customer_id
-				)
-				@customer.save_plan!.then do
-					self.class.for(@customer, @tel, finish: @finish)
+			def check_parent_balance
+				reload_customer.then do |customer|
+					if customer.balance >= CONFIG[:activation_amount_accept]
+						next BillPlan.new(customer, @tel, finish: @finish)
+					end
+
+					msg = "Account balance not enough to cover the activation"
+					invalid_code(RuntimeError.new(msg))
 				end
 			end
 		end

test/test_helper.rb 🔗

@@ -304,7 +304,7 @@ class FakeRedis
 	end
 
 	def hget(key, field)
-		@values.dig(key, field)
+		EMPromise.resolve(@values.dig(key, field))
 	end
 
 	def hexists(key, field)

test/test_registration.rb 🔗

@@ -191,6 +191,43 @@ class RegistrationTest < Minitest::Test
 		end
 		em :test_write
 
+		def test_write_with_onboarding_jid
+			Command::COMMAND_MANAGER.expect(
+				:write,
+				EMPromise.resolve(Blather::Stanza::Iq::Command.new.tap { |iq|
+					iq.form.fields = [{ var: "plan_name", value: "test_usd" }]
+				}),
+				[Matching.new do |iq|
+					assert_equal :form, iq.form.type
+					assert_equal(
+						"You've selected +15555550000 as your JMP number.",
+						iq.form.instructions.lines.first.chomp
+					)
+				end]
+			)
+			@customer.expect(
+				:jid,
+				Blather::JID.new("test\\40onboarding.example.com@proxy")
+			)
+			@customer.expect(:with_plan, @customer) do |*args, **|
+				assert_equal ["test_usd"], args
+			end
+			@customer.expect(:save_plan!, EMPromise.resolve(nil), [])
+			Registration::Activation::Payment.expect(
+				:for,
+				EMPromise.reject(:test_result),
+				[Blather::Stanza::Iq, @customer, "+15555550000"]
+			)
+			assert_equal(
+				:test_result,
+				execute_command { @activation.write.catch { |e| e } }
+			)
+			assert_mock Command::COMMAND_MANAGER
+			assert_mock @customer
+			assert_mock Registration::Activation::Payment
+		end
+		em :test_write_with_onboarding_jid
+
 		def test_write_bad_plan
 			Command::COMMAND_MANAGER.expect(
 				:write,
@@ -308,48 +345,52 @@ class RegistrationTest < Minitest::Test
 		em :test_write_with_group_code
 
 		def test_write_with_parent_code
-			Command::COMMAND_MANAGER.expect(
-				:write,
-				EMPromise.resolve(Blather::Stanza::Iq::Command.new.tap { |iq|
-					iq.form.fields = [
-						{ var: "plan_name", value: "test_usd" },
-						{ var: "code", value: "PARENT_CODE" }
-					]
-				}),
-				[Matching.new do |iq|
-					assert_equal :form, iq.form.type
-					assert_equal(
-						"You've selected +15555550000 as your JMP number.",
-						iq.form.instructions.lines.first.chomp
-					)
-				end]
-			)
-			Registration::Activation::DB.expect(
-				:query_one, {}, [String, "1"], default: {}
-			)
-			Registration::Activation::DB.expect(
-				:query_one, { c: 0 }, [String, "1"], default: { c: 0 }
-			)
-			@customer.expect(:with_plan, @customer) do |*args, **kwargs|
-				assert_equal ["test_usd"], args
-				assert_equal({ parent_customer_id: "1" }, kwargs)
+			execute_command do
+				Command::COMMAND_MANAGER.expect(
+					:write,
+					EMPromise.resolve(Blather::Stanza::Iq::Command.new.tap { |iq|
+						iq.form.fields = [
+							{ var: "plan_name", value: "test_usd" },
+							{ var: "code", value: "PARENT_CODE" }
+						]
+					}),
+					[Matching.new do |iq|
+						assert_equal :form, iq.form.type
+						assert_equal(
+							"You've selected +15555550000 as your JMP number.",
+							iq.form.instructions.lines.first.chomp
+						)
+					end]
+				)
+				Registration::Activation::DB.expect(
+					:query_one, {}, [String, "1"], default: {}
+				)
+				Registration::Activation::DB.expect(
+					:query_one, { c: 0 }, [String, "1"], default: { c: 0 }
+				)
+				@customer.expect(:with_plan, @customer) do |*args, **|
+					assert_equal ["test_usd"], args
+				end
+				@customer.expect(:with_plan, @customer) do |*, **kwargs|
+					assert_equal({ parent_customer_id: "1" }, kwargs)
+				end
+				@customer.expect(:save_plan!, EMPromise.resolve(nil), [])
+				@customer.expect(:balance, 100, [])
+				Command.execution.customer_repo.expect(
+					:find,
+					EMPromise.resolve(@customer), ["test"]
+				)
+				Registration::Payment::InviteCode::BillPlan.expect(
+					:new,
+					EMPromise.reject(:test_result)
+				) do |*args, **|
+					assert_equal "+15555550000", args[1]
+				end
+				assert_equal(
+					:test_result,
+					@activation.write.catch { |e| e }.sync
+				)
 			end
-			@customer.expect(:save_plan!, EMPromise.resolve(nil), [])
-			Registration::Activation::DB.expect(:transaction, []) { |&blk| blk.call }
-			Registration::Activation::DB.expect(
-				:exec,
-				OpenStruct.new(cmd_tuples: 0),
-				[String, ["test", "PARENT_CODE"]]
-			)
-			Registration::Activation::Payment.expect(
-				:for,
-				EMPromise.reject(:test_result),
-				[Blather::Stanza::Iq, @customer, "+15555550000"]
-			)
-			assert_equal(
-				:test_result,
-				execute_command { @activation.write.catch { |e| e } }
-			)
 			assert_mock Command::COMMAND_MANAGER
 			assert_mock @customer
 			assert_mock Registration::Activation::Payment
@@ -374,9 +415,19 @@ class RegistrationTest < Minitest::Test
 					)
 				end]
 			)
-			@customer.expect(:jid, Blather::JID.new("test@onboarding.example.com"))
+			Registration::Activation::DB.expect(
+				:query_one, {}, [String, "1"], default: {}
+			)
+			Registration::Activation::DB.expect(
+				:query_one, { c: 0 }, [String, "1"], default: { c: 0 }
+			)
+			@customer.expect(:with_plan, @customer, ["test_usd"])
+			@customer.expect(
+				:jid,
+				Blather::JID.new("test\\40onboarding.example.com@proxy")
+			)
 			iq = Blather::Stanza::Iq::Command.new
-			iq.from = "test@onboarding.example.com"
+			iq.from = "test\\40onboarding.example.com@proxied"
 			assert_equal(
 				"Please create a new Jabber ID before creating a subaccount.",
 				execute_command(iq) { @activation.write.catch(&:to_s) }
@@ -978,6 +1029,11 @@ class RegistrationTest < Minitest::Test
 			def test_write_bad_code
 				result = execute_command do
 					customer = customer(plan_name: "test_usd")
+					Registration::Payment::InviteCode::REDIS.expect(
+						:set,
+						EMPromise.resolve(nil),
+						["jmp_customer_pending_invite-test", "abc"]
+					)
 					Registration::Payment::InviteCode::REDIS.expect(
 						:get,
 						EMPromise.resolve(0),
@@ -1051,6 +1107,11 @@ class RegistrationTest < Minitest::Test
 			def test_write_group_code
 				result = execute_command do
 					customer = customer(plan_name: "test_usd")
+					Registration::Payment::InviteCode::REDIS.expect(
+						:set,
+						EMPromise.resolve(nil),
+						["jmp_customer_pending_invite-test", "abc"]
+					)
 					Registration::Payment::InviteCode::REDIS.expect(
 						:get,
 						EMPromise.resolve(0),