Fix Connect Cloud credential selection when there are multiple accounts#3446
Fix Connect Cloud credential selection when there are multiple accounts#3446
Conversation
| (!account?.id || !account?.name || !account?.displayName) && | ||
| accounts[0] | ||
| ) { | ||
| if (account?.id && account?.name && account?.displayName) { |
There was a problem hiding this comment.
Is there a reason to check each field individually here instead of just checking for a non-nullish account object?
There was a problem hiding this comment.
I think you can just check for the object like: if (account) { ... }, in fact I would write this code this way so there is always a fallback and no else if statement is needed:
// fallback to the first publishable account if the selected account is ever not found
const account = accounts.find((a) => a.displayName === pick.label) || accounts[0];
if (account) {
state.data.accountId = account.id;
state.data.accountName = account.name;
state.data.displayName = account.displayName;
}
rodriin
left a comment
There was a problem hiding this comment.
Nice catch, I suggested a small code change.
| (!account?.id || !account?.name || !account?.displayName) && | ||
| accounts[0] | ||
| ) { | ||
| if (account?.id && account?.name && account?.displayName) { |
There was a problem hiding this comment.
I think you can just check for the object like: if (account) { ... }, in fact I would write this code this way so there is always a fallback and no else if statement is needed:
// fallback to the first publishable account if the selected account is ever not found
const account = accounts.find((a) => a.displayName === pick.label) || accounts[0];
if (account) {
state.data.accountId = account.id;
state.data.accountName = account.name;
state.data.displayName = account.displayName;
}
tyatposit
left a comment
There was a problem hiding this comment.
Should we add a unit test to try to catch this should something like it happen again?
|
Marking this as a fix for #3440 |
Intent
Account selection is broken when there are multiple PCC accounts - when the user selects an account, no credential is created, and instead this is logged:
[Extension Host] User has dismissed the New Credential flow. Exiting. (at console.<anonymous> (file:///Applications/Positron.app/Contents/Resources/app/out/vs/workbench/api/node/extensionHostProcess.js:96148:14))The bug was introduced here: https://github.com/posit-dev/publisher/pull/3303/changes#diff-85757b4df2fd959fdb90483f09e44e9c37757ff9cf7995217245e569f6680e29
Type of Change
Approach
Fix the condition checking for account - We needed to check if the account from
.findis present. The actual happy path was removed and only the fallback case was handled.User Impact
Allows users who have multiple accounts to be able to create credentials again.
Automated Tests
No established pattern for unit tests in this area of the code.
Directions for Reviewers
Set up access on your Connect Cloud user to have access to multiple accounts to be able to publish to, then try to add a credential for one of them.
Checklist