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

63 commits

Author SHA1 Message Date
epriestley
fab37aa4e3 When accepting revisions, allow users to accept on behalf of a subset of reviewers
Summary:
Ref T12271. Currenty, when you "Accept" a revision, you always accept it for all reviewers you have authority over.

There are some situations where communication can be more clear if users can accept as only themselves, or for only some packages, etc. T12271 discusses some of these use cases in more depth.

Instead of making "Accept" a blanket action, default it to doing what it does now but let the user uncheck reviewers.

In cases where project/package reviewers aren't in use, this doesn't change anything.

For now, "reject" still acts the old way (reject everything). We could make that use checkboxes too, but I'm not sure there's as much of a use case for it, and I generally want users who are blocking stuff to have more direct accountability in a product sense.

Test Plan:
  - Accepted normally.
  - Accepted a subset.
  - Tried to accept none.
  - Tried to accept bogus reviewers.
  - Accepted with myself not a reviewer
  - Accepted with only one reviewer (just got normal "this will be accepted" text).

{F4251255}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12271

Differential Revision: https://secure.phabricator.com/D17533
2017-03-22 14:25:04 -07:00
epriestley
0a0ac1302f Prevent users from taking "edit"-like actions via comment forms if they don't have edit permission
Summary:
Ref T12335. Fixes T11207. Edit-like interactions which are not performed via "Edit <object>" are a bit of a grey area, policy-wise.

For example, you can correctly do these things to an object you can't edit:

  - Comment on it.
  - Award tokens.
  - Subscribe or unsubscribe.
  - Subscribe other users by mentioning them.
  - Perform review.
  - Perform audit.
  - (Maybe some other stuff.)

These behaviors are all desirable and correct. But, particularly now that we offer stacked actions, you can do a bunch of other stuff which you shouldn't really be able to, like changing the status and priority of tasks you can't edit, as long as you submit the change via the comment form.

(Before the advent of stacked actions there were fewer things you could do via the comment form, and more of them were very "grey area", especially since "Change Subscribers" was just "Add Subscribers", which you can do via mentions.)

This isn't too much of a problem in practice because we won't //show// you those actions if the edit form you'd end up on doesn't have those fields. So on intalls like ours where we've created simple + advanced flows, users who shouldn't be changing task priorities generally don't see an option to do so, even though they technically could if they mucked with the HTML.

Change this behavior to be more strict: unless an action explicitly says that it doesn't need edit permission (comment, review, audit) don't show it to users who don't have edit permission and don't let them take the action.

Test Plan:
  - As a user who could not edit a task, tried to change status via comment form; received policy exception.
  - As a user who could not edit a task, viewed a comment form: no actions available (just "comment").
  - As a user who could not edit a revision, viewed a revision form: only "review" actions available (accept, resign, etc).
  - Viewed a commit form but these are kind of moot because there's no separate edit permission.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12335, T11207

Differential Revision: https://secure.phabricator.com/D17452
2017-03-02 16:56:57 -08:00
epriestley
35750b9c61 Make some Differential comment actions (like "Accept" and "Reject") conflict with one another
Summary:
Ref T11114. When a user selects "Accept", and then selects "Reject", remove the "Accept". It does not make sense to both accept and reject a revision.

For now, every one of the "actions" conflicts: accept, reject, resign, claim, close, commandeer, etc, etc. I couldn't come up with any combinations that it seems like users are reasonably likely to want to try, and we haven't received combo-action requests in the past that I can recall.

Test Plan:
  - Selected "Accept", then selected "Reject". One replaced the other.
  - Selected "Accept", then selected "Change Subscribers". Both co-existed happily.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17132
2017-01-02 13:25:12 -08:00
epriestley
48fcfeadaf Allow comment actions to be grouped; group Differential "Review" and "Revision" actions
Summary:
Ref T11114. Differential has more actions than it once did, and may have further actions in the future.

Make this dropdown a little easier to parse by grouping similar types of actions, like "Accept" and "Reject".

(The action order still needs to be tweaked a bit.)

Test Plan: {F2274526}

Reviewers: chad

Reviewed By: chad

Subscribers: eadler

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17114
2016-12-31 10:09:41 -08:00
epriestley
3c5a17ba8a Restore "Reclaim" and "Abandon" actions to Differential on EditEngine
Summary: Ref T11114. This begins restoring comment actions to Differential, but on top of EditEngine.

Test Plan: {F2263148}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17107
2016-12-31 10:06:46 -08:00
epriestley
32ce21a181 Allow the new Differential EditEngine form to create/update diffs for revisions
Summary: Ref T11114. Much of this is around making the "comment-while-updating" flow work correctly.

Test Plan:
  - Created new diffs by copy/pasting, then:
    - used one to create a new revision;
    - used one to update an existing revision, with a comment.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11114

Differential Revision: https://secure.phabricator.com/D17053
2016-12-14 07:27:25 -08:00
epriestley
f7b0c09ac4 Make the "All Day Event" control use a checkbox instead of a dropdown
Summary:
This feels a little cleaner:

  - Clean up transaction log a bit.
  - Use a checkbox instead of a two-option dropdown.

This is a little messy because the browser doesn't send anything if the user submits a form with an un-clicked checkbox.

We now send a dummy value ("Hey, there's definitely a checkbox in this form!") so the server can figure out what to do.

Test Plan:
  - Edited all-dayness of an event.
  - Viewed transaction log.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16776
2016-10-31 14:18:59 -07:00
epriestley
2bbddb8c0f Improve some setInitialValue() behavior for PhortuneMerchants
Summary:
This fixes the permissions issue with D16750, which is actually not really a permissions issue, exactly.

This is the only place anywhere that we use a tokenizer field //and// give it a default value which is not the same as the object value (when creating a merchant, we default it to the viewer).

In other cases (like Maniphest) we avoid this because you can edit the form to have defaults, which would collide with whatever default we provide. Some disucssion in T10222.

Since we aren't going to let you edit these forms for the forseeable future, this behavior is reasonable here though.

However, it triggered a sort-of-bug related to conflict detection for these fields (see T4768). These fields actually have two values: a hidden "initial" value, and a visible edited value.

When you submit the form, we compute your edit by comparing the edited value to the initial value, then applying adds/removes, instead of just saying "set value equal to new value". This prevents issues when two people edit at the same time and both make changes to the field.

In this case, the initial value was being set to the display value, so the field would say "Value: [(alincoln x)]" but internally have that as the intitial value, too. When you submitted, it would see "you didn't change anything", and thus not add any members.

So the viewer wouldn't actually be added as a member, then the policy check would correctly fail.

Note that there are still some policy issues here (you can remove yourself from a Merchant and lock yourself out) but they fall into the realm of stuff discussed in D16677.

Test Plan: Created a merchant account with D16750 applied.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16764
2016-10-28 14:54:13 -07:00
epriestley
86a00ee4ab Make Calendar ICS imports sort of work in a crude, approximate way
Summary: Ref T10747. This barely works, but can technically import some event data.

Test Plan: Used import flow to import a ".ics" document.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16699
2016-10-12 15:29:05 -07:00
epriestley
e5256bd815 Hide time controls when editing all-day Calendar events
Summary:
Ref T11326. When an event is all-day, hide the time controls for the start/end dates. These aren't used and aren't helpful/useful.

This got a little more complicated than it used to be because EditEngine forms may have only some of these controls present.

Test Plan: Edited an all-day event; edited a normal event; swapped an event between normal and all-day.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11326

Differential Revision: https://secure.phabricator.com/D16327
2016-07-27 05:01:19 -07:00
epriestley
a46a4362db Smooth over a few more transaction compatibility/structure issues with Calendar events
Summary: Ref T9275. This gets things roughly into shape for a cutover to EditEngine, mostly by fixing some problems with "recurrence end date" not being nullable while editing events.

Test Plan: Edited events with EditPro controller, nothing was obviously broken.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9275

Differential Revision: https://secure.phabricator.com/D16282
2016-07-13 07:44:15 -07:00
epriestley
bac6acb3d1 Make EditEngine form for Calendar Events almost fully-functional
Summary:
Ref T9275. This still has a number of rough edges and other minor problems (no JS on the controls, some date handling control bugs) but I'll smooth those over in future changes.

It does make all the editable transaction types available from EditEngine, technically speaking.

Test Plan: Created and edited events with the "pro" controller, which mostly worked.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9275

Differential Revision: https://secure.phabricator.com/D16281
2016-07-13 07:43:38 -07:00
epriestley
54409e7716 Fix an issue with TextAreaEditField affecting Paste
Summary: Fixes T10952. Fixes T10930. I didn't implement this method correctly when I expanded this field for repositories.

Test Plan: Edited a paste without warnings.

Reviewers: avivey, chad

Reviewed By: chad

Maniphest Tasks: T10930, T10952

Differential Revision: https://secure.phabricator.com/D15892
2016-05-11 15:35:17 -07:00
epriestley
98b202042e Provide some more context hints for repository URIs
Summary: Ref T10923. This provides a little guidance about hosted vs observed, and points at the `diffusion.ssh-*` options.

Test Plan: Poked around in the web UI, saw useful guidance.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10923

Differential Revision: https://secure.phabricator.com/D15872
2016-05-10 05:14:29 -07:00
epriestley
311de580d6 Port "Actions" to new Repository UI
Summary: Ref T10748. This brings the "Actions" items (publish/notify + autoclose enabled) into the new UI.

Test Plan:
  - Edited this stuff via EditEngine and Conduit.
  - Viewed via new Manage UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10748

Differential Revision: https://secure.phabricator.com/D15811
2016-04-27 17:35:36 -07:00
epriestley
4c66a92f92 Port Repository "Branches" to new UI
Summary: Ref T10748. Makes a "Branches" panel, enables these transactions in the EditEngine.

Test Plan:
  - Edited via EditEngine + Conduit.
  - Viewed via manage UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10748

Differential Revision: https://secure.phabricator.com/D15809
2016-04-27 17:35:19 -07:00
epriestley
63bbe6b129 Port "Allow Dangerous Changes" to new Manage UI
Summary: Ref T10748. Brings this forward in the UI and EditEngine.

Test Plan:
  - Edited via Conduit.
  - Viewed via Manage UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10748

Differential Revision: https://secure.phabricator.com/D15805
2016-04-27 03:58:10 -07:00
epriestley
cdec319143 Convert Countdown to EditEngine
Summary: Fixes T10684. Fixes T10520. This primarily implements a date/epoch field, and then does a bunch of standard plumbing.

Test Plan:
  - Created countdowns.
  - Edited countdowns.
  - Used HTTP prefilling.
  - Created a countdown ending on "Christmas Morning", etc.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10520, T10684

Differential Revision: https://secure.phabricator.com/D15655
2016-04-07 12:34:07 -07:00
epriestley
4d32c990ab Allow stacked comment actions to be explicitly ordered
Summary:
Ref T6027. Normally, actions use the same order as the form, but in some cases (like moving stuff on workboards) it makes sense to reorder them explicitly.

Pin "Move on board" near the bottom, and "projects/subscribers" at the bottom. I think these are generally reasonable rules in all cases.

Test Plan: Opened menu, saw slightly better action order.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6027

Differential Revision: https://secure.phabricator.com/D15639
2016-04-06 09:15:27 -07:00
epriestley
67629aab14 Implement a rough optgroup-based "Move on Workboard" stacked action
Summary:
Ref T6027. Try this out and see how it feels? Clear issues:

  - This definitely shouldn't be at the top.
  - You should probably be able to select it multiple times?
  - Some of the "which columns show up" rules might need adjustment?
  - Diamond marker maybe not great?

Not sure I love this but it doesn't feel //terrible//...

Test Plan: {F1207891}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6027

Differential Revision: https://secure.phabricator.com/D15638
2016-04-06 09:15:12 -07:00
epriestley
ecd4dd4e0b Expose column positions via maniphest.edit
Summary: Ref T5214. Fixes T10486. Ref T6027. This exposes the `TYPE_COLUMNS` transaction in a usable way via API, and fixes the interactions via prefilling.

Test Plan:
  - Created tasks directly into columns via API.
  - Moved tasks between columns via API.
  - Used `?column=...` to try to create a template task with valid and bogus column PHIDs.

Reviewers: chad

Reviewed By: chad

Subscribers: AmyLewis

Maniphest Tasks: T5214, T6027, T10486

Differential Revision: https://secure.phabricator.com/D15636
2016-04-06 09:14:14 -07:00
epriestley
1bdf988556 Convert DrydockBlueprints to EditEngine
Summary:
Ref T10457. Fixes T10024. This primarily just modernizes blueprints to use EditEngine.

This also fixes T10024, which was an issue with stored properties not being flagged correctly.

Also slightly improves typeaheads for blueprints (more information, disabled state).

Test Plan:
  - Created and edited various types of blueprints.
  - Set and removed limits.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10024, T10457

Differential Revision: https://secure.phabricator.com/D15390
2016-03-03 15:21:25 -08:00
epriestley
f84130f9cd Support enabling a formal points field in Maniphest
Summary:
Ref T4427.

  - New config option for labels, enabling, etc., but no UI/niceness yet.
  - When enabled, add a field.
  - Allow nonnegative values, including fractional values.
  - EditEngine is nice and Conduit / actions basically just work with a tiny bit of extra support code.

Test Plan:
  - Edited points via "Edit".
  - Edited points via Conduit.
  - Edited points via stacked actions.
  - Tried to set "zebra" points.
  - Tried to set -1 points.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4427

Differential Revision: https://secure.phabricator.com/D15220
2016-02-08 18:14:44 -08:00
epriestley
9ae48d4026 Fix smushing of multiple values in Projects "Additional Hashtags" field
Summary: Ref T10168. When we render this control, we currently don't put commas into the value correctly if there are multiple alternative hashtags.

Test Plan: Edited a project with multiple alternate hashtags. Before change: they all got smushed together. After change: properly comma-separated.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10168

Differential Revision: https://secure.phabricator.com/D15045
2016-01-18 08:34:13 -08:00
epriestley
f24318f308 Make "profile menu" configuration mostly work
Summary:
Ref T10054. This does a big chunk of the legwork to let users reconfigure profile menus (currently, just project menus).

This includes:

  - Editing builtin items (e.g., you can rename the default items).
  - Creating new items (for now, only links are available).

This does not yet include:

  - Hiding items.
  - Reordering items.
  - Lots of fancy types of items (dashboards, etc).
  - Any UI changes.
  - Documentation (does feature: TODO link for documentation).

Test Plan:
{F1060695}

{F1060696}

{F1060697}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10054

Differential Revision: https://secure.phabricator.com/D15010
2016-01-13 11:45:31 -08:00
epriestley
add8333b98 Improve behavior of "owner" transaction in "maniphest.edit" endpoint
Summary:
Fixes T10117.

  - I accidentally broke setting `null` to unassign tasks at some point when I added richer validation.
  - Raise a better error if the user passes junk.

Test Plan:
  - Unassigned a task via API and web UI.
  - Reassigned a task via API and web UI.
  - Tried to do an invalid assign via API, got a sensible error.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10117

Differential Revision: https://secure.phabricator.com/D14992
2016-01-11 09:19:18 -08:00
epriestley
e0a97c88db Provide phame.post.edit Conduit API method
Summary:
Ref T9897. This one is a little more involved because of how getting a post on a blog works.

I also changed moving posts to be a real transaction (which shows up in history, now).

Test Plan: Created posts from web UI and conduit.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9897

Differential Revision: https://secure.phabricator.com/D14902
2015-12-28 06:55:35 -08:00
epriestley
6fe882e50a Convert projects to EditEngine
Summary: Ref T10010. This is pretty straightforward with a couple of very minor new behaviors, like the icon selector edit field.

Test Plan:
  - Created projects.
  - Edited projects.
  - Saw "Create Project" in quick create menu.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

Differential Revision: https://secure.phabricator.com/D14896
2015-12-27 15:42:50 -08:00
epriestley
aa2089ba68 Support field previews in EditEngine
Summary: Ref T10004. This primarily supports moving Phame to EditEngine.

Test Plan: {F1045166}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10004

Differential Revision: https://secure.phabricator.com/D14887
2015-12-27 08:17:18 -08:00
epriestley
ed43b31cb1 Prevent "Spaces" field from being set to inconsistent values
Summary:
At least for now, the "Space" field is just a subfield of the "Visible To" field, so:

  - it doesn't get any separate settings; and
  - it always uses the "Visible To" settings.

Test Plan:
  - Created a form with a hidden view policy field.
  - Created stuff with no "you must pick a space" errors.
  - Created stuff with a normal form.
  - Prefilled "Space" on a noraml form.
  - Verified that trying to prefill "Space" on a form with "Visible To" hidden does nothing.

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D14812
2015-12-17 11:22:32 -08:00
epriestley
6146aefcd4 Show fewer useless transactions when creating objects, especially with EditEngine forms
Summary:
Fixes T7661. Ref T9527.

When you create a task, especially with an EditEngine form, you currently get more noise than is useful. For example:

> alice created this task.
> alice changed the edit policy from "All Users" to "Community (Project)".
> alice added projects: Feature Request, Differential.
> alice added a subscriber: alice.

Transaction (1) is a little useful, since it saves us from a weird empty state and shows the object creation time.

Transaction (2) is totally useless (and even misleading) because that's the default policy for the form.

Transaction (3) isn't //completely// useless but isn't very interesting, and probably not worth the real-estate.

Transaction (4) is totally useless.

(These transactions are uniquely useless when creating objects -- when editing them later, they're fine.)

This adds two new rules to hide transactions:

  - Hide transactions from object creation if the old value is empty (e.g., set title, set projects, set subscribers).
  - Hide transactions from object creation if the old value is the same as the form default value (e.g., set policy to default, set priorities to default, set status to default).

NOTE: These rules also hide the "created this object" transaction, since it's really one of those transaction types in all cases. I want to keep that around in the long term, but just have it be a separate `TYPE_CREATE` action -- currently, it is this weird, inconsistent action where we pick some required field (like title) and special-case the rendering if the old value is `null`. So fixing that is a bit more involved. For now, I'm just dropping these transactions completely, but intend to restore them later.

Test Plan:
  - Created objects.
  - Usually saw no extra create transactions.
  - Saw extra create transactions when making an important change away from form defaults (e.g., overriding form policy).

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7661, T9527

Differential Revision: https://secure.phabricator.com/D14810
2015-12-17 10:45:01 -08:00
epriestley
161ebad56d Improve Conduit type handling for *.edit endpoints
Summary:
Ref T9964. Three goals here:

  - Make it easier to supply Conduit documentation.
  - Make automatic documentation for `*.edit` endpoints more complete, particularly for custom fields.
  - Allow type resolution via Conduit types, so you can pass `["alincoln"]` to "subscribers" instead of needing to use PHIDs.

Test Plan:
  - Viewed and used all search and edit endpoints, including custom fields.
  - Used parameter type resolution to set subscribers to user "dog" instead of "PHID-USER-whatever".
  - Viewed HTTP parameter documentation.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9964

Differential Revision: https://secure.phabricator.com/D14796
2015-12-16 08:45:46 -08:00
epriestley
1d72c97fc9 Fix overzealous subscribing in EditEngine
Summary:
See T9905#148799. The CommentEditField generated empty comment transactions; these are dropped later, but before they are dropped they would trigger implicit CCs.

The implicit CC rule should probably be narrower, but we shouldn't be generating these transactions in the first place.

Test Plan: No longer implicitly CC'd on a task when doing something minor like changing projects.

Reviewers: chad

Reviewed By: chad

Subscribers: avivey

Differential Revision: https://secure.phabricator.com/D14795
2015-12-15 16:17:26 -08:00
epriestley
4b3dcd5500 Add some documentation about how to set paths with owners.edit
Summary:
Ref T9964.

  - New mechanism for rich documentation on unusual/complicated edits.
  - Add some docs to `paths.set` since it's not self-evident what you're supposed to pass in.

Test Plan: {F1027177}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9964

Differential Revision: https://secure.phabricator.com/D14791
2015-12-15 15:04:16 -08:00
epriestley
b0a5eee238 Support editing statuses and paths in Owners via Conduit API
Summary: Ref T9964. Fixes T9752. Provides API access to enable/disable packages and change their paths.

Test Plan:
  - Changed status via Conduit.
  - Changed paths via Conduit.
  - Tried to change a path use a nonsense/bogus repository PHID, got an error.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9752, T9964

Differential Revision: https://secure.phabricator.com/D14790
2015-12-15 15:04:00 -08:00
epriestley
d7693a93b3 Provide "Change Projects" and "Change Subscribers" (instead of "Add ...") in comment actions
Summary:
Ref T9908. Fixes T6205.

This is largely some refactoring to improve the code. The new structure is:

  - Each EditField has zero or one "submit" (normal edit form) controls.
  - Each EditField has zero or one "comment" (stacked actions) controls.
    - If we want more than one in the future, we'd just add two fields.
  - Each EditField can have multiple EditTypes which provide Conduit transactions.
  - EditTypes are now lower-level and less involved on the Submit/Comment pathways.

Test Plan:
  - Added and removed projects and subscribers.
  - Changed task statuses.
  - In two windows: added some subscribers in one, removed different ones in the other. The changes did not conflict.
  - Applied changes via Conduit.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T6205, T9908

Differential Revision: https://secure.phabricator.com/D14789
2015-12-15 15:03:34 -08:00
epriestley
a8b402aa14 Allow pastes to be activated/archived via Conduit
Summary: Ref T9964. Add a `setIsConduitOnly()` method so we can mark a field as API-only.

Test Plan:
  - Created and edited pastes via web UI (no status field).
  - Adjusted status via web UI action.
  - Adjusted status via Conduit API.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9964

Differential Revision: https://secure.phabricator.com/D14788
2015-12-15 06:46:05 -08:00
epriestley
2160c45619 Implement an "Attachments" behavior for Conduit Search APIs
Summary:
Ref T9964. We have various kinds of secondary data on objects (like subscribers, projects, paste content, Owners paths, file attachments, etc) which is somewhat slow, or somewhat large, or both.

Some approaches to handling this in the API include:

  - Always return all of it (very easy, but slow).
  - Require users to make separate API calls to get each piece of data (very simple, but inefficient and really cumbersome to use).
  - Implement a hierarchical query language like GraphQL (powerful, but very complex).
  - Kind of mix-and-match a half-power query language and some extra calls? (fairly simple, not too terrible?)

We currently mix-and-match internally, with `->needStuff(true)`. This is not a general-purpose, full-power graph query language like GraphQL, and it occasionally does limit us.

For example, there is no way to do this sort of thing:

  $conpherence_thread_query = id(new ConpherenceThreadQuery())
    ->setViewer($viewer)
    // ...
    ->setNeedMessages(true)
    ->setWhenYouLoadTheMessagesTheyNeedProfilePictures(true);

However, we almost never actually need to do this and when we do want to do it we usually don't //really// want to do it, so I don't think this is a major limit to the practical power of the system for the kinds of things we really want to do with it.

Put another way, we have a lot of 1-level hierarchical queries (get pictures or repositories or projects or files or content for these objects) but few-to-no 2+ level queries (get files for these objects, then get all the projects for those files).

So even though 1-level hierarchies are not a beautiful, general-purpose, fully-abstract system, they've worked well so far in practice and I'm comfortable moving forward with them in the API.

If we do need N-level queries in the future, there is no technical reason we can't put GraphQL (or something similar) on top of this eventually, and this would represent a solid step toward that. However, I suspect we'll never need them.

Upshot: I'm pretty happy with "->needX()" for all practical purposes, so this is just adding a way to say "->needX()" to the API.

Specifically, you say:

```
{
  "attachments": {
    "subscribers": true,
  }
}
```

...and get back subscriber data. In the future (or for certain attachments), `true` might become a dictionary of extra parameters, if necessary, and could do so without breaking the API.

Test Plan:
- Ran queries to get attachments.

{F1025449}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9964

Differential Revision: https://secure.phabricator.com/D14772
2015-12-14 11:53:00 -08:00
epriestley
eef2572508 Replace workboard task creation with EditEngine
Summary: Ref T9908. This is the last of the things that need to swap over.

Test Plan:
  - Created tasks from a workboard.
  - Created tasks in different columns.
  - Edited tasks.
  - Used `?parent=..`.
  - Verified that default edit form config now affects comment actions.
  - No more weird comment thing on forms, at least for now.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9908

Differential Revision: https://secure.phabricator.com/D14715
2015-12-08 17:56:11 -08:00
epriestley
d53187e10a Make "Create Subtask" work properly in EditEngine
Summary: Ref T9908. This fixes "Create Subtask" so it works with the new stuff. Mostly straightforward.

Test Plan: Created some subtasks.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9908

Differential Revision: https://secure.phabricator.com/D14706
2015-12-08 14:29:58 -08:00
epriestley
468f785845 Support "template objects" generically in EditEngine
Summary:
Ref T9132. Ref T9908. Fixes T5622. This allows you to copy some fields (projects, subscribers, custom fields, some per-application) from another object when creating a new object by passing the `?template=xyz` parameter.

Extend "copy" support to work with all custom fields.

Test Plan:
  - Created new pastes, packages, tasks using `?template=...`
  - Viewed new template docs page.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T5622, T9132, T9908

Differential Revision: https://secure.phabricator.com/D14699
2015-12-07 13:44:07 -08:00
epriestley
273e22d59f Save stacked actions in drafts, not just comments
Summary:
Ref T9132. Fixes T4580. Thhat might actually have been fixed a while ago or something since it describes a buggy/bad interaction which doesn't reproduce for me at HEAD.

This saves and restores all the stacked actions (subscribers, projects, etc) so that you don't lose anything if you close a window by accident.

Test Plan:
Added a bunch of actions in various states, reloaded the page, draft stuck around.

Submitted form, actions didn't stick around anymore.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T4580, T9132

Differential Revision: https://secure.phabricator.com/D14675
2015-12-04 16:29:43 -08:00
epriestley
eded19a5c6 Unify EditEngine preview behavior; prepare for saving complex drafts
Summary:
Ref T9132. We currently have an old preview/draft behavior and a new actions behavior.

Let the actions behavior do drafts/previews too, so we can eventually throw away the old thing.

This is pretty much just copying the old behavior into the new one, but with a few tweaks. The major change is that we submit all the stacked actions behavior now, so the preview reflects everything the change will do (and, soon, we can save it in the draft in a consistent way).

Also includes one hack-fix that I'll clean up at some point.

Test Plan: Added a bunch of stacked actions and observed meaningful previews.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14672
2015-12-04 16:29:40 -08:00
epriestley
f9e84d1a88 Make "Assign / Claim" stacked action work properly in Maniphest
Summary:
Ref T9132. This is kind of a mess because the tokenizer rewrite left rendering tokenizers in Javascript a little rough. This causes bugs like icons not showing up on tokens in the "Policy" dialog, which there's a task for somewhere I think.

I think I've fixed it enough that the beahavior is now correct (i.e., icons show up properly), but some of the code is a bit iffy. I'll eventually clean this up properly, but it's fairly well contained for now.

Test Plan:
  - Reassigned a task.
  - Put a task up for grabs.
  - No reassign on closed tasks.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14669
2015-12-04 16:29:35 -08:00
epriestley
92ea07e787 Restore "Change Status" and "Change Priority" comment actions to Maniphest
Summary: Ref T9132. Supports selects in stacked actions and adds "Change Status" + "Change Priority".

Test Plan: Changed status and priority from stacked actions.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14667
2015-12-04 16:29:33 -08:00
epriestley
b3cf00333c Limit number of EditEngine tokenizer tokens in "Owner" field UI to 1
Summary:
Ref T9132. Only allow a task to have a single owner in the UI.

In Conduit, make this field appear and behave as "phid" instead of "list<phid>".

Test Plan: Edited a task with new fancy form, got limited to one owner. Assigned/unassigned. Used Conduit to assign/unassign.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14666
2015-12-04 16:29:31 -08:00
epriestley
dc0d914134 Basic stacked action support for EditEngine
Summary: Ref T9132. This still has a lot of rough edges but the basics seem to work OK.

Test Plan: {F1012627}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14653
2015-12-03 12:32:02 -08:00
epriestley
c1ae5321d7 Support HTTP parameter prefilling in EditEngine forms for CustomFields
Summary:
Ref T9132. This allows you to prefill custom fields with `?custom.x.y=value`, for most types of custom fields.

Dates (which are substantially more complicated) aren't supported. I'll just do those once the dust settles. Other types should work, I think.

Test Plan:
  - Verified custom fields appear on "HTTP Parameters" help UI.
  - Used `?x=y` to prefill custom fields on edit form.
  - Performed various normal edits.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14634
2015-12-02 09:32:26 -08:00
epriestley
029b1b6733 Partially support CustomFields in EditEngine
Summary:
Ref T9132. This isn't perfect, but doesn't break any existing functionality. This stuff works:

  - Editing values.
  - Reordering fields.
  - All builtin field tyepes.

This stuff may not work yet:

  - Assigning custom field defaults.
  - Some conduit stuff.
  - Fully custom fields?
  - Locking/hiding fields? Didn't actually test this one.

I'll keep chipping away at that stuff. In some cases, it may be easier to convert all the CustomField apps first, although Differential might be a fair bit of work.

Test Plan:
Created a bunch of custom fields of every avialable type and edited them.

{F1008789}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14617
2015-12-02 05:21:31 -08:00
epriestley
9d59086d01 Consolidate transaction generation in EditType objects
Summary:
Ref T9132. This is a bit more cleanup to make adding CustomField support easier.

Right now, both `EditField` and `EditType` can actually generate a transaction. This doesn't matter too much in practice today, but gets a little more complicated a couple of diffs from now with CustomField stuff.

Instead, always use `EditType` to generate the transaction. In the future, this should give us less total code and make more things work cleanly by default.

Test Plan: Used web UI and Conduit to make various edits to pastes, including doing race-condition tests on "Projects".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9132

Differential Revision: https://secure.phabricator.com/D14607
2015-11-30 09:01:00 -08:00