Summary: Fixes T3481. Sort of - this thing be very ugly. Also it assumes that you'll "always" want to sign terms. I was thinking in a future diff that should be optional as well as configurable, though it was unclear to me if either was worth pursuing... Generally very hideous as the three main elements (PHUIDocument, AphrontErrorView, and AphrontForm with an AphrontFormInset) have never really played together before.
Test Plan: agreed to some test terms. noted UI displayed nicely. reloaded and noted UI told me I had signed it already. Went to different terms and filled them out wrong and got sensical errors.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3481
Differential Revision: https://secure.phabricator.com/D6399
Summary: this let's the list controller save a query. Fixes T3488. Note didn't bother denormalizing document body at all since I don't think we want to show a snippet.
Test Plan: viewed a list of legalpad documents - yay. viewed a legalpad document - yay. created a legalpad document - yay. edited a legalpad document - yay. edited with N authors - yay.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3488
Differential Revision: https://secure.phabricator.com/D6382
Summary:
Supports !unsubscribe and commenting on replies. Subscribers get mailed something reasonable. Fixes T3480.
Sneaks in /LX/ support. In the near future I want to have that /LX/ be a clean "signature" page sans all the edit actions and other fluff... Will resolve this as part of T3481.
Test Plan: used the metamta console to add comments and unsubscribe. added a phlog() inside mail code to verify mail bodies looked okay.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3480
Differential Revision: https://secure.phabricator.com/D6369
Summary:
- Add GC support to conduit logs.
- Add Query support to conduit logs.
- Record the actual user PHID.
- Show client name.
- Support querying by specific method, so I can link to this from a setup issue.
@wez, this migration may not be fast. It took about 8 seconds for me to migrate 800,000 rows in the `conduit_methodcalllog` table. This adds a GC which should keep the table at a more manageable size in the future.
You can safely delete all data older than 30 days from this table, although you should do it by `id` instead of `dateCreated` since there's no key on `dateCreated` until this patch.
Test Plan:
- Ran GC.
- Looked at log UI.
- Ran Conduit methods.
Reviewers: btrahan
Reviewed By: btrahan
CC: wez, aran
Differential Revision: https://secure.phabricator.com/D6332
Summary:
Ref T3116. This is a large amount of schema for V0 but it seems relatively complete to the desired features in T3116.
The only thing of note that is missing is documentSignatures should have some sort of "signedStatus". "Un-signing" seemed weird to me, though I could imagination "pending signature". "Pending signature" could be done via edges pretty easily.
Plan is to have "Document" be at the top level and own policy. "DocumentBody" will store a version of title and text for each and every "edit" on a larger Document. "Edges" are to be used to tie Authors => Document for V0ish. Transactions are going to be used to store all the various edits possible here. Oh and DocumentSignatures will do what you expect, but include documentVersion as part of the key.
Test Plan: just some schema. `storage update` worked though!
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3116
Differential Revision: https://secure.phabricator.com/D6323
Summary:
Ref T2852. This table holds data about external objects and allows us to write edges to them.
Objects are identified with an `<applicationType, applicationDomain, objectType, objectID>` tuple. For example, Asana tasks will be, e.g., `<asana, asana.com, asana:task, 93829279873>` or similar.
Test Plan: Ran storage upgrade.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2852
Differential Revision: https://secure.phabricator.com/D6271
Summary:
Ref T2222. My path forward here wasn't very good -- I was thinking I could set `transactionPHID` for the inline comments as I migrated, but it must be unique and an individual DifferentialComment may have more than one inline comment. Dropping the unique requirement just creates more issues for us, not fewer.
So the migration in D6266 isn't actually useful. Undo it -- this can't be a straight revert because some installs may already have upgraded.
Test Plan: Ran new migrations, verified the world ended up back in the same place as before (made comments, viewed reivsions).
Reviewers: btrahan
Reviewed By: btrahan
CC: wez, aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D6269
Summary:
Ref T2222. This adds PHIDs to all Differential comments so I can migrate the inlinecommment table to transaction_comment in the next diff.
@wez, this will issue a few million queries for Facebook (roughly, one for each Differential comment ever made). It's safe to skip the `.php` half of the patch, bring Phabricator up normally, and then apply this patch with Phabricator running if that eases the migration, although the next few diffs will probably be downtime-required migrations so maybe it's easier to just schedule some downtime.
Test Plan: Ran migration locally. Verified existing comments and new comments received PHIDs.
Reviewers: btrahan
Reviewed By: btrahan
CC: wez, aran
Maniphest Tasks: T2222
Differential Revision: https://secure.phabricator.com/D6266
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 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: Remade auth and policy icon.
Test Plan: look at the images.
Reviewers: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6241
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: 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. 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. 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: 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. 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: 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 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:
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: 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: 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: 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:
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:
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: 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: 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. 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:
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:
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:
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
Summary: If we're unassigning an owner from a task it should set the column to `NULL` rather than an empty string. Fixes T3239
Test Plan: Assigned and Unassigned a task. Make sure the db is doing as excpected. Ran the patch, checked the db.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3239
Differential Revision: https://secure.phabricator.com/D6017
Summary:
removes the whole custom image thing, instead using a more standard application crumbs. Gives this glorious space back to the compose area which is now tens of pixels taller. Also defaults it to the people widget. Basically, fixes T3160.
For now, you **CAN NOT** edit the title of a conpherence. I didn't want to jam in too much here. Next diff will be to change the widget icons into the dropdown switcher, which will also bring back the editing of titles.
Test Plan: looked at conpherence and it was pretty. Resized it vigorously and it wasn't too bad.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3160
Differential Revision: https://secure.phabricator.com/D5998
Summary:
Ref T988. This adds basics for the non-static publishing target:
- Storage (called "Live", e.g. `DivinerLiveAtom` to distinguish it from shared classes like `DivinerAtom`).
- Mostly populate the storage.
- Some minor fixes and improvements.
Test Plan: Generated docs, looked at DB, saw mostly-sensible output.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T988
Differential Revision: https://secure.phabricator.com/D5973
Summary: Basically a one line change, plus a regen. This should be the last full-rewrite these things get since they'll stop fully rehashing all the time now.
Test Plan: See D5964.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5965
Summary: Debating removing the textures from the sidenavs, sending this around for comments.
Test Plan: Test homepage and various sidenavs, mobile layouts.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5948
Summary:
We/I broke a couple of things here recently (see D5911) and are doing some work here in general (see D5912, etc.).
Generally, this code is pretty oldschool and not especially well architected for modern application-oriented Phabricator. It hardcodes a lot of stuff which should be applications' responsibilites.
Take the first steps toward making it more solid to reduce the risk here. In particular:
- Factor out the "self mail" and "duplicate mail" checks and add unit tests.
- Make Message-ID hash handling automatic.
Test Plan: Ran unit tests.
Reviewers: btrahan, chad
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5915
Summary:
Migrate to `PhabricatorApplicationTransactions` (`ReleephRequestTransactions` applied by `ReleephRequestTransactionalEditor`, instead of `ReleephRequestEvents` created by `ReleephRequestEditor`) and migrate all the old events into transactions. Email is supported in the standard way (no more `ReleephRequestMail`) as well.
This also collapses the Releeph request create and edit controllers into one class, as well as breaking everyone's subject-based mail rules by standardising them (but which should be more easily filtered by looking at headers.)
Test Plan:
* Make requests, then pick them.
* Pick and revert the same request so that discovery happens way after `arc` has told Releeph about what's been happening.
* Try to pick something that fails to pick in a project with pick instructions (and see the instructions are in the email.)
* Load all of FB's Releeph data into my DB and run the `storage upgrade` script.
* Request a commit via the "action" in a Differential revision.
Reviewers: epriestley
Reviewed By: epriestley
CC: epriestley, aran, Korvin, wez
Maniphest Tasks: T3092, T2720
Differential Revision: https://secure.phabricator.com/D5868
Summary: Can name saved queries.
Test Plan: Try naming some saved queries using the form.
Reviewers: epriestley
CC: aran, Korvin, AnhNhan
Maniphest Tasks: T2625
Differential Revision: https://secure.phabricator.com/D5878
Conflicts:
src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
Summary:
We have a few interfaces where add "Edit", "Delete" or some other action to a list. Currently, this happens via icons, but these are cumbersome and weird, are inconsistent, can't be workflow'd, are hard to hit on desktops and virtually impossible to hit on mobile, and generally just feel iffy to me. Prominent examples are Projects and Flags. I'd like to try adding an "edit" action to Maniphest (to provide quick edit from list views, basically). It looks like some of Releeph would benefit here, as well.
Instead, provide first-class actions:
{F42978}
They produce targets which my meaty ham-fists can plausibly hit on mobile, too:
{F42979}
(We could do some kind of swipe-to-expose thing eventually, but I think putting them by default is OK?)
Test Plan: Added UIExamples. Checked desktop/mobile.
Reviewers: chad, btrahan, edward
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5890
Summary:
Fixes T3132. Currently, if a user deletes a file which is present in a mock, that mock throws an exception when loading. If the file is also the cover photo, the mock list throws an exception as well.
In other applications, we can sometimes deal with this (a sub-object vanishing) by implicitly hiding the parent object (for example, we can just vanish feed stories about objects which no longer exist). We can also sometimes deal with it by preventing sub-objects from being directly deleted.
However, neither approach is reasonable in this case.
If we vanish the whole mock, we'll lose all the comments and it will generally be weird. Vanishing a mock is a big deal compared to vanishing a feed story. We'll also need to load more data on the list view to prevent showing a mock on the list view and then realizing we need to vanish it on the detail view (because all of its images have been deleted).
We permit total deletion of files to allow users to recover from accidentally uploading sensitive files (which has happened a few times), and I'm hesitant to remove this capability because I think it serves a real need, so we can't prevent sub-objects from being deleted.
So we're left in a relatively unique situation. To solve this, I've added a "builtin" mechanism, which allows us to expose some resource we ship with as a PhabricatorFile. Then we just swap it out in place of the original file and proceed forward normally, as though nothing happened. The user sees a placeholder image instead of the original, but everything else works reasonably and this seems like a fairly acceptable outcome.
I believe we can use this mechanism to simplify some other code too, like default profile pictures.
Test Plan: Deleted a Pholio mock cover image's file. Implemented change, saw functional Pholio again with beautiful life-affirming "?" art replacing soul-shattering exception.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T3132
Differential Revision: https://secure.phabricator.com/D5870
Summary: Adding mail-keys; required for `PhabricatorApplicationTransaction` support.
Test Plan: Upgrade an old database with this patch, observe the matrix: {F42620}
Reviewers: wez, epriestley
Reviewed By: epriestley
CC: epriestley, aran
Maniphest Tasks: T2720
Differential Revision: https://secure.phabricator.com/D5852
