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.

Sign up for GitHub

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

Jump to bottom

[Cloud Security] Fix first item flyout wont open in vuln table #165214

Merged
merged 1 commit into from
Aug 30, 2023
Merged

[Cloud Security] Fix first item flyout wont open in vuln table #165214

merged 1 commit into from
Aug 30, 2023

Conversation

JordanSh
Copy link
Contributor

@JordanSh JordanSh commented Aug 30, 2023 •
edited by kibanamachine
Loading

Reverted the logic change for the function that opens the flyout

Screen.Recording.2023-08-30.at.14.07.55.mov

@JordanSh JordanSh added release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related v8.10.0 labels Aug 30, 2023
@JordanSh JordanSh self-assigned this Aug 30, 2023
@JordanSh JordanSh requested a review from a team as a code owner August 30, 2023 11:08
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

@JordanSh JordanSh requested a review from maxcold August 30, 2023 11:08
@kibana-ci
Copy link
Collaborator

💚 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
cloudSecurityPosture 264.1KB 264.1KB -9.0B

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

cc @JordanSh

maxcold
maxcold approved these changes Aug 30, 2023
View reviewed changes
Copy link
Contributor

@maxcold maxcold left a comment

Choose a reason for hiding this comment

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

Tested, it fixes the problem. @JordanSh what was the problem you were trying to solve with this change yesterday btw?

@JordanSh
Copy link
Contributor Author

JordanSh commented Aug 30, 2023 •
edited
Loading

Tested, it fixes the problem. @JordanSh what was the problem you were trying to solve with this change yesterday btw?

flyoutVulnerabilityIndex can technically be undefined, we never had a problem with that but its getting added manually to the urlQuery. when i added types, i marked it as optional. so i checked that flyoutVulnerabilityIndex exists before using it. i later removed this type since i discovered that we also use another custom parameter in the vuln table. and as i mentioned we have 6 tables in total and didn't want to make any big changes in that fix.

problem is that the 0 is also a falsy type and thats why the flyout didnt open. the most correct solution would be

const showVulnerabilityFlyout = flyoutVulnerabilityIndex !== undefined
    ? flyoutVulnerabilityIndex > invalidIndex
    : undefined;

but again, its a fix that meant to be backported. i prefer it to be as slim as possible and since the current implementation was sitting in our code forever and didnt made problems i prefer to just revert it to what it was

@JordanSh JordanSh merged commit b88547c into elastic:main Aug 30, 2023
@kibanamachine kibanamachine mentioned this pull request Aug 30, 2023
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 30, 2023
@JordanSh
[Cloud Security] Fix first item flyout wont open in vuln table (elast…
13c9d37 
…ic#165214)

(cherry picked from commit b88547c)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.10

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 Aug 30, 2023
@kibanamachine @JordanSh
[8.10] [Cloud Security] Fix first item flyout wont open in vuln table (…
67f0f33 
…#165214) (#165236)

# Backport

This will backport the following commits from `main` to `8.10`:
- [[Cloud Security] Fix first item flyout wont open in vuln table
(#165214)](#165214)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT
[{"author":{"name":"Jordan","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-30T13:14:58Z","message":"[Cloud
Security] Fix first item flyout wont open in vuln table
(#165214)","sha":"b88547c323fb97dac9e08c4ccdf12ac4e5f0899d","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Cloud
Security","v8.10.0","v8.11.0"],"number":165214,"url":"https://github.com/elastic/kibana/pull/165214","mergeCommit":{"message":"[Cloud
Security] Fix first item flyout wont open in vuln table
(#165214)","sha":"b88547c323fb97dac9e08c4ccdf12ac4e5f0899d"}},"sourceBranch":"main","suggestedTargetBranches":["8.10"],"targetPullRequestStates":[{"branch":"8.10","label":"v8.10.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/165214","number":165214,"mergeCommit":{"message":"[Cloud
Security] Fix first item flyout wont open in vuln table
(#165214)","sha":"b88547c323fb97dac9e08c4ccdf12ac4e5f0899d"}}]}]
BACKPORT-->

Co-authored-by: Jordan <[email protected]>
This was referenced Oct 1, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxcold maxcold maxcold approved these changes

Assignees

@JordanSh JordanSh

Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Cloud Security Cloud Security team related v8.10.0 v8.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JordanSh @elasticmachine @kibana-ci @kibanamachine @maxcold