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!

Revisions and Commits

Restricted Differential Revision
rP Phabricator
Closed
D12532
D12533
D12530
D12528
D12531
D12529
D12526
D12524
D12527
D12525
D12523
D12482
D12470
D12469
D12468
D12460
D12462
D12463
D12459
D12458
D12455
D12456
D12457
D12454
D12453
D12467
Audited
Audited
D12444
D12446
D12447
D12224

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
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

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.