Skip to content

Commit

Permalink
fix json validation and empty json handling in advanced settings (ela…
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisronline authored Dec 18, 2017
1 parent df790a4 commit 7173053
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
ng-keyup="maybeCancel($event, conf)"
elastic-textarea
validate-json
data-test-subj="unsavedValueJsonTextArea"
></textarea>

<p
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ uiModules.get('apps/management')
};

$scope.save = function (conf) {
// an empty JSON is valid as per the validateJson directive.
// set the value to empty JSON in this case so that its parsing upon retrieving the setting does not fail.
if (conf.type === 'json' && conf.unsavedValue === '') {
conf.unsavedValue = '{}';
}

loading(conf, function () {
if (conf.unsavedValue === conf.defVal) {
return config.remove(conf.name);
Expand Down
4 changes: 4 additions & 0 deletions src/ui/public/directives/__tests__/validate_json.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ describe('validate-json directive', function () {
checkValid(input.invalid, 'ng-invalid');
});

it('should be invalid if a number', function () {
checkValid('0', 'ng-invalid');
});

it('should update validity on changes', function () {
checkValid(input.valid, 'ng-valid');
checkValid(input.invalid, 'ng-invalid');
Expand Down
2 changes: 1 addition & 1 deletion src/ui/public/directives/validate_json.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module.directive('validateJson', function () {

// We actually need a proper object in all JSON inputs
newValue = (newValue || '').trim();
if (newValue[0] === '{' || '[') {
if (newValue[0] === '{' || newValue[0] === '[') {
try {
JSON.parse(newValue);
setValid();
Expand Down
7 changes: 7 additions & 0 deletions test/functional/apps/management/_kibana_settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ export default function ({ getService, getPageObjects }) {
expect(advancedSetting).to.be('America/Phoenix');
});

it('should coerce an empty setting of type JSON into an empty object', async function () {
await PageObjects.settings.clickKibanaSettings();
await PageObjects.settings.setAdvancedSettingsInput('query:queryString:options', '', 'unsavedValueJsonTextArea');
const advancedSetting = await PageObjects.settings.getAdvancedSettings('query:queryString:options');
expect(advancedSetting).to.be.eql('{}');
});

describe('state:storeInSessionStorage', () => {
it ('defaults to false', async () => {
await PageObjects.settings.clickKibanaSettings();
Expand Down

0 comments on commit 7173053

Please sign in to comment.