Add a Twilio SMS message adapter
ClosedPublic
Actions

Authored by epriestley on Jan 14 2019, 9:55 PM.

Details

Reviewers

amckinley

Maniphest Tasks

T920: Provide SMS Support

Commits

rP35f0e31ed3b2: Add a Twilio SMS message adapter

Summary

Ref T920. Adds a "phone number" object, an "SMS" message type, and Twilio glue.

Test Plan

Used this test script to send myself some text messages after configuring Twilio in cluster.mailers.

<?php

require_once 'scripts/init/init-script.php';

if ($argc < 3) {
  throw new Exception('usage: test.php <number> <body>');
}
$to_number = $argv[1];
$text_body = $argv[2];

$mailers = PhabricatorMetaMTAMail::newMailers(
  array(
    'outbound' => true,
    'media' => array(
      PhabricatorMailSMSMessage::MESSAGETYPE,
    ),
  ));
if (!$mailers) {
  return new Aphront404Response();
}
$mailer = head($mailers);

$message = id(new PhabricatorMailSMSMessage())
  ->setToNumber(new PhabricatorPhoneNumber($to_number))
  ->setTextBody($text_body);

$mailer->sendMessage($message);

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.Jan 14 2019, 9:55 PM

Harbormaster completed remote builds in B21522: Diff 47669.Jan 14 2019, 9:57 PM

epriestley requested review of this revision.Jan 14 2019, 9:57 PM

epriestley added inline comments.Jan 14 2019, 9:58 PM

src/applications/metamta/adapter/PhabricatorMailTwilioAdapter.php
4	You should be able to add a similar class to this for AWS SNS now (well, once all this stuff lands), I think.

amckinley accepted this revision.Jan 16 2019, 2:08 AM

amckinley added inline comments.

src/applications/metamta/message/PhabricatorPhoneNumber.php
12	Where did this regex come from? A few minutes of digging for validating of E.123 numbers points to this O'Reilly suggestion: https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s03.html

This revision is now accepted and ready to land.Jan 16 2019, 2:08 AM

epriestley added inline comments.Jan 16 2019, 2:13 AM

src/applications/metamta/message/PhabricatorPhoneNumber.php
12	I expect this will be refined a lot, but it's more-or-less from the Twilio docs here: https://www.twilio.com/docs/glossary/what-e164

Used this test script to send myself some text messages...

Why not use this to resurrect bin/sms?

The script is currently very bare-metal since PhabricatorMetaMTAMail doesn't actually have the "this might be an SMS" code yet.

That chunk of code is up relatively-soon-ish but I expect it will mean that bin/mail send-test gets a --medium sms or something rather than a separate bin/sms / bin/apns / bin/whatsapp set of entrypoints. I'll see how much of a mess that is, I might resurrect bin/sms if the flags are too confusing (or bin/mail send-short-message-like-sms-or-whatsapp or something). But the "action" bit at the end should be $mail->send(), not $mailer->sendMessage(...).

Doing $mail->send() means that we get retries, a listing in bin/mail list-outbound, an entry in /mail/, non-delivery to disabled users, and a bunch of other support infrastructure stuff.