-
Notifications
You must be signed in to change notification settings - Fork 13
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
[FYST-1089] refactor twilio service #4944
base: main
Are you sure you want to change the base?
Changes from 15 commits
7ebf9aa
a914b96
505cdfc
623d5dd
ae26344
c12385b
530dcf4
eacfb9d
ceee3f7
077654d
64d8be2
2a1a4cd
ee2f22b
bb05cb2
40e4dbd
db5747d
6812f1e
4bd6e7b
288ac36
64c28b9
4f92fea
fa01ca8
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 |
---|---|---|
|
@@ -34,7 +34,7 @@ def self.send_message(message_name, batch_size=nil, after: nil) | |
end | ||
|
||
if signup.phone_number.present? | ||
TwilioService.send_text_message(to: signup.phone_number, body: message.sms_body) | ||
TwilioService.new(:ctc).send_text_message(to: signup.phone_number, body: message.sms_body) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is actually not just a CTC thing! see: https://www.getyourrefund.org/en/sign-up There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that makes me moderately unhappy. but it's kinda sorta ok as right now ctc and gyr seem to be sharing the same twilio setup. |
||
signup.touch(sent_at_column) | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,10 @@ | ||
class TwilioService | ||
FAILED_STATUSES = ["undelivered", "failed", "delivery_unknown", "twilio_error"].freeze | ||
SUCCESSFUL_STATUSES = ["sent", "delivered"].freeze | ||
IN_PROGRESS_STATUSES = ["accepted", "queued", "sending", nil].freeze | ||
FAILED_STATUSES = %w(undelivered failed delivery_unknown twilio_error) | ||
SUCCESSFUL_STATUSES = %w(sent delivered) | ||
IN_PROGRESS_STATUSES = %w(accepted queued sending) << nil | ||
ALL_KNOWN_STATUSES = FAILED_STATUSES + SUCCESSFUL_STATUSES + IN_PROGRESS_STATUSES | ||
ORDERED_STATUSES = [nil, "twilio_error"] + %w[ | ||
ORDERED_STATUSES = %w( | ||
twilio_error | ||
queued | ||
accepted | ||
sending | ||
|
@@ -12,104 +13,101 @@ class TwilioService | |
delivered | ||
undelivered | ||
failed | ||
].freeze | ||
).unshift(nil) # why do we need nil in this list, and why must it be first? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [DUST] i assume you're planning to remove this comment before merging? would prefer not to leave stuff like this around the codebase There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes! for sure. i literally don't know the answer but would like to. 😅 |
||
|
||
class << self | ||
def valid_request?(request) | ||
validator = Twilio::Security::RequestValidator.new(EnvironmentCredentials.dig(:twilio, :auth_token)) | ||
validator.validate( | ||
request.url, | ||
request.POST, | ||
request.headers["X-Twilio-Signature"], | ||
) | ||
end | ||
attr_reader :client, :messaging_service_sid, :auth_token | ||
|
||
def fetch_attachment(url) | ||
begin | ||
response = Net::HTTP.get_response(URI(url)) # first we get a redirect from Twilio to S3 | ||
response = Net::HTTP.get_response(URI(response['location'])) # then we get a redirect from S3 to S3 | ||
response = Net::HTTP.get_response(URI(response['location'])) # finally we should get a 200 OK with the file | ||
filename_from_s3 = response['content-disposition'].split('"').last # S3 gives us the original filename | ||
{ | ||
filename: filename_from_s3, | ||
body: response.body, | ||
} | ||
rescue ArgumentError => e | ||
Rails.logger.error("Error getting attachment from Twilio: #{url}: #{response&.code}: #{response&.to_hash}") | ||
{ | ||
filename: "unknown-file", | ||
body: nil | ||
} | ||
end | ||
end | ||
def initialize(service_type = :gyr) | ||
creds = MultiTenantService.new(service_type).twilio_creds | ||
@messaging_service_sid = creds[:messaging_service_id] | ||
@auth_token = creds[:auth_token] | ||
@client = Twilio::REST::Client.new(creds[:account_sid], auth_token) | ||
end | ||
|
||
def parse_attachments(params) | ||
num_media = params["NumMedia"].to_i | ||
def send_text_message(to:, body:, status_callback: nil, outgoing_text_message: nil) | ||
arguments = { | ||
messaging_service_sid: ENV['MESSAGING_SERVICE_SID'] || messaging_service_sid, # why do we check the environment for this?? | ||
to: to, | ||
body: body | ||
} | ||
arguments[:status_callback] = status_callback if status_callback.present? | ||
|
||
(0..(num_media - 1)).map do |i| | ||
content_type = params["MediaContentType#{i}"] | ||
attachment = fetch_attachment(params["MediaUrl#{i}"]) | ||
DatadogApi.increment("twilio.outgoing_text_messages.sent") | ||
|
||
if FileTypeAllowedValidator.mime_types(Document).include?(content_type) && !attachment[:body].empty? | ||
{ | ||
content_type: params["MediaContentType#{i}"], | ||
filename: attachment[:filename], | ||
body: attachment[:body] | ||
} | ||
else | ||
{ | ||
content_type: "text/plain;charset=UTF-8", | ||
filename: "invalid-#{attachment[:filename]}.txt", | ||
body: <<~TEXT | ||
Unusable file with unknown or unsupported file type. | ||
File name: #{attachment[:filename]} | ||
File type: #{content_type} | ||
File size: #{attachment[:body].size} bytes | ||
TEXT | ||
} | ||
end | ||
client.messages.create(**arguments) | ||
rescue Twilio::REST::RestError => e | ||
status_key = | ||
if outgoing_text_message.is_a?(OutgoingMessageStatus) | ||
:delivery_status | ||
else | ||
:twilio_status | ||
end | ||
end | ||
outgoing_text_message&.update(status_key => "twilio_error") | ||
|
||
def client | ||
@@_client ||= Twilio::REST::Client.new( | ||
EnvironmentCredentials.dig(:twilio, :account_sid), | ||
EnvironmentCredentials.dig(:twilio, :auth_token) | ||
) | ||
unless e.code == 21211 # Invalid 'To' Phone Number https://www.twilio.com/docs/api/errors/21211 | ||
raise # should we include the original exception here (e)?? | ||
end | ||
|
||
def send_text_message(to:, body:, status_callback: nil, outgoing_text_message: nil) | ||
arguments = { | ||
messaging_service_sid: ENV['MESSAGING_SERVICE_SID'] || EnvironmentCredentials.dig(:twilio, :messaging_service_sid), | ||
to: to, | ||
body: body | ||
} | ||
arguments[:status_callback] = status_callback if status_callback.present? | ||
nil | ||
end | ||
|
||
DatadogApi.increment("twilio.outgoing_text_messages.sent") | ||
def get_metadata(phone_number:) | ||
client.lookups.v2.phone_numbers(phone_number).fetch(fields: 'line_type_intelligence').line_type_intelligence | ||
rescue Twilio::REST::RestError | ||
{} | ||
end | ||
|
||
client.messages.create(**arguments) | ||
rescue Twilio::REST::RestError => e | ||
status_key = | ||
if outgoing_text_message.is_a?(OutgoingMessageStatus) | ||
:delivery_status | ||
else | ||
:twilio_status | ||
end | ||
outgoing_text_message&.update(status_key => "twilio_error") | ||
def valid_request?(request) | ||
validator = Twilio::Security::RequestValidator.new(auth_token) | ||
validator.validate( | ||
request.url, | ||
request.POST, | ||
request.headers["X-Twilio-Signature"], | ||
) | ||
end | ||
|
||
unless e.code == 21211 # Invalid 'To' Phone Number https://www.twilio.com/docs/api/errors/21211 | ||
raise | ||
end | ||
def fetch_attachment(url) | ||
response = Net::HTTP.get_response(URI(url)) # first we get a redirect from Twilio to S3 | ||
response = Net::HTTP.get_response(URI(response['location'])) # then we get a redirect from S3 to S3 | ||
response = Net::HTTP.get_response(URI(response['location'])) # finally we should get a 200 OK with the file | ||
filename_from_s3 = response['content-disposition'].split('"').last # S3 gives us the original filename | ||
{ | ||
filename: filename_from_s3, | ||
body: response.body, | ||
} | ||
rescue ArgumentError => e | ||
Rails.logger.error("Error getting attachment from Twilio: #{url}: #{response&.code}: #{response&.to_hash}") | ||
{ | ||
filename: "unknown-file", | ||
body: nil | ||
} | ||
end | ||
|
||
nil | ||
end | ||
def parse_attachments(params) | ||
num_media = params["NumMedia"].to_i | ||
|
||
def get_metadata(phone_number:) | ||
client.lookups.v2.phone_numbers(phone_number).fetch(fields: 'line_type_intelligence').line_type_intelligence | ||
(0...num_media).map do |i| | ||
content_type = params["MediaContentType#{i}"] | ||
attachment = fetch_attachment(params["MediaUrl#{i}"]) | ||
|
||
rescue Twilio::REST::RestError | ||
{} | ||
if FileTypeAllowedValidator.mime_types(Document).include?(content_type) && !attachment[:body].empty? | ||
{ | ||
content_type: params["MediaContentType#{i}"], | ||
filename: attachment[:filename], | ||
body: attachment[:body] | ||
} | ||
else | ||
{ | ||
content_type: "text/plain;charset=UTF-8", | ||
filename: "invalid-#{attachment[:filename]}.txt", | ||
body: <<~TEXT | ||
Unusable file with unknown or unsupported file type. | ||
File name: #{attachment[:filename]} | ||
File type: #{content_type} | ||
File size: #{attachment[:body].size} bytes | ||
TEXT | ||
} | ||
end | ||
end | ||
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.
[DUST] again, would like to remove comments before merging
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.
absolutely. do you know if/how this job is aware of it's tenant service/environment?