Add support for [Localazy] badge #5701
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| 'use strict' | ||
|
|
||
| const LocalazyBase = require('./localazy-base.service') | ||
|
|
||
| module.exports = class LocalazyAllProgress extends LocalazyBase { | ||
| static route = this.buildRoute() | ||
|
|
||
| static examples = [ | ||
| { | ||
| title: 'Localazy translation progress', | ||
| keywords: this.keywords, | ||
| documentation: this.documentation, | ||
| namedParams: { | ||
| projectId: 'floating-apps', | ||
| }, | ||
| staticPreview: this.render({ | ||
| message: '61%', | ||
| label: 'translated', | ||
| color: '#066fef', | ||
| }), | ||
| }, | ||
| ] | ||
|
|
||
| async handle({ projectId }) { | ||
| const { message, label, color } = await this.fetch({ | ||
| projectId, | ||
| title: 'translated', | ||
| content: 'progress', | ||
| }) | ||
| return this.constructor.render({ message, label, color }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| 'use strict' | ||
|
|
||
| const t = (module.exports = require('../tester').createServiceTester()) | ||
| const { isPercentage } = require('../test-validators') | ||
|
|
||
| t.create('Localazy translation progress API') | ||
| .get('/floating-apps.json') | ||
| .expectBadge({ | ||
| label: 'translated', | ||
| color: '#066fef', | ||
| message: isPercentage, | ||
| }) | ||
|
|
||
| t.create('Localazy translation progress') | ||
| .get('/floating-apps.json') | ||
| .intercept(nock => | ||
| nock('https://connect.localazy.com') | ||
| .get('/status/floating-apps/data') | ||
| .query({ title: 'translated', content: 'progress' }) | ||
| .reply(200, { | ||
| schemaVersion: 1, | ||
| label: 'translated', | ||
| message: '61%', | ||
| color: '#066fef', | ||
| style: 'flat', | ||
| cacheSeconds: 3600, | ||
| }) | ||
| ) | ||
| .expectBadge({ | ||
| label: 'translated', | ||
| message: '61%', | ||
| color: '#066fef', | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| 'use strict' | ||
|
|
||
| const LocalazyBase = require('./localazy-base.service') | ||
|
|
||
| module.exports = class LocalazyAll extends LocalazyBase { | ||
| static route = this.buildRoute('overall') | ||
|
|
||
| static examples = [ | ||
| { | ||
| title: 'Localazy translation progress with langs', | ||
| keywords: this.keywords, | ||
| documentation: this.documentation, | ||
| namedParams: { | ||
| projectId: 'floating-apps', | ||
| }, | ||
| staticPreview: this.render({ | ||
| message: '61%, 97 langs', | ||
| label: 'translated', | ||
| color: '#066fef', | ||
| }), | ||
| }, | ||
| ] | ||
|
|
||
| async handle({ projectId }) { | ||
| const { message, label, color } = await this.fetch({ | ||
| projectId, | ||
| title: 'translated', | ||
| content: 'all', | ||
| }) | ||
| return this.constructor.render({ message, label, color }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| 'use strict' | ||
|
|
||
| const t = (module.exports = require('../tester').createServiceTester()) | ||
|
|
||
| t.create('Localazy translation progress with langs') | ||
|
There was a problem hiding this comment. Is there any specific reason why most of the tests used mocked responses? Apart from edge cases which are hard to encounter in real life, we generally prefer to hit the real service directly. This gives us much more confidence that the end-to-end flow is working correctly and allows us to be warned sooner rather than later when things unexpectedly change in upstream APIs. There was a problem hiding this comment. Okay, I can remove the mocks, it just depends on what do you want to test here. |
||
| .get('/floating-apps.json') | ||
| .intercept(nock => | ||
| nock('https://connect.localazy.com') | ||
| .get('/status/floating-apps/data') | ||
| .query({ title: 'translated', content: 'all' }) | ||
| .reply(200, { | ||
| schemaVersion: 1, | ||
| label: 'translated', | ||
| message: '61%, 97 langs', | ||
| color: '#066fef', | ||
| style: 'flat', | ||
| cacheSeconds: 3600, | ||
| }) | ||
| ) | ||
| .expectBadge({ | ||
| label: 'translated', | ||
| message: '61%, 97 langs', | ||
| color: '#066fef', | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| 'use strict' | ||
|
|
||
| const Joi = require('joi') | ||
| const { BaseJsonService } = require('..') | ||
|
|
||
| const schema = Joi.object({ | ||
| label: Joi.string(), | ||
| message: Joi.string(), | ||
| color: Joi.string(), | ||
| }) | ||
|
Comment on lines
+6
to
+10
There was a problem hiding this comment. I'm a little surprised by this schema, as it has a very Shields.io feel to it 😄 Are we utilizing an API here that's been designed specifically with our schema in mind or does the native Localazy API just happen to have contracts structured this way? There was a problem hiding this comment. It's following the shields.io structure, so we can easily proxy the calls to your api and generate the badges on-the-fly. Same API is then used by the services here to fetch the raw data. There was a problem hiding this comment.
I'm confused by this. Could you elaborate on what you mean by "your api"? If I was a user of the Localazy platform, and I wanted to integrate with it programmatically how would I go about doing that, what Localazy API(s) would I use? What's currently proposed here is a bit concerning because it looks like the core aspects of the badge rendering are being controlled by the connect.localazy.com endpoint being used which I oppose. Our core mission is providing 'concise, consistent, and legible' badges, and we do that by retrieving the raw data from the upstream system of record and using that data to produce the badges according to our specification and conventions. Ceding control of core default attributes like the badge label, message, and colors to some external endpoint is not going to be acceptable IMO. There was a problem hiding this comment. We've put the api together quickly for this purpose, so users can start using any badges before this gets merged. That's why the api copies the structure. We return these values directly for convenience and faster integration with shields.io. I can move the defaults directly to the repo if that's an issue. There was a problem hiding this comment.
We have a suggested path on how we think we can best proceed, though I want to first do some contextual level setting that I think will be helpful. I tend to view our service's badge offerings in two high level buckets:
That first bucket is the core of what we do, and is a large part of why Shields exists in the first place (more historical background here). We ensure that regardless of which platform/service is in use that the badges are consistent, both in specification and in presentation of information such as metrics and percentages, and status text like the result of a build. In order to be able to provide that consistency across our native badge offerings, we have to control core attributes of those badges we're providing which includes things like the defaults and text, color, etc. This is how we're able to provide a standard build status badge regardless of whether a user is using CI Service Foo or CI Service Bar, and the same holds true for other types of badges like code overage or localization status/progress. Yes, we do support some opt-in customization options that our users can utilize should they choose to override the defaults, but we still have to ensure that the defaults are consistent across our platform. If Shields were to incorporate a native/core badge but not have control over those core attributes then we'd no longer be able to enforce and ensure that consistency and could quickly and easily end up in a place where our native badges are directly violating our own badge specification Ultimately the challenge with this PR in it's current state is that it's proposing adding native/core badges but doing so in the BYOB fashion which just isn't compatible. We can't add a native badge that has the label, message, and color set by some external endpoint that we have no insight into nor control over. We'd love to collaborate with you all and be able to get Localazy incorporated in our native badge offering, though we'll need to pivot a bit to make that happen. The next step to achieving that would be switching the service class implementation to utilize a Localazy API endpoint that directly provides the raw localization progress data. I'm unclear on whether such an endpoint exists yet, but I believe @PyvesB had some suggestions for a contract if not. I also believe that such an endpoint is beneficial above and beyond badges to allow Localazy users to have an API option to interact with the platform indepenent of a badge-specific focus. If such an API doesn't yet exist then we can also work on helping with the implementation on the Shields side to integrate a native badge if you are willing to tackle the API implementation side. I hope this helps, but please let us know if you have any questions! There was a problem hiding this comment. Hello @calebcartwright, There was a problem hiding this comment. From the point of view of a user of the API, I can picture two endpoints:
This would allow Shields.io to be able to control how it uses the data returned by Localazy and ensure consistency with other badges it offers, but it would also allow other consumers of the API to easily make use of it in different ways. 👍🏻 There was a problem hiding this comment. Thanks for your suggestions. We'll do an update of our API and I'll get back to the PR as soon as possible. |
||
|
|
||
| module.exports = class LocalazyBase extends BaseJsonService { | ||
|
There was a problem hiding this comment. Nice extraction of common functionality. 👍🏻 |
||
| static category = 'other' | ||
|
|
||
| static route = this.buildRoute() | ||
|
|
||
| static buildRoute(base = '', pattern = '', query) { | ||
| const baseRoute = { | ||
| base: `localazy${base ? `/${base}` : ''}`, | ||
| pattern: `:projectId${pattern ? `/${pattern}` : ''}`, | ||
| } | ||
| return query ? { ...baseRoute, queryParamSchema: query } : baseRoute | ||
| } | ||
|
|
||
| static get documentation() { | ||
| return `For more badges and detailed documentation of the params visit <a href="https://github.com/localazy/shields">localazy/shields</a> repository.` | ||
| } | ||
|
|
||
| static get keywords() { | ||
| return ['l10n', 'i18n', 'localization', 'internationalization'] | ||
| } | ||
|
Comment on lines
+25
to
+31
There was a problem hiding this comment. Could you convert these to static fields like the rest ( |
||
|
|
||
| static defaultBadgeData = { label: 'translated', color: '#066fef' } | ||
|
|
||
| static render(data) { | ||
| return { | ||
| ...data, | ||
| } | ||
| } | ||
|
|
||
| async fetch({ projectId, title, content }) { | ||
| return this._requestJson({ | ||
| schema, | ||
| url: `https://connect.localazy.com/status/${projectId}/data`, | ||
| options: { | ||
| qs: { | ||
| title, | ||
| content, | ||
| }, | ||
| }, | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| 'use strict' | ||
|
|
||
| const Joi = require('joi') | ||
| const LocalazyBase = require('./localazy-base.service') | ||
|
|
||
| const queryParamSchema = Joi.object({ | ||
| locale_display: Joi.string().allow('loc', 'en').optional(), | ||
| }) | ||
|
|
||
| module.exports = class LocalazyLangProgress extends LocalazyBase { | ||
| static route = this.buildRoute( | ||
| 'lang-progress', | ||
| ':localeCode', | ||
| queryParamSchema | ||
| ) | ||
|
|
||
| static examples = [ | ||
| { | ||
| title: 'Localazy language progress with locale', | ||
| keywords: this.keywords, | ||
| documentation: this.documentation, | ||
| namedParams: { | ||
| projectId: 'floating-apps', | ||
| localeCode: 'pt_BR', | ||
| }, | ||
| staticPreview: this.render({ | ||
| message: '100%', | ||
| label: 'pt_BR', | ||
| color: '#066fef', | ||
| }), | ||
| }, | ||
| { | ||
| title: 'Localazy language progress in English', | ||
| keywords: this.keywords, | ||
| documentation: this.documentation, | ||
| namedParams: { | ||
| projectId: 'floating-apps', | ||
| localeCode: 'pt_BR', | ||
| }, | ||
| staticPreview: this.render({ | ||
| message: '100%', | ||
| label: 'Brazilian Portuguese', | ||
| color: '#066fef', | ||
| }), | ||
| queryParams: { | ||
| locale_display: 'en', | ||
| }, | ||
| }, | ||
| { | ||
| title: 'Localazy language progress with localized name', | ||
| keywords: this.keywords, | ||
| documentation: this.documentation, | ||
| namedParams: { | ||
| projectId: 'floating-apps', | ||
| localeCode: 'pt_BR', | ||
| }, | ||
| staticPreview: this.render({ | ||
| message: '100%', | ||
| label: 'Português (BR)', | ||
|
There was a problem hiding this comment. I like the concept of displaying the language name in the language itself, that's a nice user experience. 👍🏻 |
||
| color: '#066fef', | ||
| }), | ||
| queryParams: { | ||
| locale_display: 'loc', | ||
| }, | ||
| }, | ||
| ] | ||
|
|
||
| static resolveTitleType(display = '') { | ||
| if (display === 'en') { | ||
| return 'lang-name' | ||
| } | ||
| if (display === 'loc') { | ||
| return 'lang-loc-name' | ||
| } | ||
| return 'lang-code' | ||
| } | ||
|
|
||
| async handle({ projectId, localeCode }, { locale_display }) { | ||
| const { message, label, color } = await this.fetch({ | ||
| projectId, | ||
| title: this.constructor.resolveTitleType(locale_display), | ||
| content: localeCode, | ||
| }) | ||
| return this.constructor.render({ message, label, color }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| 'use strict' | ||
|
|
||
| const t = (module.exports = require('../tester').createServiceTester()) | ||
|
|
||
| t.create('Localazy language progress with locale') | ||
| .get('/floating-apps/pt_BR.json') | ||
| .intercept(nock => | ||
| nock('https://connect.localazy.com') | ||
| .get('/status/floating-apps/data') | ||
| .query({ title: 'lang-code', content: 'pt_BR' }) | ||
| .reply(200, { | ||
| schemaVersion: 1, | ||
| label: 'pt_BR', | ||
| message: '61%', | ||
| color: '#066fef', | ||
| style: 'flat', | ||
| cacheSeconds: 3600, | ||
| }) | ||
| ) | ||
| .expectBadge({ | ||
| label: 'pt_BR', | ||
| message: '61%', | ||
| color: '#066fef', | ||
| }) | ||
|
|
||
| t.create('Localazy language progress in English') | ||
| .get('/floating-apps/pt_BR.json?locale_display=en') | ||
| .intercept(nock => | ||
| nock('https://connect.localazy.com') | ||
| .get('/status/floating-apps/data') | ||
| .query({ title: 'lang-name', content: 'pt_BR' }) | ||
| .reply(200, { | ||
| schemaVersion: 1, | ||
| label: 'Brazilian Portuguese', | ||
| message: '61%', | ||
| color: '#066fef', | ||
| style: 'flat', | ||
| cacheSeconds: 3600, | ||
| }) | ||
| ) | ||
| .expectBadge({ | ||
| label: 'Brazilian Portuguese', | ||
| message: '61%', | ||
| color: '#066fef', | ||
| }) | ||
|
|
||
| t.create('Localazy language progress with localized name') | ||
| .get('/floating-apps/pt_BR.json?locale_display=loc') | ||
| .intercept(nock => | ||
| nock('https://connect.localazy.com') | ||
| .get('/status/floating-apps/data') | ||
| .query({ title: 'lang-loc-name', content: 'pt_BR' }) | ||
| .reply(200, { | ||
| schemaVersion: 1, | ||
| label: 'Português (BR)', | ||
| message: '61%', | ||
| color: '#066fef', | ||
| style: 'flat', | ||
| cacheSeconds: 3600, | ||
| }) | ||
| ) | ||
| .expectBadge({ | ||
| label: 'Português (BR)', | ||
| message: '61%', | ||
| color: '#066fef', | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| 'use strict' | ||
|
|
||
| const LocalazyBase = require('./localazy-base.service') | ||
|
|
||
| module.exports = class LocalazyLangsSimple extends LocalazyBase { | ||
|
There was a problem hiding this comment. I'm personally not a fan of this badge variant. It's almost the same as the Maybe merge |
||
| static route = this.buildRoute('langs-simple') | ||
| static examples = [ | ||
| { | ||
| title: 'Localazy simple number of languages', | ||
| keywords: this.keywords, | ||
| documentation: this.documentation, | ||
| namedParams: { | ||
| projectId: 'floating-apps', | ||
| }, | ||
| staticPreview: this.render({ | ||
| message: '97', | ||
| label: 'translated', | ||
| color: '#066fef', | ||
| }), | ||
| }, | ||
| ] | ||
|
|
||
| async handle({ projectId }) { | ||
| const { message, label, color } = await this.fetch({ | ||
| projectId, | ||
| title: 'translated', | ||
| content: 'langs-count', | ||
| }) | ||
| return this.constructor.render({ message, label, color }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| 'use strict' | ||
|
|
||
| const t = (module.exports = require('../tester').createServiceTester()) | ||
|
|
||
| t.create('Localazy simple number of languages') | ||
| .get('/floating-apps.json') | ||
| .intercept(nock => | ||
| nock('https://connect.localazy.com') | ||
| .get('/status/floating-apps/data') | ||
| .query({ title: 'translated', content: 'langs-count' }) | ||
| .reply(200, { | ||
| schemaVersion: 1, | ||
| label: 'translated', | ||
| message: '97', | ||
| color: '#066fef', | ||
| style: 'flat', | ||
| cacheSeconds: 3600, | ||
| }) | ||
| ) | ||
| .expectBadge({ | ||
| label: 'translated', | ||
| message: '97', | ||
| color: '#066fef', | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean the Localazy API returns the colour itself? This seems a little odd to me, Shields.io generally decides it own colours through a set of conventions (e.g. colour gradients for percentages), I'm not sure what to think of an API imposing its own.
What do you think @calebcartwright ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. If there's a need to use a custom scale that'd probably be okay, but the colors used for those tiers should follow our conventions to be consistent with our offering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We return colour to quickly generate the badges on our side, though in this PR, it's always possible to override it as well as the other parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the convention here? The gradient would certainly make sense for single language progress, even though it's not that simple as some code coverage for example. The rest was supposed to be just an informational value, since 100% on the project doesn't mean that the localization process is necessarily finished.