Summary:
Ref T1702. Ref T3718. There are a couple of things going on here:
**PhabricatorCustomFieldList**: I added `PhabricatorCustomFieldList`, which is just a convenience class for dealing with lists of fields. Often, current field code does something like this inline in a Controller:
foreach ($fields as $field) {
// do some junk
}
Often, that junk has some slightly subtle implications. Move all of it to `$list->doSomeJunk()` methods (like `appendFieldsToForm()`, `loadFieldsFromStorage()`) to reduce code duplication and prevent errors. This additionally moves an existing list-convenience method there, out of `PhabricatorPropertyListView`.
**PhabricatorUserConfiguredCustomFieldStorage**: Adds `PhabricatorUserConfiguredCustomFieldStorage` for storing custom field data (like "ICQ Handle", "Phone Number", "Desk", "Favorite Flower", etc).
**Configuration-Driven Custom Fields**: Previously, I was thinking about doing these with interfaces, but as I thought about it more I started to dislike that approach. Instead, I built proxies into `PhabricatorCustomField`. Basically, this means that fields (like a custom, configuration-driven "Favorite Flower" field) can just use some other Field to actually provide their implementation (like a "standard" field which knows how to render text areas). The previous approach would have involed subclasssing the "standard" field and implementing an interface, but that would mean that every application would have at least two "base" fields and generally just seemed bleh as I worked through it.
The cost of this approach is that we need a bunch of `proxy` junk in the base class, but that's a one-time cost and I think it simplifies all the implementations and makes them a lot less magical (e.g., all of the custom fields now extend the right base field classes).
**Fixed Some Bugs**: Some of this code hadn't really been run yet and had minor bugs.
Test Plan:
{F54240}
{F54241}
{F54242}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1702, T1703, T3718
Differential Revision: https://secure.phabricator.com/D6749
Summary: Ref T3656. Releeph denormalizes branch cut point identifiers into Branch objects, but this information isn't useful or used for sorting, filtering, or enforcing unique constraints. Instead, derive it via noramlized pathways from the `cutPointCommitPHID`.
Test Plan: Ran storage upgrade. Ran `releephwork.getbranch` and `releeph.getbranches`. Grepped for `cutPointCommitIdentifier`.
Reviewers: btrahan
Reviewed By: btrahan
CC: LegNeato, aran
Maniphest Tasks: T3656
Differential Revision: https://secure.phabricator.com/D6636
Summary:
Fixes T3660. Releeph Projects currently have an unused one-to-one mapping to Phabricator projects. This isn't consistent with other applications and has no integrations or uses. Get rid of it.
NOTE: Waiting for signoff from @legneato on T3660 before pulling the trigger here.
Test Plan: Created and edited Releeph projects. Grepped for references to project ID; there are a dozen or so but they're all either Releeph projects or Arcanist projects.
Reviewers: btrahan
Reviewed By: btrahan
CC: LegNeato, aran
Maniphest Tasks: T3660
Differential Revision: https://secure.phabricator.com/D6635
Summary: Ref T3655. Depends on D6633. This removes the writes and the column.
Test Plan: Created a project, edited a project. Verified the table doesn't have any keys including this column.
Reviewers: btrahan
Reviewed By: btrahan
CC: LegNeato, aran
Maniphest Tasks: T3655
Differential Revision: https://secure.phabricator.com/D6634
Summary: Ref T2769. I'm planning to keep this pretty simple, but we have this ad-hoc edit log for rules already and some other mess that we can clean up.
Test Plan: No effect yet; see future changes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2769
Differential Revision: https://secure.phabricator.com/D6654
Summary: Email replies and subscribers seem to go hand in hand so deploy both at once.
Test Plan: played around with bin/mail. Verified replies posted comments on the paste.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3650
Differential Revision: https://secure.phabricator.com/D6682
This column has no default value and throws if you have maximum warnings activated:
EXCEPTION: (AphrontQueryException) #1364: Field 'commentVersion' doesn't have a default value...
Auditors: btrahan
Summary: Ref T3650. This adds a create transaction, transactions for metadata (title, langauge, view policy), and comments. Editor is used on all create /edit paths.
Test Plan: made some pastes via web and email - yay. edited pastes - yay. verified txns showed up on pastes and in feed correctly.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3516, T3650
Differential Revision: https://secure.phabricator.com/D6645
Summary: Ref T3578. I forget if this was an explicit decision or not, but we currently let the same user answer questions multiple times. I think this probably causes more confusion than it provides freedom. In conjunction with other UI issues (commenting being weird, notably), we're seeing some use of answers to comment, which is undesirable. Require each answer's author to be unique. Merge existing nonunique authors' answers.
Test Plan: {F52062}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3578
Differential Revision: https://secure.phabricator.com/D6605
Summary: Let's see how these do, seems ok
Test Plan: Microsoft Paint
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6615
Summary:
Fixes T3579
As a basic overview, this enables the author of a question to open/close a question.
Other bits;
- Add "Open" filter to the builtin queries
- Add "Status" to search form
- Refactor ponder constants
- Add coloured bars for different question statuses
Test Plan:
- Open/Close questions
- Search for some bits
- Use filters
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3579
Differential Revision: https://secure.phabricator.com/D6590
Conflicts:
src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php
Summary: Like other icons, except white (hover states).
Test Plan: photoshop
Reviewers: epriestley, btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6591
Summary:
Now you can actually replace an image! Ref T3572. This ended up needing a wee bit of infrastructure to work...
- add replace image transaction to pholio
- add replacesImagePHID to PholioImage
- tweaks to editor to properly update images with respect to replacement
- add edges to track replacement
- expose getNodes on graph query infrastructure to query the entire graph of who replaced who
- move pholio image to new phid infrastructure
Still TODO - the history view should get chopped out a bit from the current view - no more inline comments / generally less functionality plus maybe a tweak or two to make this more sensical.
Test Plan: replaced images and played with history controller a little. works okay.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3572
Differential Revision: https://secure.phabricator.com/D6560
Summary: Fixes T3557. One thing which made T3557 kind of a mess was the lack of information about progress through temporary failures. Add a column which records a task's last failure time, and surface it in the console.
Test Plan: {F51277}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3557
Differential Revision: https://secure.phabricator.com/D6550
Summary: Ref T2715. Switch Maniphest to the new stuff.
Test Plan: Used `phid.query`; `phid.lookup` to load objects.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2715
Differential Revision: https://secure.phabricator.com/D6516
Summary:
See discussion in T2715. Currently, PHIDs are all hard coded in the PHID application. In the long run, we need to move them out into actual applications.
A specific immediate issue is Releeph, which uses a very very old and very broken mechanism to inject PHIDs in a way that only sort of works.
Moving forward, every PHID type will be provided by a `PhabricatorPHIDType` subclass, which will manage loading it, etc.
This also moves toward cleaning up the "load objects by name" (where "name" means something like `D12`) code, which is an //enormous// mess and spread across at least 4-5 callsites.
Test Plan: Used `phid.lookup` and `phid.query` to load Slowvotes.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6502
Summary: Fixes T3592, hand kerned the logo and dropped it's size 2 px.
Test Plan: pixels in photoshop, dry run in sandbox
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T3592
Differential Revision: https://secure.phabricator.com/D6505
Summary:
Fixes T2675, T2676.
- when the last person leaves a project it is archived.
- a script to archive all memberless projects
- better feed stories for the various policy edits you can do
- phriction pages are also moved as you rename projects
Test Plan: edited some projects and noted reasonable feed stories. ran script against test data and it worked! left a last man standing project and it archived. renamed a project to "a" then "b" then "a" (etc) and it worked including phrictiondocument moves
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2676, T2675
Differential Revision: https://secure.phabricator.com/D6478
Summary: Updates to the gradient logo and hashed background. Minor pixel tweaks.
Test Plan: Test desktop and mobile. Check photoshop for alignment.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D6487
Summary: Status icons for next to people names
Test Plan: photoshop
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2064
Differential Revision: https://secure.phabricator.com/D6479
Summary:
Nice title. We add three new transactions - IMAGE_FILE, IMAGE_NAME, and IMAGE_DESCRIPTION. The first is a bit like subscribers as it is a list of file phids. The latter have values of the form ($file_phid => $data), where $data is $name or $description respectively. This is because we need to collate transactions based on $file_phid...
Overall, this uses the _underyling files_ and not the "PholioImage" to determine if things are unique or not. That said, simply mark PholioImages as obsolete so inline comments about no-longer applicable PholioImages don't break.
Does a reasonable job implementing the mock. Note you can't "update" an image at this time, though you can delete and add at will.
Test Plan: played with pholio a ton.
Reviewers: epriestley
Reviewed By: epriestley
CC: chad, aran, Korvin
Maniphest Tasks: T3489
Differential Revision: https://secure.phabricator.com/D6441
Summary: Subscribing doesn't do anything yet, but you can subscribe!
Test Plan: {F50169}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6456
Summary:
Move comments from the old table to ApplicationTransactions. Patch dances around which objects it uses since I intend to delete the comment table.
NOTE: This temporarily disables comment writes. I'll restore them shortly.
Test Plan: {F50166}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6454
Summary: Schema changes to modernize this app.
Test Plan: Ran schema changes, created a new slowvote. No real effects yet.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6453
Summary: Fixes T3069. Ref T3516. For a long time, LDAP import added trailing whitespace to real names when importing them (for example, `"John Smith"` would be imported as `"John Smith "`). We no longer do this, but should clean up the database since it seems highly unlikely that any user wants trailing whitespace in their real name.
Test Plan:
$ ./bin/storage upgrade -f --apply phabricator:20130711.trimrealnames.php
Applying patch 'phabricator:20130711.trimrealnames.php'...
Trimming trailing whitespace from user real names...
Trimming user 1 from 'Evan Priestley ' to 'Evan Priestley'.
User 4 is already trim.
User 5 is already trim.
User 6 is already trim.
User 7 is already trim.
User 8 is already trim.
User 9 is already trim.
User 10 is already trim.
User 11 is already trim.
User 12 is already trim.
User 13 is already trim.
User 14 is already trim.
User 15 is already trim.
User 21 is already trim.
User 22 is already trim.
User 26 is already trim.
User 28 is already trim.
User 29 is already trim.
User 32 is already trim.
User 33 is already trim.
User 35 is already trim.
User 39 is already trim.
User 51 is already trim.
User 53 is already trim.
User 54 is already trim.
User 55 is already trim.
Done.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3069, T3516
Differential Revision: https://secure.phabricator.com/D6426
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
