From f1c5289dcd3ba30308b95027bd47e3f23f9861c0 Mon Sep 17 00:00:00 2001 From: Joe Yeager Date: Tue, 21 Jan 2025 14:44:14 -0800 Subject: [PATCH 1/2] feat: Update generic error message when repo doe not contain config.json file (#1347) --- lang/en.lyaml | 1 + lib/prompts/createProjectPrompt.ts | 16 +++++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lang/en.lyaml b/lang/en.lyaml index fd4f17732..e036e8990 100644 --- a/lang/en.lyaml +++ b/lang/en.lyaml @@ -1311,6 +1311,7 @@ en: invalidCharacters: "The selected destination contains invalid characters. Please provide a new path and try again." invalidTemplate: "[--template] Could not find template {{ template }}. Please choose an available template." noProjectsInConfig: "Please ensure that there is a config.json file that contains a \"projects\" field." + missingConfigFileTemplateSource: "Please ensure that there is a config.json file in the repository used as the --template-source" missingPropertiesInConfig: "Please ensure that each of the projects in your config.json file contain the following properties: [\"name\", \"label\", \"path\", \"insertPath\"]." selectPublicAppPrompt: selectAppIdMigrate: "[--appId] Choose an app under {{ accountName }} to migrate:" diff --git a/lib/prompts/createProjectPrompt.ts b/lib/prompts/createProjectPrompt.ts index dfae0cad5..e85eec2c3 100644 --- a/lib/prompts/createProjectPrompt.ts +++ b/lib/prompts/createProjectPrompt.ts @@ -50,11 +50,17 @@ async function createTemplateOptions( ? DEFAULT_PROJECT_TEMPLATE_BRANCH : githubRef; - const config = await fetchFileFromRepository( - templateSource || HUBSPOT_PROJECT_COMPONENTS_GITHUB_PATH, - 'config.json', - branch - ); + let config: ProjectTemplateRepoConfig; + try { + config = await fetchFileFromRepository( + templateSource || HUBSPOT_PROJECT_COMPONENTS_GITHUB_PATH, + 'config.json', + branch + ); + } catch (e) { + logger.error(i18n(`${i18nKey}.errors.missingConfigFileTemplateSource`)); + process.exit(EXIT_CODES.ERROR); + } if (!config || !config[PROJECT_COMPONENT_TYPES.PROJECTS]) { logger.error(i18n(`${i18nKey}.errors.noProjectsInConfig`)); From f936c675eebf981e496cbac360287a32f987e3d6 Mon Sep 17 00:00:00 2001 From: Branden Rodgers Date: Wed, 22 Jan 2025 12:41:09 -0500 Subject: [PATCH 2/2] chore: expand lib test coverage (hasFlags, hasFeatures, developerTestAccounts) (#1348) --- lib/__tests__/developerTestAccounts.test.ts | 221 ++++++++++++++++++++ lib/__tests__/hasFeature.test.ts | 39 ++++ lib/__tests__/hasFlag.test.ts | 23 ++ lib/developerTestAccounts.ts | 4 +- 4 files changed, 286 insertions(+), 1 deletion(-) create mode 100644 lib/__tests__/developerTestAccounts.test.ts create mode 100644 lib/__tests__/hasFeature.test.ts create mode 100644 lib/__tests__/hasFlag.test.ts diff --git a/lib/__tests__/developerTestAccounts.test.ts b/lib/__tests__/developerTestAccounts.test.ts new file mode 100644 index 000000000..033d2f48e --- /dev/null +++ b/lib/__tests__/developerTestAccounts.test.ts @@ -0,0 +1,221 @@ +import { getAccountId, getConfigAccounts } from '@hubspot/local-dev-lib/config'; +import { logger } from '@hubspot/local-dev-lib/logger'; +import { CLIAccount } from '@hubspot/local-dev-lib/types/Accounts'; +import { HubSpotHttpError } from '@hubspot/local-dev-lib/models/HubSpotHttpError'; +import { HUBSPOT_ACCOUNT_TYPES } from '@hubspot/local-dev-lib/constants/config'; +import { fetchDeveloperTestAccounts } from '@hubspot/local-dev-lib/api/developerTestAccounts'; +import * as errorHandlers from '../errorHandlers'; +import { + getHasDevTestAccounts, + handleDeveloperTestAccountCreateError, + validateDevTestAccountUsageLimits, +} from '../developerTestAccounts'; + +jest.mock('@hubspot/local-dev-lib/config'); +jest.mock('@hubspot/local-dev-lib/logger'); +jest.mock('@hubspot/local-dev-lib/api/developerTestAccounts'); +jest.mock('../errorHandlers'); + +const mockedGetAccountId = getAccountId as jest.Mock; +const mockedGetConfigAccounts = getConfigAccounts as jest.Mock; +const mockedFetchDeveloperTestAccounts = + fetchDeveloperTestAccounts as jest.Mock; + +const APP_DEVELOPER_ACCOUNT_1: CLIAccount = { + name: 'app-developer-1', + accountId: 123, + accountType: HUBSPOT_ACCOUNT_TYPES.APP_DEVELOPER, + env: 'prod', +}; + +const APP_DEVELOPER_ACCOUNT_2: CLIAccount = { + name: 'app-developer-2', + accountId: 456, + accountType: HUBSPOT_ACCOUNT_TYPES.APP_DEVELOPER, + env: 'prod', +}; + +const accounts: CLIAccount[] = [ + APP_DEVELOPER_ACCOUNT_1, + APP_DEVELOPER_ACCOUNT_2, + { + name: 'test-account', + accountId: 789, + parentAccountId: APP_DEVELOPER_ACCOUNT_1.accountId, + accountType: HUBSPOT_ACCOUNT_TYPES.DEVELOPER_TEST, + env: 'prod', + }, +]; + +const makeHubSpotHttpError = ( + message: string, + response: object +): HubSpotHttpError => { + return new HubSpotHttpError(message, { + cause: { isAxiosError: true, response }, + }); +}; + +describe('lib/developerTestAccounts', () => { + describe('getHasDevTestAccounts()', () => { + it('should return true if there are developer test accounts associated with the account', () => { + mockedGetAccountId.mockReturnValueOnce(APP_DEVELOPER_ACCOUNT_1.accountId); + mockedGetConfigAccounts.mockReturnValueOnce(accounts); + + const result = getHasDevTestAccounts(APP_DEVELOPER_ACCOUNT_1); + expect(result).toBe(true); + }); + + it('should return false if there are no developer test accounts associated with the account', () => { + mockedGetAccountId.mockReturnValueOnce(APP_DEVELOPER_ACCOUNT_2.accountId); + mockedGetConfigAccounts.mockReturnValueOnce(accounts); + + const result = getHasDevTestAccounts(APP_DEVELOPER_ACCOUNT_2); + expect(result).toBe(false); + }); + + it('should return false if there are no accounts configured', () => { + mockedGetAccountId.mockReturnValueOnce(APP_DEVELOPER_ACCOUNT_1.accountId); + mockedGetConfigAccounts.mockReturnValueOnce(undefined); + + const result = getHasDevTestAccounts(APP_DEVELOPER_ACCOUNT_1); + expect(result).toBe(false); + }); + }); + + describe('validateDevTestAccountUsageLimits()', () => { + afterEach(() => { + mockedGetAccountId.mockRestore(); + mockedFetchDeveloperTestAccounts.mockRestore(); + }); + + it('should return null if the account id is not found', async () => { + mockedGetAccountId.mockReturnValueOnce(undefined); + + const result = await validateDevTestAccountUsageLimits( + APP_DEVELOPER_ACCOUNT_1 + ); + expect(result).toBe(null); + }); + + it('should return null if there is no developer test account data', async () => { + mockedGetAccountId.mockReturnValueOnce(APP_DEVELOPER_ACCOUNT_1.accountId); + mockedFetchDeveloperTestAccounts.mockResolvedValueOnce({ + data: null, + }); + + const result = await validateDevTestAccountUsageLimits( + APP_DEVELOPER_ACCOUNT_1 + ); + expect(result).toBe(null); + }); + + it('should return the test account data if the account has not reached the limit', async () => { + mockedGetAccountId.mockReturnValueOnce(APP_DEVELOPER_ACCOUNT_1.accountId); + const testAccountData = { + maxTestPortals: 10, + results: [], + }; + mockedFetchDeveloperTestAccounts.mockResolvedValueOnce({ + data: testAccountData, + }); + + const result = await validateDevTestAccountUsageLimits( + APP_DEVELOPER_ACCOUNT_1 + ); + expect(result).toEqual(expect.objectContaining(testAccountData)); + }); + + it('should throw an error if the account has reached the limit', () => { + mockedGetAccountId.mockReturnValueOnce(APP_DEVELOPER_ACCOUNT_1.accountId); + mockedFetchDeveloperTestAccounts.mockResolvedValueOnce({ + data: { + maxTestPortals: 0, + results: [{}], + }, + }); + + expect( + validateDevTestAccountUsageLimits(APP_DEVELOPER_ACCOUNT_1) + ).rejects.toThrow(); + }); + }); + + describe('handleDeveloperTestAccountCreateError()', () => { + let loggerErrorSpy: jest.SpyInstance; + let logErrorSpy: jest.SpyInstance; + + beforeEach(() => { + loggerErrorSpy = jest.spyOn(logger, 'error'); + logErrorSpy = jest.spyOn(errorHandlers, 'logError'); + }); + + afterEach(() => { + loggerErrorSpy.mockRestore(); + logErrorSpy.mockRestore(); + }); + + it('should log and throw an error if the account is missing the required scopes', () => { + const missingScopesError = makeHubSpotHttpError('Missing scopes error', { + status: 403, + data: { + message: 'Missing scopes error', + category: 'MISSING_SCOPES', + }, + }); + + expect(() => + handleDeveloperTestAccountCreateError( + missingScopesError, + APP_DEVELOPER_ACCOUNT_1.accountId, + 'prod', + 10 + ) + ).toThrow('Missing scopes error'); + expect(loggerErrorSpy).toHaveBeenCalled(); + }); + + it('should log and throw an error if the account is missing the required scopes', () => { + const portalLimitReachedError = makeHubSpotHttpError( + 'Portal limit reached error', + { + status: 400, + data: { + message: 'Portal limit reached error', + errorType: 'TEST_PORTAL_LIMIT_REACHED', + }, + } + ); + + expect(() => + handleDeveloperTestAccountCreateError( + portalLimitReachedError, + APP_DEVELOPER_ACCOUNT_1.accountId, + 'prod', + 10 + ) + ).toThrow('Portal limit reached error'); + expect(loggerErrorSpy).toHaveBeenCalled(); + }); + + it('should log a generic error message for an unknown error type', () => { + const someUnknownError = makeHubSpotHttpError('Some unknown error', { + status: 400, + data: { + message: 'Some unknown error', + category: 'SOME_UNKNOWN_ERROR', + }, + }); + + expect(() => + handleDeveloperTestAccountCreateError( + someUnknownError, + APP_DEVELOPER_ACCOUNT_1.accountId, + 'prod', + 10 + ) + ).toThrow('Some unknown error'); + expect(logErrorSpy).toHaveBeenCalled(); + }); + }); +}); diff --git a/lib/__tests__/hasFeature.test.ts b/lib/__tests__/hasFeature.test.ts new file mode 100644 index 000000000..7d2858bb9 --- /dev/null +++ b/lib/__tests__/hasFeature.test.ts @@ -0,0 +1,39 @@ +import { fetchEnabledFeatures } from '@hubspot/local-dev-lib/api/localDevAuth'; +import { hasFeature } from '../hasFeature'; + +jest.mock('@hubspot/local-dev-lib/api/localDevAuth'); + +const mockedFetchEnabledFeatures = fetchEnabledFeatures as jest.Mock; + +describe('lib/hasFeature', () => { + describe('hasFeature()', () => { + const accountId = 123; + + beforeEach(() => { + mockedFetchEnabledFeatures.mockResolvedValueOnce({ + data: { + enabledFeatures: { + 'feature-1': true, + 'feature-2': false, + 'feature-3': true, + }, + }, + }); + }); + + it('should return true if the feature is enabled', async () => { + const result = await hasFeature(accountId, 'feature-1'); + expect(result).toBe(true); + }); + + it('should return false if the feature is not enabled', async () => { + const result = await hasFeature(accountId, 'feature-2'); + expect(result).toBe(false); + }); + + it('should return false if the feature is not present', async () => { + const result = await hasFeature(accountId, 'feature-4'); + expect(result).toBe(false); + }); + }); +}); diff --git a/lib/__tests__/hasFlag.test.ts b/lib/__tests__/hasFlag.test.ts new file mode 100644 index 000000000..6ce3db63d --- /dev/null +++ b/lib/__tests__/hasFlag.test.ts @@ -0,0 +1,23 @@ +import { hasFlag } from '../hasFlag'; + +const argvWithFlag = ['hs', 'command', '--test']; +const argvWithoutFlag = ['hs', 'command']; + +describe('lib/hasFlag', () => { + describe('hasFlag()', () => { + it('should return true if the flag is present', () => { + const flag = hasFlag('test', argvWithFlag); + expect(flag).toBe(true); + }); + + it('should return false if argv is an empty array', () => { + const flag = hasFlag('test', []); + expect(flag).toBe(false); + }); + + it('should return false if the flag is not present', () => { + const flag = hasFlag('test', argvWithoutFlag); + expect(flag).toBe(false); + }); + }); +}); diff --git a/lib/developerTestAccounts.ts b/lib/developerTestAccounts.ts index 3dcdcbbdc..7f1aab5e3 100644 --- a/lib/developerTestAccounts.ts +++ b/lib/developerTestAccounts.ts @@ -16,7 +16,9 @@ import { logError } from './errorHandlers/index'; import { FetchDeveloperTestAccountsResponse } from '@hubspot/local-dev-lib/types/developerTestAccounts'; import { Environment } from '@hubspot/local-dev-lib/types/Config'; -function getHasDevTestAccounts(appDeveloperAccountConfig: CLIAccount): boolean { +export function getHasDevTestAccounts( + appDeveloperAccountConfig: CLIAccount +): boolean { const id = getAccountIdentifier(appDeveloperAccountConfig); const parentPortalId = getAccountId(id); const accountsList = getConfigAccounts();