From 6e2992a3e37667075411e06d441ed3eecbfbde97 Mon Sep 17 00:00:00 2001 From: mbarkhau <mbarkhau@gmail.com> Date: Wed, 20 Feb 2019 04:03:45 +0100 Subject: [PATCH] Rename colorB -> color; colorA -> labelColor (#3012) This makes it possible to override the parameters `color` and `labelColor`. ref #2673 --- core/base-service/coalesce-badge.js | 23 +++++++++++-- core/base-service/coalesce-badge.spec.js | 32 +++++++++++++++++-- core/base-service/legacy-request-handler.js | 2 ++ .../legacy-request-handler.spec.js | 6 ++-- .../customizer/query-string-builder.js | 2 +- frontend/components/usage.js | 20 ++++++------ frontend/lib/badge-url.js | 2 +- frontend/lib/badge-url.spec.js | 2 +- frontend/pages/endpoint.js | 5 +-- services/endpoint/endpoint.tester.js | 29 +++++++++++++++++ 10 files changed, 100 insertions(+), 23 deletions(-) diff --git a/core/base-service/coalesce-badge.js b/core/base-service/coalesce-badge.js index 6a809cf785..30819fb814 100644 --- a/core/base-service/coalesce-badge.js +++ b/core/base-service/coalesce-badge.js @@ -41,19 +41,36 @@ module.exports = function coalesceBadge( defaultBadgeData, { category, _cacheLength: defaultCacheSeconds } = {} ) { + // The "overrideX" naming is based on services that provide badge + // parameters themselves, which can be overridden by a query string + // parameter. (For a couple services, the dynamic badge and the + // query-string-based static badge, the service never sets a value + // so the query string overrides are the _only_ way to configure + // these badge parameters. const { style: overrideStyle, label: overrideLabel, logoColor: overrideLogoColor, link: overrideLink, + colorB: legacyOverrideColor, + colorA: legacyOverrideLabelColor, } = overrides - // Scoutcamp converts numeric query params to numbers. Convert them back. let { - colorB: overrideColor, - colorA: overrideLabelColor, logoWidth: overrideLogoWidth, logoPosition: overrideLogoPosition, + color: overrideColor, + labelColor: overrideLabelColor, } = overrides + + // Only use the legacy properties of the new ones are not provided + if (typeof overrideColor === 'undefined') { + overrideColor = legacyOverrideColor + } + if (typeof overrideLabelColor === 'undefined') { + overrideLabelColor = legacyOverrideLabelColor + } + + // Scoutcamp converts numeric query params to numbers. Convert them back. if (typeof overrideColor === 'number') { overrideColor = `${overrideColor}` } diff --git a/core/base-service/coalesce-badge.spec.js b/core/base-service/coalesce-badge.spec.js index 90416ee973..e543a5599d 100644 --- a/core/base-service/coalesce-badge.spec.js +++ b/core/base-service/coalesce-badge.spec.js @@ -58,15 +58,27 @@ describe('coalesceBadge', function() { it('overrides the color', function() { expect( - coalesceBadge({ colorB: '10ADED' }, { color: 'red' }, {}).color + coalesceBadge({ color: '10ADED' }, { color: 'red' }, {}).color ).to.equal('10ADED') + // also expected for legacy name + expect( + coalesceBadge({ colorB: 'B0ADED' }, { color: 'red' }, {}).color + ).to.equal('B0ADED') }) context('In case of an error', function() { it('does not override the color', function() { expect( coalesceBadge( - { colorB: '10ADED' }, + { color: '10ADED' }, + { isError: true, color: 'lightgray' }, + {} + ).color + ).to.equal('lightgray') + // also expected for legacy name + expect( + coalesceBadge( + { colorB: 'B0ADED' }, { isError: true, color: 'lightgray' }, {} ).color @@ -92,11 +104,25 @@ describe('coalesceBadge', function() { it('overrides the label color', function() { expect( - coalesceBadge({ colorA: '42f483' }, { color: 'green' }, {}).labelColor + coalesceBadge({ labelColor: '42f483' }, { color: 'green' }, {}) + .labelColor ).to.equal('42f483') + // also expected for legacy name + expect( + coalesceBadge({ colorA: 'B2f483' }, { color: 'green' }, {}).labelColor + ).to.equal('B2f483') }) it('converts a query-string numeric color to a string', function() { + expect( + coalesceBadge( + // Scoutcamp converts numeric query params to numbers. + { color: 123 }, + { color: 'green' }, + {} + ).color + ).to.equal('123') + // also expected for legacy name expect( coalesceBadge( // Scoutcamp converts numeric query params to numbers. diff --git a/core/base-service/legacy-request-handler.js b/core/base-service/legacy-request-handler.js index 9795d77d54..f5e0903857 100644 --- a/core/base-service/legacy-request-handler.js +++ b/core/base-service/legacy-request-handler.js @@ -52,6 +52,8 @@ const globalQueryParams = new Set([ 'link', 'colorA', 'colorB', + 'color', + 'labelColor', ]) function flattenQueryParams(queryParams) { diff --git a/core/base-service/legacy-request-handler.spec.js b/core/base-service/legacy-request-handler.spec.js index f501a9eed5..b1dbc55195 100644 --- a/core/base-service/legacy-request-handler.spec.js +++ b/core/base-service/legacy-request-handler.spec.js @@ -297,17 +297,17 @@ describe('The request handler', function() { beforeEach(function() { register({ cacheHeaderConfig: standardCacheHeaders }) }) - const expectedCacheKey = '/testing/123.json?colorB=123&label=foo' + const expectedCacheKey = '/testing/123.json?color=123&label=foo' it('should match expected and use canonical order - 1', async function() { const res = await fetch( - `${baseUrl}/testing/123.json?colorB=123&label=foo` + `${baseUrl}/testing/123.json?color=123&label=foo` ) expect(res.ok).to.be.true expect(_requestCache.cache).to.have.keys(expectedCacheKey) }) it('should match expected and use canonical order - 2', async function() { const res = await fetch( - `${baseUrl}/testing/123.json?label=foo&colorB=123` + `${baseUrl}/testing/123.json?label=foo&color=123` ) expect(res.ok).to.be.true expect(_requestCache.cache).to.have.keys(expectedCacheKey) diff --git a/frontend/components/customizer/query-string-builder.js b/frontend/components/customizer/query-string-builder.js index e1aee38ecb..9297c1193f 100644 --- a/frontend/components/customizer/query-string-builder.js +++ b/frontend/components/customizer/query-string-builder.js @@ -26,7 +26,7 @@ const QueryParamCaption = styled(BuilderCaption)` const supportedBadgeOptions = [ { name: 'style', shieldsDefaultValue: 'flat' }, { name: 'label', label: 'override label' }, - { name: 'colorB', label: 'override color' }, + { name: 'color', label: 'override color' }, { name: 'logo', label: 'named logo' }, { name: 'logoColor', label: 'override logo color' }, ] diff --git a/frontend/components/usage.js b/frontend/components/usage.js index 7a88c2fef0..dfb5095442 100644 --- a/frontend/components/usage.js +++ b/frontend/components/usage.js @@ -243,7 +243,7 @@ export default class Usage extends React.PureComponent { > $.DATA.SUBDATA </a> - >&colorB=<COLOR>&prefix=<PREFIX>&suffix=<SUFFIX> + >&color=<COLOR>&prefix=<PREFIX>&suffix=<SUFFIX> </StyledCode> </p> <p> @@ -257,7 +257,7 @@ export default class Usage extends React.PureComponent { > //data/subdata </a> - >&colorB=<COLOR>&prefix=<PREFIX>&suffix=<SUFFIX> + >&color=<COLOR>&prefix=<PREFIX>&suffix=<SUFFIX> </StyledCode> </p> <p> @@ -271,7 +271,7 @@ export default class Usage extends React.PureComponent { > $.DATA.SUBDATA </a> - >&colorB=<COLOR>&prefix=<PREFIX>&suffix=<SUFFIX> + >&color=<COLOR>&prefix=<PREFIX>&suffix=<SUFFIX> </StyledCode> </p> @@ -352,22 +352,24 @@ export default class Usage extends React.PureComponent { } /> <QueryParam - key="colorA" - snippet="?colorA=abcdef" + key="labelColor" + snippet="?labelColor=abcdef" documentation={ <span> Set background of the left part (hex, rgb, rgba, hsl, hsla and - css named colors supported) + css named colors supported). The legacy name "colorA" is also + supported. </span> } /> <QueryParam - key="colorB" - snippet="?colorB=fedcba" + key="color" + snippet="?color=fedcba" documentation={ <span> Set background of the right part (hex, rgb, rgba, hsl, hsla - and css named colors supported) + and css named colors supported). The legacy name "colorB" is + also supported. </span> } /> diff --git a/frontend/lib/badge-url.js b/frontend/lib/badge-url.js index d6f5c6c5ad..5ff3aca4c7 100644 --- a/frontend/lib/badge-url.js +++ b/frontend/lib/badge-url.js @@ -38,7 +38,7 @@ export function dynamicBadgeUrl( }) if (color) { - queryParams.colorB = color + queryParams.color = color } if (prefix) { queryParams.prefix = prefix diff --git a/frontend/lib/badge-url.spec.js b/frontend/lib/badge-url.spec.js index 2265a80e06..4817e4cb13 100644 --- a/frontend/lib/badge-url.spec.js +++ b/frontend/lib/badge-url.spec.js @@ -58,7 +58,7 @@ describe('Badge URL functions', function() { '?label=foo', `&url=${encodeURIComponent(dataUrl)}`, `&query=${encodeURIComponent(query)}`, - '&colorB=blue', + '&color=blue', `&suffix=${encodeURIComponent(suffix)}`, '&style=plastic', ].join('') diff --git a/frontend/pages/endpoint.js b/frontend/pages/endpoint.js index 2ed6e34a6e..71cc07282d 100644 --- a/frontend/pages/endpoint.js +++ b/frontend/pages/endpoint.js @@ -173,11 +173,12 @@ const EndpointPage = () => ( <dd> Default: <code>lightgrey</code>. The right color. Supports the eight named colors above, as well as hex, rgb, rgba, hsl, hsla and css named - colors. + colors. This can be overridden by the query string. </dd> <dt>labelColor</dt> <dd> - Default: <code>grey</code>. The left color. + Default: <code>grey</code>. The left color. This can be overridden by + the query string. </dd> <dt>isError</dt> <dd> diff --git a/services/endpoint/endpoint.tester.js b/services/endpoint/endpoint.tester.js index 2d8fd9a4c2..371969bc13 100644 --- a/services/endpoint/endpoint.tester.js +++ b/services/endpoint/endpoint.tester.js @@ -170,6 +170,20 @@ t.create('Invalid schema (mocked)') }) t.create('User color overrides success color') + .get('.json?url=https://example.com/badge&color=101010&style=_shields_test') + .intercept(nock => + nock('https://example.com/') + .get('/badge') + .reply(200, { + schemaVersion: 1, + label: '', + message: 'yo', + color: 'blue', + }) + ) + .expectJSON({ name: '', value: 'yo', color: '#101010' }) + +t.create('User legacy color overrides success color') .get('.json?url=https://example.com/badge&colorB=101010&style=_shields_test') .intercept(nock => nock('https://example.com/') @@ -184,6 +198,21 @@ t.create('User color overrides success color') .expectJSON({ name: '', value: 'yo', color: '#101010' }) t.create('User color does not override error color') + .get('.json?url=https://example.com/badge&color=101010&style=_shields_test') + .intercept(nock => + nock('https://example.com/') + .get('/badge') + .reply(200, { + schemaVersion: 1, + isError: true, + label: 'something is', + message: 'not right', + color: 'red', + }) + ) + .expectJSON({ name: 'something is', value: 'not right', color: 'red' }) + +t.create('User legacy color does not override error color') .get('.json?url=https://example.com/badge&colorB=101010&style=_shields_test') .intercept(nock => nock('https://example.com/') -- GitLab