Page MenuHomePhabricator

"PeopleDatasource" improperly uses "closed" state to sort bot/mailing list results to the bottom of the list
Open, LowPublic

Description

See downstream https://phabricator.wikimedia.org/T227963. See also https://discourse.phabricator-community.org/t/searching-for-bot-accounts-in-the-search-box-displays-them-as-disabled-crossed-out-gray/2924/.

PhabricatorPeopleDatasource currently has this block:

$closed = null;
if ($user->getIsDisabled()) {
  $closed = pht('Disabled');
} else if ($user->getIsSystemAgent()) {
  $closed = pht('Bot');
} else if ($user->getIsMailingList()) {
  $closed = pht('Mailing List');
}

This code is very old and approximately originated in D8231.

Our goal here is twofold:

  • Visually identify these accounts as nonstandard accounts.
  • Sort them to the bottom of the list, so users typing @alinc... hit actual users before bots.

Using the setClosed("Human Readable Label") mechanism for this works well in normal tokenizers, and produces good user-facing behavior:

Screen Shot 2019-07-14 at 7.59.30 AM.png (107×302 px, 8 KB)

However, in autocomplete contexts and the global search typeahead, closed results are rendered with a strikethru:

Screen Shot 2019-07-14 at 7.59.37 AM.png (125×444 px, 12 KB)

Screen Shot 2019-07-14 at 7.59.17 AM.png (168×363 px, 14 KB)

The sorting goal is still accomplished in these interfaces, but the rendering implies these accounts are disabled and we're generally conflating "sort to the bottom", "disabled", and "has special attributes". These should probably be three separate attributes with independent behavior.