diff --git a/lib/plugins/labels.js b/lib/plugins/labels.js index 6e48d8dcb..50b8822d1 100644 --- a/lib/plugins/labels.js +++ b/lib/plugins/labels.js @@ -21,12 +21,12 @@ module.exports = class Labels extends Diffable { if (this.entries) { this.entries.forEach(label => { - // Force color to string since some hex colors can be numerical (e.g. 999999) - if (label.color) { + // Force color to string since some hex colors can be numerical (e.g. 999999). + // Numbers lose leading zeros in YAML (000001 becomes 1), so only they are padded. + if (typeof label.color === 'number') { + label.color = String(label.color).padStart(6, '0') + } else if (label.color !== undefined && label.color !== null) { label.color = String(label.color).replace(/^#/, '') - if (label.color.length < 6) { - label.color.padStart(6, '0') - } } }) } @@ -61,6 +61,9 @@ module.exports = class Labels extends Diffable { this.log.debug(`Updating labels for ${JSON.stringify(attrs, null, 4)} ${JSON.stringify(this.repo, null, 4)}`) attrs.current_name = attrs.oldname || attrs.name delete attrs.oldname + if (this.hasInvalidColor(attrs)) { + return this.invalidColorError(attrs) + } if (this.nop) { return Promise.resolve([ new NopCommand(this.constructor.name, this.repo, this.github.rest.issues.updateLabel.endpoint(this.wrapAttrs(attrs)), 'Update label') @@ -70,6 +73,9 @@ module.exports = class Labels extends Diffable { } add (attrs) { + if (this.hasInvalidColor(attrs)) { + return this.invalidColorError(attrs) + } if (this.nop) { return Promise.resolve([ new NopCommand(this.constructor.name, this.repo, this.github.rest.issues.createLabel.endpoint(this.wrapAttrs(attrs)), 'Create label') @@ -98,4 +104,18 @@ module.exports = class Labels extends Diffable { isExcluded (label) { return this.exclude.some(rx => rx.test(label.name)) } + + hasInvalidColor (label) { + return label.color !== undefined && !/^[0-9a-f]{6}$/i.test(label.color) + } + + // YAML reads unquoted colors like 5319e7 as numbers (5319 × 10^7), which can't be recovered here. + invalidColorError (label) { + const msg = `Label '${label.name}' has invalid color '${label.color}'. Colors must be 6 hex characters, e.g. '5319e7'. Quote the value in YAML so it isn't read as a number.` + if (this.nop) { + return Promise.resolve([new NopCommand(this.constructor.name, this.repo, null, msg, 'ERROR')]) + } + this.logError(msg) + return Promise.resolve([]) + } } diff --git a/test/unit/lib/plugins/labels.test.js b/test/unit/lib/plugins/labels.test.js index 3f6ebe9d6..a7ccebe98 100644 --- a/test/unit/lib/plugins/labels.test.js +++ b/test/unit/lib/plugins/labels.test.js @@ -148,5 +148,70 @@ describe('Labels', () => { expect(github.rest.issues.createLabel).toHaveBeenCalledTimes(1) }) }) + + it('pads colors that YAML read as numbers', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([])) + + const plugin = configure([ + { name: 'black', color: 0, description: '' }, // YAML treats `color: 000000` as 0 + { name: 'almost-black', color: 1, description: '' } // YAML treats `color: 000001` as 1 + ]) + + return plugin.sync().then(() => { + const createdColors = github.rest.issues.createLabel.mock.calls.map(([params]) => params.color) + expect(createdColors).toEqual(['000000', '000001']) + }) + }) + + describe('invalid colors', () => { + // YAML treats `color: 5319e7` as 5319 × 10^7 + const config = [{ name: 'hello-world', color: 5319e7, description: '' }] + const expectedMessage = "Label 'hello-world' has invalid color '53190000000'. Colors must be 6 hex characters, e.g. '5319e7'. Quote the value in YAML so it isn't read as a number." + + it('reports an error instead of creating the label', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([])) + log.error = jest.fn() + const errors = [] + const plugin = new Labels(false, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors) + + return plugin.sync().then(() => { + expect(github.rest.issues.createLabel).not.toHaveBeenCalled() + expect(errors).toEqual([expect.objectContaining({ msg: expectedMessage, plugin: 'Labels' })]) + }) + }) + + it('reports an error instead of updating the label', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([{ name: 'hello-world', color: '5319e7', description: '' }])) + log.error = jest.fn() + const errors = [] + const plugin = new Labels(false, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors) + + return plugin.sync().then(() => { + expect(github.rest.issues.updateLabel).not.toHaveBeenCalled() + expect(errors).toEqual([expect.objectContaining({ msg: expectedMessage })]) + }) + }) + + it('shows the error in the dry-run', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([])) + const plugin = new Labels(true, github, { owner: 'bkeepers', repo: 'test' }, config, log, []) + + return plugin.sync().then(results => { + expect(results.flat()).toContainEqual(expect.objectContaining({ type: 'ERROR', action: expect.objectContaining({ msg: expectedMessage }) })) + }) + }) + + it('reports a quoted non-hex color as written, without padding it', () => { + github.paginate.mockReturnValueOnce(Promise.resolve([])) + log.error = jest.fn() + const errors = [] + const plugin = new Labels(false, github, { owner: 'bkeepers', repo: 'test' }, [{ name: 'hello-world', color: 'red', description: '' }], log, errors) + + return plugin.sync().then(() => { + expect(github.rest.issues.createLabel).not.toHaveBeenCalled() + expect(errors).toEqual([expect.objectContaining({ msg: expect.stringContaining("invalid color 'red'. Colors must be 6 hex characters") })]) + }) + }) + }) }) })