Skip to content

feat: support multiple accounts #1201

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

Merged
merged 13 commits into from
Jun 10, 2024
Merged

feat: support multiple accounts #1201

merged 13 commits into from
Jun 10, 2024

Conversation

setchy
Copy link
Member

@setchy setchy commented Jun 8, 2024

This PR adds support for multiple GitHub Cloud or GitHub Enterprise Service accounts. 🎉

Majority of the code changes were to:

  • pass notification object instead of its parts
  • use account.token instead of deriving it from hostname (now that hostname isn't unique)
  • use unique getAccountUUID for notification interactions (mark as read, raise native notifications, etc) so that state is correctly managed per account and not per hostname

Closes #365

@setchy setchy added the enhancement New feature or enhancement to existing functionality label Jun 8, 2024
@setchy setchy added this to the Release 5.8.0 milestone Jun 8, 2024
@setchy setchy marked this pull request as ready for review June 10, 2024 11:41
@setchy
Copy link
Member Author

setchy commented Jun 10, 2024

I think this is ready for review and ideally further local testing.

@setchy
Copy link
Member Author

setchy commented Jun 10, 2024

looking into the test coverage drop 👀

@afonsojramos
Copy link
Member

Locally tested and everything looks awesome! One edge case that we can improve is:

image

When using the same account twice (same PAT even), the dropdown doesn't work, although it does show.

image

@setchy
Copy link
Member Author

setchy commented Jun 10, 2024

the dropdown doesn't work, although it does show.

when you scroll, do you see the duplicate account header and notifications?
UPDATE: tested myself - it behaves as I expected (ie: showing duplicate the account details)

were you expecting it to be interactive (spoiler: i was too, but atm, it isn't)

@setchy
Copy link
Member Author

setchy commented Jun 10, 2024

One edge case that we can improve

ha - you found the edge case straight away... yep, haven't implemented any duplicate checking - figured that can just be handled by end-users :)

@setchy setchy merged commit fedbfbe into main Jun 10, 2024
@setchy setchy deleted the refactor/accounts-auth-token branch June 10, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login to multiple Github accounts simultaneously
2 participants