[Security Solution] [Endpoint] Event filters uses the new card design #114126

dasansol92 · 2021-10-06T15:16:35Z

Summary

WIP

For maintainers

This was checked for breaking API changes and was labeled appropriately

dasansol92 · 2021-10-06T15:19:01Z

...s/security_solution/public/management/components/artifact_entry_card/artifact_entry_card.tsx

+              </p>
+            </EuiText>
+          ) : (
+            <CardComments comments={artifact.comments} />


As this component just displays the show/hide comments button if there is a comment, I'm thinking if we can just render it always and just let the condition for the description (if trustedApp then display description else null). Toughs?

did you forget to set data-test-subj here?

paul-tavares

Left some questions, but overall looks awesome

paul-tavares · 2021-10-06T20:44:52Z

...s/security_solution/public/management/components/artifact_entry_card/artifact_entry_card.tsx

-              {artifact.description || getEmptyValue()}
-            </p>
-          </EuiText>
+          {isTrustedApp(item as AnyArtifact) ? (


in order to avoid having to carry AnyArtifact checks, I would suggest adding an optional prop to this component called hideDescription instead. and maybe a second named hideComments. By default, both would be shown and then different implementation can hide them. So in Event filters, we would hide the description and in Trusted apps we would hide comments.

Note that if you go down that path, also review the props for the Collapsible card, since it inherits props from here and I don't think these new props would apply there.

Yeah, like it!

paul-tavares · 2021-10-06T20:48:08Z

...urity_solution/public/management/components/artifact_entry_card/components/card_comments.tsx

+      return getFormattedComments(comments);
+    }, [comments]);
+
+    const getButtonText = useCallback(


🤔 feels like this should just be a useMemo instead of callback

paul-tavares · 2021-10-06T20:49:18Z

...s/security_solution/public/management/components/artifact_entry_card/artifact_entry_card.tsx

+              </p>
+            </EuiText>
+          ) : (
+            <CardComments comments={artifact.comments} />


did you forget to set data-test-subj here?

paul-tavares · 2021-10-06T20:50:08Z

...urity_solution/public/management/components/artifact_entry_card/components/card_comments.tsx

+          onClick={onCommentsClick}
+          flush="left"
+          size="xs"
+          data-test-subj={getTestId('comments-label')}


probably don't need the comments prefix here and below.

x-pack/plugins/security_solution/public/management/components/artifact_entry_card/types.ts

paul-tavares · 2021-10-06T21:00:04Z

...y_solution/public/management/components/artifact_entry_card/utils/get_formatted_comments.tsx

+    timelineIcon: (
+      <EuiAvatar
+        size="s"
+        color={euiLightVars.euiColorLightestShade}


is this safe? what happens in dark theme? wondering if there is a way to grab the euiColorLIghtestShade that is theme agonistic?

I think I can use the useTheme hook but not sure at all. Will try it

paul-tavares · 2021-10-06T21:04:56Z

...y_solution/public/management/components/artifact_entry_card/utils/get_formatted_comments.tsx

+export const getFormattedComments = (comments: CommentsArray): EuiCommentProps[] => {
+  return comments.map((commentItem) => ({
+    username: commentItem.created_by,
+    timestamp: moment(commentItem.created_at).format('MMM D, YYYY'),


is this what they use in Exceptions? or are they using one of the Date formatters we define here: x-pack/plugins/security_solution/public/common/components/formatted_date/index.tsx (look at the <PreferenceFormattedDate /> or (my favorite) <FormattedRelativePreferenceDate />

Yes, this is a copy paste from the exception. I will take a look at the others, thanks

dasansol92 · 2021-10-07T10:30:06Z

@elasticmachine merge upstream

…esign-1767

elasticmachine · 2021-10-07T10:33:46Z

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

paul-tavares

🔥 🔥
LGTM. Awesome to see Event Filters cards match that of Trusted apps.

🚢

paul-tavares · 2021-10-11T13:15:38Z

...s/security_solution/public/management/components/artifact_entry_card/artifact_entry_card.tsx

 import { usePolicyNavLinks } from './hooks/use_policy_nav_links';
 import { MaybeImmutable } from '../../../../common/endpoint/types';

-export interface ArtifactEntryCardProps extends CommonProps {
+export interface CommonArtifactEntryCardProps extends CommonProps {


++1 on the name change

paul-tavares · 2021-10-11T13:21:44Z

x-pack/plugins/security_solution/public/management/components/artifact_entry_card/test_utils.ts

@@ -44,6 +46,14 @@ export const getExceptionProvider = () => {
        },
      ],
      tags: ['policy:all'],
+      comments: [
+        {
+          id: uuid.v4(),


I'm assuming it does not matter at this point, maybe because we are not looking for this data, but we might want to make this id and the created_at values static so that they are always the same for testing

paul-tavares · 2021-10-11T13:27:47Z

@elasticmachine merge upstream

…esign-1767

kevinlog · 2021-10-12T18:49:27Z

checked it out and ran it, looks great!

I tried adding, editing, and deleting and all of it works!

…esign-1767

kibanamachine · 2021-10-13T10:03:09Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: 00d1632
Storybooks Preview
Documentation Changes
Flaky suites:
- xpack-kibana-ciGroup11

Test Failures

Kibana Pipeline / general / X-Pack Detection Engine API Integration Tests.x-pack/test/detection_engine_api_integration/security_and_spaces/tests/aliases·ts.detection engine api security and spaces enabled Tests involving aliases of source indexes and the signals index should keep the original alias value such as "host_alias" from a source index when the value is indexed

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 1 times on tracked branches: https://github.com/elastic/kibana/issues/114581

[00:00:00]     │
[00:00:00]       └-: detection engine api security and spaces enabled
[00:00:00]         └-> "before all" hook in "detection engine api security and spaces enabled"
[00:00:00]         └-: 
[00:00:00]           └-> "before all" hook in ""
[00:00:00]           └-: Tests involving aliases of source indexes and the signals index
[00:00:00]             └-> "before all" hook for "should keep the original alias value such as "host_alias" from a source index when the value is indexed"
[00:00:00]             └-> "before all" hook for "should keep the original alias value such as "host_alias" from a source index when the value is indexed"
[00:00:00]               │ info [x-pack/test/functional/es_archives/security_solution/alias] Loading "mappings.json"
[00:00:00]               │ info [x-pack/test/functional/es_archives/security_solution/alias] Loading "data.json"
[00:00:00]               │ info [o.e.c.m.MetadataCreateIndexService] [node-01] [host_alias] creating index, cause [api], templates [], shards [1]/[1]
[00:00:00]               │ info [x-pack/test/functional/es_archives/security_solution/alias] Created index "host_alias"
[00:00:00]               │ debg [x-pack/test/functional/es_archives/security_solution/alias] "host_alias" settings {"index":{"refresh_interval":"1s","number_of_replicas":"1","number_of_shards":"1"}}
[00:00:00]               │ info [x-pack/test/functional/es_archives/security_solution/alias] Indexed 4 docs into "host_alias"
[00:00:00]             └-> should keep the original alias value such as "host_alias" from a source index when the value is indexed
[00:00:00]               └-> "before each" hook: global before each for "should keep the original alias value such as "host_alias" from a source index when the value is indexed"
[00:00:00]               └-> "before each" hook for "should keep the original alias value such as "host_alias" from a source index when the value is indexed"
[00:00:00]                 │ info [o.e.x.i.a.TransportPutLifecycleAction] [node-01] adding index lifecycle policy [.siem-signals-default-migration-cleanup]
[00:00:00]                 │ info [o.e.x.i.a.TransportPutLifecycleAction] [node-01] adding index lifecycle policy [.siem-signals-default]
[00:00:00]                 │ info [o.e.c.m.MetadataIndexTemplateService] [node-01] adding index template [.siem-signals-default] for index patterns [.siem-signals-default-*]
[00:00:00]                 │ info [o.e.c.m.MetadataCreateIndexService] [node-01] [.siem-signals-default-000001] creating index, cause [api], templates [.siem-signals-default], shards [1]/[1]
[00:00:00]                 │ info [o.e.x.i.IndexLifecycleTransition] [node-01] moving index [.siem-signals-default-000001] from [null] to [{"phase":"new","action":"complete","name":"complete"}] in policy [.siem-signals-default]
[00:00:04]               │ info [o.e.c.m.MetadataMappingService] [node-01] [.kibana_8.0.0_001/6DuR4icDRgibEQpQ3jN5fA] update_mapping [_doc]
[00:00:04]               │ info [o.e.x.i.IndexLifecycleTransition] [node-01] moving index [.siem-signals-default-000001] from [{"phase":"new","action":"complete","name":"complete"}] to [{"phase":"hot","action":"unfollow","name":"branch-check-unfollow-prerequisites"}] in policy [.siem-signals-default]
[00:00:04]               │ info [o.e.x.i.IndexLifecycleTransition] [node-01] moving index [.siem-signals-default-000001] from [{"phase":"hot","action":"unfollow","name":"branch-check-unfollow-prerequisites"}] to [{"phase":"hot","action":"rollover","name":"check-rollover-ready"}] in policy [.siem-signals-default]
[00:00:07]               │ proc [kibana] [2021-10-13T08:38:36.976+00:00][INFO ][plugins.eventLog] event logged: {"@timestamp":"2021-10-13T08:38:36.975Z","event":{"provider":"alerting","action":"execute-start","kind":"alert","category":["siem"],"start":"2021-10-13T08:38:36.975Z"},"kibana":{"saved_objects":[{"rel":"primary","type":"alert","id":"f31933a0-2c00-11ec-a1f3-abd14cfeac1c","type_id":"siem.signals"}],"task":{"scheduled":"2021-10-13T08:38:34.191Z","schedule_delay":2784000000},"server_uuid":"5b2de169-2785-441b-ae8c-186a1936b17d","version":"8.0.0"},"rule":{"id":"f31933a0-2c00-11ec-a1f3-abd14cfeac1c","license":"basic","category":"siem.signals","ruleset":"siem"},"message":"alert execution start: \"f31933a0-2c00-11ec-a1f3-abd14cfeac1c\"","ecs":{"version":"1.8.0"}}
[00:00:09]               │ proc [kibana] [2021-10-13T08:38:39.248+00:00][INFO ][plugins.securitySolution] [+] Finished indexing 4  signals searched between date ranges [
[00:00:09]               │ proc [kibana]   {
[00:00:09]               │ proc [kibana]     "to": "2021-10-13T08:38:38.262Z",
[00:00:09]               │ proc [kibana]     "from": "1900-01-01T00:00:00.000Z",
[00:00:09]               │ proc [kibana]     "maxSignals": 100
[00:00:09]               │ proc [kibana]   }
[00:00:09]               │ proc [kibana] ] name: "Signal Testing Query" id: "f31933a0-2c00-11ec-a1f3-abd14cfeac1c" rule id: "rule-1" signals index: ".siem-signals-default"
[00:00:09]               │ proc [kibana] [2021-10-13T08:38:39.267+00:00][INFO ][plugins.eventLog] event logged: {"@timestamp":"2021-10-13T08:38:36.975Z","event":{"provider":"alerting","action":"execute","kind":"alert","category":["siem"],"start":"2021-10-13T08:38:36.975Z","outcome":"success","end":"2021-10-13T08:38:39.266Z","duration":2291000000},"kibana":{"saved_objects":[{"rel":"primary","type":"alert","id":"f31933a0-2c00-11ec-a1f3-abd14cfeac1c","type_id":"siem.signals"}],"task":{"scheduled":"2021-10-13T08:38:34.191Z","schedule_delay":2784000000},"alerting":{"status":"ok"},"server_uuid":"5b2de169-2785-441b-ae8c-186a1936b17d","version":"8.0.0"},"rule":{"id":"f31933a0-2c00-11ec-a1f3-abd14cfeac1c","license":"basic","category":"siem.signals","ruleset":"siem","name":"Signal Testing Query"},"message":"alert executed: siem.signals:f31933a0-2c00-11ec-a1f3-abd14cfeac1c: 'Signal Testing Query'","ecs":{"version":"1.8.0"}}
[00:00:09]               └- ✖ fail: detection engine api security and spaces enabled  Tests involving aliases of source indexes and the signals index should keep the original alias value such as "host_alias" from a source index when the value is indexed
[00:00:09]               │       Error: expected [ 'host name 2',
[00:00:09]               │   'host name 3',
[00:00:09]               │   'host name 4',
[00:00:09]               │   'host name 1' ] to sort of equal [ 'host name 1',
[00:00:09]               │   'host name 2',
[00:00:09]               │   'host name 3',
[00:00:09]               │   'host name 4' ]
[00:00:09]               │       + expected - actual
[00:00:09]               │ 
[00:00:09]               │        [
[00:00:09]               │       +  "host name 1"
[00:00:09]               │          "host name 2"
[00:00:09]               │          "host name 3"
[00:00:09]               │          "host name 4"
[00:00:09]               │       -  "host name 1"
[00:00:09]               │        ]
[00:00:09]               │       
[00:00:09]               │       at Assertion.assert (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:100:11)
[00:00:09]               │       at Assertion.eql (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:244:8)
[00:00:09]               │       at Context.<anonymous> (test/detection_engine_api_integration/security_and_spaces/tests/aliases.ts:57:23)
[00:00:09]               │       at runMicrotasks (<anonymous>)
[00:00:09]               │       at processTicksAndRejections (internal/process/task_queues.js:95:5)
[00:00:09]               │       at Object.apply (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/test/target_node/functional_test_runner/lib/mocha/wrap_function.js:87:16)
[00:00:09]               │ 
[00:00:09]               │

Stack Trace

Error: expected [ 'host name 2',
  'host name 3',
  'host name 4',
  'host name 1' ] to sort of equal [ 'host name 1',
  'host name 2',
  'host name 3',
  'host name 4' ]
    at Assertion.assert (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:100:11)
    at Assertion.eql (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/expect/expect.js:244:8)
    at Context.<anonymous> (test/detection_engine_api_integration/security_and_spaces/tests/aliases.ts:57:23)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at Object.apply (/dev/shm/workspace/parallel/20/kibana/node_modules/@kbn/test/target_node/functional_test_runner/lib/mocha/wrap_function.js:87:16) {
  actual: '[\n  "host name 2"\n  "host name 3"\n  "host name 4"\n  "host name 1"\n]',
  expected: '[\n  "host name 1"\n  "host name 2"\n  "host name 3"\n  "host name 4"\n]',
  showDiff: true
}

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	2749	2751	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	4.6MB	4.6MB	+2.8KB

History

💚 Build #159524 succeeded 912dac3
💔 Build #158819 failed 79d0255
💔 Build #158813 failed 1c8e2f2
💔 Build #158529 failed b725008
💔 Build #158560 failed 4eabfd6

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

…elastic#114126) * Adds new card design to event filters and also adds comments list * Adds nested comments * Hides comments if there are no commentes * Fixes i18n check error because duplicated key * Fix wrong type and unit test * Fixes ts error * Address pr comments and fix unit tests Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-10-13T10:07:03Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

…#114126) (#114772) * Adds new card design to event filters and also adds comments list * Adds nested comments * Hides comments if there are no commentes * Fixes i18n check error because duplicated key * Fix wrong type and unit test * Fixes ts error * Address pr comments and fix unit tests Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: David Sánchez <davidsansol92@gmail.com>

…ide-users-to-saving-ux * 'master' of github.com:elastic/kibana: (133 commits) [DOCS] Indicate reports are a subscription feature (elastic#114653) Update namespace for indices (elastic#114612) [DOCS] Adds Logstash pipeline settings (elastic#114648) Bump EPR snapshot version used for tests (elastic#114529) [Security Solution] [Endpoint] Fleet summary card adjustments (elastic#114291) skip flaky suite (elastic#68400) [Visualizations] fix usage of optional dependencies (elastic#114286) [Security Solution] [Detections] Improves custom query rule upgrade test (elastic#114454) [fleet] Add Integration Preference selector (elastic#114432) [Reporting] Add new `data-render-error` attribute (elastic#114472) Replace EuiCodeEditor with CodeEditor in app-services code (elastic#114316) [data views] add getDefaultDataView method (elastic#113891) [Security Solution] [Endpoint] Event filters uses the new card design (elastic#114126) [fleet] Tweak Header UI (elastic#114704) [APM] Filter on tx metrics for instance stats (elastic#114758) [APM] Fix typo in linting docs (elastic#114764) [Discover] Removing SavedObject usage for savedSearch (elastic#112983) [Fleet] Add Integration Policy Page Improvements (elastic#114556) [Lens] Keep the custom label when transitioning to/from Formula (elastic#114270) [Security Solution][Endpoint] Host Isolation API changes (elastic#113621) ...

dasansol92 added 3 commits October 5, 2021 15:56

Adds new card design to event filters and also adds comments list

de0275d

Adds nested comments

af18a02

Hides comments if there are no commentes

2249554

dasansol92 added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 labels Oct 6, 2021

dasansol92 commented Oct 6, 2021

View reviewed changes

dasansol92 added 3 commits October 6, 2021 17:27

Fixes i18n check error because duplicated key

b725008

Fix wrong type and unit test

c48852a

Fixes ts error

4eabfd6

paul-tavares reviewed Oct 6, 2021

View reviewed changes

Address pr comments and fix unit tests

1c8e2f2

Merge branch 'master' into feat/olm-event_filters_uses_the_new_card_d…

79d0255

…esign-1767

dasansol92 requested a review from paul-tavares October 7, 2021 10:33

dasansol92 marked this pull request as ready for review October 7, 2021 10:33

dasansol92 requested a review from a team as a code owner October 7, 2021 10:33

dasansol92 requested a review from ashokaditya October 7, 2021 10:33

paul-tavares approved these changes Oct 11, 2021

View reviewed changes

Merge branch 'master' into feat/olm-event_filters_uses_the_new_card_d…

912dac3

…esign-1767

kevinlog approved these changes Oct 12, 2021

View reviewed changes

Merge branch 'master' into feat/olm-event_filters_uses_the_new_card_d…

00d1632

…esign-1767

dasansol92 enabled auto-merge (squash) October 13, 2021 08:01

dasansol92 merged commit 0bf0b94 into elastic:master Oct 13, 2021

kibanamachine mentioned this pull request Oct 13, 2021

[7.x] [Security Solution] [Endpoint] Event filters uses the new card design (#114126) #114772

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] [Endpoint] Event filters uses the new card design #114126

[Security Solution] [Endpoint] Event filters uses the new card design #114126

dasansol92 commented Oct 6, 2021 •

edited

Loading

dasansol92 Oct 6, 2021

paul-tavares Oct 6, 2021

paul-tavares left a comment

paul-tavares Oct 6, 2021

dasansol92 Oct 7, 2021

paul-tavares Oct 6, 2021

dasansol92 Oct 7, 2021

paul-tavares Oct 6, 2021

paul-tavares Oct 6, 2021

paul-tavares Oct 6, 2021

dasansol92 Oct 7, 2021

paul-tavares Oct 6, 2021

dasansol92 Oct 7, 2021

dasansol92 commented Oct 7, 2021

elasticmachine commented Oct 7, 2021

paul-tavares left a comment

paul-tavares Oct 11, 2021

paul-tavares Oct 11, 2021

paul-tavares commented Oct 11, 2021

kevinlog commented Oct 12, 2021

kibanamachine commented Oct 13, 2021

Standard Out

Stack Trace

kibanamachine commented Oct 13, 2021

[Security Solution] [Endpoint] Event filters uses the new card design #114126

[Security Solution] [Endpoint] Event filters uses the new card design #114126

Conversation

dasansol92 commented Oct 6, 2021 • edited Loading

Summary

For maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares left a 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

dasansol92 commented Oct 7, 2021

elasticmachine commented Oct 7, 2021

paul-tavares left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paul-tavares commented Oct 11, 2021

kevinlog commented Oct 12, 2021

kibanamachine commented Oct 13, 2021

💛 Build succeeded, but was flaky

Standard Out

Stack Trace

Metrics [docs]

Module Count

Async chunks

History

kibanamachine commented Oct 13, 2021

💚 Backport successful

dasansol92 commented Oct 6, 2021 •

edited

Loading