Skip to content

Commit

Permalink
fix(GridColumn): Allow for duplicate field coldefs
Browse files Browse the repository at this point in the history
If two column definitions were supplied with the same field and no name
property, it was causing conflicts as names need to be unique.

The default behavior now will just add an incrementing number on to the
end of extra columns, starting with "2".

Fixes #2364
  • Loading branch information
c0bra committed Jan 30, 2015
1 parent deb8e11 commit 82a7213
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 6 deletions.
56 changes: 50 additions & 6 deletions src/js/core/factories/Grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -622,19 +622,18 @@ angular.module('ui.grid')
var columnCache = self.columns.slice(0);

// We need to allow for the "row headers" when mapping from the column defs array to the columns array
// If we have a row header in columns[0] and don't account for it we'll overwrite it with the column in columnDefs[0]
var rowHeaderOffset = self.rowHeaderColumns.length;
// If we have a row header in columns[0] and don't account for it we'll overwrite it with the column in columnDefs[0]

// Go through all the column defs
for (i = 0; i < self.options.columnDefs.length; i++) {
// If the column at this index has a different name than the column at the same index in the column defs...
if (self.columns[i + rowHeaderOffset].name !== self.options.columnDefs[i].name) {
if (self.columns[i + headerOffset].name !== self.options.columnDefs[i].name) {
// Replace the one in the cache with the appropriate column
columnCache[i + rowHeaderOffset] = self.getColumn(self.options.columnDefs[i].name);
columnCache[i + headerOffset] = self.getColumn(self.options.columnDefs[i].name);
}
else {
// Otherwise just copy over the one from the initial columns
columnCache[i + rowHeaderOffset] = self.columns[i + rowHeaderOffset];
columnCache[i + headerOffset] = self.columns[i + headerOffset];
}
}

Expand Down Expand Up @@ -738,14 +737,59 @@ angular.module('ui.grid')
* validates that name or field is present
*/
Grid.prototype.preprocessColDef = function preprocessColDef(colDef) {
var self = this;

if (!colDef.field && !colDef.name) {
throw new Error('colDef.name or colDef.field property is required');
}

//maintain backwards compatibility with 2.x
//field was required in 2.x. now name is required
if (colDef.name === undefined && colDef.field !== undefined) {
colDef.name = colDef.field;
// See if the column name already exists:
var foundName = self.getColumn(colDef.field);

// If a column with this name already exists, we will add an incrementing number to the end of the new column name
if (foundName) {
// Search through the columns for names in the format: <name><1, 2 ... N>, i.e. 'Age1, Age2, Age3',
var nameRE = new RegExp('^' + colDef.field + '(\\d+)$', 'i');

var foundColumns = self.columns.filter(function (column) {
// Test against the displayName, as that's what'll have the incremented number
return nameRE.test(column.displayName);
})
// Sort the found columns by the end-number
.sort(function (a, b) {
if (a === b) {
return 0;
}
else {
var numA = a.match(nameRE)[1];
var numB = b.match(nameRE)[1];

return parseInt(numA, 10) > parseInt(numB, 10) ? 1 : -1;
}
});

// Not columns found, so start with number "2"
if (foundColumns.length === 0) {
colDef.name = colDef.field + '2';
}
else {
// Get the number from the final column
var lastNum = foundColumns[foundColumns.length-1].displayName.match(nameRE)[1];

// Make sure to parse to an int
lastNum = parseInt(lastNum, 10);

// Add 1 to the number from the last column and tack it on to the field to be the name for this new column
colDef.name = colDef.field + (lastNum + 1);
}
}
// ... otherwise just use the field as the column name
else {
colDef.name = colDef.field;
}
}

};
Expand Down
43 changes: 43 additions & 0 deletions test/unit/core/factories/GridColumn.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,49 @@ describe('GridColumn factory', function () {
it('should obey columnDef sort spec', function () {
// ... TODO(c0bra)
});

describe('when handling field-only defs', function () {
beforeEach(function () {
grid = new Grid({ id: 1 });
grid.registerColumnBuilder(gridClassFactory.defaultColumnBuilder);
});

it('should add an incrementing number to column names when they have the same field and no name', function () {
var cols = [
{ field: 'age' },
{ field: 'name' },
{ field: 'name' },
{ field: 'name' }
];

grid.options.columnDefs = cols;

buildCols();

expect(grid.columns[0].displayName).toEqual('Age');
expect(grid.columns[1].displayName).toEqual('Name');
expect(grid.columns[2].displayName).toEqual('Name2');
expect(grid.columns[3].displayName).toEqual('Name3');
});

it('should account for existing incremented names', function () {
var cols = [
{ field: 'age' },
{ field: 'name' },
{ field: 'name', name: 'Name3' },
{ field: 'name' }
];

grid.options.columnDefs = cols;

buildCols();

expect(grid.columns[0].displayName).toEqual('Age');
expect(grid.columns[1].displayName).toEqual('Name');
expect(grid.columns[2].displayName).toEqual('Name3');
expect(grid.columns[3].displayName).toEqual('Name4');
});
});
});

describe('getRenderContainer', function () {
Expand Down

0 comments on commit 82a7213

Please sign in to comment.