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

DatatableColumnMeta cleanup #78747

Closed
ppisljar opened this issue Sep 29, 2020 · 2 comments
Closed

DatatableColumnMeta cleanup #78747

ppisljar opened this issue Sep 29, 2020 · 2 comments
Assignees
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort v7.11.0 v8.0.0

Comments

@ppisljar
Copy link
Member

DatatableColumnMeta currently has the following type:

export interface DatatableColumnMeta {
  type: DatatableColumnType;	 
  field?: string;	  
  index?: string;	  
  params?: SerializedFieldFormat;	 
  source?: string;	  ;
  sourceParams?: SerializableState;	  
}	

two things are a bit unclear here:

  • meta.type vs meta.params.id ... both somehow represent the type, KBN_FIELD_TYPE vs field format type so its not clear which to use. on top of that they can also become out of sync and incompatible, (for example meta.type is string and meta.params.id is number
  • params/sourceParams

there are two options for the cleanup:

  1. replace meta.type with meta.params.id and replace meta.params with meta.params.params
    we would remove fieldType information (which is currently on meta.type) and replace it with field format id. This way its clear what the actual format is, and current fieldTypes in use are subset of available field formatters. The structure gets simplified and fieldType information is anyway redundant as we already have meta.field and meta.index with which we can find out the actual fieldType.

the new DatatableColumnMeta would become:

export interface DatatableColumnMeta {
  type: string;	 // fieldFormatId
  field?: string;	  
  index?: string;	  
  params?: SerializableState; // fieldFormatParams	 
  source?: string;	  ;
  sourceParams?: SerializableState;	  
}	
  1. rename meta.params to meta.format and meta.sourceParams to meta.params
    we would keep the redundant fieldType information (meta.type) but rather rename meta.params to be more semantic into meta.format which would still be whole SerializedFieldFormat. There is still unclear which type to use (meta.type or meta.format.id) and they may still be incompatible, but at least we have a bit more semantic naming.
@ppisljar ppisljar added Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) Team:AppArch v7.11.0 v8.0.0 labels Sep 29, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@ppisljar ppisljar self-assigned this Oct 12, 2020
@petrklapka petrklapka added 1 and removed 1 labels May 10, 2021
@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels May 13, 2021
@exalate-issue-sync exalate-issue-sync bot added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Jul 28, 2021
@exalate-issue-sync exalate-issue-sync bot added loe:small Small Level of Effort and removed loe:medium Medium Level of Effort labels Nov 22, 2021
@ppisljar
Copy link
Member Author

ppisljar commented Mar 8, 2022

as meta field on the datatable is considered internal implementation detail we won't be doing this cleanup

@ppisljar ppisljar closed this as completed Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort v7.11.0 v8.0.0
Projects
None yet
Development

No branches or pull requests

3 participants