Paths

Table of Contentst

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

	F15397233: D19982.diff
	Sun, Mar 16, 7:04 PM

	F15386810: D19982.diff
	Sat, Mar 15, 1:19 AM

	F15282869: D19982.id47824.diff
	Tue, Mar 4, 3:15 AM

	F15282868: D19982.id47827.diff
	Tue, Mar 4, 3:15 AM

	F15282867: D19982.id47825.diff
	Tue, Mar 4, 3:15 AM

	F15282866: D19982.id47826.diff
	Tue, Mar 4, 3:15 AM

	Unknown Object (File)
	Sun, Mar 2, 6:07 PM

	Unknown Object (File)
	Mon, Feb 24, 9:17 AM

Subscribers

Details

Reviewers

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

Branch

sms-redux

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 21537
Build 29355: Run Core Tests
Build 29354: 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.

Clean up to work like TwilioFuture.

Harbormaster failed remote builds in B21677: Diff 47824!Jan 24 2019, 12:06 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

Correct link.

epriestley accepted this revision.Jan 24 2019, 12:12 AM

epriestley added inline comments.

src/applications/metamta/future/PhabricatorAmazonSNSFuture.php
4 ↗	(On Diff #47825)	Is this actually used?
5 ↗	(On Diff #47825)	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 ↗	(On Diff #47825)	(I'll hit this document a bit more broadly for SMS in general at some point, too.)
216 ↗	(On Diff #47825)	Link to SES instead of SNS?
220 ↗	(On Diff #47825)	"to the SES configuration"?
src/infrastructure/cluster/config/PhabricatorClusterMailersConfigType.php
48 ↗	(On Diff #47825)	Ah, thanks.

This revision is now accepted and ready to land.Jan 24 2019, 12:12 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
4 ↗	(On Diff #47825)	If I implemented `getProxiedFuture` correctly, I think so?
5 ↗	(On Diff #47825)	Yeah, good idea.
src/docs/user/configuration/configuring_outbound_email.diviner
216 ↗	(On Diff #47825)	Hahah I noticed both of these right after submitting.

Harbormaster completed remote builds in B21680: Diff 47827.Jan 24 2019, 12:15 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

Path

Size

scripts/

sms/

21 lines

src/

__phutil_library_map__.php

applications/

config/

option/

PhabricatorAWSConfigOptions.php

13 lines

notification/

future/

PhabricatorAWSSimpleMessagingServiceFuture.php

19 lines

management/

PhabricatorNotificationManagementSMSWorkflow.php

72 lines

PhabricatorNotificationManagementWorkflow.php

4 lines

Diff 47684

scripts/sms/manage_sms.php

Loading...

src/__phutil_library_map__.php

Loading...

src/applications/config/option/PhabricatorAWSConfigOptions.php

Loading...

src/applications/notification/future/PhabricatorAWSSimpleMessagingServiceFuture.php

Loading...

src/applications/notification/management/PhabricatorNotificationManagementSMSWorkflow.php

Loading...

src/applications/notification/management/PhabricatorNotificationManagementWorkflow.php

Loading...