Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add template utils #534

Merged
merged 8 commits into from
Sep 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions packages/cli-lib/lib/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,49 @@ const PROJECT_TEMPLATE_TYPES = {
},
};

const TEMPLATE_TYPES = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily a blocker, but it would be nice if this was something that the BE could return to us so that we could keep a single source of truth. I don't imagine that it will change frequently, but it'll be yet another area where we have to manually keep data in sync.

unmapped: 0,
email_base_template: 1,
email: 2,
landing_page_base_template: 3,
landing_page: 4,
blog_base: 5,
blog: 6,
blog_listing: 42,
site_page: 8,
blog_listing_context: 9,
blog_post_context: 10,
error_page: 11,
subscription_preferences: 12,
unsubscribe_confirmation: 13,
unsubscribe_simple: 14,
optin_email: 15,
optin_followup_email: 16,
optin_confirmation_page: 17,
global_group: 18,
password_prompt_page: 19,
resubscribe_email: 20,
unsubscribe_confirmation_email: 21,
resubscribe_confirmation_email: 22,
custom_module: 23,
css: 24,
js: 25,
search_results: 27,
membership_login: 29,
membership_register: 30,
membership_reset: 31,
membership_reset_request: 32,
drag_drop_email: 34,
knowledge_article: 35,
membership_email: 36,
section: 37,
global_content_partial: 38,
simple_landing_page_template: 39,
proposal: 40,
blog_post: 41,
quote: 43,
};

const PROJECT_BUILD_STATUS = {
BUILDING: 'BUILDING',
ENQUEUED: 'ENQUEUED',
Expand Down Expand Up @@ -162,4 +205,5 @@ module.exports = {
PROJECT_BUILD_STATUS_TEXT,
PROJECT_TEMPLATE_TYPES,
SCOPE_GROUPS,
TEMPLATE_TYPES,
};
21 changes: 21 additions & 0 deletions packages/cli-lib/modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ const { getCwd, getExt, splitHubSpotPath, splitLocalPath } = require('./path');
const { walk } = require('./lib/walk');
const { MODULE_EXTENSION } = require('./lib/constants');

// Matches files named module.html
const MODULE_HTML_EXTENSION_REGEX = new RegExp(/\.module\/module\.html$/);
// Matches files named module.css
const MODULE_CSS_EXTENSION_REGEX = new RegExp(/\.module\/module\.css$/);

const isBool = x => !!x === x;

/**
Expand Down Expand Up @@ -150,9 +155,25 @@ async function validateSrcAndDestPaths(src, dest) {
return results;
}

/**
* Checks if the given path points to an .html file within a .module folder
* @param {string} filePath
* @returns {boolean}
*/
const isModuleHTMLFile = filePath => MODULE_HTML_EXTENSION_REGEX.test(filePath);

/**
* Checks if the given path points to an .css file within a .module folder
* @param {string} filePath
* @returns {boolean}
*/
const isModuleCSSFile = filePath => MODULE_CSS_EXTENSION_REGEX.test(filePath);

module.exports = {
isModuleFolder,
isModuleFolderChild,
validateSrcAndDestPaths,
ValidationIds,
isModuleHTMLFile,
isModuleCSSFile,
};
24 changes: 8 additions & 16 deletions packages/cli-lib/templates.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
const fs = require('fs');
const { logger } = require('./logger');

// Matches the .html file extension, excluding module.html
const TEMPLATE_EXTENSION_REGEX = new RegExp(/(?<!module)\.html$/);

// Matches the comment brackets that wrap annotations
const ANNOTATIONS_REGEX = /<!--([\s\S]*?)-->/;
// Matches an annotation value, ending at space, newline, or end of string
Expand All @@ -17,24 +15,18 @@ const ANNOTATION_KEYS = {
description: 'description',
};

const getFileAnnotations = filePath => {
try {
const data = fs.readFileSync(filePath, 'utf8');
const match = data.match(ANNOTATIONS_REGEX);
const annotation = match && match[1] ? match[1] : '';
return annotation;
} catch (err) {
logger.debug(err);
return '';
}
};

const getAnnotationValue = (annotations, key) => {
const valueRegex = new RegExp(`${key}${ANNOTATION_VALUE_REGEX}`);
const match = annotations.match(valueRegex);
return match ? match[1].trim() : null;
};

const buildAnnotationValueGetter = source => {
const match = source.match(ANNOTATIONS_REGEX);
const annotation = match && match[1] ? match[1] : '';
return annotationKey => getAnnotationValue(annotation, annotationKey);
};

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected here https://github.com/HubSpot/hubspot-cms-vscode/pull/117/files#diff-dfcd8afcb372a75a1eb1bdc0af57d36a5e0a727dbca99350779abb21333a298fR65

and also tested the marketplace-validate command out and that works as expected

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! This pattern looks cleaner 🙌

/*
* Returns true if:
* .html extension (ignoring module.html)
Expand All @@ -44,6 +36,6 @@ const isCodedFile = filePath => TEMPLATE_EXTENSION_REGEX.test(filePath);
module.exports = {
ANNOTATION_KEYS,
getAnnotationValue,
getFileAnnotations,
buildAnnotationValueGetter,
isCodedFile,
};
12 changes: 7 additions & 5 deletions packages/cli/lib/validators/__tests__/TemplateValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ jest.mock('@hubspot/cli-lib/templates');
const TEMPLATE_LIMIT = 50;

const mockGetAnnotationValue = (templateType, rest) => {
templates.getAnnotationValue.mockImplementation((x, key) => {
if (key === 'templateType') {
return templateType;
}
return rest;
templates.buildAnnotationValueGetter.mockImplementation(() => {
return key => {
if (key === 'templateType') {
return templateType;
}
return rest;
};
});
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
const fs = require('fs');
const {
ANNOTATION_KEYS,
getAnnotationValue,
getFileAnnotations,
buildAnnotationValueGetter,
isCodedFile,
} = require('@hubspot/cli-lib/templates');
const BaseValidator = require('../BaseValidator');
const { VALIDATOR_KEYS } = require('../../constants');
const { logger } = require('@hubspot/cli-lib/logger');

const TEMPLATE_LIMIT = 50;
const TEMPLATE_IGNORE_LIST = ['section'];
Expand Down Expand Up @@ -100,29 +101,28 @@ class TemplateValidator extends BaseValidator {

files.forEach(file => {
if (isCodedFile(file)) {
const annotations = getFileAnnotations(file);
let data;
try {
data = fs.readFileSync(file, 'utf8');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moves the file reading step from cli-lib to cli which I don't think has any negative impact

} catch (e) {
logger.error(`Error reading file ${file}`);
}
const getAnnotationValue = buildAnnotationValueGetter(data);

const isAvailableForNewContent = getAnnotationValue(
annotations,
ANNOTATION_KEYS.isAvailableForNewContent
);

if (isAvailableForNewContent !== 'false') {
const templateType = getAnnotationValue(
annotations,
ANNOTATION_KEYS.templateType
);
const templateType = getAnnotationValue(ANNOTATION_KEYS.templateType);

if (TEMPLATE_IGNORE_LIST.includes(templateType)) {
return;
}

if (templateType) {
const label = getAnnotationValue(
annotations,
ANNOTATION_KEYS.label
);
const label = getAnnotationValue(ANNOTATION_KEYS.label);
const screenshotPath = getAnnotationValue(
annotations,
ANNOTATION_KEYS.screenshotPath
);

Expand Down