Skip to content

Commit

Permalink
Fix numeric colorB (#2780)
Browse files Browse the repository at this point in the history
Numeric colors weren't properly being handled by `makeBadge` after #2742.

Since this function really does not need to be accepting colors as strings, rather than make the function more lenient to work with Scoutcamp, I coerced the types of the colors on the way in.

Two tests cover the functionality in the modern service. I don't feel strongly that the legacy version needs coverage at this point, though I've added one for the moment on the github languages badge where this manifested.

Fix #2778
  • Loading branch information
paulmelnikow authored Jan 16, 2019
1 parent 8a10279 commit 4bf55a7
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 4 deletions.
5 changes: 3 additions & 2 deletions lib/badge-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,9 @@ function makeBadgeData(defaultLabel, overrides) {
logoPosition: +overrides.logoPosition,
logoWidth: +overrides.logoWidth,
links: toArray(overrides.link),
colorA: overrides.colorA,
colorB: overrides.colorB,
// Scoutcamp sometimes turns these into numbers.
colorA: `${overrides.colorA}`,
colorB: `${overrides.colorB}`,
}
}

Expand Down
10 changes: 8 additions & 2 deletions services/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,15 @@ class BaseService {
logoColor: overrideLogoColor,
logoWidth: overrideLogoWidth,
link: overrideLink,
colorA: overrideLabelColor,
colorB: overrideColor,
} = overrides
// Scoutcamp converts numeric query params to numbers. Convert them back.
let { colorB: overrideColor, colorA: overrideLabelColor } = overrides
if (typeof overrideColor === 'number') {
overrideColor = `${overrideColor}`
}
if (typeof overrideLabelColor === 'number') {
overrideLabelColor = `${overrideLabelColor}`
}

const {
isError,
Expand Down
9 changes: 9 additions & 0 deletions services/base.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,15 @@ describe('BaseService', function() {
expect(badgeData.color).to.equal('10ADED')
})

it('converts a query-string numeric color to a string', function() {
const badgeData = DummyService._makeBadgeData(
// Scoutcamp converts numeric query params to numbers.
{ colorB: 123 },
{ color: 'green' }
)
expect(badgeData.color).to.equal('123')
})

it('does not override the color in case of an error', function() {
const badgeData = DummyService._makeBadgeData(
{ colorB: '10ADED' },
Expand Down
10 changes: 10 additions & 0 deletions services/github/github-languages.tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ t.create('top language')
})
)

t.create('top language')
.get('/top/badges/shields.json?colorB=123&format=_shields_test')
.expectJSONTypes(
Joi.object().keys({
name: 'javascript',
value: Joi.string().regex(/^([1-9]?[0-9]\.[0-9]|100\.0)%$/),
color: '#123',
})
)

t.create('top language with empty repository')
.get('/top/pyvesb/emptyrepo.json')
.expectJSON({ name: 'language', value: 'none' })
Expand Down
4 changes: 4 additions & 0 deletions services/static-badge/static-badge.tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ t.create('Override colorB')
.get('/badge/label-message-blue.json?style=_shields_test&colorB=yellow')
.expectJSON({ name: 'label', value: 'message', color: 'yellow' })

t.create('Override colorB with a number')
.get('/badge/label-message-blue.json?style=_shields_test&colorB=123')
.expectJSON({ name: 'label', value: 'message', color: '#123' })

t.create('Override label')
.get('/badge/label-message-blue.json?style=_shields_test&label=mylabel')
.expectJSON({ name: 'mylabel', value: 'message', color: 'blue' })
Expand Down

0 comments on commit 4bf55a7

Please sign in to comment.