Summary: Ref T2217. When you add comments (or use that interface to make updates), ship it through the new code.
Test Plan: Added comments, made other changes to Maniphest tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7088
Summary: Ref T2217. Ship these through the new stuff.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7087
Summary: Ref T2217. Mail writes go through the new code now.
Test Plan: Shipped a bunch of mail in with `./bin/mail`, got reasonable edits as a result.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7086
Summary: Ref T2217. Nothing too surprising here. This transaction type is weird and should be replaced with the mainstream EDGE type at some point after things clear up more.
Test Plan: Attached and detached revisions and mocks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7085
Summary:
Ref T2217. Ship "Merge in Duplicates" through the new editor. The only notable thing here is `setContinueOnMissingFields()`.
The problem this solves is that if you add a custom field and mark it as required, all existing tasks are "invalid" since they don't have a value, and trying to edit them will raise an error like "Some Custom Field is required!". This is fine for normal edits via the UI, since the user can just select/provide a value, but surgical edits to specific fields should just ignore these errors. Add the ability to ignore these errors and use it on all the field-speific editors.
Test Plan: Merged duplicates, including "invalid" duplicates with missing fields.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7084
Summary:
Ref T2217. Swaps batch edits to modern editor.
Also, fix some issues with required fields and viewers being required to render certain standard fields (notably, date).
Test Plan: Made various batch edits, verified they went through properly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7083
Summary:
Ref T2217. Drive reports out of the new table. Nothing too magical going on here.
Also fixes a bug with one of the links from reports.
Test Plan: Viewed reports.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7081
Summary: Ref T2217. All the reads route through new code already, start swapping writes over. This is the simplest writer, used when the user drag-and-drops stuff on the task list.
Test Plan: Dragged and dropped stuff across priorities. Got a transaction and some email. Verified the email and transaction looked OK, threaded properly, etc.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7080
Summary: Ref T2217. No remaining callsites. Also get rid of some methods on ManiphestTransaction that nothing calls anymore.
Test Plan: `grep`, looked at tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7079
Summary: Ref T2217. Nuke this legacy callsite.
Test Plan: Loaded a task, looked at it. Looked the same as before.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7078
Summary: Ref T2217. These are mostly me making stuff up rather than some bird's-eye view of color and iconography across applications, yell if any of these seem off once this rolls out.
Test Plan:
- Looked at a bunch of transactions, saw reasonable looking colors and icons.
- Sent email, saw appropriate subject line actions.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7075
Summary: Ref T2217. Get rid of this rendering pathway's internals and move them to the modern stuff.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7072
Summary:
Ref T2217. This showed the text diff when you updated the description of a task, but is now obsolete.
Remove flags and methods related to rendering this pathway.
Test Plan: Clicked the fancy new "Show Details" instead.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7071
Summary: Ref T2217. Route transaction rendering through modern code. This just affects the detail page. Some rough edges but nothing significant.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7070
Summary:
Ref T2217. Move this off `LegacyQuery` and on to the real deal.
The rest of this patch mostly just replaces some gymnastics to get accurate-ish timestamps for CCs/Owners with `time()`. The search feature where edge time is stored was never really used and isn't necessarily of much value -- most indexers don't bother computing it exactly, and possibly we should get rid of it entirely. If it surfaces in the product again at some point, it's easy enough to make the time data more accurate and reindex.
Test Plan: Ran `bin/search index T12`, etc.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7069
Summary:
Ref T2217. This is the risky, hard part; everything after this should be smooth sailing. This is //mostly// clean, except:
- The old format would opportunistically combine a comment with some other transaction type if it could. We no longer do that, so:
- When migrating, "edit" + "comment" transactions need to be split in two.
- When editing now, we should no longer combine these transaction types.
- These changes are pretty straightforward and low-impact.
- This migration promotes "auxiliary field" data to the new CustomField/StandardField format, so that's not a straight migration either. The formats are very similar, though.
Broadly, this takes the same attack that the auth migration did: proxy all the code through to the new storage. `ManiphestTransaction` is now just an API on top of `ManiphestTransactionPro`, which is the new storage format. The two formats are very similar, so this was mostly a straight copy from one table to the other.
Test Plan:
- Without performing the migration, made a bunch of edits and comments on tasks and verified the new code works correctly.
- Droped the test data and performed the migration.
- Looked at the resulting data for obvious discrepancies.
- Looked at a bunch of tasks and their transaction history.
- Used Conduit to pull transaction data.
- Edited task description and clicked "View Details" on transaction.
- Used batch editor.
- Made a bunch more edits.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7068
Summary:
Ref T2217. I'm going to do the fake-double-writes ("double reads"?) thing where we proxy the storage that worked pretty well for auth. That is:
- (Some more cleanup diffs next, maybe?)
- Move all the data to the new storage, and make `ManiphestTransaction` read and write by wrapping `ManiphestTransactionPro`.
- If nothing breaks, it's a straight shot to nuking ManiphestTransaction callsite by callsite.
I think Maniphest is way easier than Differential, because there are very few query sites and no inline comments.
Test Plan: `grep` to find callsites. Loaded task view, called Conduit.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7067
Summary: Ref T2217. Add the tables and comment class for the new stuff. Not used yet.
Test Plan: Ran storage upgrade, browsed Maniphest.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7066
Summary:
Ref T603. See inlines for an explanation. The case where I hit this was loading the "Pending Differential Revisions" panel in Diffusion when logged out, after making a repository public.
What happens is that we load 10 revisions (say, D1 .. D10) but the user can't see any of them. We then try to load the next 10, but since the pagination is ordered by date modified, we need to base the next query on the modified date of the last thing we loaded (D10). However, since we use the viewer's policies to load that cursor object, it fails to load, and then we just issue the same query over and over again, loading D1 .. D10 until we run out of execution time.
Test Plan: Interface now loads correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7059
Summary: Improves transaction rendering for custom fields and standard custom fields.
Test Plan: See screenshot.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7054
Summary: Gets rid of as much of this as possible. We'll batch handles and remarkup again some day, but after ApplicationTransactions.
Test Plan: Edited, viewed, and checked email for custom field edits.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7037
Summary: Ref T3794. Drop auxiliary field, use standard field.
Test Plan: Performed migration, field seemed to survive it intact. Edited and viewed tasks.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3794
Differential Revision: https://secure.phabricator.com/D7036
Summary: Makes Maniphest look more like standard fields to make the migration easier.
Test Plan: Edited tasks and users with required and invalid fields.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7035
Summary: Make Maniphest use the standard API, `renderEditControl`. Removes custom method `renderControl`.
Test Plan: Created/edited tasks with custom fields.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7034
Summary: Ref T3583. Fixes T3835. Dropbox and Disqus both want these things back, so restore them until we can do something about T3583.
Test Plan: Viewed homepage, clicked "View All X" buttons.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3583, T3835
Differential Revision: https://secure.phabricator.com/D7017
Summary: This works fine for custom queries, but not for builtins.
Test Plan: Exported a builtin query.
Reviewers: btrahan
Reviewed By: btrahan
CC: hach-que, aran
Differential Revision: https://secure.phabricator.com/D7015
Summary:
A couple of things here:
- These links got fixed, but they show all user or project tasks. They should show only open ones.
- Add an anchor so we jump you straight to the results, since the query UI is like a thousand miles tall now. We might take some other approaches here too, but let's see if this feels reasonable.
Test Plan: Clicked "View Tasks" from Profile and Projects. Executed some queries.
Reviewers: btrahan
Reviewed By: btrahan
CC: euresti, aran, chad
Differential Revision: https://secure.phabricator.com/D7014
Summary: The adds the ability to set 'properties' such as state, privacy, due date to the header of objects.
Test Plan: Implemented in Paste, Pholio. Tested various states.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7016
Summary: There's a bunch of stuff that lives only in AuxiliaryField which is called on objects which may be ManiphestCustomFields right now. This is basically a list of remaining API methods which need to be moved to the new stuff. This enables construction of new-style custom fields.
Test Plan: Created a sophisticated Maniphest custom field.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7013
Summary: Ref T418. Although Maniphest does not use ApplicationTransactions, we can fake a lot of it and provide a more uniform API. Deletes as much custom code from Maniphest as possible along the edit workflows, using core code instead.
Test Plan:
With custom fields:
- Edited a task.
- Created a task.
- Queried a task with Maniphest.
- Updated a task with Maniphest.
- Used `?template=nnn` to create a similar task.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T418
Differential Revision: https://secure.phabricator.com/D7001
Summary: Ref T418. Run all the meaningful stuff on the detail page out of shared code.
Test Plan: Looked at detail page, saw custom fields.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T418
Differential Revision: https://secure.phabricator.com/D7000
Summary: Ref T418. Moves data from the Maniphest-specific table to the general one. This patch is a bit gross, but mostly about getting the reads and writes aimed correctly. Future patches will clean things up.
Test Plan: Migrated data across formats. Verified it survied the migration. Viewed and edited tasks' custom fields.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T418
Differential Revision: https://secure.phabricator.com/D6999
Summary: Ref T418. Maniphest has an obsolete class-based field selector. Replace it with CustomField-based selectors, which use the nice config UI and are generally way easier to use.
Test Plan: Added custom fields; edited and viewed custom fields on tasks. Everything worked as expected.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T418
Differential Revision: https://secure.phabricator.com/D6998
Summary: Ref T418. Depends on D6992. This adds index and value storage for Maniphest custom fields.
Test Plan: Ran storage upgrade.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T418
Differential Revision: https://secure.phabricator.com/D6995
Summary: This class is no longer used. It has no callsites.
Test Plan: `grep`
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6996
Summary: A few things link to old URIs for Maniphest, update them.
Test Plan: Clicked all the things.
Reviewers: chad, btrahan
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D6989
Summary:
Removes a bunch of dead stuff:
- Old side nav with hard-coded filters.
- Old edit/list/delete/update interfaces for those filters.
- Old `buildStandardPageResponse()`.
- Some other junk with no callsites.
- Reduce the number of places where the "Create Task" button is built.
Test Plan: `grep`; used list view, batch editor, reports.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6985
Summary: Drive these purely out of configuration after removing behavioral hardcodes in D6981.
Test Plan:
Mucked around with them:
{F58128} {F58129} {F58130}
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Differential Revision: https://secure.phabricator.com/D6984
Summary: Accidentally lost this in the melee. Put it back.
Test Plan: Saw link, then clicked it. Great success!
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6982
Summary:
Ref T3583. Currently, we have some hard-coded behaviors associated with the "Unbreak Now" and "Needs Triage" priorities. Remove them:
- Users seem somewhat confused by these on occasion, and never seem to think they're cool/useful (that I've seen, at least).
- I think they have low utility in general, see T3583.
- Saves three queries on the home page, which can no longer use row counting since they must be policy filtered.
- Primarily, this paves the way for allowing installs to customize priorities, which is an occasional request.
Also deletes a lot of code with no callsites.
Test Plan: Mostly `grep`. Loaded home page. Viewed reports and task list.
Reviewers: btrahan
Reviewed By: btrahan
CC: chad, aran
Maniphest Tasks: T3583
Differential Revision: https://secure.phabricator.com/D6981
Summary: This marks the first time in history that "Pro" has been removed.
Test Plan: `grep`
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6980
Summary: Point these at the new data and URIs.
Test Plan:
- Batch edited some tasks.
- Exported some tasks to excel.
{F58112}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D6978
