From 230ed1cb3e3013ec871e98b659d5b3f267455319 Mon Sep 17 00:00:00 2001 From: Amolith Date: Thu, 4 Jun 2026 18:12:21 -0600 Subject: [PATCH 1/3] Improve data limit UX Show the default data limit instead of a blank value and include range/options metadata so clients can render a stepped slider. Check submitted values against the price of a 5GB top-up in the customer's currency and cap them at a fixed multiplier. --- config-schema.dhall | 4 +- config.dhall.sample | 2 + forms/plan_settings.rb | 9 +++- lib/monthly_data_limit.rb | 40 +++++++++++++++++ lib/sim_pricing.rb | 17 +++++++ sgx_jmp.rb | 11 ++++- test/test_helper.rb | 7 ++- test/test_monthly_data_limit.rb | 79 +++++++++++++++++++++++++++++++++ test/test_plan_settings_form.rb | 73 ++++++++++++++++++++++++++++++ 9 files changed, 237 insertions(+), 5 deletions(-) create mode 100644 lib/monthly_data_limit.rb create mode 100644 lib/sim_pricing.rb create mode 100644 test/test_monthly_data_limit.rb create mode 100644 test/test_plan_settings_form.rb diff --git a/config-schema.dhall b/config-schema.dhall index 4c2f69a0f5c2d8d1b3b41e61892cc45295688ccf..805b538cc41171a503f343dc18d6c18a2362edfb 100644 --- a/config-schema.dhall +++ b/config-schema.dhall @@ -81,7 +81,9 @@ } , simpleswap_api_key : Text , sims : - { esim : + { CAD : { per_gb : Natural } + , USD : { per_gb : Natural } + , esim : List { mapKey : < CAD | USD > , mapValue : { plan : Text, price : Natural } diff --git a/config.dhall.sample b/config.dhall.sample index 010940054b8020dd7f9ecddda2eec0cb49780b09..08b23cfdfb811a64c42c7d9f83ecdc4b6f50e88a 100644 --- a/config.dhall.sample +++ b/config.dhall.sample @@ -67,6 +67,8 @@ in } ], sims = { + CAD = { per_gb = 100 }, + USD = { per_gb = 100 }, sim = [ { mapKey = .CAD, mapValue = { price = 1, plan = "$1 / GB + $1 / year" } } ], diff --git a/forms/plan_settings.rb b/forms/plan_settings.rb index ea28ed24ff759b15a77b891b918af69e30d5dd99..557d171afe60dcc6bc2b7e6c50d91b7ecf351870 100644 --- a/forms/plan_settings.rb +++ b/forms/plan_settings.rb @@ -21,6 +21,11 @@ field( ) if @sims && !@sims.empty? + monthly_data_limit_options = + (0..@monthly_data_limit_max).step(@monthly_data_limit_step).map { |value| + { value: value.to_s } + } + field( var: "monthly_data_limit", type: "text-single", @@ -28,6 +33,8 @@ if @sims && !@sims.empty? label: "Dollars of data charges to allow each month", description: "0 means you will never be automatically charged", - value: @data_limit + value: (@data_limit || @monthly_data_limit_step).to_s, + range: (0..@monthly_data_limit_max), + options: monthly_data_limit_options ) end diff --git a/lib/monthly_data_limit.rb b/lib/monthly_data_limit.rb new file mode 100644 index 0000000000000000000000000000000000000000..503402a8c49500f331c214b3160085013a454e96 --- /dev/null +++ b/lib/monthly_data_limit.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require_relative "sim_pricing" + +module MonthlyDataLimit + MAX_MULTIPLIER = 10 + + def self.for_sims(sims, currency, config) + sims.empty? ? {} : for_currency(currency, config) + end + + def self.for_currency(currency, config) + step = SIMPricing.whole_dollars( + SIMPricing.five_gb_refill_price(currency, config) + ) + { + step: step, + max: step * MAX_MULTIPLIER + } + end + + def self.from_form(form, step:, max:) + limit = form.field("monthly_data_limit")&.value + return if limit.to_s.empty? + + parse(limit, step: step, max: max) + end + + def self.parse(limit, step:, max:) + limit = Integer(limit, 10) + unless (0..max).cover?(limit) + raise "Monthly data limit must be between 0 and #{max}" + end + unless (limit % step).zero? + raise "Monthly data limit must be a multiple of #{step}" + end + + limit + end +end diff --git a/lib/sim_pricing.rb b/lib/sim_pricing.rb new file mode 100644 index 0000000000000000000000000000000000000000..cc4005d778b60349cdb37bae390fd3a66cbb184b --- /dev/null +++ b/lib/sim_pricing.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require "bigdecimal" + +module SIMPricing + # SIM refill billing records currency amounts in transactions. Use BigDecimal + # because the configured per-GB price is stored in cents. + def self.five_gb_refill_price(currency, config) + (BigDecimal(config[:sims][currency][:per_gb]) / 100) * 5 + end + + # CustomerRepo#put_monthly_limits stores limits with to_i, so render monthly + # data limit form steps in that same integer-dollar unit. + def self.whole_dollars(price) + price.to_i + end +end diff --git a/sgx_jmp.rb b/sgx_jmp.rb index b63d462b75cdb99d4028294e87254cf9faef7cec..84cdc9f835ee67c5c26c772899a8818e5607382a 100644 --- a/sgx_jmp.rb +++ b/sgx_jmp.rb @@ -93,6 +93,7 @@ require_relative "lib/expiring_lock" require_relative "lib/em" require_relative "lib/form_to_h" require_relative "lib/low_balance" +require_relative "lib/monthly_data_limit" require_relative "lib/port_in_order" require_relative "lib/patches_for_sentry" require_relative "lib/payment_methods" @@ -709,15 +710,21 @@ Command.new( SIMRepo.new.owned_by(customer) ]).then { |(limit, sims)| [customer, sims, limit] } }.then do |(customer, sims, limit)| + limits = MonthlyDataLimit.for_sims(sims, customer.currency, CONFIG) Command.reply { |reply| reply.allowed_actions = [:next] reply.command << FormTemplate.render( - "plan_settings", customer: customer, sims: sims, data_limit: limit + "plan_settings", + customer: customer, sims: sims, data_limit: limit, + monthly_data_limit_step: limits[:step], + monthly_data_limit_max: limits[:max] ) }.then { |iq| kwargs = { monthly_overage_limit: iq.form.field("monthly_overage_limit")&.value, - monthly_data_limit: iq.form.field("monthly_data_limit")&.value + monthly_data_limit: ( + MonthlyDataLimit.from_form(iq.form, **limits) unless limits.empty? + ) }.compact Command.execution.customer_repo.put_monthly_limits(customer, **kwargs) }.then { Command.finish("Configuration saved!") } diff --git a/test/test_helper.rb b/test/test_helper.rb index 3387525acdb34ad37add0f2fb2bbf174effda6a8..f2b7c34f7acbacb87e9d32331d5d0d78a7742cf0 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -132,7 +132,10 @@ CONFIG = { { name: "test_cad", currency: :CAD, - monthly_price: 10000 + monthly_price: 10000, + messages: :unlimited, + minutes: { included: 10440, price: 87 }, + allow_register: true }, { name: "test_usd_old_billing", @@ -158,6 +161,8 @@ CONFIG = { electrum_notify_url: ->(*) { "http://notify.example.com" }, admin_notify: "admin_room@example.com", sims: { + USD: { per_gb: 100 }, + CAD: { per_gb: 120 }, sim: { USD: { price: 500, plan: "1GB" }, CAD: { price: 600, plan: "1GB" } diff --git a/test/test_monthly_data_limit.rb b/test/test_monthly_data_limit.rb new file mode 100644 index 0000000000000000000000000000000000000000..354e0001b9530a7b0446fad3aa14ceb08605fc60 --- /dev/null +++ b/test/test_monthly_data_limit.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require "test_helper" +require "monthly_data_limit" + +class MonthlyDataLimitTest < Minitest::Test + def parse(limit) + MonthlyDataLimit.parse(limit, step: 5, max: 50) + end + + def test_accepts_multiple_of_five + assert_equal 10, parse("10") + end + + def test_accepts_zero + assert_equal 0, parse("0") + end + + def test_accepts_maximum + assert_equal 50, parse("50") + end + + def test_ignores_blank_form_field + form = Struct.new(:value) do + def field(*) + self + end + end.new("") + + assert_nil MonthlyDataLimit.from_form(form, step: 5, max: 50) + end + + def test_rejects_non_multiple_of_five + error = assert_raises(RuntimeError) { parse("11") } + + assert_equal "Monthly data limit must be a multiple of 5", error.message + end + + def test_rejects_non_multiple_of_currency_step + error = assert_raises(RuntimeError) { + MonthlyDataLimit.parse("10", step: 6, max: 60) + } + + assert_equal "Monthly data limit must be a multiple of 6", error.message + end + + def test_rejects_negative_values + error = assert_raises(RuntimeError) { parse("-5") } + + assert_equal "Monthly data limit must be between 0 and 50", error.message + end + + def test_rejects_values_above_maximum + error = assert_raises(RuntimeError) { parse("55") } + + assert_equal "Monthly data limit must be between 0 and 50", error.message + end + + def test_uses_sim_refill_price_for_step + limits = MonthlyDataLimit.for_currency(:CAD, CONFIG) + + assert_equal 6, limits[:step] + assert_equal 60, limits[:max] + end + + def test_does_not_need_sim_pricing_without_sims + assert_equal({}, MonthlyDataLimit.for_sims([], :CAD, CONFIG)) + end + + def test_uses_integer_dollars_for_fractional_refill_price + config = CONFIG.merge( + sims: CONFIG[:sims].merge(CAD: { per_gb: 125 }) + ) + limits = MonthlyDataLimit.for_currency(:CAD, config) + + assert_equal 6, limits[:step] + assert_equal 60, limits[:max] + end +end diff --git a/test/test_plan_settings_form.rb b/test/test_plan_settings_form.rb new file mode 100644 index 0000000000000000000000000000000000000000..4ff925a35fa041468f564210fc60df41a5484cc4 --- /dev/null +++ b/test/test_plan_settings_form.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require "test_helper" +require "customer" +require "form_template" +require "monthly_data_limit" + +class PlanSettingsFormTest < Minitest::Test + def render(data_limit: nil, plan_name: "test_usd") + customer = customer(plan_name: plan_name) + limits = MonthlyDataLimit.for_currency(customer.currency, CONFIG) + FormTemplate.render( + "plan_settings", + customer: customer, + sims: [OpenStruct.new], + data_limit: data_limit, + monthly_data_limit_step: limits[:step], + monthly_data_limit_max: limits[:max] + ) + end + + def test_monthly_data_limit_is_integer_field + field = render.field("monthly_data_limit") + validate = field.find( + "ns:validate", + ns: "http://jabber.org/protocol/xdata-validate" + ).first + + assert_equal "text-single", field.type + assert_equal "xs:integer", validate[:datatype] + end + + def test_monthly_data_limit_has_slider_range + validate = render.field("monthly_data_limit").find( + "ns:validate", + ns: "http://jabber.org/protocol/xdata-validate" + ).first + range = validate.children.first + + assert_equal "range", range.name + assert_equal "0", range[:min] + assert_equal "50", range[:max] + end + + def test_monthly_data_limit_options_step_by_five_gb_cost + options = render.field("monthly_data_limit").options.map(&:value) + + assert_equal (0..50).step(5).map(&:to_s), options + end + + def test_monthly_data_limit_defaults_to_one_five_gb_step + assert_equal "5", render.field("monthly_data_limit").value + end + + def test_monthly_data_limit_uses_customer_currency + field = render(plan_name: "test_cad").field("monthly_data_limit") + validate = field.find( + "ns:validate", + ns: "http://jabber.org/protocol/xdata-validate" + ).first + range = validate.children.first + + assert_equal "60", range[:max] + assert_equal "6", field.value + assert_equal (0..60).step(6).map(&:to_s), field.options.map(&:value) + end + + def test_monthly_data_limit_keeps_existing_value + field = render(data_limit: "10").field("monthly_data_limit") + + assert_equal "10", field.value + end +end From 07283776f5fa02f27792a44835735b362bc80fef Mon Sep 17 00:00:00 2001 From: Amolith Date: Thu, 4 Jun 2026 18:13:27 -0600 Subject: [PATCH 2/3] Flatten main SIM pricing config Replace the main app's SIM pricing maps with fixed CAD/USD records for SIM, eSIM, per-GB refill, and annual prices. Derive the displayed plan label from the per-GB and annual prices so the one-time SIM prices and ongoing plan pricing are not duplicated in config. --- config-schema.dhall | 16 ++++------------ config.dhall.sample | 12 ++++-------- lib/sim_kind.rb | 23 ++++++++++++++--------- lib/sim_pricing.rb | 28 +++++++++++++++++++++++++++- sgx_jmp.rb | 6 ++---- test/test_helper.rb | 18 ++++++++++++------ test/test_monthly_data_limit.rb | 15 ++++++++++++++- 7 files changed, 77 insertions(+), 41 deletions(-) diff --git a/config-schema.dhall b/config-schema.dhall index 805b538cc41171a503f343dc18d6c18a2362edfb..b6086665413504b4b65634cdc877a3951662a234 100644 --- a/config-schema.dhall +++ b/config-schema.dhall @@ -81,18 +81,10 @@ } , simpleswap_api_key : Text , sims : - { CAD : { per_gb : Natural } - , USD : { per_gb : Natural } - , esim : - List - { mapKey : < CAD | USD > - , mapValue : { plan : Text, price : Natural } - } - , sim : - List - { mapKey : < CAD | USD > - , mapValue : { plan : Text, price : Natural } - } + { annual : { CAD : Natural, USD : Natural } + , esim : { CAD : Natural, USD : Natural } + , per_gb : { CAD : Natural, USD : Natural } + , sim : { CAD : Natural, USD : Natural } } , sip : { app : Text, realm : Text } , sip_host : Text diff --git a/config.dhall.sample b/config.dhall.sample index 08b23cfdfb811a64c42c7d9f83ecdc4b6f50e88a..3d7a39b0fa3ebc95ed0c07eb21cb7d484dacfba0 100644 --- a/config.dhall.sample +++ b/config.dhall.sample @@ -67,14 +67,10 @@ in } ], sims = { - CAD = { per_gb = 100 }, - USD = { per_gb = 100 }, - sim = [ - { mapKey = .CAD, mapValue = { price = 1, plan = "$1 / GB + $1 / year" } } - ], - esim = [ - { mapKey = .CAD, mapValue = { price = 1, plan = "$1 / GB + $1 / year" } } - ] + annual = { CAD = 100, USD = 100 }, + esim = { CAD = 1, USD = 1 }, + per_gb = { CAD = 100, USD = 100 }, + sim = { CAD = 1, USD = 1 } }, electrum = { rpc_uri = "", diff --git a/lib/sim_kind.rb b/lib/sim_kind.rb index 553552ffd8c5fc6dc4e00b2c4afd32036f6eece4..c232bd1397917efe595c23af895c8508016657ca 100644 --- a/lib/sim_kind.rb +++ b/lib/sim_kind.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative "sim_pricing" + class SIMKind def initialize(variant) @klass = @@ -19,13 +21,16 @@ class SIMKind end # @param [Customer] customer - # @raise [ArgumentError] If matching `CONFIG[:sims]` - # key not found for `@variant` and customer + # @raise [ArgumentError] If matching SIM price is missing from `CONFIG[:sims]` def get_processor(customer) - cfg = cfg(customer.currency) - raise ArgumentError, "Invalid config" unless cfg - - @klass.for(customer, **cfg) + price = price_cents(customer.currency) + raise ArgumentError, "Invalid config" unless price + + @klass.for( + customer, + price: price, + plan: SIMPricing.plan_label(customer.currency, CONFIG) + ) end # @return [String] The result of either @@ -38,15 +43,15 @@ class SIMKind # @return [Float, NilClass] Returns nil if `customer.currency` # is nil, or if @variant is malformed. def price(customer) - cfg = cfg(customer.currency || :USD) - cfg[:price] / 100.to_d + price = price_cents(customer.currency || :USD) + price / 100.to_d if price end protected # @param [Symbol] currency One of the currencies from # CONFIG[:plans] entries - def cfg(currency) + def price_cents(currency) CONFIG.dig(:sims, @variant.to_sym, currency) end end diff --git a/lib/sim_pricing.rb b/lib/sim_pricing.rb index cc4005d778b60349cdb37bae390fd3a66cbb184b..9374187c26027be9bddbfbbbc7186310b12abcd2 100644 --- a/lib/sim_pricing.rb +++ b/lib/sim_pricing.rb @@ -5,8 +5,34 @@ require "bigdecimal" module SIMPricing # SIM refill billing records currency amounts in transactions. Use BigDecimal # because the configured per-GB price is stored in cents. + def self.per_gb_price(currency, config) + configured_price_dollars(:per_gb, currency, config) + end + + def self.annual_price(currency, config) + configured_price_dollars(:annual, currency, config) + end + + def self.configured_price_dollars(price_type, currency, config) + BigDecimal( + config.fetch(:sims).fetch(price_type).fetch(currency) + ) / 100 + end + private_class_method :configured_price_dollars + def self.five_gb_refill_price(currency, config) - (BigDecimal(config[:sims][currency][:per_gb]) / 100) * 5 + per_gb_price(currency, config) * 5 + end + + def self.per_gb_label(currency, config) + "$%.2f / GB" % per_gb_price(currency, config) + end + + def self.plan_label(currency, config) + "$%.2f / GB + $%.2f / year" % [ + per_gb_price(currency, config), + annual_price(currency, config) + ] end # CustomerRepo#put_monthly_limits stores limits with to_i, so render monthly diff --git a/sgx_jmp.rb b/sgx_jmp.rb index 84cdc9f835ee67c5c26c772899a8818e5607382a..0d0c056cb1095fea3ab69908dfd55e46d5925fac 100644 --- a/sgx_jmp.rb +++ b/sgx_jmp.rb @@ -783,11 +783,9 @@ Command.new( }.then { |iq| case iq.form.field("http://jabber.org/protocol/commands#actions")&.value when "order-sim" - SIMOrder.for(customer, **CONFIG.dig(:sims, :sim, customer.currency)) + SIMKind.new("sim").get_processor(customer) when "order-esim" - SIMOrder::ESIM.for( - customer, **CONFIG.dig(:sims, :esim, customer.currency) - ) + SIMKind.new("esim").get_processor(customer) when "edit-nicknames" EditSimNicknames.new(customer, sims) else diff --git a/test/test_helper.rb b/test/test_helper.rb index f2b7c34f7acbacb87e9d32331d5d0d78a7742cf0..cca7599c3535f814c1463e8ac30636981b9f4df9 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -161,15 +161,21 @@ CONFIG = { electrum_notify_url: ->(*) { "http://notify.example.com" }, admin_notify: "admin_room@example.com", sims: { - USD: { per_gb: 100 }, - CAD: { per_gb: 120 }, + annual: { + USD: 100, + CAD: 120 + }, + per_gb: { + USD: 100, + CAD: 120 + }, sim: { - USD: { price: 500, plan: "1GB" }, - CAD: { price: 600, plan: "1GB" } + USD: 500, + CAD: 600 }, esim: { - USD: { price: 300, plan: "500MB" }, - CAD: { price: 400, plan: "500MB" } + USD: 300, + CAD: 400 } }, keep_area_codes: [ diff --git a/test/test_monthly_data_limit.rb b/test/test_monthly_data_limit.rb index 354e0001b9530a7b0446fad3aa14ceb08605fc60..7d4ac2f96276565663d8e0db09c15781add7f3a6 100644 --- a/test/test_monthly_data_limit.rb +++ b/test/test_monthly_data_limit.rb @@ -63,13 +63,26 @@ class MonthlyDataLimitTest < Minitest::Test assert_equal 60, limits[:max] end + def test_formats_refill_price_for_display + assert_equal "$1.20 / GB", SIMPricing.per_gb_label(:CAD, CONFIG) + end + + def test_formats_plan_price_for_display + assert_equal( + "$1.20 / GB + $1.20 / year", + SIMPricing.plan_label(:CAD, CONFIG) + ) + end + def test_does_not_need_sim_pricing_without_sims assert_equal({}, MonthlyDataLimit.for_sims([], :CAD, CONFIG)) end def test_uses_integer_dollars_for_fractional_refill_price config = CONFIG.merge( - sims: CONFIG[:sims].merge(CAD: { per_gb: 125 }) + sims: CONFIG[:sims].merge( + per_gb: CONFIG[:sims][:per_gb].merge(CAD: 125) + ) ) limits = MonthlyDataLimit.for_currency(:CAD, config) From 5b038160f74a757b33e12bd19d92b5faf1d9056d Mon Sep 17 00:00:00 2001 From: Amolith Date: Thu, 4 Jun 2026 18:14:03 -0600 Subject: [PATCH 3/3] Flatten SIM job pricing config Update the SIM job's inline Dhall schema to use fixed CAD/USD records for the annual and per-GB prices it reads. --- bin/sim_job | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bin/sim_job b/bin/sim_job index 72d6be793f640b4152439baf857010276cb9b438..7a4ef2486ddfe2f3ecf0e49e1297f2b8f566bd67 100755 --- a/bin/sim_job +++ b/bin/sim_job @@ -32,8 +32,8 @@ SCHEMA = "{ component: { jid: Text }, keepgo: Optional { access_token: Text, api_key: Text }, sims: { - CAD: { per_gb: Natural, annual: Natural }, - USD: { per_gb: Natural, annual: Natural } + annual: { CAD: Natural, USD: Natural }, + per_gb: { CAD: Natural, USD: Natural } }, plans: List { currency: < CAD | USD >, @@ -69,6 +69,7 @@ require_relative "../lib/expiring_lock" require_relative "../lib/low_balance" require_relative "../lib/postgres" require_relative "../lib/sim_repo" +require_relative "../lib/sim_pricing" require_relative "../lib/transaction" BRAINTREE = AsyncBraintree.new(**CONFIG[:braintree]) @@ -107,7 +108,7 @@ module SimAction end def refill_price - (BigDecimal(CONFIG[:sims][customer.currency][:per_gb]) / 100) * 5 + SIMPricing.five_gb_refill_price(customer.currency, CONFIG) end def refill_and_bill(data, price, note) @@ -206,7 +207,7 @@ class SimAnnual end def annual_price - BigDecimal(CONFIG[:sims][customer.currency][:annual]) / 100 + SIMPricing.annual_price(customer.currency, CONFIG) end def call