From cd0ff105f68a112ae56c133637b06de5bf32b821 Mon Sep 17 00:00:00 2001
From: Paul Melnikow <github@paulmelnikow.com>
Date: Wed, 9 Jan 2019 16:32:28 -0500
Subject: [PATCH] User color should not override error color (#2693)
Fix #1446.
---
services/base-json.spec.js | 2 ++
services/base-svg-scraping.spec.js | 1 +
services/base-xml.spec.js | 2 ++
services/base-yaml.spec.js | 2 ++
services/base.js | 19 ++++++++---
services/base.spec.js | 17 +++++++++-
services/dynamic/dynamic-json.tester.js | 15 ++++-----
services/dynamic/dynamic-xml.tester.js | 36 ++++++++++----------
services/dynamic/dynamic-yaml.tester.js | 45 ++++++++++++-------------
9 files changed, 83 insertions(+), 56 deletions(-)
diff --git a/services/base-json.spec.js b/services/base-json.spec.js
index bc8ba92a10..41bcc32306 100644
--- a/services/base-json.spec.js
+++ b/services/base-json.spec.js
@@ -109,6 +109,7 @@ describe('BaseJsonService', function() {
{ handleInternalErrors: false }
)
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'invalid response data',
})
@@ -125,6 +126,7 @@ describe('BaseJsonService', function() {
{ handleInternalErrors: false }
)
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'unparseable json response',
})
diff --git a/services/base-svg-scraping.spec.js b/services/base-svg-scraping.spec.js
index cfa37b2bce..da484af58e 100644
--- a/services/base-svg-scraping.spec.js
+++ b/services/base-svg-scraping.spec.js
@@ -157,6 +157,7 @@ describe('BaseSvgScrapingService', function() {
{ handleInternalErrors: false }
)
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'unparseable svg response',
})
diff --git a/services/base-xml.spec.js b/services/base-xml.spec.js
index 5c3ad696ab..a02ac9709c 100644
--- a/services/base-xml.spec.js
+++ b/services/base-xml.spec.js
@@ -136,6 +136,7 @@ describe('BaseXmlService', function() {
{ handleInternalErrors: false }
)
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'invalid response data',
})
@@ -152,6 +153,7 @@ describe('BaseXmlService', function() {
{ handleInternalErrors: false }
)
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'unparseable xml response',
})
diff --git a/services/base-yaml.spec.js b/services/base-yaml.spec.js
index 134236472b..b7a6be4d25 100644
--- a/services/base-yaml.spec.js
+++ b/services/base-yaml.spec.js
@@ -132,6 +132,7 @@ describe('BaseYamlService', function() {
{ handleInternalErrors: false }
)
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'invalid response data',
})
@@ -148,6 +149,7 @@ describe('BaseYamlService', function() {
{ handleInternalErrors: false }
)
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'unparseable yaml response',
})
diff --git a/services/base.js b/services/base.js
index 5b1329529f..86aa7f1bbc 100644
--- a/services/base.js
+++ b/services/base.js
@@ -244,6 +244,7 @@ class BaseService {
if (error instanceof NotFound || error instanceof InvalidParameter) {
trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
return {
+ isError: true,
message: error.prettyMessage,
color: 'red',
}
@@ -254,6 +255,7 @@ class BaseService {
) {
trace.logTrace('outbound', emojic.noGoodWoman, 'Handled error', error)
return {
+ isError: true,
message: error.prettyMessage,
color: 'lightgray',
}
@@ -271,6 +273,7 @@ class BaseService {
console.log(error)
}
return {
+ isError: true,
label: 'shields',
message: 'internal error',
color: 'lightgray',
@@ -318,11 +321,12 @@ class BaseService {
logoColor: overrideLogoColor,
logoWidth: overrideLogoWidth,
link: overrideLink,
- colorA: overrideColorA,
- colorB: overrideColorB,
+ colorA: overrideLabelColor,
+ colorB: overrideColor,
} = overrides
const {
+ isError,
label: serviceLabel,
message: serviceMessage,
color: serviceColor,
@@ -349,9 +353,16 @@ class BaseService {
}),
logoWidth: +overrideLogoWidth,
links: toArray(overrideLink || serviceLink),
- colorA: makeColor(overrideColorA),
+ colorA: makeColor(overrideLabelColor),
+ }
+
+ let color
+ if (isError) {
+ // Disregard the override color.
+ color = coalesce(serviceColor, defaultColor, 'lightgrey')
+ } else {
+ color = coalesce(overrideColor, serviceColor, defaultColor, 'lightgrey')
}
- const color = overrideColorB || serviceColor || defaultColor || 'lightgrey'
setBadgeColor(badgeData, color)
return badgeData
diff --git a/services/base.spec.js b/services/base.spec.js
index f8c954211f..7de1269aff 100644
--- a/services/base.spec.js
+++ b/services/base.spec.js
@@ -260,6 +260,7 @@ describe('BaseService', function() {
{ namedParamA: 'bar.bar.bar' }
)
).to.deep.equal({
+ isError: true,
color: 'lightgray',
label: 'shields',
message: 'internal error',
@@ -276,6 +277,7 @@ describe('BaseService', function() {
expect(
await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' })
).to.deep.equal({
+ isError: true,
color: 'red',
message: 'not found',
})
@@ -290,6 +292,7 @@ describe('BaseService', function() {
expect(
await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' })
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'inaccessible',
})
@@ -304,6 +307,7 @@ describe('BaseService', function() {
expect(
await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' })
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'invalid',
})
@@ -318,6 +322,7 @@ describe('BaseService', function() {
expect(
await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' })
).to.deep.equal({
+ isError: true,
color: 'lightgray',
message: 'no longer available',
})
@@ -332,6 +337,7 @@ describe('BaseService', function() {
expect(
await ThrowingService.invoke({}, {}, { namedParamA: 'bar.bar.bar' })
).to.deep.equal({
+ isError: true,
color: 'red',
message: 'invalid parameter',
})
@@ -357,7 +363,7 @@ describe('BaseService', function() {
expect(badgeData.colorA).to.equal('#42f483')
})
- it('overrides the colorB', function() {
+ it('overrides the color', function() {
const badgeData = DummyService._makeBadgeData(
{ colorB: '10ADED' },
{ color: 'red' }
@@ -365,6 +371,15 @@ describe('BaseService', function() {
expect(badgeData.colorB).to.equal('#10ADED')
})
+ it('does not override the color in case of an error', function() {
+ const badgeData = DummyService._makeBadgeData(
+ { colorB: '10ADED' },
+ { isError: true, color: 'lightgray' }
+ )
+ expect(badgeData.colorB).to.be.undefined
+ expect(badgeData.colorscheme).to.equal('lightgray')
+ })
+
it('overrides the logo', function() {
const expLogo =
'data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSI0MCIgaGVpZ2h0PSI0MCIgdmlld0JveD0iMTIgMTIgNDAgNDAiPgo8cGF0aCBmaWxsPSIjMzMzMzMzIiBkPSJNMzIsMTMuNGMtMTAuNSwwLTE5LDguNS0xOSwxOWMwLDguNCw1LjUsMTUuNSwxMywxOGMxLDAuMiwxLjMtMC40LDEuMy0wLjljMC0wLjUsMC0xLjcsMC0zLjIgYy01LjMsMS4xLTYuNC0yLjYtNi40LTIuNkMyMCw0MS42LDE4LjgsNDEsMTguOCw0MWMtMS43LTEuMiwwLjEtMS4xLDAuMS0xLjFjMS45LDAuMSwyLjksMiwyLjksMmMxLjcsMi45LDQuNSwyLjEsNS41LDEuNiBjMC4yLTEuMiwwLjctMi4xLDEuMi0yLjZjLTQuMi0wLjUtOC43LTIuMS04LjctOS40YzAtMi4xLDAuNy0zLjcsMi01LjFjLTAuMi0wLjUtMC44LTIuNCwwLjItNWMwLDAsMS42LTAuNSw1LjIsMiBjMS41LTAuNCwzLjEtMC43LDQuOC0wLjdjMS42LDAsMy4zLDAuMiw0LjcsMC43YzMuNi0yLjQsNS4yLTIsNS4yLTJjMSwyLjYsMC40LDQuNiwwLjIsNWMxLjIsMS4zLDIsMywyLDUuMWMwLDcuMy00LjUsOC45LTguNyw5LjQgYzAuNywwLjYsMS4zLDEuNywxLjMsMy41YzAsMi42LDAsNC42LDAsNS4yYzAsMC41LDAuNCwxLjEsMS4zLDAuOWM3LjUtMi42LDEzLTkuNywxMy0xOC4xQzUxLDIxLjksNDIuNSwxMy40LDMyLDEzLjR6Ii8+Cjwvc3ZnPgo='
diff --git a/services/dynamic/dynamic-json.tester.js b/services/dynamic/dynamic-json.tester.js
index 93fb7835f7..1370b61527 100644
--- a/services/dynamic/dynamic-json.tester.js
+++ b/services/dynamic/dynamic-json.tester.js
@@ -129,14 +129,13 @@ t.create('JSON from url | error color overrides default')
colorB: colorsB.red,
})
-// FIXME This is a regression which should be fixed in BaseService.
-// t.create('JSON from url | error color overrides user specified')
-// .get('.json?query=$.version&colorB=10ADED&style=_shields_test')
-// .expectJSON({
-// name: 'custom badge',
-// value: 'invalid query parameter: url',
-// colorB: colorsB.red,
-// })
+t.create('JSON from url | error color overrides user specified')
+ .get('.json?query=$.version&colorB=10ADED&style=_shields_test')
+ .expectJSON({
+ name: 'custom badge',
+ value: 'invalid query parameter: url',
+ colorB: colorsB.red,
+ })
let headers
t.create('JSON from url | request should set Accept header')
diff --git a/services/dynamic/dynamic-xml.tester.js b/services/dynamic/dynamic-xml.tester.js
index 2690931b1a..ecd567bf55 100644
--- a/services/dynamic/dynamic-xml.tester.js
+++ b/services/dynamic/dynamic-xml.tester.js
@@ -131,25 +131,23 @@ t.create('XML from url | user color overrides default')
colorB: '#10ADED',
})
-// bug: https://github.com/badges/shields/issues/1446
-// t.create('XML from url | error color overrides default')
-// .get(
-// '.json?url=https://github.com/badges/shields/raw/master/notafile.xml&query=//version&style=_shields_test'
-// )
-// .expectJSON({
-// name: 'custom badge',
-// value: 'resource not found',
-// colorB: colorsB.lightgrey,
-// })
-
-// bug: https://github.com/badges/shields/issues/1446
-// t.create('XML from url | error color overrides user specified')
-// .get('.json?query=//version&colorB=10ADED&style=_shields_test')
-// .expectJSON({
-// name: 'custom badge',
-// value: 'invalid query parameter: url',
-// colorB: colorsB.red,
-// })
+t.create('XML from url | error color overrides default')
+ .get(
+ '.json?url=https://github.com/badges/shields/raw/master/notafile.xml&query=//version&style=_shields_test'
+ )
+ .expectJSON({
+ name: 'custom badge',
+ value: 'resource not found',
+ colorB: colorsB.red,
+ })
+
+t.create('XML from url | error color overrides user specified')
+ .get('.json?query=//version&colorB=10ADED&style=_shields_test')
+ .expectJSON({
+ name: 'custom badge',
+ value: 'invalid query parameter: url',
+ colorB: colorsB.red,
+ })
let headers
t.create('XML from url | request should set Accept header')
diff --git a/services/dynamic/dynamic-yaml.tester.js b/services/dynamic/dynamic-yaml.tester.js
index 08aaa0b9ff..d88ab35cef 100644
--- a/services/dynamic/dynamic-yaml.tester.js
+++ b/services/dynamic/dynamic-yaml.tester.js
@@ -80,29 +80,26 @@ t.create('YAML from url | invalid url')
colorB: colorsB.red,
})
-// bug: https://github.com/badges/shields/issues/1446
-// t.create('YAML from url | user color overrides default')
-// .get(
-// '.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&query=$.name&colorB=10ADED&style=_shields_test'
-// )
-// .expectJSON({ name: 'custom badge', value: 'coredns', colorB: '#10ADED' })
+t.create('YAML from url | user color overrides default')
+ .get(
+ '.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/Chart.yaml&query=$.name&colorB=10ADED&style=_shields_test'
+ )
+ .expectJSON({ name: 'custom badge', value: 'coredns', colorB: '#10ADED' })
-// bug: https://github.com/badges/shields/issues/1446
-// t.create('YAML from url | error color overrides default')
-// .get(
-// '.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/notafile.yaml&query=$.version&style=_shields_test'
-// )
-// .expectJSON({
-// name: 'custom badge',
-// value: 'resource not found',
-// colorB: colorsB.lightgrey,
-// })
+t.create('YAML from url | error color overrides default')
+ .get(
+ '.json?url=https://raw.githubusercontent.com/kubernetes/charts/568291d6e476c39ca8322c30c3f601d0383d4760/stable/coredns/notafile.yaml&query=$.version&style=_shields_test'
+ )
+ .expectJSON({
+ name: 'custom badge',
+ value: 'resource not found',
+ colorB: colorsB.red,
+ })
-// bug: https://github.com/badges/shields/issues/1446
-// t.create('YAML from url | error color overrides user specified')
-// .get('.json?query=$.version&colorB=10ADED&style=_shields_test')
-// .expectJSON({
-// name: 'custom badge',
-// value: 'invalid query parameter: url',
-// colorB: colorsB.red,
-// })
+t.create('YAML from url | error color overrides user specified')
+ .get('.json?query=$.version&colorB=10ADED&style=_shields_test')
+ .expectJSON({
+ name: 'custom badge',
+ value: 'invalid query parameter: url',
+ colorB: colorsB.red,
+ })
--
GitLab