From 098049a7d6a91f479fe383a183a335db7d8c7279 Mon Sep 17 00:00:00 2001 From: Phillip Davis Date: Wed, 18 Feb 2026 09:32:28 -0500 Subject: [PATCH] fix/regression: support 'migrate billing' - `MigrateBilling` passes a string for a tel, previously causing NoMethodError when `Payment.for` says `product.price` - Introduces a new MigrateBilling class in lib so that unit tests capture any changes that break this in the future - Edit .rubocop.yml to exclude `when_ready` from `Lint/ConstantDefinitionInBlock`, which was raised just now because this commit touched sgx_jmp.rb . I think this is the best solution because those constants (`REDIS`, `DB`, etc.) are used as globals throughout the codebase. Since many of them require a running EM reactor, changing this would require extensive refactoring. --- .rubocop.yml | 4 ++ lib/migrate_billing.rb | 42 +++++++++++++ sgx_jmp.rb | 26 +------- test/test_helper.rb | 8 +++ test/test_migrate_billing.rb | 113 +++++++++++++++++++++++++++++++++++ 5 files changed, 169 insertions(+), 24 deletions(-) create mode 100644 lib/migrate_billing.rb create mode 100644 test/test_migrate_billing.rb diff --git a/.rubocop.yml b/.rubocop.yml index 0b470abd7d329869ce538688266e031d9e17ea5f..0b53b1fb7e8db646bbe2c6129252d6a923869636 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -122,6 +122,10 @@ Lint/OutOfRangeRegexpRef: Lint/MissingSuper: Enabled: false +Lint/ConstantDefinitionInBlock: + AllowedMethods: + - when_ready + Style/BlockDelimiters: EnforcedStyle: semantic AllowBracesOnProceduralOneLiners: true diff --git a/lib/migrate_billing.rb b/lib/migrate_billing.rb new file mode 100644 index 0000000000000000000000000000000000000000..3b3c31505222d6fe5a57e61b400774f1660357c4 --- /dev/null +++ b/lib/migrate_billing.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require_relative "./paypal_done" + +class MigrateBilling + def initialize(customer, finish: PaypalDone) + @customer = customer + @finish = finish + end + + def parse(iq) + plan_name = iq.form.field("plan_name").value.to_s + (@customer = @customer.with_plan(plan_name)).save_plan!.then { + Registration::Payment.for( + iq, @customer, + OpenStruct.new(price: 0, to_s: @customer.registered?.phone), + finish: @finish + ) + }.then(&:write) + end + + def msg + "#{@customer.customer_id} migrated to #{@customer.currency}" + end + + def write + Command.reply(&method(:form)).then(&method(:parse)).catch_only( + Command::Execution::FinalStanza + ) do |s| + BLATHER.join(CONFIG[:notify_admin], "sgx-jmp") + BLATHER.say(CONFIG[:notify_admin], msg, :groupchat) + EMPromise.reject(s) + end + end + +protected + + def form(stanza) + stanza.allowed_actions = [:next] + stanza.command << FormTemplate.render("migrate_billing") + end +end diff --git a/sgx_jmp.rb b/sgx_jmp.rb index 72552f0d2235a15f9f66a5e3e881a3c9bbe1ffd2..7aaef894ec29d0195356f2e4776aa6294c80cf45 100644 --- a/sgx_jmp.rb +++ b/sgx_jmp.rb @@ -96,6 +96,7 @@ require_relative "lib/port_in_order" require_relative "lib/patches_for_sentry" require_relative "lib/payment_methods" require_relative "lib/paypal_done" +require_relative "lib/migrate_billing" require_relative "lib/postgres" require_relative "lib/reachability_form" require_relative "lib/reachability_repo" @@ -616,30 +617,7 @@ Command.new( list_for: ->(tel:, customer:, **) { tel && !customer&.currency }, customer_repo: CustomerRepo.new(sgx_repo: Bwmsgsv2Repo.new) ) { - EMPromise.all([ - Command.customer, - Command.reply do |reply| - reply.allowed_actions = [:next] - reply.command << FormTemplate.render("migrate_billing") - end - ]).then do |(customer, iq)| - plan_name = iq.form.field("plan_name").value.to_s - customer = customer.with_plan(plan_name) - customer.save_plan!.then { - Registration::Payment.for( - iq, customer, customer.registered?.phone, - finish: PaypalDone - ) - }.then(&:write).catch_only(Command::Execution::FinalStanza) do |s| - BLATHER.join(CONFIG[:notify_admin], "sgx-jmp") - BLATHER.say( - CONFIG[:notify_admin], - "#{customer.customer_id} migrated to #{customer.currency}", - :groupchat - ) - EMPromise.reject(s) - end - end + Command.customer.then(&MigrateBilling.method(:new)).then(&:write) }.register(self).then(&CommandList.method(:register)) Command.new( diff --git a/test/test_helper.rb b/test/test_helper.rb index a424f07f0b047ccea604e08e5af55f0f71b86978..ac1946ed4319cf25f95108c611b623f764864a5f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -131,6 +131,13 @@ CONFIG = { name: "test_cad", currency: :CAD, monthly_price: 10000 + }, + { + name: "test_usd_old_billing", + currency: nil, + messages: :unlimited, + minutes: { included: 10440, price: 87 }, + allow_register: false } ], braintree: { @@ -147,6 +154,7 @@ CONFIG = { ], credit_card_url: ->(*) { "http://creditcard.example.com?" }, electrum_notify_url: ->(*) { "http://notify.example.com" }, + admin_notify: "admin_room@example.com", sims: { sim: { USD: { price: 500, plan: "1GB" }, diff --git a/test/test_migrate_billing.rb b/test/test_migrate_billing.rb new file mode 100644 index 0000000000000000000000000000000000000000..38a3a78f81ed715f536d4ceb30f2ec0ecee79e8d --- /dev/null +++ b/test/test_migrate_billing.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require "minitest" + +require "test_helper" + +require "paypal_done" +require "migrate_billing" + +Command::COMMAND_MANAGER = Minitest::Mock.new +CustomerPlan::DB = Minitest::Mock.new +MigrateBilling::BLATHER = Minitest::Mock.new + +class TestMigrateBilling < Minitest::Test + def setup + @sgx = Minitest::Mock.new + end + + def test_write_registered + execute_command do |exe| + Command::COMMAND_MANAGER.expect( + :write, + EMPromise.resolve(Blather::Stanza::Iq::Command.new.tap { |iq| + iq.form.fields = [ + { var: "activation_method", value: "credit_card" }, + { var: "plan_name", value: "test_usd" } + ] + iq.from = "test@example.com" + }), + [Matching.new { |iq| + assert_equal :form, iq.form.type + assert iq.form.instructions.include?("legacy PayPal") + }] + ) + + CustomerPlan::DB.expect( + :exec_defer, + nil, + [ + String, + Matching.new do |(customer_id, plan_name, parent_customer_id)| + assert_equal "test", customer_id + assert_equal "test_usd", plan_name + assert_nil parent_customer_id + end + ] + ) + + Command::COMMAND_MANAGER.expect( + :write, + EMPromise.resolve(Blather::Stanza::Iq::Command.new.tap { |iq| + iq.action = :next + }), + [Matching.new { |iq| + assert_equal :executing, iq.status + assert_equal( + "http://creditcard.example.com?&amount=1", + OOB.find_or_create(iq.command).url + ) + }] + ) + + @sgx.expect( + :registered?, + OpenStruct.new(phone: "+15550000") + ) + + exe.customer_repo.expect( + :find, + EMPromise.resolve( + customer( + plan_name: "test_usd", + sgx: @sgx + ).with(balance: 100) + ), + ["test"] + ) + # Apparently the constant assignment from + # `RegistrationTest::InviteCodeTest` pollutes this + # test + Registration::Payment::MaybeBill::BillPlan.expect( + :new, + PaypalDone.new + ) { |*| true } + + MigrateBilling::BLATHER.expect( + :join, + nil, + [CONFIG[:notify_admin], "sgx-jmp"] + ) + MigrateBilling::BLATHER.expect( + :say, + nil, + [ + CONFIG[:notify_admin], + "test migrated to USD", + :groupchat + ] + ) + + cust = customer(plan_name: "test_usd_old_billing", sgx: @sgx) + result = MigrateBilling.new(cust).write.catch { |e| + e + }.sync + + assert_equal(result.stanza.note.text, PaypalDone::MESSAGE) + assert_mock Registration::Payment::MaybeBill::BillPlan + assert_mock Command::COMMAND_MANAGER + assert_mock @sgx + end + end + em :test_write_registered +end