TelSelections returns object instead of string

Amolith created 2 years ago

Two new objects, TelSelection::ChooseTel::Tn::Bandwidth and
Tn::LocalInventory and TelSelections now returns and persists through
pending state the correct variant.

No new functionality at this time, but formatted_number is now used when
displaying the number to the user at registration.

Signed-off-by: Amolith <[e-mail]>

Change summary

lib/registration.rb         |  12 ++--
lib/tel_selections.rb       | 102 ++++++++++++++++++++++++++------------
test/test_helper.rb         |   4 +
test/test_registration.rb   |  45 ++++++++---------
test/test_tel_selections.rb |  16 ++++-
5 files changed, 111 insertions(+), 68 deletions(-)

Detailed changes

lib/registration.rb

@@ -278,7 +278,7 @@ class Registration
 			attr_reader :customer_id, :tel
 
 			def save
-				REDIS.setex("pending_tel_for-#{@customer.jid}", THIRTY_DAYS, tel)
+				TEL_SELECTION.set(@customer.jid, @tel)
 			end
 
 			def form(rate, addr)
@@ -584,9 +584,9 @@ class Registration
 		end
 
 		def write
-			BandwidthTnReservationRepo.new.get(@customer, @tel).then do |rid|
+			BandwidthTnReservationRepo.new.get(@customer, @tel.tel).then do |rid|
 				BandwidthTNOrder.create(
-					@tel,
+					@tel.tel,
 					customer_order_id: @customer.customer_id,
 					reservation_id: rid
 				).then(&:poll).then(
@@ -619,7 +619,7 @@ class Registration
 		end
 
 		def put_default_fwd
-			Bwmsgsv2Repo.new.put_fwd(@customer.customer_id, @tel, CustomerFwd.for(
+			Bwmsgsv2Repo.new.put_fwd(@customer.customer_id, @tel.tel, CustomerFwd.for(
 				uri: "xmpp:#{@customer.jid}",
 				voicemail_enabled: true
 			))
@@ -640,9 +640,9 @@ class Registration
 		end
 
 		def customer_active_tel_purchased
-			@customer.register!(@tel).catch(&method(:raise_setup_error)).then {
+			@customer.register!(@tel.tel).catch(&method(:raise_setup_error)).then {
 				EMPromise.all([
-					REDIS.del("pending_tel_for-#{@customer.jid}"),
+					TEL_SELECTIONS.delete(@customer.jid),
 					put_default_fwd,
 					use_referral_code
 				])

lib/tel_selections.rb

@@ -19,7 +19,7 @@ class TelSelections
 	end
 
 	def set(jid, tel)
-		@redis.setex("pending_tel_for-#{jid}", THIRTY_DAYS, tel)
+		@redis.setex("pending_tel_for-#{jid}", THIRTY_DAYS, tel.pending_value)
 	end
 
 	def delete(jid)
@@ -34,7 +34,7 @@ class TelSelections
 
 	class HaveTel
 		def initialize(tel)
-			@tel = tel
+			@tel = ChooseTel::Tn.for_pending_value(tel)
 		end
 
 		def choose_tel
@@ -69,13 +69,17 @@ class TelSelections
 				reply.allowed_actions = [:next, :prev]
 				reply.command << FormTemplate.render("tn_list", tns: tns)
 			}.then { |iq|
-				tel = iq.form.field("tel")&.value
-				next choose_tel if iq.prev? || !tel
-
-				tel.to_s.strip
+				choose_from_list_result(tns, iq)
 			}
 		end
 
+		def choose_from_list_result(tns, iq)
+			tel = iq.form.field("tel")&.value
+			return choose_tel if iq.prev? || !tel
+
+			tns.find { |tn| tn.tel == tel } || Tn::Bandwidth.new(Tn.new(tel))
+		end
+
 		class AvailableNumber
 			def self.for(form, db: DB, memcache: MEMCACHE)
 				qs = form.field("q")&.value.to_s.strip
@@ -114,23 +118,27 @@ class TelSelections
 			def tns
 				Command.log.debug("BandwidthIris::AvailableNumber.list", @iris_query)
 				unless (result = fetch_cache)
-					result =
-						BandwidthIris::AvailableNumber.list(@iris_query) +
-						fetch_local_inventory.sync
+					result = fetch_bandwidth_inventory + fetch_local_inventory.sync
 				end
 				return next_fallback if result.empty? && !@fallback.empty?
 
-				result.map { |tn| Tn.new(**tn) }
+				result
+			end
+
+			def fetch_bandwidth_inventory
+				BandwidthIris::AvailableNumber
+					.list(@iris_query)
+					.map { |tn| Tn::Bandwidth.new(Tn::Option.new(**tn)) }
 			end
 
 			def fetch_local_inventory
 				@db.query_defer(@sql_query[0], @sql_query[1..-1]).then { |rows|
 					rows.map { |row|
-						{
+						Tn::LocalInventory.new(Tn::Option.new(
 							full_number: row["tel"].sub(/\A\+1/, ""),
 							city: row["locality"],
 							state: row["region"]
-						}
+						))
 					}
 				}
 			end
@@ -195,10 +203,16 @@ class TelSelections
 		class Tn
 			attr_reader :tel
 
-			def initialize(full_number:, city:, state:, **)
-				@tel = "+1#{full_number}"
-				@locality = city
-				@region = state
+			def self.for_pending_value(value)
+				if value.start_with?("LocalInventory/")
+					LocalInventory.new(Tn.new(value.sub(/\ALocalInventory\//, "")))
+				else
+					Bandwidth.new(Tn.new(value))
+				end
+			end
+
+			def initialize(tel)
+				@tel = tel
 			end
 
 			def formatted_tel
@@ -206,26 +220,50 @@ class TelSelections
 				"(#{$1}) #{$2}-#{$3}"
 			end
 
-			def option
-				op = Blather::Stanza::X::Field::Option.new(value: tel, label: to_s)
-				op << reference
-				op
+			def to_s
+				formatted_tel
+			end
+
+			class Option < Tn
+				def initialize(full_number:, city:, state:, **)
+					@tel = "+1#{full_number}"
+					@locality = city
+					@region = state
+				end
+
+				def option
+					op = Blather::Stanza::X::Field::Option.new(value: tel, label: to_s)
+					op << reference
+					op
+				end
+
+				def reference
+					Nokogiri::XML::Builder.new { |xml|
+						xml.reference(
+							xmlns: "urn:xmpp:reference:0",
+							begin: 0,
+							end: formatted_tel.length - 1,
+							type: "data",
+							uri: "tel:#{tel}"
+						)
+					}.doc.root
+				end
+
+				def to_s
+					"#{formatted_tel} (#{@locality}, #{@region})"
+				end
 			end
 
-			def reference
-				Nokogiri::XML::Builder.new { |xml|
-					xml.reference(
-						xmlns: "urn:xmpp:reference:0",
-						begin: 0,
-						end: formatted_tel.length - 1,
-						type: "data",
-						uri: "tel:#{tel}"
-					)
-				}.doc.root
+			class Bandwidth < SimpleDelegator
+				def pending_value
+					tel
+				end
 			end
 
-			def to_s
-				"#{formatted_tel} (#{@locality}, #{@region})"
+			class LocalInventory < SimpleDelegator
+				def pending_value
+					"LocalInventory/#{tel}"
+				end
 			end
 		end
 

test/test_helper.rb

@@ -211,7 +211,9 @@ class FakeTelSelections
 	end
 
 	def set(jid, tel)
-		@selections[jid] = EMPromise.resolve(TelSelections::HaveTel.new(tel))
+		@selections[jid] = EMPromise.resolve(
+			TelSelections::HaveTel.new(tel.pending_value)
+		)
 	end
 
 	def delete(jid)

test/test_registration.rb

@@ -32,7 +32,10 @@ class RegistrationTest < Minitest::Test
 		web_manager = TelSelections.new(
 			redis: FakeRedis.new, db: FakeDB.new, memcache: FakeMemcache.new
 		)
-		web_manager.set("[e-mail]", "[phone]")
+		web_manager.set(
+			"[e-mail]",
+			TelSelections::ChooseTel::Tn.for_pending_value("[phone]")
+		)
 		result = execute_command do
 			sgx = OpenStruct.new(registered?: false)
 			Registration.for(
@@ -55,7 +58,10 @@ class RegistrationTest < Minitest::Test
 		web_manager = TelSelections.new(
 			redis: FakeRedis.new, db: FakeDB.new, memcache: FakeMemcache.new
 		)
-		web_manager.set("test\\40approved.example.com@component", "[phone]")
+		web_manager.set(
+			"test\\40approved.example.com@component",
+			TelSelections::ChooseTel::Tn.for_pending_value("[phone]")
+		)
 		iq = Blather::Stanza::Iq::Command.new
 		iq.from = "[e-mail]"
 		result = execute_command(iq) do
@@ -77,7 +83,10 @@ class RegistrationTest < Minitest::Test
 		web_manager = TelSelections.new(
 			redis: FakeRedis.new, db: FakeDB.new, memcache: FakeMemcache.new
 		)
-		web_manager.set("[e-mail]", "[phone]")
+		web_manager.set(
+			"[e-mail]",
+			TelSelections::ChooseTel::Tn.for_pending_value("[phone]")
+		)
 		iq = Blather::Stanza::Iq::Command.new
 		iq.from = "[e-mail]"
 		result = execute_command(iq) do
@@ -96,7 +105,10 @@ class RegistrationTest < Minitest::Test
 		web_manager = TelSelections.new(
 			redis: FakeRedis.new, db: FakeDB.new, memcache: FakeMemcache.new
 		)
-		web_manager.set("[e-mail]", "[phone]")
+		web_manager.set(
+			"[e-mail]",
+			TelSelections::ChooseTel::Tn.for_pending_value("[phone]")
+		)
 		iq = Blather::Stanza::Iq::Command.new
 		iq.from = "[e-mail]"
 		result = execute_command(iq) do
@@ -1157,7 +1169,7 @@ class RegistrationTest < Minitest::Test
 			iq.from = "test\\[e-mail]"
 			@finish = Registration::Finish.new(
 				customer(sgx: @sgx, plan_name: "test_usd"),
-				"[phone]"
+				TelSelections::ChooseTel::Tn.for_pending_value("[phone]")
 			)
 		end
 
@@ -1189,11 +1201,6 @@ class RegistrationTest < Minitest::Test
 				:post,
 				"https://dashboard.bandwidth.com/v1.0/accounts//sites//sippeers//movetns"
 			)
-			Registration::Finish::REDIS.expect(
-				:del,
-				nil,
-				["[e-mail]"]
-			)
 			Registration::Finish::REDIS.expect(
 				:get,
 				nil,
@@ -1238,7 +1245,7 @@ class RegistrationTest < Minitest::Test
 					assert_equal :completed, reply.status
 					assert_equal :info, reply.note_type
 					assert_equal(
-						"Your JMP account has been activated as [phone]",
+						"Your JMP account has been activated as [phone]",
 						reply.note.content
 					)
 				end]
@@ -1293,11 +1300,6 @@ class RegistrationTest < Minitest::Test
 				:post,
 				"https://dashboard.bandwidth.com/v1.0/accounts//sites//sippeers//movetns"
 			)
-			Registration::Finish::REDIS.expect(
-				:del,
-				nil,
-				["[e-mail]"]
-			)
 			Registration::Finish::REDIS.expect(
 				:get,
 				EMPromise.resolve("123"),
@@ -1361,7 +1363,7 @@ class RegistrationTest < Minitest::Test
 					assert_equal :completed, reply.status
 					assert_equal :info, reply.note_type
 					assert_equal(
-						"Your JMP account has been activated as [phone]",
+						"Your JMP account has been activated as [phone]",
 						reply.note.content
 					)
 				end]
@@ -1417,11 +1419,6 @@ class RegistrationTest < Minitest::Test
 				:post,
 				"https://dashboard.bandwidth.com/v1.0/accounts//sites//sippeers//movetns"
 			)
-			Registration::Finish::REDIS.expect(
-				:del,
-				nil,
-				["pending_tel_for-test\\40onboarding.example.com@proxy"]
-			)
 			Registration::Finish::REDIS.expect(
 				:get,
 				nil,
@@ -1475,7 +1472,7 @@ class RegistrationTest < Minitest::Test
 						sgx: @sgx,
 						jid: Blather::JID.new("test\\40onboarding.example.com@proxy")
 					),
-					"[phone]"
+					TelSelections::ChooseTel::Tn.for_pending_value("[phone]")
 				).write.catch { |e| e }
 			end
 			assert_equal :test_result, result
@@ -1514,7 +1511,7 @@ class RegistrationTest < Minitest::Test
 					[Matching.new do |iq|
 						assert_equal :form, iq.form.type
 						assert_equal(
-							"The JMP number [phone] is no longer available.",
+							"The JMP number [phone] is no longer available.",
 							iq.form.instructions
 						)
 					end]

test/test_tel_selections.rb

@@ -14,15 +14,21 @@ class TelSelectionsTest < Minitest::Test
 
 	def test_set_get
 		assert_kind_of TelSelections::ChooseTel, @manager["[e-mail]"].sync
-		@manager.set("[e-mail]", "[phone]").sync
+		@manager.set(
+			"[e-mail]",
+			TelSelections::ChooseTel::Tn.for_pending_value("[phone]")
+		).sync
 		assert_kind_of TelSelections::HaveTel, @manager["[e-mail]"].sync
 	end
 	em :test_set_get
 
 	def test_choose_tel_have_tel
 		jid = "[e-mail]"
-		@manager.set(jid, "[phone]").sync
-		assert_equal "[phone]", @manager[jid].then(&:choose_tel).sync
+		@manager.set(
+			jid,
+			TelSelections::ChooseTel::Tn.for_pending_value("[phone]")
+		).sync
+		assert_equal "[phone]", @manager[jid].then(&:choose_tel).sync.tel
 	end
 	em :test_choose_tel_have_tel
 
@@ -153,9 +159,9 @@ class TelSelectionsTest < Minitest::Test
 		em :test_local_inventory
 	end
 
-	class TnTest < Minitest::Test
+	class TnOptionTest < Minitest::Test
 		def setup
-			@tn = TelSelections::ChooseTel::Tn.new(
+			@tn = TelSelections::ChooseTel::Tn::Option.new(
 				full_number: "5551234567",
 				city: "Toronto",
 				state: "ON",