UI: Change global search to use fuzzy search API #10412
Conversation
I’ll delete these later after I’ve replaced them.
Ember Asset Size actionAs of d2aa291 Files that got Smaller 🎉:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
backspace
force-pushed
the
f-ui/fuzzy-search
branch
from
1dad60c to
5097053
Compare
backspace
force-pushed
the
f-ui/fuzzy-search
branch
from
cdf88c7 to
72d6200
Compare
| id, | ||
| namespace, | ||
| label: name, | ||
| })); |
There was a problem hiding this comment.
This is a really clean way to handle the API response shape.
| if (!this.task) { | ||
| return ''; | ||
| } | ||
|
|
There was a problem hiding this comment.
This seems unrelated. How'd you end up here?
There was a problem hiding this comment.
I don’t totally understand but I was experiencing test failures when navigating to some search results and had to add a couple of null-handlings, I suspect it has something to do with the way the Mirage models are being constructed. With the time constraint I sadly just added the guards without digging further to grasp the underlying issue 😞
There was a problem hiding this comment.
That's reasonable!
| exchangeOneTimeToken = this.get('token').exchangeOneTimeToken(transition.to.queryParams.ott); | ||
| // service:router#transitionTo can cause this to rerun because of refreshModel on | ||
| // the region query parameter, this skips rerunning the detection/loading queries. | ||
| if (transition.queryParamsOnly) { |
There was a problem hiding this comment.
TIL. Nice sleuthing here.
| plugins: truncatedPlugins.length < transformedPlugins.length, | ||
| }, | ||
| } | ||
| }); |
There was a problem hiding this comment.
Woooah, super cool to see this emulated in Mirage.
| function nonSearchPOSTS() { | ||
| return server.pretender.handledRequests.reject(request => request.url.includes('fuzzy')).filterBy('method', 'POST'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Seeing stuff like this always makes me want to write some sort of lib that makes building this type of behavior on top of Pretender easier.
|
the test audit problem seems to have resolved itself? 🙃🎉 |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This updates the UI to use the new fuzzy search API. It’s a drop-in replacement so the
/shortcut to jump to search is preserved, and results can be cycled through and chosen via arrow keys and the enter key.It doesn’t use everything returned by the API:
Other design choices that might warrant changes:
+to the full count when the response indicates server-side truncation._
route:application#beforeModelthat acts as feature detection: if the attempt to query fails (500), the global search field is hidden.Upon having added another query on load, I realised that
beforeModelwas being triggered any timeservice:router#transitionTowas being called, which happens upon navigating to a search result, for instance, because ofrefreshModelbeing present on the region query parameter. This PR adds a check fortransition.queryParamsOnlyand skips rerunning the onload queries (token permissions check, license check, fuzzy search feature detection).Implementation notes:
undefinedwere required to address failures when navigating to an allocationmin_term_lengthin the UIFuture API improvements/wishlist to record elsewhere:
min_term_lengthhas been changed from the default of 2FINALLY, there’s a mysterious problem with the Ember test audit comparison where entire test files aren’t running. I spent 1.5 days trying to address this through CI laparoscopy to no avail. Since it’s not affecting CircleCI nor happening locally, I’m submitting this for review with that known problem that I’ll continue trying to address 😞