feat: Convert deprecated configs back to old format before writing (#341)

* Write to deprecated configs with old formatting

* Actually use the reformatted config

* fix tests

---------

Co-authored-by: Branden Rodgers <branden@rodgersworld.com>

Author: camden11

config/__tests__/config.test.ts

@@ -277,7 +277,7 @@ describe('config/index', () => {
 
       expect(mockFs.writeFileSync).toHaveBeenCalledWith(
         getLocalConfigDefaultFilePath(),
-        yaml.dump({ accounts: [] })
+        yaml.dump({ portals: [] })
       );
     });
   });

config/__tests__/utils.test.ts

@@ -1,17 +1,22 @@
-import findup from 'findup-sync';
 import fs from 'fs-extra';
 import {
   getLocalConfigDefaultFilePath,
   getConfigPathEnvironmentVariables,
+  doesConfigFileExistAtPath,
   readConfigFile,
   removeUndefinedFieldsFromConfigAccount,
+  formatConfigForWrite,
   writeConfigFile,
+  parseConfig,
   normalizeParsedConfig,
+  convertToDeprecatedConfig,
   buildConfigFromEnvironment,
+  getAccountIdentifierAndType,
   getConfigAccountByIdentifier,
+  getConfigAccountByInferredIdentifier,
   getConfigAccountIndexById,
   validateConfigAccount,
-  getAccountIdentifierAndType,
+  handleConfigFileSystemError,
 } from '../utils';
 import { HubSpotConfigError } from '../../models/HubSpotConfigError';
 import { getCwd } from '../../lib/path';
@@ -39,7 +44,6 @@ jest.mock('findup-sync');
 jest.mock('../../lib/path');
 jest.mock('fs-extra');
 
-const mockFindup = findup as jest.MockedFunction<typeof findup>;
 const mockCwd = getCwd as jest.MockedFunction<typeof getCwd>;
 const mockFs = fs as jest.Mocked<typeof fs>;
 
@@ -224,6 +228,127 @@ describe('config/utils', () => {
     });
   });
 
+  describe('convertToDeprecatedConfig()', () => {
+    it('converts account fields to portal fields', () => {
+      const deprecated = convertToDeprecatedConfig(CONFIG);
+
+      expect(deprecated.portals).toBeDefined();
+      expect(deprecated.accounts).toBeUndefined();
+      expect(deprecated.portals).toHaveLength(1);
+      expect(deprecated.portals![0]).toHaveProperty('portalId', 123);
+      expect(deprecated.portals![0]).not.toHaveProperty('accountId');
+    });
+
+    it('converts defaultAccount to defaultPortal', () => {
+      const deprecated = convertToDeprecatedConfig(CONFIG);
+
+      expect(deprecated.defaultPortal).toBe('test-account');
+      expect(deprecated.defaultAccount).toBeUndefined();
+    });
+
+    it('preserves personal access key account fields', () => {
+      const deprecated = convertToDeprecatedConfig(CONFIG);
+
+      expect(deprecated.portals![0]).toMatchObject({
+        name: 'test-account',
+        portalId: 123,
+        authType: 'personalaccesskey',
+        personalAccessKey: 'test-key',
+        env: 'qa',
+      });
+    });
+
+    it('preserves OAuth account fields', () => {
+      const oauthConfig = {
+        ...CONFIG,
+        accounts: [OAUTH_ACCOUNT],
+        defaultAccount: 123,
+      };
+      const deprecated = convertToDeprecatedConfig(oauthConfig);
+
+      expect(deprecated.portals![0]).toMatchObject({
+        name: '123',
+        portalId: 123,
+        authType: 'oauth2',
+        auth: {
+          clientId: 'test-client-id',
+          clientSecret: 'test-client-secret',
+          tokenInfo: {
+            refreshToken: 'test-refresh-token',
+          },
+          scopes: ['content', 'hubdb', 'files'],
+        },
+      });
+    });
+
+    it('preserves API key account fields', () => {
+      const apiKeyConfig = {
+        ...CONFIG,
+        accounts: [API_KEY_ACCOUNT],
+        defaultAccount: 123,
+      };
+      const deprecated = convertToDeprecatedConfig(apiKeyConfig);
+
+      expect(deprecated.portals![0]).toMatchObject({
+        name: '123',
+        portalId: 123,
+        authType: 'apikey',
+        apiKey: 'test-api-key',
+      });
+    });
+
+    it('handles multiple accounts', () => {
+      const multiAccountConfig = {
+        ...CONFIG,
+        accounts: [PAK_ACCOUNT, OAUTH_ACCOUNT, API_KEY_ACCOUNT],
+        defaultAccount: 123,
+      };
+      const deprecated = convertToDeprecatedConfig(multiAccountConfig);
+
+      expect(deprecated.portals).toHaveLength(3);
+      expect(deprecated.portals![0].portalId).toBe(123);
+      expect(deprecated.portals![1].portalId).toBe(123);
+      expect(deprecated.portals![2].portalId).toBe(123);
+    });
+
+    it('handles config without defaultAccount', () => {
+      const configWithoutDefault = {
+        ...CONFIG,
+        defaultAccount: undefined,
+      };
+      const deprecated = convertToDeprecatedConfig(configWithoutDefault);
+
+      expect(deprecated.defaultPortal).toBeUndefined();
+      expect(deprecated.defaultAccount).toBeUndefined();
+    });
+
+    it('preserves all other config fields', () => {
+      const deprecated = convertToDeprecatedConfig(CONFIG);
+
+      expect(deprecated.defaultCmsPublishMode).toBe('publish');
+      expect(deprecated.httpTimeout).toBe(1000);
+      expect(deprecated.httpUseLocalhost).toBe(true);
+      expect(deprecated.allowUsageTracking).toBe(true);
+    });
+
+    it('does not mutate the original config', () => {
+      const testConfig = structuredClone(CONFIG);
+
+      convertToDeprecatedConfig(testConfig);
+
+      // Verify original structure is preserved
+      expect(testConfig.accounts).toBeDefined();
+      expect(testConfig.accounts).toHaveLength(1);
+      expect(testConfig.accounts[0]).toHaveProperty('accountId', 123);
+      expect(testConfig.defaultAccount).toBe(123);
+
+      // Verify deprecated fields were not added to original
+      expect(testConfig).not.toHaveProperty('portals');
+      expect(testConfig).not.toHaveProperty('defaultPortal');
+      expect(testConfig.accounts[0]).not.toHaveProperty('portalId');
+    });
+  });
+
   describe('buildConfigFromEnvironment()', () => {
     it('builds personal access key config', () => {
       process.env[ENVIRONMENT_VARIABLES.HUBSPOT_PERSONAL_ACCESS_KEY] =
@@ -385,4 +510,193 @@ describe('config/utils', () => {
       expect(identifierType).toBe('accountId');
     });
   });
+
+  describe('doesConfigFileExistAtPath()', () => {
+    it('returns true when file exists', () => {
+      mockFs.existsSync.mockReturnValue(true);
+      expect(doesConfigFileExistAtPath('/path/to/config.yml')).toBe(true);
+    });
+
+    it('returns false when file does not exist', () => {
+      mockFs.existsSync.mockReturnValue(false);
+      expect(doesConfigFileExistAtPath('/path/to/config.yml')).toBe(false);
+    });
+
+    it('throws HubSpotConfigError on error', () => {
+      mockFs.existsSync.mockImplementation(() => {
+        throw new Error('Permission denied');
+      });
+      expect(() => doesConfigFileExistAtPath('/path/to/config.yml')).toThrow(
+        HubSpotConfigError
+      );
+    });
+  });
+
+  describe('formatConfigForWrite()', () => {
+    it('formats config with consistent field order', () => {
+      const formattedConfig = formatConfigForWrite(CONFIG);
+      const keys = Object.keys(formattedConfig);
+      expect(keys[0]).toBe('defaultAccount');
+      expect(keys[keys.length - 1]).toBe('accounts');
+    });
+
+    it('removes undefined fields from accounts', () => {
+      const configWithUndefined = {
+        ...CONFIG,
+        accounts: [
+          {
+            ...PAK_ACCOUNT,
+            someUndefinedField: undefined,
+          } as any,
+        ],
+      };
+      const formattedConfig = formatConfigForWrite(configWithUndefined);
+      expect(formattedConfig.accounts[0]).not.toHaveProperty(
+        'someUndefinedField'
+      );
+    });
+
+    it('preserves all account types', () => {
+      const multiAccountConfig = {
+        ...CONFIG,
+        accounts: [PAK_ACCOUNT, OAUTH_ACCOUNT, API_KEY_ACCOUNT],
+      };
+      const formatted = formatConfigForWrite(multiAccountConfig);
+      expect(formatted.accounts).toHaveLength(3);
+      expect(formatted.accounts[0].authType).toBe('personalaccesskey');
+      expect(formatted.accounts[1].authType).toBe('oauth2');
+      expect(formatted.accounts[2].authType).toBe('apikey');
+    });
+  });
+
+  describe('parseConfig()', () => {
+    it('parses valid YAML config', () => {
+      const yamlConfig = `
+accounts:
+  - name: test-account
+    accountId: 123
+    authType: personalaccesskey
+    personalAccessKey: test-key
+    env: qa
+    auth:
+      tokenInfo: {}
+    accountType: STANDARD
+defaultAccount: 123
+defaultCmsPublishMode: publish
+httpTimeout: 1000
+httpUseLocalhost: true
+allowUsageTracking: true
+`;
+      const parsed = parseConfig(yamlConfig, '/path/to/config.yml');
+      expect(parsed).toEqual(CONFIG);
+    });
+
+    it('throws HubSpotConfigError on invalid YAML', () => {
+      const invalidYaml = 'invalid: yaml: content: [';
+      expect(() => parseConfig(invalidYaml, '/path/to/config.yml')).toThrow(
+        HubSpotConfigError
+      );
+    });
+
+    it('normalizes deprecated config format', () => {
+      const deprecatedYaml = `
+portals:
+  - name: test-account
+    portalId: 123
+    authType: personalaccesskey
+    personalAccessKey: test-key
+    env: qa
+    auth:
+      tokenInfo: {}
+defaultPortal: test-account
+defaultMode: publish
+httpTimeout: 1000
+httpUseLocalhost: true
+allowUsageTracking: true
+`;
+      const parsed = parseConfig(deprecatedYaml, '/path/to/config.yml');
+      expect(parsed).toEqual(CONFIG);
+    });
+  });
+
+  describe('getConfigAccountByInferredIdentifier()', () => {
+    it('finds account by numeric accountId', () => {
+      const account = getConfigAccountByInferredIdentifier(
+        CONFIG.accounts,
+        123
+      );
+      expect(account).toEqual(PAK_ACCOUNT);
+    });
+
+    it('finds account by string accountId', () => {
+      const account = getConfigAccountByInferredIdentifier(
+        CONFIG.accounts,
+        '123'
+      );
+      expect(account).toEqual(PAK_ACCOUNT);
+    });
+
+    it('finds account by name', () => {
+      const account = getConfigAccountByInferredIdentifier(
+        CONFIG.accounts,
+        'test-account'
+      );
+      expect(account).toEqual(PAK_ACCOUNT);
+    });
+
+    it('handles accounts with numeric names as fallback', () => {
+      const accountWithNumericName = {
+        ...PAK_ACCOUNT,
+        name: '456',
+        accountId: 789,
+      };
+      const accounts = [accountWithNumericName];
+      const account = getConfigAccountByInferredIdentifier(accounts, '456');
+      expect(account).toEqual(accountWithNumericName);
+    });
+
+    it('returns undefined when account not found', () => {
+      const account = getConfigAccountByInferredIdentifier(
+        CONFIG.accounts,
+        999
+      );
+      expect(account).toBeUndefined();
+    });
+  });
+
+  describe('handleConfigFileSystemError()', () => {
+    it('handles ENOENT error', () => {
+      const error = Object.assign(new Error('File not found'), {
+        code: 'ENOENT',
+      });
+      const { message, type } = handleConfigFileSystemError(
+        error,
+        '/path/to/config.yml'
+      );
+      expect(message).toContain('No config file found');
+      expect(type).toBe('CONFIG_NOT_FOUND');
+    });
+
+    it('handles EACCES error', () => {
+      const error = Object.assign(new Error('Permission denied'), {
+        code: 'EACCES',
+      });
+      const { message, type } = handleConfigFileSystemError(
+        error,
+        '/path/to/config.yml'
+      );
+      expect(message).toContain('Insufficient permissions');
+      expect(type).toBe('INSUFFICIENT_PERMISSIONS');
+    });
+
+    it('handles unknown errors', () => {
+      const error = new Error('Unknown error');
+      const { message, type } = handleConfigFileSystemError(
+        error,
+        '/path/to/config.yml'
+      );
+      expect(message).toBeUndefined();
+      expect(type).toBe('UNKNOWN');
+    });
+  });
 });