Page MenuHomePhabricator

Add email invites to Phabricator (logic only)
ClosedPublic

Authored by epriestley on Feb 9 2015, 9:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 22, 1:35 PM
Unknown Object (File)
Sun, Dec 22, 1:35 PM
Unknown Object (File)
Thu, Dec 19, 8:53 PM
Unknown Object (File)
Tue, Dec 17, 1:21 PM
Unknown Object (File)
Thu, Dec 12, 3:54 AM
Unknown Object (File)
Wed, Dec 11, 11:21 PM
Unknown Object (File)
Wed, Dec 11, 9:39 PM
Unknown Object (File)
Sat, Dec 7, 4:24 PM
Subscribers

Details

Summary

Ref T7152. This builds the core of email invites and implements all the hard logic for them, covering it with a pile of tests.

There's no UI to create these yet, so users can't actually get invites (and administrators can't send them).

This stuff is a complicated mess because there are so many interactions between accounts, email addresses, email verification, email primary-ness, and user verification. However, I think I got it right and got test coverage everwhere.

The degree to which this is exception-driven is a little icky, but I think it's a reasonable way to get the testability we want while still making it hard for callers to get the flow wrong. In particular, I expect there to be at least two callers (one invite flow in the upstream, and one derived invite flow in Instances) so I believe there is merit in burying as much of this logic inside the Engine as is reasonably possible.

Test Plan

Unit tests only.

Diff Detail

Repository
rP Phabricator
Branch
invite
Lint
Lint Passed
SeverityLocationCodeMessage
Advicesrc/applications/auth/controller/PhabricatorAuthInviteController.php:52XHP16TODO Comment
Unit
Tests Passed
Build Status
Buildable 4413
Build 4426: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Add email invites to Phabricator (logic only).
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.

awesometown

src/applications/auth/engine/PhabricatorAuthInviteEngine.php
199–203

ACTION_STEAL_EMAIL in the user log maybe?

src/applications/auth/factor/__tests__/PhabricatorAuthInviteTestCase.php
248

is it worth making sure the email steal went correctly in this case too?

This revision is now accepted and ready to land.Feb 9 2015, 9:39 PM

Ah, those are good points. I'll drop email theft into the UserEditor, get it logging, and add some extra test coverage.

epriestley edited edge metadata.
  • Move email reassignment to UserEditor.
  • Log email reassignment.
  • Test that emails were reassigned.
  • Instead of deleting invites, mark them accepted (and store the accepting account).
  • Test that invites get marked accepted.
  • Prevent duplicate accepts.
  • Test duplicate accepts.
  • Make /auth/invite/asdf/ get slightly further.
This revision was automatically updated to reflect the committed changes.