[Security solution] Attack Discovery "View in AI Assistant" button fix#192416
Merged
stephmilovic merged 4 commits intoelastic:mainfrom Sep 12, 2024
Merged
[Security solution] Attack Discovery "View in AI Assistant" button fix#192416stephmilovic merged 4 commits intoelastic:mainfrom
stephmilovic merged 4 commits intoelastic:mainfrom
Conversation
stephmilovic
added
release_note:fix
Team: SecuritySolution
Contributor
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
spong
reviewed
Sep 11, 2024
Comment on lines
+156
to
+158
| } catch (e) { | ||
| /* empty */ | ||
| } |
Member
There was a problem hiding this comment.
Was curious about the error behavior here so tested a bit and all looks good. 👍 If thrown, createConversation will throw up a toast, and the assistant flyout will open to the lastConversationId with the AttackDiscovery context available.
spong
reviewed
Sep 11, 2024
...lic/attack_discovery/attack_discovery_panel/view_in_ai_assistant/use_view_in_ai_assistant.ts
Outdated
Show resolved
Hide resolved
spong
approved these changes
Sep 11, 2024
Member
spong
left a comment
spong
left a comment
There was a problem hiding this comment.
Checked out, tested locally, and code reviewed -- LGTM! 👍
Appreciate the additional tests and thorough description around the ways this could've been addressed and your logic in providing the fix. Thanks @stephmilovic! 🎉
Contributor
Author
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Sep 12, 2024
elastic#192416) (cherry picked from commit ea6bb9e)
Contributor
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Sep 12, 2024
…t" button fix (#192416) (#192709) # Backport This will backport the following commits from `main` to `8.15`: - [[Security solution] Attack Discovery "View in AI Assistant" button fix (#192416)](#192416) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Steph Milovic","email":"stephanie.milovic@elastic.co"},"sourceCommit":{"committedDate":"2024-09-12T14:35:44Z","message":"[Security solution] Attack Discovery \"View in AI Assistant\" button fix (#192416)","sha":"ea6bb9e0b76088cf08d2786e653d73bee5152dd3","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team: SecuritySolution","Team:Security Generative AI","v8.16.0","v8.15.2"],"title":"[Security solution] Attack Discovery \"View in AI Assistant\" button fix","number":192416,"url":"https://github.com/elastic/kibana/pull/192416","mergeCommit":{"message":"[Security solution] Attack Discovery \"View in AI Assistant\" button fix (#192416)","sha":"ea6bb9e0b76088cf08d2786e653d73bee5152dd3"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192416","number":192416,"mergeCommit":{"message":"[Security solution] Attack Discovery \"View in AI Assistant\" button fix (#192416)","sha":"ea6bb9e0b76088cf08d2786e653d73bee5152dd3"}},{"branch":"8.15","label":"v8.15.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Steph Milovic <stephanie.milovic@elastic.co>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In #190151 I inadvertently broke the Attack Discovery "View in AI Assistant" button. In the past, clicking this button would open a conversation titled whatever the discovery was titled. Now in main, clicking this button will add the context to welcome convo.
I had tried to keep calls to
createConversationinuseCurrentConversationonly. I eliminated the need for it elsewhere in cases of default conversations. However, for new custom conversations created viauseAssistantOverlaywe still need thecreateConversation. I tried to approach it fromuseCurrentConversation, however there was no way for me to determine if an undefined conversation title is coming from something like the "View In AI Assistant" button or from thelastConversationIdin local storage (valid case where a convo gets deleted, and we do not want to re-create it because of an undefined conversation title). Therefore, I decided to bringcreateConversationback touseAssistantOverlay. To prevent from calling this when we do not intend, I added a conditionshouldCreateConversationthat explicitly tells theshowOverlaycall to create a new conversation. So far the only place whereshouldCreateConversation = trueis coming fromuseViewInAiAssistant(this already had the second boolean passed toshowOverlay, I think a stale property)Additional consideration
Well... what if a month from now I have a discovery named the same thing as another one I've already opened a conversation for? The old conversation will load with the new context. Sounds confusing. To prevent this, I added the last 5 digits of the UUID of the attack discovery id to the title.
To test