-
Notifications
You must be signed in to change notification settings - Fork 4k
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, iOS): include missing email and credential in account-exists-with-different-credential error #17180
base: main
Are you sure you want to change the base?
Conversation
…s-with-different-credential error
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.
Could you add a test?
@Lyokone, I'm not sure how to write a test for this since the issue isn't reproducible against a Firebase Emulator instance. It only occurs when testing with a live project. Could you share your thoughts on how the test should be approached? |
completion([PigeonParser getPigeonUserCredentialFromAuthResult:authResult | ||
authorizationCode:nil], | ||
nil); |
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.
I see here that you have changed the order in completion, is this intentional?
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.
My mistake it's because the whole indentation changed 😓
[auth signInWithCredential:credentials | ||
completion:^(FIRAuthDataResult *authResult, NSError *error) { | ||
if (error != nil) { |
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.
The whole indentation changed because of this which makes reviewing harder, why formatting changed?
Description
Fixes missing email and credential properties in account-exists-with-different-credential error on iOS
Related Issues
Fixes #17160
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
).This will ensure a smooth and quick review process. Updating the
pubspec.yaml
and changelogs is not required.///
).melos run analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?