Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SecuritySolution] Fix issue with duplicate timeline reloading #198652

Merged

Conversation

janmonschke
Copy link
Contributor

@janmonschke janmonschke commented Nov 1, 2024

Summary

Fixes: #173240

Unsaved duplicated timelines will now show the save notification when the user is trying to leave security solution without saving.

The PR also re-enables the unsaved_timelines cypress tests.

Screen.Recording.2024-11-01.at.20.15.36.mov

Checklist

@janmonschke janmonschke added release_note:fix Team:Threat Hunting:Investigations Security Solution Investigations Team backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 1, 2024
@janmonschke janmonschke marked this pull request as ready for review November 1, 2024 19:09
@janmonschke janmonschke requested a review from a team as a code owner November 1, 2024 19:09
@elasticmachine
Copy link
Contributor

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

@janmonschke
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
securitySolution 21.0MB 21.0MB -20.0B

History

Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

the code looks good, but I looked at the ticket and this does not really fix that specific issue (though is related). I think the behavior mentioned in the ticket was fixed between December 2023 and now, as I cannot reproduce on main.

Would you be ok changing the fixes to related to in the PR description to be super specific?

Also I feel like it would be safer to run the flaky test runner for the file that re-enables the Cypress tests? Wdyt?

@janmonschke
Copy link
Contributor Author

janmonschke commented Nov 5, 2024

Flaky test runner (❌ )

@PhilippeOberti yep, there was some confusion there because the original ticket wasn't closed and this fix should've been its own ticket. More context: #173240 (comment)

@PhilippeOberti
Copy link
Contributor

Flaky test runner (⌛)

The flaky test runner is all red, but it seems that it's because of issues unrelated to any actual tests. Can you run it again?

@janmonschke janmonschke added this to the 8.17 milestone Nov 5, 2024
@janmonschke janmonschke requested a review from a team as a code owner November 5, 2024 15:41
@janmonschke
Copy link
Contributor Author

janmonschke commented Nov 5, 2024

Flaky Test runner (2) [🟢 ]

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7340

[✅] Security Solution Investigations - Cypress: 25/25 tests passed.
[✅] [Serverless] Security Solution Investigations - Cypress: 25/25 tests passed.

see run history

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #6 / Users filter should search comma separated strings as multiple users
  • [job] [logs] Jest Tests #6 / Users filter should search on given search string on enter

Metrics [docs]

Async chunks

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

id before after diff
securitySolution 21.0MB 21.0MB -20.0B

History

@janmonschke janmonschke removed the request for review from a team November 5, 2024 19:17
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#7332

[❌] Security Solution Investigations - Cypress: 0/25 tests passed.
[❌] [Serverless] Security Solution Investigations - Cypress: 0/25 tests passed.

see run history

@PhilippeOberti
Copy link
Contributor

Approved even if the last flaky test runner failed. It failed for a different reason

@janmonschke janmonschke merged commit bde5e11 into elastic:main Nov 6, 2024
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 198652

Questions ?

Please refer to the Backport tool documentation

@janmonschke
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

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

Questions ?

Please refer to the Backport tool documentation

janmonschke added a commit to janmonschke/kibana that referenced this pull request Nov 6, 2024
…ic#198652)

Fixes: elastic#173240

Unsaved duplicated timelines will now show the save notification when
the user is trying to leave security solution without saving.

The PR also re-enables the `unsaved_timelines` cypress tests.

https://github.com/user-attachments/assets/429d23fc-0cd5-4f0a-bcc9-0fd025856b6e

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit bde5e11)
janmonschke added a commit that referenced this pull request Nov 6, 2024
…198652) (#199144)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[SecuritySolution] Fix issue with duplicate timeline reloading
(#198652)](#198652)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Jan
Monschke","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-06T07:55:19Z","message":"[SecuritySolution]
Fix issue with duplicate timeline reloading (#198652)\n\n##
Summary\r\n\r\nFixes:
https://github.com/elastic/kibana/issues/173240\r\n\r\nUnsaved
duplicated timelines will now show the save notification when\r\nthe
user is trying to leave security solution without saving.\r\n\r\nThe PR
also re-enables the `unsaved_timelines` cypress
tests.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/429d23fc-0cd5-4f0a-bcc9-0fd025856b6e\r\n\r\n\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"bde5e11526c222dd0697dbeb092b8f53dbd2c859","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:Threat
Hunting:Investigations","backport:prev-minor"],"number":198652,"url":"https://github.com/elastic/kibana/pull/198652","mergeCommit":{"message":"[SecuritySolution]
Fix issue with duplicate timeline reloading (#198652)\n\n##
Summary\r\n\r\nFixes:
https://github.com/elastic/kibana/issues/173240\r\n\r\nUnsaved
duplicated timelines will now show the save notification when\r\nthe
user is trying to leave security solution without saving.\r\n\r\nThe PR
also re-enables the `unsaved_timelines` cypress
tests.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/429d23fc-0cd5-4f0a-bcc9-0fd025856b6e\r\n\r\n\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"bde5e11526c222dd0697dbeb092b8f53dbd2c859"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198652","number":198652,"mergeCommit":{"message":"[SecuritySolution]
Fix issue with duplicate timeline reloading (#198652)\n\n##
Summary\r\n\r\nFixes:
https://github.com/elastic/kibana/issues/173240\r\n\r\nUnsaved
duplicated timelines will now show the save notification when\r\nthe
user is trying to leave security solution without saving.\r\n\r\nThe PR
also re-enables the `unsaved_timelines` cypress
tests.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/429d23fc-0cd5-4f0a-bcc9-0fd025856b6e\r\n\r\n\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"bde5e11526c222dd0697dbeb092b8f53dbd2c859"}}]}]
BACKPORT-->
mgadewoll pushed a commit to mgadewoll/kibana that referenced this pull request Nov 7, 2024
…ic#198652)

## Summary

Fixes: elastic#173240

Unsaved duplicated timelines will now show the save notification when
the user is trying to leave security solution without saving.

The PR also re-enables the `unsaved_timelines` cypress tests.


https://github.com/user-attachments/assets/429d23fc-0cd5-4f0a-bcc9-0fd025856b6e




### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:fix Team:Threat Hunting:Investigations Security Solution Investigations Team v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security Solution]unsaved changes tag should appear on duplicating timeline
4 participants