From 230ed1cb3e3013ec871e98b659d5b3f267455319 Mon Sep 17 00:00:00 2001 From: Amolith Date: Thu, 4 Jun 2026 18:12:21 -0600 Subject: [PATCH] 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