From 5f30fd792a817d2b053f66b97b1b060cea27c6b4 Mon Sep 17 00:00:00 2001 From: Christopher Vollick <0@psycoti.ca> Date: Thu, 5 May 2022 16:41:05 -0400 Subject: [PATCH 1/6] No Settled Transactions is 0, not Null Unexpectedly this doesn't return either no columns or a column with a value of 0, but instead a column with no value... --- lib/trust_level_repo.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/trust_level_repo.rb b/lib/trust_level_repo.rb index 61e9925483a5e835e7a7494163362b2aee62a7b2..74fbecbc8df820f3f34e4e97529ceff49647b264 100644 --- a/lib/trust_level_repo.rb +++ b/lib/trust_level_repo.rb @@ -27,7 +27,7 @@ protected def fetch_settled_amount(customer_id) db.query_one(<<~SQL, customer_id, default: {}) - SELECT SUM(amount) AS settled_amount FROM transactions + SELECT COALESCE(SUM(amount), 0) AS settled_amount FROM transactions WHERE customer_id=$1 AND settled_after < LOCALTIMESTAMP AND amount > 0 SQL end From fd7951d8f25db86a0345ab0ac1869f0f7bb0af5d Mon Sep 17 00:00:00 2001 From: Christopher Vollick <0@psycoti.ca> Date: Thu, 5 May 2022 16:41:06 -0400 Subject: [PATCH 2/6] PromiseHash Here in info I'm doing a lot of this pattern: EMPromise.all([one, two three]).then { |one, two, three| new(one: one, two: two, three: three) } Which felt really noisy for what is a pretty logical operation. So now I can keep the keys and values together and do: PromiseHash.all(one: one, two: two, three:three).then(&method(:new)) It's smaller, but more importantly it's more meaningful to me. It keeps the declaration of the meaning together with the sourcing of the value. And because it uses EMPromise.all in the guts it's not a problem if some of the keys are not promises. Easy-peasy! --- lib/customer_info.rb | 32 ++++++++++++++------------------ lib/promise_hash.rb | 12 ++++++++++++ 2 files changed, 26 insertions(+), 18 deletions(-) create mode 100644 lib/promise_hash.rb diff --git a/lib/customer_info.rb b/lib/customer_info.rb index ebe13a8c90f6ad6a100e614f70354d21cfda3193..353f59dfce318afe29f76c952543cf286bf3b126 100644 --- a/lib/customer_info.rb +++ b/lib/customer_info.rb @@ -7,6 +7,7 @@ require "value_semantics/monkey_patched" require_relative "proxied_jid" require_relative "customer_plan" require_relative "form_template" +require_relative "promise_hash" class PlanInfo extend Forwardable @@ -78,14 +79,12 @@ class CustomerInfo end def self.for(customer, plan) - PlanInfo.for(plan).then do |plan_info| - new( - plan_info: plan_info, - tel: customer.registered? ? customer.registered?.phone : nil, - balance: customer.balance, - cnam: customer.tndetails.dig(:features, :lidb, :subscriber_information) - ) - end + PromiseHash.all( + plan_info: PlanInfo.for(plan), + tel: customer.registered? ? customer.registered?.phone : nil, + balance: customer.balance, + cnam: customer.tndetails.dig(:features, :lidb, :subscriber_information) + ).then(&method(:new)) end def form @@ -103,16 +102,13 @@ class AdminInfo end def self.for(customer, plan) - EMPromise.all([ - CustomerInfo.for(customer, plan), - customer.api - ]).then do |info, api_value| - new( - jid: customer.jid, - customer_id: customer.customer_id, - fwd: customer.fwd, info: info, api: api_value - ) - end + PromiseHash.all( + jid: customer.jid, + customer_id: customer.customer_id, + fwd: customer.fwd, + info: CustomerInfo.for(customer, plan), + api: customer.api + ).then(&method(:new)) end def form diff --git a/lib/promise_hash.rb b/lib/promise_hash.rb new file mode 100644 index 0000000000000000000000000000000000000000..3228497788bf86707366eef7463387edc93df808 --- /dev/null +++ b/lib/promise_hash.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require "em_promise" + +module PromiseHash + def self.all(**kwargs) + keys = kwargs.keys + EMPromise.all(kwargs.values).then { |results| + Hash[keys.zip(results)] + } + end +end From dbf8df70705bce09ae7ab67ba400b8ea918a1d84 Mon Sep 17 00:00:00 2001 From: Christopher Vollick <0@psycoti.ca> Date: Thu, 5 May 2022 16:41:07 -0400 Subject: [PATCH 3/6] Show Callability State in Customer Info This shows relatively easily which class a given user finds themselves in. Whether they can't call because they have no balance, or if they have lots of room, or if they're being asked. Hopefully this will make it easier to tell at a glance if a calling issue is due to a few things. --- forms/admin_info.rb | 6 ++++++ lib/call_attempt.rb | 21 +++++++++++++++++++-- lib/customer_info.rb | 24 +++++++++++++++++++++++- test/test_customer_info.rb | 2 ++ 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/forms/admin_info.rb b/forms/admin_info.rb index 1da597a6efb4b17c9ed2a36a3870ce1707aa6935..c655c80735ef8875ed2c1b4e57c33e013f0d61d7 100644 --- a/forms/admin_info.rb +++ b/forms/admin_info.rb @@ -45,6 +45,12 @@ if @admin_info.fwd.uri ) end +field( + var: "call_info", + label: "Call Status", + value: @admin_info.call_info +) + field( var: "api", label: "API", diff --git a/lib/call_attempt.rb b/lib/call_attempt.rb index 16546689a9221d71d591af767892e9db1475cd22..92bce9bc53c761bc4c3bd6b6bf6cdaa4a647d3f9 100644 --- a/lib/call_attempt.rb +++ b/lib/call_attempt.rb @@ -48,6 +48,10 @@ class CallAttempt ["#{direction}/connect", { locals: to_h }] end + def to_s + "Allowed(max_minutes: #{max_minutes}, limit_remaining: #{limit_remaining})" + end + def create_call(fwd, *args, &block) fwd.create_call(*args, &block) end @@ -119,6 +123,10 @@ class CallAttempt [view] end + def to_s + "Unsupported" + end + def create_call(*); end def as_json(*) @@ -135,8 +143,8 @@ class CallAttempt self.for(rate: rate, **kwargs) if credit < rate * 10 end - def self.for(customer:, direction:, **kwargs) - LowBalance.for(customer).then(&:notify!).then do |amount| + def self.for(customer:, direction:, low_balance: LowBalance, **kwargs) + low_balance.for(customer).then(&:notify!).then do |amount| if amount&.positive? CallAttempt.for( customer: customer.with_balance(customer.balance + amount), @@ -165,6 +173,10 @@ class CallAttempt [view, { locals: to_h }] end + def to_s + "NoBalance" + end + def create_call(*); end def as_json(*) @@ -211,6 +223,11 @@ class CallAttempt [view, { locals: to_h }] end + def to_s + "AtLimit(max_minutes: #{max_minutes}, "\ + "limit_remaining: #{limit_remaining})" + end + def create_call(fwd, *args, &block) fwd.create_call(*args, &block) end diff --git a/lib/customer_info.rb b/lib/customer_info.rb index 353f59dfce318afe29f76c952543cf286bf3b126..ddc39a0b97fbe4fe1acbd557a0cce952daf0cc5f 100644 --- a/lib/customer_info.rb +++ b/lib/customer_info.rb @@ -99,6 +99,7 @@ class AdminInfo fwd Either(CustomerFwd, nil) info CustomerInfo api API + call_info String end def self.for(customer, plan) @@ -107,10 +108,31 @@ class AdminInfo customer_id: customer.customer_id, fwd: customer.fwd, info: CustomerInfo.for(customer, plan), - api: customer.api + api: customer.api, + call_info: call_info(customer) ).then(&method(:new)) end + class FakeLowBalance + def self.for(_) + self + end + + def self.notify! + EMPromise.resolve(0) + end + end + + def self.call_info(customer) + if customer.registered? + CallAttemptRepo.new + .find_outbound(customer, "+1", call_id: "", low_balance: FakeLowBalance) + .then(&:to_s) + else + EMPromise.resolve("No calling") + end + end + def form FormTemplate.render("admin_info", admin_info: self) end diff --git a/test/test_customer_info.rb b/test/test_customer_info.rb index 14e371980a10cc694ba0c76b1b474904742ab32a..91239c4df51aaae9828cbd829969f139b86df5dc 100644 --- a/test/test_customer_info.rb +++ b/test/test_customer_info.rb @@ -37,6 +37,7 @@ class CustomerInfoTest < Minitest::Test sgx.expect(:registered?, false) fwd = CustomerFwd.for(uri: "tel:+12223334444", timeout: 15) sgx.expect(:fwd, fwd) + sgx.expect(:registered?, false) CustomerPlan::DB.expect( :query_one, @@ -69,6 +70,7 @@ class CustomerInfoTest < Minitest::Test def test_inactive_admin_info_does_not_crash sgx = Minitest::Mock.new sgx.expect(:registered?, false) + sgx.expect(:registered?, false) sgx.expect(:fwd, CustomerFwd::None.new(uri: nil, timeout: nil)) plan = CustomerPlan.new("test", plan: nil, expires_at: nil) From 8041a508c6d65ec0b52f4ddd3cd80b82b5e9e169 Mon Sep 17 00:00:00 2001 From: Christopher Vollick <0@psycoti.ca> Date: Thu, 5 May 2022 16:41:08 -0400 Subject: [PATCH 4/6] Trust Level in Customer Info We have this thing, so we should probably be able to see it --- forms/admin_info.rb | 6 ++++++ lib/customer.rb | 5 +++-- lib/customer_info.rb | 6 ++++-- lib/trust_level.rb | 26 +++++++++++++++++++++++++- test/test_customer_info.rb | 15 +++++++++++++-- test/test_trust_level_repo.rb | 10 +++++----- 6 files changed, 56 insertions(+), 12 deletions(-) diff --git a/forms/admin_info.rb b/forms/admin_info.rb index c655c80735ef8875ed2c1b4e57c33e013f0d61d7..a7cb70cdb328d598a2a1324fee855e870e0cfebd 100644 --- a/forms/admin_info.rb +++ b/forms/admin_info.rb @@ -51,6 +51,12 @@ field( value: @admin_info.call_info ) +field( + var: "trust_level", + label: "Trust Level", + value: @admin_info.trust_level +) + field( var: "api", label: "API", diff --git a/lib/customer.rb b/lib/customer.rb index f837ed7f4239b8a73aad26f35f8e07467addcc92..ae36f35109ea2b8f2d3659dfa1fe46b41854bbdb 100644 --- a/lib/customer.rb +++ b/lib/customer.rb @@ -111,8 +111,9 @@ class Customer API.for(self) end - def admin_info - AdminInfo.for(self, @plan) + # kwargs are passed through for dependency injection from tests + def admin_info(**kwargs) + AdminInfo.for(self, @plan, **kwargs) end def info diff --git a/lib/customer_info.rb b/lib/customer_info.rb index ddc39a0b97fbe4fe1acbd557a0cce952daf0cc5f..4565149f023280d839229dbd8f907ab294b66162 100644 --- a/lib/customer_info.rb +++ b/lib/customer_info.rb @@ -100,16 +100,18 @@ class AdminInfo info CustomerInfo api API call_info String + trust_level String end - def self.for(customer, plan) + def self.for(customer, plan, trust_level_repo: TrustLevelRepo.new) PromiseHash.all( jid: customer.jid, customer_id: customer.customer_id, fwd: customer.fwd, info: CustomerInfo.for(customer, plan), api: customer.api, - call_info: call_info(customer) + call_info: call_info(customer), + trust_level: trust_level_repo.find(customer).then(&:to_s) ).then(&method(:new)) end diff --git a/lib/trust_level.rb b/lib/trust_level.rb index c70edeb92de07261af31ee26a308c5f477705f46..8719521e1337713e69f9f600143acd1517c0828e 100644 --- a/lib/trust_level.rb +++ b/lib/trust_level.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "delegate" + module TrustLevel def self.for(plan_name:, settled_amount: 0, manual: nil) @levels.each do |level| @@ -8,7 +10,7 @@ module TrustLevel settled_amount: settled_amount, manual: manual ) - return tl if tl + return manual ? Manual.new(tl) : tl if tl end raise "No TrustLevel matched" @@ -19,6 +21,12 @@ module TrustLevel @levels << maybe_mk end + class Manual < SimpleDelegator + def to_s + "Manual(#{super})" + end + end + class Tomb TrustLevel.register do |manual:, **| new if manual == "Tomb" @@ -31,6 +39,10 @@ module TrustLevel def send_message?(*) false end + + def to_s + "Tomb" + end end class Basement @@ -45,6 +57,10 @@ module TrustLevel def send_message?(messages_today) messages_today < 200 end + + def to_s + "Basement" + end end class Paragon @@ -59,6 +75,10 @@ module TrustLevel def send_message?(messages_today) messages_today < 700 end + + def to_s + "Paragon" + end end class Customer @@ -86,5 +106,9 @@ module TrustLevel def send_message?(messages_today) messages_today < 500 end + + def to_s + "Customer" + end end end diff --git a/test/test_customer_info.rb b/test/test_customer_info.rb index 91239c4df51aaae9828cbd829969f139b86df5dc..6abafbcc2577b46efa7e7bfd9e60195335203c30 100644 --- a/test/test_customer_info.rb +++ b/test/test_customer_info.rb @@ -2,6 +2,8 @@ require "test_helper" require "customer_info" +require "trust_level_repo" +require "trust_level" API::REDIS = FakeRedis.new CustomerPlan::REDIS = Minitest::Mock.new @@ -46,8 +48,13 @@ class CustomerInfoTest < Minitest::Test ) cust = customer(sgx: sgx, plan_name: "test_usd") - assert cust.admin_info.sync.form + + trust_repo = Minitest::Mock.new + trust_repo.expect(:find, TrustLevel::Basement, [cust]) + + assert cust.admin_info(trust_level_repo: trust_repo).sync.form assert_mock sgx + assert_mock trust_repo end em :test_admin_info_does_not_crash @@ -81,8 +88,12 @@ class CustomerInfoTest < Minitest::Test sgx: sgx ) - assert cust.admin_info.sync.form + trust_repo = Minitest::Mock.new + trust_repo.expect(:find, TrustLevel::Basement, [cust]) + + assert cust.admin_info(trust_level_repo: trust_repo).sync.form assert_mock sgx + assert_mock trust_repo end em :test_inactive_admin_info_does_not_crash diff --git a/test/test_trust_level_repo.rb b/test/test_trust_level_repo.rb index a6b8add0ef06db2cb9dad503746e06afe925d4a9..0f7c7f795d566723ae05b8c1456120c76fb677ae 100644 --- a/test/test_trust_level_repo.rb +++ b/test/test_trust_level_repo.rb @@ -10,7 +10,7 @@ class TrustLevelRepoTest < Minitest::Test "jmp_customer_trust_level-test" => "Tomb" ) ).find(OpenStruct.new(customer_id: "test", plan_name: "usd")).sync - assert_kind_of TrustLevel::Tomb, trust_level + assert_equal "Manual(Tomb)", trust_level.to_s end em :test_manual_tomb @@ -21,7 +21,7 @@ class TrustLevelRepoTest < Minitest::Test "jmp_customer_trust_level-test" => "Basement" ) ).find(OpenStruct.new(customer_id: "test", plan_name: "usd")).sync - assert_kind_of TrustLevel::Basement, trust_level + assert_equal "Manual(Basement)", trust_level.to_s end em :test_manual_basement @@ -32,7 +32,7 @@ class TrustLevelRepoTest < Minitest::Test "jmp_customer_trust_level-test" => "Customer" ) ).find(OpenStruct.new(customer_id: "test", plan_name: "usd")).sync - assert_kind_of TrustLevel::Customer, trust_level + assert_equal "Manual(Customer)", trust_level.to_s end em :test_manual_customer @@ -43,7 +43,7 @@ class TrustLevelRepoTest < Minitest::Test "jmp_customer_trust_level-test" => "Paragon" ) ).find(OpenStruct.new(customer_id: "test", plan_name: "usd")).sync - assert_kind_of TrustLevel::Paragon, trust_level + assert_equal "Manual(Paragon)", trust_level.to_s end em :test_manual_paragon @@ -54,7 +54,7 @@ class TrustLevelRepoTest < Minitest::Test "jmp_customer_trust_level-test" => "UNKNOWN" ) ).find(OpenStruct.new(customer_id: "test", plan_name: "usd")).sync - assert_kind_of TrustLevel::Customer, trust_level + assert_equal "Manual(Customer)", trust_level.to_s end em :test_manual_unknown From ebd6fccf2e9c980a7ef9e7617a755807b3fe7b6a Mon Sep 17 00:00:00 2001 From: Christopher Vollick <0@psycoti.ca> Date: Thu, 5 May 2022 16:41:09 -0400 Subject: [PATCH 5/6] Refetch Customer on Repeated Customer Info Calls This means if I fetch a user, make a change (add a transaction, set their trust level, etc), I can re-run info from the menu to get the same user with up-to-date info, so you can see the thing I just did reflected back to me. --- lib/admin_command.rb | 25 ++++++++++++++----------- sgx_jmp.rb | 12 +++++++----- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/lib/admin_command.rb b/lib/admin_command.rb index c408998c8a59300273650f64c7a209ed375dfb30..7d8a47a41bf36cf207301386591d3b2a23185ab5 100644 --- a/lib/admin_command.rb +++ b/lib/admin_command.rb @@ -6,12 +6,15 @@ require_relative "financial_info" require_relative "form_template" class AdminCommand - def initialize(target_customer) + def initialize(target_customer, customer_repo) @target_customer = target_customer + @customer_repo = customer_repo end def start - action_info.then { menu_or_done } + @target_customer.admin_info.then { |info| + reply(info.form) + }.then { menu_or_done } end def reply(form) @@ -40,19 +43,19 @@ class AdminCommand end def new_context(q) - CustomerInfoForm.new.parse_something(q).then do |new_customer| - if new_customer.respond_to?(:customer_id) - AdminCommand.new(new_customer).start - else - reply(new_customer.form) + CustomerInfoForm.new(@customer_repo) + .parse_something(q).then do |new_customer| + if new_customer.respond_to?(:customer_id) + AdminCommand.new(new_customer, @customer_repo).start + else + reply(new_customer.form) + end end - end end def action_info - @target_customer.admin_info.then do |info| - reply(info.form) - end + # Refresh the data + new_context(@target_customer.customer_id) end def action_financial diff --git a/sgx_jmp.rb b/sgx_jmp.rb index be6cbf60462a24ba756f7f997875fc962500dc15..763e025f11ada9a550d34290a4a67a5dc9116471 100644 --- a/sgx_jmp.rb +++ b/sgx_jmp.rb @@ -755,16 +755,18 @@ Command.new( Command.customer.then do |customer| raise AuthError, "You are not an admin" unless customer&.admin? + customer_repo = CustomerRepo.new( + sgx_repo: Bwmsgsv2Repo.new, + bandwidth_tn_repo: EmptyRepo.new # No CNAM in admin + ) + Command.reply { |reply| reply.allowed_actions = [:next] reply.command << FormTemplate.render("customer_picker") }.then { |response| - CustomerInfoForm.new(CustomerRepo.new( - sgx_repo: Bwmsgsv2Repo.new, - bandwidth_tn_repo: EmptyRepo.new # No CNAM in admin - )).find_customer(response) + CustomerInfoForm.new(customer_repo).find_customer(response) }.then do |target_customer| - AdminCommand.new(target_customer).start + AdminCommand.new(target_customer, customer_repo).start end end }.register(self).then(&CommandList.method(:register)) From 84c77d857f375a3e673f088cd2a48ca070d36f94 Mon Sep 17 00:00:00 2001 From: Christopher Vollick <0@psycoti.ca> Date: Thu, 5 May 2022 16:41:10 -0400 Subject: [PATCH 6/6] Test Admin Info with Numbers My current two AdminInfo tests have unregistered customers because it's easier, but it means that some parts of the logic don't get tested. This one digs a bit deeper to test this part of the forms, but it requires injecting a repo --- lib/customer_info.rb | 14 +++++++++----- test/test_customer_info.rb | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/lib/customer_info.rb b/lib/customer_info.rb index 4565149f023280d839229dbd8f907ab294b66162..880e85677b49d4fa9be27c497c6f40df00e79015 100644 --- a/lib/customer_info.rb +++ b/lib/customer_info.rb @@ -103,14 +103,18 @@ class AdminInfo trust_level String end - def self.for(customer, plan, trust_level_repo: TrustLevelRepo.new) + def self.for( + customer, plan, + trust_level_repo: TrustLevelRepo.new, + call_attempt_repo: CallAttemptRepo.new + ) PromiseHash.all( jid: customer.jid, customer_id: customer.customer_id, fwd: customer.fwd, info: CustomerInfo.for(customer, plan), api: customer.api, - call_info: call_info(customer), + call_info: call_info(customer, call_attempt_repo), trust_level: trust_level_repo.find(customer).then(&:to_s) ).then(&method(:new)) end @@ -125,10 +129,10 @@ class AdminInfo end end - def self.call_info(customer) + def self.call_info(customer, call_attempt_repo) if customer.registered? - CallAttemptRepo.new - .find_outbound(customer, "+1", call_id: "", low_balance: FakeLowBalance) + call_attempt_repo + .find_outbound(customer, "+1", call_id: "dry_run") .then(&:to_s) else EMPromise.resolve("No calling") diff --git a/test/test_customer_info.rb b/test/test_customer_info.rb index 6abafbcc2577b46efa7e7bfd9e60195335203c30..19e2454043001e958927285981e0b5b1da48d18c 100644 --- a/test/test_customer_info.rb +++ b/test/test_customer_info.rb @@ -58,6 +58,39 @@ class CustomerInfoTest < Minitest::Test end em :test_admin_info_does_not_crash + def test_admin_info_with_tel_does_not_crash + registered = Struct.new(:phone).new("+12223334444") + fwd = CustomerFwd.for(uri: "tel:+12223334444", timeout: 15) + sgx = Struct.new(:registered?, :fwd).new(registered, fwd) + + CustomerPlan::DB.expect( + :query_one, + EMPromise.resolve({ start_date: Time.now }), + [String, "test"] + ) + + cust = customer(sgx: sgx, plan_name: "test_usd") + + call_attempt_repo = Minitest::Mock.new + call_attempt_repo.expect( + :find_outbound, + CallAttempt::Unsupported.new(direction: :outbound), + [cust, "+1", { call_id: "dry_run" }] + ) + + trust_repo = Minitest::Mock.new + trust_repo.expect(:find, TrustLevel::Basement, [cust]) + + assert cust + .admin_info( + trust_level_repo: trust_repo, + call_attempt_repo: call_attempt_repo + ).sync.form + assert_mock call_attempt_repo + assert_mock trust_repo + end + em :test_admin_info_with_tel_does_not_crash + def test_inactive_info_does_not_crash sgx = Minitest::Mock.new sgx.expect(:registered?, false)