Page MenuHomePhabricator
Differential D19982

Add infrastructure for sending SMS via AWS SNS
ClosedPublic
Actions

Authored by amckinley on Jan 16 2019, 7:09 PM.
Tags
None
Referenced Files
F13275106: D19982.diff
Fri, May 31, 4:24 AM
F13271388: D19982.diff
Wed, May 29, 10:23 PM
F13261165: D19982.diff
Mon, May 27, 12:45 AM
F13248205: D19982.id47825.diff
Fri, May 24, 1:55 AM
F13247695: D19982.id47827.diff
Thu, May 23, 9:44 PM
F13245135: D19982.diff
Thu, May 23, 5:46 AM
F13229780: D19982.id47825.diff
Mon, May 20, 7:05 PM
F13206526: D19982.id47828.diff
Wed, May 15, 11:34 AM
View All Files
Subscribers
Korvin

Details

Reviewers
epriestley
Maniphest Tasks
T920: Provide SMS Support
T13235: Replace "SimpleEmailService" with a subclass of "PhutilAWSFuture" that surfaces errors more clearly
Commits
rPe72684a4ba5d: Add infrastructure for sending SMS via AWS SNS
Summary

Ref T920. Ref T13235. This adds a Future, similar to TwilioFuture, for interacting with Amazon's SNS service.

Also updates the documentation.

Also makes the code consistent with the documentation by accepting a media argument.

Test Plan

Clicked the "send test message" button from the Settings UI.

Diff Detail

Repository
rP Phabricator
Branch
sms-redux
Lint
Lint Errors
SeverityLocationCodeMessage
Error/Users/amckinley/src/phacility/core/lib/libphutil/src/filesystem/Filesystem.php:428PHL1Unknown Symbol
Error/Users/amckinley/src/phacility/core/lib/libphutil/src/filesystem/Filesystem.php:568PHL1Unknown Symbol
Unit
Tests Passed
Build Status
Buildable 21679
Build 29565: Run Core Tests
Build 29564: arc lint + arc unit

Event Timeline

amckinley created this revision.Jan 16 2019, 7:09 PM
Harbormaster completed remote builds in B21537: Diff 47684.Jan 16 2019, 7:11 PM
amckinley requested review of this revision.Jan 16 2019, 7:11 PM
Herald added a subscriber: Korvin. · View Herald TranscriptJan 16 2019, 7:11 PM
amckinley planned changes to this revision.Jan 16 2019, 7:11 PM
amckinley mentioned this in T13235: Replace "SimpleEmailService" with a subclass of "PhutilAWSFuture" that surfaces errors more clearly.
amckinley updated this revision to Diff 47824.Jan 24 2019, 12:05 AM

Clean up to work like TwilioFuture.

Harbormaster failed remote builds in B21677: Diff 47824!Jan 24 2019, 12:06 AM
amckinley updated this revision to Diff 47825.Jan 24 2019, 12:09 AM

Restore deleted file.

amckinley edited the summary of this revision. (Show Details)Jan 24 2019, 12:10 AM
amckinley edited the test plan for this revision. (Show Details)
Harbormaster completed remote builds in B21678: Diff 47825.Jan 24 2019, 12:11 AM
amckinley updated this revision to Diff 47826.Jan 24 2019, 12:12 AM

Correct link.

epriestley accepted this revision.Jan 24 2019, 12:12 AM
epriestley added inline comments.
src/applications/metamta/future/PhabricatorAmazonSNSFuture.php
5

Is this actually used?

6

Doesn't necessarily need to be in this diff, but can we put this in PhutilAWSFuture?

src/docs/user/configuration/configuring_outbound_email.diviner
100

(I'll hit this document a bit more broadly for SMS in general at some point, too.)

216

Link to SES instead of SNS?

220

"to the SES configuration"?

src/infrastructure/cluster/config/PhabricatorClusterMailersConfigType.php
48

Ah, thanks.

This revision is now accepted and ready to land.Jan 24 2019, 12:12 AM
amckinley updated this revision to Diff 47827.Jan 24 2019, 12:13 AM

Typo fix.

Harbormaster completed remote builds in B21679: Diff 47826.Jan 24 2019, 12:13 AM
amckinley marked 2 inline comments as done.Jan 24 2019, 12:15 AM
amckinley added inline comments.
src/applications/metamta/future/PhabricatorAmazonSNSFuture.php
5

If I implemented getProxiedFuture correctly, I think so?

6

Yeah, good idea.

src/docs/user/configuration/configuring_outbound_email.diviner
216

Hahah I noticed both of these right after submitting.

Harbormaster completed remote builds in B21680: Diff 47827.Jan 24 2019, 12:15 AM
epriestley added a comment.Jan 24 2019, 12:17 AM

Oh, I see what's going on with parameters, the base class is just a little bit unusual. Carry on.

Closed by commit rPe72684a4ba5d: Add infrastructure for sending SMS via AWS SNS (authored by amckinley). · Explain WhyJan 24 2019, 12:30 AM
This revision was automatically updated to reflect the committed changes.
amckinley marked an inline comment as done.

Revision Contents
Changeset List

PathSize
src/
applications/
metamta/
adapter/
66 lines
future/
41 lines
docs/
user/
configuration/
10 lines
infrastructure/
cluster/
config/
1 line

Diff 47826

View Options

src/__phutil_library_map__.php

Loading...
View Options

src/applications/metamta/adapter/PhabricatorMailAmazonSNSAdapter.php

Loading...
View Options

src/applications/metamta/future/PhabricatorAmazonSNSFuture.php

Loading...
View Options

src/docs/user/configuration/configuring_outbound_email.diviner

Loading...
View Options

src/infrastructure/cluster/config/PhabricatorClusterMailersConfigType.php

Loading...
Phabricator · Built by Phacility