diff --git a/packages/cli/commands/marketplaceValidate/validateTheme.js b/packages/cli/commands/marketplaceValidate/validateTheme.js index a173940b2..f648edb50 100644 --- a/packages/cli/commands/marketplaceValidate/validateTheme.js +++ b/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); } }); diff --git a/packages/cli/lib/validators/__tests__/ThemeValdiator.js b/packages/cli/lib/validators/__tests__/ThemeConfigValidator.js similarity index 74% rename from packages/cli/lib/validators/__tests__/ThemeValdiator.js rename to packages/cli/lib/validators/__tests__/ThemeConfigValidator.js index 3d59d6665..b96d9840c 100644 --- a/packages/cli/lib/validators/__tests__/ThemeValdiator.js +++ b/packages/cli/lib/validators/__tests__/ThemeConfigValidator.js @@ -1,20 +1,20 @@ 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); }); @@ -22,7 +22,7 @@ describe('validators/marketplaceValidators/theme/ThemeValidator', () => { 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); }); @@ -30,7 +30,7 @@ describe('validators/marketplaceValidators/theme/ThemeValidator', () => { 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); }); }); diff --git a/packages/cli/lib/validators/applyValidators.js b/packages/cli/lib/validators/applyValidators.js index 0e30a171b..fbf1ace11 100644 --- a/packages/cli/lib/validators/applyValidators.js +++ b/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 }; diff --git a/packages/cli/lib/validators/constants.js b/packages/cli/lib/validators/constants.js index 4f2c6a63e..9a2b5eb00 100644 --- a/packages/cli/lib/validators/constants.js +++ b/packages/cli/lib/validators/constants.js @@ -3,5 +3,11 @@ const FATAL = 'FATAL'; const SUCCESS = 'SUCCESS'; const VALIDATION_RESULT = { WARNING, FATAL, SUCCESS }; +const VALIDATOR_KEYS = { + dependency: 'dependency', + module: 'module', + template: 'template', + themeConfig: 'themeConfig', +}; -module.exports = { VALIDATION_RESULT }; +module.exports = { VALIDATOR_KEYS, VALIDATION_RESULT }; diff --git a/packages/cli/lib/validators/index.js b/packages/cli/lib/validators/index.js index f593fc2fa..e996c8af4 100644 --- a/packages/cli/lib/validators/index.js +++ b/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, diff --git a/packages/cli/lib/validators/logValidatorResults.js b/packages/cli/lib/validators/logValidatorResults.js index 8fe0447ea..48bc9fc3b 100644 --- a/packages/cli/lib/validators/logValidatorResults.js +++ b/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 }; diff --git a/packages/cli/lib/validators/marketplaceValidators/BaseValidator.js b/packages/cli/lib/validators/marketplaceValidators/BaseValidator.js index 735384e7b..a406c0883 100644 --- a/packages/cli/lib/validators/marketplaceValidators/BaseValidator.js +++ b/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, }; } } diff --git a/packages/cli/lib/validators/marketplaceValidators/theme/DependencyValidator.js b/packages/cli/lib/validators/marketplaceValidators/theme/DependencyValidator.js index 66c600fba..c1825c553 100644 --- a/packages/cli/lib/validators/marketplaceValidators/theme/DependencyValidator.js +++ b/packages/cli/lib/validators/marketplaceValidators/theme/DependencyValidator.js @@ -10,6 +10,7 @@ const { fetchDependencies } = require('@hubspot/cli-lib/api/marketplace'); const { getExt, isRelativePath } = require('@hubspot/cli-lib/path'); const BaseValidator = require('../BaseValidator'); +const { VALIDATOR_KEYS } = require('../../constants'); const MISSING_ASSET_CATEGORY_TYPES = [ 'MISSING_RESOURCE', @@ -24,17 +25,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 +123,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, }) ); } @@ -142,5 +144,5 @@ class DependencyValidator extends BaseValidator { module.exports = new DependencyValidator({ name: 'Dependency', - key: 'dependency', + key: VALIDATOR_KEYS.dependency, }); diff --git a/packages/cli/lib/validators/marketplaceValidators/theme/ModuleValidator.js b/packages/cli/lib/validators/marketplaceValidators/theme/ModuleValidator.js index 313a63b33..fb203ca3a 100644 --- a/packages/cli/lib/validators/marketplaceValidators/theme/ModuleValidator.js +++ b/packages/cli/lib/validators/marketplaceValidators/theme/ModuleValidator.js @@ -1,8 +1,9 @@ const fs = require('fs'); const path = require('path'); -const BaseValidator = require('../BaseValidator'); const { isModuleFolderChild } = require('@hubspot/cli-lib/modules'); +const BaseValidator = require('../BaseValidator'); +const { VALIDATOR_KEYS } = require('../../constants'); const MODULE_LIMIT = 50; @@ -14,25 +15,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`, }, }; } @@ -110,5 +113,5 @@ class ModuleValidator extends BaseValidator { module.exports = new ModuleValidator({ name: 'Module', - key: 'module', + key: VALIDATOR_KEYS.module, }); diff --git a/packages/cli/lib/validators/marketplaceValidators/theme/TemplateValidator.js b/packages/cli/lib/validators/marketplaceValidators/theme/TemplateValidator.js index c4bcecec4..939661191 100644 --- a/packages/cli/lib/validators/marketplaceValidators/theme/TemplateValidator.js +++ b/packages/cli/lib/validators/marketplaceValidators/theme/TemplateValidator.js @@ -5,6 +5,7 @@ const { isCodedFile, } = require('@hubspot/cli-lib/templates'); const BaseValidator = require('../BaseValidator'); +const { VALIDATOR_KEYS } = require('../../constants'); const TEMPLATE_LIMIT = 50; const TEMPLATE_COUNT_IGNORE_LIST = ['global_partial', 'none']; @@ -57,29 +58,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`, }, }; } @@ -170,5 +174,5 @@ class TemplateValidator extends BaseValidator { module.exports = new TemplateValidator({ name: 'Template', - key: 'template', + key: VALIDATOR_KEYS.template, }); diff --git a/packages/cli/lib/validators/marketplaceValidators/theme/ThemeValidator.js b/packages/cli/lib/validators/marketplaceValidators/theme/ThemeConfigValidator.js similarity index 76% rename from packages/cli/lib/validators/marketplaceValidators/theme/ThemeValidator.js rename to packages/cli/lib/validators/marketplaceValidators/theme/ThemeConfigValidator.js index 821c7afae..1e943a3f0 100644 --- a/packages/cli/lib/validators/marketplaceValidators/theme/ThemeValidator.js +++ b/packages/cli/lib/validators/marketplaceValidators/theme/ThemeConfigValidator.js @@ -1,8 +1,9 @@ const fs = require('fs'); const path = require('path'); -const BaseValidator = require('../BaseValidator'); const { isRelativePath } = require('@hubspot/cli-lib/path'); +const BaseValidator = require('../BaseValidator'); +const { VALIDATOR_KEYS } = require('../../constants'); class ThemeValidator extends BaseValidator { constructor(options) { @@ -11,30 +12,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 +101,6 @@ class ThemeValidator extends BaseValidator { } module.exports = new ThemeValidator({ - name: 'Theme', - key: 'theme', + name: 'Theme config', + key: VALIDATOR_KEYS.themeConfig, });