Replace empty registry value name in FIM inventory #3279
Merged
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
All the tables in the Integrity monitoring section that show an empty field now display a label with an icon noting them as empty, to help distinguish them from whitespace strings and to improve the look.
mpRegalado
force-pushed
the
enhancement/replace-empty-registry-value_3220
branch
from
9616105 to
3803ee9
Compare
gabiwassan
suggested changes
May 21, 2021
Comment on lines
1
to
33
| /* | ||
| * Wazuh app - Integrity monitoring components | ||
| * Copyright (C) 2015-2021 Wazuh, Inc. | ||
| * | ||
| * This program is free software; you can redistribute it and/or modify | ||
| * it under the terms of the GNU General Public License as published by | ||
| * the Free Software Foundation; either version 2 of the License, or | ||
| * (at your option) any later version. | ||
| * | ||
| * Find more information about this on the LICENSE file. | ||
| */ | ||
|
|
||
| import { EuiCode, EuiIcon } from '@elastic/eui' | ||
| import React from 'react' | ||
|
|
||
| /* This function can be used to render possibly empty fields. | ||
| It takes a render function suitable for an EuiTable and returns another. */ | ||
| export const emptyFieldHandler = (renderFn = (value,record) => value) => { | ||
| return (value, record) => { | ||
| if (value === "" || value === undefined) { | ||
| return ( | ||
| <> | ||
| <EuiIcon type="iInCircle"/> | ||
| <EuiCode> | ||
| Empty field | ||
| </EuiCode> | ||
| </> | ||
| ) | ||
| } else { | ||
| return renderFn(value,record); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
- The name of this file should be with kebab-case:
empty-field-handler.js(if possible we have legacy files with camel case) - We use 2 spaces on code style. Maybe you are not using
prettieron your IDE.
Desvelao
reviewed
May 21, 2021
| @@ -15,6 +15,7 @@ import { WzRequest } from '../../../../../react-services'; | |||
| import React, { useEffect, useState } from 'react'; | |||
| import valuesMock from './values.json'; | |||
| import { DIRECTIONS } from '@elastic/eui/src/components/flex/flex_group'; | |||
| import { emptyFieldHandler } from '../lib' | |||
Desvelao
reviewed
May 21, 2021
| }), | ||
| 'syscheck.value_name': (content, rowData, element, options) => { | ||
| if (content) return; | ||
| const container = document.createElement("tr"); |
There was a problem hiding this comment.
why is the container an tr HTML element?
There was a problem hiding this comment.
It was meant to be a td element to match the rest of the cells in the table. Fixed in 97821ec
Desvelao
requested changes
May 21, 2021
Cell in the table should be contained by a "td" tag not a "tr"
Desvelao
reviewed
May 24, 2021
| }), | ||
| 'syscheck.value_name': (content, rowData, element, options) => { | ||
| if (content) return; | ||
| const container = document.createElement("td"); |
There was a problem hiding this comment.
Maybe a div or span instead of an HTML related to a table. The td/tr elements are set by the table itself.
I need to redo the test to verify it.
There was a problem hiding this comment.
Just checked and you were right, replaced the element with a span
pablomarga
approved these changes
May 26, 2021
There was a problem hiding this comment.
CR: ✔️
Testing: ✔️ it works fine but as @Desvelao commented when you reduce the window the cell looks a little strange.
frankeros
suggested changes
May 26, 2021
frankeros
suggested changes
May 26, 2021
| }, | ||
| { | ||
| field: 'value', | ||
| name: 'Value name', | ||
| sortable: true, | ||
| render: (item) => item.name, | ||
| render: emptyFieldHandler((item) => item.name), |
There was a problem hiding this comment.
remove please emptyFieldHandler from all but this.
we'll only notify this field when is empty.
…ped values and not the root object.
frankeros
approved these changes
May 27, 2021
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.
Closes #3220
Presented empty fields with a notice
Added a function that returns a render method compatible with EuiColumn objects that, if the object is either an empty string or undefined, shows a label stating as such (for now only in Value Name on Registry details).
Also replicated the same functionality for the
syscheck.value_namein the FIM Discovery dashboard to address the situation in the issue #3220Testing
Create a enviroment with at least 1 windows agent with
syscheckenabledpublic/kibana-integrations/discover/application/components/table/table.tsxwithvalue={""}will empty all the information displayed in all the extended rows of FIM discovery, checking thesyscheck.value_namefield should show the label.Screenshots
