-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
fix(auth, android): return credential for signin if phone auth has link collision #7793
Conversation
You can now upgrade anonymous users using phone auth on Android. Credit to @Shaninnik.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for posting this - looks promising with some comments as indicated
I made them all in the form of suggestions, I'll apply them + reformat + push to the branch then do final review
I think this can go in though
...ges/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java
Outdated
Show resolved
Hide resolved
...ges/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java
Outdated
Show resolved
Hide resolved
...ges/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java
Outdated
Show resolved
Hide resolved
...ges/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java
Outdated
Show resolved
Hide resolved
...ges/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java
Outdated
Show resolved
Hide resolved
...ges/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java
Outdated
Show resolved
Hide resolved
...ges/auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java
Outdated
Show resolved
Hide resolved
cleaning things up after merging in suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable, if it passes CI I can merge this in
Ah, I do love a thorough test suite - it picked something up on this one so it will need some further examination before merge:
|
This is fantastic, thank you so much @mikehardy for the fixes and the merge! |
Is there anywhere I can look to keep track for your release schedule so I know when to upgrade my dependencies? Thanks! |
Unfortunately our release process doesn't formally make releases in GitHub so you can't really subscribe there, apologies. I usually release things about as soon as I merge them, this one just was held up a few days on CI issues, it's out now though |
No problem, yesterday I found you had a CHANGELOG file and noticed the new releases, so I upgraded our dependencies. Everythings working smoothly ✌️, thanks! |
Description
You can now upgrade anonymous users using phone auth on Android. Credit to @Shaninnik.
Fixes: