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

feat: Parse screenName in ProvidedUserInfo #611

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

sixstone-qq
Copy link

Discussion

It tries to fix #610 .

Testing

Adapt existing mocked data.

API Changes

Adding a new public field to UserInfo only populated under ProvidedUserInfo that is available in public API

Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

Thank you @sixstone-qq ! LGTM!

Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

Reverting my review till we get approval from @pragatimodi

@sixstone-qq sixstone-qq force-pushed the sixstone-qq/add-screen-name-get-user branch 2 times, most recently from 5da2b98 to 8a82f96 Compare April 11, 2024 10:07
@sixstone-qq sixstone-qq force-pushed the sixstone-qq/add-screen-name-get-user branch from 8a82f96 to 502f91e Compare July 22, 2024 10:34
This is available when having multiple providers where it provides
a screen name such as Twitter or GitHub.

See
https://cloud.google.com/identity-platform/docs/reference/rest/v1/ProviderUserInfo
for details.

It's available at endpoints like accounts.lookup or
projects.queryAccounts

Signed-off-by: Enrique J. Hernández <[email protected]>
@sixstone-qq sixstone-qq force-pushed the sixstone-qq/add-screen-name-get-user branch from 502f91e to b77aea8 Compare July 22, 2024 10:36
@sixstone-qq
Copy link
Author

@pragatimodi it would be great if you can take a look on it to merge it upstream? Thanks a lot for your help here.

@pragatimodi
Copy link
Contributor

@lahirumaramba would this need an API council approval? or do we have an approved proposal for this already?

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.

3 participants