Skip to content

Commit dd6dc15

Browse files
committed
fix(Grid): fix buildColumns handling same field
When buildColumns received the same field multiple times in columnDefs it would eventually throw an exception due to a typo where it was trying to generate incremental display names. This change fixes that and includes tests for the exception as well as incremental displayName creation. Fixes #2789
1 parent 735af76 commit dd6dc15

File tree

3 files changed

+178
-2
lines changed

3 files changed

+178
-2
lines changed

src/js/core/factories/Grid.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -793,8 +793,8 @@ angular.module('ui.grid')
793793
return 0;
794794
}
795795
else {
796-
var numA = a.match(nameRE)[1];
797-
var numB = b.match(nameRE)[1];
796+
var numA = a.displayName.match(nameRE)[1];
797+
var numB = b.displayName.match(nameRE)[1];
798798

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

test/karma.debug.conf.js

+144
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// Karma configuration
2+
// Generated on Fri Nov 08 2013 09:25:16 GMT-0600 (Central Standard Time)
3+
var util = require('../lib/grunt/utils.js');
4+
var grunt = require('grunt');
5+
6+
module.exports = function(config) {
7+
config.set({
8+
9+
// base path, that will be used to resolve files and exclude
10+
basePath: '../',
11+
12+
// frameworks to use
13+
frameworks: ['jasmine'],
14+
15+
// list of files / patterns to load in the browser (we add more dynamically in our tasks)
16+
files: [
17+
'bower_components/jquery/jquery.min.js',
18+
'lib/test/jquery.simulate.js',
19+
'bower_components/lodash/dist/lodash.min.js',
20+
21+
'lib/test/angular/1.3.6/angular.js',
22+
'lib/test/angular/1.3.6/angular-mocks.js',
23+
'lib/test/angular/1.3.6/angular-animate.js',
24+
25+
'src/js/core/bootstrap.js',
26+
'src/js/**/*.js',
27+
'src/features/**/js/**/*.js',
28+
'test/unit/**/*.spec.js',
29+
'src/features/**/test/**/*.spec.js',
30+
31+
'dist/release/ui-grid.css',
32+
33+
'.tmp/template.js' //templates
34+
],
35+
36+
37+
// list of files to exclude
38+
exclude: [
39+
],
40+
41+
42+
// test results reporter to use
43+
// possible values: 'dots', 'progress', 'junit', 'growl', 'coverage'
44+
reporters: ['dots', 'coverage'],
45+
46+
preprocessors: {
47+
// Cover source files but ignore any .spec.js files. Thanks goodness I found the pattern here: https://github.com/karma-runner/karma/pull/834#issuecomment-35626132
48+
'src/**/!(*.spec)+(.js)': ['coverage']
49+
},
50+
51+
coverageReporter: {
52+
type: 'lcov',
53+
dir: 'coverage',
54+
subdir: '.'
55+
},
56+
57+
// web server port
58+
port: 9876,
59+
60+
61+
// enable / disable colors in the output (reporters and logs)
62+
colors: true,
63+
64+
65+
// level of logging
66+
// possible values: config.LOG_DISABLE || config.LOG_ERROR || config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG
67+
logLevel: config.LOG_INFO,
68+
69+
70+
// enable / disable watching file and executing tests whenever any file changes
71+
autoWatch: false,
72+
73+
74+
// Start these browsers, currently available:
75+
// - Chrome
76+
// - ChromeCanary
77+
// - Firefox
78+
// - Opera (has to be installed with `npm install karma-opera-launcher`)
79+
// - Safari (only Mac; has to be installed with `npm install karma-safari-launcher`)
80+
// - PhantomJS
81+
// - IE (only Windows; has to be installed with `npm install karma-ie-launcher`)
82+
browsers: ['PhantomJS'],
83+
84+
85+
// If browser does not capture in given timeout [ms], kill it
86+
captureTimeout: 60000,
87+
88+
89+
// Continuous Integration mode
90+
// if true, it capture browsers, run tests and exit
91+
singleRun: false,
92+
93+
browserDisconnectTimeout: 10000,
94+
browserDisconnectTolerance: 2,
95+
browserNoActivityTimeout: 45000, // 20000,
96+
97+
sauceLabs: {
98+
username: 'nggrid',
99+
startConnect: false,
100+
testName: 'ui-grid unit tests'
101+
},
102+
103+
// For more browsers on Sauce Labs see:
104+
// https://saucelabs.com/docs/platforms/webdriver
105+
customLaunchers: util.customLaunchers()
106+
107+
});
108+
109+
// TODO(c0bra): remove once SauceLabs supports websockets.
110+
// This speeds up the capturing a bit, as browsers don't even try to use websocket. -- (thanks vojta)
111+
if (process.env.TRAVIS) {
112+
config.logLevel = config.LOG_INFO;
113+
config.browserNoActivityTimeout = 120000; // NOTE: from angular.js, for socket.io buffer
114+
config.reporters = ['dots', 'coverage'];
115+
116+
var buildLabel = 'TRAVIS #' + process.env.TRAVIS_BUILD_NUMBER + ' (' + process.env.TRAVIS_BUILD_ID + ')';
117+
118+
// config.transports = ['websocket', 'xhr-polling'];
119+
120+
config.sauceLabs.build = buildLabel;
121+
config.sauceLabs.startConnect = false;
122+
config.sauceLabs.tunnelIdentifier = process.env.TRAVIS_JOB_NUMBER;
123+
124+
config.transports = ['xhr-polling'];
125+
126+
// Debug logging into a file, that we print out at the end of the build.
127+
config.loggers.push({
128+
type: 'file',
129+
filename: process.env.LOGS_DIR + '/' + ('karma.log')
130+
});
131+
132+
133+
// NOTE: From angular.js project, only applies to SauceLabs -- (thanks Vojta again)
134+
// Allocating a browser can take pretty long (eg. if we are out of capacity and need to wait
135+
// for another build to finish) and so the `captureTimeout` typically kills
136+
// an in-queue-pending request, which makes no sense.
137+
config.captureTimeout = 0;
138+
}
139+
140+
if (grunt.option('browsers')) {
141+
var bs = grunt.option('browsers').split(/,/).map(function(b) { return b.trim(); });
142+
config.browsers = bs;
143+
}
144+
};

test/unit/core/factories/Grid.spec.js

+32
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,38 @@ describe('Grid factory', function () {
396396
expect(grid1.columns[4].name).toEqual('5');
397397
expect(grid1.columns[5].name).toEqual('5.5');
398398
});
399+
400+
describe('when adding the same field multiple times', function () {
401+
var grid;
402+
403+
beforeEach(function () {
404+
grid = new Grid({ id: 1 });
405+
grid.options.columnDefs = [{ field: 'a' }];
406+
grid.buildColumns();
407+
});
408+
409+
it('should not throw an exception', function () {
410+
expect(function () {
411+
for (var i = 1; i<=4; i++) {
412+
grid.options.columnDefs.push({ field: 'a' });
413+
grid.buildColumns();
414+
}
415+
}).not.toThrow();
416+
});
417+
418+
it('should create incremental displayNames', function () {
419+
for (var i = 1; i<=4; i++) {
420+
grid.options.columnDefs.push({ field: 'a' });
421+
}
422+
grid.buildColumns();
423+
424+
expect(grid.columns[0].displayName).toEqual('A');
425+
expect(grid.columns[1].displayName).toEqual('A2');
426+
expect(grid.columns[2].displayName).toEqual('A3');
427+
expect(grid.columns[3].displayName).toEqual('A4');
428+
expect(grid.columns[4].displayName).toEqual('A5');
429+
});
430+
});
399431
});
400432

401433
describe('follow source array', function() {

0 commit comments

Comments
 (0)