Page MenuHomePhabricator

Search field in header doesn't show both tags and (disabled, bot, or mailing list) users with same name
Closed, ResolvedPublic

Description

Observed on Phacility

We have a bot user called instabug and a project called instabug: search field only shows one

Event Timeline

swisspol created this task.Apr 11 2017, 6:23 PM

This reproduces for me locally. (??!?)

This is a sort-of-intended result of several different rules colliding to make a mess. Here's the primary rule:

  • Disabled users (and other types of disabled objects, but that isn't relevant here) are considered "closed".
  • Some typeaheads, including this one, only show "closed" results if there are only "closed" results to display. The logic here is that if you type ev..., you probably mean evan (active user), not evaaaaaa (disabled user) 99% of the time. You can get the disabled user by typing enough of their name that they are the only match.
  • Mailing list and bot users are also considered "closed" because you usually want to hit normal users instead of them.

So the logic goes:

  • There's an open result (the project) and a closed result (the bot user) matching the query. It is overwhelmingly more likely that the user wants to select the open result than the closed result, so hide the closed result.
  • (Hiding is relevant if there are 5+ open results.)

Obviously, this does not produce the right outcome here, although fixing it properly is probably somewhat involved (e.g., see T11110, T6906).

All that said, I'm not sure we need the filtering at all (?) because later changes also do this with sorting. Except not in this typeahead. But it isn't really important anyway. Probably. I'm going to try a one-line "fix" here and see if it digs us any deeper or not.

epriestley renamed this task from Search field in header doesn't show both tags and users with same name to Search field in header doesn't show both tags and (disabled, bot, or mailing list) users with same name.Apr 11 2017, 7:13 PM

D17656 is legitimately pretty hacky so this is now entangled with T11110, etc.

epriestley triaged this task as Low priority.Apr 11 2017, 7:40 PM

My intended fix here is:

  • Remove all callers to JX.Prefab.filterClosedResults() and stop filtering results. This will globally remove the behavior where disabled results are filtered from typeaheads.
  • Instead, sort closed results to the bottom. JX.Prefab.sortHandler() should already do this, but the behavior-search-typeahead.js sort handler currently does not. This will mean that closed/disabled results always appear, but are always ranked lower than open results.
  • We could selectively retain the current behavior for usernames only. However, I think it already has a bug which no one has managed to hit: if user @abc is disabled and user @abcd is enabled, I think it is currently impossible to select user @abc. The correct suggestions in this case clearly seem to be: @abc, @abcd which suggests we should drop the filtering rule even from username typeaheads.
  • Datasources may eventually need to perform equivalent ranking, but the scenario which breaks is obscure and would already cause issues. It looks like this:
    • Users @abc001, @abc002, ..., @abc100, all disabled.
    • User @abc101 is enabled.
    • You type abc.
    • @abc101 should be the top hit, but will not be part of the result page because there are 100 other results, so it will not appear. (You can find it by querying for abc1 instead.)
    • This is a pre-existing problem with filtering, too, and can be remedied through server-side pre-sorting or datasource query phases (D16838) if it ever occurs. It is possible it may occur for projects in the wild eventually, for example with 100 disabled milestones.

Another instance of the bug her:

Interestingly, when I click on the search button of the search field, then filter by "instabug", I see both the project and user, BUT the user is struck through for some reason I don't understand?

Ah I missed that sentence above, this is why the bot user is disabled:

Mailing list and bot users are also considered "closed" because you usually want to hit normal users instead of them.

Yeah, the user is struck through because they are "closed", which is because they are a bot. This isn't the best way to deal with this, but it dates from early 2014 (D8231) when everything was simpler.

T12157 lists some other cases where we're currently inconsistent about this that should be consolidated.

More modern object types are more nuanced about these sorts of distinctions, user accounts just haven't changed much in a long time.