-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Enterprise Search] [Behavioral analytics] Add search to collection list #155524
Conversation
actions.makeRequest({}); | ||
fetchAnalyticsCollections: async ({ query }, breakpoint) => { | ||
if (query) { | ||
await breakpoint(200); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: make it a constant.
@@ -50,12 +50,25 @@ export function registerAnalyticsRoutes({ | |||
router.get( | |||
{ | |||
path: '/internal/enterprise_search/analytics/collections', | |||
validate: {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we add a testcase for this?
}, | ||
}), | ||
path: ['enterprise_search', 'analytics', 'collections'], | ||
reducers: { | ||
isSearchRequest: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we still need isSearchRequest, now that we have searchQuery
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can avoid, but in that case we need to put some logic in query. Like if it's null(because we can't have undefined) that means we just fetched items, if it's empty that means we clear search. I would even split in two functions, like search and fetch, to make it more obvious. Because now when we use this functions we need to keep in mind logic besides it
05a258e
to
188c94e
Compare
… requests for fetching and searching
188c94e
to
73e0b7e
Compare
08b5aae
to
f7f29ae
Compare
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
…ist (elastic#155524) ✔️ Implement search for collection list Page ✔️ Add search query to fetch collection request ✔️ Update server-side to support new search query <img width="1162" alt="image" src="https://user-images.githubusercontent.com/17390745/233668294-0b3dd60b-a016-43dd-8362-91a368212d5c.png"> --------- Co-authored-by: Joseph McElroy <joseph.mcelroy@elastic.co>
✔️ Implement search for collection list Page
✔️ Add search query to fetch collection request
✔️ Update server-side to support new search query