Updating context passed for marketplace validation

packages/cli/commands/marketplaceValidate/validateTheme.js

@@ -67,10 +67,14 @@ exports.handler = async options => {
     absoluteSrcPath,
     themeFiles,
     accountId
-  ).then(results => {
-    logValidatorResults(results, { logAsJson: options.json });
+  ).then(groupedResults => {
+    logValidatorResults(groupedResults, { logAsJson: options.json });
 
-    if (results.some(result => result.result === VALIDATION_RESULT.FATAL)) {
+    if (
+      groupedResults
+        .flat()
+        .some(result => result.result === VALIDATION_RESULT.FATAL)
+    ) {
       process.exit(1);
     }
   });

packages/cli/lib/validators/__tests__/ThemeValdiator.js → packages/cli/lib/validators/__tests__/ThemeConfigValidator.js

@@ -1,36 +1,36 @@
 const fs = require('fs');
 const path = require('path');
 
-const ThemeValidator = require('../../validators/marketplaceValidators/theme/ThemeValidator');
+const ThemeConfigValidator = require('../../validators/marketplaceValidators/theme/ThemeConfigValidator');
 const { VALIDATION_RESULT } = require('../../validators/constants');
 const { THEME_PATH } = require('./validatorTestUtils');
 
 jest.mock('fs');
 jest.mock('path');
 
-describe('validators/marketplaceValidators/theme/ThemeValidator', () => {
+describe('validators/marketplaceValidators/theme/ThemeConfigValidator', () => {
   beforeEach(() => {
-    ThemeValidator.setThemePath(THEME_PATH);
+    ThemeConfigValidator.setThemePath(THEME_PATH);
   });
 
   it('returns error if no theme.json file exists', async () => {
-    const validationErrors = ThemeValidator.validate(['someFile.html']);
+    const validationErrors = ThemeConfigValidator.validate(['someFile.html']);
     expect(validationErrors.length).toBe(1);
     expect(validationErrors[0].result).toBe(VALIDATION_RESULT.FATAL);
   });
 
   it('returns error if theme.json file has invalid json', async () => {
     fs.readFileSync.mockReturnValue('{} bad json }');
 
-    const validationErrors = ThemeValidator.validate(['theme.json']);
+    const validationErrors = ThemeConfigValidator.validate(['theme.json']);
     expect(validationErrors.length).toBe(1);
     expect(validationErrors[0].result).toBe(VALIDATION_RESULT.FATAL);
   });
 
   it('returns error if theme.json file is missing a label field', async () => {
     fs.readFileSync.mockReturnValue('{ "screenshot_path": "./relative/path" }');
 
-    const validationErrors = ThemeValidator.validate(['theme.json']);
+    const validationErrors = ThemeConfigValidator.validate(['theme.json']);
     expect(validationErrors.length).toBe(1);
     expect(validationErrors[0].result).toBe(VALIDATION_RESULT.FATAL);
   });
@@ -40,7 +40,7 @@ describe('validators/marketplaceValidators/theme/ThemeValidator', () => {
       '{ "label": "yay", "screenshot_path": "/absolute/path" }'
     );
 
-    const validationErrors = ThemeValidator.validate(['theme.json']);
+    const validationErrors = ThemeConfigValidator.validate(['theme.json']);
     expect(validationErrors.length).toBe(1);
     expect(validationErrors[0].result).toBe(VALIDATION_RESULT.FATAL);
   });
@@ -52,7 +52,7 @@ describe('validators/marketplaceValidators/theme/ThemeValidator', () => {
     path.relative.mockReturnValue('theme.json');
     fs.existsSync.mockReturnValue(false);
 
-    const validationErrors = ThemeValidator.validate(['theme.json']);
+    const validationErrors = ThemeConfigValidator.validate(['theme.json']);
     expect(validationErrors.length).toBe(1);
     expect(validationErrors[0].result).toBe(VALIDATION_RESULT.FATAL);
   });
@@ -64,7 +64,7 @@ describe('validators/marketplaceValidators/theme/ThemeValidator', () => {
     path.relative.mockReturnValue('theme.json');
     fs.existsSync.mockReturnValue(true);
 
-    const validationErrors = ThemeValidator.validate(['theme.json']);
+    const validationErrors = ThemeConfigValidator.validate(['theme.json']);
     expect(validationErrors.length).toBe(0);
   });
 });

packages/cli/lib/validators/applyValidators.js

@@ -11,7 +11,7 @@ async function applyValidators(validators, absoluteThemePath, ...args) {
       }
       return validationResult;
     })
-  ).then(errorsGroupedByValidatorType => errorsGroupedByValidatorType.flat());
+  );
 }
 
 module.exports = { applyValidators };

packages/cli/lib/validators/index.js

@@ -1,11 +1,11 @@
-const ThemeValidator = require('./marketplaceValidators/theme/ThemeValidator');
+const ThemeConfigValidator = require('./marketplaceValidators/theme/ThemeConfigValidator');
 const TemplateValidator = require('./marketplaceValidators/theme/TemplateValidator');
 const ModuleValidator = require('./marketplaceValidators/theme/ModuleValidator');
 const DependencyValidator = require('./marketplaceValidators/theme/DependencyValidator');
 
 const MARKETPLACE_VALIDATORS = {
   theme: [
-    ThemeValidator,
+    ThemeConfigValidator,
     TemplateValidator,
     ModuleValidator,
     DependencyValidator,

packages/cli/lib/validators/logValidatorResults.js

@@ -1,21 +1,24 @@
+const chalk = require('chalk');
 const { logger } = require('@hubspot/cli-lib/logger');
 const { VALIDATION_RESULT } = require('./constants');
 
-function logResultsAsJson(results) {
+function logResultsAsJson(groupedResults) {
   let success = true;
-  const resultObj = results.reduce((acc, result) => {
-    if (!acc[result.validatorKey]) {
-      if (success && result.result !== VALIDATION_RESULT.SUCCESS) {
-        success = false;
+  const resultObj = groupedResults.reduce((acc, resultList) => {
+    resultList.forEach(result => {
+      if (!acc[result.validatorKey]) {
+        if (success && result.result !== VALIDATION_RESULT.SUCCESS) {
+          success = false;
+        }
+        acc[result.validatorKey] = [];
       }
-      acc[result.validatorKey] = [];
-    }
-    const {
-      validatorName: __omit1, // eslint-disable-line no-unused-vars
-      validatorKey: __omit2, // eslint-disable-line no-unused-vars
-      ...resultWithOmittedValues
-    } = result;
-    acc[result.validatorKey].push(resultWithOmittedValues);
+      const {
+        validatorName: __omit1, // eslint-disable-line no-unused-vars
+        validatorKey: __omit2, // eslint-disable-line no-unused-vars
+        ...resultWithOmittedValues
+      } = result;
+      acc[result.validatorKey].push(resultWithOmittedValues);
+    });
     return acc;
   }, {});
   const result = {
@@ -26,27 +29,31 @@ function logResultsAsJson(results) {
   process.stdout.write(JSON.stringify(result) + '\n');
 }
 
-function logValidatorResults(results, { logAsJson = false } = {}) {
+function logValidatorResults(groupedResults, { logAsJson = false } = {}) {
   if (logAsJson) {
-    return logResultsAsJson(results);
+    return logResultsAsJson(groupedResults);
   }
-  results.forEach(({ error, validatorName, result }) => {
-    const message = `${validatorName}: ${error}`;
-    switch (result) {
-      case VALIDATION_RESULT.WARNING:
-        logger.warn(message);
-        break;
-      case VALIDATION_RESULT.FATAL:
-        logger.error(message);
-        break;
-      case VALIDATION_RESULT.SUCCESS:
-        logger.success(validatorName);
-        break;
-      default:
-        logger.log(message);
-    }
+  groupedResults.forEach(results => {
+    logger.log(chalk.bold(`${results[0].validatorName} validation results:`));
+    logger.group();
+    results.forEach(({ error, result }) => {
+      switch (result) {
+        case VALIDATION_RESULT.WARNING:
+          logger.warn(error);
+          break;
+        case VALIDATION_RESULT.FATAL:
+          logger.error(error);
+          break;
+        case VALIDATION_RESULT.SUCCESS:
+          logger.success('No errors');
+          break;
+        default:
+          logger.log(error);
+      }
+    });
+    logger.log('\n');
+    logger.groupEnd();
   });
-  logger.log('\n');
 }
 
 module.exports = { logValidatorResults };

packages/cli/lib/validators/marketplaceValidators/BaseValidator.js

@@ -30,19 +30,19 @@ class BaseValidator {
     };
   }
 
-  getError(errorObj, file, extraCopyPlaceholders = {}) {
+  getError(errorObj, file, extraContext = {}) {
     const relativeFilePath = file ? this.getRelativePath(file) : null;
-    const copyPlaceholders = {
-      file: relativeFilePath,
-      ...extraCopyPlaceholders,
+    const context = {
+      filePath: relativeFilePath,
+      ...extraContext,
     };
     return {
       validatorKey: this.key,
       validatorName: this.name,
-      error: errorObj.getCopy(copyPlaceholders),
+      error: errorObj.getCopy(context),
       result: errorObj.severity || VALIDATION_RESULT.FATAL,
       key: `${this.key}.${errorObj.key}`,
-      file: relativeFilePath,
+      context,
     };
   }
 }

packages/cli/lib/validators/marketplaceValidators/theme/DependencyValidator.js

@@ -24,17 +24,18 @@ class DependencyValidator extends BaseValidator {
     this.errors = {
       FAILED_TO_FETCH_DEPS: {
         key: 'failedDepFetch',
-        getCopy: ({ file }) => `Failed to fetch dependencies for ${file}`,
+        getCopy: ({ filePath }) =>
+          `Internal Error. Failed to fetch dependencies for ${filePath}. Please try again`,
       },
       EXTERNAL_DEPENDENCY: {
         key: 'externalDependency',
-        getCopy: ({ file, path }) =>
-          `${file} contains a path that points to an external dependency (${path})`,
+        getCopy: ({ filePath, referencedFilePath }) =>
+          `External dependency. ${filePath} references a file (${referencedFilePath}) that is outside of the theme`,
       },
       ABSOLUTE_DEPENDENCY_PATH: {
         key: 'absoluteDependencyPath',
-        getCopy: ({ file, path }) =>
-          `${file} contains an absolute path (${path})`,
+        getCopy: ({ filePath, referencedFilePath }) =>
+          `Relative path required. ${filePath} references a file (${referencedFilePath}) using an absolute path`,
       },
     };
   }
@@ -121,13 +122,13 @@ class DependencyValidator extends BaseValidator {
             if (!isRelativePath(dependency)) {
               validationErrors.push(
                 this.getError(this.errors.ABSOLUTE_DEPENDENCY_PATH, file, {
-                  path: dependency,
+                  referencedFilePath: dependency,
                 })
               );
             } else if (this.isExternalDep(file, dependency)) {
               validationErrors.push(
                 this.getError(this.errors.EXTERNAL_DEPENDENCY, file, {
-                  path: dependency,
+                  referencedFilePath: dependency,
                 })
               );
             }

packages/cli/lib/validators/marketplaceValidators/theme/ModuleValidator.js

@@ -14,25 +14,27 @@ class ModuleValidator extends BaseValidator {
       LIMIT_EXCEEDED: {
         key: 'limitExceeded',
         getCopy: ({ limit, total }) =>
-          `Cannot exceed ${limit} modules in your theme (found ${total})`,
+          `Module limit exceeded. Themes can only have ${limit} modules, but this theme has ${total}`,
       },
       MISSING_META_JSON: {
         key: 'missingMetaJSON',
-        getCopy: ({ file }) => `Missing a meta.json file for ${file}`,
+        getCopy: ({ filePath }) =>
+          `Module ${filePath} is missing the meta.json file`,
       },
       INVALID_META_JSON: {
         key: 'invalidMetaJSON',
-        getCopy: ({ file }) => `Invalid json in meta.json file for ${file}`,
+        getCopy: ({ filePath }) =>
+          `Module ${filePath} has invalid json in the meta.json file`,
       },
       MISSING_LABEL: {
         key: 'missingLabel',
-        getCopy: ({ file }) =>
-          `The meta.json file is missing a "label" field for ${file}`,
+        getCopy: ({ filePath }) =>
+          `Missing required field for ${filePath}. The meta.json file is missing the "label" field`,
       },
       MISSING_ICON: {
         key: 'missingIcon',
-        getCopy: ({ file }) =>
-          `The meta.json file is missing an "icon" field for ${file}`,
+        getCopy: ({ filePath }) =>
+          `Missing required field for ${filePath}. The meta.json file is missing the "icon" field`,
       },
     };
   }

packages/cli/lib/validators/marketplaceValidators/theme/TemplateValidator.js

@@ -57,29 +57,32 @@ class TemplateValidator extends BaseValidator {
       LIMIT_EXCEEDED: {
         key: 'limitExceeded',
         getCopy: ({ limit, total }) =>
-          `Cannot exceed ${limit} templates in your theme (found ${total})`,
+          `Template limit exceeded. Themes can only have ${limit} templates, but this theme has ${total}`,
       },
       MISSING_TEMPLATE_TYPE: {
         key: 'missingTemplateType',
-        getCopy: ({ file }) => `Missing template type for ${file}`,
+        getCopy: ({ filePath }) =>
+          `Missing required field for ${filePath}. The template is missing the "templateType" field`,
       },
       UNKNOWN_TEMPLATE_TYPE: {
         key: 'unknownTemplateType',
-        getCopy: ({ file, templateType }) =>
-          `Unknown template type ${templateType} for ${file}`,
+        getCopy: ({ filePath, templateType }) =>
+          `Template ${filePath} has an unknown template type  of ${templateType}`,
       },
       RESTRICTED_TEMPLATE_TYPE: {
         key: 'restrictedTemplateType',
-        getCopy: ({ file, templateType }) =>
-          `Cannot have template type ${templateType} for ${file}`,
+        getCopy: ({ filePath, templateType }) =>
+          `Template ${filePath} has a restricted template type of ${templateType}`,
       },
       MISSING_LABEL: {
         key: 'missingLabel',
-        getCopy: ({ file }) => `Missing a "label" annotation in ${file}`,
+        getCopy: ({ filePath }) =>
+          `Missing required field for ${filePath}. The template is missing the "label" field`,
       },
       MISSING_SCREENSHOT_PATH: {
         key: 'missingScreenshotPath',
-        getCopy: ({ file }) => `The screenshotPath is missing in ${file}`,
+        getCopy: ({ filePath }) =>
+          `Missing required field for ${filePath}. The template is missing the "screenshotPath" field`,
       },
     };
   }

packages/cli/lib/validators/marketplaceValidators/theme/ThemeValidator.js → packages/cli/lib/validators/marketplaceValidators/theme/ThemeConfigValidator.js

@@ -11,30 +11,32 @@ class ThemeValidator extends BaseValidator {
     this.errors = {
       MISSING_THEME_JSON: {
         key: 'missingThemeJSON',
-        getCopy: () => 'Missing a theme.json file',
+        getCopy: () =>
+          'Missing the theme.json file. This file is required in all themes',
       },
       INVALID_THEME_JSON: {
         key: 'invalidThemeJSON',
-        getCopy: () => 'Invalid json in theme.json file',
+        getCopy: ({ filePath }) => `Invalid json in the ${filePath} file`,
       },
       MISSING_LABEL: {
         key: 'missingLabel',
-        getCopy: () => 'The theme.json file is missing a "label" field',
+        getCopy: ({ filePath }) =>
+          `Missing required field in ${filePath}. The "label" field is required`,
       },
       MISSING_SCREENSHOT_PATH: {
         key: 'missingScreenshotPath',
-        getCopy: () =>
-          'The theme.json file is missing a "screenshot_path" field',
+        getCopy: ({ filePath }) =>
+          `Missing required field in ${filePath}. The "screenshot_path" field is required`,
       },
       ABSOLUTE_SCREENSHOT_PATH: {
         key: 'absoluteScreenshotPath',
-        getCopy: () =>
-          'The path for "screenshot_path" in theme.json must be relative',
+        getCopy: ({ fieldPath }) =>
+          `Relative path required. The path for "screenshot_path" in ${fieldPath} must be relative`,
       },
       MISSING_SCREENSHOT: {
         key: 'missingScreenshot',
-        getCopy: () =>
-          'The path for "screenshot_path" in theme.json is not resolving',
+        getCopy: ({ fieldPath }) =>
+          `File not found. No file exists for the provided "screenshot_path" in ${fieldPath}`,
       },
     };
   }
@@ -98,6 +100,6 @@ class ThemeValidator extends BaseValidator {
 }
 
 module.exports = new ThemeValidator({
-  name: 'Theme',
-  key: 'theme',
+  name: 'Theme config',
+  key: 'themeConfig',
 });