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.

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

[Cases] Custom fields: List type #176491

Open
cnasikas opened this issue Feb 8, 2024 · 10 comments · May be fixed by #194236
Open

[Cases] Custom fields: List type #176491

cnasikas opened this issue Feb 8, 2024 · 10 comments · May be fixed by #194236
Assignees
Labels
Feature:Cases Cases feature Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@cnasikas
Copy link
Member

cnasikas commented Feb 8, 2024

Summary

In issue #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

  • Users can configure the custom field of type single-list. Users can define a list of available options a user can select. The field can be required and a default value can be selected.
  • Only a single value can be selected at a time.
  • Users can delete available list options when configuring the custom field.
  • Users can filter based on the configured options of the list type (stretch goal).

Out of scope

  • Sorting
  • Renaming of options

Research issue: #168378

@cnasikas cnasikas added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature labels Feb 8, 2024
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@Zacqary Zacqary self-assigned this Sep 19, 2024
@Zacqary
Copy link
Contributor

Zacqary commented Sep 23, 2024

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
Copy link
Member Author

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
Copy link
Contributor

Zacqary commented Sep 24, 2024

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
Copy link
Member Author

cnasikas commented Sep 25, 2024

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
Copy link
Contributor

Zacqary commented Sep 25, 2024

@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
Copy link
Contributor

Zacqary commented Sep 25, 2024

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:

  • An API call attempting to save something like key: "department", value: { sales: "Sales" }, which could theoretically work, except
  • This throws some kind of very deep SavedObject parsing error that I had a difficult time tracking down

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
Copy link
Contributor

Zacqary commented Sep 26, 2024

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.

  • Default values are assigned based on which keys are missing, so we need to add another transformation layer in fillMissingCustomFields that only applies to the List type
  • Custom fields are displayed in the UI based on their exact configured key and don't parse dot notation, so we need to add another transform layer
  • The schema in the research issue is actually invalid for this issue because selecting multiple values is not permitted. Having department.engineering and department.sales in the same customFields array creates a conflict, and we'd need to validate against that

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.

@Zacqary Zacqary linked a pull request Sep 26, 2024 that will close this issue
2 tasks
@cnasikas
Copy link
Member Author

cnasikas commented Sep 30, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Cases Cases feature Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants