[SecuritySolution] Replace dashboard listing with Kibana dashboard listing #155716

angorayc · 2023-04-25T14:07:47Z

Summary

1. Header styles fitted into SecuritySolution's layout.
2. Allow for Non-Removable Security Solution Tags in Portable Dashboard Listing Page

For example here are all the dashboards and attached tags:

Before:
https://kibana.siem.estc.dev/s/dashboard-test/app/security/dashboards?sourcerer=(default:(id:security-solution-dashboard-test,selectedPatterns:!(%27auditbeat-*%27,%27filebeat-*%27,%27logs-*%27,%27packetbeat-*%27,%27winlogbeat-*%27)))

After:

Screen.Recording.2023-05-18.at.15.37.34.mov

This is the dashboards and tags I used to test. Please download it and rename txt to ndjson and import them from
stack management > saved objects

export.txt

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

…-ref HEAD~1..HEAD --fix'

…ashboard-listing

…-ref HEAD~1..HEAD --fix'

…ashboard-listing

ThomThomson

Looked through the code and tested this locally by making sure that the Dashboard Listing page still works the same way, and by checking out the design of the Security dashboard listing page. Everything LGTM from the Presentation team side of things! Left a few followup comments and nits.

Also a note, I created a dashboard from the top right link in the security solution and it wasn't automatically assigned the security-solution tag at first. As I tried again a few more times it started being assigned consistently. Is that functionality flaky? I tested in 8.7 and when I navigated via the security solution create dashboard link the new dashboard was automatically assigned the security-solution tag every time.

x-pack/plugins/security_solution/public/dashboards/pages/landing_page/index.tsx

ThomThomson · 2023-05-16T21:59:52Z

packages/content-management/table_list/src/components/tag_filter_panel.tsx

@@ -143,7 +152,7 @@ export const TagFilterPanel: FC<Props> = ({
          renderOption={(option) => option.view}
          emptyMessage="There aren't any tags"
          noMatchesMessage="No tag matches the search"
-          onChange={onSelectChange}
+          onChange={!disableActions ? onSelectChange : noop}


Maybe as a future enhancement here instead of disabling the actions entirely, we could treat the fixedTag as a special entry, not allowing it to be de-selected, but allowing all other tag systems to work.

Screen.Recording.2023-05-18.at.15.37.34.mov

Yup, updated, rest of the tags are still listed and working.

…fix'

…ashboard-listing

elasticmachine · 2023-05-18T14:41:34Z

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

elasticmachine · 2023-05-18T14:41:36Z

Pinging @elastic/security-solution (Team: SecuritySolution)

…fix'

sebelga · 2023-05-18T16:08:20Z

packages/content-management/table_list/src/table_list_view.tsx

-            clearTagSelection={clearTagSelection}
+            tableSort={tableSort}
+            tagsToTableItemMap={tagsToTableItemMap}
+            fixedTagReferences={fixedTagReferences}


It seems that you wanted to sort the props by alphabetic order.. but added the new prop at the end 😊 Is that correct? Is there any new prop to the component?

IMO it would be easier to review if prop order was not changed (here or in the Props interface)

Sorry for the confusion, fixedTagReferences is the new prop.

…ashboard-listing

…fix'

…ashboard-listing

…fix'

kibana-ci · 2023-05-19T12:07:36Z

💚 Build Succeeded

Buildkite Build
Commit: 3b84127

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3873	3869	-4

Async chunks

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

id	before	after	diff
`dashboard`	436.4KB	437.8KB	+1.4KB
`filesManagement`	87.6KB	88.7KB	+1.1KB
`graph`	453.9KB	455.0KB	+1.1KB
`maps`	2.8MB	2.8MB	+1.1KB
`securitySolution`	9.2MB	9.2MB	-1.4KB
`visualizations`	260.2KB	261.3KB	+1.1KB
total			+4.2KB

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`securitySolution`	400	404	+4
total			+6

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`securitySolution`	480	484	+4
total			+6

History

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

cc @angorayc

sebelga

I'm half way through the code review and I have some concerns with the UX of the feature.

Rendering a tag in the tag panel that is not actionable seems like a UX bug. The user will wonder "Why can't I unselect this tag?"

I think a better UI would be to:

filter out the "fixed" tags from the tag panel list.
not to fill the search bar with the "tag:("Security Solution")" text
hide the tags in the table row (here too, those tags are actionable but now don't do anything)

The prop to configure the table could be withTags

<DashboardListingTable
  withTags={{
    tags: [tag1, tag2],
    hideFromTable: true, // default "true"
    hideFromSearch: true, // default "true"
  }}
  disableCreateDashboardButton={loadingCreateDashboardUrl}
  getDashboardUrl={getSecuritySolutionDashboardUrl}
  ...

The <TableListView /> is already a bloated component (I know, it should be refactored). I wouldn't just push another prop to block behaviour but think how a nice UX would be if it is used in a new context.

Here what we really want is a table with specific items that can't be removed and we use the tag mechanism for this filtering. Let's hide our internals (the "Security Solution" tag) from the user. 😊

sebelga · 2023-05-22T14:14:35Z

packages/content-management/table_list/src/components/use_tag_filter_panel.tsx

@@ -53,21 +64,31 @@ export const useTagFilterPanel = ({
  const [isInUse, setIsInUse] = useState(false);
  const [options, setOptions] = useState<TagOptionItem[]>([]);
  const [tagSelection, setTagSelection] = useState<TagSelection>({});
-  const totalActiveFilters = Object.keys(tagSelection).length;
+  const totalActiveFilters = useMemo(
+    () => getTotalActiveFilters(options, fixedTagReferences),


Why do we pass options and not tagSelection ? I would also pass an array if fixedTagReferences is undefined or null to make getTotalActiveFilters simpler

const getTotalActiveFilters = ( tagSelection: TagSelection, fixedTagReferences: TagReference[] ) => Math.max( fixedTagReferences.length, Object.keys(tagSelection).length ); const totalActiveFilters = useMemo( () => getTotalActiveFilters(tagSelection, fixedTagReferences ?? []), [fixedTagReferences, tagSelection] ); ...

It could really just be this

const totalActiveFilters = useMemo(() => ( Math.max((fixedTagReferences ?? []).length, Object.keys(tagSelection).length) )), [fixedTagReferences, tagSelection] ); ...

As the tagSelection seemed to have problem displaying the actual count when a few tags sharing the same name.

stephmilovic

Nice work, excellent job on test coverage. Desk testing went perfectly, LGTM. Thanks @angorayc

angorayc · 2023-05-23T14:45:36Z

I'm half way through the code review and I have some concerns with the UX of the feature.

Rendering a tag in the tag panel that is not actionable seems like a UX bug. The user will wonder "Why can't I unselect this tag?"

Thanks for your review. As the listing is used under Security Solution, so it'd make more sense if users couldn't remove the default selected Security Solution tag.

I think a better UI would be to:

filter out the "fixed" tags from the tag panel list.

not to fill the search bar with the "tag:("Security Solution")" text

I tried the Kibana Dashboard listing and found the status filter and query were aligned, so I would leaning towards keeping this behaviour.

hide the tags in the table row (here too, those tags are actionable but now don't do anything)

Given that we have default selected tags, I'd like to keep the tags in each row so users could understand how the filter affects the table. If the tag is one of the unremovable tags, I'd disable it's filter behaviour.

The prop to configure the table could be withTags
<DashboardListingTable
  withTags={{
    tags: [tag1, tag2],
    hideFromTable: true, // default "true"
    hideFromSearch: true, // default "true"
  }}
  disableCreateDashboardButton={loadingCreateDashboardUrl}
  getDashboardUrl={getSecuritySolutionDashboardUrl}
  ...
The <TableListView /> is already a bloated component (I know, it should be refactored). I wouldn't just push another prop to block behaviour but think how a nice UX would be if it is used in a new context.

Sure, I'll rename the prop to withTags.

Here what we really want is a table with specific items that can't be removed and we use the tag mechanism for this filtering. Let's hide our internals (the "Security Solution" tag) from the user. 😊

angorayc · 2023-06-22T15:08:07Z

After talking to the team, we decide to give the non-removable tag request. What we will do is to select Security Solution tag by default but users are still able to remove the selection. #160540

replace dashboard listing

cbeb700

This was referenced Apr 25, 2023

[Portable Dashboard] Allow for Non-Removable Tags in Portable Dashboard Listing Page #155089

Closed

[Security Solution] Create Dashboard view page #152955

Closed

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

ff32ec5

…-ref HEAD~1..HEAD --fix'

angorayc changed the title ~~replace dashboard listing~~ [SecuritySolution] Replace dashboard listing with Kibana dashboard listing Apr 25, 2023

angorayc self-assigned this Apr 25, 2023

angorayc added Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore v8.9.0 labels Apr 25, 2023

angorayc and others added 10 commits April 25, 2023 15:30

styling

576ec4a

Merge branch 'dashboard-listing' of github.com:angorayc/kibana into d…

29f2a65

…ashboard-listing

add fixed tag

2fbf9b8

Merge remote-tracking branch 'upstream/main' into dashboard-listing

7f33c9b

disable actions

6fa35ce

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

6b1d0b8

…-ref HEAD~1..HEAD --fix'

types

6a5a791

Merge branch 'dashboard-listing' of github.com:angorayc/kibana into d…

a9a63a5

…ashboard-listing

rm default value

6f04f3b

rm default value

0f04412

ThomThomson approved these changes May 16, 2023

View reviewed changes

angorayc and others added 7 commits May 17, 2023 12:34

Merge branch 'main' of github.com:elastic/kibana into dashboard-listing

205a5dc

update tags selection behaviour

498ae01

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

f92bca0

…fix'

Merge branch 'main' of github.com:elastic/kibana into dashboard-listing

c961de4

Merge branch 'dashboard-listing' of github.com:angorayc/kibana into d…

55df917

…ashboard-listing

unit tests

fe9d202

unit tests

b743ea0

angorayc marked this pull request as ready for review May 18, 2023 14:41

angorayc requested a review from a team as a code owner May 18, 2023 14:41

angorayc requested review from a team as code owners May 18, 2023 14:41

ThomThomson approved these changes May 18, 2023

View reviewed changes

angorayc added the release_note:enhancement label May 18, 2023

angorayc and others added 3 commits May 18, 2023 15:56

fix selection

eb0b111

fix types

47ca980

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

770cfee

…fix'

jbudz approved these changes May 18, 2023

View reviewed changes

sebelga reviewed May 18, 2023

View reviewed changes

angorayc and others added 6 commits May 18, 2023 17:18

fix types

b797fc0

Merge branch 'dashboard-listing' of github.com:angorayc/kibana into d…

23728b5

…ashboard-listing

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

de5584b

…fix'

fix types

3bbac62

Merge branch 'dashboard-listing' of github.com:angorayc/kibana into d…

9581c92

…ashboard-listing

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

3b84127

…fix'

sebelga requested changes May 22, 2023

View reviewed changes

stephmilovic approved these changes May 22, 2023

View reviewed changes

angorayc closed this May 30, 2023

sebelga mentioned this pull request Jun 20, 2023

[TableListView] Upcoming requirements #159995

Open

angorayc added v8.10.0 backport:skip This commit does not require backporting and removed v8.9.0 labels Jun 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SecuritySolution] Replace dashboard listing with Kibana dashboard listing #155716

[SecuritySolution] Replace dashboard listing with Kibana dashboard listing #155716

angorayc commented Apr 25, 2023 •

edited

Loading

ThomThomson left a comment •

edited

Loading

ThomThomson May 16, 2023

angorayc May 18, 2023

angorayc May 18, 2023

elasticmachine commented May 18, 2023

elasticmachine commented May 18, 2023

sebelga May 18, 2023

sebelga May 18, 2023 •

edited

Loading

angorayc May 18, 2023

kibana-ci commented May 19, 2023

ESLint disabled line counts

Total ESLint disabled count

sebelga left a comment

sebelga May 22, 2023

sebelga May 22, 2023

angorayc May 23, 2023

angorayc May 23, 2023

stephmilovic left a comment

angorayc commented May 23, 2023

angorayc commented Jun 22, 2023 •

edited

Loading

[SecuritySolution] Replace dashboard listing with Kibana dashboard listing #155716

[SecuritySolution] Replace dashboard listing with Kibana dashboard listing #155716

Conversation

angorayc commented Apr 25, 2023 • edited Loading

Summary

Checklist

ThomThomson left a comment • edited Loading

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

elasticmachine commented May 18, 2023

elasticmachine commented May 18, 2023

Choose a reason for hiding this comment

sebelga May 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented May 19, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

sebelga 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

stephmilovic left a comment

Choose a reason for hiding this comment

angorayc commented May 23, 2023

angorayc commented Jun 22, 2023 • edited Loading

angorayc commented Apr 25, 2023 •

edited

Loading

ThomThomson left a comment •

edited

Loading

sebelga May 18, 2023 •

edited

Loading

angorayc commented Jun 22, 2023 •

edited

Loading