Merge pull request #2297 from alphagov/plugin-validator-meta

Adding plugin metadata to validator

govuk-prototype-kit.config.json

@@ -1,4 +1,12 @@
 {
+  "meta": {
+    "description": "Quickly make interactive, accessible and realistic prototypes of GOV.UK services.",
+    "urls": {
+      "documentation": "https://prototype-kit.service.gov.uk/?linkFromKit={{kitVersion}}%3A{{version}}",
+      "versionHistory": "https://github.com/alphagov/govuk-prototype-kit/releases",
+      "releaseNotes": "https://github.com/alphagov/govuk-prototype-kit/releases/tag/v{{version}}"
+    }
+  },
   "assets": [
     "/lib/assets/images",
     "/lib/assets/javascripts/optional",

lib/plugins/plugin-validator.js

@@ -4,21 +4,19 @@ const ansiColors = require('ansi-colors')
 
 const errors = []
 
-function getKnownKeys () {
-  const knownKeys = [
-    'assets',
-    'importNunjucksMacrosInto',
-    'nunjucksPaths',
-    'nunjucksMacros',
-    'nunjucksFilters',
-    'sass',
-    'scripts',
-    'stylesheets',
-    'templates',
-    'pluginDependencies'
-  ]
-  return knownKeys
-}
+const knownKeys = [
+  'assets',
+  'importNunjucksMacrosInto',
+  'nunjucksPaths',
+  'nunjucksMacros',
+  'nunjucksFilters',
+  'sass',
+  'scripts',
+  'stylesheets',
+  'templates',
+  'pluginDependencies',
+  'meta'
+]
 
 function checkPathExists (executionPath, pathToValidate, key) {
   const absolutePathToValidate = path.join(executionPath, pathToValidate)
@@ -46,9 +44,12 @@ function checkNunjucksMacroExists (executionPath, nunjucksFileName, nunjucksPath
   if (!nunjucksPathExists) errors.push(`The nunjucks file '${nunjucksFileName}' does not exist`)
 }
 
+function reportInvalidKeys (invalidKeys) {
+  errors.push(`The following invalid keys exist in your config: ${invalidKeys}`)
+}
+
 function validateConfigKeys (pluginConfig) {
   console.log('Config file exists, validating contents.')
-  const knownKeys = getKnownKeys()
   const invalidKeys = []
 
   const validKeysPluginConfig = Object.fromEntries(Object.entries(pluginConfig).filter(([key]) => {
@@ -61,12 +62,81 @@ function validateConfigKeys (pluginConfig) {
 
   // Add any invalid config keys to the list of errors
   if (invalidKeys.length > 0) {
-    errors.push(`The following invalid keys exist in your config: ${invalidKeys}`)
+    reportInvalidKeys(invalidKeys)
   }
 
   return validKeysPluginConfig
 }
 
+function reportUnknownKeys (objectToEvaluate, allowedKeys, keyPath) {
+  const invalidMetaUrlKeys = Object.keys(objectToEvaluate).filter(key => !allowedKeys.includes(key)).map(key => `${keyPath || ''}${key}`)
+  if (invalidMetaUrlKeys.length > 0) {
+    reportInvalidKeys(invalidMetaUrlKeys)
+  }
+}
+
+function isValidUrl (url) {
+  if (!url.startsWith('https://') && !url.startsWith('http://')) {
+    return false
+  }
+  if (!url.split('://')[1].split('/')[0].includes('.')) {
+    return false
+  }
+  return true
+}
+
+function validateMetaUrls (metaUrls) {
+  if (typeof metaUrls === 'undefined') {
+    return
+  }
+
+  if (typeof metaUrls !== 'object') {
+    errors.push('The meta.urls must be an object if entered')
+    return
+  }
+
+  const keyPath = 'meta.urls.'
+  reportUnknownKeys(metaUrls, [
+    'documentation',
+    'releaseNotes',
+    'versionHistory'
+  ], keyPath)
+
+  const allowedVariables = ['version', 'kitVersion'].map(variable => `{{${variable}}}`)
+
+  Object.keys(metaUrls).forEach(key => {
+    const url = metaUrls[key]
+    if (!isValidUrl(url)) {
+      errors.push(`meta.urls.${key} doesn't appear to be a public URL`)
+    }
+
+    const unknownVariables = (url.match(/{{(\w+)}}/g) || []).filter(variable => !allowedVariables.includes(variable))
+
+    unknownVariables.forEach(variable => errors.push(`The URL ${keyPath}${key} contains an unknown variable ${variable}`))
+  })
+}
+
+function validateMeta (meta) {
+  const metaKeys = ['urls', 'description']
+
+  if (typeof meta !== 'object') {
+    errors.push('The meta must be an object if entered')
+    return
+  }
+
+  if (typeof meta.description !== 'string' && typeof meta.description !== 'undefined') {
+    errors.push('The meta.description must be a string if entered')
+    return
+  }
+
+  const invalidMetaUrlKeys = Object.keys(meta).filter(key => !metaKeys.includes(key)).map(key => `meta.${key}`)
+  if (invalidMetaUrlKeys.length > 0) {
+    reportInvalidKeys(invalidMetaUrlKeys)
+  }
+
+  validateMetaUrls(meta.urls)
+}
+
 function validatePluginDependency (key, configEntry) {
   if (typeof configEntry === 'string') {
     return
@@ -82,15 +152,15 @@ function validatePluginDependency (key, configEntry) {
   }
   // The minVersion is optional but must be a string if entered
   if (Object.keys(configEntry).includes('minVersion') && typeof minVersion !== 'string') {
-    errors.push(`In section ${key}, the minVersion '${minVersion}' should be a valid version`)
+    errors.push(`In section ${key}, the minVersion '${minVersion}' should be a valid version if entered`)
   }
   // The maxVersion is optional but must be a string if entered
   if (Object.keys(configEntry).includes('maxVersion') && typeof maxVersion !== 'string' && typeof maxVersion !== 'undefined') {
     errors.push(`In section ${key}, the maxVersion '${maxVersion}' should be a valid version if entered`)
   }
 }
 
-function validateConfigPaths (pluginConfig, executionPath) {
+function validateConfigurationValues (pluginConfig, executionPath) {
   console.log('Validating whether config paths meet criteria.')
   const keysToValidate = Object.keys(pluginConfig)
 
@@ -103,7 +173,9 @@ function validateConfigPaths (pluginConfig, executionPath) {
 
     criteriaConfig.forEach((configEntry) => {
       try {
-        if (key === 'pluginDependencies') {
+        if (key === 'meta') {
+          validateMeta(configEntry)
+        } else if (key === 'pluginDependencies') {
           validatePluginDependency(key, configEntry)
         } else if ((key === 'templates' && configEntry.path[0] === '/') ||
           (key === 'scripts' && configEntry.path !== undefined && configEntry.path[0] === '/')) {
@@ -141,18 +213,21 @@ async function validatePlugin (executionPath) {
 
       // Check if the json has contents
       let isConfigEmpty = false
-      if (JSON.stringify(pluginConfig) === '{}') {
+      const configClone = Object.assign({}, pluginConfig)
+      delete configClone.meta
+      if (JSON.stringify(configClone) === '{}') {
         isConfigEmpty = true
       }
 
       // Continue with the validation if there are no syntax errors in the config
       if (pluginConfig) {
         if (isConfigEmpty) {
-          errors.push('There are no contents in your govuk-prototype.config file!')
+          const caveat = pluginConfig.meta ? ' other than the metadata' : ''
+          errors.push(`There are no contents in your govuk-prototype.config file${caveat}!`)
         } else {
           // Perform the rest of the checks if the config file has contents
           const validKeysPluginConfig = validateConfigKeys(pluginConfig)
-          validateConfigPaths(validKeysPluginConfig, executionPath)
+          validateConfigurationValues(validKeysPluginConfig, executionPath)
         }
       }
     } else {
@@ -185,5 +260,6 @@ module.exports = {
   clearErrors,
   getErrors,
   validatePlugin,
+  validateMeta,
   validatePluginDependency
 }

lib/plugins/plugin-validator.spec.js

@@ -1,4 +1,4 @@
-const { validatePluginDependency, clearErrors, getErrors } = require('./plugin-validator')
+const { validatePluginDependency, clearErrors, getErrors, validateMeta } = require('./plugin-validator')
 
 describe('plugin-validator', () => {
   beforeEach(() => {
@@ -56,9 +56,101 @@ describe('plugin-validator', () => {
         maxVersion: 100
       })
       expect(getErrors()).toEqual([
-        'In section pluginDependencies, the minVersion \'null\' should be a valid version',
+        'In section pluginDependencies, the minVersion \'null\' should be a valid version if entered',
         'In section pluginDependencies, the maxVersion \'100\' should be a valid version if entered'
       ])
     })
   })
+  describe('meta', () => {
+    it('should allow all the valid options', () => {
+      validateMeta({
+        description: 'Hello world',
+        urls: {
+          documentation: 'https://example.com/',
+          releaseNotes: 'http://example.com/',
+          versionHistory: 'https://example.com/'
+        }
+      })
+      expect(getErrors()).toEqual([])
+    })
+    it('should check that urls is an object', () => {
+      validateMeta({
+        urls: 'abc'
+      })
+      expect(getErrors()).toEqual(['The meta.urls must be an object if entered'])
+    })
+    it('should check that urls contain a valid protocol', () => {
+      validateMeta({
+        urls: {
+          documentation: 'ftp://example.com'
+        }
+      })
+      expect(getErrors()).toEqual(['meta.urls.documentation doesn\'t appear to be a public URL'])
+    })
+    it('should fail if URL doesn\'t contain a tld', () => {
+      validateMeta({
+        urls: {
+          releaseNotes: 'https://example'
+        }
+      })
+      expect(getErrors()).toEqual(['meta.urls.releaseNotes doesn\'t appear to be a public URL'])
+    })
+    it('should fail if URL doesn\'t contain a tld but does contain a path with a dot', () => {
+      validateMeta({
+        urls: {
+          versionHistory: 'https://example/index.html'
+        }
+      })
+      expect(getErrors()).toEqual(['meta.urls.versionHistory doesn\'t appear to be a public URL'])
+    })
+    it('should fail if URL doesn\'t contain a tld but does contain a path with a dot', () => {
+      validateMeta({
+        urls: {
+          versionHistory: 'https://example/index.html'
+        }
+      })
+      expect(getErrors()).toEqual(['meta.urls.versionHistory doesn\'t appear to be a public URL'])
+    })
+    it('should fail if unknown URL provided', () => {
+      validateMeta({
+        urls: {
+          versionHistory2: 'https://example.com/index.html'
+        }
+      })
+      expect(getErrors()).toEqual(['The following invalid keys exist in your config: meta.urls.versionHistory2'])
+    })
+    it('should fail if description is not a string', () => {
+      validateMeta({
+        description: {}
+      })
+      expect(getErrors()).toEqual(['The meta.description must be a string if entered'])
+    })
+    it('should allow known variables in URLs', () => {
+      validateMeta({
+        urls: {
+          documentation: 'https://example.com/{{version}}?kitVersion={{kitVersion}}'
+        }
+      })
+      expect(getErrors()).toEqual([])
+    })
+    it('should fail if unknown variables are present', () => {
+      validateMeta({
+        urls: {
+          documentation: 'https://example.com/{{versions}}?kitVersion={{kitVersions}}'
+        }
+      })
+      expect(getErrors()).toEqual([
+        'The URL meta.urls.documentation contains an unknown variable {{versions}}',
+        'The URL meta.urls.documentation contains an unknown variable {{kitVersions}}'
+      ])
+    })
+    it('should allow spaces in variables', () => {
+      validateMeta({
+        urls: {
+          documentation: 'https://example.com/{{            version }}?kitVersion={{ kitVersions }}'
+        }
+      })
+      expect(getErrors()).toEqual([])
+    })
+  })
 })

package.json

@@ -46,6 +46,7 @@
     "cypress:e2e:plugins": "cypress run --spec \"cypress/e2e/plugins/*/*\"",
     "cypress:e2e:errors": "cypress run --spec \"cypress/e2e/errors/*/*\"",
     "test:heroku": "cross-env KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test:heroku 3000 cypress:e2e:smoke",
+    "test:acceptance": "npm run test:acceptance:dev && npm run test:acceptance:prod && npm run test:acceptance:smoke && npm run test:acceptance:styles && npm run test:acceptance:plugins && npm run test:acceptance:errors",
     "test:acceptance:dev": "cross-env KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test 3000 cypress:e2e:dev",
     "test:acceptance:prod": "cross-env PASSWORD=password KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test:prod 3000 cypress:e2e:prod",
     "test:acceptance:smoke": "cross-env KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test 3000 cypress:e2e:smoke",
@@ -55,6 +56,7 @@
     "test:acceptance:open": "cross-env KIT_TEST_DIR=tmp/test-prototype start-server-and-test start:test 3000 cypress:open",
     "test:unit": "jest --detectOpenHandles lib bin migrator",
     "test:integration": "cross-env CREATE_KIT_TIMEOUT=240000 jest --detectOpenHandles --testTimeout=60000 __tests__",
+    "test:full": "npm test && npm run test:acceptance",
     "test": "npm run test:unit && npm run test:integration && npm run lint"
   },
   "dependencies": {