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

Refactor loadAndValidateOptions p3: Add config validation #1307

Merged
merged 13 commits into from
Dec 20, 2024
27 changes: 18 additions & 9 deletions acceptance-tests/tests/commands/create.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('hs create', () => {
});

it('creates a module', async () => {
await testState.cli.execute(
await testState.cli.executeWithTestConfig(
['create', 'module', FOLDERS.module.name],
['label', ENTER, ENTER, ENTER, 'y', ENTER, ENTER]
);
Expand All @@ -80,7 +80,7 @@ describe('hs create', () => {
});

it('creates a template', async () => {
await testState.cli.execute(
await testState.cli.executeWithTestConfig(
['create', 'template', FOLDERS.template.name],
[ENTER]
);
Expand All @@ -90,35 +90,44 @@ describe('hs create', () => {
});

it('website-theme', async () => {
await testState.cli.execute(['create', FOLDERS.websiteTheme.name]);
await testState.cli.executeWithTestConfig([
'create',
FOLDERS.websiteTheme.name,
]);
expect(
testState.existsInTestOutputDirectory(FOLDERS.websiteTheme.folder)
).toBe(true);
});

it('react-app', async () => {
await testState.cli.execute(['create', FOLDERS.reactApp.name]);
await testState.cli.executeWithTestConfig([
'create',
FOLDERS.reactApp.name,
]);
expect(testState.existsInTestOutputDirectory(FOLDERS.reactApp.folder)).toBe(
true
);
});

it('vue-app', async () => {
await testState.cli.execute(['create', FOLDERS.vueApp.name]);
await testState.cli.executeWithTestConfig(['create', FOLDERS.vueApp.name]);
expect(testState.existsInTestOutputDirectory(FOLDERS.vueApp.folder)).toBe(
true
);
});

it('webpack-serverless', async () => {
await testState.cli.execute(['create', FOLDERS.webpackServerless.name]);
await testState.cli.executeWithTestConfig([
'create',
FOLDERS.webpackServerless.name,
]);
expect(
testState.existsInTestOutputDirectory(FOLDERS.webpackServerless.folder)
).toBe(true);
});

it('api-sample', async () => {
await testState.cli.execute(
await testState.cli.executeWithTestConfig(
['create', FOLDERS.apiSample.name, FOLDERS.apiSample.name],
[ENTER, ENTER]
);
Expand All @@ -129,14 +138,14 @@ describe('hs create', () => {
});

it('app', async () => {
await testState.cli.execute(['create', FOLDERS.app.name]);
await testState.cli.executeWithTestConfig(['create', FOLDERS.app.name]);
expect(testState.existsInTestOutputDirectory(FOLDERS.app.folder)).toBe(
true
);
});

it('function', async () => {
await testState.cli.execute(
await testState.cli.executeWithTestConfig(
['create', 'function'],
[
FOLDERS.function.name,
Expand Down
11 changes: 5 additions & 6 deletions acceptance-tests/tests/workflows/accountManagementFlow.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,11 @@ describe('Account Management Flow', () => {

describe('hs accounts list', () => {
it('should not list the removed authenticated account', async () => {
const output = await testState.cli.executeWithTestConfig([
'accounts',
'list',
]);

expect(output).not.toContain(accountName);
await expect(() =>
testState.cli.executeWithTestConfig(['accounts', 'list'])
).rejects.toThrow(
/There are no accounts defined in the configuration file/
);
});
});

Expand Down
2 changes: 1 addition & 1 deletion acceptance-tests/tests/workflows/cmsTemplateFlow.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('CMS Template Flow', () => {

describe('hs create', () => {
it('should create a CMS template', async () => {
await testState.cli.execute(
await testState.cli.executeWithTestConfig(
['create', 'template', TEMPLATE_NAME],
[ENTER]
);
Expand Down
47 changes: 24 additions & 23 deletions bin/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@
const yargs = require('yargs');
const updateNotifier = require('update-notifier');
const chalk = require('chalk');
const fs = require('fs');

const { logger } = require('@hubspot/local-dev-lib/logger');
const { addUserAgentHeader } = require('@hubspot/local-dev-lib/http');
const {
loadConfig,
getAndLoadConfigIfNeeded,
configFileExists,
getConfigPath,
validateConfig,
} = require('@hubspot/local-dev-lib/config');
const { logError } = require('../lib/errorHandlers/index');
const {
Expand Down Expand Up @@ -148,33 +147,35 @@ const setRequestHeaders = () => {
addUserAgentHeader('HubSpot CLI', pkg.version);
};

const loadConfigMiddleware = async options => {
// Load the new config and check for the conflicting config flag
if (configFileExists(true)) {
loadConfig('', options);
const NO_CONFIG_VALIDATION = {
init: { skip: true },
auth: { skip: true },
};

if (options.config) {
logger.error(
i18n(`${i18nKey}.loadConfigMiddleware.configFileExists`, {
configPath: getConfigPath(),
})
);
const loadConfigMiddleware = async options => {
const maybeValidateConfig = () => {
const shouldValidate = options._.every(
kemmerle marked this conversation as resolved.
Show resolved Hide resolved
brandenrodgers marked this conversation as resolved.
Show resolved Hide resolved
command =>
!(NO_CONFIG_VALIDATION[command] && NO_CONFIG_VALIDATION[command].skip)
);
if (shouldValidate && !validateConfig()) {
process.exit(EXIT_CODES.ERROR);
}
return;
}

// We need to load the config when options.config exists,
// so that getAccountIdFromConfig() in injectAccountIdMiddleware reads from the right config
if (options.config && fs.existsSync(options.config)) {
};

if (configFileExists(true) && options.config) {
logger.error(
i18n(`${i18nKey}.loadConfigMiddleware.configFileExists`, {
configPath: getConfigPath(),
})
);
process.exit(EXIT_CODES.ERROR);
} else if (!options._.includes('init')) {
const { config: configPath } = options;
await loadConfig(configPath, options);
return;
loadConfig(configPath, options);
}

// Load deprecated config without a config flag and with no warnings
getAndLoadConfigIfNeeded(options);
return;
maybeValidateConfig();
};

const checkAndWarnGitInclusionMiddleware = () => {
Expand Down
2 changes: 1 addition & 1 deletion commands/__tests__/create.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('commands/create', () => {
it('should support the correct options', () => {
createCommand.builder(yargs);

expect(yargs.option).toHaveBeenCalledTimes(2);
expect(yargs.option).toHaveBeenCalledTimes(3);
expect(yargs.option).toHaveBeenCalledWith(
'internal',
expect.objectContaining({ type: 'boolean', hidden: true })
Expand Down
7 changes: 6 additions & 1 deletion commands/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@
const fs = require('fs-extra');
const { logError } = require('../lib/errorHandlers/index');
const { logger } = require('@hubspot/local-dev-lib/logger');
const { setLogLevel, addGlobalOptions } = require('../lib/commonOpts');
const {
setLogLevel,
addGlobalOptions,
addConfigOptions,
} = require('../lib/commonOpts');
const { resolveLocalPath } = require('../lib/filesystem');
const { trackCommandUsage } = require('../lib/usageTracking');
const assets = require('./create/index');
Expand Down Expand Up @@ -117,6 +121,7 @@ exports.builder = yargs => {
hidden: true,
});

addConfigOptions(yargs);
addGlobalOptions(yargs);

return yargs;
Expand Down
3 changes: 1 addition & 2 deletions lib/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const {
const { commaSeparatedValues } = require('@hubspot/local-dev-lib/text');
const {
getConfigPath,
validateConfig,
getAccountConfig,
loadConfigFromEnvironment,
} = require('@hubspot/local-dev-lib/config');
Expand All @@ -34,7 +33,7 @@ async function loadAndValidateOptions(options, shouldValidateAccount = true) {
validAccount = await validateAccount(options);
}

if (!(validateConfig() && validAccount)) {
if (!validAccount) {
kemmerle marked this conversation as resolved.
Show resolved Hide resolved
process.exit(EXIT_CODES.ERROR);
}
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"license": "Apache-2.0",
"repository": "https://github.com/HubSpot/hubspot-cli",
"dependencies": {
"@hubspot/local-dev-lib": "3.0.2",
"@hubspot/local-dev-lib": "3.1.0",
"@hubspot/serverless-dev-runtime": "7.0.0",
"@hubspot/theme-preview-dev-server": "0.0.10",
"@hubspot/ui-extensions-dev-server": "0.8.33",
Expand Down
Loading
Loading