Declare static examples using namedParams (#2308)

This continues the work from #2279, by allowing example badges to be specified using `namedParams`. Using an object makes it possible for us to display these in form fields down the line. (#701) I've called this the "preferred" way, and labeled the other ways deprecated. I've also added some doc to the `examples` property in BaseService. Then I realized we had some doc in the tutorial, though I think it's fine to have a short version in the tutorial, and the gory detail in BaseService. I've also added a `pattern` keyword, and made `urlPattern` an alias. Closes #2050.
2018-11-17 09:47:25 -05:00
parent 00d5f87a77
commit 84a5be3946
10 changed files with 230 additions and 78 deletions
--- a/doc/TUTORIAL.md
+++ b/doc/TUTORIAL.md
@@ -255,9 +255,8 @@ module.exports = class GemVersion extends BaseJsonService {
    return [
      {                                                         // (3)
        title: 'Gem',
-        urlPattern: ':package',
+        namedParams: { gem: 'formatador' },
        staticExample: this.render({ version: '2.1.0' }),
-        exampleUrl: 'formatador',
        keywords: ['ruby'],
      },
    ]
@@ -270,9 +269,9 @@ module.exports = class GemVersion extends BaseJsonService {
 2. The examples property defines an array of examples. In this case the array will contain a single object, but in some cases it is helpful to provide multiple usage examples.
 3. Our example object should contain the following properties:
    * `title`: Descriptive text that will be shown next to the badge
-    * `urlPattern`: Describe the variable part of the route using `:param` syntax.
+    * `namedParams`: Provide a valid example of params we can substitute into
+      the pattern. In this case we need a valid ruby gem, so we've picked [formatador](https://rubygems.org/gems/formatador).
    * `staticExample`: On the index page we want to show an example badge, but for performance reasons we want that example to be generated without making an API call. `staticExample` should be populated by calling our `render()` method with some valid data.
-    * `exampleUrl`: Provide a valid example of params we can call the badge with. In this case we need a valid ruby gem, so we've picked [formatador](https://rubygems.org/gems/formatador)
    * `keywords`: If we want to provide additional keywords other than the title, we can add them here. This helps users to search for relevant badges.

 Save, run `npm start`, and you can see it [locally](http://127.0.0.1:3000/).
--- a/services/base.js
+++ b/services/base.js
@@ -21,6 +21,7 @@ const {
 } = require('../lib/badge-data')
 const { staticBadgeUrl } = require('../lib/make-badge-url')
 const trace = require('./trace')
+const validateExample = require('./validate-example')

 function coalesce(...candidates) {
  return candidates.find(c => c !== undefined)
@@ -84,6 +85,32 @@ class BaseService {
   * Example URLs for this service. These should use the format
   * specified in `route`, and can be used to demonstrate how to use badges for
   * this service.
+   *
+   * The preferred way to specify an example is with `namedParams` which are
+   * substitued into the service's compiled route pattern. The rendered badge
+   * is specified with `staticExample`.
+   *
+   * For services which use a route `format`, the `pattern` can be specified as
+   * part of the example.
+   *
+   * title: Descriptive text that will be shown next to the badge. The default
+   *   is to use the service class name, which probably is not what you want.
+   * namedParams: An object containing the values of named parameters to
+   *   substitute into the compiled route pattern.
+   * query: An object containing query parameters to include in the example URLs.
+   * pattern: The route pattern to compile. Defaults to `this.route.pattern`.
+   * urlPattern: Deprecated. An alias for `pattern`.
+   * staticExample: A rendered badge of the sort returned by `handle()` or
+   *   `render()`: an object containing `message` and optional `label` and
+   *   `color`. This is usually generated by invoking `this.render()` with some
+   *   explicit props.
+   * previewUrl: Deprecated. An explicit example which is rendered as part of
+   *   the badge listing.
+   * exampleUrl: Deprecated. An explicit example which will be displayed to
+   *   the user, but not rendered.
+   * keywords: Additional keywords, other than words in the title. This helps
+   *   users locate relevant badges.
+   * documentation: An HTML string that is included in the badge popup.
   */
  static get examples() {
    return []
@@ -93,6 +120,19 @@ class BaseService {
    return `/${[this.route.base, partialUrl].filter(Boolean).join('/')}`
  }

+  static _makeFullUrlFromParams(pattern, namedParams, ext = 'svg') {
+    const fullPattern = `${this._makeFullUrl(
+      pattern
+    )}.:ext(svg|png|gif|jpg|json)`
+
+    const toPath = pathToRegexp.compile(fullPattern, {
+      strict: true,
+      sensitive: true,
+    })
+
+    return toPath({ ext, ...namedParams })
+  }
+
  static _makeStaticExampleUrl(serviceData) {
    const badgeData = this._makeBadgeData({}, serviceData)
    return staticBadgeUrl({
@@ -115,69 +155,52 @@ class BaseService {
   * schema in `lib/all-badge-examples.js`.
   */
  static prepareExamples() {
-    return this.examples.map(
-      (
-        {
-          title,
-          query,
-          exampleUrl,
-          previewUrl,
-          urlPattern,
-          staticExample,
-          documentation,
-          keywords,
-        },
-        index
-      ) => {
-        if (staticExample) {
-          if (!urlPattern) {
-            throw new Error(
-              `Static example for ${
-                this.name
-              } at index ${index} does not declare a urlPattern`
-            )
-          }
-          if (!exampleUrl) {
-            throw new Error(
-              `Static example for ${
-                this.name
-              } at index ${index} does not declare an exampleUrl`
-            )
-          }
-          if (previewUrl) {
-            throw new Error(
-              `Static example for ${
-                this.name
-              } at index ${index} also declares a dynamic previewUrl, which is not allowed`
-            )
-          }
-        } else if (!previewUrl) {
-          throw Error(
-            `Example for ${
-              this.name
-            } at index ${index} is missing required previewUrl or staticExample`
-          )
-        }
+    return this.examples.map((example, index) => {
+      const {
+        title,
+        query,
+        namedParams,
+        exampleUrl,
+        previewUrl,
+        pattern,
+        staticExample,
+        documentation,
+        keywords,
+      } = validateExample(example, index, this)

-        const stringified = queryString.stringify(query)
-        const suffix = stringified ? `?${stringified}` : ''
+      const stringified = queryString.stringify(query)
+      const suffix = stringified ? `?${stringified}` : ''

-        return {
-          title: title ? `${title}` : this.name,
-          exampleUrl: exampleUrl
-            ? `${this._dotSvg(this._makeFullUrl(exampleUrl))}${suffix}`
-            : undefined,
-          previewUrl: staticExample
-            ? this._makeStaticExampleUrl(staticExample)
-            : `${this._dotSvg(this._makeFullUrl(previewUrl))}${suffix}`,
-          urlPattern: urlPattern
-            ? `${this._dotSvg(this._makeFullUrl(urlPattern))}${suffix}`
-            : undefined,
-          documentation,
-          keywords,
-        }
+      let outExampleUrl
+      let outPreviewUrl
+      let outPattern
+      if (namedParams) {
+        outExampleUrl = this._makeFullUrlFromParams(pattern, namedParams)
+        outPreviewUrl = this._makeStaticExampleUrl(staticExample)
+        outPattern = `${this._dotSvg(this._makeFullUrl(pattern))}${suffix}`
+      } else if (staticExample) {
+        outExampleUrl = `${this._dotSvg(
+          this._makeFullUrl(exampleUrl)
+        )}${suffix}`
+        outPreviewUrl = this._makeStaticExampleUrl(staticExample)
+        outPattern = `${this._dotSvg(this._makeFullUrl(pattern))}${suffix}`
+      } else {
+        outExampleUrl = undefined
+        outPreviewUrl = `${this._dotSvg(
+          this._makeFullUrl(previewUrl)
+        )}${suffix}`
+        outPattern = undefined
      }
-    )
+
+      return {
+        title: title ? `${title}` : this.name,
+        exampleUrl: outExampleUrl,
+        previewUrl: outPreviewUrl,
+        urlPattern: outPattern,
+        documentation,
+        keywords,
+      }
+    })
  }

  static get _regexFromPath() {
--- a/services/base.spec.js
+++ b/services/base.spec.js
@@ -42,6 +42,18 @@ class DummyService extends BaseService {
        staticExample: this.render({ namedParamA: 'foo', queryParamA: 'bar' }),
        keywords: ['hello'],
      },
+      {
+        pattern: ':world',
+        exampleUrl: 'World',
+        staticExample: this.render({ namedParamA: 'foo', queryParamA: 'bar' }),
+        keywords: ['hello'],
+      },
+      {
+        pattern: ':world',
+        namedParams: { world: 'World' },
+        staticExample: this.render({ namedParamA: 'foo', queryParamA: 'bar' }),
+        keywords: ['hello'],
+      },
    ]
  }
  static get route() {
@@ -457,7 +469,13 @@ describe('BaseService', function() {

  describe('prepareExamples', function() {
    it('returns the expected result', function() {
-      const [first, second, third] = DummyService.prepareExamples()
+      const [
+        first,
+        second,
+        third,
+        fourth,
+        fifth,
+      ] = DummyService.prepareExamples()
      expect(first).to.deep.equal({
        title: 'DummyService',
        exampleUrl: undefined,
@@ -474,7 +492,7 @@ describe('BaseService', function() {
        documentation: undefined,
        keywords: undefined,
      })
-      expect(third).to.deep.equal({
+      const preparedStaticExample = {
        title: 'DummyService',
        exampleUrl: '/foo/World.svg',
        previewUrl:
@@ -482,7 +500,10 @@ describe('BaseService', function() {
        urlPattern: '/foo/:world.svg',
        documentation: undefined,
        keywords: ['hello'],
-      })
+      }
+      expect(third).to.deep.equal(preparedStaticExample)
+      expect(fourth).to.deep.equal(preparedStaticExample)
+      expect(fifth).to.deep.equal(preparedStaticExample)
    })
  })

--- a/services/gem/gem-version.service.js
+++ b/services/gem/gem-version.service.js
@@ -48,8 +48,7 @@ module.exports = class GemVersion extends BaseJsonService {
    return [
      {
        title: 'Gem',
-        exampleUrl: 'formatador',
-        urlPattern: ':package',
+        namedParams: { gem: 'formatador' },
        staticExample: this.render({ version: '2.1.0' }),
        keywords: ['ruby'],
      },
--- a/services/validate-example.js
+++ b/services/validate-example.js
@@ -0,0 +1,68 @@
+'use strict'
+
+module.exports = function validateExample(
+  {
+    title,
+    query,
+    namedParams,
+    exampleUrl,
+    previewUrl,
+    pattern,
+    urlPattern,
+    staticExample,
+    documentation,
+    keywords,
+  },
+  index,
+  ServiceClass
+) {
+  pattern = pattern || urlPattern || ServiceClass.route.pattern
+
+  if (staticExample) {
+    if (!pattern) {
+      throw new Error(
+        `Static example for ${
+          ServiceClass.name
+        } at index ${index} does not declare a pattern`
+      )
+    }
+    if (namedParams && exampleUrl) {
+      throw new Error(
+        `Static example for ${
+          ServiceClass.name
+        } at index ${index} declares both namedParams and exampleUrl`
+      )
+    } else if (!namedParams && !exampleUrl) {
+      throw new Error(
+        `Static example for ${
+          ServiceClass.name
+        } at index ${index} does not declare namedParams nor exampleUrl`
+      )
+    }
+    if (previewUrl) {
+      throw new Error(
+        `Static example for ${
+          ServiceClass.name
+        } at index ${index} also declares a dynamic previewUrl, which is not allowed`
+      )
+    }
+  } else if (!previewUrl) {
+    throw Error(
+      `Example for ${
+        ServiceClass.name
+      } at index ${index} is missing required previewUrl or staticExample`
+    )
+  }
+
+  return {
+    title,
+    query,
+    namedParams,
+    exampleUrl,
+    previewUrl,
+    pattern,
+    staticExample,
+    documentation,
+    keywords,
+  }
+}
--- a/services/validate-example.spec.js
+++ b/services/validate-example.spec.js
@@ -0,0 +1,45 @@
+'use strict'
+
+const { expect } = require('chai')
+const validateExample = require('./validate-example')
+
+describe('validateExample function', function() {
+  it('passes valid examples', function() {
+    const validExamples = [
+      { staticExample: {}, pattern: 'dt/:package', exampleUrl: 'dt/mypackage' },
+      {
+        staticExample: {},
+        pattern: 'dt/:package',
+        namedParams: { package: 'mypackage' },
+      },
+      { previewUrl: 'dt/mypackage' },
+    ]
+
+    validExamples.forEach(example => {
+      expect(() =>
+        validateExample(example, 0, { route: {}, name: 'mockService' })
+      ).not.to.throw(Error)
+    })
+  })
+
+  it('rejects invalid examples', function() {
+    const invalidExamples = [
+      {},
+      { staticExample: {} },
+      {
+        staticExample: {},
+        pattern: 'dt/:package',
+        namedParams: { package: 'mypackage' },
+        exampleUrl: 'dt/mypackage',
+      },
+      { staticExample: {}, pattern: 'dt/:package' },
+      { staticExample: {}, pattern: 'dt/:package', previewUrl: 'dt/mypackage' },
+    ]
+
+    invalidExamples.forEach(example => {
+      expect(() =>
+        validateExample(example, 0, { route: {}, name: 'mockService' })
+      ).to.throw(Error)
+    })
+  })
+})
--- a/services/wordpress/wordpress-downloads.service.js
+++ b/services/wordpress/wordpress-downloads.service.js
@@ -55,8 +55,7 @@ function DownloadsForExtensionType(extensionType) {
      return [
        {
          title: `Wordpress ${capt} Downloads`,
-          exampleUrl: exampleSlug,
-          urlPattern: ':slug',
+          namedParams: { slug: exampleSlug },
          staticExample: this.render({ response: { downloaded: 200000 } }),
          keywords: ['wordpress'],
        },
--- a/services/wordpress/wordpress-platform.service.js
+++ b/services/wordpress/wordpress-platform.service.js
@@ -55,8 +55,7 @@ class WordpressPluginRequiresVersion extends BaseWordpressPlatform {
    return [
      {
        title: 'Wordpress Plugin: Required WP Version',
-        exampleUrl: 'bbpress',
-        urlPattern: ':slug',
+        namedParams: { slug: 'bbpress' },
        staticExample: this.render({ response: { requires: '4.8' } }),
        keywords: ['wordpress'],
      },
--- a/services/wordpress/wordpress-rating.service.js
+++ b/services/wordpress/wordpress-rating.service.js
@@ -93,8 +93,8 @@ function StarsForExtensionType(extensionType) {
      return [
        {
          title: `Wordpress ${capt} Rating`,
-          exampleUrl: `stars/${exampleSlug}`,
-          urlPattern: 'stars/:slug',
+          pattern: 'stars/:slug',
+          namedParams: { slug: exampleSlug },
          staticExample: this.render({
            response: {
              rating: 80,
@@ -102,7 +102,7 @@ function StarsForExtensionType(extensionType) {
            },
          }),
          keywords: ['wordpress'],
-          documentation: 'There is an alias <code>/r/:slug.svg</code> aswell',
+          documentation: 'There is an alias <code>/r/:slug.svg</code> as well.',
        },
      ]
    }
--- a/services/wordpress/wordpress-version.service.js
+++ b/services/wordpress/wordpress-version.service.js
@@ -47,8 +47,7 @@ function VersionForExtensionType(extensionType) {
      return [
        {
          title: `Wordpress ${capt} Version`,
-          exampleUrl: exampleSlug,
-          urlPattern: ':slug',
+          namedParams: { slug: exampleSlug },
          staticExample: this.render({ response: { version: 2.5 } }),
          keywords: ['wordpress'],
        },