Summary:
Ref T10004. After a user logs in, we send them to the "next" URI cookie if there is one, but currently don't always do a very good job of selecting a "next" URI, especially if they tried to do something with a dialog before being asked to log in.
In particular, if a logged-out user clicks an action like "Edit Blocking Tasks" on a Maniphest task, the default behavior is to send them to the standalone page for that dialog after they log in. This can be pretty confusing.
See T2691 and D6416 for earlier efforts here. At that time, we added a mechanism to //manually// override the default behavior, and fixed the most common links. This worked, but I'd like to fix the //default// beahvior so we don't need to remember to `setObjectURI()` correctly all over the place.
ApplicationEditor has also introduced new cases which are more difficult to get right. While we could get them right by using the override and being careful about things, this also motivates fixing the default behavior.
Finally, we have better tools for fixing the default behavior now than we did in 2013.
Instead of using manual overrides, have JS include an "X-Phabricator-Via" header in Ajax requests. This is basically like a referrer header, and will contain the page the user's browser is on.
In essentially every case, this should be a very good place (and often the best place) to send them after login. For all pages currently using `setObjectURI()`, it should produce the same behavior by default.
I'll remove the `setObjectURI()` mechanism in the next diff.
Test Plan: Clicked various workflow actions while logged out, saw "next" get set to a reasonable value, was redirected to a sensible, non-confusing page after login (the page with whatever button I clicked on it).
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10004
Differential Revision: https://secure.phabricator.com/D14804
Summary:
Fixes T6864. This creates a sort of busy menu but I think that's proably fine -- users are opting into activating these fields for search anyway.
In the future, we could refine this as, e.g.:
- don't show these options in the dropdown;
- do show them on some new "http prefilling" sort of page;
- then you access them as an advanced user with `?order=secret-magic`.
But I'm not going to bother for now.
Test Plan: Ordered by an int field, then reversed the order.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T6864
Differential Revision: https://secure.phabricator.com/D14800
Summary: This seems to work, but I couldn't figure out how to pass over a Caption for a text field.
Test Plan: New blog, Edit blog.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14770
Summary:
Ref T9992. This is a step on the path to getting EditEngine working in Badges, Projects and Calendar.
This doesn't add a new `EditField` for icons yet, just standardizes the old stuff. New stuff is more general and I saved 150 lines of code.
I put the endpoint in Files because the similar "choose a profile picture" endpoint will definitely go there, and this endpoint might eventually feature, like, "draw your own icon~~" or something.
Test Plan:
- Created events, projects and badges with custom icons.
- Edited events, projects and badges, changing their icons.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9992
Differential Revision: https://secure.phabricator.com/D14799
Summary: Ref T9964. Create some docuemntation for this stuff, and clean up the *.edit endpoints a bit.
Test Plan: Read documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14798
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
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
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
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
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
Summary:
Fixes T9997. This was in the database since v0, I just never hooked up the UI since it wasn't previously meaningful.
However, it now makes sense to have a provider like Asana with login disabled and use it only for integrations.
Test Plan: Disabled login on a provider, verified it was no longer available for login/registration but still linkable.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9997
Differential Revision: https://secure.phabricator.com/D14794
Summary:
Fixes T9995. I think letting users customize slugs is not a hugely compelling as a product feature, and this fixes the issue with slugs that have "/" characters in them and makes the move to EditEngine easier since I don't have to deal with the weird JS thing.
Instead, just generate slugs automatically. No more JS, no more separate field, things automatically update if you rename a blog, and now that URIs have IDs in them the old URI will still work after a rename.
Test Plan:
- Applied migration.
- Created new posts.
- Edited existing posts.
- Visited various posts.
- Created a post with a bunch of "/" in the title, things still worked fine.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9995
Differential Revision: https://secure.phabricator.com/D14792
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
Summary: Fixes T9988. This logic got inverted by accident at some point.
Test Plan:
- Edited a task, shifting spaces.
- Created a task in default space.
- Created a task in custom space.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9988
Differential Revision: https://secure.phabricator.com/D14787
Summary: Ref T9983. This method is spelled wrong.
Test Plan: Hit this case, got a dialog instead of a fatal.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9983
Differential Revision: https://secure.phabricator.com/D14786
Summary:
Fixes T9984. When a tokenizer only allows one selection (like "Task Owner:" or "Land Onto Branch:"), keep the browse button active but have it //replace// values.
Also, have "Create Subtask" default to the system default status, so subtasks of closed tasks are not also closed.
Test Plan:
- Browsed an empty limit=1 tokenizer.
- Replaced a full limit=1 tokenizer.
- Browsed an empty no-limit tokenizer.
- Browsed more tokens into the no-limit tokenizer.
- Typed some tokens normally.
- Created a subtask of a closed task.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9984
Differential Revision: https://secure.phabricator.com/D14785
Summary:
Ref T9980. No magic here, just write a little bit about how to find outdated callers. Update the technical doc.
Also:
- Fix an unrelated bug where you couldn't leave comments if an object had missing, required, custom fields.
- Restore the ConduitConnectionLog table so `bin/storage adjust` doesn't complain.
Test Plan: Read docs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9980
Differential Revision: https://secure.phabricator.com/D14784
Summary:
Ref T9908. These meta-edit-engines are used to generate the main editengine UIs, but they're also editable.
Fix an exception when trying to edit the meta editengine.
Test Plan: Edited editengineconfiguration editengine.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14783
Summary:
Ref T9980. By default, show the viewer //their// calls.
Make it easy to find their own deprecated calls.
I don't like the word "My" but couldn't come up with anything better that didn't feel like a big loss of clarity.
The permissions on this log are also a little weird: non-admins can see everyone else's calls.
I think we should eventually lock that down, but plan to keep it this way for now:
First, a lot of your calls end up with no caller set right now, because we don't set the caller early enough in the process so a lot differnet types of errors can leave us with no user on the log. Fixing that isn't trivial, and users may reasonably want to access to these "no caller" logs to check for errors or debug stuff.
Second, none of it is really that sensitive?
Third, it's reasonable for users to want to look at bots?
I'd plan to maybe do this eventually:
- Make the caller get populated more often after auth code is simplified.
- Only let users look at their calls and maybe bot calls and anonymous calls.
- Let admins look at everything.
But for now everyone can see everything.
Test Plan: {F1025867}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9980
Differential Revision: https://secure.phabricator.com/D14782
Summary: Ref T9980. This makes it much easier to look for calls to deprecated methods.
Test Plan: {F1025851}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9980
Differential Revision: https://secure.phabricator.com/D14781
Summary:
Ref T5955, T9980, T9982.
We currently store two types of Conduit logs: //connection// logs and //method// logs.
Originally, Conduit worked like web logins: you'd call `conduit.connect` and then get a session back. This approach still works, but new clients don't use it and it will probably stop working eventually after T5955 is further along.
There was no real reason for things to work like this and no other API in the world does, I think it was just slightly easier to implement back in 2011.
This table was used to group up related calls in a UI long ago, I think, but that got deleted at some point. In any case, it serves no purpose in modern Phabricator.
Test Plan: `grep`
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5955, T9980, T9982
Differential Revision: https://secure.phabricator.com/D14780
Summary:
Ref T9980. Start making this UI more useful and powerful so we can give administrators a better toolset for reacting to API changes.
Fixes T9755. We were logging the caller, just not rendering it properly.
Test Plan: {F1025799}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9755, T9980
Differential Revision: https://secure.phabricator.com/D14779
Summary: Ref T9980. I don't think this is actually useful, and plan to give users and administrators more powerful tools instead.
Test Plan: Loaded setup warnings.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9980
Differential Revision: https://secure.phabricator.com/D14778
Summary: Ref T9964. Priorities and statuses have metadata (colors, names, etc) which we can reasonably just return from API callers. This is so ligthweight that I think it doesn't really make sense to put in an attachment. We also use this information in `arc tasks`.
Test Plan: Called API, got sensible looking status and priority data back.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14777
Summary: Ref T9964. This just adds more structure to application fields, to make it harder to make typos and easier to validate them later.
Test Plan: Viewed APIs, called some APIs, saw good documentation and correct results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14776
Summary:
Ref T9964.
- Add a "paths" attachment for fetching paths.
- Always load owners. We will need this to do policy checks in the future, anyway, and this data is not large, is very useful, and is reasonable to load unconditionally.
Test Plan:
- Queried packages via API.
- Edited packages (paths, owners).
- Created a package.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14775
Summary: Ref T9964. Builds on D14772. Allows callers to get the raw content of pastes as an attachment.
Test Plan:
- Read docs.
- Executed attachment query.
- Saw raw paste content.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14774
Summary: Ref T9964. Builds on D14772. Allows callers to request project PHIDs for objects.
Test Plan: {F1025468}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14773
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
Summary:
Ref T9964. I added several hacks to get these working. Clean them up and pull this into a proper extension.
The behavior in the web UI is:
- they work in all applications; but
- they only show up in the UI if a value is specified.
So if you visit `/view/?ids=1,2` you get the field, but normally it's not present. We could refine this later. I'm going to add documentation about how to prefill these forms regardless, which should make this discoverable by reading the documentation.
There's one teensey weensey hack: in the API, I push these fields to the top of the table. That one feels OK, since it's purely a convenience/display adjustment.
Test Plan: Queried by IDs, reviewed docs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14769
Summary:
Ref T9964. Building tables in Remarkup is kind of neat-ish but ends up feeling kind of hacky, and requires weird workarounds if any of the values have `|` in them.
Switch to normal elements instead.
Also move the magic "ids" and "phids" to be more like real fields. I'll clean this up fully in a diff or two, it's just a little tricky because Maniphest has an "ids" field.
Test Plan: {F1024294}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14768
Summary: Ref T9964. I left a couple of these unsupported for now since they're weird in some way.
Test Plan: {F1024031}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14767
Summary:
Ref T9964. Fill in more parameter types and descriptions.
(No date support yet since it's a bit more involved.)
Test Plan: {F1024022}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14766
Summary: Ref T9964. This fills in types and descriptions for ApplicationSearch fields in Paste.
Test Plan:
Got this nice table now:
{F1023999}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14765
Summary:
Ref T9964. ApplicationSearch currently has a bunch of hard-coded `if ($object instanceof thing)` stuff.
Pull that out so it can live in extensions.
Test Plan:
- Searched by spaces, subscribers, projects.
{F1023921}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14764
Summary:
Ref T9964. I want to show users what we're expecting in "constraints", and let constraints like "authors=epriestley" work to make things easier.
I'm generally very happy with the "HTTPParameterType" stuff from EditEngine, so add a parallel set of "ConduitParameterType" classes. These are a little simpler than the HTTP ones, but have a little more validation logic.
Test Plan:
This is really just a proof of concept; some of these fields are now filled in:
{F1023845}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14763
Summary:
Ref T9964. The new `*.search` and `*.edit` methods generate documentation which depends on the viewer.
For example, the `*.search` methods show a reference table of the keys for all your saved queries.
Give them a real viewer to work with.
During normal execution, just populate this viewer with the request's viewer, so `$request->getViewer()` and `$this->getViewer()` both work and mean the same thing.
Test Plan: {F1023780}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14761
Summary: Ref T9964. This is a basic implementation of the new "maniphest.search" endpoint.
Test Plan: Clicked the button in the web UI, got meaningful results back.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14760
Summary:
Ref T9964. Adds a new-style "owners.search" endpoint, and an extension for customfields.
Puts enough indirection in place to give us nice, consistent "custom.key" user-facing keys instead of "std:custom:owners:na0shf9a8dfdsafl" junk.
Test Plan:
- Searched Owners via API.
- Searched by ID.
- Ordered by custom fields.
- Reviewed API docs.
- Used normal search with ordering.
- Viewed custom field values in search results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9964
Differential Revision: https://secure.phabricator.com/D14758
Summary: Adds a list of your drafts. Fixes T9927y
Test Plan:
Load up home, see my drafts. Fake 0 drafts, see fallback message.
{F1023139}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14756
Summary: Better color, space for Blog List.
Test Plan: Review PhameHome at mobile, tablet and desktop breakpoints.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D14749
Summary:
Ref T9360. Fixes the double-rendering of post bodies in feed stories.
Downside is that 'publish' (on its own) no longer shows a body, but that seems fine.
Test Plan:
- Got some double bodies.
- Applied patch.
- No more double bodies.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9360
Differential Revision: https://secure.phabricator.com/D14748
Summary: Ref T9968. Some of the crumb/route handling wasn't quite tight enough and could hit a fatal.
Test Plan: Hit previously-fataling URI, got a 404 instead.
Reviewers: chad
Reviewed By: chad
Subscribers: starruler
Maniphest Tasks: T9968
Differential Revision: https://secure.phabricator.com/D14747
Summary: Ref T9927. Adds a "Blogs" section to PhameHome. Removes "New Post" Controller. Adds flipped layout for PHUITwoColumnView
Test Plan:
Test PhameHome, Ponder, New Post, etc. Mobile and Desktop states.
{F1022080}
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: johnny-bit, Korvin
Maniphest Tasks: T9927
Differential Revision: https://secure.phabricator.com/D14744
Summary: Fixes T9966. In this unusual, difficult-to-reach case, we throw `Exception` (which has no censoring) instead of `CommandException` (which has censoring). Throw `CommandException` instead.
Test Plan:
- Hacked up a bunch of stuff in order to hit this: disabled origin validation, origin correction, and pointed repository at a bad domain.
- Verified message is now censored correctly.
{F1022217}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9966
Differential Revision: https://secure.phabricator.com/D14745
Summary:
Ref T9964. See that task for some context and discussion.
Ref T7715, which has the bigger picture here.
Basically, I want Conduit read endpoints to be full-power, ApplicationSearch-driven endpoints, so that applications can:
- Write one EditEngine and get web + conduit writes for free.
- Write one SearchEngine and get web + conduit reads for free.
I previously made some steps toward this, but this puts more of the structure in place.
Test Plan:
Viewed API console endpoint and read 20 pages of docs:
{F1021961}
Made various calls: with query keys, constraints, pagination, and limits.
Viewed new {nav Config > Modules} page.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T7715, T9964
Differential Revision: https://secure.phabricator.com/D14743
Summary: Ref T9897. If you visit `/post/123/spoderman/` it will try to redirect you to `/post/123/spiderman/`, but currently only internal views work because these redirects aren't marked as safe/external.
Test Plan: Visited a misspelled/out-of-date URI on an external blog view, got a good redirect.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9897
Differential Revision: https://secure.phabricator.com/D14741
