Use terms aggs instead of composite refactor of Authentications Table based on feedback #29283

FrankHassanabad · 2019-01-24T18:20:15Z

Made it sort by desc of number of failures
Reduced type usages where possible to ECS based types
Added newer fields to the Authentications based on feedback which is based on last success and last failure.
Removed the upperLimit from uncommon process and refactored to be the same terms patter that Authentications is using
Fixed the bug with uncommon process where it could not re-limit its self back to 5 rows if you expanded it.
Added a new extendMap for ECS mapping fields to make things easier
Replaced ares where unit tests as it( in favor of test(
Added a escapeDataProviderId and consolidated code to use that
Replaced Types that were incorrect in areas from functions to ActionCreator
Pushed up higher the Types of Edges in the code to remove less boilerplate
Introduced a few functions and unit tests to reduce boiler plate in a few areas
Made all appropriate cells of AuthenticationTable draggable
Replaced areas that use moment().valueOf() for Date.now() as it's more semantic and we can remove moment where that's all we were using it for.
Added unit tests for all the above or refactored existing unit tests.
https://github.com/elastic/ingest-dev/issues/218

After:

After in dark mode where other columns are draggable:

Before:

elasticmachine · 2019-01-24T18:23:26Z

Pinging @elastic/secops

x-pack/plugins/secops/public/components/page/hosts/authentications_table/index.tsx

x-pack/plugins/secops/server/lib/ecs_fields/extend_map.ts

x-pack/plugins/secops/server/lib/authentications/query.dsl.ts

* Made it sort by desc of number of failures * Reduced type usages where possible to Ecs based types * Added newer fields to the Authentications based on feedback

FrankHassanabad · 2019-01-28T04:31:18Z

x-pack/plugins/secops/public/components/flyout/index.test.tsx

@@ -204,7 +205,7 @@ describe('Flyout', () => {
    });

    test('should call the onOpen when the mouse is clicked for rendering', () => {
-      const showTimeline = jest.fn();
+      const showTimeline = (jest.fn() as unknown) as ActionCreator<{ id: string; show: boolean }>;


note: with stricter types using ActionCreator, jest.fn() needs me to use this ugly casting to work? If there's another way I would like to know.

FrankHassanabad · 2019-01-28T04:31:23Z

x-pack/plugins/secops/public/components/flyout/index.test.tsx

@@ -234,7 +235,7 @@ describe('Flyout', () => {
      const stateShowIsTrue = set('local.timeline.timelineById.test.show', true, state);
      const storeShowIsTrue = createStore(stateShowIsTrue);

-      const showTimeline = jest.fn();
+      const showTimeline = (jest.fn() as unknown) as ActionCreator<{ id: string; show: boolean }>;


note: with stricter types using ActionCreator, jest.fn() needs me to use this ugly casting to work? If there's another way I would like to know.

elasticmachine · 2019-01-28T05:54:03Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

x-pack/plugins/secops/public/components/empty_page/index.test.tsx

spong · 2019-01-28T19:38:13Z

x-pack/plugins/secops/public/components/page/hosts/authentications_table/index.tsx

              },
              queryDate: {
                from: startDate,
-                to: moment().valueOf(),
+                to: Date.now(),


I've seen this a few other places (like in HostsTable), but what's the intent of using the current time for queryDate.to? Why would this not be the to value of the GlobalTime component that sets the time constraints for the widget itself?

It should probably be constrained to the global date time. The Date.now() kind of shows the bug more clearly it looks like.

andrew-goldstein · 2019-01-28T20:44:00Z

x-pack/plugins/secops/public/components/page/hosts/events_table/index.tsx

          <DraggableWrapper
            dataProvider={{
              and: [],
              enabled: true,
-              id: node._id!,
+              id: escapeDataProviderId(`events-table-${node._id!}-hostName-${hostName}`),


much appreciated!

andrew-goldstein · 2019-01-28T20:44:53Z

x-pack/plugins/secops/public/components/page/hosts/hosts_table/index.tsx

          <DraggableWrapper
            dataProvider={{
              and: [],
              enabled: true,
              excluded: false,
-              id: node._id!,
+              id: escapeDataProviderId(`hosts-table-${node._id!}-hostName-${hostName}`),


thanks for adding this

andrew-goldstein

The additional query capabilities in the form of draggable content to the timeline, fixes to make provider IDs globally unique, and type safety in this PR are all appreciated @FrankHassanabad !

spong

Echoing Andrew -- the type cleanup (especially in regards to ActionCreator) is greatly appreciated. As well with the introduction of escapeDataProviderId() for dataprovider id generation -- will use this going forward. LGTM!

FrankHassanabad added the loe:medium Medium Level of Effort label Jan 24, 2019

FrankHassanabad requested review from spong, XavierM and andrew-goldstein January 24, 2019 18:21

FrankHassanabad added the Team:SIEM label Jan 24, 2019

This comment has been minimized.

Sign in to view

andrew-goldstein reviewed Jan 24, 2019

View reviewed changes

x-pack/plugins/secops/public/components/page/hosts/authentications_table/index.tsx Outdated Show resolved Hide resolved

andrew-goldstein reviewed Jan 24, 2019

View reviewed changes

x-pack/plugins/secops/public/components/page/hosts/authentications_table/index.tsx Outdated Show resolved Hide resolved

andrew-goldstein reviewed Jan 24, 2019

View reviewed changes

x-pack/plugins/secops/public/components/page/hosts/authentications_table/index.tsx Outdated Show resolved Hide resolved

andrew-goldstein reviewed Jan 24, 2019

View reviewed changes

x-pack/plugins/secops/public/components/page/hosts/authentications_table/index.tsx Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

andrew-goldstein reviewed Jan 24, 2019

View reviewed changes

x-pack/plugins/secops/server/lib/ecs_fields/extend_map.ts Show resolved Hide resolved

andrew-goldstein reviewed Jan 25, 2019

View reviewed changes

x-pack/plugins/secops/server/lib/authentications/query.dsl.ts Show resolved Hide resolved

Refactored plumbing of agg terms of Authentications

5d8917d

* Made it sort by desc of number of failures * Reduced type usages where possible to Ecs based types * Added newer fields to the Authentications based on feedback

FrankHassanabad force-pushed the use-terms-aggs branch from 1972f3f to 5d8917d Compare January 25, 2019 19:54

This comment has been minimized.

Sign in to view

Updated from more feedback and fixed more types and other misc issues

308d536

FrankHassanabad commented Jan 28, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

Fixed ordering of imports

78b962c

This comment has been minimized.

Sign in to view

Add forgotten type change.

8b5766e

spong reviewed Jan 28, 2019

View reviewed changes

x-pack/plugins/secops/public/components/empty_page/index.test.tsx Show resolved Hide resolved

spong reviewed Jan 28, 2019

View reviewed changes

andrew-goldstein reviewed Jan 28, 2019

View reviewed changes

andrew-goldstein approved these changes Jan 28, 2019

View reviewed changes

spong approved these changes Jan 28, 2019

View reviewed changes

FrankHassanabad merged commit 43b02a3 into elastic:feature-secops Jan 28, 2019

FrankHassanabad deleted the use-terms-aggs branch January 28, 2019 21:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use terms aggs instead of composite refactor of Authentications Table based on feedback #29283

Use terms aggs instead of composite refactor of Authentications Table based on feedback #29283

FrankHassanabad commented Jan 24, 2019 •

edited

Loading

elasticmachine commented Jan 24, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

FrankHassanabad Jan 28, 2019 •

edited

Loading

FrankHassanabad Jan 28, 2019 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

elasticmachine commented Jan 28, 2019

spong Jan 28, 2019

andrew-goldstein Jan 28, 2019

FrankHassanabad Jan 28, 2019

andrew-goldstein Jan 28, 2019

andrew-goldstein Jan 28, 2019

andrew-goldstein left a comment

spong left a comment

Use terms aggs instead of composite refactor of Authentications Table based on feedback #29283

Use terms aggs instead of composite refactor of Authentications Table based on feedback #29283

Conversation

FrankHassanabad commented Jan 24, 2019 • edited Loading

elasticmachine commented Jan 24, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

FrankHassanabad Jan 28, 2019 • edited Loading

Choose a reason for hiding this comment

FrankHassanabad Jan 28, 2019 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

elasticmachine commented Jan 28, 2019

💚 Build Succeeded

spong Jan 28, 2019

Choose a reason for hiding this comment

andrew-goldstein Jan 28, 2019

Choose a reason for hiding this comment

FrankHassanabad Jan 28, 2019

Choose a reason for hiding this comment

andrew-goldstein Jan 28, 2019

Choose a reason for hiding this comment

andrew-goldstein Jan 28, 2019

Choose a reason for hiding this comment

andrew-goldstein left a comment

Choose a reason for hiding this comment

spong left a comment

Choose a reason for hiding this comment

FrankHassanabad commented Jan 24, 2019 •

edited

Loading

FrankHassanabad Jan 28, 2019 •

edited

Loading

FrankHassanabad Jan 28, 2019 •

edited

Loading