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

CellActions component should hide ShowTopN action for nested fields #150347

Closed
machadoum opened this issue Feb 6, 2023 · 3 comments · Fixed by #157834
Closed

CellActions component should hide ShowTopN action for nested fields #150347

machadoum opened this issue Feb 6, 2023 · 3 comments · Fixed by #157834
Assignees
@machadoum @semd
Labels
bug Fixes for quality problems that affect the customer experience Feature:Cell Actions Security Solution Cell Actions feature fixed Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team

Comments

@machadoum
Copy link
Member

machadoum commented Feb 6, 2023 •
edited
Loading

Lens does not support nested fields

Screenshot 2023-02-06 at 16 47 19

Lens only supports esType: keyword, and non-nested field. We have to add additional checks for esTypes and nested fields.

The major challenge here is that CellActions API currently doesn't support field?.subType.

Nested field example:
Screenshot 2023-02-06 at 17 00 05

Screenshot 2023-02-06 at 17 09 55
@machadoum machadoum self-assigned this Feb 6, 2023
@machadoum machadoum added Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore labels Feb 6, 2023
@machadoum machadoum mentioned this issue Feb 6, 2023
Merged
2 tasks
@semd
Copy link
Contributor

semd commented Feb 8, 2023 •
edited
Loading

Adding properties on demand to the field type may not be the best solution, every time we add a property we have to refactor all the places the CellActions are used to pass it.
Could it be a better solution to use the FieldSpec type from DataView as the field type?

kibana/src/plugins/data_views/common/types.ts

Line 380 in ebc6750

export type FieldSpec = DataViewFieldBase & {

It is a pretty standard type used by DataView and it has a very similar definition to Security's BrowserFields.
We could have the following CellActionsExecutionContext type:

{
   field: FieldSpec,
   value: string | string[] | null | undefined,
   [...] 
}

This way actions will have the flexibility to check any field information available. We'll have to adapt all places that use CellActions, but we'll have to do it only once.

@YulNaumenko YulNaumenko mentioned this issue Feb 9, 2023
Closed
29 tasks
@semd
Copy link
Contributor

semd commented Mar 6, 2023

Added a link to an issue where the Filter actions are not working because the field is not mapped in the index.
After we adopt the FieldSpec for the field type of the actions we'll be able to check the isMapped flag in the action's isCompatible to hide the FilterIn/Out actions.

@stephmilovic stephmilovic added the bug Fixes for quality problems that affect the customer experience label Apr 6, 2023
@semd semd mentioned this issue Apr 11, 2023
Open
@machadoum machadoum added the Feature:Cell Actions Security Solution Cell Actions feature label May 2, 2023
This was referenced May 10, 2023
Closed
Merged
machadoum added a commit that referenced this issue Jun 22, 2023
@machadoum
[Security Solution] Update CellActions field type to be FieldSpec #15…
5fb9709 
…7243 (#157834)

issue: #150347

## Context
Some Actions need to access `FieldSpec` data, which is not present on
the `CellActions` API (`subType`and `isMapped`). So we are updating the
`CellActions` `field` property to be compatible with `FieldSpec`.

## Summary

This PR is the first step to fix
#150347.
* Updates the `CellActions` to support an array of data objects, each
containing field (`FieldSpec`) and value
* Create a new `SecurityCellActions` component that accepts a field name
and loads `FieldSpec` from the Dataview.

## Examples
Before: 
```tsx
 <SecurityCellActions
      value={'admin'}
      field={{
        name: 'user.name',
        type: 'keyword',
        searchable: true,
        aggregatable: true,
        ...
      }} />
```
After:
```tsx
 <SecurityCellActions data={{ field: 'user.name', value: 'admin' }}/>
```
`SecurityCellActions` will load the spec from the Dataview and provide
it to `CellActons`.

`CellActons` now also support an of fields instead of only one field. It
will be useful when rendering cell actions for aggregated data like on
the Entity Analytic page. But for now, the actions are not supporting
multiple values, we need to rewrite them
#159480.

### Next steps
We must refactor the Security Solution to get `FieldSpec` from the
`DataView` instead of using BrowserFields. Ideally, we have to do that
for every `CellAction` call so the actions can access the `subType`
property.
- [x] ~Refactor the Security Solution CellActions calls to get
`FieldSpec` from the `DataView`~
- [x] Refactor data grid cell actions to get `FieldSpec` from the
`DataView`
- [ ] Rewrite actions to support multiple fields and use them on the
investigation in timeline (.andFilters)
- [ ] Fix #150347 using
`subType` from `fieldSpec`
- [ ] Fix #154714 using
`isMapped` from `fieldSpec`

### Extra information
*** Previously we were mixing `esTypes` and `kbnTypes`. For example, if
the `esType` is a keyword the `kbnType` has to be a `string`.

[Here](https://github.com/machadoum/kibana/blob/9799dbba27c5baf594357eae0bbfc79b4e7da77c/packages/kbn-field-types/src/types.ts#L22)
you can check all possible ES and KBN types and
[here](https://github.com/machadoum/kibana/blob/9799dbba27c5baf594357eae0bbfc79b4e7da77c/packages/kbn-field-types/src/kbn_field_types_factory.ts)
you can see the mapping between esType and kbnType


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
@machadoum
Copy link
Member Author

auto-closed by mistake

@machadoum machadoum reopened this Jun 23, 2023
@machadoum machadoum mentioned this issue Jun 30, 2023
Merged
1 task
machadoum added a commit that referenced this issue Jul 5, 2023
@machadoum
[Security Solutions] Fix CellActions component should hide ShowTopN a…
7be72ef 
…ction for nested fields (#159645)

issue: #150347

## Summary
CellActions component should hide `ShowTopN` action for nested fields
because Lens doesn't support it.

* Add validations to exclude field that are not supported by Lens

**After:**
<img width="1533" alt="Screenshot 2023-06-30 at 14 26 09"
src="https://github.com/elastic/kibana/assets/1490444/e3318e05-6a07-44e6-9eb8-861670ecb161">


### How to test
* Go to Security Solutions Alerts page
* Add `Endpoint.policy.applied.artifacts.global.identifiers.name` field
* Check if `ShowTopN` is displayed

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
@semd semd closed this as completed Jul 13, 2023
@semd semd added the fixed label Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@machadoum machadoum

@semd semd

Labels
bug Fixes for quality problems that affect the customer experience Feature:Cell Actions Security Solution Cell Actions feature fixed Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team
Projects
None yet
Milestone
No milestone
3 participants
@machadoum @stephmilovic @semd