From 223ee6c8ece43eb81ca2ad70b9638d61f4958694 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Wed, 18 Jun 2025 13:12:30 -0500 Subject: [PATCH] Unify more invite code handling 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. --- 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(-) diff --git a/lib/invites_repo.rb b/lib/invites_repo.rb index 023a06bb4aad43c5722f3dc63db0c198e99a3d09..a760988eb6143850356ef7f035f6ce5f4c17f1e1 100644 --- a/lib/invites_repo.rb +++ b/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) diff --git a/lib/parent_code_repo.rb b/lib/parent_code_repo.rb index ed92d25103053df67548f787595e53d5ad9269a4..5631a913d8ff5aaed04bad79ed5fb58f1eba5202 100644 --- a/lib/parent_code_repo.rb +++ b/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 } diff --git a/lib/registration.rb b/lib/registration.rb index 7c2e23c0dcac3da4f5bc29c5524948ca86a4d2c9..c8c1acc6b84e40a6f713e3878a296bca8dc5d54d 100644 --- a/lib/registration.rb +++ b/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 diff --git a/test/test_helper.rb b/test/test_helper.rb index 539f3e5ae84b5135bdc65c7f1cf3d9f9677a77ec..7729ff95c5fcb1373721cc7b1ced347bea51a889 100644 --- a/test/test_helper.rb +++ b/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) diff --git a/test/test_registration.rb b/test/test_registration.rb index d510a9000565c06a06a9a132c92dc0a72f5e5814..388ce05fcac268f46ad211e35faaacff0b8c2e45 100644 --- a/test/test_registration.rb +++ b/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),