Allow modifying unknown/custom advanced settings #4924
Conversation
If the kibana index includes any custom key/value pairs in the config, they will be editable on the advanced settings panel. All custom settings appear at the bottom of the list and are visually labeled as a "custom setting". When the trash can icon is clicked, custom settings are removed entirely from the index.
A unit test is also included since the function is now importable.
The function is now unit tested as well. There are a few flags of the resulting configuration object that are not yet tested, but they're really behaviors of the returned value based on the current state rather than behaviors of toEditableConfig itself.
And test it.
| define(function (require) { | ||
| var _ = require('lodash'); | ||
|
|
||
| var IMMUTABLE_CONFIG_VALS = ['buildNum']; |
There was a problem hiding this comment.
I'm not sure if there are any other fields in .kibana/config that we should whitelist. buildNum was the only one in my local setup.
There was a problem hiding this comment.
What do you think about adding buildNum to the defaults file (which is more of a schema file at this point) and giving it a readOnly attribute, or something to that effect? It would probably be easier to manage the the schema if all of the field properties were listed in one place, rather than in defaults and is_mutable_config.js.
|
Assuming my assumption about immutable fields is correct, this should be good to go. |
Rather than having to maintain possible configuration values in two different places, the "immutable field" logic is removed in favor of defining readonly fields in the default config file.
|
@spalger How about that? |
|
👍 |
|
I've never noticed Seems like |
| @@ -3,8 +3,7 @@ | |||
| <div class="bs-callout bs-callout-warning"> | |||
| <h4>Caution: You can break stuff here</h4> | |||
| Be careful in here, these settings are for very advanced users only. Tweaks you make here can break large portions of Kibana. | |||
| Some of these settings may be undocumented, unsupported or experimental. Blanking a field will cause Kibana to use its internal | |||
| defaults which may be unacceptable given other configuration directives. | |||
| Some of these settings may be undocumented, unsupported or experimental. If a field has a default value, blanking the field will reset it to its default which may be unacceptable given other configuration directives. Deleting a custom setting will permanently remove it from Kibana's config. | |||
There was a problem hiding this comment.
Mind putting some new lines in here? We don't really enforce line length in HTML, but newlines in HTML text don't affect the result either.
There was a problem hiding this comment.
Not at all! I actually prefer to hard break paragraphs in html.
|
So |
|
Or perhaps something like |
|
Unless @spalger thinks it's a good idea for some reason. Sounds like he has some plans to change how the test paths will work in the future. |
|
I personally would put the tests right next to the thing they exercise. Not only does it make it easier to see that there are tests for this bit of code, it makes refactoring a bit easier and eventually will allow us to do something like |
The __tests__ directories should reside alongside of the code they are testing.
|
Works for me. I moved the whole |
|
jenkins, test it |
|
This LGTM. I found an issue while testing, and submitted a pr to you against this branch: epixa#1 Take a look at that, it'd be nice to get that into this PR as well. |
unhook the change:config listener on destroy
|
Merged. |
Allow modifying unknown/custom advanced settings
If the kibana index includes any custom key/value pairs in the config,
they will be editable on the advanced settings panel. All custom
settings appear at the bottom of the list and are visually labeled as a
"custom setting". When the trash can icon is clicked, custom settings
are removed entirely from the index.
Fixes #4684