Repository: badges/shields
Pull request #3095 (2019-02-24 19:33:31)

Rewrite [GitHubLicense]

shields-ci: 
<!--
  0 failure: 
  0 warning: 
  1 messages
  
  DangerID: danger-id-default;
-->



<table>
  <thead>
    <tr>
      <th width="50"></th>
      <th width="100%" data-danger-table="true">Messages</th>
    </tr>
  </thead>
  <tbody><tr>
      <td>:book:</td>
      <td>:sparkles: Thanks for your contribution to Shields, @paulmelnikow!</td>
    </tr>
  </tbody>
</table>


<p align="right">
  Generated by :no_entry_sign: <a href="https://danger.systems/js">dangerJS</a> against 2e0924c038a34760e0dbdb691fb74da83783f685
</p>


Patch:
diff --git a/services/github/github-license.service.js b/services/github/github-license.service.js
index e8f83e66de..6f35df555e 100644
--- a/services/github/github-license.service.js
+++ b/services/github/github-license.service.js
@@ -1,21 +1,16 @@
 'use strict'
 
-const LegacyService = require('../legacy-service')
-const { makeBadgeData: getBadgeData } = require('../../lib/badge-data')
-const { makeLogo: getLogo } = require('../../lib/logos')
-const { licenseToColor } = require('../../lib/licenses')
-const {
-  documentation,
-  checkErrorResponse: githubCheckErrorResponse,
-} = require('./github-helpers')
+const Joi = require('joi')
+const { renderLicenseBadge } = require('../../lib/licenses')
+const { GithubAuthService } = require('./github-auth-service')
+const { documentation, errorMessagesFor } = require('./github-helpers')
 
-// This legacy service should be rewritten to use e.g. BaseJsonService.
-//
-// Tips for rewriting:
-// https://github.com/badges/shields/blob/master/doc/rewriting-services.md
-//
-// Do not base new services on this code.
-module.exports = class GithubLicense extends LegacyService {
+const schema = Joi.object({
+  // Some repos do not have a license, in which case GitHub returns `{ license: null }`.
+  license: Joi.object({ spdx_id: Joi.string().required() }).allow(null),
+}).required()
+
+module.exports = class GithubLicense extends GithubAuthService {
   static get category() {
     return 'license'
   }
@@ -43,49 +38,30 @@ module.exports = class GithubLicense extends LegacyService {
     ]
   }
 
-  static registerLegacyRouteHandler({ camp, cache, githubApiProvider }) {
-    camp.route(
-      /^\/github\/license\/([^/]+)\/([^/]+)\.(svg|png|gif|jpg|json)$/,
-      cache((data, match, sendBadge, request) => {
-        const user = match[1] // eg, mashape
-        const repo = match[2] // eg, apistatus
-        const format = match[3]
-        const apiUrl = `/repos/${user}/${repo}`
-        const badgeData = getBadgeData('license', data)
-        if (badgeData.template === 'social') {
-          badgeData.logo = getLogo('github', data)
-        }
-        githubApiProvider.request(request, apiUrl, {}, (err, res, buffer) => {
-          if (
-            githubCheckErrorResponse(badgeData, err, res, 'repo not found', {
-              403: 'access denied',
-            })
-          ) {
-            sendBadge(format, badgeData)
-            return
-          }
-          try {
-            const body = JSON.parse(buffer)
-            const license = body.license
-            if (license != null) {
-              if (!license.spdx_id || license.spdx_id === 'NOASSERTION') {
-                badgeData.text[1] = 'unknown'
-              } else {
-                badgeData.text[1] = license.spdx_id
-              }
-              badgeData.colorB = licenseToColor(license.spdx_id)
-              sendBadge(format, badgeData)
-            } else {
-              badgeData.text[1] = 'missing'
-              badgeData.colorscheme = 'red'
-              sendBadge(format, badgeData)
-            }
-          } catch (e) {
-            badgeData.text[1] = 'invalid'
-            sendBadge(format, badgeData)
-          }
-        })
-      })
-    )
+  static get defaultBadgeData() {
+    return {
+      label: 'license',
+    }
+  }
+
+  static render({ license }) {
+    if (license === 'NOASSERTION') {
+      return { message: 'not identifiable by github' }
+    } else if (license) {
+      return renderLicenseBadge({ license })
+    } else {
+      return { message: 'not specified' }
+    }
+  }
+
+  async handle({ user, repo }) {
+    const { license: licenseObject } = await this._requestJson({
+      schema,
+      url: `/repos/${user}/${repo}`,
+      errorMessages: errorMessagesFor('repo not found'),
+    })
+
+    const license = licenseObject ? licenseObject.spdx_id : undefined
+    return this.constructor.render({ license })
   }
 }
diff --git a/services/github/github-license.spec.js b/services/github/github-license.spec.js
new file mode 100644
index 0000000000..82359204cc
--- /dev/null
+++ b/services/github/github-license.spec.js
@@ -0,0 +1,12 @@
+'use strict'
+
+const { test, given } = require('sazerac')
+const GithubLicense = require('./github-license.service')
+
+test(GithubLicense.render, () => {
+  given({ license: undefined }).expect({ message: 'not specified' })
+  given({ license: 'NOASSERTION' }).expect({
+    message: 'not identifiable by github',
+  })
+  given({ license: 'MIT' }).expect({ message: 'MIT', color: 'green' })
+})


Test patch:
diff --git a/services/github/github-license.tester.js b/services/github/github-license.tester.js
index f2eb11dd00..c482ce80f0 100644
--- a/services/github/github-license.tester.js
+++ b/services/github/github-license.tester.js
@@ -4,11 +4,9 @@ const { licenseToColor } = require('../../lib/licenses')
 const t = (module.exports = require('../tester').createServiceTester())
 
 const publicDomainLicenseColor = licenseToColor('CC0-1.0')
-const permissiveLicenseColor = licenseToColor('MIT')
-const copyleftLicenseColor = licenseToColor('GPL-3.0')
 const unknownLicenseColor = licenseToColor()
 
-t.create('Public domain license')
+t.create('License')
   .get('/github/gitignore.json?style=_shields_test')
   .expectJSON({
     name: 'license',
@@ -16,27 +14,15 @@ t.create('Public domain license')
     color: `#${publicDomainLicenseColor}`,
   })
 
-t.create('Copyleft license')
-  .get('/ansible/ansible.json?style=_shields_test')
-  .expectJSON({
-    name: 'license',
-    value: 'GPL-3.0',
-    color: copyleftLicenseColor,
-  })
-
-t.create('Permissive license')
-  .get('/atom/atom.json?style=_shields_test')
-  .expectJSON({ name: 'license', value: 'MIT', color: permissiveLicenseColor })
-
 t.create('License for repo without a license')
   .get('/badges/badger.json?style=_shields_test')
-  .expectJSON({ name: 'license', value: 'missing', color: 'red' })
+  .expectJSON({ name: 'license', value: 'not specified', color: 'lightgrey' })
 
 t.create('License for repo with an unrecognized license')
   .get('/philokev/sopel-noblerealms.json?style=_shields_test')
   .expectJSON({
     name: 'license',
-    value: 'unknown',
+    value: 'not identifiable by github',
     color: unknownLicenseColor,
   })
 
@@ -68,23 +54,5 @@ t.create('License for unknown repo')
   .expectJSON({
     name: 'license',
     value: 'repo not found',
-    color: 'lightgrey',
-  })
-
-t.create('License - API rate limit exceeded')
-  .get('/user1/repo1.json?style=_shields_test')
-  .intercept(nock =>
-    nock('https://api.github.com')
-      .get('/repos/user1/repo1')
-      .query(true)
-      .reply(403, {
-        message:
-          "API rate limit exceeded for 123.123.123.123. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",
-        documentation_url: 'https://developer.github.com/v3/#rate-limiting',
-      })
-  )
-  .expectJSON({
-    name: 'license',
-    value: 'access denied',
-    color: 'lightgrey',
+    color: 'red',
   })