Summary:
Fixes T4158. Two possible refinements:
- Maybe we should make all of these things respect `ManiphestCapabilityEditAssign::CAPABILITY`, etc. I think it's reasonable either way, and this is probably more intuitive and useful for most cases.
- Maybe we should check that you can see the policies before copying them. Again, this is sort of reasonable either way.
Test Plan: Created a new task from a template, saw that it inherited policies.
Reviewers: btrahan, hach-que
Reviewed By: hach-que
CC: aran
Maniphest Tasks: T4158
Differential Revision: https://secure.phabricator.com/D7649
Summary: Simplifies the code a bit and fixes all the wonky previews. Fixes T4053
Test Plan: Test all pages, logged in and logged out.
Reviewers: epriestley
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Maniphest Tasks: T4053
Differential Revision: https://secure.phabricator.com/D7622
Summary:
Fixes T4095. Fixes T3817.
- The batch editor has some funky handle code which misses projects, share that.
- Remove some hacks for T3817 that should be good now.
Test Plan: Looked at batch editor, saw projects. Looked at task list.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, martin.schulz
Maniphest Tasks: T3817, T4095
Differential Revision: https://secure.phabricator.com/D7578
Summary: Adds summary (description) and test plan icons to make these area's more unique and differentiated over general sections.
Test Plan: Test a diff, a commit, a task
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7493
Summary: Some scripts might find it easier to work with PHIDs instead of user names.
Test Plan:
Use ?assign=<username> and ?assign=<PHID-USER> with the create task URI.
See assignee input being filled correctly.
Reviewers: #blessed_reviewers, epriestley
Reviewed By: epriestley
CC: epriestley, aran, Korvin
Differential Revision: https://secure.phabricator.com/D7401
Summary:
Projects and priority inputs can be prefilled similar to how title
and description fields work.
Prefilling of projects already worked but used PHIDs instead of
more user friendly name so I changed that too.
Test Plan:
Visit [[/maniphest/task/create/?projects=Maniphest;Easy&priority=100&assign=vrana&title=Hip-hip&description=hooray!|example]]
and see prefilled form fields.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7394
Summary:
Ref T1344. This is //very// rough. Some UI issues:
- Empty states for the board and columns are junky.
- Column widths are crazy. I think we need to set them to fixed-width, since we may have an arbitrarily large number of columns?
- I don't think we have the header UI elements in M10 yet and that mock is pretty old, so I sort of very roughly approximated it.
- What should we do when you click a task title? Popping the whole task in a dialog is possible but needs a bunch of work to actually work. Might need to build "sheets" or something.
- Icons are slightly clipped for some reason.
- All the backend stuff is totally faked.
Generally, my plan is just to use these to implement all of T390. Specifically:
- "Kanban" projects will have "Backlog" on the left. You'll drag them toward the right as you make progress.
- "Milestone" projects will have "No Milestone" on the left, then "Milestone 9", "Milestone 8", etc.
- "Sprint" projects will have "Backlog" on the left, then "Sprint 31", "Sprint 30", etc.
So all of these things end up being pretty much exactly the same, with some minor text changes and new columns showing up on the left vs the right or whatever.
Test Plan: See screenshot.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: chad, aran, sascha-egerer
Maniphest Tasks: T1344
Differential Revision: https://secure.phabricator.com/D7374
Summary:
Fixes T3675.
- Maniphest had a couple of old non-event listeners; move them to events.
- Make most of the similar listeners a little more similar.
- Add checks for access to the application.
Test Plan:
- Viewed profile, project, task, revision.
- Clicked all the actions.
- Blocked access to various applications and verified the actions vanished.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T3675
Differential Revision: https://secure.phabricator.com/D7365
Summary:
This is primarily a client request, and a little bit use-case specific, but policies seem to be holding up well and I'm getting more comfortable about maintaining this. Much if it can run through ApplicationTransactions.
Allow the ability to edit status, policies, priorities, assignees and projects of a task to be restricted to some subset of users. Also allow bulk edit to be locked. This affects the editor itself and the edit, view and list interfaces.
Test Plan: As a restricted user, created, edited and commented on tasks. Tried to drag them around.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7357
Summary: Drop the "Pro" bit.
Test Plan: Created/edited tasks, moved tasks around, generally made a mess. Nothing burned down.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7352
Summary: Ref T603. While policies aren't completely perfect, they are substantially functional to the best of my knowledge -- definitely in good enough shape that we want to hear about issues with them, now.
Test Plan: Edited a task, repository, and project.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7343
Summary:
Ref T603. Fixes T2823. This updates Paste and Macro.
- **Paste**
- Added default view policy.
- I didn't add a "create" policy, since I can't come up with any realistic scenario where you'd give users access to pastes but not let them create them.
- **Macro**
- Added a "manage" policy, which covers creating and editing macros. This lets an install only allow "People With An Approved Sense of Humor" or whatever to create macros.
- Removed the "edit" policy, since giving individual users access to specific macros doesn't make much sense to me.
- Changed the view policy to the "most public" policy the install allows.
- Added view policy information to the header.
Also fix a couple of minor things in Maniphest.
Test Plan:
- Set Paste policy, created pastes via web and Conduit, saw they got the right default policies.
- Set Macro policy, tried to create/edit macros with valid and unauthorized users.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2823, T603
Differential Revision: https://secure.phabricator.com/D7317
Summary: This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.
Test Plan: Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, epriestley, aran
Differential Revision: https://secure.phabricator.com/D7283
Summary: Ref T603. Allow global default policies to be configured for tasks.
Test Plan:
- Created task via web UI.
- Created task via Conduit.
- Created task via email.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7267
Summary: Ref T1279. Prerequisite for adding icons or other type information to tokenizers, since we don't currently have enough information to prefill them when rendering things from the server side. By passing handles in, the tokenizer can extract type information.
Test Plan:
- Searched by user in Audit.
- Sent Conpherence from profile page.
- Tried to send an empty conpherence.
- Searched Countdown by user.
- Edited CCs in Differential.
- Edited reviewers in Differential.
- Edited a commit's projects.
- Searched lint by owner.
- Searched feed by owner/project.
- Searched files by owner.
- Searched Herald by owner.
- Searched Legalpad by owner.
- Searched Macro by owner.
- Filtered Maniphest reports by project.
- Edited CCs in Maniphest.
- Searched Owners by owner.
- Edited an Owners package.
- Searched Paste by owner.
- Searched activity logs by owner.
- Searched for mocks by owner.
- Edited a mock's CCs.
- Searched Ponder by owner.
- Searched projects by owner.
- Edited a Releeph project's pushers.
- Searched Releeph by requestor.
- Edited "Uses Symbols" for an Arcanist project.
- Edited all tokenizers in main search.
- Searched Slowvote by user.
Reviewers: chad, btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T1279
Differential Revision: https://secure.phabricator.com/D7248
Summary: Ref T603. Swaps out most `PhabricatorFile` loads for `PhabricatorFileQuery`.
Test Plan:
- Viewed Differential changesets.
- Used `file.info`.
- Used `file.download`.
- Viewed a file.
- Deleted a file.
- Used `/Fnnnn` to access a file.
- Uploaded an image, verified a thumbnail generated.
- Created and edited a macro.
- Added a meme.
- Did old-school attach-a-file-to-a-task.
- Viewed a paste.
- Viewed a mock.
- Embedded a mock.
- Profiled a page.
- Parsed a commit with image files linked to a revision with image files.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7178
Summary:
Three changes here.
- Add `setActionList()`, and use that to set the action list.
- Add `setPropertyList()`, and use that to set the property list.
These will let us add some apropriate CSS so we can fix the border issue, and get rid of a bunch of goofy `.x + .y` selectors.
- Replace `addContent()` with `appendChild()`.
This is just a consistency thing; `AphrontView` already provides `appendChild()`, and `addContent()` did the same thing.
Test Plan:
- Viewed "All Config".
- Viewed a countdown.
- Viewed a revision (add comment, change list, table of contents, comment, local commits, open revisions affecting these files, update history).
- Viewed Diffusion (browse, change, history, repository, lint).
- Viewed Drydock (resource, lease).
- Viewed Files.
- Viewed Herald.
- Viewed Legalpad.
- Viewed macro (edit, edit audio, view).
- Viewed Maniphest.
- Viewed Applications.
- Viewed Paste.
- Viewed People.
- Viewed Phulux.
- Viewed Pholio.
- Viewed Phame (blog, post).
- Viewed Phortune (account, product).
- Viewed Ponder (questions, answers, comments).
- Viewed Releeph.
- Viewed Projects.
- Viewed Slowvote.
NOTE: Images in Files aren't on a black background anymore -- I assume that's on purpose?
NOTE: Some jankiness in Phortune, I'll clean that up when I get back to it. Not related to this diff.
Reviewers: chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D7174
Summary: This adds the 'PHUIObjectBox' to nearly every place that should get it. I need to comb through Diffusion a little more. I've left Differential mostly alone, but may decide to do it anyways this weekend. I'm sure I missed something else, but these are easy enough to update.
Test Plan: tested each new layout.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7162
Summary:
Ref T603. Basically:
- Hide "Reports".
- Hide "batch edit" and "export to excel".
- Hide reprioritization controls.
- I left the edit controls, they show a "login to continue" dialog when hit.
- Allow tokenizer results to fill for public users.
- Fix a bug where membership in projects was computed incorrectly in certain cases.
- Add a unit test covering the project membership bug.
Test Plan: Viewed /maniphest/ when logged out, and while logged in.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7126
Summary:
Ref T603. Disable things the user can't use, allow logged-out users to get a reasonable version of the page.
Also allow logged-out users to view edit history of comments if they're able to see the object.
Test Plan: Viewed Maniphest detail as a logged-out user, got a largely sensible page.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7124
Summary:
Ref T603. Adds policy controls to the task edit UI.
@chad, status + policy renders a little weird -- did I mess something up? See screenshot.
Test Plan: Edited policies, viewed a task.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7123
Summary:
Ref T603. Make almost every task read policy-aware. Notable exceptions are:
- Edge editor -- this stuff is prescreened and should be moved to ApplicationTransactions eventually anyway.
- Search/attach stuff -- this stuff needs some general work. The actual list should be fine since you can't pull handles. There may be a very indirect hole here where you could attach an object you can't see (but do know the ID of) to an object you can see. Pretty fluff.
- The "Tasks" field in Differential will let you reference objects you can't see. Possibly this is desirable, in the case of commandeering revisions. Mostly, it was inconvenient to get a viewer (I think).
Test Plan:
- Called `maniphest.info`.
- Called `maniphest.update`.
- Batch edited tasks.
- Dragged and dropped tasks to change subpriority.
- Subscribed and unsubscribed from a task.
- Edited a task.
- Created a task.
- Created a task with a parent.
- Created a task with a template.
- Previewed a task update.
- Commented on a task.
- Added a dependency.
- Searched for "T33" in object search dialog.
- Created a branch "T33", ran `arc diff`, verified link.
- Pushed a commit with "Fixes T33", verified close.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T603
Differential Revision: https://secure.phabricator.com/D7119
Summary: I'd like to reuse this for other content areas, renaming for now. This might be weird to keep setForm, but I can fix that later if we need.
Test Plan: reload a few forms in maniphest, projects, differential
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: Korvin, aran
Differential Revision: https://secure.phabricator.com/D7120
Summary: Ref T2217. These checks are no longer necessary, ApplicationTransactions handle them for us.
Test Plan:
- Made a no-effect edit, verified no new transactions showed up.
- Made real edits, saw them happen and leave transactions.
- Made an edit which just reorders CCs, saw it detected as no-effect.
- As above, with projects.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7117
Summary:
Ref T2217. Fixes two issues:
# The "task created" email didn't include the task description, but should.
# We were treaging the "status" event as the "create", but that's kind of a mess. Treat the "title" event as the "create" instead. This makes initial emails say "[Created]".
Test Plan: Created some tasks, got better emails.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T2217
Differential Revision: https://secure.phabricator.com/D7115
Summary: These constants have moved to ManiphestTransaction. The other method only has one plausible callsite, just inline it.
Test Plan: Used Maniphest.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D7113
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
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
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
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
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
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
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
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
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 "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. 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. 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. 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: 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
