-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Allow modifying unknown/custom advanced settings #4924
Changes from 4 commits
2378e62
3932bd0
120a2a7
8c49d4f
cb97f79
659a6da
5a4cadb
34992c0
075a18e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
|
||
var getEditorType = require('plugins/kibana/settings/sections/advanced/lib/get_editor_type'); | ||
var expect = require('expect.js'); | ||
|
||
describe('Settings', function () { | ||
describe('Advanced', function () { | ||
describe('getEditorType(conf)', function () { | ||
context('when given type has a named editor', function () { | ||
it('returns that named editor', function () { | ||
expect(getEditorType({ type: 'json' })).to.equal('json'); | ||
expect(getEditorType({ type: 'array' })).to.equal('array'); | ||
expect(getEditorType({ type: 'boolean' })).to.equal('boolean'); | ||
expect(getEditorType({ type: 'select' })).to.equal('select'); | ||
}); | ||
}); | ||
|
||
context('when given a type of number, string, null, or undefined', function () { | ||
it('returns "normal"', function () { | ||
expect(getEditorType({ type: 'number' })).to.equal('normal'); | ||
expect(getEditorType({ type: 'string' })).to.equal('normal'); | ||
expect(getEditorType({ type: 'null' })).to.equal('normal'); | ||
expect(getEditorType({ type: 'undefined' })).to.equal('normal'); | ||
}); | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
|
||
var isImmutableConfig = require('plugins/kibana/settings/sections/advanced/lib/is_immutable_config'); | ||
var expect = require('expect.js'); | ||
|
||
describe('Settings', function () { | ||
describe('Advanced', function () { | ||
describe('isImmutableConfig(configName)', function () { | ||
it('returns true given an immutable field name', function () { | ||
expect(isImmutableConfig('buildNum')).to.be.true; | ||
}); | ||
|
||
it('returns false given any value that does not match the whitelist', function () { | ||
expect(isImmutableConfig('something else')).to.be.false; | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
|
||
var toEditableConfig = require('plugins/kibana/settings/sections/advanced/lib/to_editable_config'); | ||
var expect = require('expect.js'); | ||
|
||
describe('Settings', function () { | ||
describe('Advanced', function () { | ||
describe('toEditableConfig(def, name, value)', function () { | ||
it('sets name', function () { | ||
expect(invoke({ name: 'who' }).name).to.equal('who'); | ||
}); | ||
|
||
it('sets value', function () { | ||
expect(invoke({ value: 'what' }).value).to.equal('what'); | ||
}); | ||
|
||
it('sets type', function () { | ||
expect(invoke({ value: 'what' }).type).to.be('string'); | ||
expect(invoke({ value: 0 }).type).to.be('number'); | ||
expect(invoke({ value: [] }).type).to.be('array'); | ||
}); | ||
|
||
context('when given a setting definition object', function () { | ||
var def; | ||
beforeEach(function () { | ||
def = { | ||
value: 'the original', | ||
description: 'the one and only', | ||
options: 'all the options' | ||
}; | ||
}); | ||
|
||
it('is not marked as custom', function () { | ||
expect(invoke({ def }).isCustom).to.be.false; | ||
}); | ||
|
||
it('sets a default value', function () { | ||
expect(invoke({ def }).defVal).to.equal(def.value); | ||
}); | ||
|
||
it('sets a description', function () { | ||
expect(invoke({ def }).description).to.equal(def.description); | ||
}); | ||
|
||
it('sets options', function () { | ||
expect(invoke({ def }).options).to.equal(def.options); | ||
}); | ||
|
||
context('that contains a type', function () { | ||
it('sets that type', function () { | ||
def.type = 'something'; | ||
expect(invoke({ def }).type).to.equal(def.type); | ||
}); | ||
}); | ||
|
||
context('that contains a value of type array', function () { | ||
it('sets type to array', function () { | ||
def.value = []; | ||
expect(invoke({ def }).type).to.equal('array'); | ||
}); | ||
}); | ||
}); | ||
|
||
context('when not given a setting definition object', function () { | ||
it('is marked as custom', function () { | ||
expect(invoke().isCustom).to.be.true; | ||
}); | ||
|
||
it('sets defVal to undefined', function () { | ||
expect(invoke().defVal).to.be.undefined; | ||
}); | ||
|
||
it('sets description to undefined', function () { | ||
expect(invoke().description).to.be.undefined; | ||
}); | ||
|
||
it('sets options to undefined', function () { | ||
expect(invoke().options).to.be.undefined; | ||
}); | ||
}); | ||
}); | ||
}); | ||
}); | ||
|
||
function invoke({ def = false, name = 'woah', value = 'forreal' } = {}) { | ||
return toEditableConfig(def, name, value); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
define(function (require) { | ||
var _ = require('lodash'); | ||
|
||
var NAMED_EDITORS = ['json', 'array', 'boolean', 'select']; | ||
var NORMAL_EDITOR = ['number', 'string', 'null', 'undefined']; | ||
|
||
/** | ||
* @param {object} advanced setting configuration object | ||
* @returns {string} the editor type to use when editing value | ||
*/ | ||
function getEditorType(conf) { | ||
if (_.contains(NAMED_EDITORS, conf.type)) return conf.type; | ||
if (_.contains(NORMAL_EDITOR, conf.type)) return 'normal'; | ||
} | ||
|
||
return getEditorType; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
define(function (require) { | ||
var _ = require('lodash'); | ||
|
||
var IMMUTABLE_CONFIG_VALS = ['buildNum']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if there are any other fields in .kibana/config that we should whitelist. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great idea |
||
|
||
/** | ||
* @param {string} name of configuration | ||
* @returns {boolean} whether the given name matches an immutable field name | ||
*/ | ||
function isImmutableConfig(configName) { | ||
return _.contains(IMMUTABLE_CONFIG_VALS, configName); | ||
} | ||
|
||
return isImmutableConfig; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
define(function (require) { | ||
var _ = require('lodash'); | ||
var getValType = require('./get_val_type'); | ||
var getEditorType = require('./get_editor_type'); | ||
|
||
/** | ||
* @param {object} advanced setting definition object | ||
* @param {object} name of setting | ||
* @param {object} current value of setting | ||
* @returns {object} the editable config object | ||
*/ | ||
function toEditableConfig(def, name, value) { | ||
var isCustom = !def; | ||
if (isCustom) def = {}; | ||
|
||
var conf = { | ||
name, | ||
value, | ||
isCustom, | ||
defVal: def.value, | ||
type: getValType(def, value), | ||
description: def.description, | ||
options: def.options | ||
}; | ||
|
||
var editor = getEditorType(conf); | ||
conf.json = editor === 'json'; | ||
conf.select = editor === 'select'; | ||
conf.bool = editor === 'boolean'; | ||
conf.array = editor === 'array'; | ||
conf.normal = editor === 'normal'; | ||
conf.tooComplex = !editor; | ||
|
||
return conf; | ||
} | ||
|
||
return toEditableConfig; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not at all! I actually prefer to hard break paragraphs in html.