Summary:
Ref T2222. Ref T1460. Depends on D6260.
This creates the new tables, but doesn't start using them. I added three new fields for {T1460}, to represent fixed/done/replied states.
Test Plan: Ran storage upgrade.
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T1460, T2222
Differential Revision: https://secure.phabricator.com/D6261
Summary:
Ref T2222.
I'm thinking about how I want to approach the Asana sync, and I want to try to do T2222 first so that we can build it cleanly on top of ApplicationTransactions. I think we can at least walk down this road a little bit and if it turns out to be scary we can take another approach.
I was generally very happy with how the auth migration turned out (seemingly, it was almost completely clean), and want to pursue a similar strategy here. Basically:
- Wrap the new objects in the old objects for reads/writes.
- Migrate all the existing data to the new table.
- Everything hard is done; move things over a piece at a time at a leisurely pace in lots of smallish, relatively-easy-to-understand changes.
This deletes or abstracts all reads of the DifferentialComment table. In particular, these things are **deleted**:
- The script `undo_commits.php`, which I haven't pointed anyone at in a very long time.
- The `differential.getrevisionfeedback` Conduit method, which has been marked deprecated for a year or more.
- The `/stats/` interface in Differential, which should be rebuilt on Fact and has never been exposed in the UI. It does a ton of joins and such which are prohibitively difficult to migrate.
This leaves a small number of reading interfaces, which I replaced with a new `DifferentialCommentQuery`. Some future change will make this actually load transactions and wrap them with DifferentialComment interfaces.
Test Plan: Viewed a revision; made revision comments
Reviewers: btrahan
Reviewed By: btrahan
CC: edward, chad, aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D6260
Summary: Ref T1536. Like Google, GitHub is actually strict about callback URIs too. Keep them pointed at the old URIs until we can gradually migrate.
Test Plan: Logged in with GitHub.
Reviewers: garoevans, davidreuss, btrahan
Reviewed By: garoevans
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6265
Summary: Removes extra padding on rendering notifications in jx-notification.
Test Plan: test a notification
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6259
Summary: Changes it to a dialog view, tweaks some layout bugs on full width forms.
Test Plan: Tested loging in and resetting my password. Chrome + Mobile
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, nrp
Differential Revision: https://secure.phabricator.com/D6257
Summary: Used Differential emails as a formatting guide. This also includes "Image X: <inline comment>" similarly to how Differential has "<file path><line(s)>: <inline comment>" or what have you. Fixes T3138.
Test Plan: inserted some debugging code and verified the mail body format looked okay ish
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3138
Differential Revision: https://secure.phabricator.com/D6258
Summary: in applyExternalEffects, for subscriber transactions, we now re-load subscribers. also fixes a bug where a user can get emailed 2x when they take an action on a mock they created.
Test Plan: made some mocks. verified one copy sent to creator and one to each subscriber. (note having problems with email so I verified the phids mail was supposed to be sent to and did not get the actual email delivered)
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3315
Differential Revision: https://secure.phabricator.com/D6206
Summary:
Ref T1536.
- When users try to add a one-of provider which already exists, give them a better error (a dialog explaining what's up with reasonable choices).
- Disable such providers and label why they're disabled on the "new provider" screen.
Test Plan:
{F47012}
{F47013}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6256
Summary: Ref T1536. Mostly, this puts "username/password" (which is probably a common selection) first on the list.
Test Plan: {F47010}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6254
Summary: Ref T1536. This "should never happen", but can if you're developing custom providers. Improve the robustness of this interface in the presence of missing provider implementations.
Test Plan: {F47008}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6253
Summary: Not sure how to test this, but assume it's coming from this hover.
Test Plan: n/a
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3425
Differential Revision: https://secure.phabricator.com/D6255
Summary: Generally prefer 'cards' to represent individual 'items' or 'action items', so I think it works here.
Test Plan: Reload setup issues pages.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6252
Summary:
Ref T1536.
- Move all the provider-specific help into contextual help in Auth.
- This provides help much more contextually, and we can just tell the user the right values to use to configure things.
- Rewrite account/registration help to reflect the newer state of the word.
- Also clean up a few other loose ends.
Test Plan: {F46937}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6247
Summary: Ref T1536. This is the last major migration. Moves us over to the DB and drops all the config stuff.
Test Plan:
- Ran the migration.
- Saw all my old config brought forward and respected, with accurate settings.
- Ran LDAP import.
- Grepped for all removed config options.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran, wez
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6243
Summary:
Ref T1536. This sets us for the "Config -> Database" migration. Basically:
- If stuff is defined in the database, respect the database stuff (no installs have anything defined yet since they can't reach the interfaces/code).
- Otherwise, respect the config stuff (all installs currently do this).
Test Plan: Saw database stuff respected when database stuff was defined; saw config stuff respected otherwise.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6240
Summary:
Adds the phids of users entered into any user project query to handles phids for handle loading
Fixes T3395
Test Plan: Load page that was previous breaking
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3395
Differential Revision: https://secure.phabricator.com/D6250
Summary:
D6057 introduced images in the typeahead results, but not all
projects return a valid result. This silently broke /owners/new because
the exception "Call to a member function loadProfileImageURI() on a non-object"
is swallowed somewhere in the handler.
Test Plan: go to /owners/new and type something in the primary owner field
Reviewers: epriestley, nh, Afaque_Hussain
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6245
Summary: Added in color variables in most used places. Tweaked green to be a bit more serious.
Test Plan: Tested Tags, Error View, Timeline, Object Views, and Color Palette.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6244
Summary:
Ref T1536. Currently, when you install Phabricator you're dumped on the login screen and have to consult the documentation to learn about `bin/accountadmin`.
Instead, detect that an install is running first-time setup:
- It has no configured providers; and
- it has no user accounts.
We can safely deduce that such an install isn't configured yet, and let the user create an admin account from the web UI.
After they login, we raise a setup issue and lead them to configure authentication.
(This could probably use some UI and copy tweaks.)
Test Plan:
{F46738}
{F46739}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6228
Summary: Remade auth and policy icon.
Test Plan: look at the images.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6241
Summary: This adds an 83% Light set of colors for highlights, warnings, etc.
Test Plan: Tested Notifications, Error View, and Color Palette page. Test is out, not quite sure on notifications.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6239
Summary: See inlines.
Test Plan: Loaded timeline UIExample.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6238
Summary: Ref T1536. Make this UI a bit more human-friendly.
Test Plan: {F46873}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6237
Summary: Moves old auth icon to 'policy' and new icon is keys.
Test Plan: photoshop
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6236
Summary: We end up with both "user.id" and "email.id". Disambiguate for ORDER.
Test Plan: Ran Conduit user.query query with "email".
Reviewers: wez, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6234
Summary:
Ref T1536. This is extremely reachable and changes the login code to the new stuff.
Notes:
- I've hard-disabled password registration since I want installs to explicitly flip it on via config if they want it. New installs will get it by default in the future, but old installs shouldn't have their auth options change.
- Google doesn't let us change the redirect URI, so keep the old one working.
- We need to keep a bit of LDAP around for now for LDAP import.
- **Facebook:** This causes substantive changes in what login code is executed.
Test Plan:
- Logged in / logged out / registered, hit new flows.
- Logged in with google.
- Verified no password registration by default.
Reviewers: btrahan, chad
Reviewed By: chad
CC: wez, nh, aran, mbishopim3
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6222
Summary:
Ref T1536. Because Facebook publishes data from Phabricator to user profiles and that data is sensitive, it wants to require secure browsing to be enabled in order to login.
Respect the existing option, and support it in the UI.
The UI part isn't reachable yet.
Test Plan: {F46723}
Reviewers: chad, btrahan
Reviewed By: chad
CC: arice, wez, aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6224
Summary: Picked a set of standard colors. Based on our current Maniphest color set, but tweaked to the same hue with http://color.hailpixel.com/
Test Plan: Not intended to be end all be all, but a decent first cut. Applied to Maniphest and Tags.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6229
Summary: Ref T1536. Love me some LDAP.
Test Plan: Viewed and edited form. Looked through transactions.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6227
Summary:
Ref T1536.
- When we render a dialog on a page by itself, put it on a dust background.
- Currently, we render "Logout" in two different places. Stop doing that.
- Make sure the surviving one has workflow so we get a modal ajax dialog if possible.
Test Plan: {F46731}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6226
Summary: Ref T1536. If you only have button-based logins, the new login screen looks weird.
Test Plan:
Before
{F46725}
After
{F46726}
Reviewers: chad, jamesr
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6225
Summary:
Ref T1536. None of this code is reachable.
For the new web UI for auth edits, give providers more and better customization options for handling the form. Allow them to format transactions.
Also fix the "Auth" application icon.
Test Plan: {F46718}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6221
Summary: Touches a lot of little spacing things here and there, stuck to 4px grid when possible, checked mobile views.
Test Plan: Mobile, Logging In, Multiple Providers.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6220
Summary:
Ref T1536. Above, we load all providers, which is intentional (if a user has a link with a previously-enabled but now-disabled provider, we should enrich it with provider information).
However, before showing linking options we should drop disabled providers.
Test Plan: Disabled Disqus, reloaded, didn't see Disqus anymore.
Reviewers: mbishopim3
Reviewed By: mbishopim3
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6219
Summary:
Ref T1536.
- When linking accounts after initially failing, make the crumb say "Link Account" instead of "Login".
- When on the LDAP failure form, show a "Cancel" button returning to start (if logging in) or settings (if linking accounts).
- Allow providers to distinguish between "start", "login" and "link" rendering.
Test Plan: Linked and logged in with LDAP and other registration mechainsms.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6214
Summary:
Ref T1536.
- Add a "Cancel" button, to get back to login.
- Add a crumb showing the registering provider.
- Add an account card when registering with an external account
- Tailor some language to make it less ambiguous ("Phabricator Username", "Register Phabricator Account").
Test Plan:
{F46618}
{F46619}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6213
Summary: Ref T1536. This gets the single queries out of the View and builds a propery Query class for ExternalAccount.
Test Plan: Linked/unlinked accounts, logged out, logged in.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6212
Summary: Ref T1536. This script basically exists to restore access if/when users shoot themselves in the foot by disabling all auth providers and can no longer log in.
Test Plan: {F46411}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6205
Summary: Ref T1536. When auth providers are edited, show the edit history.
Test Plan: {F46400}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6203
Summary: Ref T1536. Many rough / broken edges, but adds the rough skeleton of the provider edit workflow.
Test Plan: {F46333}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6200
Summary: Ref T1536. Adds an initial "choose a provider type" screen for adding a new provider. This doesn't go anywhere yet.
Test Plan: {F46316}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6199
Summary:
Ref T1536. Currently, we have about 40 auth-related configuration options. This is already roughly 20% of our config, and we want to add more providers. Additionally, we want to turn some of these auth options into multi-auth options (e.g., allow multiple Phabricator OAuth installs, or, theoretically multiple LDAP servers).
I'm going to move this into a separate "Auth" tool with a minimal CLI (`bin/auth`) interface and a more full web interface. Roughly:
- Administrators will use the app to manage authentication providers.
- The `bin/auth` CLI will provide a safety hatch if you lock yourself out by disabling all usable providers somehow.
- We'll migrate existing configuration into the app and remove it.
General goals:
- Make it much easier to configure authentication by providing an interface for it.
- Make it easier to configure everything else by reducing the total number of available options.
Test Plan: Ran storage upgrade.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6196
Summary: Ref T1536. These can probably use some design tweaking and there's a bit of a bug with profile images for some providers, but generally seems to be in the right ballpark.
Test Plan:
{F46604}
{F46605}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6210
Summary:
Ref T1536. Currently, we have separate panels for each link/unlink and separate controllers for OAuth vs LDAP.
Instead, provide a single "External Accounts" panel which shows all linked accounts and allows you to link/unlink more easily.
Move link/unlink over to a full externalaccount-based workflow.
Test Plan:
- Linked and unlinked OAuth accounts.
- Linked and unlinked LDAP accounts.
- Registered new accounts.
- Exercised most/all of the error cases.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran, mbishopim3
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6189
Summary:
Ref T1536. Facebook currently does a check which should be on-login in registration hooks, and this is generally a reasonable hook to provide.
The "will login" event allows listeners to reject or modify a login, or just log it or whatever.
NOTE: This doesn't cover non-web logins right now -- notably Conduit. That's presumably fine.
(This can't land for a while, it depends on about 10 uncommitted revisions.)
Test Plan: Logged out and in again.
Reviewers: wez, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6202
Summary: Ref T1536. Error state is a bit gross but we need to sort that out in general.
Test Plan:
{F46549}
{F46550}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6208
Summary: Ref T1536. Like D6080, we don't need to store the registration key itself. This prevents a theoretical attacker who can read the database but not write to it from hijacking registrations.
Test Plan: Registered a new account.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6188
Summary:
Ref T1445. Ref T1536. Although we have separate CSRF protection and have never been vulnerable to OAuth hijacking, properly implementing the "state" parameter provides a little more certainty.
Before OAuth, we set a random value on the client, and pass its hash as the "state" parameter. Upon return, validate that (a) the user has a nonempty "phcid" cookie and (b) the OAuth endpoint passed back the correct state (the hash of that cookie).
Test Plan: Logged in with all OAuth providers, which all apparently support `state`.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, arice
Maniphest Tasks: T1445, T1536
Differential Revision: https://secure.phabricator.com/D6179
Summary: Ref T1536. We can safely replace the old login validation controller with this new one, and reduce code dplication while we're at it.
Test Plan: Logged in with LDAP, logged in with OAuth, logged in with username/password, did a password reset.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6178
Summary:
Ref T1536. LDAP is very likely the worst thing in existence.
This has some rough edges (error handling isn't perfect) but is already better than the current LDAP experience! durrr
Test Plan: Registered and logged in using LDAP.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, mbishopim3
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6177
Summary: Ref T1536. Support for GitHub on new flows.
Test Plan: Registered and logged in with GitHub.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6166
Summary: Ref T1536. Adds Disqus as a Provider.
Test Plan: Registered and logged in with Disqus.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6165
Summary:
Ref T1536. Ref T1930. Code is not reachable.
This provides password authentication and registration on the new provider/adapter framework.
I sort of cheated a little bit and don't really route any password logic through the adapter (instead, this provider uses an empty adapter and just sets the type/domain on it). I think the right way to do this //conceptually// is to treat username/passwords as an external black box which the adapter communicates with. However, this creates a lot of practical implementation and UX problems:
- There would basically be two steps -- in the first one, you interact with the "password black box", which behaves like an OAuth provider. This produces some ExternalAccount associated with the username/password pair, then we go into normal registration.
- In normal registration, we'd proceed normally.
This means:
- The registration flow would be split into two parts, one where you select a username/password (interacting with the black box) and one where you actually register (interacting with the generic flow). This is unusual and probably confusing for users.
- We would need to do a lot of re-hashing of passwords, since passwords currently depend on the username and user PHID, which won't exist yet during registration or the "black box" phase. This is a big mess I don't want to deal with.
- We hit a weird condition where two users complete step 1 with the same username but don't complete step 2 yet. The box knows about two different copies of the username, with two different passwords. When we arrive at step 2 the second time we have a lot of bad choices about how to reoslve it, most of which create security problems. The most stragihtforward and "pure" way to resolve the issues is to put password-auth usernames in a separate space, but this would be incredibly confusuing to users (your login name might not be the same as your username, which is bizarre).
- If we change this, we need to update all the other password-related code, which I don't want to bother with (at least for now).
Instead, let registration know about a "default" registration controller (which is always password, if enabled), and let it require a password. This gives us a much simpler (albeit slightly less pure) implementation:
- All the fields are on one form.
- Password adapter is just a shell.
- Password provider does the heavy lifting.
We might make this more pure at some point, but I'm generally pretty satisfied with this.
This doesn't implement the brute-force CAPTCHA protection, that will be coming soon.
Test Plan: Registered with password only and logged in with a password. Hit various error conditions.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T1536, T1930
Differential Revision: https://secure.phabricator.com/D6164
Summary:
Ref T1536. None of this code is reachable.
Implements new-auth login (so you can actually login) and login validation (which checks that cookies were set correctly).
Test Plan: Manually enabled FB auth, went through the auth flow to login/logout. Manually hit most of the validation errors.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6162
Summary:
Ref T1536. Code is intentionally made unreachable (see PhabricatorAuthProviderOAuthFacebook->isEnabled()).
This adds:
- A provider-driven "start" screen (this has the list of ways you can login/register).
- Registration actually works.
- Facebook OAuth works.
@chad, do you have any design ideas on the start screen? I think we poked at it before, but the big issue was that there were a limitless number of providers. Today, we have:
- Password
- LDAP
- Facebook
- GitHub
- Phabricator
- Disqus
- Google
We plan to add:
- Asana
- An arbitrary number of additional instances of Phabricator
Users want to add:
- OpenID
- Custom providers
And I'd like to have these at some point:
- Stripe
- WePay
- Amazon
- Bitbucket
So basically any UI for this has to accommodate 300 zillion auth options. I don't think we need to solve any UX problems here (realistically, installs enable 1-2 auth options and users don't actually face an overwhelming number of choices) but making the login forms less ugly would be nice. No combination of prebuilt elements seems to look very good for this use case.
Test Plan: Registered a new acount with Facebook.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6161
Summary:
Ref T1536. None of this code is reachable.
`PhabricatorAuthLoginController` provides a completely generic login/link flow, similar to how D6155 provides a generic registration flow.
`PhabricatorAuthProvider` wraps a `PhutilAuthAdapter` and glues the generic top-level flow to a concrete authentication provider.
Test Plan: Static only, code isn't meaningfully reachable.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6159
Summary:
Currently, registration and authentication are pretty messy. Two concrete problems:
- The `PhabricatorLDAPRegistrationController` and `PhabricatorOAuthDefaultRegistrationController` controllers are giant copy/pastes of one another. This is really bad.
- We can't practically implement OpenID because we can't reissue the authentication request.
Additionally, the OAuth registration controller can be replaced wholesale by config, which is a huge API surface area and a giant mess.
Broadly, the problem right now is that registration does too much: we hand it some set of indirect credentials (like OAuth tokens) and expect it to take those the entire way to a registered user. Instead, break registration into smaller steps:
- User authenticates with remote service.
- Phabricator pulls information (remote account ID, username, email, real name, profile picture, etc) from the remote service and saves it as `PhabricatorUserCredentials`.
- Phabricator hands the `PhabricatorUserCredentials` to the registration form, which is agnostic about where they originate from: it can process LDAP credentials, OAuth credentials, plain old email credentials, HTTP basic auth credentials, etc.
This doesn't do anything yet -- there is no way to create credentials objects (and no storage patch), but I wanted to get any initial feedback, especially about the event call for T2394. In particular, I think the implementation would look something like this:
$profile = $event->getValue('profile')
$username = $profile->getDefaultUsername();
$is_employee = is_this_a_facebook_employee($username);
if (!$is_employee) {
throw new Exception("You are not employed at Facebook.");
}
$fbid = get_fbid_for_facebook_username($username);
$profile->setDefaultEmail($fbid);
$profile->setCanEditUsername(false);
$profile->setCanEditEmail(false);
$profile->setCanEditRealName(false);
$profile->setShouldVerifyEmail(true);
Seem reasonable?
Test Plan: N/A yet, probably fatals.
Reviewers: vrana, btrahan, codeblock, chad
Reviewed By: btrahan
CC: aran, asherkin, nh, wez
Maniphest Tasks: T1536, T2394
Differential Revision: https://secure.phabricator.com/D4647
Summary: Ref T1536. This is similar to D6172 but much simpler: we don't need to retain external interfaces here and can do a straight migration.
Test Plan: TBA
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6173
Summary: make it work with z-index and make it grey.
Test Plan:
clicked the grey button and it worked! Safari and Chrome
clicked around and observed loading mask functioning correctly
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3396
Differential Revision: https://secure.phabricator.com/D6207
Summary: Ref T1536. Migrates the OAuthInfo table to ExternalAccount, and makes `PhabricatorUserOAuthInfo` a wrapper for an ExternalAccount.
Test Plan: Logged in with OAuth, registered with OAuth, linked/unlinked OAuth accounts, checked OAuth status screen, deleted an account with related OAuth.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6172
Summary:
Ref T1536. Move all access to the underlying storage to inside the class. My plan is:
- Migrate the table to ExternalAccount.
- Nuke the table.
- Make this class read from and write to ExternalAccount instead.
We can't get rid of OAuthInfo completely because Facebook still depends on it for now, via registration hooks.
Test Plan: Logged in and registered with OAuth.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6171
Summary:
Ref T1536. Currently, we store OAuth tokens along with their expiry times and status. However, all we use this for is refreshing profile pictures and showing a silly (and probably somewhat confusing) interface about token status.
I want to move this storage over to `PhabricatorExternalAccount` to make the cutover easier. Drop it for now, including all the profile image stuff (I plan to rebuild that in a more sensible way anyway).
Test Plan: Viewed screen; linked/unlinked accounts.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6170
Summary:
Ref T1536. This is the schema code for `PhabricatorExternalAccount` which was previously in D4647. I'm splitting it out so I can put it earlier in the sequence and because it's simple and standalone.
Expands `PhabricatorExternalAccount` to have everything we need for the rest of registration.
Test Plan: Implemented the remainder of new registration on top of this.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1536
Differential Revision: https://secure.phabricator.com/D6169
Summary: Ref T3377. MySQL ignores indexes if we hand it mismatched datatypes. This seems colossally dumb, but give it what it expects.
Test Plan: wat
Reviewers: wez, btrahan
Reviewed By: wez
CC: aran
Maniphest Tasks: T3377
Differential Revision: https://secure.phabricator.com/D6201
Summary: Rough pass at a PHUIButtonView Class. Keeps phutil_tag intact and adds some image features if you use the class.
Test Plan: UIExamples
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6192
Summary: Restrict the menu hovers to desktop
Test Plan: test desktop and mobile
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6190
Summary:
Ref T3354. There's no way for us to test most of the config options which actually affect this limit, so the Phabricator config is basically a canary value to indicate "the administrator hasn't configured anything yet".
Raise a setup issue if it isn't set. There's a trail to get here from Files, but we've de-emphasized the old-school upload form so it's hard to unearth.
Emphasize the warning that you need to read the documentation and configure like 30 other things to make this work.
Test Plan: Cleared my config, verified I got the issue, read it, set my config, issue went away.
Reviewers: jamesr, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3354
Differential Revision: https://secure.phabricator.com/D6185
Summary: Fixes T3242. Changes the red and orange objects to match the transactions. Also adds a highlight color to 'cards'.
Test Plan: Review my audits in my sandbox
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3242
Differential Revision: https://secure.phabricator.com/D6184
Summary: Decided to just remove the hover grey to white, seems fine with the new white icons.
Test Plan: use homepage + icons
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6181
Summary: the people widget was returning a comma-delimited list of HTML nodes so kill that noise with some hsprintf action. We also weren't consistently updating the latest transaction id so simplify those codepaths (widgets vs pontificate) a bit. Fixes T3336.
Test Plan: left some messages, added some participants. noted that the people widget looked good and only the pertinent transactions were pulled down on updates.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3336
Differential Revision: https://secure.phabricator.com/D6180
Summary: The shadow on the white icons was too harsh for their size, looked bad on timelines.
Test Plan: Check timeline example, phuilist example.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6176
Summary: Took a stab at some login icons for buttons.
Test Plan: photoshop
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6174
Summary: Fixes T3330
Test Plan: Test desktop and mobile menus in chrome and ios.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3330
Differential Revision: https://secure.phabricator.com/D6157
Summary:
Ref T1703. This sets the stage for (but does not yet implement) custom UI types for config. In particular, a draggable list for custom fields.
I might make all the builtin types go through this at some point too, but don't really want to bother for the moment. It would be very slightly cleaner but woudn't get us much of anything.
Test Plan:
UI now renders via custom code, although that code does nothing (produces an unadorned text field):
{F45693}
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1703
Differential Revision: https://secure.phabricator.com/D6154
Summary:
Ref T1703.
- Adds "Title".
- Adds "Blurb".
- Adds `user.fields` config for selecting and reordering. This will get UI in the next patch.
Test Plan:
{F45689}
{F45690}
Edited the fields, too.
Reviewers: chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T1703
Differential Revision: https://secure.phabricator.com/D6153
Summary:
Adds a profile edit controller (with just one field and on links to it) that uses ApplicationTransactions and CustomField.
{F45617}
My plan is to move the other profile fields to this interface and get rid of Settings -> Profile. Basically, these will be "settings":
- Sex
- Language
- Timezone
These will be "profile":
- Real Name
- Title
- Blurb
- Profile Image (but I'm going to put this on a separate UI)
- Other custom fields
Test Plan: Edited my realname using the new interface.
Reviewers: chad, seporaitis
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6152
Summary: Used more logical icons for subscribe, auto, and delete instead of the mail icons. Fixes T3329
Test Plan: Tested subscribing and unsubscribing in Maniphest.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3329
Differential Revision: https://secure.phabricator.com/D6151
Summary:
None of this code is reachable yet. See discussion in D6147. Ref T1703.
Provide tighter integration between ApplicationTransactions and CustomField. Basically, I'm just trying to get all the shared stuff into the base implementation.
Test Plan: Code not reachable.
Reviewers: chad, seporaitis
Reviewed By: chad
CC: aran
Maniphest Tasks: T1703
Differential Revision: https://secure.phabricator.com/D6149
Summary:
Ref T1703. We have currently have two custom field implementations (Maniphest, Differential) and are about to add a third (User, see D6122). I'd like to generalize custom fields before doing a third implementation, so we don't back ourselves into the ApplicationTransactions corner we have with Maniphest/Differential/Audit.
For the most part, the existing custom fields work well and can be directly generalized. There are three specific things I want to improve, though:
- Integration with ApplicationSearch: Custom fields aren't indexable. ApplicationSearch is now online and seems stable and good. D5278 provides a template for a backend which can integrate with ApplicationSearch, and ApplicationSearch solves many of the other UI problems implied by exposing custom fields into search (principally, giant pages full of query fields). Generally, I want to provide stronger builtin integration between custom fields and ApplicationSearch.
- Integration with ApplicationTransactions: Likewise, custom fields should support more native integrations with ApplicationTransactions, which are also online and seem stable and well designed.
- Selection and sorting: Selecting and sorting custom fields is a huge mess right now. I want to move this into config now that we have the UI to support it, and move away from requiring users to subclass a ton of stuff just to add a field.
For ApplicationSearch, I've adopted and generalized D5278.
For ApplicationTransactions, I haven't made any specific affordances yet.
For selection and sorting, I've partially implemented config-based selection and sorting. It will work like this:
- We add a new configuration value, like `differential.fields`. In the UI, this is a draggable list of supported fields. Fields can be reordered, and most fields can be disabled.
- We load every avialable field to populate this list. New fields will appear at the bottom.
- There are two downsides to this approach:
- If we add fields in the upstream at a later date, they will appear at the end of the list if an install has customized list order or disabled fields, even if we insert them elsewhere in the upstream.
- If we reorder fields in the upstream, the reordering will not be reflected in install which have customized the order/availability.
- I think these are both acceptable costs. We only incur them if an admin edits this config, which implies they'll know how to fix it if they want to.
- We can fix both of these problems with a straightforward configuration migration if we want to bother.
- There are numerous upsides to this approach:
- We can delete a bunch of code and replace it with simple configuration.
- In general, we don't need the "selector" classes anymore.
- Users can enable available-but-disabled fields with one click.
- Users can add fields by putting their implementations in `src/extensions/` with zero subclassing or libphutil stuff.
- Generally, it's super easy for users to understand.
This doesn't actually do anything yet and will probably see some adjustments before anything starts running it.
Test Plan: Static checks only, this code isn't reachable yet.
Reviewers: chad, seporaitis
Reviewed By: chad
CC: aran
Maniphest Tasks: T1703
Differential Revision: https://secure.phabricator.com/D6147
Summary: Adds collapsing of the sidebar, also allows you to say where it goes on mobile (above or below content). ToC for example, above. General Navbar, below. Up to you.
Test Plan: Review UIExamples and Diviner.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6150
Summary: Fixes some issues with lists and tablet/mobile layouts.
Test Plan: shrink my screen
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6148
Summary: We need to make sure the symbol is always saved before the atom. I mucked this up recently and didn't catch it locally since I'd already generated the atoms.
Test Plan: Ran `bin/diviner generate` after truncating all diviner tables.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6146
Summary: Gets TOC populated for articles, at least, and fixes a few other things.
Test Plan: {F45474}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6144
Summary: Tweaks the dark, grey, and white icons to match the action-icons. Also added a home icon for navigation.
Test Plan: looked at list navs, action menus.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6142
Summary: Allow users to set a default by dragging it to the top. When they land on a page without a saved query, choose their default.
Test Plan: Hit `/paste/`, got my default results, etc.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6140
Summary:
- Use the same styles for shared operations (`drag-ghost`, `drag-dragging`).
- Move shared code into the base class.
Test Plan: Dragged around tasks and named queries.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6141
Summary: Needed to be more restrictive
Test Plan: Test maniphest and list examples.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6139
Summary:
Also you have to drop them. So drag, and then drop.
This needs some cleanup and reconciliation/generalization with the Maniphest implementation. In particular, you can't drag things to the very top right now, and they should share more CSS and more behaviors.
Test Plan:
Look I alphabetized them:
{F45286}
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6136
Summary: See discussion in D6131, D6130. This turned into 35 layers of mess so throw it away and just tweak the JS to be more flexible.
Test Plan:
- Clicked "Show More Applications".
- Clicked "Show Fewer Applications".
- Edited tasks using popup dialog.
- Tried to drag tasks using pencil icon (correctly no longer works).
- Changed threads in Conpherence.
- Not sure how to actually hit the Conpherence "Load ... Threads" thing since
it seems to auto-load? But that works, at least, and the code doesn't really
care what you hit.
- Added a conpherence participant.
- Added a new calendar item.
- Poked around other menus.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6133
Summary:
See discussion in D6130. Basically, all of these should activate workflow:
<a data-sigil="workflow" href="...">...</a>
<div data-sigil="workflow">
<a href="...">...</a>
</div>
<form data-sigil="workflow" action="...">...</form>
<div data-sigil="workflow">
<form action="...">...</form>
</div>
The only case where we don't want to activate workflow is this one:
<form data-sigil="workflow">
<a href="...">...</a>
</form>
Here, the form should workflow but the `<a />` should not.
These cases aren't really covered:
// Undefined no matter where "workflow" is because it's nonsense.
<a><a>...</a></a>
// As above except like a million times more dumb.
<form><form>...</form></form>
// This one is ambiguous. The <a /> will currently workflow. We don't do
// this anywhere and probably never will. If we want a different rule we
// can cross that bridge when we come to it.
<div data-sigil="workflow">
<form action="...">
<a href="...">...</a>
</form>
</div>
Test Plan: Clicked/submitted some things with workflow.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6131
Summary: This adds examples and abstracts out CSS for common nav re-use.
Test Plan: Tested DocumentExample and ListExample
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6138
Summary: I made changes here recently to improve robustness in the presence of missing files, but accidentally caused the results to re-key. Some callers depend on the mapping, and every other query is consistent about it. Restore the original behavior.
Test Plan: `Pnnn` works again in remarkup.
Reviewers: dctrwatson, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6134
Summary:
Ref T3320. Request from Dropbox. You can currently prefill "title" and some other stuff (including most of the fields by using templates) but there's no way to prefill description and assigned to right now.
- Allow "Description" to be prefilled with a string ("description=...").
- Allow "Assigned To" to be prefilled with a user rather than a PHID ("assign=username").
Test Plan:
Hit `/maniphest/task/create/?assign=epriestley&description=derpderp&title=blarpblarp` locally and got prefills.
{F45259}
Reviewers: chad, deuresti
Reviewed By: chad
CC: aran, euresti
Maniphest Tasks: T3320
Differential Revision: https://secure.phabricator.com/D6132
Summary:
This diff covers a bit of ground.
- PHUIDocumentExample has been added
- PHUIDocument has been extended with new features
- PhabricatorMenuView is now PHUIListView
- PhabricatorMenuItemView is now PHUIItemListView
Overall - I think I've gotten all the edges covered here. There is some derpi-ness that we can talk about, comments in the code. Responsive design is missing from the new features on PHUIDocument, will follow up later.
Test Plan: Tested mobile and desktop menus, old phriction layout, new document views, new lists, and object lists.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6130
Summary: At the global level, truncate emails at a user-configured size.
Test Plan: Untested, as I could not get PHP to send emails on my box, but if you can this should be very easy to test. Just set the max size to something like .001 kilobytes and make sure it does the right thing.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T1392
Differential Revision: https://secure.phabricator.com/D6118
Summary:
Applications come with builtin queries, but users might want to get rid of them. Allow users to disable named queries if they prefer.
This has one funky behavior, which is that the first time you disable a named query it goes to the top of your list. That will be fixed in the next diff, which will make them reorderable.
Test Plan: Added/edited/removed named queries, disabled/enabled builtin named queries.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6128
Summary:
I want to use draggable lists in at least three other interfaces:
- (Today) Reorganizing named search queries.
- (Today) Reorganizing custom fields.
- (Future) Dragging tasks around on boards.
This mostly generalizes the drag-and-drop code in Maniphest's task list. It isn't a total generalization and will need some more tweaking (for example, Maniphest's list is unusual in that the user can't drag items to the top of the list), but it substantially separates the Maniphest-specific behaviors from the general dragging behaviors.
This diff causes no functional changes.
Test Plan: Dragged and dropped tasks in Maniphest.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6124
Summary: Ref T988. Mostly backend changes, with a very rough frontend on top of them. See Conpherence discussion.
Test Plan: {F45010}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6113
Summary: Adds 'Add Project...' if no projects present on Maniphest items. Also - I can't seem to get a dialog to pop, what am I missing? Fixes T3308
Test Plan: Click add project, get edit form.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3308
Differential Revision: https://secure.phabricator.com/D6121
Summary: D6114 fixed some bugs but on production it shows up as a new bug where Saturday is the first day? stop messing with the DateTime object so much and do some old school epoch manipulation. This works correctly on my laptop and my still fail in production, but it will rule out DateTime suckage.
Test Plan: still works on laptop
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6115
Summary: See Conpherence discussion. This probably works?
Test Plan:
Didn't test at all!
bwahaha
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6120
Summary: Pholio stories now reference the mock they're talking about.
Test Plan: Generate each type of story and check that they make sense in feed/inline.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2988
Differential Revision: https://secure.phabricator.com/D6117
Summary: See D6115.
Test Plan:
- Ran unit tests.
- Viewed reports.
- Created a countdown.
- Searched chatlog.
- Searched pastes by created date.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6116
Summary: I needed to port my extremely clever "last sunday as of tomorrow" trick to the display layer. Also found a fun bug in testing where +N days was changing it to 1:00 AM from 00:00 AM with my timezone configuration. Presumably all sorts of whacky hyjinx ensue when you modify DateTime and you need to re-specify the timezone after to get it to work
Test Plan: verified that Today, SUNDAY, we see TODAY -> Saturday and it all looks good. Verified midnite -> just before midnight status events span but a single day.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6114
Summary: Provide all the setSigil() / setID() sorts of calls. No functional changes.
Test Plan: Viewed object item lists.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6112
Summary: Missed this in the PHUIDocumentView thing.
Test Plan: Previewed a Phriction edit; grepped more/harder.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6110
Summary: Ref T3155. "last sunday" is "last sunday at 00:00" so you have to include a day for Sunday itself.
Test Plan: calendar renders correctly today - saturday - which is the edge case of this
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T3155
Differential Revision: https://secure.phabricator.com/D6108
Summary:
Provide a Content-Length header so that browsers can estimate time
remaining for file downloads.
Test Plan: Tested on our local phabricator install.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6107
Summary: Ref T988. Lots of rough edges still, but this pulls the right data and dumps it into a reasonable-looking shell.
Test Plan: {F44883}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D6104
Summary:
Ref T988. Fixes T3150. I want to use this element in Diviner, so separate it from Phriction.
This makes no changes to the actual display except for fixing {T3150} by adding `overflow: hidden;`.
Test Plan: Viewed Phriction documents in mobile and desktop views.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T988, T3150
Differential Revision: https://secure.phabricator.com/D6101
Summary: this does a few things. Fixes T3253. Including the Sunday -> Saturday list view part. Cleans up the display when there are no events, getting rid of this spacer thing. Also fixes Calendar CSS for device-tablet where we had a 2px gap on the calendar from the header.
Test Plan: played with calendar widget a bunch
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T3253
Differential Revision: https://secure.phabricator.com/D6102
Summary: Last of the methods. Fixes T3166.
Test Plan: updated a thread in all the various ways except remove and it worked. removed myself and it worked! tried to remove someone else and it yelled at me.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3166
Differential Revision: https://secure.phabricator.com/D6103
Summary: Ref T2625. This is now "query" instead of "filter" with the new search stuff.
Test Plan: Uploaded a file. Grepped for 'file/filter'.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6105
Summary: This mainly affects crumbs, makes it more clear that the Conpherence doesn't have a set title.
Test Plan: Clear a title, see [No Title]
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6099
Summary: Ref T3166. Returned all the data I could think of, though notable "metadata" isn't used in conpherence (yet afaik) AND its somewhat silly to return the conpherence id / phid you specified, but seems handy.
Test Plan: played with conduit console.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3166
Differential Revision: https://secure.phabricator.com/D6098
Summary: Ref T988. Ref T2625. Rough cut of ApplicationSearch in Diviner, for detailed Atom queries. This isn't useful yet, and isn't linked in the UI.
Test Plan: {F44836}
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T988, T2625
Differential Revision: https://secure.phabricator.com/D6094
Summary:
Ref T2625. Fixes T2812. Implement ApplicationSearch in People.
{F44788}
Test Plan: Made People queries. Used Conduit. Used `@mentions`.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2625, T2812
Differential Revision: https://secure.phabricator.com/D6092
Summary: Ref T2625. Ref T1163. A couple of small generalization nudges, but this is almost entirely straightforward.
Test Plan: Executed various File queries.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1163, T2625
Differential Revision: https://secure.phabricator.com/D6091
Summary:
Ref T2625.
- Build the mobile menu from the delegating controller.
- Make the result header look a little better (still a bit funky).
Test Plan:
{F44774}
{F44775}
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6090
Summary: nice title. also adds a description to the create thread method which I forgot to add... Ref T3166.
Test Plan: queried threads by ids, by phids, and by offset / limit tweakage. Got the right stuff!
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3166
Differential Revision: https://secure.phabricator.com/D6096
Summary: Cleaning up the spacing on ObjectItemView to be on a 4px grid and a little more consistent.
Test Plan: Review changes on a grid in Photoshop
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6088
Summary: We were clipping this to 300px, which is arbitrary to iPhone.
Test Plan: test on Nexus, iPhone
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6089
Summary:
Fixes T1945. Ref T2947. At various times, installs (Disqus, Dropbox, etc.) have asked for a way to edit tasks more quickly. Provide edit-from-lists.
{F44700}
{F44701}
The one rough edge on this is that if you change the task priority we update it inline but don't move it. It's probably infeasible to actually move it, but maybe we could give it some sort of visual style to indicate that it's dirty.
Test Plan: Edited tasks normally and via this action thing.
Reviewers: chad, btrahan
Reviewed By: chad
CC: tido, deuresti, ahoffer, aran
Maniphest Tasks: T1945, T2947
Differential Revision: https://secure.phabricator.com/D6086
Summary:
Ref T2625. Ref T3273. This is mostly a UI foil for T3273. Right now, to find tasks without owners or without projects you search for the magic strings "upforgrabs" and "noproject". Unsurprisingly, no users have ever figured this out. I want to get rid of it. Instead, these interfaces will look like:
Assigned: [ Type a user name... ]
[ X ] Find unassigned tasks.
Projects: [ Type a project name... ]
[ X ] Find tasks with no projects.
Seems reasonable, I think?
Test Plan: Searched for "rainbow, js", "rainbow + no language", "no language", date ranges, etc.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T2625, T3273
Differential Revision: https://secure.phabricator.com/D6085
Summary:
This was mentioned in T2928 and nobody objected.
It just references the task instead of fixing it as that would be too aggressive.
It also doesn't check assignee of the task (by purpose).
Test Plan: Created diff from a branch named T2928.
Reviewers: epriestley, edward
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2928
Differential Revision: https://secure.phabricator.com/D5640
Summary:
Ref T1163. Ref T2625. This could probably use some tweaks, but I kept things mostly-generic.
I added a new control for freeform dates so we can have it render help or whatever later on.
Test Plan: See screenshots.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625, T1163
Differential Revision: https://secure.phabricator.com/D6084
Summary:
This prevents security by obscurity.
If I have read-only access to the database then I can pretend to be any logged-in user.
I've used `PhabricatorHash::digest()` (even though we don't need salt as the hashed string is random) to be compatible with user log.
Test Plan:
Applied patch.
Verified I'm still logged in.
Logged out.
Logged in.
$ arc tasks
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6080
Summary: Ref T3166. I moved the create logic into a static method in the editor class to keep things tidy.
Test Plan: created a conpherence from UI. purdy. tried errors and got UI to show "required". for conduit, created a thread with all the bells and whistles and it worked. verified i got proper exceptions with bum conduit calls
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3166
Differential Revision: https://secure.phabricator.com/D6083
Summary:
Fixes T3279. For ApplicationSearch (and in some other cases) I'd like users to be able to provide an optional date. This isn't currently possible.
Add a checkbox which disables or enables the input.
Test Plan: Used UIExample to enter dates. Used Calendar to enter dates.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Maniphest Tasks: T3279
Differential Revision: https://secure.phabricator.com/D6082
Summary: nice title. Fixes T3203. If its been N days and now its Tuesday, it just shows a single marker for Tuesday.
Test Plan: Viewed a conpherence and there were date dividers!
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3203
Differential Revision: https://secure.phabricator.com/D6081
Summary: Ref T2625. Works out the last kinks of generalization and gives Macros the more powerful new query engine. Overall, this feels pretty good to me.
Test Plan: Executed, saved and edited a bunch of Macro queries.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6078
Summary:
Ref T2625. Lifts almost all of the search logic out of Paste controllers and into Search.
This uses controller delegation for generalization. We use this in a few places, but don't use it very much yet. I think it's pretty reasonable as-is, but I might be able to make even more stuff free.
There are some slightly rough edges around routes, still, but I want to hit Phame and Differential (which both have multiple application search engines) before trying to generalize that.
Test Plan: Executed, browsed and managed Paste searches.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6073
Summary: Fixes T3280 - when a pontificate brought back multiple transactions, we were rendering a comma. Yay hsprintf. Also fixes the noconpherences view, which broke at some point recently.
Test Plan: sent comment, then replied from different browser. when both comments loaded noted no comma. loaded a conpherence view with no conpherences and verified it looked good.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, chad
Maniphest Tasks: T3280
Differential Revision: https://secure.phabricator.com/D6079
Summary:
Fixes T3252. Other enhancements:
- Header in widget panel was 2px too short.
- Typeahead in add people only allowed one person
- Typeahead in add people was cutoff by overflow:hidden
- X in remove has been changed to unicode (multiply)
- Add people dialog form fields are full width
- Some other CSS tweaks.
Test Plan: Add, Remove people.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3252
Differential Revision: https://secure.phabricator.com/D6076
Summary: and now you can add more than one at a time! Also adds the 'add participants' and 'new calendar event' options to mobile view. Fixes T3251. Ref T3253.
Test Plan: loaded up these "adders" on both desktop and device-ish views and it went well!
Reviewers: epriestley, chad
Reviewed By: chad
CC: chad, aran, Korvin
Maniphest Tasks: T3251, T3253
Differential Revision: https://secure.phabricator.com/D6075
Summary:
Currently, the author of an image macro is read from the attached file. This is messy and necessitates a join, and is not always correct. Instead, store the data when the macro is created.
This lays the groundwork for generalizing ApplicationSearch here. Ref T2625.
Test Plan: Migrated existing macros, created a new macro, checked web UI.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6071
Summary: Fixes T3253 by shifting the display to the "next 3 days". Also adds in the "create" functionality for calendar on desktop view only, ref T3251. As part of T3251, I plan to make this work on mobile too.
Test Plan: added statuses and noted errors showed up. noted on success the widget pane refreshed. also made sure the regular old /calendar/status/create/ page still worked.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3251, T3253
Differential Revision: https://secure.phabricator.com/D6072
Summary:
Ref T2625. @chad, you might have some feedback here. The behaviors this implements are:
- When the user selects "Advanced Search", we show the full search UI and no results (for performance and clarity).
- When the user submits a search which //is not// a named search, we show the full search UI and the "Save Custom Query..." button.
- When the user submits a search which //is// a named search, we show "Results for search X." with an "Edit Query..." button. The button expands the search form.
- When the user selects a builtin query (like "All Pastes"), we don't show any search UI, but I'm probably going to make this behave more like named searches.
Test Plan:
{F44346}
{F44347}
Reviewers: chad, btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6063
Summary: this diff tries to polish the poo out of the JS layer while achieving fixes T3157 accolades.
Test Plan: introduced sleeps in the various controllers and clicked about. verified good "loading" UI in the menu / message / widget section as appropros. Loaded up in device size and resize and desktop sized and resized and all was good.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3164, T3157
Differential Revision: https://secure.phabricator.com/D6069
Summary:
We can lose file data through various means; one reasonable way is if files get deleted from disk with 'local-disk' storage. If data goes missing,
Ref T3265. Also, reduce some code duplication.
Test Plan:
Ran `bin/files purge`, `bin/files migrate`, `bin/files rebuild` with various args.
Deleted a file with "local-disk" storage, ran `bin/files purge`, made sure it got picked up.
Reviewers: dctrwatson, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3265
Differential Revision: https://secure.phabricator.com/D6068
Summary:
We allow files to be deleted from the web UI, including paste files. When a paste's file is deleted, we currently continue to show it in the paste list and then 400 when it's clicked on.
Instead, remove it from the list. (Note that it may stick around if it's cached.)
Test Plan: Purged general cache, deleted a paste's file, viewed paste list, saw no pastes.
Reviewers: dctrwatson, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3265
Differential Revision: https://secure.phabricator.com/D6067
Summary: Highlights which day is today on the calendar list in conpherence. Fixes T3254
Test Plan: Made sure today was Tuesday.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3254
Differential Revision: https://secure.phabricator.com/D6065
Summary:
In Releeph, `ReleephRequestHeaderView` will render you a "mark-manually-picked" button if (a) you are authoritative (chuckr) or (b) you are the person who created this Releeph request.
However `ReleephRequestActionController`, which handles these button presses, will only do something if you are (a). This patch honors your button pressing if you are (b) as well.
Test Plan: Push buttons. There's another bug in the Javascript do-stuff-without-reloading-the-page code that handles these button pushes, but I'd like to fix that separately from handling this hi-pri bug.
Reviewers: epriestley, wez
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6036
Summary:
Ref T2625. The specialized buildSearchForm() method has significant amounts of generic form construction responsibility right now. Lift the generic stuff above the Engine level. Also:
- Rename "users" to "authors".
- Use "users", not "searchowners" (which incorrectly includes "upforgrabs").
- No need for "set_" prefixes anymore since we do GET redirects with query keys.
- Use newer style for search stuff.
Test Plan:
Searched for stuff?
{F44342}
Reviewers: btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6061
Summary: Ref T2625. We currently hard-code the URI; instead, derive it from the Engine. I weakened the strength of getQueryResultsPageURI to let it build from a NamedQuery or a SavedQuery, because constructing a SavedQuery for a builtin NamedQuery is a bit of a pain.
Test Plan: Clicked links on the saved queries page, got query results.
Reviewers: btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6060
Summary:
Ref T2625. Currently, custom saved queries can be edited but not deleted. Allow them to be deleted. Also:
- Clean up an unused property in `PhabricatorPasteViewController`.
- Fix an issue with left nav highlighting of builtin queries.
- Improve submit behavior for edits.
- Add a cancel button on edits.
Test Plan: Saved, edited and deleted queries.
Reviewers: btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6059
Summary:
Ref T2625. Currently, Paste hard-codes its filters as a separate layer above the query layer. Instead, expose these as "Builtin" queries which we construct at runtime. They act like normal saved queries, except in cases where it doesn't make sense.
(I'm probably going to let you hide them too, and maybe even rename them, although for now they're just immutable.)
Test Plan: {F44340}
Reviewers: btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6058
Summary: Ref T2625. Rename the "Name" controller to "Edit" and allow it to edit named queries. Also some UI touchups. Add an edit link to the saved query browse view.
Test Plan: Created and edited named queries.
Reviewers: btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6056
Summary:
Ref T2625. Currently, after saving a query the user is redirected to "/search/", which isn't especially useful. Instead, redirect them back into the application they came from and to the query results page.
Also, query hashes may contain ".", which does not match `\w`. Use `[^/]` instead.
Test Plan: Saved some custom queries.
Reviewers: btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6055
Summary: Same as D6051, but for SavedQueries instead of NamedQueries. These are POLICY_PUBLIC because you need to know the hash to access them, and because we want to let users copy/paste query URLs. Ref T2625.
Test Plan: Saved a query, reused a saved query.
Reviewers: btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6054
Summary:
Ref T2625.
- Show saved queries in the left nav.
- Highlight the correct stuff in the left nav.
Test Plan: Clicked all left-nav stuff.
Reviewers: btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6053
Summary: Adds a first-class Query object for querying NamedQueries. xzibit would be proud.
Test Plan: Updated query edit interface to use this query, verified it works correctly.
Reviewers: btrahan, blc
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D6051
Summary:
These are a bit tricky because we don't want to require you to install a VCS you don't use just to use Phabricator. Test that repositories exist before performing the checks.
I'll couple this with additional checks during repository creation.
Test Plan: Changed binary names to nonexistent ones, verified setup issues raised properly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6040
Summary:
A few more of these issues have cropped up recently. Basically:
- Webservers often (by default, I guess?) have a different or nonexistent $PATH.
- Users have a hard time figuring this out, since it's not obvious that the webserver might have a different configuration than the CLI, and they can run "git" and such themselves fine, and they don't normally use SetEnv or similar in webserver config.
I've been pursuing one prong of attack here (better detection and more tailored errors); this is a second prong (try to just guess the configuration correctly).
In 99% of cases, the binaries in question are in one of these three places, so just make them the default appended paths. If users have wacky configs they can override the setting.
Test Plan: Viewed config locally.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6039
Summary:
Ref T3201. D6009 added this flag, but I don't think it actually works as advertised? We either need a Conduit patch to interpret `null` as `upforgrabs` (which seems 100% reasonable to me) or this (which is kind of nasty).
@garoevans, was there a Phabricator-side diff for the conduit part that just got dropped? I'll probably replace this with that if not, but figured I'd check before I poke anything.
Test Plan: Ran `arc diff --unassigned`
Reviewers: garoevans
Reviewed By: garoevans
CC: aran
Maniphest Tasks: T3201
Differential Revision: https://secure.phabricator.com/D6062
Summary: Search typeahead now returns project objects with project images :). I have set the displaytype to "Project" if that's okay. Alternatively We can also set it to some short description of a given project. All this time, I was thinking Jump Nav to be the search typeahead ... I am so dumb ... Lolz :P
Test Plan: {F44336}
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2948
Differential Revision: https://secure.phabricator.com/D6057
Summary: Semi-decent pass at cleaning up the Conpherence dropdown and widgets. Will continue to update but have diff questions.
Test Plan: Testing Conpherence in my sb.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6043
Summary:
Fixes T3139. See that task for discussion.
If all mentions are removed because they're already subscribed, we currently generate an empty transaction, which later gets picked up as having no effect and the user gets yelled at.
Instead, don't generate a transaction if no PHIDs remain after filtering already-subscribed PHIDs.
Test Plan: Followed plan in T3139.
Reviewers: garoevans, btrahan
Reviewed By: garoevans
CC: aran
Maniphest Tasks: T3139
Differential Revision: https://secure.phabricator.com/D6048
Summary: Add a button to the Remarkup area that explains how to attach an image and remove the separate upload field.
Test Plan: Check that the dialog pops up correctly and that dropping images onto the Remarkup area works.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T879
Differential Revision: https://secure.phabricator.com/D6049
Summary:
Kind of a quick look at an idea for T2184
Ref T2184
Test Plan: Make sure the site still loads
Reviewers: epriestley
CC: aran, Korvin, mbishopim3
Maniphest Tasks: T2184
Differential Revision: https://secure.phabricator.com/D6045
Summary: Ref T3184. See discussion in D6042.
Test Plan: {F44265}
Reviewers: garoevans, btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3184
Differential Revision: https://secure.phabricator.com/D6047
Summary:
Fixes T3184
We make sure that we're not working with a global policy that just sets the title correctly. Otherwise we have a PHID to work with and set the handle as required.
Test Plan: Change policies on a mock and see the title render correctly.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3184
Differential Revision: https://secure.phabricator.com/D6042
Summary:
Kind of just guessed what to do here. It seemed reasonable.
Take the transaction, use the author to get an actor image. Get the main object handle type, use that for the app icon. Use the transaction's created date as a time.
Ref T2988
@chad I think there's still no icon for mock?
Test Plan: Look at my feed. See some info.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: chad, aran, epriestley, Korvin
Maniphest Tasks: T2988
Differential Revision: https://secure.phabricator.com/D6044
Summary: We were missing this option on the response, so notifications about commits (e.g., tokens given) don't clear when viewing the commit.
Test Plan: Gave a commit a token with user A, viewed notification with user B, viewed commit with user B and verified notification cleared.
Reviewers: garoevans, btrahan
Reviewed By: garoevans
CC: aran
Differential Revision: https://secure.phabricator.com/D6041
Summary: Adds support for the "encoding" field to the new transactional interface.
Test Plan:
{F44189}
{F44190}
Some of the encodings in the second screen are from testing, and can no longer be set.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6035
Summary:
At one point this was sort of a one-line summary but it isn't really anymore (and doesn't appear on the list view). We could add a summary in the future if we wanted.
- Change the control from a text area to a remarkup area.
- Change the display to remarkup.
Test Plan:
{F44183}
{F44184}
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6034
Summary: I want to use some of these for instructional text in Diffusion. Provide a concrete class to make one-offs cacheable and switch Releeph to use it.
Test Plan: {F44175}
Reviewers: btrahan, chad, edward
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6032
Summary:
Ref T2231, T603. Plan of attack here is pretty much:
- Built out a new (currently not linked in the UI) edit interface in Diffusion which is transaction-based and has a sensible layout.
- Build out a new create interface based on PagedForm which dumps into the new edit interface.
- Throw the old stuff away.
- Everyone lives happily ever after.
Test Plan:
{F44163}
{F44164}
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2231
Differential Revision: https://secure.phabricator.com/D6029
