Skip to content

Commit

Permalink
Merge pull request elastic#7950 from ck-lee/better-csv-import-errors-…
Browse files Browse the repository at this point in the history
…messages

Fix elastic#7722 Improved header validation on CSV import
  • Loading branch information
Matt Bargar authored Aug 17, 2016
2 parents b98339b + 49c4d7b commit 56b2695
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import validateHeaders from '../validate_headers';
import expect from 'expect.js';

describe('validateHeaders', function () {

describe('basic validations', function () {

it('should return empty array if fields are valid', function () {
const errors = validateHeaders(['aa', 'bb', 'cc', 'dd', 'ee']);
expect(errors).to.eql([]);
});

it('should return empty array if it contains white space field name', function () {
const errors = validateHeaders(['aa', 'bb', ' ', ' ', 'ee']);
expect(errors.length).to.be(0);
});

it('should return an error if contains blank field name', function () {
const errors = validateHeaders(['aa', 'bb', 'cc', '', 'ee']);
expect(errors.length).to.be(1);
expect(errors[0]).eql({ type: 'blank', positions: [4] });
});

it('should return an error if fields contain duplicate field name', function () {
const errors = validateHeaders(['aa', 'bb', 'cc', 'dd', 'aa']);
expect(errors.length).to.be(1);
expect(errors[0]).eql({ type: 'duplicate', positions: [1, 5], fieldName: 'aa' });
});

it('should return multiple errors if fields contain duplicate and blank field name', function () {
const errors = validateHeaders(['aa', 'bb', '', 'dd', 'aa', '', 'aa', 'dd', 'hh']);
expect(errors.length).to.be(3);
expect(errors[0]).eql({ type: 'duplicate', positions: [1, 5, 7], fieldName: 'aa' });
expect(errors[1]).eql({ type: 'blank', positions: [3, 6] });
expect(errors[2]).eql({ type: 'duplicate', positions: [4, 8], fieldName: 'dd' });
});
});

});
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import _ from 'lodash';

/**
* @typedef error
* @type Object
* @property {String} fieldName The field name with error.
* @property {Number[]} positions The field positions with error. One-based.
* @property {String} type The type of error. Eg: duplicate, blank
*/

/**
* Check for errors with header fields for csv import preview.
* Returns an empty array if no error is found.
*
* @param {String[]} fields An array of field names
* @returns {error[]} errors An array of error objects
*/
export default function validateHeaders(fields) {
const errors = [];

const fieldsWithPositions = _.reduce(fields, (result, field, index) => {
(result[field] || (result[field] = [])).push(index + 1);
return result;
}, {});

_.forEach(fieldsWithPositions, (positions, field) => {
if (_.isEmpty(field)) {
errors.push({
positions: positions,
type: 'blank'
});
}
else if (positions.length > 1) {
errors.push({
fieldName: field,
positions: positions,
type: 'duplicate'
});
}
});

return errors;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import _ from 'lodash';
import Papa from 'papaparse';
import modules from 'ui/modules';
import validateHeaders from './lib/validate_headers';
import template from './parse_csv_step.html';
import './styles/_add_data_parse_csv_step.less';

Expand Down Expand Up @@ -67,20 +68,15 @@ modules.get('apps/management')
return;
}
if (row === 1) {
// Collect general information on the first pass
if (results.meta.fields.length > _.uniq(results.meta.fields).length) {
this.formattedErrors.push('Column names must be unique');
}

let hasEmptyHeader = false;
_.forEach(results.meta.fields, (field) => {
if (_.isEmpty(field)) {
hasEmptyHeader = true;
// Check for header errors on the first row
const errors = validateHeaders(results.meta.fields);
_.forEach(errors, (error) => {
if (error.type === 'duplicate') {
this.formattedErrors.push(`Columns at positions [${error.positions}] have duplicate name "${error.fieldName}"`);
} else if (error.type === 'blank') {
this.formattedErrors.push(`Columns at positions [${error.positions}] must not be blank`);
}
});
if (hasEmptyHeader) {
this.formattedErrors.push('Column names must not be blank');
}

if (results.meta.fields.length > maxSampleColumns) {
this.formattedWarnings.push(`Preview truncated to ${maxSampleColumns} columns`);
Expand Down

0 comments on commit 56b2695

Please sign in to comment.