refactor: [M3-9510] - Clean up main search implementation #11819
Conversation
bnussman-akamai
requested review from
dwiley-akamai and
hana-akamai
and removed request for
a team
bnussman-akamai
changed the title
bnussman-akamai
changed the title
| export const getDescriptionForCluster = (cluster: KubernetesCluster) => { | ||
| const description: string[] = [ | ||
| `Kubernetes ${cluster.k8s_version}`, | ||
| region?.label ?? cluster.region, |
There was a problem hiding this comment.
The one trade-off this PR makes is that our "entityToSearchableItem" functions must be pure and not require extra API data. This is functionality we can add back in the future, but not as easily right now.
|
Coverage Report: ❌ |
jaalah-akamai
left a comment
jaalah-akamai
left a comment
There was a problem hiding this comment.
✅ Tested operators: is, or, and, >
✅ Was able to filter by tags and labels
✅ Product family icons appear to work again
❓ I was not able to search for stackscripts in top menu search bar
|
@jaalah-akamai It should be working but I was missing some cache invalidations. Should work for sure now. 8c336b7 |
|
✅ confirmed |
jaalah-akamai
left a comment
jaalah-akamai
left a comment
There was a problem hiding this comment.
Nice work - approving
| const combinedResults = result.flatMap( | ||
| (r) => | ||
| r.data?.pages.flatMap((p) => | ||
| p.data.map(r.getSearchableItem as (i: unknown) => SearchableItem) |
There was a problem hiding this comment.
I'm sure this type assertion is fine here. Adding a type guard would be nice, but it looks to add a bunch more boilerplate for something I think we can be pretty certain about.
There was a problem hiding this comment.
Yeah, I don't like this, but I think it is safe enough for now.
Ultimately, I'd like to refactor or remove the SearchableItem type completely because its type-safety is questionable at best. This PR improves it a tiny bit, but not significantly
dwiley-akamai
left a comment
dwiley-akamai
left a comment
There was a problem hiding this comment.
No functionality regressions observed ✅
Search queries & basic error handling with REACT_APP_FORCE_SEARCH_TYPE=api set ✅
packages/manager/.changeset/pr-11819-tech-stories-1741629552993.md
Outdated
Show resolved
Hide resolved
bnussman-akamai
added
Approved
…3.md Co-authored-by: Dajahi Wiley <[email protected]>
Co-authored-by: Dajahi Wiley <[email protected]>
Cloud Manager UI test results🎉 539 passing tests on test run #19 ↗︎
|
5 participants
Description 📝
This PR performs a light refactor of Cloud Manager's main search 🔍
By no means does it make search as good as I want it to be, but this takes a step in the right direction by cleaning up the implementation and introducing "Search v2" for large customers.
Changes 🔄
@linode/searchparser (Search v2) to give large customers better functionality (likeand,or,>,<, etc...) ✨1500Linodes will still use the existing search implementationREACT_APP_FORCE_SEARCH_TYPEenv variable to manually set which search engine Cloud Manager should useclientwill force Cloud Manager to use the existing client size search implimentationapiwill force Cloud Manager to use Search v2, which uses server-side API filteringPreview 📷
Note
No significant UI changes are expected
How to test 🧪
REACT_APP_FORCE_SEARCH_TYPE=apiin your .env file and test the new API search as if you were a large customertag: prod,label: lke or label: prod, etc...)Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅