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
F18846291: D19982.id47684.diff
Wed, Oct 29, 6:21 PM
F18844844: D19982.diff
Wed, Oct 29, 7:49 AM
F18813127: D19982.id.diff
Mon, Oct 20, 2:13 PM
F18803853: D19982.diff
Sat, Oct 18, 4:17 AM
F18783779: D19982.id47828.diff
Mon, Oct 13, 9:30 AM
F18772014: D19982.diff
Wed, Oct 8, 11:34 PM
F18761338: D19982.id47827.diff
Mon, Oct 6, 1:46 PM
F18745041: D19982.id47684.diff
Fri, Oct 3, 5:05 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