-
-
Notifications
You must be signed in to change notification settings - Fork 1
Add Twilio LCR Option to send call straight to voicemail #147
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,7 @@ class LiveCallRouting::Twilio::V3 < Integration | |||||||||||||||||||||
| {key: :api_region, type: :string, default: "ashburn.us1"}, | ||||||||||||||||||||||
| {key: :force_input, type: :boolean, default: false}, | ||||||||||||||||||||||
| {key: :record, type: :boolean, default: false}, | ||||||||||||||||||||||
| {key: :send_straight_to_voicemail, type: :boolean, default: false}, | ||||||||||||||||||||||
| {key: :record_email, type: :string, default: ""}, | ||||||||||||||||||||||
| {key: :banned_phone, type: :string, default: ""}, | ||||||||||||||||||||||
| {key: :dial_pause, type: :integer}, | ||||||||||||||||||||||
|
|
@@ -30,6 +31,7 @@ class LiveCallRouting::Twilio::V3 < Integration | |||||||||||||||||||||
| validates :option_api_region, inclusion: {in: API_REGIONS} | ||||||||||||||||||||||
| validates :option_force_input, inclusion: {in: [true, false]} | ||||||||||||||||||||||
| validates :option_record, inclusion: {in: [true, false]} | ||||||||||||||||||||||
| validates :option_send_straight_to_voicemail, inclusion: {in: [true, false]} | ||||||||||||||||||||||
| validates :option_max_wait_time, numericality: {greater_than_or_equal_to: 30, less_than_or_equal_to: 3600}, allow_nil: true | ||||||||||||||||||||||
| validate :validate_record_emails | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
Comment on lines
36
to
37
|
||||||||||||||||||||||
| validate :validate_record_emails | |
| validate :validate_record_emails | |
| validate :validate_send_straight_to_voicemail_record_dependency | |
| def validate_send_straight_to_voicemail_record_dependency | |
| return unless option_send_straight_to_voicemail | |
| return if option_record | |
| errors.add(:option_send_straight_to_voicemail, "can only be enabled when recording is enabled") | |
| end |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new straight-to-voicemail feature should have test coverage. Tests should verify: (1) the option is properly saved and validated, (2) calls are redirected to voicemail without being queued when the option is enabled, (3) alerts are routed only after a voicemail is recorded, and (4) the feature only works when voicemail recording is enabled.
Copilot
AI
Dec 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array wrapper with include? is unnecessary when checking for a single value. Consider using a direct equality check: if call_status == "completed" for better clarity and performance.
| if ["completed"].include?(call_status) | |
| if call_status == "completed" |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new call_status callback handling logic should have test coverage. Tests should verify: (1) alerts are properly resolved when a caller hangs up without leaving a voicemail in straight-to-voicemail mode, (2) alerts are properly routed when a voicemail is left, and (3) alerts are resolved when callers hang up before being queued in normal mode.
Copilot
AI
Dec 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new call_status callback functionality and the adapter_process_call_status_deferred method lack test coverage. Consider adding tests to verify the behavior when a caller hangs up before leaving a voicemail, when they leave a voicemail successfully, and when they hang up before being placed in a queue.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| member do | ||
| get :music | ||
| post :queue_status | ||
| post :call_status | ||
| post :dropped | ||
| end | ||
| end | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new call_status controller endpoint should have test coverage. Tests should verify that the endpoint properly queues a deferred request and processes the call status callback correctly.