Page MenuHomePhabricator

Support query parameterization in typeaheads
Closed, ResolvedPublic

Description

It would be nice if we could create maniphest queries which were parameterized by the viewer's PHID. For example, I might like to have a query which was "All tickets I'm CC'd on", and there seems to be no reason for me to have to hardcode my own name in that query. If instead I could use a variable like $USER, then I could share that query with other people without everybody having to edit it to put their own name in. Additionally, then the built-in "Authored" and "Assigned" saved queries could actually be implemented using the normal saved query stuff rather than being hardcoded in PHP. Double-win!

Details

Differential Revisions
D7586: Add a default "Subscribed" query to Maniphest
Commits
D12532 / rP83617073f5dc: Rename TypehaeadUserParameterizedDatasource to PeopleUserFunctionDatasource
D12533 / rPcf154ae9f499: Support tokenizer functions in Diffusion
D12530 / rPf64ca87bfea0: Implement open() and closed() Maniphest tokenizer status functions, plus cleanup
D12531 / rPe094af461871: Parameterize search in Pholio
D12528 / rPf69f53124ce9: Use typeaheads instead of checkbox lists for task status / priority
D12529 / rPccd6770a098f: Fix an issue where "Browse > Select" did not work for dynamic tokenizers
D12526 / rP22e3e35418be: Move ManiphestTaskQuery to EdgeLogic
D12524 / rP3e9ebb8e20e7: Add a "mailable" function datasource
D12527 / rPfcaf56332fad: Use parameterized datasources for Maniphest authors and subscribers
D12525 / rP7ab025eef559: Use a function typeahead for "projects" in Maniphest
D12523 / rP135c8799e67c: Refine global search UI for tokenizer functions
D12482 / rPbae0a85dbc52: Add contextual typeahead function documentation
Restricted Differential Revision / Restricted Diffusion Commit
D12470 / rP6fa9b10914e6: Collapse "Show unassigned tasks" in Maniphest into a parameterized owners…
D12469 / rPe5e5974d9fe1: Give typeahead browse dialogs sensible titles
D12468 / rPb85f8b91de69: Implement readProjectsFromRequest() helper for SearchEngines
D12460 / rPc2fc065bfb3c: Implement a project logic datasource
D12462 / rPf331463d5072: Make "user's projects" a logical datasource and add viewerprojects()
D12463 / rP4005bff56702: Implement function-driven logical project queries in Differential
D12459 / rPeb69b115a7fb: Simplify members() datasource function
D12458 / rPc246aa26382a: Implement a projects() datasource function
D12455 / rPe27c0b416d45: Add "Edge Logic" support to PolicyAwareQuery
D12456 / rPa11dab59b0d5: Require TokenizerControl to always have a datasource
D12457 / rP2bbe3b0cbfce: Move token rendering into Datasources
D12454 / rP55e49d7e316c: Provide more buildXClause() and buildXClauseParts() on PolicyAwareQuery
D12453 / rPf5580c7a083e: Make buildWhereClause() a method of AphrontCursorPagedPolicyAwareQuery
D12467 / rP8f61eb45ab3b: Give tokenizer tokens CSS color classes on the container instead of the icon
rP52c6a9707f92: Force typeahead datasource response into expected array format
rPe558a2e729ee: Always set a typeahead result limit, even for non-browse queries
D12444 / rP845466b49bf8: Implement viewer() and members(project) typeahead functions
D12446 / rP76448a75de30: Make token UI stronger and more consistent
D12447 / rPea97d75a6775: Make Differential use viewer's token instead of viewer() token again
D12224 / rPd403700e1fc6: Convert all tokenizers to take token/scalar inputs

Related Objects

StatusAssignedTask
Resolvedchad
ResolvedNone
Resolvedepriestley
DuplicateNone
ResolvedNone
Wontfixepriestley
Resolvedepriestley
Openepriestley
Resolvedepriestley
ResolvedNone
ResolvedNone
DuplicateNone
Wontfixepriestley
DuplicateNone
Resolvedepriestley
OpenNone
Wontfixepriestley
Resolvedepriestley
Resolvedchad
Resolvedepriestley
Wontfixepriestley
OpenNone
OpenNone
Resolvedepriestley
ResolvedNone
Resolvedepriestley
Resolvedepriestley
Resolvedepriestley
Wontfixepriestley

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
witrin added a subscriber: witrin.
Pawka added a subscriber: Pawka.Apr 13 2015, 11:42 AM
epriestley raised the priority of this task from Wishlist to Normal.

In D12444 I changed the default Differential query to use viewer(), instead of implicitly resolving the user's PHID. That means that when you "Edit Query", you get:

Responsible Users: [ Current Viewer ]

...instead of this, which is what you'd see previously:

Responsible Users: [ epriestley ]

I'm leaning toward reverting this. While using viewer() helps users discover tokenizer functions, I think these are a pretty advanced feature, and a new user is more likely to get a more useful understanding by seeing their own name (i.e., it implies "I can type a username to chose a user", while the viewer() token doesn't really suggest anything helpful about the typeahead).

It also makes the "Edit Query -> Change Something -> Execute -> Share URL" workflow more confusing, because the URL with someone else as the viewer will show them a completely different result set. (And if the recipient is logged out, the query will fail with an error.) This probably isn't very common, but using "epriestley" gives this workflow more predictable behavior.

Defaulting to viewer() never seems better for workflow reasons (except in cases where whoever is editing the query is sophisticated and intentionally trying to construct a parametrized query, but we don't need to look out for sophisticated users as much), and I think it's advanced enough that it doesn't make sense to try to drive function discovery on a user's first encounter with typeaheads. Function results show up as normal typeahead results and are discoverable through "Browse..." and will get documented at some point. These feel more appropriate for discovery of this feature than surfacing defaults in terms of tokenizer functions.

A big chunk of this is in HEAD. This should be where we are, roughly:

  • All typeaheads/tokenizers should still work correctly.
  • "Browse" button should work correctly everywhere.
  • Differential "Responsible Users" field should now let you you use viewer() and members() functions.

This stuff is still known to be janky:

  • Design needs a bit of work.
  • Every search UI should support functions.
  • We should have more functions.
  • Function sort order can probably be better.
  • Browse UI could probably do a better job of explaining function tokens.
  • Browse UI dialog title is off.
  • Documentation.

All typeaheads/tokenizers should still work correctly.

I'm seeing some issues on this install that I can't reproduce locally, looking into it...

epriestley added a revision: Restricted Differential Revision.Apr 20 2015, 12:40 PM
epriestley added a commit: Restricted Diffusion Commit.Apr 20 2015, 7:53 PM
talshiri removed a subscriber: talshiri.Apr 23 2015, 6:51 PM
epriestley closed this task as Resolved.Apr 23 2015, 7:08 PM

These aren't universally deployed yet, but are substantially complete and are deployed in multiple applications. See T7858 for some followups. These will deploy everywhere over time, although they may not come to some lesser-used applications for a bit.