[Cloud Security]Added Support to handle Labels on Unified Data Table #184295
Conversation
This file contains 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
|
/ci |
Closed
5 tasks
|
/ci |
|
/ci |
opauloh
requested changes
May 27, 2024
Comment on lines
139
to
146
| const columnNameWithoutLabel = | ||
| columnName === '_source' | ||
| ? i18n.translate('unifiedDataTable.grid.documentHeader', { | ||
| defaultMessage: 'Document', | ||
| }) | ||
| : dataViewField?.displayName || columnName; | ||
|
|
||
| const columnDisplayName = columnHeaders ? columnHeaders[columnName] : columnNameWithoutLabel; |
There was a problem hiding this comment.
I believe we can move that logic to a function now:
const getColumnDisplayName = (columnName, dataViewField, columnHeaders) => {
if (columnHeaders?.[columnName]) {
return columnHeaders[columnName];
}
if (columnName === '_source') {
return i18n.translate('unifiedDataTable.grid.documentHeader', {
defaultMessage: 'Document',
});
}
return dataViewField?.displayName || columnName;
};This way we declare more the intent of columnHeaders taking precedence of dataViewField.displayName.
and then you can use it the following way:
const columnDisplayName = getColumnDisplayName(columnName, dataViewField, columnHeaders);
packages/kbn-unified-data-table/src/components/data_table_columns.tsx
Outdated
Show resolved
Hide resolved
packages/kbn-unified-data-table/src/components/data_table_column_header.tsx
Show resolved
Hide resolved
x-pack/plugins/cloud_security_posture/public/pages/table_field_labels.ts
Outdated
Show resolved
Hide resolved
|
We should also add this logic to the unit tests of the unified datatable component |
|
/ci |
|
/ci |
|
Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security) |
animehart
added
v8.15.0
release_note:skip
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
|
/ci |
opauloh
reviewed
May 28, 2024
packages/kbn-unified-data-table/src/components/data_table.test.tsx
Outdated
Show resolved
Hide resolved
kfirpeled
reviewed
May 28, 2024
davismcphee
reviewed
May 29, 2024
There was a problem hiding this comment.
In general the code changes look good, but I wonder if we could achieve this by modifying one of the existing props instead of introducing a new columnHeaders prop, especially where the Unified Data Table API is already so sprawling.
These existing props are used for similar purposes and maybe one of them could be extended to support this:
columnsMeta- Currently used for customizing a columntypeandesType, could be extended to support something likedisplayName.settings- Currently used to set column widths, could also be extended to support something likedisplayName.customGridColumnsConfiguration- Allows customizingEuiDataGridColumnitems before they're passed to the data grid. This one already supports customizing the header display, but might not work for this use case since the display name is used elsewhere too.
WDYT? Also I'm not very familiar with Findings so I wasn't really sure how to test this, any tips on how to test in my dev env?
Comment on lines
770
to
774
| const getColumnHeaders = () => | ||
| screen | ||
| .getAllByRole('columnheader') | ||
| .map((header) => header.querySelector('.euiDataGridHeaderCell__content')?.textContent); | ||
|
|
There was a problem hiding this comment.
It looks like this can probably be reverted since it's the only change in the file now.
There was a problem hiding this comment.
oh woops, yea this one I originally added a unit test here that was using this function so moved this function to be global in this file. But then I moved the test to some other file (where its more appropriate) and I seem to not have revert this correctly
There was a problem hiding this comment.
Regarding your first comment I will try to extend existing props then instead adding new props to UnifiedDataTable
2 tasks
animehart
added
backport
and removed
backport:skip
animehart
added
v8.13.5
v8.14.2
backport:prev-minor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
animehart
added
backport:prev-minor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
animehart
added
backport:prev-minor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
animehart
added a commit
that referenced
this pull request
Jun 18, 2024
… Table (#184295) (#186346) # Backport This will backport the following commits from `main` to `8.14`: - [[Cloud Security]Added Support to handle Labels on Unified Data Table (#184295)](#184295) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Rickyanto Ang","email":"rickyangwyn@gmail.com"},"sourceCommit":{"committedDate":"2024-05-30T18:38:25Z","message":"[Cloud Security]Added Support to handle Labels on Unified Data Table (#184295)\n\n## Summary\r\n\r\nAdded Support to handle Labels on Unified Data Table. Previously\r\nFindings and Vulnerabilities table columns shows Labels by using the\r\ncustomLabel field in Dataview and we did this by Updating the Dataview\r\nwhen we first retrieve the Dataview. However, we have removed the Update\r\nlogic, as such we would need a different way to do this which is this PR\r\n\r\nWe used the **settings** prop on UnifiedDataTable to pass on the Column\r\nLabels that will be shown in Findings/Vulnerabilities table","sha":"1bd6c24ba6e04c5c9dab1c80d7f2d8e84d43ca59","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:DataDiscovery","Team:Cloud Security","backport:prev-minor","v8.15.0","v8.14.2"],"number":184295,"url":"#184295 Security]Added Support to handle Labels on Unified Data Table (#184295)\n\n## Summary\r\n\r\nAdded Support to handle Labels on Unified Data Table. Previously\r\nFindings and Vulnerabilities table columns shows Labels by using the\r\ncustomLabel field in Dataview and we did this by Updating the Dataview\r\nwhen we first retrieve the Dataview. However, we have removed the Update\r\nlogic, as such we would need a different way to do this which is this PR\r\n\r\nWe used the **settings** prop on UnifiedDataTable to pass on the Column\r\nLabels that will be shown in Findings/Vulnerabilities table","sha":"1bd6c24ba6e04c5c9dab1c80d7f2d8e84d43ca59"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"main","label":"v8.15.0","labelRegex":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#184295 Security]Added Support to handle Labels on Unified Data Table (#184295)\n\n## Summary\r\n\r\nAdded Support to handle Labels on Unified Data Table. Previously\r\nFindings and Vulnerabilities table columns shows Labels by using the\r\ncustomLabel field in Dataview and we did this by Updating the Dataview\r\nwhen we first retrieve the Dataview. However, we have removed the Update\r\nlogic, as such we would need a different way to do this which is this PR\r\n\r\nWe used the **settings** prop on UnifiedDataTable to pass on the Column\r\nLabels that will be shown in Findings/Vulnerabilities table","sha":"1bd6c24ba6e04c5c9dab1c80d7f2d8e84d43ca59"}},{"branch":"8.14","label":"v8.14.2","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: Kfir Peled <61654899+kfirpeled@users.noreply.github.com>
animehart
added a commit
that referenced
this pull request
Jun 20, 2024
…ble (#186425) ## Summary <img width="1478" alt="Screenshot 2024-06-18 at 6 10 05 PM" src="https://github.com/elastic/kibana/assets/8703149/f095591d-f0ee-41bd-8b7d-07880bcf61d9"> Currently we have an issue where if user already has localStorageKey from previous version where we still use Update for our Column Label and then proceed to upgrading to version where we no longer use that, the column name in Findings table will show field name (it shows resource.id instead of Resource ID) also because we changed the logic and not allow users to change the column headers in the data grid, option to **edit data view field** is removed for Cloud Security Table <img width="741" alt="Screenshot 2024-06-19 at 9 16 06 AM" src="https://github.com/elastic/kibana/assets/8703149/df1ec765-89de-4f43-a723-daf9558af135"> This patch fixes that issues Related to #184295
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Jun 20, 2024
…ble (elastic#186425) ## Summary <img width="1478" alt="Screenshot 2024-06-18 at 6 10 05 PM" src="https://github.com/elastic/kibana/assets/8703149/f095591d-f0ee-41bd-8b7d-07880bcf61d9"> Currently we have an issue where if user already has localStorageKey from previous version where we still use Update for our Column Label and then proceed to upgrading to version where we no longer use that, the column name in Findings table will show field name (it shows resource.id instead of Resource ID) also because we changed the logic and not allow users to change the column headers in the data grid, option to **edit data view field** is removed for Cloud Security Table <img width="741" alt="Screenshot 2024-06-19 at 9 16 06 AM" src="https://github.com/elastic/kibana/assets/8703149/df1ec765-89de-4f43-a723-daf9558af135"> This patch fixes that issues Related to elastic#184295 (cherry picked from commit 7304484)
kibanamachine
added a commit
that referenced
this pull request
Jun 20, 2024
…Data table (#186425) (#186545) # Backport This will backport the following commits from `main` to `8.14`: - [[Cloud Security] Patch fix for Column label on Cloud Security Data table (#186425)](#186425) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Rickyanto Ang","email":"rickyangwyn@gmail.com"},"sourceCommit":{"committedDate":"2024-06-20T15:30:19Z","message":"[Cloud Security] Patch fix for Column label on Cloud Security Data table (#186425)\n\n## Summary\r\n<img width=\"1478\" alt=\"Screenshot 2024-06-18 at 6 10 05 PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/8703149/f095591d-f0ee-41bd-8b7d-07880bcf61d9\">\r\n\r\n\r\nCurrently we have an issue where if user already has localStorageKey\r\nfrom previous version where we still use Update for our Column Label and\r\nthen proceed to upgrading to version where we no longer use that, the\r\ncolumn name in Findings table will show field name (it shows resource.id\r\ninstead of Resource ID)\r\n\r\nalso because we changed the logic and not allow users to change the\r\ncolumn headers in the data grid, option to **edit data view field** is\r\nremoved for Cloud Security Table\r\n<img width=\"741\" alt=\"Screenshot 2024-06-19 at 9 16 06 AM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/8703149/df1ec765-89de-4f43-a723-daf9558af135\">\r\n\r\n\r\nThis patch fixes that issues\r\n\r\nRelated to #184295","sha":"7304484cf4f160b92d477f85b59815d8d0c56986","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Cloud Security","backport:prev-minor","v8.15.0","v8.14.2"],"title":"[Cloud Security] Patch fix for Column label on Cloud Security Data table","number":186425,"url":"#186425 Security] Patch fix for Column label on Cloud Security Data table (#186425)\n\n## Summary\r\n<img width=\"1478\" alt=\"Screenshot 2024-06-18 at 6 10 05 PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/8703149/f095591d-f0ee-41bd-8b7d-07880bcf61d9\">\r\n\r\n\r\nCurrently we have an issue where if user already has localStorageKey\r\nfrom previous version where we still use Update for our Column Label and\r\nthen proceed to upgrading to version where we no longer use that, the\r\ncolumn name in Findings table will show field name (it shows resource.id\r\ninstead of Resource ID)\r\n\r\nalso because we changed the logic and not allow users to change the\r\ncolumn headers in the data grid, option to **edit data view field** is\r\nremoved for Cloud Security Table\r\n<img width=\"741\" alt=\"Screenshot 2024-06-19 at 9 16 06 AM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/8703149/df1ec765-89de-4f43-a723-daf9558af135\">\r\n\r\n\r\nThis patch fixes that issues\r\n\r\nRelated to #184295","sha":"7304484cf4f160b92d477f85b59815d8d0c56986"}},"sourceBranch":"main","suggestedTargetBranches":["8.14"],"targetPullRequestStates":[{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"#186425 Security] Patch fix for Column label on Cloud Security Data table (#186425)\n\n## Summary\r\n<img width=\"1478\" alt=\"Screenshot 2024-06-18 at 6 10 05 PM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/8703149/f095591d-f0ee-41bd-8b7d-07880bcf61d9\">\r\n\r\n\r\nCurrently we have an issue where if user already has localStorageKey\r\nfrom previous version where we still use Update for our Column Label and\r\nthen proceed to upgrading to version where we no longer use that, the\r\ncolumn name in Findings table will show field name (it shows resource.id\r\ninstead of Resource ID)\r\n\r\nalso because we changed the logic and not allow users to change the\r\ncolumn headers in the data grid, option to **edit data view field** is\r\nremoved for Cloud Security Table\r\n<img width=\"741\" alt=\"Screenshot 2024-06-19 at 9 16 06 AM\"\r\nsrc=\"https://github.com/elastic/kibana/assets/8703149/df1ec765-89de-4f43-a723-daf9558af135\">\r\n\r\n\r\nThis patch fixes that issues\r\n\r\nRelated to #184295","sha":"7304484cf4f160b92d477f85b59815d8d0c56986"}},{"branch":"8.14","label":"v8.14.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: Rickyanto Ang <rickyangwyn@gmail.com>
seanrathier
pushed a commit
to seanrathier/kibana
that referenced
this pull request
Jun 21, 2024
…ble (elastic#186425) ## Summary <img width="1478" alt="Screenshot 2024-06-18 at 6 10 05 PM" src="https://github.com/elastic/kibana/assets/8703149/f095591d-f0ee-41bd-8b7d-07880bcf61d9"> Currently we have an issue where if user already has localStorageKey from previous version where we still use Update for our Column Label and then proceed to upgrading to version where we no longer use that, the column name in Findings table will show field name (it shows resource.id instead of Resource ID) also because we changed the logic and not allow users to change the column headers in the data grid, option to **edit data view field** is removed for Cloud Security Table <img width="741" alt="Screenshot 2024-06-19 at 9 16 06 AM" src="https://github.com/elastic/kibana/assets/8703149/df1ec765-89de-4f43-a723-daf9558af135"> This patch fixes that issues Related to elastic#184295
bhapas
pushed a commit
to bhapas/kibana
that referenced
this pull request
Jun 24, 2024
…ble (elastic#186425) ## Summary <img width="1478" alt="Screenshot 2024-06-18 at 6 10 05 PM" src="https://github.com/elastic/kibana/assets/8703149/f095591d-f0ee-41bd-8b7d-07880bcf61d9"> Currently we have an issue where if user already has localStorageKey from previous version where we still use Update for our Column Label and then proceed to upgrading to version where we no longer use that, the column name in Findings table will show field name (it shows resource.id instead of Resource ID) also because we changed the logic and not allow users to change the column headers in the data grid, option to **edit data view field** is removed for Cloud Security Table <img width="741" alt="Screenshot 2024-06-19 at 9 16 06 AM" src="https://github.com/elastic/kibana/assets/8703149/df1ec765-89de-4f43-a723-daf9558af135"> This patch fixes that issues Related to elastic#184295
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Added Support to handle Labels on Unified Data Table. Previously Findings and Vulnerabilities table columns shows Labels by using the customLabel field in Dataview and we did this by Updating the Dataview when we first retrieve the Dataview. However, we have removed the Update logic, as such we would need a different way to do this which is this PR
We used the settings prop on UnifiedDataTable to pass on the Column Labels that will be shown in Findings/Vulnerabilities table