-
Notifications
You must be signed in to change notification settings - Fork 72
Support dev on dev sandbox accounts if the portal has access #1535
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
joe-yeager
left a comment
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.
Code LGTM, just a non-blocking nit about splitting the logic up
| if (accountType === HUBSPOT_ACCOUNT_TYPES.DEVELOPER_TEST) { | ||
| const devAccountPromptResponse = | ||
| await selectDeveloperTestTargetAccountPrompt(accounts!, accountConfig); | ||
|
|
||
| targetTestingAccountId = devAccountPromptResponse.targetAccountId; | ||
|
|
||
| if (!!devAccountPromptResponse.notInConfigAccount) { | ||
| // When the developer test account isn't configured in the CLI config yet | ||
| // Walk the user through adding the account's PAK to the config | ||
| await useExistingDevTestAccount( | ||
| env, | ||
| devAccountPromptResponse.notInConfigAccount | ||
| ); | ||
| } else if (devAccountPromptResponse.createNestedAccount) { | ||
| // Create a new developer test account and automatically add it to the CLI config | ||
| targetTestingAccountId = await createDeveloperTestAccountForLocalDev( | ||
| targetProjectAccountId, | ||
| accountConfig, | ||
| env | ||
| ); | ||
| } | ||
| } else if (accountType === HUBSPOT_ACCOUNT_TYPES.DEVELOPMENT_SANDBOX) { | ||
| const sandboxAccountPromptResponse = | ||
| await selectSandboxTargetAccountPrompt(accounts!, accountConfig); | ||
|
|
||
| targetTestingAccountId = sandboxAccountPromptResponse.targetAccountId; | ||
| } else { | ||
| targetTestingAccountId = targetProjectAccountId; |
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.
Non-blocking nit: This section might benefit from a refactor to split this up into smaller pieces.
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.
I may hold off and let Camden break this up however he wants to when he gets back. He was another WIP pr that touches this code too.
Description and Context
Add support for selecting dev sandbox accounts during the
hs project devinit flow if the user's account has access to them. There are quite a few permutations to highlight for this. I'll try to call out the most important flows.Previously we were requiring that the default account was a developer account or a standard account in order to run local dev for unified apps. This was an attempt to make sure that the account was opted in to leverage some of the new flows. Now we have a gate checks for it, so we use that instead.
If the user provides
--account, we'll make the project account and the testing account the same thing. i.e. we'll upload the project to the same account that we're testing on. Longer term we're thinking about splitting this arg into something along the lines oftargetProjectAccountandtargetTestingAccountto give more flexibility.Screenshots
TODO
Who to Notify