Summary:
Added `renderNotificationView()` abstract function to `PhabricatorFeedStory` base class.
Fixed duplicate line in `PhabricatorFeedStoryManiphest` class.
Fixed spacing/formatting in `ManiphestTransactionEditor`.
Test Plan: No functional changes
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: allenjohnashton, ddfisher, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2698
Summary: The notification implementation has been extended to Differential. Appropriate changes have been made to the Differential editors and Differential feed story.
Test Plan: Tested out various actions available for Differential and confirmed that the notifications get delivered correctly and feed is generated.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: allenjohnashton, ddfisher, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2696
Summary:
HPHP doesn't like resolved symlinks.
Also I like this code better.
Test Plan: Used and not used custom Celerity map.
Reviewers: epriestley, nh
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2694
Summary: After D2571, feed for maniphest task creation was being generated non-full-size. Fixed this by properly getting the maniphest action from story data.
Test Plan: Feed seems to work fine and the feed for task creation is full-sized.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: allenjohnashton, ddfisher, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2691
Summary: Moved the files in notification one level up to match the rest of code base. And ran arc liberate.
Test Plan: Made sure __phutil_library_map__ points to the right files.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: allenjohnashton, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2688
Summary: Added `PhabricatorBuiltinPatchList` entry so that "storage upgrade" will update the database. Renamed and numbered the notification.sql patch.
Test Plan: Drop phabricator_feed.feed_storynotification table if it exists and run bin/storage upgrade to check if the patch is correctly applied.
Reviewers: epriestley, btrahan, allenjohnashton
Reviewed By: epriestley
CC: ddfisher, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2687
Summary:
Default of 300 seconds is more than likely too much in most cases.
Provide the option to override.
Test Plan:
Blocked ElasticSearch with iptables
Set timeout to 5 seconds and make sure we error early
Reviewers: epriestley, vrana
Reviewed By: vrana
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2678
Summary:
TransactionType gives us more information than
update, open, close, assign. We can display those in feed/notifications along with and comments on the actions.
Test Plan: did on local machine tested out.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: ddfisher, keebuhm, aran, Korvin
Differential Revision: https://secure.phabricator.com/D2683
Summary: First diff in a series of diffs to add notifications to Phabricator. This is the notification application ONLY. This commit does not include the changes to other applications that makes them add notifications. As such, no notifications will be generated beyond the initial database import.
Test Plan: This is part of the notifications architecture which has been running on http://theoryphabricator.com for the past several months.
Reviewers: epriestley, btrahan, ddfisher
Reviewed By: epriestley
CC: allenjohnashton, keebuhm, aran, Korvin, jungejason, nh
Maniphest Tasks: T974
Differential Revision: https://secure.phabricator.com/D2571
Summary:
I need this information quite often.
I don't know how many people are working on a single project and this information will be useless for them but I guess it won't hurt much?
Test Plan: Commented on accepted revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2674
Summary: These columns holds the same value in most cases which irritates me.
Test Plan: Displayed history with same author and committer, emulated different committer.
Reviewers: hsb, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2671
Summary: As per title: when browsing files in Diffusion, set "Highlighted with blame" as the default view instead of falling back to "Highlighted".
Test Plan: Tested view with a local Diffusion setup, defaults correctly.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1278
Differential Revision: https://secure.phabricator.com/D2669
Summary:
This also changes some stuff:
- Reviewers used to be at top, now they are under comments.
- Primary reviewer is now rendered as first.
Test Plan: Added `renderValueForMail()` to a custom field, created diff, commented on it and verified e-mails.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D2664
Summary:
- The config is called "resource-path" and the script references "resource-path", but the actual value checked for is "resource-map".
- Use nonempty(), since defaulting with getEnvConfig() will give you null if the setting exists but is set to null. This default is nearly useless so maybe we should change it to use coalesce().
- Remove Celerity map initialization from warmup. We don't currently initialize the environment in warmup, and Celerity initialization now depends on the environment.
Test Plan: Ran patch locally and on FPM-Warmup.
Reviewers: vrana, btrahan
Reviewed By: vrana
CC: hsb, aran
Differential Revision: https://secure.phabricator.com/D2662
Summary:
Give them a big essay about how it's dangerous, but allow them to do it formally.
Because the username is part of the password salt, users must change their passwords after a username change.
Make password reset links work for already-logged-in-users since there's no reason not to (if you have a reset link, you can log out and use it) and it's much less confusing if you get this email and are already logged in.
Depends on: D2651
Test Plan: Changed a user's username to all kinds of crazy things. Clicked reset links in email. Tried to make invalid/nonsense name changes.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1303
Differential Revision: https://secure.phabricator.com/D2657
Summary:
See T1303, which presents a reasonable case for inclusion of these characters in valid usernames.
Also, unify username validity handling.
Test Plan: Created a new user with a valid name. Tried to create a new user with an invalid name. Ran unit tests.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1303
Differential Revision: https://secure.phabricator.com/D2651
Summary:
See https://github.com/facebook/phabricator/issues/117
- The $user save can hit a duplicate key exception like the email, but we don't handle it correctly.
- When the $user saves but the $email does not, the $user is left with a (rolled-back, invalid) ID. This makes the UI glitch out a bit. Wipe the ID if we abort the transaction.
- We show the "Required" star marker even if the email is filled in.
The ID issue is sort of a general problem, but I think it's fairly rare: you must be doing inserts on related objects and the caller must catch the transaction failure and attempt to handle it in some way.
I can think of three approaches:
- Manually "roll back" the objects inside the transaction, as here. Seems OK if this really is a rare problem.
- Automatically roll back the 'id' and 'phid' columns (if they exist). Seems reasonable but maybe more complicated than necessary. Won't get every case right. For instance, if we inserted a third object here and that failed, $email would still have the userPHID set.
- Automatically roll back the entire object. We can do this by cloning all the writable fields. Seems like it might be way too magical, but maybe the right solution? Might have weird bugs with nonwritable fields and other random stuff.
We can trigger the rollback by storing objects we updated on the transaction, and either throwing them away or rolling them back on saveTransaction() / killTransaction().
These fancier approaches all seem to have some tradeoffs though, and I don't think we need to pick one yet, since this has only caused problems in one case.
Test Plan: Tried to create a new user (via People -> Create New User) with a duplicate username. Got a proper UI message with no exception and no UI glitchiness.
Reviewers: btrahan, vrana, hgrimberg, hgrimberg01
Reviewed By: hgrimberg01
CC: aran
Differential Revision: https://secure.phabricator.com/D2650
Summary: replace differential.find with differential.query and display in requested order
Test Plan: say D444D222D443D442 and ensure they are in the correct order
Reviewers: epriestley
Reviewed By: epriestley
CC: Mnkras, aran
Differential Revision: https://secure.phabricator.com/D2656
Summary:
We have custom static resources.
We currently include them in Phabricator's celerity resource map which is causing some pain - we need to regenerate the file without our custom resources before pushing upstream, we need to discard our changes before pulling from upstream and we need to rebuild with our changes to run Phabricator.
This diff allows writing and reading the map in other location.
The plan is this - I will run `celerity_mapper.php` twice - once to build Phabricator-only resources (to push to upstream) and once to build Phabricator + ours resoruces to put in our directory.
Better solution would be to create a map just with our resources and read and combine it with Phabricator resources.
But it is complicated because we have dependencies on Phabricator resources.
Test Plan:
`celerity_mapper.php webroot`
`celerity_mapper.php webroot ../facebook/src/__celerity_resource_map__.php`
Delete Phabricator's celerity map, set 'celerity.resource-path' and successfully load Phabricator.
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T721
Differential Revision: https://secure.phabricator.com/D2630
Summary:
We need to generate the URI dynamically.
This code is also generally better.
Test Plan: Created custom search selector passing the custom URI to engine.
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2655
Summary: If, e.g., $PATH is broken we may not be able to run "ps". We'll explode pretty hard, currently. Instead, just show a harsher warning.
Test Plan: Changed "ps auxwww" to "psq", which doesn't exist on my system. Loaded page, got warning instead of explosion.
Reviewers: nathanws, vrana, btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2624
Summary:
"configuration/default" moved to "default". Rename it to "configuration" and put config stuff back there.
Put writeguard into writeguard/.
Test Plan: tools
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1103
Differential Revision: https://secure.phabricator.com/D2639
Summary: These were in an unusual location, but are better back in policy/
Test Plan: implicit arc unit
Reviewers: vrana
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1103
Differential Revision: https://secure.phabricator.com/D2638
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.
NOTE: `arc diff` timed out so I'm pushing it without review.
Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.
Auditors: epriestley
Maniphest Tasks: T1103
Summary: They were only displaying seconds. I found a function in viewutils.php that allowed for single-unit precision formatting, but I wanted more, so I wrote another function to allow more detail.
Test Plan: [site]/mail, and watch it work. It's a new function, so it shouldn't break anything else.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1296
Differential Revision: https://secure.phabricator.com/D2616
Summary: Explain what lint is, how to configure it, what we ship with, etc. Also wax philosophical.
Test Plan: Generated docs, read "carefully".
Reviewers: csilvers, btrahan, vrana
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D2628
Summary: These fail on 32-bit machines and seem not valuable enough to try to make work with `bc`, etc.
Test Plan: Ran tests (depends on D2622).
Reviewers: floatinglomas, vrana, btrahan
Reviewed By: vrana
CC: aran
Maniphest Tasks: T1298
Differential Revision: https://secure.phabricator.com/D2623
Summary: Mark these actions with the same markers we use in Differential.
Test Plan: {F12094}
Reviewers: csilvers, jungejason, btrahan
Reviewed By: csilvers
CC: aran
Maniphest Tasks: T1289
Differential Revision: https://secure.phabricator.com/D2601
Summary: D2216 tried to ask the user, this one is explicit.
Test Plan: Click the button
Reviewers: epriestley, lucian
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2600
Summary: This is an example of code simplification with D2557.
Test Plan: Display user list, verify the SQL queries.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2558
Summary:
Some lint errors (e.g. Javelin) don't have a line number.
Put them on the first line.
Putting them above the first line would be even nicer but much more complicated.
Test Plan: Display diff with lint error on line 0 (D2583).
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2599
Summary: Also fix the notice text.
Test Plan:
Display diff with inline comments and lint errors.
Click on inline comment and lint link.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2594
Summary:
Allow allowed email addresses to be restricted to certain domains. This implies email must be verified.
This probably isn't QUITE ready for prime-time without a few other tweaks (better administrative tools, notably) but we're nearly there.
Test Plan:
- With no restrictions:
- Registered with OAuth
- Created an account with accountadmin
- Added an email
- With restrictions:
- Tried to OAuth register with a restricted address, was prompted to provide a valid one.
- Tried to OAuth register with a valid address, worked fine.
- Tried to accountadmin a restricted address, got blocked.
- Tried to accountadmin a valid address, worked fine.
- Tried to add a restricted address, blocked.
- Tried to add a valid address, worked fine.
- Created a user with People with an invalid address, got blocked.
- Created a user with People with a valid address, worked fine.
Reviewers: btrahan, csilvers
Reviewed By: csilvers
CC: aran, joe, csilvers
Maniphest Tasks: T1184
Differential Revision: https://secure.phabricator.com/D2581
Summary:
This adds a link to [Closed] e-mail if it detects some changes.
It compares added and removed lines with 3 lines context.
The subtle form of informing is permissive to false negatives and positives.
I have an e-mail filter for [Closed] e-mails so I wouldn't personally notice this change - we should probably promote this feature a little bit.
Test Plan:
Reparse a diff with a change after last update.
Reparse a diff without a change after last update.
Reviewers: jungejason, epriestley
Reviewed By: jungejason
CC: aran, Koolvin, btrahan
Maniphest Tasks: T201
Differential Revision: https://secure.phabricator.com/D2540
Summary:
Our code is quite complex in areas where we prevents the 1+N queries problem explained in [[ http://www.phabricator.com/docs/phabricator/article/Performance_N+1_Query_Problem.html | a performance chapter ]].
This diff adds an abstraction for preventing this code.
Test Plan:
Run all examples mentioned in the doc-comments with logging the queries.
Generate and read docs.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2557
Summary:
I thought about it a little bit and this makes the most sense for me:
# Original author usually writes at least something and commander only updates it.
# There's a creation date of revision (= first diff) by these comments. I don't want to change this date because I use this information. Author should correspond to this date.
# It solves all our repros.
Test Plan: Display commandeered revision.
Reviewers: nh, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2577
Summary: this section gets updated for each and every request. clicking a given entry updates the larger dark-console area to have the information from that request
Test Plan: clicked around in maniphest and observed request log populating correctly. clicked a few entries in request log and saw it updated properly. clicked a different tab in the dark-console and it worked. clicked a different request log entry and it opened the dark console to the proper request on the proper tab.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1136
Differential Revision: https://secure.phabricator.com/D2574
Summary:
- We currently have some bugs in account creation due to nontransactional user/email editing.
- We save $user, then try to save $email. This may fail for various reasons, commonly because the email isn't unique.
- This leaves us with a $user with no email.
- Also, logging of edits is somewhat inconsistent across various edit mechanisms.
- Move all editing to a `PhabricatorUserEditor` class.
- Handle some broken-data cases more gracefully.
Test Plan:
- Created and edited a user with `accountadmin`.
- Created a user with `add_user.php`
- Created and edited a user with People editor.
- Created a user with OAuth.
- Edited user information via Settings.
- Tried to create an OAuth user with a duplicate email address, got a proper error.
- Tried to create a user via People with a duplicate email address, got a proper error.
Reviewers: btrahan, vrana, jungejason
Reviewed By: btrahan
CC: tberman, aran
Maniphest Tasks: T1184
Differential Revision: https://secure.phabricator.com/D2569
Summary: Better solution would be to reload the page for user with valid token and all data he inserted but I guess that we don't have enough infrastructure for this.
Test Plan: Mangle token and send form.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2570
Summary:
Unittest databases are not always destroyed in our setup.
It could be caused by `__destruct()` not called in case of a fatal error.
Test Plan:
arc unit src/applications/calendar/storage/holiday
Reviewers: edward, epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2568
Summary:
- Add "role" information, so clients can identify disabled users.
- Formally deprecate `user.info`
Test Plan: Ran "user.query" and "user.whoami", inspected output. Verified "user.info" appears as deprecated in method list and console.
Reviewers: csilvers, btrahan
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2565
Summary:
Instead of assuming the test plan and summary are written by the author
of the differential revision, let's assume they are written by the author
of the latest differential diff.
Test Plan: viewed a drev that had been commandeered but not updated to check authors
Reviewers: epriestley, jungejason, vrana
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1235
Differential Revision: https://secure.phabricator.com/D2550
Summary: Just because I like it more.
Test Plan: View diff with comment from disabled user.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2525
Summary:
It's also more readable so I think it's OK.
I've also filed a bug for HHVM.
Test Plan: `arc unit` in HHVM
Reviewers: epriestley, jungejason
Reviewed By: jungejason
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2551
Summary:
- If you have an unverified primary email, we show a disabled "Primary" button right now in the "Status" column. Instead we should show an enabled "Verify" button, to allow you to re-send the verification email.
- Sort addresses in a predictable way.
Test Plan:
- Added, verified and removed a secondary email address.
- Resent verification email for primary address.
- Changed primary address.
Reviewers: btrahan, csilvers
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1184
Differential Revision: https://secure.phabricator.com/D2548
Summary: also makes the UI more general for this username + password business.
Test Plan:
- configure a phabricator repository from the svn server @asherwin provided which is configured for svn protocol with SASL
- observed phabricator failing without my patch
- upgraded my SVN client to support SASL (protip for mac users - http://www.wandisco.com/subversion/download#osx)
- applied patch to phabricator
- restarted daemons
- noted daemon success - diffusion populating nicely
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1260
Differential Revision: https://secure.phabricator.com/D2549
Summary:
Git and hg (supposedly..) differentiate between an author (who wrote the patch)
and a committer (who applied the patch).
This patch allows Phabricator to note when a patch is committed
by someone other than the Author.
Test Plan:
Created 2 accounts,
- U (Account with a PHID)
- U' (Account without a PHID)
and had them create and commit commits
testing if their username/real name would be displayed correctly in Diffusion,
- BrowserTable
- HistoryTable
- Code revision
Teztz,
A(uthor)/C(ommitter)
If it's A/A then Author committed
UL = User link (<a href="/p/username">username</a>)
UN = User name ("Firstname Lastname")
Tezt | Expected in table | Got
-------------------------------------------
A/A | UL/UL | UL/UL
A'/C | UN/UL | UN/UL
A/C' | UL/UN | UL/UN
A'/C' | UN/UN | UN/UN
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T688
Differential Revision: https://secure.phabricator.com/D2541
Summary:
If someone typos one name in a cc or reviewer list, it would be nice if we
display all of the valid names in the field when running arc diff (in addition
to the error message).
Test Plan:
used the conduit console to check that calling differential.parsecommitmessage
with a list of some valid and some invalid ccs returns a result with both an
error and a list of some ccs. Also ran arc diff with that list of ccs to check
for the correct user experience.
Reviewers: epriestley, jungejason, vrana
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2542
Summary:
This attaches commit diff to its associated revision as any other diff.
The consequence is that the revision page now shows the actual commit instead of the last diff. It may be disturbing but it is desired.
Another consequence is that lint and unit results are displayed as skipped after the revision was committed. I want to fix it somehow.
My next plan is to automatically diff against the last normal diff and include the link to this diff in commit e-mail if non-empty.
Test Plan:
reparse.php
Diff against last normal diff.
Reviewers: epriestley, jungejason
Reviewed By: jungejason
CC: aran, Koolvin, jungejason
Maniphest Tasks: T201
Differential Revision: https://secure.phabricator.com/D2530
Summary:
Allow callers to find all packages/paths owned by a given
user/project.
Test Plan:
Used conduit api page with --
user owner: PHID-USER-6ce1c976b86e5f3c34f6 (tried both loadPackagesFromProjects
true/false)
proj owner: PHID-PROJ-r5wnmmaawqsn4tvjmqm4
repo/path: E, /tfb/trunk/www/flib/privacy. Checked both path.getowners
and owners.query
(all of these are defined internally at fb)
Reviewers: epriestley, btrahan, nh
Reviewed By: btrahan
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2482
Summary:
Mercurial renamed "--only-branch" to "--branch" about two years ago. "-b" exists in both versions.
http://www.selenic.com/pipermail/mercurial-devel/2010-April/020469.html
We have a few other cases where we use features that exist only in recent Mercurial (notably, 'ancestors' in log) but we can work around this one easily.
Test Plan: Looked at a Mercurial repo in Diffusion, verified that "log -b" commands issued and that the output was correct.
Reviewers: btrahan, Makinde, ipalaus
Reviewed By: ipalaus
CC: aran
Maniphest Tasks: T1264
Differential Revision: https://secure.phabricator.com/D2533
Summary: Add support for "tests", "testplan" and "tested" as alias of "Test Plan".
Test Plan: Created a diff with test plan specified in "tests".
Reviewers: csilvers, btrahan
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2531
Summary:
It's currently possible to configure Phabricator to send mail to some address it recognizes as relating to an object.
When we receive mail from Phabricator, drop it unconditionally.
Test Plan: Wrote two emails, one with the header and one without. Piped them to `mail_handler.php`, one was dropped immediately.
Reviewers: btrahan, nh, mikaaay, jungejason
Reviewed By: jungejason
CC: aran
Differential Revision: https://secure.phabricator.com/D2529
Summary:
Since user emails aren't in the user table, we had to do extra data fetching
for handles, and the emails are only used in MetaMTA, so we move the email
code into MetaMTA and remove it from handles.
Test Plan: send test emails
Reviewers: jungejason, vrana, epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2494
Summary:
We introduced a "user.query" call recently which is only about two weeks old. Bump versions so users get a forced upgrade.
Also, we raise a fairly confusing message when the user calls a nonexistent method. This is not the intent; `class_exists()` throws. Tailor this exception more carefully.
Test Plan:
- Ran `echo {} | arc call-conduit derp.derp`, got a better exception.
- Bumped version, ran `arc list`, got told to upgrade.
Reviewers: indiefan, nh, vrana, btrahan, jungejason, Makinde
Reviewed By: vrana
CC: aran
Differential Revision: https://secure.phabricator.com/D2527
Summary: as title
Test Plan: no
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2526
Summary:
The current state is very confusing:
{F11734, size=full}
Test Plan: Display calendar for user with two different events in the same week.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2522
Summary:
Allow installs to require users to verify email addresses before they can use Phabricator. If a user logs in without a verified email address, they're given instructions to verify their address.
This isn't too useful on its own since we don't actually have arbitrary email registration, but the next step is to allow installs to restrict email to only some domains (e.g., @mycompany.com).
Test Plan:
- Verification
- Set verification requirement to `true`.
- Tried to use Phabricator with an unverified account, was told to verify.
- Tried to use Conduit, was given a verification error.
- Verified account, used Phabricator.
- Unverified account, reset password, verified implicit verification, used Phabricator.
- People Admin Interface
- Viewed as admin. Clicked "Administrate User".
- Viewed as non-admin
- Sanity Checks
- Used Conduit normally from web/CLI with a verified account.
- Logged in/out.
- Sent password reset email.
- Created a new user.
- Logged in with an unverified user but with the configuration set to off.
Reviewers: btrahan, vrana, jungejason
Reviewed By: btrahan
CC: aran, csilvers
Maniphest Tasks: T1184
Differential Revision: https://secure.phabricator.com/D2520
Summary: Ideally there should be a "send epriestley this profile" button but this is a reasonable step forward. Add a "download .xhprof profile" button to profiles, since walking through these things remotely is pretty awkward. Also expand "Excl" and "Incl" acronyms.
Test Plan: Clicked download button.
Reviewers: csilvers, btrahan, vrana
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2523
Summary:
This is a rough cut, but gets some of the basics at least. Here's what it looks like:
{F11690}
Some things that would be nice for future diffs:
- Different colors for different event types (tasks? MEETINGS?!)
- When events span across multiple days, keep them in the same row.
- Switch which month you're looking at.
- Show specific users instead of all.
- etc etc etc
Test Plan: See screenshot.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2514
Summary:
we were parsing the git log output slightly incorrectly and over-exploding on spaces. we also needed to escape the path %20 stuff`.
Not sure if there's something fancy to do given folks should reparse their repos if they are impacted by this issue.
Test Plan:
made a directory with spaces and some dummy revisions. observed diffs wouldn't load and links broken.
with patch, ran scripts/reparse.php for pertinent revisions and diffs loaded and links weren't broken.
Reviewers: floatinglomas, epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1252
Differential Revision: https://secure.phabricator.com/D2510
Summary: See T1254, until this gets paginated properly we can at least show more results. 25 is pretty anemic.
Test Plan: Tweaked limit, verified result count was affected.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1254
Differential Revision: https://secure.phabricator.com/D2513
Summary: If you have an empty value saved in the "default branch" field, we default to empty string (or null, or whatever) instead of the correct default.
Test Plan:
- Looked at a Git repo with an empty default branch, got a default to "master" instead of an error.
- Looked at a Mercurial repo with an empty default branch, got a default to "default" instead of an error.
Reviewers: btrahan, csilvers
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1237
Differential Revision: https://secure.phabricator.com/D2512
Summary:
- We currently write every PHID we generate to a table. This was motivated by two concerns:
- **Understanding Data**: At Facebook, the data was sometimes kind of a mess. You could look at a random user in the ID tool and see 9000 assocs with random binary data attached to them, pointing at a zillion other objects with no idea how any of it got there. I originally created this table to have a canonical source of truth about PHID basics, at least. In practice, our data model has been really tidy and consistent, and we don't use any of the auxiliary data in this table (or even write it). The handle abstraction is powerful and covers essentially all of the useful data in the app, and we have human-readable types in the keys. So I don't think we have a real need here, and this table isn't serving it if we do.
- **Uniqueness**: With a unique key, we can be sure they're unique, even if we get astronomically unlucky and get a collision. But every table we use them in has a unique key anyway. So we actually get pretty much nothing here, except maybe some vague guarantee that we won't reallocate a key later if the original object is deleted. But it's hard to imagine any install will ever have a collision, given that the key space is 36^20 per object type.
- We also currently use PHIDs and Users in tests sometimes. This is silly and can break (see D2461).
- Drop the PHID database.
- Introduce a "Harbormaster" database (the eventual CI tool, after Drydock).
- Add a scratch table to the Harbormaster database for doing unit test meta-tests.
- Now, PHID generation does no writes, and unit tests are isolated from the application.
- @csilvers: This should slightly improve the performance of the large query-bound tail in D2457.
Test Plan: Ran unit tests. Ran storage upgrade.
Reviewers: btrahan, vrana, jungejason
Reviewed By: btrahan
CC: csilvers, aran, nh, edward
Differential Revision: https://secure.phabricator.com/D2466
Summary: These patterns are hard-coded, allow them to match case-insenstiviely.
Test Plan: Typed "d3" and "D3", got the right object in the attach dialog.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1253
Differential Revision: https://secure.phabricator.com/D2511
Summary: ...they were broken...
Test Plan: clicked links for both SVN and Git repos and got working results
Reviewers: vrana, floatinglomas, 20after4
Reviewed By: floatinglomas
CC: aran, epriestley
Maniphest Tasks: T1250
Differential Revision: https://secure.phabricator.com/D2505
Summary:
only show the blank, "create new" wiki page for the project if the project actually exists; only allow edit if the project actually exists.
Small wrinkle here is not checking if the project actually exists if the page already exists.
Test Plan:
- viewed a project wiki page
- viewed a prokect wiki page for a fake project and got a 404
- edited a project wiki page
- edited a project wiki page for a fake project and got a 404
Reviewers: epriestley, jacktrades
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1248
Differential Revision: https://secure.phabricator.com/D2506
Summary: we weren't actually removing any edges. now we do.
Test Plan: had a commit associated with task x; removed association. had a commit associated with task x; removed association while adding a different one.
Reviewers: floatinglomas, epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1256
Differential Revision: https://secure.phabricator.com/D2509
Summary: we need a user (the viewer in this case) for the status to render correctly with respect to timezone
Test Plan: my profile no longer fatals with an away status
Reviewers: davidreuss, vrana
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2504
Summary: D2490 was not my finest hour and I incorrectly thought it was a null value from error. In reality this error is impossible and its just a valid empty array so instead use the empty predicate to bail.
Test Plan: with our logic combined, this be tested
Reviewers: vrana, epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2503
Summary: see T1241, T1242, T1244 for some examples of crud getting saved
Test Plan: threw some crud in my conduit console and got reasonable errors back
Reviewers: mikaaay, epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1241, T1242, T1244
Differential Revision: https://secure.phabricator.com/D2487
Summary: D2492#6
Test Plan:
`user.query` of user which is away.
`user.query` of user which is not away.
`user.whoami` - no information there.
Reviewers: epriestley
Reviewed By: epriestley
CC: btrahan, aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2502
Summary: there were 6 values, but we only need 4. delete the two empty ones as the others are all stylistically valid.
Test Plan: ...looks good!
Reviewers: ric03uec, vrana, epriestley
Reviewed By: epriestley
CC: aran, epriestley
Maniphest Tasks: T1247
Differential Revision: https://secure.phabricator.com/D2498
Summary: There's no method for enabling users somewhat intentionally.
Test Plan: Disable myself (oops, this is probably my last diff ever).
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2497
Test Plan: Display revision list both with last reviewer and without.
Reviewers: epriestley, btrahan
Reviewed By: btrahan
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2495
Summary:
This is not so general as `getRequiredHandlePHIDs()`.
It allows bulk loading of user statuses only in revision list.
It also loads data in `render()`. I'm not sure if it's OK.
Maybe we can use the colorful point here.
Or maybe some unicode symbol?
Test Plan: {F11451, size=full}
Reviewers: btrahan, epriestley
Reviewed By: btrahan
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2484
Summary:
I want to use this to warn user if he specifies reviewers that are away.
We can also implement a general query method but I think that this usage is the
most useful not only for me but also in general case.
Test Plan:
Call the method for user which is away and which is not away.
Add user status through Conduit.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley
Differential Revision: https://secure.phabricator.com/D2492
Summary: 'cuz github issue 114 came into existence. instead, just return false early here. note i am not sure if I should phlog that this is happening or not but its not exception worthy IMO.
Test Plan: lint-only 'cuz i don't want to setup an IRC server locally / somehow get my local phabricator instance accessible out there. happy to test end to end if there's an easier way...!
Reviewers: vrana, epriestley
Reviewed By: vrana
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2490
Summary:
I've tried colors, italics, strike-through and this is a compromise between me and @leebyron.
NOTE: I don't want to disturb @epriestley from playing Diablo 3.
Test Plan: {F11439, size=full}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, epriestley, leebyron
Differential Revision: https://secure.phabricator.com/D2481
Summary:
Occasionally a bot will get subscribed to a differential revision; when
this happens we shouldn't send them an email, because otherwise the
author of the diff will get a bounce email if the bot has an invalid
emal address.
Test Plan:
re-sent a message that had a bot on the cc list, saw that it was no longer
included when it got sent.
Reviewers: epriestley, jungejason, vrana
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2479
Summary: the existing, more custom text doesn't make sense when viewing someone else's revisions AND in someplaces its the less interesting "no data found".
Test Plan: clicked around the various http://phabricator.dev/differential/filter/* pages with various users.
Reviewers: epriestley, ry
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1222
Differential Revision: https://secure.phabricator.com/D2475
Summary:
D2470 added Package mailhandler, which was configured incorrectly in the getRequiredClasses function. this makes it like the other mail handlers
Reported at https://github.com/facebook/phabricator/issues/112
Test Plan: setup mode no longer fails
Reviewers: epriestley, jungejason, royklopper
Reviewed By: royklopper
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2476
Summary:
D2457 and D2459 made some changes here. This little guy just needed to be touched so it could be arc lint'd to have the proper updated include paths
Sorry to spam reviewers; Evan is in Diablo 3 somewhere. :D
Test Plan: no more fatal on test everything implemented (arc unit src/infrastructure/__tests__/)
Reviewers: epriestley, jungejason, nh, csilvers
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2474
Summary:
For package creation and deletion, send email to all the owners For
package modification, detect important fields such as owners and paths, and then
send out emails to all owners (including deleted owners and current owners)
Also start using transaction for package creation/deletion/modification.
Test Plan:
- tested mail creation and deletion
- tested modification to auditing enabled, primary owners, owners, paths
Reviewers: epriestley, nh, vrana
Reviewed By: epriestley
CC: prithvi, aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2470
Summary:
Required in order to run tests successfully in the HipHop interpreter.
Similar to D2362.
Test Plan: Run the tests in an HipHop runtime.
Reviewers: epriestley
Reviewed By: epriestley
CC: jungejason, aran, Koolvin, vrana
Differential Revision: https://secure.phabricator.com/D2365
Summary: We had a real issue where revision was marked as accepted but the comment wasn't saved.
Test Plan: Reclaim abandoned revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: jungejason, aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2463
Summary: Prelminary, I want to test this a bit more when I'm better rested. Provide a "bulk" import mode for Mercurial so we can do initial discovery more quickly.
Test Plan: Imported the Mercurial repository in 2m45s, blocked on MySQL I/O rather than Mercurial I/O.
Reviewers: csilvers, btrahan
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2457
Summary:
Also add some formatting and links.
Also fix test broken by D2393.
Test Plan:
diviner .
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Differential Revision: https://secure.phabricator.com/D2461
Summary: This could use some additional explanation.
Test Plan: Read text.
Reviewers: btrahan, aurelijus
Reviewed By: aurelijus
CC: aran
Maniphest Tasks: T1204
Differential Revision: https://secure.phabricator.com/D2446
Summary: For git workflows where developers push personal feature branches to the origin, this is a quick workaround until T1210 is implemented properly. I'll also mention this in D2446.
Test Plan:
- Committed a revision in an "autoclose" repository; it was autoclosed. Committed a revision in a no-autoclose repository, it was not autoclosed.
- Edited repositories, saving the "autoclose" setting as enabled/disabled.
Reviewers: aurelijus, btrahan
Reviewed By: aurelijus
CC: aran
Maniphest Tasks: T1210
Differential Revision: https://secure.phabricator.com/D2448
Summary:
- Support offset/limit as added by D2442, for Mercurial.
- We just list everything and slice, no clear way to do better and this isn't a major perf issue.
- No clear way to easily get by-update sorting, we can implement this by looking up revs if someone asks.
- Bury some of the Git implementation details inside the Git query.
Test Plan:
- Looked at a Git repo, clicked "View All Branches".
- Looked at a Hg repo, clicked "View All Branches".
- Limited page size to 1, made sure it limited properly.
Reviewers: aurelijus, btrahan, vrana
Reviewed By: aurelijus
CC: aran
Differential Revision: https://secure.phabricator.com/D2453
Summary:
Sorting by last commit date
Branch view limit to 25 branches
All branches table page with pagination on Git
Test Plan:
* Check repository view for expected behavior on branch table view
* Check all branches page & test pagination
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1200
Differential Revision: https://secure.phabricator.com/D2442
Summary: Revisions don't necessarily have a backing repository, as in "--raw".
Test Plan: Looked at a "--raw" revision in accepted and closed states.
Reviewers: nh, btrahan
Reviewed By: nh
CC: aran
Differential Revision: https://secure.phabricator.com/D2444
Summary:
The highlighting is distracting according to Nick Shrock and others.
Real designer, Lee Byron, helped me with this.
It also gives us unagressive target for jumping to the source line in future.
Another feature I will probably implement is highlighting also the source of copies/moves.
I will use the right side of the left column for it.
Test Plan:
Hover copied notifier.
Hover coverage notifier.
I've also checked that this doesn't break our super-flaky old/new code JavaScript detector.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin, leebyron, schrockn
Differential Revision: https://secure.phabricator.com/D2443
Summary:
- When you have an un-cloned repository, we currently throw random-looking Git/Hg exception. Instead, throw a useful error.
- When you have a cloned but undiscovered repository, we show no commits. This is crazy confusing. Instead, show commits as "importing...".
- Fix some warnings and errors for empty path table cases, etc.
Test Plan:
- Wiped database.
- Added Mercurial repo without running daemons. Viewed in Diffusion, got a good exception.
- Pulled Mercurial repo without discovering it. Got "Importing...".
- Discovered Mercurial repo without parsing it. Got "Importing..." plus date information.
- Parsed Mercurial repo, got everything working properly.
- Added Git repo without running daemons, did all the stuff above, same results.
- This doesn't improve SVN much but that's a trickier case since we don't actually make SVN calls and rely only on the parse state.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T776
Differential Revision: https://secure.phabricator.com/D2439
Summary:
- Merge CommitTask daemon into PullLocal daemon. This is another artifact of past instability (and order-dependent parsers). We still publish to the timeline, although this was the last consumer. Long term we'll probably delete timeline and move to webhooks, since everyone who has asked about this stuff has been eager to trade away the durability and ordering of the timeline for the ease of use of webhooks. There's also no reason to timeline this anymore since parsing is no longer order-dependent.
- Add `phd start` to start all the daemons you need. Add `phd restart` to restart all the daemons you need. So cool~
- Simplify and improve phd and Diffusion daemon documentation.
Test Plan:
- Ran `phd start`.
- Ran `phd restart`.
- Generated/read documentation.
- Imported some stuff, got clean parses.
Reviewers: btrahan, csilvers
Reviewed By: csilvers
CC: aran, jungejason, nh
Differential Revision: https://secure.phabricator.com/D2433
Summary: We raise an improved exception for missing tables/columns, but not databases.
Test Plan: Hit a "no such database error", got a better error message pointing me at storage upgrades.
Reviewers: btrahan, jungejason
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D2429
Summary: When the server version is ahead of the client version, send a more exciting error!!!
Test Plan: omg~~~
Reviewers: nh, Makinde, btrahan, jungejason
Reviewed By: nh
CC: aran
Differential Revision: https://secure.phabricator.com/D2436
Summary: Add history column & history link per branch on branch table view, also later add some more features like last commit date, etc.
Test Plan: Checked if History column is in browser table view & history links are properly linked.
Reviewers: epriestley
Reviewed By: epriestley
CC: davidreuss, aran, Koolvin
Maniphest Tasks: T1201, T1202, T1200
Differential Revision: https://secure.phabricator.com/D2432
Summary:
See D2418. This merges the commit discovery daemon into the same single daemon, and applies all the same rules to it.
There are relatively few implementation changes, but a few things did change:
- I simplified/improved Mercurial importing, by finding full branch tip hashes with "--debug branches" and using "parents --template {node}" so we don't need to do separate "--debug id" calls.
- Added a new "--not" flag to exclude repositories, since I switched to real arg parsing anyway.
- I removed a web UI notification that you need to restart the daemons, this is no longer true.
- I added a web UI notification that no pull daemon is running on the machine.
NOTE: @makinde, this doesn't change anything from your perspective, but it something breaks this is the likely cause.
This implicitly resolves T792, because discovery no longer runs before pulling.
Test Plan:
- Swapped databases to a fresh install.
- Ran "pulllocal" in debug mode. Verified it correctly does nothing (fixed a minor issue with min() on empty array).
- Added an SVN repository. Verified it cloned and discovered correctly.
- Added a Mercurial repository. Verified it cloned and discovered correctly.
- Added a Git repository. Verified it cloned and discovered correctly.
- Ran with arguments to verify behaviors: "--not MTEST --not STEST", "P --no-discovery", "P".
Reviewers: btrahan, csilvers, Makinde
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T792
Differential Revision: https://secure.phabricator.com/D2430
Summary:
also fix some bugs where we weren't properly capturing the expiry value or scope of access tokens.
This code isn't the cleanest as some providers don't confirm what scope you've been granted. In that case, assume the access token is of the minimum scope Phabricator requires. This seems more useful to me as only Phabricator at the moment really easily / consistently lets the user increase / decrease the granted scope so its basically always the correct assumption at the time we make it.
Test Plan: linked and unlinked Phabricator, Github, Disqus and Facebook accounts from Phabricator instaneces
Reviewers: epriestley
Reviewed By: epriestley
CC: zeeg, aran, Koolvin
Maniphest Tasks: T1110
Differential Revision: https://secure.phabricator.com/D2431
Summary:
For most actions (like "accept"), we write a row only if you aren't acting on behalf of anything else. This avoids cases like every accept causing two relationships:
Some Project | Accept
Some User | Accept
For "Resign", we must always write the row. Break the logic out and handle it separately.
Test Plan: Poked it locally, but let me know if this fixes things?
Reviewers: 20after4, btrahan
Reviewed By: 20after4
CC: aran
Differential Revision: https://secure.phabricator.com/D2423
Summary: Mention that you don't actually need an `.arcconfig` any more.
Test Plan: Read documentation.
Reviewers: btrahan, csilvers
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2425
Summary:
Allow the pull daemon to take a list of repositories. By default, pull all repositories.
Make some effort to respect pull frequencies, although we'll necessarily suffer a bit if running with only one process.
NOTE: We still launch one discovery daemon per working copy, so this only cuts the daemon count in half.
Test Plan:
- Ran `phd debug pulllocal`, verified behavior.
- Ran `pull.php P MTEST SVNTEST --trace`, verified it pulled the repos and ran the right commands.
- Ran `phd repository-launch-master`, verified the right daemons launched, checked daemon console.
- Ran `phd repository-launch-readonly`, verified the right daemon launched, checked daemon console.
Reviewers: btrahan, csilvers, davidreuss
Reviewed By: csilvers
CC: aran
Differential Revision: https://secure.phabricator.com/D2418
Summary: specify user table to make things not ambiguous
Test Plan: conduit console still works, including ID query...!
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Koolvin
Maniphest Tasks: T1075
Differential Revision: https://secure.phabricator.com/D2419
Summary:
When you create a new task, the UI gives you the option to create another similar task. We copy some fields, but not others.
Currently, the field list is hard-coded and excludes auxiliary fields. Instead, allow auxiliary fields to elect to be copied.
Test Plan:
- Created a new task, verified appropriate field defuaults.
- Created a new "similar" task, verified 'copy' fields copied in.
- Edited an existing task, verified appropriate values.
- Edited-with-errors, verified new values didn't get reverted in the form.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1193
Differential Revision: https://secure.phabricator.com/D2410
Summary: The PHID list is a list, not a map -- I must have broken this in refactoring or something, since everything else works fine. See D2013.
Test Plan: Viewed a resignable revision, saw "Resign" (new after this commit), resignd.
Reviewers: 20after4, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T904
Differential Revision: https://secure.phabricator.com/D2417
Summary:
- Move email to a separate table.
- Migrate existing email to new storage.
- Allow users to add and remove email addresses.
- Allow users to verify email addresses.
- Allow users to change their primary email address.
- Convert all the registration/reset/login code to understand these changes.
- There are a few security considerations here but I think I've addressed them. Principally, it is important to never let a user acquire a verified email address they don't actually own. We ensure this by tightening the scoping of token generation rules to be (user, email) specific.
- This should have essentially zero impact on Facebook, but may require some minor changes in the registration code -- I don't exactly remember how it is set up.
Not included here (next steps):
- Allow configuration to restrict email to certain domains.
- Allow configuration to require validated email.
Test Plan:
This is a fairly extensive, difficult-to-test change.
- From "Email Addresses" interface:
- Added new email (verified email verifications sent).
- Changed primary email (verified old/new notificactions sent).
- Resent verification emails (verified they sent).
- Removed email.
- Tried to add already-owned email.
- Created new users with "accountadmin". Edited existing users with "accountadmin".
- Created new users with "add_user.php".
- Created new users with web interface.
- Clicked welcome email link, verified it verified email.
- Reset password.
- Linked/unlinked oauth accounts.
- Logged in with oauth account.
- Logged in with email.
- Registered with Oauth account.
- Tried to register with OAuth account with duplicate email.
- Verified errors for email verification with bad tokens, etc.
Reviewers: btrahan, vrana, jungejason
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1184
Differential Revision: https://secure.phabricator.com/D2393
Summary: The various interfaces here are in conflict about what a role is and isn't. Make them all consistent.
Test Plan: Edited some users into various roles, verified they reported correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1190
Differential Revision: https://secure.phabricator.com/D2415
