elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.65k stars 8.23k forks source link

[Cases] Custom fields: List type #176491

Open cnasikas opened 9 months ago

cnasikas commented 9 months ago

Summary

In issue https://github.com/elastic/kibana/issues/168378 research was done on how to support the list custom field type in cases. We should follow the research issue to implement the list custom field.

DoD

Out of scope

Research issue: https://github.com/elastic/kibana/issues/168378

elasticmachine commented 9 months ago

Pinging @elastic/response-ops (Team:ResponseOps)

elasticmachine commented 9 months ago

Pinging @elastic/response-ops-cases (Feature:Cases)

Zacqary commented 1 month ago

Working on implementing this with the schema of value, text instead of key, label, in order to match EuiSelectOption schema. This simplifies things so that we don't have to parse and convert all over the place.

cnasikas commented 1 month ago

I would advise keeping the key, label to be consistent with the schema we have so far with custom fields. The value, text seems like an implementation detail of the UI which is not related to a consumer of our APIs. Also, the parse and conversion will have to be moved to the SO level to be able to have one query for all custom fields. Wdyt?

Zacqary commented 1 month ago

From the research issue:

"cases": {
  "customFields": [
    { "type": "list", "key": "department.engineering", "value": ["Super Awesome Eng"] } 
    { "type": "list", "key": "department.sales", "value": ["Sales"] }
  ]
}

This helps us easily display the correct label text in the UI, but it won't correctly update the label if the user changes it in the field configuration.

For example:

  1. User creates a list field with the values Hello, World
  2. Custom field configuration generates uuid keys for each of these values: 00000: Hello, 11111: World
  3. User creates a case and sets this field to World
  4. User updates the list field configuration and changes World to List, schema is now 11111: List

With the research issue's implementation, we would still be displaying World on all pre-existing cases until the user edits them.

View components and getEuiTableColumn functions are only ever passed the exact schema of the customField, but for a List field type we'd really want to be querying the options for whatever the current defined label is.

We can conceivably hack View to do this because it's always wrapped in Edit, which has access to the custom field configuration. getEuiTableColumn does NOT have access to the custom field configuration, though.

cnasikas commented 1 month ago

Hey @Zacqary! Great question. If you see this issue in the "Out of scope" section, renaming will not supported. So feel free to ignore this scenario at the moment. In the research issue, the "B. Option Renaming" section describes how we want to do renaming in the future. While we can show the configuration's value on the UI this will be misleading as the data will be different and filtering or sorting will not work as expected (users will see a different value in the UI than the actual data where the filtering or sorting is performed in the cases table).

Zacqary commented 1 month ago

@cnasikas Got it, sorry I was so focused on reading the research issue I completely missed the out of scope section for this one

Zacqary commented 1 month ago

Actually implementing this schema is proving problematic:

 "customFields": [
    { "type": "list", "key": "department.engineering", "value": ["Super Awesome Eng"] } 
    { "type": "list", "key": "department.sales", "value": ["Sales"] }
  ]

There's several layers of APIs in both the form library and the SavedObject parsing libraries that result in:

However, I've managed to get this schema working perfectly:

 "customFields": [
    { "type": "list", "key": "department", "value": ["engineering", "Super Awesome Eng"] } 
    { "type": "list", "key": "department", "value": ["sales", "Sales"] }
  ]

There's an open question in the research issue asking if we need to set both the key and the label as the value, and so far that seems to be the most workable change.

@cnasikas Do you think the above schema (key is the custom field key, value is an array of [key, value]) fits our future needs for searching and sorting?

Zacqary commented 1 month ago

As per Zoom discussion with @cnasikas, I was able to transform requests from the UI to successfully save a schema of:

 { "type": "list", "key": "department.engineering", "value": ["Super Awesome Eng"] } 

The problem I'm now facing is that none of the rest of the existing custom field logic is able to handle dot notation in keys.

In working on implementing this, I've realized it's problematic to save the selected label in the SavedObject at all, and that it's going to be an enormous implementation headache that will create a massive amount of extra stuff to maintain going forward, and it will actively make it more difficult to handle renaming of options in the future.

I'm going to work on implementing the following schema:

 { "type": "list", "key": "department", "value": "engineering" } 

and to add an additional layer on the UI to pull the current label from the custom field's configuration for display. This will give us renaming of labels for free, and we may be able to use a similar function to convert a key to a label to handle alphabetical sorting.

cnasikas commented 1 month ago

@Zacqary About the following schema:

 "type": "list", "key": "department", "value": "engineering" } 

We thought about it when we researched but it cannot work with searching and sorting. Imagine that the keys can be anything like abc8f (users can also set their own as they like) unrelated to the value. Sorting by having the key as the value will result in wrong sorting. Unfortunately, we have to put the value in the cases SO. We discussed it a lot when we did the research and we concluded that this is the only way. We wish we could avoid it as it creates issues with renaming etc.

I was able to transform requests from the UI to successfully save a schema of { "type": "list", "key": "department.engineering", "value": ["Super Awesome Eng"] }

The transformation of this schema needs to happen on the backend before storing it in the SO. I would suggest the following:

  1. In the UI, using the serializer, transform the schema from the UI schema to the API schema which should be { "type": "list", "key": "department", "value": "Super Awesome Eng" }. In the research issue, we had the value as an array because we thought the list type would accept multiple values. If we want to be the future proof we can have it as an array. If not we can leave it as a single string. I really liked your suggestion in our call so it can also be { "type": "list", "key": "department", "value": { "engineering": "Super Awesome Eng" } }. This way you also have access to the key and you can validate easily. The transformation should happen in createFormSerializer used in x-pack/plugins/cases/public/components/create/form_context.tsx.
  2. Perform validations against the configuration on the cases client. For example, if the value is valid etc.
  3. Transform the API schema to the SO schema which should be { "type": "list", "key": "department.engineering", "value": ["Super Awesome Eng"] }. The key is constructed as ${parentKey}.${optionKey}. The transformation should happen in the Cases Service (x-pack/plugins/cases/server/services/cases/index.ts) in the transformAttributesToESModel functions.

On the way back it will be the opposite. transformSavedObjectToExternalModel from SO schema to API schema and createFormDeserializer from the API schema to the UI schema.

the problem I'm now facing is that none of the rest of the existing custom field logic is able to handle dot notation in keys.

Only the Cases Service is aware of the dot notation. The cases client, the APIs, and the UI should not be aware of this implementation detail (dot notation) and I think all the issues you may face will be eliminated.