[Security Solution][Resolver] Resolver query panel load more #79160

kqualters-elastic · 2020-10-01T16:28:23Z

Summary

This pr adds a load more feature to the events by type panel view of resolver. When a user clicks into the events by type panel, either from the graph directly or by drilling down, a request is made for the first 25 events in a category. If there are >25 total events, a cursor field is provided, a base64 encoded string of the id and timestamp of event 25, which is then provided in the next request for the 2nd page. Each response is appended to the list of events in the UI each time a user selects the "Load More Data" button. Once a response comes back without the nextEvent cursor, the button is hidden. An error message is shown for either the initial or paginated requests.

Checklist

Delete any items that are not applicable to this PR.

[ * ] Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

oatkiller · 2020-10-05T14:33:02Z

x-pack/plugins/security_solution/public/resolver/store/data/reducer.ts

+    const nextState: DataState = {
+      ...state,
+      nodeEventsInCategory: {
+        nodeID: '',


I'm not following the logic here.

You're spreading in state.nodeEventsInCategory. You shouldn't need to add their nullish fields. What TS error are you getting?

oatkiller · 2020-10-05T14:42:32Z

I think the code here needs to be removed:
https://github.com/elastic/kibana/pull/79160/files#diff-9d589d4a609bfedc40cd62d6da18fbc2R55-R65

elasticmachine · 2020-10-05T20:52:54Z

Pinging @elastic/endpoint-app-team (Feature:Resolver)

oatkiller · 2020-10-05T14:36:02Z

x-pack/plugins/security_solution/public/resolver/store/middleware/related_events_fetcher.ts

+    } else if (action.type === 'userRequestedAdditionalRelatedEvents') {
+      const nodeEventsInCategory = state.data.nodeEventsInCategory;
+      if (nodeEventsInCategory !== undefined) {
+        api.dispatch({


Can we do away with this action? The state should already know that loading is happening. Also, the initial request for data doesn't fire any action like this.

oatkiller · 2020-10-05T14:38:09Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx

+  const handleLoadMore = useCallback(() => {
+    dispatch({
+      type: 'userRequestedAdditionalRelatedEvents',
+      payload: {},


Can you remove the payload here.

oatkiller · 2020-10-05T14:38:25Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx

+      {hasMore && (
+        <EuiFlexItem grow={false}>
+          <EuiButton color={'primary'} size="s" fill onClick={handleLoadMore} isLoading={isLoading}>
+            {'Load More Data'}


Can you wrap this in i18n.

oatkiller · 2020-10-05T20:30:10Z

x-pack/plugins/security_solution/public/resolver/data_access_layer/factory.ts

@@ -50,7 +52,7 @@ export function dataAccessLayerFactory(
      after?: string
    ): Promise<ResolverPaginatedEvents> {
      return context.services.http.post('/api/endpoint/resolver/events', {
-        query: { afterEvent: after },
+        query: { afterEvent: after, limit: relatedEventsPaginationSize },


Maybe this should be specified on the BE as the 'default' instead of specifying it here. That way we don't have two places defining this value.

oatkiller · 2020-10-05T20:33:12Z

...y_solution/public/resolver/data_access_layer/mocks/one_node_with_paginated_related_events.ts

+          (event) => event.event?.category === category
+        );
+        if (after === undefined) {
+          events = eventsOfCategory.slice(0, relatedEventsPaginationSize);


I don't think this mock should reference relatedEventsPaginationSize. I think you could redefine this here const relatedEventsPaginationSize = 25. I think you even use any arbitrary number you want. Is there some reason I'm missing?

oatkiller · 2020-10-05T20:53:51Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.test.tsx

+            () =>
+              simulator().testSubject('resolver:panel:node-events-in-category:event-link').length
+          )
+        ).toYieldEqualTo(30);


The test says 31 but this checks for 30. can you correct the test message?

oatkiller · 2020-10-05T20:54:14Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.test.tsx

+            () => simulator().testSubject('resolver:nodeEventsInCategory:loadMore').length
+          )
+        ).toYieldEqualTo(0);
+        await expect(


can you combine this expect with the one above it?

oatkiller · 2020-10-05T20:54:36Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx

+              iconType="alert"
+              data-test-subj="resolver:nodeEventsInCategory:error"
+            >
+              <p>{'An error occurred when fetching the events.'}</p>


Can you put this in i18n?

oatkiller · 2020-10-05T20:54:39Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx

-          <NodeEventList eventCategory={eventCategory} nodeID={nodeID} events={events} />
+          {hasError ? (
+            <EuiCallOut
+              title="Unable to load events."


Can you put this in i18n?

oatkiller · 2020-10-05T20:57:13Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx

@@ -207,7 +254,7 @@ const NodeEventsInCategoryBreadcrumbs = memo(function ({
          text: (
            <FormattedMessage
              id="xpack.securitySolution.endpoint.resolver.panel.relatedEventList.numberOfEvents"
-              values={{ totalCount: eventCount }}
+              values={{ totalCount: eventCount || '0' }}


Can you instead hide this text when the event count hasn't loaded yet? Showing 0 during loading could be strange if the request is slow to come back.

…of state

michaelolo24 · 2020-10-05T21:42:42Z

x-pack/plugins/security_solution/public/resolver/mocks/resolver_tree.ts

+  for (let i = 0; i < eventsToGenerate; i++) {
+    const newEvent = mockEndpointEvent({
+      entityID: originID,
+      eventID: 'test',


Maybe make the eventID something like test-${i} in case we want to interact with the individual events on a page-by-page basis in the test we can, and verify the right event shows up at the right time

michaelolo24 · 2020-10-05T22:00:51Z

x-pack/plugins/security_solution/public/resolver/store/middleware/related_events_fetcher.ts

            },
          });
        }
-      } else if (newParams.panelView === 'eventDetail') {


michaelolo24 · 2020-10-05T22:02:04Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx

  return (
    <>
-      {eventCount === undefined || processEvent === null ? (
+      {isLoading || processEvent === null ? (


michaelolo24 · 2020-10-05T22:03:27Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx

+              )}
+              color="danger"
+              iconType="alert"
+              data-test-subj="resolver:nodeEventsInCategory:error"


doesn't really matter (nit?). I think we're hyphenating these node-events-in-category

I'll add follow up ticket

michaelolo24 · 2020-10-05T22:12:15Z

x-pack/plugins/security_solution/public/resolver/view/panels/node_events_of_type.tsx

-          <EuiSpacer size="l" />
-          <NodeEventList eventCategory={eventCategory} nodeID={nodeID} events={events} />
+          {hasError ? (
+            <EuiCallOut


Maybe we migrate this to panel_content_error and expand from this to error, 404, and loading component to be used in the graph itself and the panel like @oatkiller suggested a while back

michaelolo24

Just tested it, looks good. Nice work! 💪

kibanamachine · 2020-10-06T00:06:15Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 540a6a7

Metrics [docs]

async chunks size

id	before	after	diff
`securitySolution`	10.3MB	10.3MB	+14.1KB

History

💔 Build #79918 failed fff7a39
💚 Build #79889 succeeded bef65d3
💚 Build #79858 succeeded 52f4d1912b63257b62d98f510971092ec8273bf0
💚 Build #79843 succeeded ceb4363
💚 Build #79622 succeeded 532332d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…#79160)

…79160) (#79603)

* master: (85 commits) Refactor attribute service (elastic#78414) [APM] Add default message to alerts. (elastic#78930) [Discover] Modify columns and sort when switching index pattern (elastic#74336) Document ts project references setup (elastic#78586) build all ts refs in single kbn:bootstrap (elastic#79438) [TSVB] Allow string fields on value count aggregation (elastic#79267) [SECURITY SOLUTION] Investigate EQL signal in timeline (elastic#79049) [Fleet] Add loading spinner to page headers (elastic#79568) [Security Solution][Resolver] Resolver query panel load more (elastic#79160) Add type row to monitor detail page. (elastic#79556) Remove license refresh from setup (elastic#79518) [docker] add reporting fonts (elastic#74806) [SECURITY_SOLUTION][ENDPOINT] Add info about trusted apps to the page subtitle + create flyout (elastic#79558) Trim Hash value before validating it (elastic#79545) Warn users when security is not configured (elastic#78545) update copy styling (elastic#79313) Update dependency @elastic/charts to v23.1.1 (elastic#78459) Introduce geo-threshold alerts (elastic#76285) elastic#76920 Show base breadcrumb when there is an error booting the app (elastic#79571) remove react-intl from kibana and keep it inside only i18n package (elastic#78956) ...

kqualters-elastic added 5 commits September 30, 2020 11:35

Button that doesn't do anything

4105a7f

Loads more data, needs polish

5cf23f9

Add loading state

5db93a7

Merge branch 'master' into resolver-query-panel-load-more

2545069

Add error handling action

532332d

kqualters-elastic force-pushed the resolver-query-panel-load-more branch from 737c82e to 532332d Compare October 5, 2020 13:57

oatkiller reviewed Oct 5, 2020

View reviewed changes

kqualters-elastic added 2 commits October 5, 2020 15:29

Add simulator test, refactor some selectors

ceb4363

Fix off by one in mocks

bef65d3

kqualters-elastic force-pushed the resolver-query-panel-load-more branch from 52f4d19 to bef65d3 Compare October 5, 2020 20:43

kqualters-elastic added v7.10.0 Feature:Resolver Security Solution Resolver feature release_note:skip Skip the PR/issue when compiling release notes labels Oct 5, 2020

kqualters-elastic marked this pull request as ready for review October 5, 2020 20:49

kqualters-elastic requested review from a team as code owners October 5, 2020 20:49

kqualters-elastic changed the title ~~Resolver query panel load more~~ [Security Solution][Resolver] Resolver query panel load more Oct 5, 2020

oatkiller suggested changes Oct 5, 2020

View reviewed changes

Address pr comments, change middleware and remove unnecessary pieces …

fff7a39

…of state

michaelolo24 reviewed Oct 5, 2020

View reviewed changes

Dedupe middlware logic, more pr feedback

44885fb

michaelolo24 reviewed Oct 5, 2020

View reviewed changes

michaelolo24 approved these changes Oct 5, 2020

View reviewed changes

Rename selector

540a6a7

oatkiller approved these changes Oct 5, 2020

View reviewed changes

kqualters-elastic merged commit 32d45b6 into elastic:master Oct 6, 2020

kqualters-elastic deleted the resolver-query-panel-load-more branch October 6, 2020 01:43

kqualters-elastic mentioned this pull request Oct 6, 2020

[7.x] [Security Solution][Resolver] Resolver query panel load more (#79160) #79603

Merged

kqualters-elastic added a commit to kqualters-elastic/kibana that referenced this pull request Oct 6, 2020

[Security Solution][Resolver] Resolver query panel load more (elastic…

fde45f6

…#79160)

kqualters-elastic added a commit that referenced this pull request Oct 6, 2020

[7.x] [Security Solution][Resolver] Resolver query panel load more (#…

649536c

…79160) (#79603)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Resolver] Resolver query panel load more #79160

[Security Solution][Resolver] Resolver query panel load more #79160

kqualters-elastic commented Oct 1, 2020 •

edited

Loading

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller commented Oct 5, 2020

elasticmachine commented Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

oatkiller Oct 5, 2020

michaelolo24 Oct 5, 2020

michaelolo24 Oct 5, 2020

michaelolo24 Oct 5, 2020

michaelolo24 Oct 5, 2020

oatkiller Oct 5, 2020

michaelolo24 Oct 5, 2020

michaelolo24 left a comment

kibanamachine commented Oct 6, 2020

[Security Solution][Resolver] Resolver query panel load more #79160

[Security Solution][Resolver] Resolver query panel load more #79160

Conversation

kqualters-elastic commented Oct 1, 2020 • edited Loading

Summary

Checklist

For maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oatkiller commented Oct 5, 2020

elasticmachine commented Oct 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 6, 2020

💚 Build Succeeded

Metrics [docs]

async chunks size

History

kqualters-elastic commented Oct 1, 2020 •

edited

Loading