Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of callerId for national dialing #294

Merged
merged 2 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion components/app/app/models/call_properties.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,8 @@
:from,
:sip_headers,
keyword_init: true
)
) do
def inbound?
direction == "inbound"
end
end
20 changes: 15 additions & 5 deletions components/app/app/models/dial_string.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,31 @@ class DialString
EXTERNAL_PROFILE = "external".freeze
EXTERNAL_NAT_INSTANCE_PROFILE = "alternative-outbound".freeze

attr_reader :address, :symmetric_latching
attr_reader :options

def initialize(options)
options.symbolize_keys!
@symmetric_latching = options.fetch(:symmetric_latching, true)
@address = options.fetch(:address) { RoutingParameters.new(options).address }
@options = options.symbolize_keys
end

def to_s
"sofia/#{external_profile}/#{address}"
end

def address
options.fetch(:address) { routing_parameters.address }
end

def format_number(...)
routing_parameters.format_number(...)
end

private

def routing_parameters
@routing_parameters ||= RoutingParameters.new(options)
end

def external_profile
symmetric_latching ? EXTERNAL_PROFILE : EXTERNAL_NAT_INSTANCE_PROFILE
options.fetch(:symmetric_latching, true) ? EXTERNAL_PROFILE : EXTERNAL_NAT_INSTANCE_PROFILE
end
end
33 changes: 10 additions & 23 deletions components/app/app/models/execute_twiml.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,36 +171,23 @@ def execute_dial(verb)
dial_content = nested_noun.content.strip

if nested_noun.text? || nested_noun.name == "Number"
routing_parameters = build_routing_parameters(dial_content)
dial_string = DialString.new(build_routing_parameters(dial_content))
caller_id = dial_string.format_number(
attributes.fetch("callerId") { call_properties.inbound? ? call_properties.from : call_properties.to }
)
elsif nested_noun.name == "Sip"
routing_parameters = {
address: dial_content.delete_prefix("sip:")
}
end

dial_string = DialString.new(routing_parameters).to_s

break dial_string if nested_noun.text?

unless ["Number", "Sip"].include?(nested_noun.name)
dial_string = DialString.new(address: dial_content.delete_prefix("sip:"))
else
raise Errors::TwiMLError, "Nested noun <#{nested_noun.name}> not allowed within <Dial>"
end

nested_noun_attributes = twiml_attributes(nested_noun)
result[dial_string] = {
from: nested_noun_attributes["callerId"],
ringback: nested_noun_attributes["ringToneUrl"]
result[dial_string.to_s] = {
from: caller_id,
for: attributes.fetch("timeout", 30).to_i.seconds
}.compact
end

dial_status = dial(
to,
{
from: attributes["callerId"],
ringback: attributes["ringToneUrl"],
for: attributes.fetch("timeout", 30).to_i.seconds
}.compact
)
dial_status = dial(to)

return if attributes["action"].blank?

Expand Down
8 changes: 5 additions & 3 deletions components/app/app/models/outbound_call.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ def initiate
account_sid: call_params.fetch("account_sid")
)

dial_string = DialString.new(call_params.fetch("routing_parameters"))

Adhearsion::OutboundCall.originate(
DialString.new(call_params.fetch("routing_parameters")).to_s,
from: call_params.fetch("from"),
dial_string.to_s,
from: dial_string.format_number(call_params.fetch("from")),
controller: CallController,
controller_metadata: {
call_properties: CallProperties.new(
Expand All @@ -27,7 +29,7 @@ def initiate
api_version: call_params.fetch("api_version"),
from: call_params.fetch("from"),
to: call_params.fetch("to"),
sip_headers: sip_headers
sip_headers:
)
},
headers: build_call_headers(sip_headers)
Expand Down
9 changes: 7 additions & 2 deletions components/app/app/models/routing_parameters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,19 @@ def initialize(options)
end

def address
result = national_dialing ? Phony.format(destination, format: :national, spaces: "") : destination
result.gsub!(/\D/, "")
result = format_number(destination)
result = username.present? ? client_gateway_address(result) : public_gateway_address(result)
result.prepend(dial_string_prefix) if dial_string_prefix.present?
result.prepend("+") if plus_prefix
result
end

def format_number(value)
result = value.gsub(/\D/, "")
result = Phony.format(result, format: :national, spaces: "") if national_dialing && Phony.plausible?(result)
result.gsub(/\D/, "")
end

private

def client_gateway_address(destination)
Expand Down
104 changes: 56 additions & 48 deletions components/app/spec/call_controllers/dial_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
controller = build_controller(
stub_voice_commands: [:play_audio, { dial: build_dial_status }],
call_properties: {
account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34"
account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34",
from: "855715100860",
direction: "inbound"
}
)

Expand All @@ -44,12 +46,39 @@
controller.run

expect(controller).to have_received(:dial).with(
dial_string("016701721"),
for: 30.seconds
include(
dial_string("85516701721") => { for: 30.seconds, from: "855715100860" }
)
)
expect(controller).to have_received(:play_audio).with("foo.mp3")
end

it "handles national dialing", :vcr, cassette: :build_routing_parameters_with_national_dialing do
controller = build_controller(
stub_voice_commands: [:play_audio, { dial: build_dial_status }],
call_properties: {
account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34",
to: "855715100860",
direction: "outbound-api"
}
)

stub_twiml_request(controller, response: <<~TWIML)
<?xml version="1.0" encoding="UTF-8" ?>
<Response>
<Dial>85516701721</Dial>
</Response>
TWIML

controller.run

expect(controller).to have_received(:dial).with(
include(
dial_string("016701721") => hash_including(from: "0715100860")
)
)
end

it "handles numbers without symmetric latching support", :vcr, cassette: :build_routing_parameters_without_symmetric_latching do
controller = build_controller(
stub_voice_commands: [{ dial: build_dial_status }],
Expand All @@ -68,16 +97,19 @@
controller.run

expect(controller).to have_received(:dial).with(
dial_string("016701721", profile: "alternative-outbound"),
any_args
include(
dial_string("016701721", profile: "alternative-outbound") => be_a_kind_of(Hash)
)
)
end

it "dials to <Number>", :vcr, cassette: :build_multiple_routing_parameters do
controller = build_controller(
stub_voice_commands: { dial: build_dial_status },
call_properties: {
account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34"
account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34",
from: "855715200860",
direction: "inbound"
}
)

Expand All @@ -96,9 +128,9 @@

expect(controller).to have_received(:dial).with(
include(
dial_string("016701721") => {},
dial_string("0715100860") => {},
dial_string("010555777") => {}
dial_string("85516701721") => hash_including(from: "855715200860"),
dial_string("0715100860", profile: "alternative-outbound") => hash_including(from: "0715200860"),
dial_string("85510555777") => hash_including(from: "855715200860")
),
any_args
)
Expand All @@ -125,9 +157,8 @@

expect(controller).to have_received(:dial).with(
include(
"sofia/external/[email protected]" => {}
),
any_args
"sofia/external/[email protected]" => { for: 30.seconds }
)
)
end

Expand All @@ -139,9 +170,9 @@
stub_twiml_request(controller, response: <<~TWIML)
<?xml version="1.0" encoding="UTF-8" ?>
<Response>
<Dial>
<Number callerId="2442">85516701721</Number>
<Number callerId="2443">855715100860</Number>
<Dial callerId="85523238265">
<Number>85516701721</Number>
<Number>855715100860</Number>
<Number>85510555777</Number>
</Dial>
</Response>
Expand All @@ -151,11 +182,10 @@

expect(controller).to have_received(:dial).with(
include(
dial_string("016701721") => { from: "2442" },
dial_string("0715100860") => { from: "2443" },
dial_string("010555777") => {}
),
any_args
dial_string("85516701721") => hash_including(from: "85523238265"),
dial_string("0715100860", profile: "alternative-outbound") => hash_including(from: "023238265"),
dial_string("85510555777") => hash_including(from: "85523238265")
)
)
end

Expand Down Expand Up @@ -373,8 +403,9 @@
controller.run

expect(controller).to have_received(:dial).with(
dial_string("016701721"),
hash_including(from: "2442")
include(
dial_string("85516701721") => hash_including(from: "2442")
)
)
end
end
Expand All @@ -395,32 +426,9 @@
controller.run

expect(controller).to have_received(:dial).with(
dial_string("016701721"),
hash_including(for: 10.seconds)
)
end
end

describe "ringToneUrl" do
# Not supported by Twilio

it "handles a ringToneUrl", :vcr, cassette: :build_routing_parameters do
controller = build_controller(
stub_voice_commands: { dial: build_dial_status }
)

stub_twiml_request(controller, response: <<~TWIML)
<?xml version="1.0" encoding="UTF-8" ?>
<Response>
<Dial ringToneUrl="http://api.twilio.com/cowbell.mp3">+85516701721</Dial>
</Response>
TWIML

controller.run

expect(controller).to have_received(:dial).with(
dial_string("016701721"),
hash_including(ringback: "http://api.twilio.com/cowbell.mp3")
include(
dial_string("85516701721") => hash_including(for: 10.seconds)
)
)
end
end
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading