-
Notifications
You must be signed in to change notification settings - Fork 72
Support projectAccount and testingAccount flags + clean up unified app account logic #1542
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
Conversation
commands/project/dev/index.ts
Outdated
| if ( | ||
| (testingAccount && !projectAccount) || | ||
| (projectAccount && !testingAccount) | ||
| ) { | ||
| uiLogger.error(commands.project.dev.errors.invalidAccountFlags); | ||
| process.exit(EXIT_CODES.ERROR); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might be able to leverage yargs.implies for this. It also looks like there is an implies key that can be passed to yargs.option so we can make this args require each other
lang/en.ts
Outdated
| noRunnableComponents: (command: string) => | ||
| `No supported components were found in this project. Run ${chalk.bold(command)} to see a list of available components and add one to your project.`, | ||
| noRunnableComponents: `No supported components were found in this project. Run ${uiCommandReference('hs project add')} to see a list of available components and add one to your project.`, | ||
| accountNotCombined: `Local development of Unified Apps is currently only compatible with accounts that are opted into the unified apps beta. Make sure that this account is opted in or switch accounts using ${uiCommandReference('hs account use')}.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: unified apps is capitalized in one usage, but not the other.
lang/en.ts
Outdated
| unsupportedAccountFlagLegacy: | ||
| 'The --projectAccount and --testingAccount flags are not supported for projects with platform versions earlier than 2025.2.', | ||
| unsupportedAccountFlagV3: | ||
| 'The --account flag is is not supported supported for projects with platform versions 2025.2 and newer. User --testingAccount and --projectAccount flags to specify accounts to use for local dev', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: User --testingAccount and --projectAccount should be Use --testingAccount and --projectAccount
| 'Projects cannot contain both private and public apps. Move your apps to separate projects before attempting local development.', | ||
| noRunnableComponents: (command: string) => | ||
| `No supported components were found in this project. Run ${chalk.bold(command)} to see a list of available components and add one to your project.`, | ||
| noRunnableComponents: `No supported components were found in this project. Run ${uiCommandReference('hs project add')} to see a list of available components and add one to your project.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I believe we are calling components features to external users
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I wasn't aware of that, did that change recently? We have a lot of references to components in the CLI. Even the sample repo is called hubspot-project-components
If that is changing, I think it'd make more sense to go through and update all of the references at once
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's a fairly recent change (last few months).
Even the sample repo is called hubspot-project-components
It's fine in internal references or repo names (probably) I thinks it's just the user facing copy that UX wants us to align on ASAP. Follow up PR sounds reasonable, I'm not sure when they want us to have it switched over by.
Maybe @brandenrodgers knows more
commands/project/dev/unifiedFlow.ts
Outdated
| accountUseCommand: uiCommandReference('hs account use'), | ||
| }) | ||
| ); | ||
| uiLogger.log(''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I believe we don't need to add these uiLogger.log('') to get a new line anymore. We could just prefix commands.project.dev.errors.accountNotCombined with a \n
Description and Context
This PR makes many of the changes from #1529 without changing where projects are uploaded to:
--projectAccountand--testingAccountflags--accountflag from projects with platform version2025.2and abovecommands/dev/indexandunfiedFlowTesting
hs project devwith the--projectAccountand--testingAccountflags and confirm testing account prompt is skipped and accounts are targeted as expectedhs project devwithout the flags and confirm flow hasn't changedWho to Notify
@brandenrodgers @joe-yeager