Skip to content

[Incident Management] Render suggested dashboards#223424

Merged
cesco-f merged 20 commits intoelastic:mainfrom
cesco-f:render-suggested-dashboards
Jun 13, 2025
Merged

[Incident Management] Render suggested dashboards#223424
cesco-f merged 20 commits intoelastic:mainfrom
cesco-f:render-suggested-dashboards

Conversation

@cesco-f
Copy link
Contributor

@cesco-f cesco-f commented Jun 11, 2025

This PR closes #221947.

Screen.Recording.2025-06-11.at.17.12.26.mov

The last part of the video is me trying to find the telemetry event fired when clicking the button.

Acceptance Criteria:

  • The list should appear on the Related Dashboards tab, under the Linked Dashboards ✅
  • Users should have the ability to add or "promote" a suggested dashboard to the list of linked dashboards stored on this rule, in a single click ✅
  • We are collecting telemetry so that we know how many times users click the button to promote a suggested dashboard to a linked dashboard ✅

@cesco-f cesco-f added release_note:feature Makes this part of the condensed release notes backport:version Backport to applied version labels v9.1.0 v8.19.0 labels Jun 11, 2025
@github-actions github-actions bot added the author:obs-ux-management PRs authored by the obs ux management team label Jun 11, 2025
@cesco-f cesco-f force-pushed the render-suggested-dashboards branch 2 times, most recently from 89bc9fa to 9738339 Compare June 12, 2025 09:40
@cesco-f cesco-f marked this pull request as ready for review June 12, 2025 10:01
@cesco-f cesco-f requested a review from a team as a code owner June 12, 2025 10:01
@botelastic botelastic bot added the Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. label Jun 12, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@cesco-f cesco-f force-pushed the render-suggested-dashboards branch 2 times, most recently from 0616232 to 1c0f8c3 Compare June 12, 2025 14:11
cesco-f and others added 17 commits June 13, 2025 09:06
@mgiota
Copy link
Contributor

mgiota commented Jun 13, 2025

Hey @mgiota, it's fixed already here 😊

Yep thanks a lot for this! I am doing the code review now and I see you already fixed it! Cool!

if (alertDetail) {
setRuleTypeModel(ruleTypeRegistry.get(alertDetail?.formatted.fields[ALERT_RULE_TYPE_ID]!));
setAlertStatus(alertDetail?.formatted?.fields[ALERT_STATUS] as AlertStatus);
setActiveTabId(OVERVIEW_TAB_ID);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this removal exactly do? Why did you remove it? Does it fix something specifically?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also curious about this change. This line was added to make sure overview tab is selected when user opens alert details page from "Related alerts" tab.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With that the tabId query parameter in the url is ignored, the user will always land in the overview tab.

Is that the expected behavior?

Copy link
Contributor

@baileycash-elastic baileycash-elastic Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cesco-f I noticed this as well while working on my PR and changed the line to be

    if (passedTab === OVERVIEW_TAB_ID || passedTab === null) setActiveTabId(OVERVIEW_TAB_ID);

Granted passedTab is a new value added as part of my PR but it's a way to formally track the url state.

}
}, [onPageReady, alertDetail, isLoading, activeTabId]);

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see all this code being removed in favor of the new api!

const tabs: Array<Omit<EuiTabbedContentTab, 'id'> & { id: TabId }> = [
{
id: OVERVIEW_TAB_ID,
id: 'overview',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any strong objection, but why did you replace the consts?

Copy link
Contributor Author

@cesco-f cesco-f Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this change whenever we wanted to add a new tab we had to:

  1. Create the constant
  2. Use the constant to change the TabId type
  3. Add the constant to the array we use to set the activeTabId

With my change by just adding the new tab string to the TAB_IDS array everything else will be done automatically.

},
]
: []),
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we were not showing the tab if there were no linked dashboards. With this change, I understand we will always show the tab, independent if we have related dashboards or not. Do we handle the empty state and what do we display in this case? I am half way through the review, so I might get the answer!

ruleType: string;
}

export function DashboardTile({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it DashboardTile or maybe you meant to write DashboardTitle?

Copy link
Contributor Author

@cesco-f cesco-f Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I meant to write DashboardTile 😂

Is it not clear what a tile is in this case? I used this naming a lot in my previous job, maybe it's not that clear.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance I thought it might refer to the title only, but it is clear. Just wanted to double check

return wrapWithHeader(
<EuiText>
{i18n.translate('xpack.observability.relatedDashboards.noDashboardsTextLabel', {
defaultMessage: 'No dashboards',
Copy link
Contributor

@mgiota mgiota Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to my comment above, I see you display the text No dashboards, which is fine for now I guess.

Similar to this issue, I think we need design input from @maciejforcone regarding Related Dashboards empty state.

@mgiota
Copy link
Contributor

mgiota commented Jun 13, 2025

@cesco-f Great work! I tested the feature and it works well! Other than a few comments/questions, I don't have anything blocking this PR.

Maybe I would like us to get @maciejforcone 's input regarding empty state as a follow up PR (similar to the empty state issue for Investigation guide). Here's how it looks right now. Do we want to describe the feature better? We could for example have a notification saying you can edit the rule to manually link dashboards. What do you think?

Screenshot 2025-06-13 at 14 36 10

Copy link
Contributor

@mgiota mgiota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@cesco-f cesco-f enabled auto-merge (squash) June 13, 2025 13:57
@cesco-f cesco-f merged commit 123091c into elastic:main Jun 13, 2025
10 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/15637044307

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
observability 1305 1308 +3

Async chunks

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

id before after diff
observability 1.3MB 1.3MB +3.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
observability 93.5KB 93.7KB +120.0B

History

@cesco-f cesco-f deleted the render-suggested-dashboards branch June 13, 2025 14:34
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.19 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.19:
- [Incident Management] [Related dashboards api] Handle missing lens attributes gracefully (#223619)
- [Incident Management] [Suggested dashboards] Deduplicated linked dashboards from list of linked dashboards (#221972)

Manual backport

To create the backport manually run:

node scripts/backport --pr 223424

Questions ?

Please refer to the Backport tool documentation

iblancof pushed a commit to iblancof/kibana that referenced this pull request Jun 16, 2025
This PR closes elastic#221947.



https://github.com/user-attachments/assets/25beac10-5677-42ef-9544-b3ede0bf9fa1

The last part of the video is me trying to find the telemetry event
fired when clicking the button.

**Acceptance Criteria:**

- The list should appear on the Related Dashboards tab, under the Linked
Dashboards ✅
- Users should have the ability to add or "promote" a suggested
dashboard to the list of linked dashboards stored on this rule, in a
single click ✅
- We are collecting telemetry so that we know how many times users click
the button to promote a suggested dashboard to a linked dashboard ✅

---------

Co-authored-by: Dominique Belcher <dominique.clarke@elastic.co>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 17, 2025
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 223424 locally
cc: @cesco-f

@cesco-f
Copy link
Contributor Author

cesco-f commented Jun 17, 2025

💚 All backports created successfully

Status Branch Result
8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

cesco-f added a commit that referenced this pull request Jun 18, 2025
…224298)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[Incident Management] Render suggested dashboards
(#223424)](#223424)

<!--- Backport version: 10.0.1 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Francesco
Fagnani","email":"fagnani.francesco@gmail.com"},"sourceCommit":{"committedDate":"2025-06-13T14:31:21Z","message":"[Incident
Management] Render suggested dashboards (#223424)\n\nThis PR closes
#221947.\n\n\n\nhttps://github.com/user-attachments/assets/25beac10-5677-42ef-9544-b3ede0bf9fa1\n\nThe
last part of the video is me trying to find the telemetry event\nfired
when clicking the button.\n\n**Acceptance Criteria:**\n\n- The list
should appear on the Related Dashboards tab, under the
Linked\nDashboards ✅\n- Users should have the ability to add or
\"promote\" a suggested\ndashboard to the list of linked dashboards
stored on this rule, in a\nsingle click ✅\n- We are collecting telemetry
so that we know how many times users click\nthe button to promote a
suggested dashboard to a linked dashboard
✅\n\n---------\n\nCo-authored-by: Dominique Belcher
<dominique.clarke@elastic.co>","sha":"123091cb8344b27b576b9e534a92b5a7afd94b70","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport
missing","release_note:feature","Team:obs-ux-management","backport:version","v9.1.0","v8.19.0","author:obs-ux-management"],"title":"[Incident
Management] Render suggested
dashboards","number":223424,"url":"https://github.com/elastic/kibana/pull/223424","mergeCommit":{"message":"[Incident
Management] Render suggested dashboards (#223424)\n\nThis PR closes
#221947.\n\n\n\nhttps://github.com/user-attachments/assets/25beac10-5677-42ef-9544-b3ede0bf9fa1\n\nThe
last part of the video is me trying to find the telemetry event\nfired
when clicking the button.\n\n**Acceptance Criteria:**\n\n- The list
should appear on the Related Dashboards tab, under the
Linked\nDashboards ✅\n- Users should have the ability to add or
\"promote\" a suggested\ndashboard to the list of linked dashboards
stored on this rule, in a\nsingle click ✅\n- We are collecting telemetry
so that we know how many times users click\nthe button to promote a
suggested dashboard to a linked dashboard
✅\n\n---------\n\nCo-authored-by: Dominique Belcher
<dominique.clarke@elastic.co>","sha":"123091cb8344b27b576b9e534a92b5a7afd94b70"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/223424","number":223424,"mergeCommit":{"message":"[Incident
Management] Render suggested dashboards (#223424)\n\nThis PR closes
#221947.\n\n\n\nhttps://github.com/user-attachments/assets/25beac10-5677-42ef-9544-b3ede0bf9fa1\n\nThe
last part of the video is me trying to find the telemetry event\nfired
when clicking the button.\n\n**Acceptance Criteria:**\n\n- The list
should appear on the Related Dashboards tab, under the
Linked\nDashboards ✅\n- Users should have the ability to add or
\"promote\" a suggested\ndashboard to the list of linked dashboards
stored on this rule, in a\nsingle click ✅\n- We are collecting telemetry
so that we know how many times users click\nthe button to promote a
suggested dashboard to a linked dashboard
✅\n\n---------\n\nCo-authored-by: Dominique Belcher
<dominique.clarke@elastic.co>","sha":"123091cb8344b27b576b9e534a92b5a7afd94b70"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Dominique Belcher <dominique.clarke@elastic.co>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author:obs-ux-management PRs authored by the obs ux management team backport:version Backport to applied version labels release_note:feature Makes this part of the condensed release notes Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Incident Management] [Alert Details] [Suggested Dashboards] Render suggested dashboard list on page

7 participants