From aab558d05d3d7a412f81ff1e44739cb0c5015d11 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Tue, 23 May 2023 13:25:40 -0500 Subject: [PATCH 1/2] Allow setting parent during signup using a special referral code Lookup any referral code to see if it is one for setting a parent, if so set the parent when we set the plan. In the invite code flow, reload customer and check balance and if there is enough then we can bill customer and proceed, no need to add more credit or another code. Verify parent when setting to make sure it has the same currency as the child plan at creation time (note that updating the parent plan in the future can violate this, so be very careful if/when we allow for that!) --- lib/customer.rb | 4 +- lib/customer_plan.rb | 19 +++++++++- lib/registration.rb | 51 ++++++++++++++++++++------ test/test_customer.rb | 5 +++ test/test_customer_repo.rb | 5 +++ test/test_registration.rb | 75 +++++++++++++++++++++++++++++++++----- 6 files changed, 135 insertions(+), 24 deletions(-) diff --git a/lib/customer.rb b/lib/customer.rb index 84baa8bfea84cc2d5ca8903d31d1881eb77d3e91..62d5c58e0f8d500a2d77fde60c74f34b9136f044 100644 --- a/lib/customer.rb +++ b/lib/customer.rb @@ -81,10 +81,10 @@ class Customer ) end - def with_plan(plan_name) + def with_plan(plan_name, **kwargs) self.class.new( @customer_id, @jid, - plan: @plan.with_plan_name(plan_name), + plan: @plan.with_plan_name(plan_name, **kwargs), balance: @balance, tndetails: @tndetails, sgx: @sgx ) end diff --git a/lib/customer_plan.rb b/lib/customer_plan.rb index 63f2700703ca867c54dc589211180305868adb6d..bacd26640dde3febd4475ccca29a9b00dbd272fa 100644 --- a/lib/customer_plan.rb +++ b/lib/customer_plan.rb @@ -70,15 +70,29 @@ class CustomerPlan :expired end - def with_plan_name(plan_name) + def with_plan_name(plan_name, **kwargs) self.class.new( @customer_id, plan: Plan.for(plan_name), - expires_at: @expires_at + expires_at: @expires_at, **kwargs ) end + def verify_parent! + return unless @parent_customer_id + + result = DB.query(<<~SQL, [@parent_customer_id]) + SELECT plan_name FROM customer_plans WHERE customer_id=$1 + SQL + + raise "Invalid parent account" if !result || !result.first + + plan = Plan.for(result.first["plan_name"]) + raise "Parent currency mismatch" unless plan.currency == currency + end + def save_plan! + verify_parent! DB.exec_defer(<<~SQL, [@customer_id, plan_name, @parent_customer_id]) INSERT INTO plan_log (customer_id, plan_name, parent_customer_id, date_range) @@ -107,6 +121,7 @@ class CustomerPlan end def activate_plan_starting_now + verify_parent! activated = DB.exec(<<~SQL, [@customer_id, plan_name, @parent_customer_id]) INSERT INTO plan_log (customer_id, plan_name, date_range, parent_customer_id) VALUES ($1, $2, tsrange(LOCALTIMESTAMP, LOCALTIMESTAMP + '1 month'), $3) diff --git a/lib/registration.rb b/lib/registration.rb index 81ae35868dde9158b0f441a41398b868845203de..b001261992cb4d4b2083ac4bde40880ea20f1eaa 100644 --- a/lib/registration.rb +++ b/lib/registration.rb @@ -11,6 +11,7 @@ require_relative "./command" require_relative "./em" require_relative "./invites_repo" require_relative "./oob" +require_relative "./parent_code_repo" require_relative "./proxied_jid" require_relative "./tel_selections" require_relative "./welcome_message" @@ -105,7 +106,7 @@ class Registration def next_step(iq) code = iq.form.field("code")&.value&.to_s - save_customer_plan(iq).then { + 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 @@ -124,10 +125,12 @@ class Registration end end - def save_customer_plan(iq) - plan_name = iq.form.field("plan_name").value.to_s - @customer = @customer.with_plan(plan_name) - @customer.save_plan! + def save_customer_plan(iq, code) + ParentCodeRepo.new(REDIS).find(code).then do |parent| + plan_name = iq.form.field("plan_name").value.to_s + @customer = @customer.with_plan(plan_name, parent_customer_id: parent) + @customer.save_plan! + end end class GooglePlay @@ -136,6 +139,7 @@ class Registration @google_play_userid = google_play_userid @tel = tel @invites = InvitesRepo.new(DB, REDIS) + @parent_code_repo = ParentCodeRepo.new(REDIS) end def used @@ -163,17 +167,25 @@ class Registration end def activate(iq) - REDIS.sadd("google_play_userids", @google_play_userid).then { - plan_name = iq.form.field("plan_name").value.to_s - @customer = @customer.with_plan(plan_name) - @customer.activate_plan_starting_now + plan_name = iq.form.field("plan_name").value + code = iq.form.field("code")&.value + EMPromise.all([ + @parent_code_repo.find(code), + REDIS.sadd("google_play_userids", @google_play_userid) + ]).then { |(parent, _)| + save_active_plan(plan_name, parent) }.then do - use_referral_code(iq.form.field("code")&.value&.to_s) + use_referral_code(code) end end protected + def save_active_plan(plan_name, parent) + @customer = @customer.with_plan(plan_name, parent_customer_id: parent) + @customer.activate_plan_starting_now + end + def use_referral_code(code) EMPromise.resolve(nil).then { @invites.claim_code(@customer.customer_id, code) { @@ -406,7 +418,24 @@ class Registration end class InviteCode - Payment.kinds[:code] = method(:new) + 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) + end + end + + def self.reload_customer(customer) + Command.execution.customer_repo.find(customer.customer_id) + end FIELDS = [{ var: "code", diff --git a/test/test_customer.rb b/test/test_customer.rb index 0fb7b7c4db8914cc06ce0eafbd5c05814f01ec94..6da8192abf0942d38d4e193b38afcaeeb9ca3038 100644 --- a/test/test_customer.rb +++ b/test/test_customer.rb @@ -49,6 +49,11 @@ class CustomerTest < Minitest::Test em :test_bill_plan_activate def test_bill_plan_reactivate_child + CustomerPlan::DB.expect( + :query, + [{ "plan_name" => "test_usd" }], + [String, ["parent"]] + ) CustomerPlan::DB.expect(:transaction, nil) do |&block| block.call true diff --git a/test/test_customer_repo.rb b/test/test_customer_repo.rb index 1d84993728615f3647451ab7c5fb5c4cd99dec80..bf7b6d0befdda7cef72cbab5456737de2ef4fe66 100644 --- a/test/test_customer_repo.rb +++ b/test/test_customer_repo.rb @@ -194,6 +194,11 @@ class CustomerRepoTest < Minitest::Test EMPromise.resolve([]), ["jmp_customer_feature_flags-testp"] ) + CustomerPlan::DB.expect( + :query, + [{ "plan_name" => "test_usd" }], + [String, ["1234"]] + ) CustomerPlan::DB.expect( :exec_defer, EMPromise.resolve(nil), diff --git a/test/test_registration.rb b/test/test_registration.rb index 7be8c1de9bb6579713097477d7425d14a07959c8..1d89b139957b234d65418d18d1da79594d215171 100644 --- a/test/test_registration.rb +++ b/test/test_registration.rb @@ -112,7 +112,9 @@ class RegistrationTest < Minitest::Test class ActivationTest < Minitest::Test Registration::Activation::DB = Minitest::Mock.new - Registration::Activation::REDIS = FakeRedis.new + Registration::Activation::REDIS = FakeRedis.new( + "jmp_parent_codes" => { "PARENT_CODE" => 1 } + ) Registration::Activation::Payment = Minitest::Mock.new Registration::Activation::Finish = Minitest::Mock.new Command::COMMAND_MANAGER = Minitest::Mock.new @@ -136,7 +138,9 @@ class RegistrationTest < Minitest::Test ) end] ) - @customer.expect(:with_plan, @customer, ["test_usd"]) + @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, @@ -170,7 +174,9 @@ class RegistrationTest < Minitest::Test ) end] ) - @customer.expect(:with_plan, @customer, ["test_usd"]) + @customer.expect(:with_plan, @customer) do |*args, **| + assert_equal ["test_usd"], args + end @customer.expect(:save_plan!, EMPromise.resolve(nil), []) @customer.expect(:activate_plan_starting_now, EMPromise.resolve(nil), []) Registration::Activation::DB.expect(:transaction, []) { |&blk| blk.call } @@ -212,7 +218,9 @@ class RegistrationTest < Minitest::Test ) end] ) - @customer.expect(:with_plan, @customer, ["test_usd"]) + @customer.expect(:with_plan, @customer) do |*args, **| + assert_equal ["test_usd"], args + end @customer.expect(:save_plan!, EMPromise.resolve(nil), []) Registration::Activation::DB.expect(:transaction, []) { |&blk| blk.call } Registration::Activation::DB.expect( @@ -241,6 +249,50 @@ class RegistrationTest < Minitest::Test assert_mock Registration::Activation::DB end 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] + ) + @customer.expect(:with_plan, @customer) do |*args, **kwargs| + assert_equal ["test_usd"], args + assert_equal({ parent_customer_id: 1 }, kwargs) + 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 + assert_mock Registration::Activation::DB + end + em :test_write_with_parent_code end class AllowTest < Minitest::Test @@ -426,13 +478,18 @@ class RegistrationTest < Minitest::Test { var: "activation_method", value: "code" }, { var: "plan_name", value: "test_usd" } ] - result = Registration::Payment.for( - iq, - customer, - "+15555550000" - ) + cust = customer + result = execute_command do + Command.execution.customer_repo.expect(:find, cust, ["test"]) + Registration::Payment.for( + iq, + cust, + "+15555550000" + ) + end assert_kind_of Registration::Payment::InviteCode, result end + em :test_for_code class BitcoinTest < Minitest::Test Registration::Payment::Bitcoin::BTC_SELL_PRICES = Minitest::Mock.new From eeb7208d13b06498ba150743ed2082ea3e781c99 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Mon, 29 May 2023 12:10:09 -0500 Subject: [PATCH 2/2] Refactor CutomerPlan to use value semantics --- lib/customer.rb | 2 +- lib/customer_plan.rb | 82 ++++++++++++++++++++------------------------ lib/plan.rb | 2 ++ 3 files changed, 40 insertions(+), 46 deletions(-) diff --git a/lib/customer.rb b/lib/customer.rb index 62d5c58e0f8d500a2d77fde60c74f34b9136f044..76bbfb4b0d5fc849abd9347c8cbab895f03055f4 100644 --- a/lib/customer.rb +++ b/lib/customer.rb @@ -84,7 +84,7 @@ class Customer def with_plan(plan_name, **kwargs) self.class.new( @customer_id, @jid, - plan: @plan.with_plan_name(plan_name, **kwargs), + plan: @plan.with(plan_name: plan_name, **kwargs), balance: @balance, tndetails: @tndetails, sgx: @sgx ) end diff --git a/lib/customer_plan.rb b/lib/customer_plan.rb index bacd26640dde3febd4475ccca29a9b00dbd272fa..b63230867b5bc1dfc532ee96ea1b7ae20b680aa9 100644 --- a/lib/customer_plan.rb +++ b/lib/customer_plan.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "forwardable" +require "value_semantics/monkey_patched" require_relative "em" require_relative "plan" @@ -8,21 +9,24 @@ require_relative "plan" class CustomerPlan extend Forwardable - attr_reader :expires_at, :auto_top_up_amount, :monthly_overage_limit, - :parent_customer_id - - def_delegator :@plan, :name, :plan_name - def_delegators :@plan, :currency, :merchant_account, :monthly_price, + def_delegator :plan, :name, :plan_name + def_delegators :plan, :currency, :merchant_account, :monthly_price, :minute_limit, :message_limit - def self.for(customer_id, plan_name: nil, **kwargs) - new(customer_id, plan: plan_name&.then(&Plan.method(:for)), **kwargs) + value_semantics do + customer_id String + plan Anything(), default: nil, coerce: true + expires_at Either(Time, nil), default_generator: -> { Time.now } + auto_top_up_amount Integer, default: 0 + monthly_overage_limit Integer, default: 0 + pending Bool(), default: false + parent_customer_id Either(String, nil), default: nil end def self.default(customer_id, jid) config = CONFIG[:parented_domains][Blather::JID.new(jid).domain] if config - self.for( + new( customer_id, plan_name: config[:plan_name], parent_customer_id: config[:customer_id] @@ -33,7 +37,7 @@ class CustomerPlan end def self.extract(customer_id, **kwargs) - self.for( + new( customer_id, **kwargs.slice( :plan_name, :expires_at, :parent_customer_id, :pending, @@ -42,46 +46,32 @@ class CustomerPlan ) end - def initialize( - customer_id, - plan: nil, - expires_at: Time.now, - auto_top_up_amount: 0, - monthly_overage_limit: 0, - pending: false, parent_customer_id: nil - ) - @customer_id = customer_id - @plan = plan || OpenStruct.new - @expires_at = expires_at - @auto_top_up_amount = auto_top_up_amount || 0 - @monthly_overage_limit = monthly_overage_limit || 0 - @pending = pending - @parent_customer_id = parent_customer_id + def self.coerce_plan(plan_or_name_or_nil) + return OpenStruct.new unless plan_or_name_or_nil + + Plan.for(plan_or_name_or_nil) + end + + def initialize(customer_id=nil, **kwargs) + kwargs[:plan] = kwargs.delete(:plan_name) if kwargs.key?(:plan_name) + super(customer_id ? kwargs.merge(customer_id: customer_id) : kwargs) end def active? - plan_name && @expires_at > Time.now + plan_name && expires_at > Time.now end def status return :active if active? - return :pending if @pending + return :pending if pending :expired end - def with_plan_name(plan_name, **kwargs) - self.class.new( - @customer_id, - plan: Plan.for(plan_name), - expires_at: @expires_at, **kwargs - ) - end - def verify_parent! - return unless @parent_customer_id + return unless parent_customer_id - result = DB.query(<<~SQL, [@parent_customer_id]) + result = DB.query(<<~SQL, [parent_customer_id]) SELECT plan_name FROM customer_plans WHERE customer_id=$1 SQL @@ -93,7 +83,7 @@ class CustomerPlan def save_plan! verify_parent! - DB.exec_defer(<<~SQL, [@customer_id, plan_name, @parent_customer_id]) + DB.exec_defer(<<~SQL, [customer_id, plan_name, parent_customer_id]) INSERT INTO plan_log (customer_id, plan_name, parent_customer_id, date_range) VALUES ( @@ -122,7 +112,7 @@ class CustomerPlan def activate_plan_starting_now verify_parent! - activated = DB.exec(<<~SQL, [@customer_id, plan_name, @parent_customer_id]) + activated = DB.exec(<<~SQL, [customer_id, plan_name, parent_customer_id]) INSERT INTO plan_log (customer_id, plan_name, date_range, parent_customer_id) VALUES ($1, $2, tsrange(LOCALTIMESTAMP, LOCALTIMESTAMP + '1 month'), $3) ON CONFLICT DO NOTHING @@ -130,7 +120,7 @@ class CustomerPlan activated = activated.cmd_tuples.positive? return false unless activated - DB.exec(<<~SQL, [@customer_id]) + DB.exec(<<~SQL, [customer_id]) DELETE FROM plan_log WHERE customer_id=$1 AND date_range << '[now,now]' AND upper(date_range) - lower(date_range) < '2 seconds' SQL @@ -141,22 +131,24 @@ class CustomerPlan end def activation_date - DB.query_one(<<~SQL, @customer_id).then { |r| r[:start_date] } + DB.query_one(<<~SQL, customer_id).then { |r| r[:start_date] } SELECT MIN(LOWER(date_range)) AS start_date FROM plan_log WHERE customer_id = $1; SQL end + protected :customer_id, :plan, :pending, :[] + protected def charge_for_plan(note) - raise "No plan setup" unless @plan + raise "No plan setup" unless plan params = [ - @customer_id, - "#{@customer_id}-bill-#{plan_name}-at-#{Time.now.to_i}", - -@plan.monthly_price, + customer_id, + "#{customer_id}-bill-#{plan_name}-at-#{Time.now.to_i}", + -plan.monthly_price, note ] DB.exec(<<~SQL, params) @@ -167,7 +159,7 @@ protected end def add_one_month_to_current_plan - DB.exec(<<~SQL, [@customer_id]) + DB.exec(<<~SQL, [customer_id]) UPDATE plan_log SET date_range=range_merge( date_range, tsrange( diff --git a/lib/plan.rb b/lib/plan.rb index 2db0dad2b4fae843461047ab60b30e5427e0b81f..46671495f37d7949d7c91375de58133f1d88ad09 100644 --- a/lib/plan.rb +++ b/lib/plan.rb @@ -2,6 +2,8 @@ class Plan def self.for(plan_name) + return plan_name if plan_name.is_a?(Plan) + plan = CONFIG[:plans].find { |p| p[:name] == plan_name } raise "No plan by that name" unless plan