From aa7a06c177a3a63d832afdd85ac547950c2b5d6f Mon Sep 17 00:00:00 2001 From: Branden Rodgers Date: Fri, 10 Jan 2025 15:21:08 -0500 Subject: [PATCH 1/2] Remove CLI getAccountId util --- commands/account/remove.ts | 12 ++++------ commands/account/use.ts | 10 +++----- commands/auth.ts | 4 ++-- .../project/__tests__/installDeps.test.ts | 5 +--- commands/project/installDeps.ts | 6 ++--- commands/sandbox/delete.ts | 18 ++++++-------- lib/__tests__/commonOpts.test.ts | 2 +- lib/__tests__/validation.test.ts | 6 +++-- lib/commonOpts.ts | 24 +++++-------------- lib/validation.ts | 24 ++++++++++--------- 10 files changed, 43 insertions(+), 68 deletions(-) diff --git a/commands/account/remove.ts b/commands/account/remove.ts index b0a11533f..a82ef6f03 100644 --- a/commands/account/remove.ts +++ b/commands/account/remove.ts @@ -6,7 +6,7 @@ const { getConfigPath, deleteAccount, getConfigDefaultAccount, - getAccountId: getAccountIdFromConfig, + getAccountId, updateDefaultAccount, } = require('@hubspot/local-dev-lib/config'); @@ -23,7 +23,7 @@ exports.handler = async options => { const { account } = options; let accountToRemove = account; - if (accountToRemove && !getAccountIdFromConfig(accountToRemove)) { + if (accountToRemove && !getAccountId(accountToRemove)) { logger.error( i18n(`${i18nKey}.errors.accountNotFound`, { specifiedAccount: accountToRemove, @@ -32,17 +32,13 @@ exports.handler = async options => { ); } - if (!accountToRemove || !getAccountIdFromConfig(accountToRemove)) { + if (!accountToRemove || !getAccountId(accountToRemove)) { accountToRemove = await selectAccountFromConfig( i18n(`${i18nKey}.prompts.selectAccountToRemove`) ); } - trackCommandUsage( - 'accounts-remove', - null, - getAccountIdFromConfig(accountToRemove) - ); + trackCommandUsage('accounts-remove', null, getAccountId(accountToRemove)); const currentDefaultAccount = getConfigDefaultAccount(); diff --git a/commands/account/use.ts b/commands/account/use.ts index df2ec0594..e4cb208ab 100644 --- a/commands/account/use.ts +++ b/commands/account/use.ts @@ -3,7 +3,7 @@ const { logger } = require('@hubspot/local-dev-lib/logger'); const { getConfigPath, updateDefaultAccount, - getAccountId: getAccountIdFromConfig, + getAccountId, } = require('@hubspot/local-dev-lib/config'); const { trackCommandUsage } = require('../../lib/usageTracking'); @@ -20,7 +20,7 @@ exports.handler = async options => { if (!newDefaultAccount) { newDefaultAccount = await selectAccountFromConfig(); - } else if (!getAccountIdFromConfig(newDefaultAccount)) { + } else if (!getAccountId(newDefaultAccount)) { logger.error( i18n(`${i18nKey}.errors.accountNotFound`, { specifiedAccount: newDefaultAccount, @@ -30,11 +30,7 @@ exports.handler = async options => { newDefaultAccount = await selectAccountFromConfig(); } - trackCommandUsage( - 'accounts-use', - null, - getAccountIdFromConfig(newDefaultAccount) - ); + trackCommandUsage('accounts-use', null, getAccountId(newDefaultAccount)); updateDefaultAccount(newDefaultAccount); diff --git a/commands/auth.ts b/commands/auth.ts index b0d26e1d5..89aafc2d0 100644 --- a/commands/auth.ts +++ b/commands/auth.ts @@ -22,6 +22,7 @@ const { getConfigPath, loadConfig, getConfigDefaultAccount, + getAccountId, } = require('@hubspot/local-dev-lib/config'); const { commaSeparatedValues, @@ -39,7 +40,6 @@ const { const { addConfigOptions, setLogLevel, - getAccountId, addTestingOptions, addGlobalOptions, } = require('../lib/commonOpts'); @@ -197,7 +197,7 @@ exports.handler = async options => { 'accountsListCommand', ]); - const accountId = getAccountId({ account: accountName }); + const accountId = getAccountId(accountName); await trackAuthAction('auth', authType, TRACKING_STATUS.COMPLETE, accountId); process.exit(EXIT_CODES.SUCCESS); diff --git a/commands/project/__tests__/installDeps.test.ts b/commands/project/__tests__/installDeps.test.ts index d4c7bc901..3d0135e9a 100644 --- a/commands/project/__tests__/installDeps.test.ts +++ b/commands/project/__tests__/installDeps.test.ts @@ -5,7 +5,6 @@ const { logger } = require('@hubspot/local-dev-lib/logger'); const { getProjectConfig } = require('../../../lib/projects'); const { EXIT_CODES } = require('../../../lib/enums/exitCodes'); const { trackCommandUsage } = require('../../../lib/usageTracking'); -const { getAccountId } = require('../../../lib/commonOpts'); const { installPackages, getProjectPackageJsonLocations, @@ -56,10 +55,8 @@ describe('commands/project/installDeps', () => { it('should track the command usage', async () => { const accountId = 999999; - getAccountId.mockReturnValue(accountId); - await installDepsCommand.handler({}); + await installDepsCommand.handler({ derivedAccountId: accountId }); - expect(getAccountId).toHaveBeenCalledTimes(1); expect(trackCommandUsage).toHaveBeenCalledTimes(1); expect(trackCommandUsage).toHaveBeenCalledWith( 'project-install-deps', diff --git a/commands/project/installDeps.ts b/commands/project/installDeps.ts index c85b2102c..987d80fce 100644 --- a/commands/project/installDeps.ts +++ b/commands/project/installDeps.ts @@ -10,7 +10,6 @@ const { promptUser } = require('../../lib/prompts/promptUtils'); const path = require('path'); const { i18n } = require('../../lib/lang'); const { trackCommandUsage } = require('../../lib/usageTracking'); -const { getAccountId } = require('../../lib/commonOpts'); const { uiBetaTag } = require('../../lib/ui'); const i18nKey = `commands.project.subcommands.installDeps`; @@ -19,10 +18,9 @@ exports.command = 'install-deps [packages..]'; exports.describe = uiBetaTag(i18n(`${i18nKey}.help.describe`), false); exports.handler = async options => { - const { packages } = options || {}; + const { derivedAccountId, packages } = options || {}; try { - const accountId = getAccountId(options); - trackCommandUsage('project-install-deps', null, accountId); + trackCommandUsage('project-install-deps', null, derivedAccountId); const projectConfig = await getProjectConfig(); if (!projectConfig || !projectConfig.projectDir) { diff --git a/commands/sandbox/delete.ts b/commands/sandbox/delete.ts index 290acd49d..5be8826c2 100644 --- a/commands/sandbox/delete.ts +++ b/commands/sandbox/delete.ts @@ -3,7 +3,6 @@ const { addAccountOptions, addConfigOptions, addUseEnvironmentOptions, - getAccountId, addTestingOptions, } = require('../../lib/commonOpts'); const { logger } = require('@hubspot/local-dev-lib/logger'); @@ -17,7 +16,7 @@ const { getEnv, removeSandboxAccountFromConfig, updateDefaultAccount, - getConfigDefaultAccount, + getAccountId, getConfigAccounts, } = require('@hubspot/local-dev-lib/config'); const { @@ -61,11 +60,10 @@ exports.handler = async options => { } } - const sandboxAccountId = getAccountId({ - account: providedAccountId || accountPrompt.account, - }); - const isDefaultAccount = - sandboxAccountId === getAccountId(getConfigDefaultAccount()); + const sandboxAccountId = getAccountId( + providedAccountId || accountPrompt.account + ); + const isDefaultAccount = sandboxAccountId === getAccountId(); const baseUrl = getHubSpotWebsiteOrigin( getValidEnv(getEnv(sandboxAccountId)) @@ -79,9 +77,7 @@ exports.handler = async options => { parentAccountId = portal.parentAccountId; } else if (!force) { const parentAccountPrompt = await deleteSandboxPrompt(true); - parentAccountId = getAccountId({ - account: parentAccountPrompt.account, - }); + parentAccountId = getAccountId(parentAccountPrompt.account); } else { logger.error(i18n(`${i18nKey}.failure.noParentAccount`)); process.exit(EXIT_CODES.ERROR); @@ -94,7 +90,7 @@ exports.handler = async options => { getEnv(sandboxAccountId) === 'qa' ? '--qa' : '' } --account=${parentAccountId}`; - if (parentAccountId && !getAccountId({ account: parentAccountId })) { + if (parentAccountId && !getAccountId(parentAccountId)) { logger.log(''); logger.error( i18n(`${i18nKey}.failure.noParentPortalAvailable`, { diff --git a/lib/__tests__/commonOpts.test.ts b/lib/__tests__/commonOpts.test.ts index 1354c0ec5..9271a3666 100644 --- a/lib/__tests__/commonOpts.test.ts +++ b/lib/__tests__/commonOpts.test.ts @@ -5,8 +5,8 @@ const { } = require('@hubspot/local-dev-lib/constants/files'); const { getAndLoadConfigIfNeeded, - getAccountId, getAccountConfig, + getAccountId, loadConfigFromEnvironment, } = require('@hubspot/local-dev-lib/config'); const { getCmsPublishMode } = require('../commonOpts'); diff --git a/lib/__tests__/validation.test.ts b/lib/__tests__/validation.test.ts index ba44922a6..25e302e52 100644 --- a/lib/__tests__/validation.test.ts +++ b/lib/__tests__/validation.test.ts @@ -1,10 +1,12 @@ // @ts-nocheck -const { getAccountConfig } = require('@hubspot/local-dev-lib/config'); +const { + getAccountId, + getAccountConfig, +} = require('@hubspot/local-dev-lib/config'); const { getOauthManager } = require('@hubspot/local-dev-lib/oauth'); const { accessTokenForPersonalAccessKey, } = require('@hubspot/local-dev-lib/personalAccessKey'); -const { getAccountId } = require('../commonOpts'); const { validateAccount } = require('../validation'); jest.mock('@hubspot/local-dev-lib/config'); diff --git a/lib/commonOpts.ts b/lib/commonOpts.ts index c2d37d152..9a5a1196e 100644 --- a/lib/commonOpts.ts +++ b/lib/commonOpts.ts @@ -8,7 +8,7 @@ import { } from '@hubspot/local-dev-lib/constants/files'; import { CmsPublishMode } from '@hubspot/local-dev-lib/types/Files'; import { - getAccountId as getAccountIdFromConfig, + getAccountId, getAccountConfig, getAndLoadConfigIfNeeded, } from '@hubspot/local-dev-lib/config'; @@ -102,23 +102,11 @@ export function getCommandName(argv: Arguments<{ _?: string[] }>): string { return (argv && argv._ && argv._[0]) || ''; } -/** - * Obtains accountId using supplied --account flag or from environment variables - */ -export function getAccountId( - options: Arguments<{ account?: number | string }> -): number | null { - const { account } = options || {}; - - if (options?.useEnv && process.env.HUBSPOT_ACCOUNT_ID) { - return parseInt(process.env.HUBSPOT_ACCOUNT_ID, 10); - } - - return getAccountIdFromConfig(account); -} - export function getCmsPublishMode( - options: Arguments<{ cmsPublishMode?: CmsPublishMode }> + options: Arguments<{ + cmsPublishMode?: CmsPublishMode; + derivedAccountId?: number; + }> ): CmsPublishMode { // 1. --cmsPublishMode const { cmsPublishMode } = options; @@ -126,7 +114,7 @@ export function getCmsPublishMode( return cmsPublishMode.toLowerCase() as CmsPublishMode; } // 2. config[account].defaultCmsPublishMode - const accountId = getAccountId(options); + const accountId = getAccountId(options.derivedAccountId); if (accountId) { const accountConfig = getAccountConfig(accountId); if (accountConfig && accountConfig.defaultCmsPublishMode) { diff --git a/lib/validation.ts b/lib/validation.ts index 859ebde44..6269f8c75 100644 --- a/lib/validation.ts +++ b/lib/validation.ts @@ -13,6 +13,7 @@ import { commaSeparatedValues } from '@hubspot/local-dev-lib/text'; import { getConfigPath, getAccountConfig, + getAccountId, loadConfigFromEnvironment, } from '@hubspot/local-dev-lib/config'; import { getOauthManager } from '@hubspot/local-dev-lib/oauth'; @@ -22,23 +23,24 @@ import { getCwd, getExt, } from '@hubspot/local-dev-lib/path'; -import { getAccountId, getCmsPublishMode } from './commonOpts'; +import { getCmsPublishMode } from './commonOpts'; import { logError } from './errorHandlers/index'; export async function validateAccount( - options: Arguments<{ account?: string; accountId?: string }> + options: Arguments<{ + account?: string; + accountId?: string; + derivedAccountId?: number; + providedAccountId?: string; + }> ): Promise { - const accountId = getAccountId(options); - const { accountId: accountIdOption, account: accountOption } = options; + const { derivedAccountId, providedAccountId } = options; + const accountId = getAccountId(derivedAccountId); if (!accountId) { - if (accountOption) { + if (providedAccountId) { logger.error( - `The account "${accountOption}" could not be found in the config` - ); - } else if (accountIdOption) { - logger.error( - `The account "${accountIdOption}" could not be found in the config` + `The account "${providedAccountId}" could not be found in the config` ); } else { logger.error( @@ -48,7 +50,7 @@ export async function validateAccount( return false; } - if (accountOption && loadConfigFromEnvironment()) { + if (providedAccountId && loadConfigFromEnvironment()) { throw new Error( 'Cannot specify an account when environment variables are supplied. Please unset the environment variables or do not use the "--account" flag.' ); From 5dc138bb57fcee3175e2403a17cbdb604e0a322b Mon Sep 17 00:00:00 2001 From: Branden Rodgers Date: Fri, 10 Jan 2025 15:29:29 -0500 Subject: [PATCH 2/2] this can be simplified since derivedAccountID is already the id --- lib/__tests__/commonOpts.test.ts | 4 ---- lib/commonOpts.ts | 6 ++---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/__tests__/commonOpts.test.ts b/lib/__tests__/commonOpts.test.ts index 9271a3666..f2292d1f7 100644 --- a/lib/__tests__/commonOpts.test.ts +++ b/lib/__tests__/commonOpts.test.ts @@ -6,7 +6,6 @@ const { const { getAndLoadConfigIfNeeded, getAccountConfig, - getAccountId, loadConfigFromEnvironment, } = require('@hubspot/local-dev-lib/config'); const { getCmsPublishMode } = require('../commonOpts'); @@ -40,7 +39,6 @@ describe('lib/commonOpts', () => { afterEach(() => { getAndLoadConfigIfNeeded.mockReset(); - getAccountId.mockReset(); getAccountConfig.mockReset(); loadConfigFromEnvironment.mockReset(); }); @@ -68,7 +66,6 @@ describe('lib/commonOpts', () => { getAndLoadConfigIfNeeded.mockReturnValue( configWithDefaultCmsPublishMode ); - getAccountId.mockReturnValue(accounts.DEV); getAccountConfig.mockReturnValue(devAccountConfig); loadConfigFromEnvironment.mockReturnValue(undefined); expect(getCmsPublishMode({ account: accounts.DEV })).toBe( @@ -81,7 +78,6 @@ describe('lib/commonOpts', () => { getAndLoadConfigIfNeeded.mockReturnValue( configWithDefaultCmsPublishMode ); - getAccountId.mockReturnValue(accounts.PROD); getAccountConfig.mockReturnValue(prodAccountConfig); loadConfigFromEnvironment.mockReturnValue(undefined); expect(getCmsPublishMode({ account: accounts.PROD })).toBe( diff --git a/lib/commonOpts.ts b/lib/commonOpts.ts index 9a5a1196e..5a6794924 100644 --- a/lib/commonOpts.ts +++ b/lib/commonOpts.ts @@ -8,7 +8,6 @@ import { } from '@hubspot/local-dev-lib/constants/files'; import { CmsPublishMode } from '@hubspot/local-dev-lib/types/Files'; import { - getAccountId, getAccountConfig, getAndLoadConfigIfNeeded, } from '@hubspot/local-dev-lib/config'; @@ -114,9 +113,8 @@ export function getCmsPublishMode( return cmsPublishMode.toLowerCase() as CmsPublishMode; } // 2. config[account].defaultCmsPublishMode - const accountId = getAccountId(options.derivedAccountId); - if (accountId) { - const accountConfig = getAccountConfig(accountId); + if (options.derivedAccountId) { + const accountConfig = getAccountConfig(options.derivedAccountId); if (accountConfig && accountConfig.defaultCmsPublishMode) { return accountConfig.defaultCmsPublishMode; }