1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-13 18:32:41 +01:00
Commit graph

543 commits

Author SHA1 Message Date
epriestley
960da82f33 Fix two migration issues with Maniphest
Summary: The new editor saves tasks before applying transactions, so fields need default values.
Also, I goofed the table name.

Auditors: btrahan
2013-09-24 12:03:47 -07:00
epriestley
099aaa4f94 Render Maniphest custom fields last
Summary: Ref T2217. Fixes T3865. Eventually we'll probably make more of this configurable, but for now shove custom fields down instead of sort of arbitrarily putting them in the middle.

Test Plan: Looked at a task with attached revisions and custom fields, saw custom fields last.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217, T3865

Differential Revision: https://secure.phabricator.com/D7103
2013-09-24 11:10:46 -07:00
epriestley
79ec5222a5 Reduce Maniphest fulltext limit from PHP_INT_MAX to 10k
Summary:
PHP_INT_MAX is rejected by ElasticSearch since it's outside of the representable integer range (see: <https://gist.github.com/JustinTulloss/c4ac0e1c93d6d1e91744>).

Just use 10K, as matching more than 10K results probably isn't useful to anyone.

Test Plan: Confirmed this fixes the issue in IRC. Ran a fulltext search.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D7097
2013-09-24 10:50:56 -07:00
epriestley
6b245c0665 Further smooth "Add CC" interaction in Maniphest
Summary: Ref T2217. Fixes T3877. Improves more behaviors in the presence of "no effect" detection and new comment storage.

Test Plan: Added CCs, added existing CCs, implicitly added CCs via reassign, added CCs with comment.

Reviewers: btrahan, garoevans

Reviewed By: garoevans

CC: aran

Maniphest Tasks: T2217, T3877

Differential Revision: https://secure.phabricator.com/D7100
2013-09-24 10:50:22 -07:00
epriestley
dfcac84a69 Improve some new string translations
Summary: Ref T2217. Cleans up some of the "attached %d file(s)" stuff.

Test Plan: Generated some of these transactions and verified they render more naturally.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7096
2013-09-24 10:49:54 -07:00
epriestley
1327bd2c8a Smooth "no effect" detection in Maniphest for @mentions
Summary:
Ref T2217. Since we aren't actually using subscriptions yet, the "transactions have no effect" detection can trigger for `@mention`s of users who are already CC'd on a task.

Be more conservative about generating a CC transaction.

Test Plan: See screenshot.

Reviewers: btrahan, chad

Reviewed By: chad

CC: chad, aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7095
2013-09-24 10:49:29 -07:00
epriestley
1d7cbe202f Rename "transactionpro" table to "transaction"
Summary: Ref T2217. Cleans up the table names. Moves old data to `maniphest_transaction_legacy`. We'll drop that eventually once it's more clear that I didn't break the world.

Test Plan: Did reads/writes to/from these tables.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7094
2013-09-24 10:49:16 -07:00
epriestley
01ecf1a236 Rename ManiphestTransactionPro -> ManiphestTransaction
Summary: Ref T2217. Pro is the new standard.

Test Plan: Lots of `grep`, made a pile of Maniphest views/edits.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7093
2013-09-24 10:49:06 -07:00
epriestley
5a4e4eb3fd Remove ManiphestTransaction
Summary: Ref T2217. The preview had the last callsite, nuke it.

Test Plan: Used preview. Grepped for `ManiphestTransaction(`, `ManiphestTransaction::`, `'ManiphestTransaction'`, `"ManiphestTransaction"`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7092
2013-09-24 10:48:58 -07:00
Chad Little
0d77a7f39f ObjectHeader Status icons
Summary: Adds status icons and colors to Maniphest and Differential. Also minor tweaks to them in hovercards. Probably some other stuff too.

Test Plan: Test many diff and task states.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D7098
2013-09-24 08:42:04 -07:00
epriestley
9a850b67b0 Minor, sneak in one fix from future Maniphest patches ahead of review
Summary: I goofed this constant when defining mail tags.

Auditors: btrahan
2013-09-24 06:49:31 -07:00
epriestley
2f694f5e3f Gut ManiphestTransactionEditor
Summary:
Ref T2217. Removes most of the code from ManiphestTransactionEditor.

  - Provides mail tag support in ManiphestTransactionEditorPro.
  - There was one more write (subscribe/unsubscribe button) that I'd missed; modernize that.

Test Plan:
  - Clicked subscribe/unsubscribe.
  - Made some edits, verified mail had appropriate mail tags.

Reviewers: btrahan, garoevans

Reviewed By: garoevans

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7091
2013-09-24 06:27:07 -07:00
epriestley
234de8f824 Swap Maniphest edit writes to new transactions
Summary: Ref T2217. This is essentially the last writer, should be able to start deleting code now.

Test Plan: Used "Edit Task" to make a bunch of task edits.

Reviewers: btrahan, garoevans

Reviewed By: garoevans

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7090
2013-09-24 06:26:59 -07:00
epriestley
b9778d89ca Fix assign-on-close behavior for comment controller
Summary: Ref T2217. Missed this while converting this endpoint.

Test Plan: Closed an unassigned task, got assigned.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7089
2013-09-24 05:10:55 -07:00
epriestley
bf14bb45f1 Route Maniphest comments/detail edits through new code
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
2013-09-23 14:33:09 -07:00
epriestley
2a78767a4a Route lipsum writes through the new Maniphest transaction code
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
2013-09-23 14:33:00 -07:00
epriestley
11c7eee783 Route reply handler Maniphest writes through new transaction code
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
2013-09-23 14:32:51 -07:00
epriestley
889eac44ae Route attached objects through new editor
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
2013-09-23 14:32:41 -07:00
epriestley
9afbb9b83b Route task merges through new editor
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
2013-09-23 14:32:32 -07:00
epriestley
6fd1e01fe7 Run Maniphest batch edits through modern editor
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
2013-09-23 14:32:22 -07:00
epriestley
2bc46f097e Run Maniphest Conduit writes through new editor
Summary: Ref T2217. Swap the editors for the Conduit writes.

Test Plan: Made a bazillion Conduit calls, saw tasks updated appropriately.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7082
2013-09-23 14:32:08 -07:00
epriestley
efc7e7cd7e Point Maniphest reports at new transaction table
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
2013-09-23 14:31:58 -07:00
epriestley
69523d30cc Add new-style transaction editor to Maniphest and switch priority edits to it
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
2013-09-23 14:31:47 -07:00
epriestley
28e66090fd Remove ManiphestLegacyTransactionQuery
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
2013-09-23 14:31:13 -07:00
epriestley
6919dddaab Use ManiphestTransactionQuery in detail view
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
2013-09-23 14:31:03 -07:00
epriestley
425590a03a Use ManiphestTransactionQuery directly in maniphest.gettasktransactions
Summary: Ref T2217. Nukes a LegacyQuery callsite.

Test Plan: Called method using console, verified data looks sane.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7077
2013-09-23 14:30:54 -07:00
epriestley
02ed9f1368 Remove ManiphestTransactionDetailView
Summary: Ref T2217. No remaining callsites. Also nukes associated CSS.

Test Plan: `grep`, looked at some tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7076
2013-09-23 14:30:38 -07:00
epriestley
5a104ed905 Add icons, colors and action names to Maniphest timeline view
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
2013-09-23 14:30:29 -07:00
epriestley
d9aa9eec78 Route Maniphest email through the transaction core
Summary: Ref T2217. Build transaction details using transaction code.

Test Plan: See screenshot.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7074
2013-09-23 14:30:20 -07:00
epriestley
1881ac8ad8 Remove ManiphestTransactionListView
Summary: Ref T2217. No remaining callsites.

Test Plan: `grep`

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2217

Differential Revision: https://secure.phabricator.com/D7073
2013-09-23 14:30:12 -07:00
epriestley
e27a83960c Send Maniphest transaction preview through new code
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
2013-09-23 14:29:59 -07:00
epriestley
781c11560f Remove ManiphestTaskDescriptionChangeController
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
2013-09-23 14:29:50 -07:00
epriestley
3ba3745d67 Switch Maniphest to modern ApplicationTransaction rendering
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
2013-09-23 14:29:40 -07:00
epriestley
77475736a3 Switch Maniphest search indexing to real ApplicationTransactions
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
2013-09-23 14:29:31 -07:00
epriestley
7abe9dc4c0 Migrate all Maniphest transaction data to new storage
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
2013-09-23 14:25:28 -07:00
epriestley
17e1732152 Route all ManiphestTransaction reads through a single class
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
2013-09-23 14:25:14 -07:00
epriestley
4c2bd2ca8e Add storage for new Maniphest transactions
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
2013-09-23 14:24:58 -07:00
epriestley
e7a7e43104 Fix a bug where policy queries with cursor-based pagers and non-ID orders can go into infinite loops
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
2013-09-21 16:23:44 -07:00
epriestley
a82992e9a5 Render Maniphest fields in an application-transactions-compatible way
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
2013-09-21 16:23:17 -07:00
epriestley
5251e2f14d Remove vestigal AuxiliaryField code from Maniphest
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
2013-09-19 11:56:30 -07:00
epriestley
0558d53273 Convert maniphest to use standard fields
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
2013-09-19 11:56:15 -07:00
epriestley
2088b99078 Normalize validation/requried API
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
2013-09-19 11:55:54 -07:00
epriestley
30159d9ad9 Normalize renderControl API
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
2013-09-19 11:55:45 -07:00
epriestley
24e07df434 Restore "Unbreak Now" and "Needs Triage" homepage boxes as cusomizable elements
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
2013-09-17 11:31:32 -07:00
epriestley
bab9a28ab9 Fix excel export of builtin queries
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
2013-09-17 11:30:48 -07:00
epriestley
a695f264bf Make user and project profile links into Maniphest show only open tasks
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
2013-09-17 11:30:39 -07:00
Chad Little
e8bb24fd60 Policy, Status in PHUIHeaderView
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
2013-09-17 09:12:37 -07:00
epriestley
01eedd6e6a Make ManiphestCustomField actually implement the interface it ostensibly exposes
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
2013-09-16 16:05:29 -07:00
epriestley
ed126cd47e Provide ApplicationSearch hooks in Maniphest
Summary: Ref T418. Adds hooks to support customized ApplicationSearch (you can't currently add indexable fields without writing custom code).

Test Plan: Wrote custom code to add an indexable field.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T418

Differential Revision: https://secure.phabricator.com/D7002
2013-09-16 16:03:09 -07:00
epriestley
dd4537edef Route Maniphest editing and transactions primarily through shared code
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
2013-09-16 16:03:01 -07:00
epriestley
7034ac3a5a Route Maniphest detail view through common custom field code
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
2013-09-16 16:02:27 -07:00
epriestley
bd40e74400 Migrate auxiliary field storage to common field storage
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
2013-09-16 16:02:06 -07:00
epriestley
28eaacb491 Remove ManiphestTaskExtensions
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
2013-09-16 15:58:35 -07:00
epriestley
b0e145f2fe Provide generalized custom field storage and custom field indexes in Maniphest
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
2013-09-16 14:06:41 -07:00
epriestley
0fa8db1413 Remove ManiphestSavedQuery
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
2013-09-15 17:32:13 -07:00
epriestley
c6ea59ae0d Fix some Maniphest links
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
2013-09-13 16:56:21 -07:00
epriestley
23c5fccaa3 Clean up dead code from old task list
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
2013-09-13 11:50:29 -07:00
epriestley
d97d8d5fc4 Make Maniphest task priorites user-customizable
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
2013-09-13 11:50:28 -07:00
epriestley
7139655969 Restore missing "Reports" link to Maniphest
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
2013-09-13 11:50:26 -07:00
epriestley
092f540199 Remove all hardcoded behaviors associated with task priorities
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
2013-09-13 11:50:15 -07:00
epriestley
91880b5a51 Rename ManiphestTaskListControllerPro to ManiphestTaskListController
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
2013-09-13 11:49:49 -07:00
epriestley
b558e1b4a4 Remove ManiphestTaskListController
Summary:
Fixes T3273. Fixes T3088. Fixes T2992. Fixes T1163. Fixes T1008. Ref T3241. Ref T1386. Ref T418. Ref T2625. Ref T603.

Replaces task list with one powered by ApplicationSearch.

Test Plan:
After many days of work, the task search interface now looks exactly the same as it did before.

{F58114}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, chad

Maniphest Tasks: T418, T603, T1008, T1163, T1386, T2625, T2992, T3088, T3241, T3273

Differential Revision: https://secure.phabricator.com/D6979
2013-09-13 11:49:48 -07:00
epriestley
0803f5cb45 Update batch edit and export to excel for "pro" queries
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
2013-09-13 11:49:47 -07:00
epriestley
a82c3a8914 Allow Maniphest queries to have configurable page sizes
Summary:
Current page size is `1000`. This is nice to have in some cases, but makes pages slower than necessary in others. Task lists are generally dominated by rendering costs.

For example, my default is "recent tasks", which just lists all tasks ordered by date created. Showing 100 tasks here instead of 1000 makes this several times faster without compromising utility.

I don't want to force the default to 100, though, since sometimes listing everything is quite useful and I think an advantage of Maniphest is that it generally deals reasonably well with large task sets.

(This `limit` property is actually read by the default implementation of `getPageSize()` in the parent class.)

Test Plan: Made queries with page sizes 1, 100, 12, 9, 3000, etc.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6976
2013-09-13 11:49:44 -07:00
epriestley
62e66e3cf6 Completely implement cursor paging in Maniphest
Summary: Ref T2625. Fixes user and project paging. Adds visibility-aware project group filtering.

Test Plan: Set page size very small and paged forward and backward in Maniphest, particularly with "Assigned" and "Project" group-by filters.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6973
2013-09-13 11:49:43 -07:00
epriestley
5062abc534 Most implement cursor paging in Maniphest
Summary:
Ref T2625. Depends on D6971. Maniphest is complicated to implement cursor paging for. Builds on D6971 to do so.

This is //almost// complete. Paging on projects and authors doesn't quite work, I'll clean that up shortly. Left some TODOs.

Test Plan: Set page size to `3`, paged forward and backward in a bunch of group/order modes. Results seemed to be as expected.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6972
2013-09-13 11:49:42 -07:00
epriestley
eca3f44301 Make "pro" controller render results the same way the less-pro controller does
Summary: Swaps the rendering over to the current rendering. This is mostly copy/paste out of TaskListController, which is going to get nuked, with some cleanup.

Test Plan:
{F58064}

  - Ran a bunch of queries.
  - Viewed empty states.
  - Drag-and-dropped stuff.
  - (Batch editor / excel export need a tweak to run the new-style queries.)

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6961
2013-09-12 16:58:09 -07:00
epriestley
9b3520ea57 Add "group" to Maniphest "pro" search
Summary: This is the last missing filter.

Test Plan: Grouped results by a bunch of stuff.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6960
2013-09-12 14:48:52 -07:00
epriestley
ddfc6bbc9e Service "Group by: Project" in Maniphest out of a local index
Summary:
See discussion in D6955. Currently, the logic for "Group by: Project" is roughly:

  - Load every possible result.
  - Lots of in-process garbage.

Instead, use the new local project name index (from D6957) to service this query more reasonably. Basically:

  - Join a table which has keyed project names.
  - Order by that table.

Test Plan: {F58033}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6958
2013-09-12 13:08:25 -07:00
epriestley
e50eccf109 Provide and populate an object name index for Maniphest
Summary: See discussion in D6955. This provides a table we can JOIN against to (effectively) "ORDER BY project name", populates it intially, and keeps it up to date as projects are edited.

Test Plan:
  - Ran storage upgrade, verified projects populated into the table.
  - Edited a project, verified its entry updated.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6957
2013-09-12 13:06:44 -07:00
epriestley
df86f87289 Add a "dateCreated" key to Maniphest
Summary: Depends on D6952. Unpunts there since I'm rolling into a swamp full of schema changes.

Test Plan: Issued date-constrained query and saw key as a candidate.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6954
2013-09-12 13:04:31 -07:00
epriestley
8f8c61be31 Remove legacy "touched" table and indexing
Summary: Noticed this in the schema. "Touches" were an idea that never really got off the ground, as we built out more/better notification channels instead. Essentially, they recorded any object you'd ever interacted with. Maybe this will be useful some day, but for now it does nothing and can't be interacted with. Nuke it.

Test Plan: `grep`, loaded Maniphest.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6953
2013-09-12 13:04:09 -07:00
epriestley
1c43fceffb Add date filtering to Maniphest "pro" search
Summary: Adds date created filtering. There's a task for this somewhere that I can't immediately find.

Test Plan: Filtered tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6952
2013-09-12 13:03:39 -07:00
epriestley
bdef58216e Restore project filtering to Maniphest "pro" search
Summary: Restores any/all/user/exclude project filters to the new search.

Test Plan: Filtered stuff by projects.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6951
2013-09-12 13:03:14 -07:00
epriestley
f679ea7d7e Add "fulltext" to Maniphest pro search
Summary: Restores this field to the new ApplicationSearch-based search.

Test Plan: Used fulltext search to find tasks.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6950
2013-09-12 13:03:05 -07:00
epriestley
a3c6e9aebf Move ManiphestTaskQuery into query/
Summary: Move this into a more consistent location.

Test Plan: Loaded Maniphest.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D6949
2013-09-12 13:02:55 -07:00
Bob Trahan
b902005bed Kill PhabricatorObjectDataHandle
Summary: Ref T603. Killing this class is cool because the classes that replace it are policy-aware. Tried to keep my wits about me as I did this and fixed a few random things along the way. (Ones I remember right now are pulling a query outside of a foreach loop in Releeph and fixing the text in UIExample to note that the ace of hearts if "a powerful" card and not the "most powerful" card (Q of spades gets that honor IMO))

Test Plan: tested the first few changes (execute, executeOne X handle, object) then got real mechanical / careful with the other changes.

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran, FacebookPOC

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D6941
2013-09-11 12:27:28 -07:00
Bob Trahan
07b8becfc6 Policy - introduce parentQuery and pass around policy configuration from parent to child
Summary: Ref T603. Ref D6941.

Test Plan: Clicked around all over - looked good. I plan to re-test D6941 to make sure the executeOne case works now as intended

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D6944
2013-09-11 12:19:34 -07:00
epriestley
0ce3121170 Continue not fataling on legacy Maniphest queries
Summary: Ref T3817. See that task for discussion.

Auditors: btrahan
2013-09-11 08:50:39 -07:00
epriestley
004edbaf7c Don't fatal after failing to load grouped projects in Maniphest
Summary: Fixes T3817. This junk is getting wiped out soon so I'm punting here
and fixing the symptom rather than the root cause.

Auditors: btrahan
2013-09-11 08:44:53 -07:00
epriestley
a8171889bd Add "IDs" and "Priority" to pro search
Summary: Ref T2625. Restore these, too.

Test Plan: Executed queries using these fields.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6937
2013-09-10 15:34:13 -07:00
epriestley
f386099735 Add support for "status" and "order" to pro search
Summary: Ref T2625. Further expands the "pro" search.

Test Plan: Used new options to query tasks.

Reviewers: btrahan, garoevans

Reviewed By: garoevans

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6935
2013-09-10 15:34:11 -07:00
epriestley
8c9d61bedc Add "assigned" and "authors" to Maniphest pro search
Summary: Ref T2625. Moves this a step toward being able to replace the current search.

Test Plan: Used search interface.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2625

Differential Revision: https://secure.phabricator.com/D6934
2013-09-10 15:34:10 -07:00
epriestley
e814291526 Introduce ManiphestTaskSearchEngine plus ManiphestTaskListControllerPro
Summary:
Ref T603. Ref T2625. Cutting this over is tricky because of Maniphest's existing saved queries. Plan here is:

  - Build out the "pro" controller at `/maniphest/query/`.
  - Once it's at parity, migrate custom queries.
  - Nuke the old UI.

This provides a minimal implementation with no filter support.

Test Plan: Looked at `/maniphest/query/`, saw results technically available.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603, T2625

Differential Revision: https://secure.phabricator.com/D6933
2013-09-10 15:34:08 -07:00
epriestley
1e42c62b8f Make ManiphestTaskQuery a (mostly) policy-aware query
Summary: Ref T603.

Test Plan: Viewed home and maniphest, fiddled all the knobs.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D6930
2013-09-10 15:34:06 -07:00
epriestley
a2571de575 Remove obsolete/deprecated withTaskIDs() / withTaskPHIDs()
Summary: Ref T603. These were deprecated some time ago in favor of the more standard withIDs() / withPHIDs().

Test Plan: `grep`, loaded some interfaces.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D6929
2013-09-10 15:34:04 -07:00
epriestley
e625c91867 Pass viewer to all ManiphestTaskQuery objects
Summary: Ref T603. Prepare for conversion to a policy-aware query.

Test Plan: Browsed various interfaces which use this stuff.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D6928
2013-09-10 15:34:03 -07:00
epriestley
5651141520 Make Maniphest custom fields extend PhabricatorCustomField
Summary:
Ref T418. These implementations share no method names, so we can safely just move Maniphest fields into the `PhabricatorCustomField` hierarchy.

Replaces two Maniphest-specific custom field exceptions which nothing catches.

Test Plan: Viewed Maniphest, edited/altered custom fields.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T418

Differential Revision: https://secure.phabricator.com/D6927
2013-09-10 15:31:48 -07:00
Chad Little
5ba20b8924 Move PhabricatorObjectItem to PHUIObjectItem, add 'plain' setting for lists.
Summary: Adds plain support for object lists that just look like lists

Test Plan: review UIexamples and a number of other applications

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6922
2013-09-09 14:14:34 -07:00
Gareth Evans
fcba0c74d9 Replace all "attach first..." exceptions with assertAttached()
Summary:
Ref T3599
Go through everything, grep a bit, replace some bits.

Test Plan: Navigate around a bit

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Maniphest Tasks: T3599

Differential Revision: https://secure.phabricator.com/D6871
2013-09-03 06:02:14 -07:00
Bob Trahan
83e0380046 Maniphest - fix fatal in custom query for users with no projects
Summary: See https://github.com/facebook/phabricator/issues/380 for report.

Test Plan: Maniphest -> Custom Query -> Put user with no project in "Any User Projects" field -> Search : observe no fatal

Reviewers: epriestley

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6862
2013-09-02 13:24:41 -07:00
Chad Little
fe2a96e37f Update Form Layouts
Summary:
This attempts some consistency in form layouts. Notably, they all now contain headers and are 16px off the sides and tops of pages. Also updated dialogs to the same look and feel. I think I got 98% of forms with this pass, but it's likely I missed some buried somewhere.

TODO: will take another pass as consolidating these colors and new gradients in another diff.

Test Plan: Played in my sandbox all week. Please play with it too and let me know how they feel.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: Korvin, aran

Differential Revision: https://secure.phabricator.com/D6806
2013-08-26 11:53:11 -07:00
epriestley
751cd547c2 Remove dust from page construction
Summary:
  ^\s+(['"])dust\1\s*=>\s*true,?\s*$\n

Test Plan: Looked through the diff.

Reviewers: chad

Reviewed By: chad

CC: aran

Differential Revision: https://secure.phabricator.com/D6769
2013-08-19 18:09:35 -07:00
Eric Stern
563f3ea236 Create 'Add Task' link on project/view page
Reviewed by: epriestley

See: https://github.com/facebook/phabricator/pull/368
2013-08-12 08:13:31 -07:00
epriestley
193a9611e4 Partially generalize Remarkup previews and add support to Differential
Summary:
Ref T3671. A lot of applications have pretty ad-hoc preview code. Clean it up a bit and add Summary preview to Differential.

After ApplicationTransactions we might want to try to serialize the whole form and show a preview of all the transactions, but this seems not very useful in most cases (I'd guess that Remarkup previews are 99% of the value) and tricky to get right (e.g., adding images which don't exist yet to Pholio mocks).

I think I can add this in a few other places, too.

Test Plan:
Edited Maniphest Tasks and Differential Revisions, mashed some buttons. Verified previews rendered correctly. Grepped for removed CSS classes (no hits).

{F52907}

Reviewers: btrahan, Firehed

Reviewed By: btrahan

CC: aran, chad

Maniphest Tasks: T3671

Differential Revision: https://secure.phabricator.com/D6673
2013-08-05 10:46:39 -07:00
epriestley
02ccca4bbd Fix Maniphest fatal if attached tasks are not an array
Summary: Fixes T3678. I think some very old rows may have a junk value here. This will be obsoleted by ApplicationTransactions and other modernization, most likely, so just fix it locally.

Test Plan: looked at a task

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3678

Differential Revision: https://secure.phabricator.com/D6677
2013-08-05 10:27:50 -07:00
Bob Trahan
1cb0db8755 Move PhabricatorUser to new phid stuff
Summary: Ref T2715. Had to start loading status information in the query class. Debated trying to clean up some of the attach / load stuff but decided to just add status under the new paradigm for now.

Test Plan: phid.query  also made a status and checked that out. also played in conpherence.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2715

Differential Revision: https://secure.phabricator.com/D6585
2013-07-26 14:05:19 -07:00
epriestley
4a48d8a1f5 Minor, fix a small typo in ManiphestTaskQuery. 2013-07-24 17:31:49 -07:00
epriestley
69d5a9af58 Use Application PHIDs in Projects
Summary: Ref T2715. Move Projects to the new stuff.

Test Plan: Used `phid.query` to load projects.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2715

Differential Revision: https://secure.phabricator.com/D6526
2013-07-22 12:17:53 -07:00
epriestley
7cbe82d777 Use modern UI elements and application/event-oriented construction in Projects
Summary:
Ref T2715. This partially modernizes projects.

Precursor to using ApplicationSearch, which is a precursor to using application PHIDs.

Then, some day, we will make projects good.

Test Plan: {F51126}

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran

Maniphest Tasks: T2715

Differential Revision: https://secure.phabricator.com/D6524
2013-07-22 12:17:51 -07:00