Summary: Ref T10054. This is all pretty straightforward. Also include some project-specific examples in the project documentation.
Test Plan: Read documentation.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15090
Summary:
Ref T10054.
- Just let users delete non-builtin items.
- Let users choose a default item explicitly.
- Do a better job of cleaning up items which no longer exist or belong to uninstalled applications.
(NOTE) This has one user-facing change: workboards are no longer the default on projects with workboards. I think this is probably OK since we're giving users a ton of new toys at the same time, but I'll write some docs at least.
Test Plan:
- Deleted custom items.
- Disabled/enabled builtin items.
- Made various things defaults.
- Uninstalled Maniphest, saw Workboards tab disappear entirely.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15089
Summary: `alterClass()` is strict about true/false but we set 0/1 elsewhere.
Test Plan: Collapsed/expanded menu, reloaded expanded menu, clicked collapse, got immediate collapse.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15087
Summary:
Fixes T10196. This is a weird interaction and this might not be the best long-term fix, but just get it working OK for now.
General problem is that Quicksand doesn't currently use GET for requests. This is a very unusual case where the method is relevant. In the future, I might change Quicksand to use GET.
Test Plan: Clicked "Open Tasks" with Quicksand active, got a results list.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10196
Differential Revision: https://secure.phabricator.com/D15082
Summary: Mostly a visual spacing pass, also adds in circle icons for edit, collapse. For now removing the fixed position on the icons for simplicity while the basics are being polished.
Test Plan: Projects, Profiles, wide and narrow.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin
Differential Revision: https://secure.phabricator.com/D15081
Summary:
Ref T10054. I think this gets everything except:
- circles on icons;
- I spent ~15 minutes poking at animations but wasn't able to get anything that looked reasonable whatsoever.
Test Plan:
- Collapsed menus.
- Expanded menus.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15056
Summary: Ref T10054. The daemons look for this but currently can't find it.
Test Plan: Ran daemons, clean exit on profile menu edits instead of permanent failure.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15053
Summary: Motivate your employees with inspirational quotes. A new quote every day!
Test Plan: So inspirational.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D15026
Summary:
Ref T10054. Primary goal is to be able to remove IconNav from the codebase.
I've made these non-editable so users can't customize them yet. We //might// want administrators to customize these globally instead? In any case, we avoid a bunch of product questions by just locking these down for now.
Test Plan: {F1061348}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15020
Summary: Ref T10054. Take specialization off the objects and put it on Engine subclasses instead. One reason for this is that certain objects (like users) might have multiple different sets of panels in the future (e.g., their user profile and their home page).
Test Plan:
- No user-visible changes.
- PanelEngine no longer has any hardcoded "project" stuff.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15018
Summary:
Ref T10054. I haven't done any of the big-picture layout stuff yet, but this should get look-and-feel somewhere in the ballpark of reasonablness, I think.
Major missing stuff:
- No "collapse" state or action yet.
- Menu is not full-height (requires changes to the rendering pipeline).
Test Plan: {F1060941}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15016
Summary:
Ref T10054.
I made this a dropdown (currently: "Visible" or "Disabled") since I imagine we //miiiight// want to add a "Hidden, but click 'More' to reveal" state or do other special stuff in this vein. Not 100% sold on that but seemed within the realm of plausibility.
Test Plan: {F1060759}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15012
Summary: Ref T10054. Allows users to drag menu items to reorder them.
Test Plan: Reordered a project menu.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15011
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
Summary: Ref T10054. This has no product impact, but prepares us for customizable side nav on "profiles" (today, projects; probably users some day; and maybe other stuff down the road).
Test Plan: Clicked all links on a profile, everything was exactly the same as before.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10054
Differential Revision: https://secure.phabricator.com/D15007
Summary:
If you try to pull the hovercard of something you can no longer see (maybe you loaded the page, then the policy changed) there won't be a value in the array here.
(The rest of the code anticipates this possibility.)
Test Plan: Hovered some stuff.
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14987
Summary: Ref T4245. Before doing any hard work here, we can dramatically reduce the number of things that make calls to `getCallsign()` to make navigating things easier. Almost all of them only care about a monogram, URI, or display name.
Test Plan:
- Searched for `r uniquename` in jump nav.
- Ran `bin/repository reparse --change rXXXyyyyy --trace`, observed query against bad commit table.
- Ran `bin/search index rXXXyyyy --trace --force`, observed proper title when indexing commit.
- Browed repository list, saw proper `rXXX` and appropriate link targets.
- Mentioned `rXXX` in Remarkup, got a link to the right place.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T4245
Differential Revision: https://secure.phabricator.com/D14923
Summary: Ref T9897. Adds basic blog query support.
Test Plan: Ran some queries.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9897
Differential Revision: https://secure.phabricator.com/D14900
Summary: Ref T10010. This still needs support for attachments (to get members) and more constraints (like slugs), but mostly works.
Test Plan: Ran query, saw basically sensible results.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10010
Differential Revision: https://secure.phabricator.com/D14889
Summary:
Ref T8980. Move away from events to EngineExtensions.
This also simplifies hovercards a bit:
- Removes tasks from revision cards.
- Removes blockers/blocked from task cards.
- Removes "Send Message" from user cards.
These mostly felt cluttery to me. Open to arguments to retain them. I think we can make better use of the space, though (e.g., flags, projects + board columns).
Test Plan:
- Viewed people, task, revision, commit and project hovercards.
{F1043256}
{F1043257}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T8980
Differential Revision: https://secure.phabricator.com/D14878
Summary:
Ref T9979. This uses ngrams (specifically, trigrams) to build a reasonably efficient index for substring matching. Specifically, for a package like "Example", with ID 123, we store rows like this:
```
< ex, 123>
<exa, 123>
<xam, 123>
<amp, 123>
<mpl, 123>
<ple, 123>
<le , 123>
```
When the user searches for `exam`, we join this table for packages with tokens `exa` and `xam`. MySQL can do this a lot more efficiently than it can process a `LIKE "%exam%"` query against a huge table.
When the user searches for a one-letter or two-letter string, we only search the beginnings of words. This is probably what they want, the only thing we can do quickly, and a reasonable/expected behavior for typeaheads.
Test Plan:
- Ran storage upgrades and search indexer.
- Searched for stuff with "name contains".
- Used typehaead and got sensible results.
- Searched for `aabbccddeeffgghhiijjkkllmmnnooppqqrrssttuuvvwwxxyyzz` and saw only 16 joins.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14846
Summary:
Fixes T9890. This allows IndexExtensions to emit an object version.
Before we build indexes, we check if the indexed version is the same as the current version. If it is, we just don't call that extension.
T9890 has a case where this is useful: a script went crazy and posted thousands of comments to a single task.
Without versioning, that results in the same comments being indexed over and over again. With versioning, most of the queue could just exit without doing any work.
Test Plan:
- Added a `sleep(1)` to the actual indexing, used `bin/search index --background` to queue up a lot of tasks, ran them with `bin/phd debug task`, saw them complete very quickly with only one actual index operation performed.
- Used `bin/search index --trace` and `bin/search index --trace --background` to observe the behavior of queries against the index version store, which looked sensible.
- Made comments/transactions, saw versions update.
- Used `bin/remove destroy`, verified index versions were purged.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9890
Differential Revision: https://secure.phabricator.com/D14845
Summary:
Ref T9979. I picked this name long before the advent of modern "Engine" architecture and it ended up being pretty confusing.
Rename "SearchEngine" (currently: mysql or elasticsearch, used to store and query fulltext indexes) to "FulltextStorageEngine" to make it more clear what it does and disambituate it from ApplicationSearch, which also has a bunch of stuff called "SearchEngine", "SearchEngineExtension", etc.
Test Plan: Grepped for `phabricatorsearchengine`.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14843
Summary: Ref T9979. This simplifies/standardizes the code a bit, but mostly gives us more consistent class names and structure.
Test Plan:
- Used `bin/search index --type ...` to index documents of every indexable type.
- Searched for documents by unique text, found them.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14842
Summary:
Ref T9979. There are currently some hacks around Conpherence indexing: it does not really use the fulltext index, but its own specialized index. However, it's kind of hacked up so it can get reindexed by the normal indexing pipeline.
Lift it up into IndexEngine, instead of FulltextEngine. Specifically, the new stuff is going to look like this:
- IndexEngine: Rebuild all indexes.
- ConpherenceIndexExtension: Rebuild thread indexes.
- ProjectMemberIndexExtension: Rebuild project membership views.
- NgramIndexExtension: Rebuild ngram indexes.
- FulltextIndexExtension / FulltextEngine: Rebuild fulltext indexes, a special type of index.
- FulltextCommentExtension: Rebuild comment fulltext indexes.
- FulltextProjectExtension: Rebuild project fulltext indexes.
- etc.
Most of this is at least sort-of-in-place as of this diff, although some of the part in the middle is still pretty rough.
Test Plan:
- Made a unique comment in a Conpherence thread.
- Used `bin/search index --force` to rebuild the index.
- Searched for the comment.
- Found the thread.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14841
Summary:
Ref T9979. This is currently hard-coded but can be done in a generic way.
This has one minor behavioral changes: answer text is no longer included in the question text index in Ponder. I'm not planning to accommodate that for now since I don't want to dig this hole any deeper than I already have. This behavior should be different anyway (e.g., index the answer, then show the question in the results or something).
Test Plan:
- Put a unique word in a Maniphest comment.
- Searched for the word.
- Found the task.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14837
Summary:
Ref T9979. This event had one weird callsite and no known third-party callers. It can be done more cleanly as an extension, now.
This index is used to allow us to "Group By: Project" in Maniphest without joining into the Projects database.
Test Plan:
- Ran a query with "Group By: Project" in Maniphest.
- Renamed project "Apples" to "Zebras".
- Reloaded page.
- UI properly moved "Zebras" tasks to the bottom of the list.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14836
Summary: Ref T9979. This is going to become `FulltextEngine`, but pave the way for that by pulling extensions out of it.
Test Plan:
{F1036624}
- Used `bin/search index Txxx`, saw projects, subscribers and custom fields rebuild in the index.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14835
Summary:
Ref T9890. Ref T9979. Several adjacent goals:
- The `SearchEngine` vs `ApplicationSearchEngine` thing is really confusing. There are also a bunch of confusing class names and class relationships within the fulltext indexing. I want to rename these classes to be more standard (`IndexEngine`, `IndexEngineExtension`, etc). Rename `SearchIndexer` to `IndexEngine`. A future change will rename `SearchEngine`.
- Add the index locks described in T9890.
- Structure things a little more normally so future diffs can do the "EngineExtension" thing more cleanly.
Test Plan:
Indexing:
- Renamed a task to have a unique word in the title.
- Ran `bin/search index Txxx`.
- Searched for unique word.
- Found task.
Locking:
- Added a `sleep(10)` after the `lock()` call.
- Ran `bin/search index Txxx` in two windows.
- Saw first one lock, sleep 10 seconds, index.
- Saw second one give up temporarily after failing to grab the lock.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9890, T9979
Differential Revision: https://secure.phabricator.com/D14834
Summary:
Ref T9979. The general shape of "engine" code feels pretty good, and I plan to move indexing to be more in line with other modern engines, with the ultimate goal of supporting subprojects (T10010) and several intermediate goals.
Before moving indexing, clean up Destruction, since some of the new indexes will need destruction hooks and destruction currently has a lot of `instanceof` stuff that should be easy to fix by applying more modern approaches.
Test Plan:
- Used `bin/remove destroy` to destory an Almanac device.
- Verified that properties for the device were destroyed.
- Viewed module panel in UI.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9979
Differential Revision: https://secure.phabricator.com/D14831
Summary:
This is just putting a hook in that pretty much works. Behavior:
- If you visit `/maniphest/?nux=true`, it always shows NUX for testing.
- Otherwise, it shows NUX if there are no objects in the application yet.
Test Plan: {F1031846}
Reviewers: chad
Reviewed By: chad
Differential Revision: https://secure.phabricator.com/D14828
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. 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. 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. 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. 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:
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 T9908. No more callsites. Also:
- Phurl a couple of documentation URIs.
- Get rid of "task:" in the global search since it doesn't really make sense anymore.
Test Plan: `grep`, edited/created tasks.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9908
Differential Revision: https://secure.phabricator.com/D14716
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
