Repository: badges/shields
Pull request #3119 (2019-02-28 01:46:35)

Rewrite [GitHubStars]

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 b1d9446bc8a835cd2299d4daf4185fc45792d5c1
</p>


Patch:
diff --git a/core/base-service/base.js b/core/base-service/base.js
index f8460958d1..2b7352bec1 100644
--- a/core/base-service/base.js
+++ b/core/base-service/base.js
@@ -49,7 +49,10 @@ const serviceDataSchema = Joi.object({
   // `render()` to always return a string.
   message: Joi.alternatives(Joi.string().allow(''), Joi.number()).required(),
   color: Joi.string(),
-  link: Joi.string().uri(),
+  link: Joi.array()
+    .items(Joi.string().uri())
+    .single()
+    .max(2),
   // Generally services should not use these options, which are provided to
   // support the Endpoint badge.
   labelColor: Joi.string(),
diff --git a/core/base-service/base.spec.js b/core/base-service/base.spec.js
index a3c06f63ed..6cd39c5bcd 100644
--- a/core/base-service/base.spec.js
+++ b/core/base-service/base.spec.js
@@ -170,6 +170,52 @@ describe('BaseService', function() {
     })
   })
 
+  describe('Service data validation', function() {
+    it('Allows a link array', async function() {
+      const message = 'hello'
+      const link = ['https://example.com/', 'https://other.example.com/']
+      class LinkService extends DummyService {
+        async handle() {
+          return { message, link }
+        }
+      }
+
+      const serviceData = await LinkService.invoke(
+        {},
+        { handleInternalErrors: false },
+        { namedParamA: 'bar.bar.bar' }
+      )
+
+      expect(serviceData).to.deep.equal({
+        message,
+        link,
+      })
+    })
+
+    it('Throws a validation error on invalid data', async function() {
+      class ThrowingService extends DummyService {
+        async handle() {
+          return {
+            some: 'nonsense',
+          }
+        }
+      }
+      try {
+        await ThrowingService.invoke(
+          {},
+          { handleInternalErrors: false },
+          { namedParamA: 'bar.bar.bar' }
+        )
+        expect.fail('Expected to throw')
+      } catch (e) {
+        expect(e.name).to.equal('ValidationError')
+        expect(e.details.map(({ message }) => message)).to.deep.equal([
+          '"message" is required',
+        ])
+      }
+    })
+  })
+
   describe('Error handling', function() {
     it('Handles internal errors', async function() {
       class ThrowingService extends DummyService {
@@ -191,31 +237,6 @@ describe('BaseService', function() {
       })
     })
 
-    context('handle() returns invalid data', function() {
-      it('Throws a validation error', async function() {
-        class ThrowingService extends DummyService {
-          async handle() {
-            return {
-              some: 'nonsense',
-            }
-          }
-        }
-        try {
-          await ThrowingService.invoke(
-            {},
-            { handleInternalErrors: false },
-            { namedParamA: 'bar.bar.bar' }
-          )
-          expect.fail('Expected to throw')
-        } catch (e) {
-          expect(e.name).to.equal('ValidationError')
-          expect(e.details.map(({ message }) => message)).to.deep.equal([
-            '"message" is required',
-          ])
-        }
-      })
-    })
-
     describe('Handles known subtypes of ShieldsInternalError', function() {
       it('handles NotFound errors', async function() {
         class ThrowingService extends DummyService {
diff --git a/services/github/github-stars.service.js b/services/github/github-stars.service.js
index d65d230340..d5c1f72cfa 100644
--- a/services/github/github-stars.service.js
+++ b/services/github/github-stars.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 Joi = require('joi')
 const { metric } = require('../text-formatters')
-const {
-  documentation,
-  checkErrorResponse: githubCheckErrorResponse,
-} = require('./github-helpers')
+const { nonNegativeInteger } = require('../validators')
+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 GithubStars extends LegacyService {
+const schema = Joi.object({
+  stargazers_count: nonNegativeInteger,
+}).required()
+
+module.exports = class GithubStars extends GithubAuthService {
   static get category() {
     return 'social'
   }
@@ -36,6 +31,8 @@ module.exports = class GithubStars extends LegacyService {
           repo: 'shields',
         },
         queryParams: { style: 'social' },
+        // TODO: This is currently a literal, as `staticPreview` doesn't
+        // support `link`.
         staticPreview: {
           label: 'Stars',
           message: '7k',
@@ -48,41 +45,28 @@ module.exports = class GithubStars extends LegacyService {
 
   static get defaultBadgeData() {
     return {
+      label: 'stars',
       namedLogo: 'github',
     }
   }
 
-  static registerLegacyRouteHandler({ camp, cache, githubApiProvider }) {
-    camp.route(
-      /^\/github\/stars\/([^/]+)\/([^/]+)\.(svg|png|gif|jpg|json)$/,
-      cache((data, match, sendBadge, request) => {
-        const user = match[1] // eg, qubyte/rubidium
-        const repo = match[2]
-        const format = match[3]
-        const apiUrl = `/repos/${user}/${repo}`
-        const badgeData = getBadgeData('stars', data)
-        if (badgeData.template === 'social') {
-          badgeData.logo = getLogo('github', data)
-          badgeData.links = [
-            `https://github.com/${user}/${repo}`,
-            `https://github.com/${user}/${repo}/stargazers`,
-          ]
-        }
-        githubApiProvider.request(request, apiUrl, {}, (err, res, buffer) => {
-          if (githubCheckErrorResponse(badgeData, err, res)) {
-            sendBadge(format, badgeData)
-            return
-          }
-          try {
-            badgeData.text[1] = metric(JSON.parse(buffer).stargazers_count)
-            badgeData.colorscheme = 'blue'
-            sendBadge(format, badgeData)
-          } catch (e) {
-            badgeData.text[1] = 'invalid'
-            sendBadge(format, badgeData)
-          }
-        })
-      })
-    )
+  static render({ stars, user, repo }) {
+    return {
+      message: metric(stars),
+      color: 'blue',
+      link: [
+        `https://github.com/${user}/${repo}`,
+        `https://github.com/${user}/${repo}/stargazers`,
+      ],
+    }
+  }
+
+  async handle({ user, repo }) {
+    const { stargazers_count: stars } = await this._requestJson({
+      url: `/repos/${user}/${repo}`,
+      schema,
+      errorMessages: errorMessagesFor(),
+    })
+    return this.constructor.render({ user, repo, stars })
   }
 }


Test patch:
diff --git a/services/github/github-stars.tester.js b/services/github/github-stars.tester.js
index b6c4505fd9..6f6f8794e3 100644
--- a/services/github/github-stars.tester.js
+++ b/services/github/github-stars.tester.js
@@ -16,19 +16,3 @@ t.create('Stars (repo not found)')
     label: 'stars',
     message: 'repo not found',
   })
-
-t.create('Stars (named color override)')
-  .get('/badges/shields.json?color=yellow&style=_shields_test')
-  .expectBadge({
-    label: 'stars',
-    message: Joi.string().regex(/^\w+$/),
-    color: 'yellow',
-  })
-
-t.create('Stars (hex color override)')
-  .get('/badges/shields.json?color=abcdef&style=_shields_test')
-  .expectBadge({
-    label: 'stars',
-    message: Joi.string().regex(/^\w+$/),
-    color: '#abcdef',
-  })