Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move JSON rendering from badge-maker to server [*****] #4980

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 42 additions & 4 deletions badge-maker/lib/color.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,50 @@ test(isHexColor, () => {
})

test(normalizeColor, () => {
// Shields named color.
given('red').expect('red')
given('green').expect('green')
given('blue').expect('blue')
given('4c1').expect('#4c1')
given('f00f00').expect('#f00f00')
calebcartwright marked this conversation as resolved.
Show resolved Hide resolved
given('ABC123').expect('#abc123')
given('#ABC123').expect('#abc123')
given('yellow').expect('yellow')

// valid hex
forCases([given('#4c1'), given('#4C1'), given('4C1'), given('4c1')]).expect(
'#4c1'
)
forCases([
given('#abc123'),
given('#ABC123'),
given('abc123'),
given('ABC123'),
]).expect('#abc123')

// valid rgb(a)
given('rgb(0,128,255)').expect('rgb(0,128,255)')
given('rgba(0,128,255,0)').expect('rgba(0,128,255,0)')
// valid hsl(a)
given('hsl(100, 56%, 10%)').expect('hsl(100, 56%, 10%)')
given('hsla(25,20%,0%,0.1)').expect('hsla(25,20%,0%,0.1)')

// CSS named color.
given('papayawhip').expect('papayawhip')
given('purple').expect('purple')

forCases([
// invalid hex
given('#123red'), // contains letter above F
given('#red'), // contains letter above F
// invalid rgb(a)
given('rgb(220,128,255,0.5)'), // has alpha
given('rgba(0,0,255)'), // no alpha
// invalid hsl(a)
given('hsl(360,50%,50%,0.5)'), // has alpha
given('hsla(0,50%,101%)'), // no alpha
// neither a css named color nor colorscheme
given('notacolor'),
given('bluish'),
given('almostred'),
given('brightmaroon'),
given('cactus'),
given(''),
given('not-a-color'),
given(undefined),
Expand All @@ -36,6 +71,9 @@ test(normalizeColor, () => {
given({}),
given(() => {}),
]).expect(undefined)

// Semantic color alias
given('success').expect('brightgreen')
given('lightgray').expect('lightgrey')
given('informational').expect('blue')
})
Expand Down
46 changes: 30 additions & 16 deletions badge-maker/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

const _makeBadge = require('./make-badge')
const { normalizeColor } = require('./color')

class ValidationError extends Error {}

Expand All @@ -16,9 +17,18 @@ function _validate(format) {
throw new ValidationError('Field `message` is required')
}

const stringFields = ['labelColor', 'color', 'message', 'label']
stringFields.forEach(function(field) {
const requiredStringFields = ['message', 'label']
requiredStringFields.forEach(field => {
if (field in format && typeof format[field] !== 'string') {
throw new ValidationError(
`Field \`${field}\` is required and must be of type string`
)
}
})

const optionalStringFields = ['labelColor', 'color']
optionalStringFields.forEach(field => {
if (format[field] != null && typeof format[field] !== 'string') {
throw new ValidationError(`Field \`${field}\` must be of type string`)
}
})
Expand All @@ -42,23 +52,26 @@ function _clean(format) {

const cleaned = {}
Object.keys(format).forEach(key => {
if (format[key] != null && expectedKeys.includes(key)) {
cleaned[key] = format[key]
} else {
throw new ValidationError(
`Unexpected field '${key}'. Allowed values are (${expectedKeys.toString()})`
)
if (format[key] != null) {
if (expectedKeys.includes(key)) {
cleaned[key] = format[key]
} else {
throw new ValidationError(
`Unexpected field '${key}'. Allowed values are (${expectedKeys.toString()})`
)
}
}
})

// convert "public" format to "internal" format
cleaned.text = [cleaned.label || '', cleaned.message]
delete cleaned.label
delete cleaned.message
if ('style' in cleaned) {
cleaned.template = cleaned.style
delete cleaned.style
}
// Whitespace removal.
cleaned.label = `${cleaned.label}`.trim()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a message only badge, running

cleaned.label = `${cleaned.label}`.trim()

would result in the label property being the string literal "undefined". This is not what we want.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I'll fix this up tomorrow.

cleaned.message = `${cleaned.message}`.trim()

cleaned.color = normalizeColor(cleaned.color)
cleaned.labelColor = normalizeColor(cleaned.labelColor)

cleaned.style = cleaned.style || 'flat'
cleaned.links = ['', '']

return cleaned
}
Expand All @@ -84,4 +97,5 @@ function makeBadge(format) {
module.exports = {
makeBadge,
ValidationError,
_clean,
}
23 changes: 23 additions & 0 deletions badge-maker/lib/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,29 @@ describe('makeBadge function', function() {
style: 'flat',
})
).to.satisfy(isSvg)
expect(
makeBadge({
label: 'build',
message: 'passed',
labelColor: undefined,
})
).to.satisfy(isSvg)
})

// This test needs to move up a level.
it('should replace undefined svg template with "flat"', function() {
const jsonBadgeWithUnknownStyle = makeBadge({
label: 'name',
message: 'Bob',
})
const jsonBadgeWithDefaultStyle = makeBadge({
label: 'name',
message: 'Bob',
style: 'flat',
})
expect(jsonBadgeWithUnknownStyle)
.to.equal(jsonBadgeWithDefaultStyle)
.and.to.satisfy(isSvg)
})

it('should throw a ValidationError with invalid inputs', function() {
Expand Down
36 changes: 7 additions & 29 deletions badge-maker/lib/make-badge.js
Original file line number Diff line number Diff line change
@@ -1,49 +1,27 @@
'use strict'

const camelcase = require('camelcase')
const { normalizeColor, toSvgColor } = require('./color')
const { toSvgColor } = require('./color')
const badgeRenderers = require('./badge-renderers')

/*
note: makeBadge() is fairly thinly wrapped so if we are making changes here
it is likely this will impact on the package's public interface in index.js
*/
module.exports = function makeBadge({
format,
template = 'flat',
text,
style,
label,
message,
color,
labelColor,
logo,
logoPosition,
logoWidth,
links = ['', ''],
links,
}) {
// String coercion and whitespace removal.
text = text.map(value => `${value}`.trim())

const [label, message] = text

color = normalizeColor(color)
labelColor = normalizeColor(labelColor)

// This ought to be the responsibility of the server, not `makeBadge`.
if (format === 'json') {
return JSON.stringify({
label,
message,
logoWidth,
color,
labelColor,
link: links,
name: label,
value: message,
})
}

const methodName = camelcase(template)
const methodName = camelcase(style)
if (!(methodName in badgeRenderers)) {
throw new Error(`Unknown template: '${template}'`)
throw new Error(`Unknown style: '${style}'`)
}
const render = badgeRenderers[methodName]

Expand Down
Loading