[7.5] [Index template] Fix editor should support mappings types (#55804) #56279
Conversation
sebelga
added
bug
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
sebelga
added
backport
and removed
Feature:Index Management
There was a problem hiding this comment.
I tested locally and it works great! Custom types can be loaded, added/removed, and the changes preserved on save.
I found a lot of code that looks like it's unused. Was this intentional? My initial thought is that if the code isn't needed we should avoid backporting it, to keep both the codebase and the PR easy to understand.
|
|
||
| type ParametersOptions = ParameterName | 'languageAnalyzer'; | ||
|
|
||
| export const PARAMETERS_OPTIONS: { |
There was a problem hiding this comment.
It doesn't look like this is being used. Can we remove it? If so, will this allow us to remove more code as well?
|
|
||
| const DATE_FORMAT_OPTIONS = DATE_FORMATS.map(({ label }) => ({ label })); | ||
|
|
||
| export const ALL_DATE_FORMAT_OPTIONS = [...DATE_FORMAT_OPTIONS, ...STRICT_DATE_FORMAT_OPTIONS]; |
| | TermVectorOptions | ||
| | OrientationOptions; | ||
|
|
||
| export const FIELD_OPTIONS_TEXTS: { [key in FieldOption]: Optioni18n } = { |
There was a problem hiding this comment.
It looks like removing PARAMETERS_OPTIONS lets us remove getOptionTexts which will also allow us to remove FIELD_OPTIONS_TEXTS.
| * | ||
| * @param properties A mappings "properties" object | ||
| */ | ||
| export const validateProperties = (properties = {}): PropertiesValidatorResponse => { |
There was a problem hiding this comment.
This looks unused too. Removing it will let us remove its test file too, which will address the CI failure.
There was a problem hiding this comment.
Actually, this one is quite important as it is used by the validateMappings() function on L282
There was a problem hiding this comment.
EDIT: Sorry my bad, validateMappings() is not used on this branch, great catch! I also removed it.
|
Thanks for the review @cjcenizal !
I haven't cleaned up the files as I first thought it was better to have them as close as possible from the 7.x branch for future backports... but now that I think about it, it will never happen on those files 😊 So you're right, we need to remove all the unused code. Can you have another look? thanks 👍 |
There was a problem hiding this comment.
@sebelga I found some more opportunities to remove code via sebelga#15. I tested locally and everything works as expected with those changes. If you're OK with merging my PR then I think we're good to go!
|
Great thanks for the PR @cjcenizal ! |
|
@elasticmachine merge upstream |
…-support-mappings-types
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
sebelga
deleted the
backport/7.5/fix-Index-template-editor-should-support-mappings-types
branch
This PR is the backport of #55804 for the
7.5branch. As the original fix relies on the mappings editorlibfiles, I manually copied the neededlibfiles,constantsfiles andtype.tsfile from the mappings editor.I slightly changed the content of the
constantsfile to avoid having to bring with the whole jungle 😊 (the form hook lib for example).How to test
Follow the steps in the original PR #55804
The only difference is that here we directly edit the JSON to create custom types.
As this is not a 1:1 backport from the original PR can you have a look @cjcenizal ? Thanks!