Respond to SUPERCILEX review feedback

samtstern · samtstern · commit f55c73ba6f3a · 2017-06-08T09:17:43.000-07:00
Change-Id: I6eac4c2e92c13b1e5ad9a2953441566ad2d7c29e
diff --git a/auth/src/main/java/com/firebase/ui/auth/AuthUI.java b/auth/src/main/java/com/firebase/ui/auth/AuthUI.java
@@ -453,7 +453,7 @@ public T setAvailableProviders(@NonNull List<IdpConfig> idpConfigs) {
 
                 if (config.getProviderId().equals(FACEBOOK_PROVIDER)) {
                     try {
-                        Class c = com.facebook.FacebookCallback.class;
+                        Class c = com.facebook.FacebookSdk.class;
                     } catch (NoClassDefFoundError e) {
                         throw new RuntimeException("Facebook provider cannot be configured " +
                                "without dependency. Did you forget to add " +
diff --git a/auth/src/main/java/com/firebase/ui/auth/IdpResponse.java b/auth/src/main/java/com/firebase/ui/auth/IdpResponse.java
@@ -100,6 +100,14 @@ public String getEmail() {
         return mEmail;
     }
 
+    /**
+     * Get the phone number used to sign in.
+     */
+    @Nullable
+    public String getPhoneNumber() {
+        return mPhoneNumber;
+    }
+
     /**
      * Get the token received as a result of logging in with the specified IDP
      */
diff --git a/auth/src/main/java/com/firebase/ui/auth/provider/EmailProvider.java b/auth/src/main/java/com/firebase/ui/auth/provider/EmailProvider.java
@@ -5,6 +5,7 @@
 import android.content.Intent;
 import android.support.annotation.LayoutRes;
 
+import com.firebase.ui.auth.AuthUI;
 import com.firebase.ui.auth.R;
 import com.firebase.ui.auth.ResultCodes;
 import com.firebase.ui.auth.ui.BaseHelper;
@@ -28,6 +29,7 @@ public String getName(Context context) {
     }
 
     @Override
+    @AuthUI.SupportedProvider
     public String getProviderId() {
         return EmailAuthProvider.PROVIDER_ID;
     }
diff --git a/auth/src/main/java/com/firebase/ui/auth/provider/PhoneProvider.java b/auth/src/main/java/com/firebase/ui/auth/provider/PhoneProvider.java
@@ -3,7 +3,9 @@
 import android.app.Activity;
 import android.content.Context;
 import android.content.Intent;
+import android.support.annotation.LayoutRes;
 
+import com.firebase.ui.auth.AuthUI;
 import com.firebase.ui.auth.R;
 import com.firebase.ui.auth.ResultCodes;
 import com.firebase.ui.auth.ui.BaseHelper;
@@ -28,11 +30,13 @@ public String getName(Context context) {
     }
 
     @Override
+    @AuthUI.SupportedProvider
     public String getProviderId() {
         return PhoneAuthProvider.PROVIDER_ID;
     }
 
     @Override
+    @LayoutRes
     public int getButtonLayout() {
         return R.layout.provider_button_phone;
     }
diff --git a/auth/src/main/java/com/firebase/ui/auth/provider/TwitterProvider.java b/auth/src/main/java/com/firebase/ui/auth/provider/TwitterProvider.java
@@ -55,11 +55,12 @@ public static void initialize(Context context) {
 
     public static void signout(Context context) {
         try {
-            signOut();
+            Twitter.getInstance();
         } catch (IllegalStateException e) {
             initialize(context);
-            signOut();
         }
+
+        signOut();
     }
 
     @Override
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/FragmentHelper.java b/auth/src/main/java/com/firebase/ui/auth/ui/FragmentHelper.java
@@ -13,7 +13,7 @@ public class FragmentHelper extends BaseHelper {
     private Fragment mFragment;
 
     public FragmentHelper(Fragment fragment) {
-        super(fragment.getContext(),
+        super(fragment.getContext().getApplicationContext(),
               (FlowParameters) fragment.getArguments().getParcelable(ExtraConstants.EXTRA_FLOW_PARAMS));
         mFragment = fragment;
     }
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/TermsTextView.java b/auth/src/main/java/com/firebase/ui/auth/ui/TermsTextView.java
@@ -16,6 +16,7 @@
 
 import android.content.Context;
 import android.support.annotation.StringRes;
+import android.support.v7.widget.AppCompatTextView;
 import android.util.AttributeSet;
 
 import com.firebase.ui.auth.ui.email.PreambleHandler;
@@ -25,7 +26,7 @@
  * The view helps display TOS linking to the provided custom URI.
  * It handles the styling of the link and opens the uri in a CustomTabs on click.
  */
-public class TermsTextView extends android.support.v7.widget.AppCompatTextView {
+public class TermsTextView extends AppCompatTextView {
     public TermsTextView(Context context) {
         super(context);
     }
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/phone/CountryListSpinner.java b/auth/src/main/java/com/firebase/ui/auth/ui/phone/CountryListSpinner.java
@@ -21,6 +21,7 @@
 import android.content.Context;
 import android.content.DialogInterface;
 import android.os.AsyncTask;
+import android.support.annotation.VisibleForTesting;
 import android.support.v7.widget.AppCompatEditText;
 import android.text.TextUtils;
 import android.util.AttributeSet;
@@ -53,9 +54,7 @@ public CountryListSpinner(Context context, AttributeSet attrs, int defStyle) {
         init();
     }
 
-    /**
-     * Only for Testing
-     */
+    @VisibleForTesting
     void setDialogPopup(DialogPopup dialog) {
         this.dialogPopup = dialog;
     }
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/phone/PhoneVerificationActivity.java b/auth/src/main/java/com/firebase/ui/auth/ui/phone/PhoneVerificationActivity.java
@@ -100,7 +100,7 @@ protected void onCreate(final Bundle savedInstance) {
         }
 
         String phone = getIntent().getExtras().getString(ExtraConstants.EXTRA_PHONE);
-        VerifyPhoneNumberFragment fragment = VerifyPhoneNumberFragment.getInstance
+        VerifyPhoneNumberFragment fragment = VerifyPhoneNumberFragment.newInstance
                 (mActivityHelper.getFlowParams(), phone);
         getSupportFragmentManager().beginTransaction().replace(R.id.fragment_verify_phone,
                 fragment, VerifyPhoneNumberFragment.TAG).disallowAddToBackStack().commit();
@@ -274,7 +274,7 @@ protected AlertDialog getAlertDialog() {
     private void showSubmitCodeFragment() {
         // idempotent function
         if (getSubmitConfirmationCodeFragment() == null) {
-            SubmitConfirmationCodeFragment f = SubmitConfirmationCodeFragment.getInstance
+            SubmitConfirmationCodeFragment f = SubmitConfirmationCodeFragment.newInstance
                     (mActivityHelper.getFlowParams(), mPhoneNumber);
             FragmentTransaction t = getSupportFragmentManager().beginTransaction().replace(R.id
                     .fragment_verify_phone, f, SubmitConfirmationCodeFragment.TAG).addToBackStack
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/phone/SpacedEditText.java b/auth/src/main/java/com/firebase/ui/auth/ui/phone/SpacedEditText.java
@@ -20,6 +20,7 @@
 
 import android.content.Context;
 import android.content.res.TypedArray;
+import android.support.v7.widget.AppCompatEditText;
 import android.text.Editable;
 import android.text.Spannable;
 import android.text.SpannableStringBuilder;
@@ -33,7 +34,7 @@
  * spaces using spannables.
  * This is required since Android's letter spacing is not available until API 21.
  */
-public final class SpacedEditText extends android.support.v7.widget.AppCompatEditText {
+public final class SpacedEditText extends AppCompatEditText {
     private float proportion;
     private SpannableStringBuilder originalText;
 
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/phone/SubmitConfirmationCodeFragment.java b/auth/src/main/java/com/firebase/ui/auth/ui/phone/SubmitConfirmationCodeFragment.java
@@ -46,19 +46,17 @@ public class SubmitConfirmationCodeFragment extends FragmentBase {
     private static final long RESEND_WAIT_MILLIS = 15000;
     private static final String EXTRA_MILLIS_UNTIL_FINISHED = "EXTRA_MILLIS_UNTIL_FINISHED";
 
-    TextView mEditPhoneTextView;
-    TextView mResendCodeTextView;
-    TextView mCountDownTextView;
-    SpacedEditText mConfirmationCodeEditText;
-    Button mSubmitConfirmationButton;
-    CustomCountDownTimer mCountdownTimer;
-    PhoneVerificationActivity mVerifier;
-    TermsTextView mAgreementText;
-    long mMillisUntilFinished;
-
-
-
-    public static SubmitConfirmationCodeFragment getInstance(FlowParameters flowParameters,
+    private TextView mEditPhoneTextView;
+    private TextView mResendCodeTextView;
+    private TextView mCountDownTextView;
+    private SpacedEditText mConfirmationCodeEditText;
+    private Button mSubmitConfirmationButton;
+    private CustomCountDownTimer mCountdownTimer;
+    private PhoneVerificationActivity mVerifier;
+    private TermsTextView mAgreementText;
+    private long mMillisUntilFinished;
+
+    public static SubmitConfirmationCodeFragment newInstance(FlowParameters flowParameters,
                                                              String phoneNumber) {
         SubmitConfirmationCodeFragment fragment = new SubmitConfirmationCodeFragment();
 
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/phone/VerifyPhoneNumberFragment.java b/auth/src/main/java/com/firebase/ui/auth/ui/phone/VerifyPhoneNumberFragment.java
@@ -52,17 +52,17 @@
 public class VerifyPhoneNumberFragment extends FragmentBase implements View.OnClickListener {
     public static final String TAG = "VerifyPhoneFragment";
 
-    CountryListSpinner countryListSpinner;
-    EditText mPhoneEditText;
-    TextView errorEditText;
-    Button sendCodeButton;
-    PhoneVerificationActivity mVerifier;
-    TextView mSmsTermsText;
+    private CountryListSpinner countryListSpinner;
+    private EditText mPhoneEditText;
+    private TextView errorEditText;
+    private Button sendCodeButton;
+    private PhoneVerificationActivity mVerifier;
+    private TextView mSmsTermsText;
 
     private static final int RC_PHONE_HINT = 22;
 
-    public static VerifyPhoneNumberFragment getInstance(FlowParameters flowParameters, String
-            phone) {
+    public static VerifyPhoneNumberFragment newInstance(FlowParameters flowParameters,
+                                                        String phone) {
         VerifyPhoneNumberFragment fragment = new VerifyPhoneNumberFragment();
 
         Bundle args = new Bundle();
diff --git a/auth/src/main/java/com/firebase/ui/auth/util/signincontainer/SaveSmartLock.java b/auth/src/main/java/com/firebase/ui/auth/util/signincontainer/SaveSmartLock.java
@@ -93,6 +93,7 @@ public void onConnected(Bundle bundle) {
             finish();
             return;
         }
+
         Credential.Builder builder = new Credential.Builder(mEmail);
         builder.setPassword(mPassword);
         if (mPassword == null && mResponse != null) {
diff --git a/auth/src/main/java/com/firebase/ui/auth/util/signincontainer/SmartLockBase.java b/auth/src/main/java/com/firebase/ui/auth/util/signincontainer/SmartLockBase.java
@@ -55,6 +55,9 @@ public static String providerIdToAccountType(@AuthUI.SupportedProvider @NonNull
             case EmailAuthProvider.PROVIDER_ID:
                 // The account type for email/password creds is null
                 return null;
+            case PhoneAuthProvider.PROVIDER_ID:
+                // The account type for phone creds is null
+                return null;
             default:
                 return null;
         }
diff --git a/auth/src/test/java/com/firebase/ui/auth/testhelpers/ActivityHelperShadow.java b/auth/src/test/java/com/firebase/ui/auth/testhelpers/ActivityHelperShadow.java
@@ -16,7 +16,6 @@
 
 import com.firebase.ui.auth.ui.ActivityHelper;
 import com.firebase.ui.auth.util.signincontainer.SaveSmartLock;
-import com.google.firebase.auth.PhoneAuthProvider;
 
 import org.robolectric.annotation.Implementation;
 import org.robolectric.annotation.Implements;
diff --git a/auth/src/test/java/com/firebase/ui/auth/testhelpers/TestConstants.java b/auth/src/test/java/com/firebase/ui/auth/testhelpers/TestConstants.java
@@ -18,7 +18,7 @@
 
 public class TestConstants {
     public static final String EMAIL = "test@example.com";
-    public static final String PHONE = "+14347565353";
+    public static final String PHONE = "+15555555555";
     public static final String PASSWORD = "hunter2";
     public static final String NAME = "Test Testerson";
     public static final String TOKEN = "token";
diff --git a/auth/src/test/java/com/firebase/ui/auth/testhelpers/TestHelper.java b/auth/src/test/java/com/firebase/ui/auth/testhelpers/TestHelper.java
@@ -70,6 +70,12 @@ public static FlowParameters getFlowParameters(List<String> providerIds) {
     }
 
     public static void verifySmartLockSave(String providerId, String email, String password) {
+        verifySmartLockSave(providerId, email, password, null);
+    }
+
+    public static void verifySmartLockSave(String providerId, String email,
+                                           String password, String phoneNumber) {
+
         ArgumentCaptor<FirebaseUser> userCaptor = ArgumentCaptor.forClass(FirebaseUser.class);
         ArgumentCaptor<String> passwordCaptor = ArgumentCaptor.forClass(String.class);
         ArgumentCaptor<IdpResponse> idpResponseCaptor = ArgumentCaptor.forClass(IdpResponse.class);
@@ -79,30 +85,20 @@ public static void verifySmartLockSave(String providerId, String email, String p
                 passwordCaptor.capture(),
                 idpResponseCaptor.capture());
 
+        // Check email and password
         assertEquals(email, userCaptor.getValue().getEmail());
         assertEquals(password, passwordCaptor.getValue());
-        assertEquals(providerId, idpResponseCaptor.getValue().getProviderType());
-    }
 
-    public static void verifySmartLockSave(String providerId, String email, String phoneNumber,
-                                           String password) {
-        ArgumentCaptor<FirebaseUser> userArgumentCaptor =
-                ArgumentCaptor.forClass(FirebaseUser.class);
-        ArgumentCaptor<IdpResponse> idpResponseArgumentCaptor =
-                ArgumentCaptor.forClass(IdpResponse.class);
-        ArgumentCaptor<String> passwordArgumentCaptor = ArgumentCaptor.forClass(String.class);
-        verify(ActivityHelperShadow.sSaveSmartLock)
-                .saveCredentialsOrFinish(userArgumentCaptor.capture(),
-                        passwordArgumentCaptor.capture(), idpResponseArgumentCaptor.capture());
-        assertEquals(email, userArgumentCaptor.getValue().getEmail());
-        assertEquals(phoneNumber, userArgumentCaptor.getValue().getPhoneNumber());
-        assertEquals(password, passwordArgumentCaptor.getValue());
+        // Check phone number (if necessary)
+        if (phoneNumber != null) {
+            assertEquals(phoneNumber, userCaptor.getValue().getPhoneNumber());
+        }
+
+        // Check provider id
         if (providerId == null) {
-            assertNull(idpResponseArgumentCaptor.getValue());
+            assertNull(idpResponseCaptor.getValue());
         } else {
-            assertEquals(
-                    providerId,
-                    idpResponseArgumentCaptor.getValue().getProviderType());
+            assertEquals(providerId, idpResponseCaptor.getValue().getProviderType());
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@`
`5`	`5`	`import android.content.Intent;`
`6`	`6`	`import android.support.annotation.LayoutRes;`
`7`	`7`
	`8`	`+import com.firebase.ui.auth.AuthUI;`
`8`	`9`	`import com.firebase.ui.auth.R;`
`9`	`10`	`import com.firebase.ui.auth.ResultCodes;`
`10`	`11`	`import com.firebase.ui.auth.ui.BaseHelper;`
`@@ -28,6 +29,7 @@ public String getName(Context context) {`
`28`	`29`	`}`
`29`	`30`
`30`	`31`	`@Override`
	`32`	`+ @AuthUI.SupportedProvider`
`31`	`33`	`public String getProviderId() {`
`32`	`34`	`return EmailAuthProvider.PROVIDER_ID;`
`33`	`35`	`}`
Original file line number	Diff line number	Diff line change
`@@ -55,11 +55,12 @@ public static void initialize(Context context) {`
`55`	`55`
`56`	`56`	`public static void signout(Context context) {`
`57`	`57`	`try {`
`58`		`- signOut();`
	`58`	`+ Twitter.getInstance();`
`59`	`59`	`} catch (IllegalStateException e) {`
`60`	`60`	`initialize(context);`
`61`		`- signOut();`
`62`	`61`	`}`
	`62`	`+`
	`63`	`+ signOut();`
`63`	`64`	`}`
`64`	`65`
`65`	`66`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,7 @@ public class FragmentHelper extends BaseHelper {`
`13`	`13`	`private Fragment mFragment;`
`14`	`14`
`15`	`15`	`public FragmentHelper(Fragment fragment) {`
`16`		`- super(fragment.getContext(),`
	`16`	`+ super(fragment.getContext().getApplicationContext(),`
`17`	`17`	`(FlowParameters) fragment.getArguments().getParcelable(ExtraConstants.EXTRA_FLOW_PARAMS));`
`18`	`18`	`mFragment = fragment;`
`19`	`19`	`}`