Skip to content
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

Replace "Sign Out" with an account dialog #2233

Merged
merged 21 commits into from
Dec 11, 2023
Merged

Replace "Sign Out" with an account dialog #2233

merged 21 commits into from
Dec 11, 2023

Conversation

toolmantim
Copy link
Contributor

@toolmantim toolmantim commented Dec 9, 2023

Replaces the "Sign Out" option with a new Account dialog that gives you visibility of what user you're signed in as, which server, and what plan.

State Screenshot
Cody Pro FF Not Enabled Screenshot 2023-12-11 at 8 39 59 pm
Cody Pro FF Enabled Screenshot 2023-12-11 at 8 40 22 pm
Enterprise Instance Screenshot 2023-12-11 at 8 41 16 pm

Notes:

  • Display name is often missing in Enterprise accounts, so we just use the email

Fixes #1520

Test plan

  • Signed into DotCom
    • Clicked account, switch, sign out, etc
    • Changed plan, return, clicked account, verified plan has changed
    • Turn off CodyPro feature flag, clicked account, verify no manage or plan
  • Signed in to S2 (Enterprise), clicked account, switch, sign out, etc

@toolmantim toolmantim changed the title Account dialog Add account dialog with Dec 9, 2023
@toolmantim toolmantim changed the title Add account dialog with Replace "Sign Out" with an account dialog that shows endpoint, plan and manage link Dec 9, 2023
@toolmantim toolmantim marked this pull request as ready for review December 11, 2023 06:11
Copy link
Contributor

@dominiccooney dominiccooney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to look at the failing agent test.

If you have time and are so inclined, you can now e2e test the isDotCom/or not paths in e2e cribbing from here: https://github.com/sourcegraph/cody/blob/main/vscode/test/e2e/local-embeddings.test.ts#L18-L20

vscode/src/chat/utils.test.ts Outdated Show resolved Hide resolved
vscode/src/chat/utils.ts Outdated Show resolved Hide resolved
vscode/src/services/AuthProvider.ts Outdated Show resolved Hide resolved
vscode/src/services/TreeViewProvider.test.ts Outdated Show resolved Hide resolved
@dominiccooney
Copy link
Contributor

I think the agent tests just needed a replay archive update, I have pushed one to your branch.

Copy link
Contributor

@dominiccooney dominiccooney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving but double check #2233 (comment)

@toolmantim toolmantim changed the title Replace "Sign Out" with an account dialog that shows endpoint, plan and manage link Replace "Sign Out" with an account dialog Dec 11, 2023
@toolmantim
Copy link
Contributor Author

If you have time and are so inclined, you can now e2e test the isDotCom/or not paths in e2e cribbing from here: https://github.com/sourcegraph/cody/blob/main/vscode/test/e2e/local-embeddings.test.ts#L18-L20

@dominiccooney this is awesome! I started on an e2e test for this, but couldn't get Playwright to work nicely with the dialogs :(

@toolmantim toolmantim merged commit 78d8748 into main Dec 11, 2023
14 checks passed
@toolmantim toolmantim deleted the tl/account-dialog branch December 11, 2023 10:38
abeatrix added a commit that referenced this pull request Jan 10, 2024
CLOSE #2663 &
#2664

A bug was introduced in #2233
where we added a requirement for `primaryEmail` during auth, resulting
in uncaught error that stopped the extension from starting.

For Enterprise instances, emails are not always required to set up an
account.

This PR includes the following changes to fix the issue above:
- update authStatus and graphQL query to reflect that `primaryEmail` and
`displayName` fields are optional
- add tests for the above change
- use username instead of displayName for enterprise instances as
username is a required field
- replace replace getEnterpriseCurrentUserInfo and
getDotComCurrentUserInfo with the new getCurrentUserInfo
- update agent tests with the changes above

## Test plan

<!-- Required. See
https://sourcegraph.com/docs/dev/background-information/testing_principles.
-->

1. Log into Cody using an enterprise soucegraph (e.g. S2) token
associated with a user that doesn't have emails set up for their
account.
2. Try reloading VS Code

### Before

Unable to start Cody on reload


https://github.com/sourcegraph/cody/assets/68532117/85ad63ef-c422-45a4-b4f8-640d5e11ecf3

### After


https://www.loom.com/share/b9049f54cb0f48da87fc630c84cc736e?sid=0e1b3677-9c3e-4c28-84b5-846f95ad4a16


https://github.com/sourcegraph/cody/assets/68532117/dd2a54f9-dfdf-4ff2-b789-a032dc6b3b1d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VSCE: “Sign Out” needs a confirmation
3 participants