Skip to content

Remove unnecessary (?) SignInActivity #443

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

Merged
merged 36 commits into from
Dec 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
d515d5f
Initial refactor
SUPERCILEX Nov 24, 2016
cf342ce
Fix broken tests
SUPERCILEX Nov 24, 2016
c8a0d26
Cleanup
SUPERCILEX Nov 24, 2016
b7d9054
Merge remote-tracking branch 'firebase/master' into register-email-re…
SUPERCILEX Nov 28, 2016
0c5701a
Fix merge mistakes
SUPERCILEX Nov 28, 2016
a793f8c
Merge remote-tracking branch 'firebase/master' into register-email-re…
SUPERCILEX Nov 30, 2016
d7ebebc
Remove unused resources
SUPERCILEX Nov 30, 2016
78e83ca
Fix smart lock memory leak
SUPERCILEX Dec 1, 2016
f304277
Fix another memory leak
SUPERCILEX Dec 1, 2016
5aa595c
Cleanup
SUPERCILEX Dec 1, 2016
5080f82
Merge remote-tracking branch 'firebase/master' into register-email-re…
SUPERCILEX Dec 2, 2016
98ed856
Address review comments and add CCT
SUPERCILEX Dec 2, 2016
44408f1
Merge remote-tracking branch 'firebase/master' into register-email-re…
SUPERCILEX Dec 2, 2016
8d162f5
Tie WelcomeBackPasswordPrompt.java failure listener to activity
SUPERCILEX Dec 2, 2016
3568e84
Cleanup
SUPERCILEX Dec 3, 2016
7022cb6
Remove SignInActivity.java
SUPERCILEX Dec 3, 2016
f4a1790
Update gradle, update dependencies, fix Facebook theme, fix Facebook CCT
SUPERCILEX Dec 6, 2016
d901d4c
Remove unnecessary setApplicationId
SUPERCILEX Dec 6, 2016
93e78ef
Remove unnecessary theme
SUPERCILEX Dec 6, 2016
fe4739c
Fix activity leak
SUPERCILEX Dec 6, 2016
f84f06b
Cleanup
SUPERCILEX Dec 6, 2016
38da2d2
Cleanup
SUPERCILEX Dec 6, 2016
f0707fa
Merge remote-tracking branch 'firebase/master' into fb-mem-leak
SUPERCILEX Dec 6, 2016
37dd743
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into remove…
SUPERCILEX Dec 6, 2016
36a9321
Fix merge mistakes
SUPERCILEX Dec 6, 2016
9397787
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into remove…
SUPERCILEX Dec 6, 2016
8f6ac6c
Remove unused resources
SUPERCILEX Dec 6, 2016
203a831
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into fb-mem…
SUPERCILEX Dec 6, 2016
7b3d0a7
Remove Twitter request email until #436 discussion is resolved
SUPERCILEX Dec 7, 2016
971c8e6
Cleanup
SUPERCILEX Dec 7, 2016
171ec53
Remove unnecessary sign out
SUPERCILEX Dec 7, 2016
e417da4
Show keyboard on activity start
SUPERCILEX Dec 7, 2016
c04dc2f
Cleanup
SUPERCILEX Dec 7, 2016
b94c0fe
Merge remote-tracking branch 'firebase/version-1.1.0-dev' into remove…
SUPERCILEX Dec 7, 2016
ded6bcf
Merge branch 'fb-mem-leak' into remove-unnecessary-sign-in-activity
SUPERCILEX Dec 7, 2016
52695a7
Fix broken error handling
SUPERCILEX Dec 7, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions auth/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,6 @@
android:exported="false"
android:theme="@style/FirebaseUI"/>

<activity
android:name=".ui.email.SignInActivity"
android:label="@string/title_sign_in_activity"
android:exported="false"
android:theme="@style/FirebaseUI"/>

<activity
android:name=".ui.accountlink.WelcomeBackIdpPrompt"
android:label="@string/title_welcome_back_idp_prompt"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,11 @@ public void saveCredentialsOrFinish(
IdpResponse response) {
saveCredentialsOrFinish(saveSmartLock, mActivity, firebaseUser, password, response);
}

public void saveCredentialsOrFinish(
@Nullable SaveSmartLock saveSmartLock,
FirebaseUser firebaseUser,
IdpResponse response) {
saveCredentialsOrFinish(saveSmartLock, mActivity, firebaseUser, null, response);
}
}
4 changes: 0 additions & 4 deletions auth/src/main/java/com/firebase/ui/auth/ui/BaseHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ public boolean isProgressDialogShowing() {
return mProgressDialog != null && mProgressDialog.isShowing();
}

public Context getApplicationContext() {
return mContext.getApplicationContext();
}

public String getAppName() {
return mFlowParams.appName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import android.text.TextUtils;
import android.text.style.StyleSpan;
import android.view.View;
import android.view.WindowManager;
import android.widget.EditText;
import android.widget.TextView;

Expand All @@ -44,6 +45,7 @@
import com.google.android.gms.tasks.OnSuccessListener;
import com.google.firebase.auth.AuthCredential;
import com.google.firebase.auth.AuthResult;
import com.google.firebase.auth.EmailAuthProvider;
import com.google.firebase.auth.FirebaseAuth;

/**
Expand All @@ -65,13 +67,17 @@ public class WelcomeBackPasswordPrompt extends AppCompatBase implements View.OnC
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.welcome_back_password_prompt_layout);
mSaveSmartLock = mActivityHelper.getSaveSmartLockInstance();
mPasswordLayout = (TextInputLayout) findViewById(R.id.password_layout);
mPasswordField = (EditText) findViewById(R.id.password);

// Show keyboard
getWindow().setSoftInputMode(WindowManager.LayoutParams.SOFT_INPUT_STATE_VISIBLE);

mSaveSmartLock = mActivityHelper.getSaveSmartLockInstance();
mIdpResponse = IdpResponse.fromResultIntent(getIntent());
mEmail = mIdpResponse.getEmail();

mPasswordLayout = (TextInputLayout) findViewById(R.id.password_layout);
mPasswordField = (EditText) findViewById(R.id.password);

// Create welcome back text with email bolded
String bodyText = getResources().getString(R.string.welcome_back_password_prompt_body);
bodyText = String.format(bodyText, mEmail);
Expand All @@ -94,7 +100,6 @@ protected void onCreate(Bundle savedInstanceState) {
public void onClick(View view) {
final int id = view.getId();
if (id == R.id.button_done) {
mActivityHelper.showLoadingDialog(R.string.progress_dialog_signing_in);
next(mEmail, mPasswordField.getText().toString());
} else if (id == R.id.trouble_signing_in) {
startActivity(RecoverPasswordActivity.createIntent(
Expand All @@ -108,11 +113,12 @@ public void onClick(View view) {
private void next(final String email, final String password) {
// Check for null or empty password
if (TextUtils.isEmpty(password)) {
mPasswordField.setError(getString(R.string.required_field));
mPasswordLayout.setError(getString(R.string.required_field));
return;
} else {
mPasswordField.setError(null);
mPasswordLayout.setError(null);
}
mActivityHelper.showLoadingDialog(R.string.progress_dialog_signing_in);

final FirebaseAuth firebaseAuth = mActivityHelper.getFirebaseAuth();
// Sign in with known email and the password provided
Expand All @@ -122,29 +128,32 @@ private void next(final String email, final String password) {
.addOnSuccessListener(new OnSuccessListener<AuthResult>() {
@Override
public void onSuccess(AuthResult authResult) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@samtstern Do you know why we were doing this? It seems unnecessary and just using linkWithCredential works fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it was added in this commit

The only reason I can think to do this is that known issue about FirebaseUser changes not taking effect until signout/signin, but I believe that's only for explicit profile information change requests.

One thing I am seeing now is that the linkWithCredential call is done as fire-and-forget, which will almost certainly create some strange race conditions.

Seems like the original author should have been chaining the link-->signout-->sign in operations together.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Humm, so should I revert and use a continuation or leave it at linkWithCredential?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we leave it with your new implementation. I can't see a reason for the sign out thing.

// Get the social AuthCredential from the IDPResponse object, link
// it to the email/password account.
AuthCredential authCredential =
AuthCredentialHelper.getAuthCredential(mIdpResponse);
authResult.getUser().linkWithCredential(authCredential);
firebaseAuth.signOut();

// Sign in with the credential
firebaseAuth.signInWithCredential(authCredential)
.addOnFailureListener(
new TaskFailureLogger(TAG,
"Error signing in with credential"))
.addOnSuccessListener(
new OnSuccessListener<AuthResult>() {
@Override
public void onSuccess(AuthResult authResult) {
mActivityHelper.saveCredentialsOrFinish(
mSaveSmartLock,
authResult.getUser(),
password,
mIdpResponse);
}
});

// If authCredential is null, the user only has an email account.
// Otherwise, the user has an email account that we need to link to an idp.
if (authCredential == null) {
mActivityHelper.saveCredentialsOrFinish(
mSaveSmartLock,
authResult.getUser(),
password,
new IdpResponse(EmailAuthProvider.PROVIDER_ID, email));
} else {
authResult.getUser()
.linkWithCredential(authCredential)
.addOnFailureListener(new TaskFailureLogger(
TAG, "Error signing in with credential"))
.addOnSuccessListener(new OnSuccessListener<AuthResult>() {
@Override
public void onSuccess(AuthResult authResult) {
mActivityHelper.saveCredentialsOrFinish(
mSaveSmartLock,
authResult.getUser(),
mIdpResponse);
}
});
}
}
})
.addOnFailureListener(this, new OnFailureListener() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.firebase.ui.auth.ui.email.fieldvalidators.EmailFieldValidator;
import com.google.android.gms.tasks.OnFailureListener;
import com.google.android.gms.tasks.OnSuccessListener;
import com.google.firebase.auth.FirebaseAuth;
import com.google.firebase.auth.FirebaseAuthInvalidUserException;

/**
Expand All @@ -47,12 +46,12 @@ public class RecoverPasswordActivity extends AppCompatBase implements View.OnCli
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.forgot_password_layout);
String email = getIntent().getStringExtra(ExtraConstants.EXTRA_EMAIL);

mEmailFieldValidator = new EmailFieldValidator(
(TextInputLayout) findViewById(R.id.email_layout));

mEmailFieldValidator =
new EmailFieldValidator((TextInputLayout) findViewById(R.id.email_layout));
mEmailEditText = (EditText) findViewById(R.id.email);

String email = getIntent().getStringExtra(ExtraConstants.EXTRA_EMAIL);
if (email != null) {
mEmailEditText.setText(email);
}
Expand All @@ -61,8 +60,8 @@ protected void onCreate(Bundle savedInstanceState) {
}

private void next(final String email) {
FirebaseAuth firebaseAuth = mActivityHelper.getFirebaseAuth();
firebaseAuth.sendPasswordResetEmail(email)
mActivityHelper.getFirebaseAuth()
.sendPasswordResetEmail(email)
.addOnFailureListener(
new TaskFailureLogger(TAG, "Error sending password reset email"))
.addOnSuccessListener(new OnSuccessListener<Void>() {
Expand All @@ -89,11 +88,10 @@ public void onFailure(@NonNull Exception e) {
@Override
public void onClick(View view) {
if (view.getId() == R.id.button_done) {
if (!mEmailFieldValidator.validate(mEmailEditText.getText())) {
return;
if (mEmailFieldValidator.validate(mEmailEditText.getText())) {
mActivityHelper.showLoadingDialog(R.string.progress_dialog_sending);
next(mEmailEditText.getText().toString());
}
mActivityHelper.showLoadingDialog(R.string.progress_dialog_sending);
next(mEmailEditText.getText().toString());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.firebase.ui.auth.ui.FlowParameters;
import com.firebase.ui.auth.ui.TaskFailureLogger;
import com.firebase.ui.auth.ui.accountlink.WelcomeBackIdpPrompt;
import com.firebase.ui.auth.ui.accountlink.WelcomeBackPasswordPrompt;
import com.firebase.ui.auth.ui.email.fieldvalidators.EmailFieldValidator;
import com.firebase.ui.auth.ui.email.fieldvalidators.PasswordFieldValidator;
import com.firebase.ui.auth.ui.email.fieldvalidators.RequiredFieldValidator;
Expand Down Expand Up @@ -249,12 +250,13 @@ public void onSuccess(ProviderQueryResult result) {

String provider = providers.get(0);
if (provider.equalsIgnoreCase(EmailAuthProvider.PROVIDER_ID)) {
Intent signInIntent = SignInActivity.createIntent(
RegisterEmailActivity.this,
mActivityHelper.getFlowParams(),
email);
mActivityHelper.startActivityForResult(signInIntent,
RC_SIGN_IN);
mActivityHelper.startActivityForResult(
WelcomeBackPasswordPrompt.createIntent(
RegisterEmailActivity.this,
mActivityHelper.getFlowParams(),
new IdpResponse(EmailAuthProvider.PROVIDER_ID,
email)),
RC_SIGN_IN);
} else {
Intent intent = WelcomeBackIdpPrompt.createIntent(
RegisterEmailActivity.this,
Expand Down Expand Up @@ -313,8 +315,7 @@ public void onComplete(@NonNull Task<Void> task) {
mSaveSmartLock,
user,
password,
new IdpResponse(EmailAuthProvider.PROVIDER_ID,
email));
new IdpResponse(EmailAuthProvider.PROVIDER_ID, email));
}
});
}
Expand Down
131 changes: 0 additions & 131 deletions auth/src/main/java/com/firebase/ui/auth/ui/email/SignInActivity.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import com.firebase.ui.auth.util.signincontainer.SaveSmartLock;
import com.google.firebase.auth.AuthCredential;
import com.google.firebase.auth.FacebookAuthProvider;
import com.google.firebase.auth.FirebaseAuth;
import com.google.firebase.auth.GoogleAuthProvider;
import com.google.firebase.auth.TwitterAuthProvider;

Expand Down Expand Up @@ -159,9 +158,7 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) {
@Override
public void onSuccess(final IdpResponse response) {
AuthCredential credential = AuthCredentialHelper.getAuthCredential(response);
final FirebaseAuth firebaseAuth = mActivityHelper.getFirebaseAuth();

firebaseAuth
mActivityHelper.getFirebaseAuth()
.signInWithCredential(credential)
.addOnFailureListener(
new TaskFailureLogger(TAG, "Firebase sign in with credential unsuccessful"))
Expand Down
Loading