-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Security Solution][Endpoint][Response Actions] Show shell info above execute
action output
#154318
[Security Solution][Endpoint][Response Actions] Show shell info above execute
action output
#154318
Conversation
Pinging @elastic/security-defend-workflows (Team:Defend Workflows) |
So I first went with a version where the labels were not bold similar to other action outputs, but then I switched to the version where the output sections are a bit more legible in terms of contrast with bold. What do you all think? Here are screenshots without the strong labels. |
@ashokaditya - yeah, I'm not loving the display of it. It just looks like its part of download link information. It just all blends in. Its also pushing the output further down, which might not be a real concern in the console, but in the history log, the user does have limited view windows (expandable row) to see the content, so we're just forcing them to scroll can you try a few other options and post screen captures? some Ideas:
|
So we agreed in the grooming meeting to move the shell info into an accordion at the top and show it collapsed. Changes added in 9814303 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks and works great! 🚀
expect(renderResult.getByTestId(`test-executeResponseOutput-context`)).toBeTruthy(); | ||
expect(renderResult.getByTestId(`test-executeResponseOutput-shell`)).toBeTruthy(); | ||
expect(renderResult.getByTestId(`test-executeResponseOutput-cwd`)).toBeTruthy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: instead of toBeTruthy()
, it may be semantically better to use toBeInTheDocument()
. In this case, you can also use queryByX
instead of getByX
, so if the queried element is not present, the query function won't fail, but the toBeInTheDocument()
will.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I like that better. Will update it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done 177a31b
review changes @gergoabraham
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @ashokaditya |
Summary
Shows shell name, shell execution return code, and current working directory info along with the command execution output for
execute
action response.console
actions history
Checklist
Delete any items that are not applicable to this PR.