[Saved Object Management] Address v1 HTTP API TODOs#150892
[Saved Object Management] Address v1 HTTP API TODOs#150892jloleysens merged 8 commits intoelastic:mainfrom
v1 HTTP API TODOs#150892Conversation
…ve types directly
…d addressed find v1 todos
jloleysens
added
Team:Core
| const sortFieldSchema: Type<keyof v1.SavedObjectWithMetadata> = schema.oneOf([ | ||
| schema.literal('created_at'), | ||
| schema.literal('updated_at'), | ||
| schema.literal('type'), | ||
| ]); |
There was a problem hiding this comment.
This is probably the riskiest change to introduce here. The sort columns we define for the front end are pretty much fixed to these values but it is possible for columns to be registered dynamically and attempt to sort by a nested attribute.* field. This seems very unlikely and it does not look like this feature is being used by any other Kibana code.
There was a problem hiding this comment.
Seems reasonable to me. In fact, I like the approach that we have pre-defined options for sorting. We do, however, have to consider the case where a type may not have these fields, since we don't have a default option to sort on here.
What do you think about defaulting to type if the other fields are missing? It's unlikely they can be though, so probably not needed.
|
Pinging @elastic/kibana-core (Team:Core) |
There was a problem hiding this comment.
Looking good to me (but I'll defer the final review to @TinaHeiligers )
...ins/saved_objects_management/public/management_section/objects_table/saved_objects_table.tsx
Show resolved
Hide resolved
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @jloleysens |
| const sortFieldSchema: Type<keyof v1.SavedObjectWithMetadata> = schema.oneOf([ | ||
| schema.literal('created_at'), | ||
| schema.literal('updated_at'), | ||
| schema.literal('type'), | ||
| ]); |
There was a problem hiding this comment.
Seems reasonable to me. In fact, I like the approach that we have pre-defined options for sorting. We do, however, have to consider the case where a type may not have these fields, since we don't have a default option to sort on here.
What do you think about defaulting to type if the other fields are missing? It's unlikely they can be though, so probably not needed.
6 participants
Summary
This PR addresses 4 TODOs identified in #149495. We address these TODOs by taking a closer look at actual use of flagged fields in public code:
FindQueryHTTP['namespaceType']: Probably OK to directly expose this as it seems highly unlikely this value will change soon... We should also be able to version it since it is a well known set.FindQueryHTTP['search']: looks like it is allowing users to enter any search value against SO attribs. However, this functionality is constrained because it only takes a search term and runs it against a server-side determined search field. The risk is that consumers of the server-side functionality do not keep their default search field up-to-date.FindQueryHTTP['fields']: Our UI only ever passes inidhere. We should consider a follow up PR to add some counter telemetry to track usage of this option and consider removing it entirely.FindQueryHTTP['sortField']: Lock this down to a well known set of values (probably the riskiest change here).