Implemented FirebaseAuth.GetUser based on the Java implementation #49
Conversation
…serRecord object for the specified user ID.
|
Thanks @renatoc8. I think we'll need the exact API structure currently available in Java. Specifically, we'd want to have an Other high-level comments:
|
|
Thanks for reviewing my pull requests. I will get those changes implemented shortly. |
…seAdmin.Auth. -Renamed GetUser() to GetUserAsync(), and added an overload that accepts a cancellationToken. -Added IUserInfo and implemented it in UserRecord and ProviderUserInfo.
…cord and ProviderUserInfo to public.
|
Ok, those changes have been implemented. Anything else you can think of @hiranya911 ? |
hiranya911
left a comment
hiranya911
left a comment
There was a problem hiding this comment.
Thanks @renatoc8. Looks mostly good. I've pointed out a few minor things to change. Also note that Travis CI is detecting a test failure, that should be fixed.
|
@hiranya911 I believe I've marked all of the conversations as resolved, but I'm not sure how to mark the requested changes as complete. Is there anything else I have to do? |
renatoc8
left a comment
renatoc8
left a comment
There was a problem hiding this comment.
Trying to mark the requested changes as completed. My apologies if this is the wrong place.
hiranya911
left a comment
hiranya911
left a comment
There was a problem hiding this comment.
Thanks @renatoc8. I think we are almost there. Just pointed out what I think is the last set of comments I have on this implementation.
Since this is a new API for this SDK, we also need to get it approved internally. I'm working on that right now.
FirebaseAdmin/FirebaseAdmin.Tests/Auth/FirebaseUserManagerTest.cs
Outdated
Show resolved
Hide resolved
FirebaseAdmin/FirebaseAdmin/Auth/Internal/GetAccountInfoResponse.cs
Outdated
Show resolved
Hide resolved
FirebaseAdmin/FirebaseAdmin/Auth/Internal/GetAccountInfoResponse.cs
Outdated
Show resolved
Hide resolved
…nse. Converted ProviderUserInfo to be an internal class, with auto properties. Fixed the FirebaseUserManagerTest test so that it would fail when instantiating a UserRecord class with a uid that's longer than 128 characters.
hiranya911
left a comment
hiranya911
left a comment
There was a problem hiding this comment.
LGTM 👍
Hang tight until our internal API review process is complete. I'll merge it then.
|
@renatoc8 can you also add a line to the |
…n the FirebaseUserManager class.
|
Added the entry to CHANGELOG. |
|
This API is still under review. But I'll go ahead and merge the PR. If any changes are suggested by the review team, we can implement them separately. |
2 participants
This pull request implements the GetUser() function in FirebaseAuth, closely following the implementation found in the firebase-admin-java project.