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

OAuth account management #243

Merged
merged 10 commits into from Mar 24, 2020
Merged

OAuth account management #243

merged 10 commits into from Mar 24, 2020

Conversation

@ngerakines
Copy link
Collaborator

ngerakines commented Jan 15, 2020

This PR introduces oauth account management functionality on the account settings page. With it, oauth accounts can be added and removed from existing accounts.


  • I have signed the CLA
…nd required data migration. T713
@ngerakines ngerakines requested a review from thebaer Jan 15, 2020
@ngerakines ngerakines self-assigned this Jan 15, 2020
@ngerakines ngerakines marked this pull request as ready for review Jan 15, 2020
Copy link
Member

thebaer left a comment

Just one note / question about the schema design after reading through. Otherwise everything looks great!

Next I'll run this and possibly tweak the design.

migrations/v6.go Outdated Show resolved Hide resolved
@thebaer thebaer added this to the 0.12 milestone Jan 16, 2020
@thebaer

This comment has been minimized.

Copy link
Member

thebaer commented Feb 10, 2020

I went ahead and updated the schema and logic around that column so it's a NULL now. Only issue I ran into was this:

  1. Already have a WF Account A connected to a Slack account
  2. I log out of it
  3. I log into another WF Account B, then choose to attach Slack
  4. I go through the normal flow to attach the Slack account to Account B, but it brings me back to WF logged in as Account A.

Ideally, we could show an error in this case or some other kind indication to the user, as this is a little confusing.

thebaer and others added 6 commits Mar 19, 2020
… a user that already has the slack account attached. Added GitLab to settings page as oauth option.
This includes the chosen GitLab display name in the button text.
- Break up linked / to-link sections
- Add logos for all services
- Lay out buttons horizontally
- Tweak the copy

Ref T713
@thebaer

This comment has been minimized.

Copy link
Member

thebaer commented Mar 24, 2020

Looks great! Verified the issue is fixed.

I tweaked the design and wording on the Account Settings page (see below), and made a few other changes. Now I'm just verifying different configurations and edge cases. If that looks good, I'll merge.

No accounts linked

Screenshot from 2020-03-24 10-33-13

Slack linked

Screenshot from 2020-03-24 10-30-40

@thebaer

This comment has been minimized.

Copy link
Member

thebaer commented Mar 24, 2020

Merging now! Thanks @ngerakines!

@thebaer thebaer merged commit 0acc630 into develop Mar 24, 2020
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@thebaer thebaer deleted the T713-oauth-account-management branch Mar 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.