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.

Sign up for GitHub

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
Merged

Refactor loadAndValidateOptions p3: Add config validation #1307

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
dc5bf05
Add validation to config middleware
kemmerle Dec 17, 2024
24c78e2
Address feedback p1
kemmerle Dec 17, 2024
1b06e9c
Refactor no validate list
kemmerle Dec 17, 2024
3b63df8
Add more descriptive variables
kemmerle Dec 17, 2024
2d21901
Remove validateConfig from loadAndValidateOptions
kemmerle Dec 18, 2024
47028a4
Bump LDL version
kemmerle Dec 18, 2024
c3fd8cc
Address feedback p1
kemmerle Dec 19, 2024
584706b
Merge next
kemmerle Dec 19, 2024
77bebbd
Fix acceptance tests
kemmerle Dec 19, 2024
ab3c51f
Streamline loading config
kemmerle Dec 19, 2024
df81f4f
Streamline loading config p2
kemmerle Dec 19, 2024
a85ce94
Fix tests p2
kemmerle Dec 19, 2024
24a38b5
Simplify loading config
kemmerle Dec 20, 2024
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
11 changes: 5 additions & 6 deletions acceptance-tests/tests/workflows/accountManagementFlow.spec.ts
View file
Open in desktop
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
23 changes: 22 additions & 1 deletion bin/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const {
getAndLoadConfigIfNeeded,
configFileExists,
getConfigPath,
validateConfig,
} = require('@hubspot/local-dev-lib/config');
const { logError } = require('../lib/errorHandlers/index');
const {
Expand Down Expand Up @@ -148,7 +149,24 @@ const setRequestHeaders = () => {
addUserAgentHeader('HubSpot CLI', pkg.version);
};

const NO_CONFIG_VALIDATION = {
init: { skip: true },
auth: { skip: true },
};

kemmerle marked this conversation as resolved.
Show resolved Hide resolved
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) {
if (!validateConfig()) {
kemmerle marked this conversation as resolved.
Show resolved Hide resolved
process.exit(EXIT_CODES.ERROR);
}
}
};

// Load the new config and check for the conflicting config flag
if (configFileExists(true)) {
loadConfig('', options);
Expand All @@ -161,19 +179,22 @@ const loadConfigMiddleware = async options => {
);
process.exit(EXIT_CODES.ERROR);
}
maybeValidateConfig();
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)) {
const { config: configPath } = options;
await loadConfig(configPath, options);
loadConfig(configPath, options);
maybeValidateConfig();
kemmerle marked this conversation as resolved.
Show resolved Hide resolved
return;
}

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

Expand Down
3 changes: 1 addition & 2 deletions lib/validation.ts
View file
Open in desktop
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
View file
Open in desktop
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.1",
"@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
40 changes: 8 additions & 32 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1463,10 +1463,10 @@
semver "^6.3.0"
unixify "^1.0.0"

"@hubspot/local-dev-lib@3.0.1":
version "3.0.1"
resolved "https://registry.yarnpkg.com/@hubspot/local-dev-lib/-/local-dev-lib-3.0.1.tgz#1dd22f439d6e262353f14915a354115bbc1f5f76"
integrity sha512-h1jOmZJNdHZFbrOA5Gn815YCsix8eY81A4dkrUuDZI4MzVaJH3o4RoRalMl+Hr2e35nDrbrbcIR1RMQADPSmwg==
"@hubspot/local-dev-lib@3.1.0":
version "3.1.0"
resolved "https://registry.yarnpkg.com/@hubspot/local-dev-lib/-/local-dev-lib-3.1.0.tgz#76b5d524aa694aad2bfc6e199ee1dac314f15774"
integrity sha512-iop3PgZ0ZWejCH6PmSeYnnGwV8vGIuA4F+w7SxukdX3QfhivlczClATWPaZanv1CYJHdflqoItq1kdF8ASJJmA==
dependencies:
address "^2.0.1"
axios "^1.3.5"
Expand Down Expand Up @@ -10427,16 +10427,7 @@ string-length@^4.0.1:
char-regex "^1.0.2"
strip-ansi "^6.0.0"

"string-width-cjs@npm:string-width@^4.2.0":
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
dependencies:
emoji-regex "^8.0.0"
is-fullwidth-code-point "^3.0.0"
strip-ansi "^6.0.1"

string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3:
"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3:
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
Expand Down Expand Up @@ -10505,7 +10496,7 @@ stringify-object@^3.2.1, stringify-object@^3.3.0:
is-obj "^1.0.1"
is-regexp "^1.0.0"

"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1:
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
Expand All @@ -10519,13 +10510,6 @@ strip-ansi@^5.2.0:
dependencies:
ansi-regex "^4.1.0"

strip-ansi@^6.0.0, strip-ansi@^6.0.1:
version "6.0.1"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
dependencies:
ansi-regex "^5.0.1"

strip-ansi@^7.0.1:
version "7.1.0"
resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.1.0.tgz#d5b6568ca689d8561370b0707685d22434faff45"
Expand Down Expand Up @@ -11512,7 +11496,8 @@ wordwrap@^1.0.0:
resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-1.0.0.tgz#27584810891456a4171c8d0226441ade90cbcaeb"
integrity sha512-gvVzJFlPycKc5dZN4yPkP8w7Dc37BtP1yczEneOb4uq34pXZcvrtRTmWV8W+Ume+XCxKgbjM+nevkyFPMybd4Q==

"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
name wrap-ansi-cjs
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
Expand All @@ -11530,15 +11515,6 @@ wrap-ansi@^6.2.0:
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^7.0.0:
version "7.0.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
dependencies:
ansi-styles "^4.0.0"
string-width "^4.1.0"
strip-ansi "^6.0.0"

wrap-ansi@^8.1.0:
version "8.1.0"
resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214"
Expand Down
Loading