Adds AuthDataResult to signInWithEmailPassword Method #484

protocol86 · 2017-11-21T20:33:16Z

No description provided.

ch40w31 · 2017-11-21T20:50:40Z

Firebase/Auth/Source/FIRAuth.m

+                                                  isNewUser:NO];
+       FIRAuthDataResult *result = [[FIRAuthDataResult alloc] initWithUser:user
+                                                        additionalUserInfo:additionalUserInfo];
+       decoratedCallback(result, nil);


Last 3 lines are misaligned.

Done, thanks.

ch40w31 · 2017-11-21T20:51:27Z

Firebase/Auth/Source/FIRAuth.m

+      completion(nil, error);
+      return;
+    }
+    completion(response, error);


Pass completion to FIRAuthBackend verifiyPassword:callback: directly.

done, thanks

XiangtianDai · 2017-11-21T22:22:09Z

Firebase/Auth/Source/FIRAuth.m

@@ -512,46 +512,59 @@ - (void)signInWithEmail:(NSString *)email
               password:(NSString *)password
             completion:(FIRAuthResultCallback)completion {


Any reason not to call signInAndRetrieveDataWithEmail:password:completion:, just like signInWithCredential:completion: delegates to signInAndRetrieveDataWithCredential:completion:?

Done thank!

XiangtianDai · 2017-11-21T22:23:48Z

Firebase/Auth/Source/FIRAuth.m

+- (void)signInAndRetrieveDataWithEmail:(NSString *)email
+                              password:(NSString *)password
+                            completion:(FIRAuthDataResultCallback)completion {
+  dispatch_async(FIRAuthGlobalWorkQueue(), ^{


Same question here: can we call signInAndRetriveDataWithCredential:completion: to avoid repeated logic?

yramanchuk · 2017-11-21T22:24:18Z

Firebase/Auth/Source/FIRAuth.m

+    [self internalSignInWithEmail:email
+                         password:password
+                       completion:^(FIRVerifyPasswordResponse *_Nullable response,
+                                    NSError * _Nullable error) {


remove space: '*_Nullable'

yramanchuk · 2017-11-21T23:01:17Z

Firebase/Auth/Source/FIRAuth.m

@@ -1151,6 +1177,30 @@ - (void)phoneNumberSignInWithRequest:(FIRVerifyPhoneNumberRequest *)request
 }
 #endif

+/** @fn internalSignInWithEmail:password:callback:


replace with 'completion'

XiangtianDai · 2017-11-22T01:13:51Z

Firebase/Auth/Source/FIRAuth.m

+                                        password:password
+                                      completion:^(FIRAuthDataResult *_Nullable authResult,
+                                                   NSError *_Nullable error) {
+      if (error) {


It seems that checking for error is unnecessary. Simply calling decoratedCallback(authResult.user, error); should work for both cases.

XiangtianDai · 2017-11-22T01:14:29Z

Firebase/Auth/Source/FIRAuth.m

+      [self signInFlowAuthDataResultCallbackByDecoratingCallback:completion];
+    [self internalSignInAndRetrieveDataWithEmail:email
+                                        password:password
+                                      completion:^(FIRAuthDataResult *_Nullable authResult,


Just to pass in decoratedCallback directly?

XiangtianDai · 2017-11-22T01:15:48Z

Firebase/Auth/Source/FIRAuth.m

@@ -587,7 +630,7 @@ - (void)internalSignInWithCredential:(FIRAuthCredential *)credential
 - (void)internalSignInAndRetrieveDataWithCredential:(FIRAuthCredential *)credential
                                 isReauthentication:(BOOL)isReauthentication
                                           callback:(nullable FIRAuthDataResultCallback)callback {
-  if ([credential isKindOfClass:[FIREmailPasswordAuthCredential class]]) {
+    if ([credential isKindOfClass:[FIREmailPasswordAuthCredential class]]) {


Unintended extra indent?

Fixed, thank you.

* Adds AuthDataResult to signInWithEmail:Password * Addresses comments * Addresses comments * addresses more comments * Fixes broken tests

Adds AuthDataResult to signInWithEmail:Password

bb3a686

protocol86 requested review from paulb777, ch40w31 and XiangtianDai November 21, 2017 20:33

googlebot added the cla: yes label Nov 21, 2017

ch40w31 reviewed Nov 21, 2017

View reviewed changes

protocol86 requested review from ch40w31 and yramanchuk November 21, 2017 21:50

Addresses comments

77a9ff9

XiangtianDai reviewed Nov 21, 2017

View reviewed changes

yramanchuk reviewed Nov 21, 2017

View reviewed changes

Addresses comments

0795d2e

XiangtianDai reviewed Nov 22, 2017

View reviewed changes

XiangtianDai approved these changes Nov 22, 2017

View reviewed changes

protocol86 added 2 commits November 21, 2017 18:13

addresses more comments

2a5e786

Fixes broken tests

48d9d7b

protocol86 merged commit bc2d784 into master Nov 22, 2017

protocol86 added the api: auth label Nov 22, 2017

paulb777 deleted the Add_AuthResult_to_SignInWithEmailPassword branch August 8, 2018 17:06

paulb777 added a commit that referenced this pull request Aug 19, 2019

Stage RemoteConfig for open sourcing (#484)

55f6719

firebase locked and limited conversation to collaborators Nov 11, 2019

		@@ -512,46 +512,59 @@ - (void)signInWithEmail:(NSString *)email
		password:(NSString *)password
		completion:(FIRAuthResultCallback)completion {

Adds AuthDataResult to signInWithEmailPassword Method #484

Adds AuthDataResult to signInWithEmailPassword Method #484

Uh oh!

Conversation

protocol86 commented Nov 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!