Page MenuHomePhabricator

Typeahead - filter typeaheads that the viewer can't see in typeahead debug tool
ClosedPublic

Authored by btrahan on Apr 6 2015, 7:23 PM.
Tags
None
Referenced Files
F13180967: D12307.id29550.diff
Thu, May 9, 6:35 AM
F13180958: D12307.id29571.diff
Thu, May 9, 6:31 AM
Unknown Object (File)
Tue, May 7, 7:36 AM
Unknown Object (File)
Sat, May 4, 3:22 AM
Unknown Object (File)
Fri, May 3, 7:00 AM
Unknown Object (File)
Thu, Apr 25, 1:23 AM
Unknown Object (File)
Apr 2 2024, 6:37 AM
Unknown Object (File)
Mar 28 2024, 1:48 PM
Subscribers

Details

Summary

Fixes T7255.

Note however that some datasources - notably user or project - don't implement the class thing in a clean way since multiple classes apply. For now, we just show these datasources to the user.

Also, I guess this could be done more efficiently by querying for all the applications at once via an application query? LMK if you want me to make that change.

Test Plan

loaded /typeahead/class/ and played with it a bit with no issues

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan retitled this revision from to Typeahead - filter typeaheads that the viewer can't see in typeahead debug tool.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley edited edge metadata.

For now, we just show these datasources to the user.

Yep, that's correct.

querying for all the applications at once via an application query

We should either do this or do the filtering lower down -- see inlines.

src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php
24

This controller gets hit when you're just typing stuff in a tokenizer, too, which is latency-sensitive. Although doing policy checks normally shouldn't be slow, this endpoint is one of the most important to keep responsive.

Particularly, when $class is set and the request is AJAX (by far the most common case), we only need to filter one source (and we don't really "need" to filter it, but it's probably good to do it).

As written, this will filter everything every time in a somewhat-inefficient way. Instead...

33–34

If we hit this, just check $source. This is "probably" not necessary, but desirable to add for completeness and safety.

54

Do the full-list check here, at the last moment before we generate $options. That way, the code won't even run on the common path (generating real typeahead results). It's fine to do it not-totally-efficiently here since it's only impacting the debug view.

This revision now requires changes to proceed.Apr 6 2015, 7:30 PM
btrahan edited edge metadata.

do a better job not pillaging performance

epriestley edited edge metadata.
This revision is now accepted and ready to land.Apr 6 2015, 10:23 PM
This revision was automatically updated to reflect the committed changes.