fix: resolve bounty issue #5 - save user data before sign out on email collision#16
Open
bomlinux92-byte wants to merge 2 commits into
Open
fix: resolve bounty issue #5 - save user data before sign out on email collision#16bomlinux92-byte wants to merge 2 commits into
bomlinux92-byte wants to merge 2 commits into
Conversation
added 2 commits
May 15, 2026 00:20
…ollisionException properly When email linking fails with FirebaseAuthUserCollisionException, the user was left with a phone-only account that cannot login with email/password. Fix: sign out the orphaned phone-only account and redirect to login screen with the email pre-filled, so the user can login with the existing account.
…gn out on email collision When a user signs up with phone+email and the email already exists in Firebase Auth (FirebaseAuthUserCollisionException), the original code would: 1. Sign out the phone-only account immediately 2. Redirect to login with prefilled email The problem: user data (name, phone, etc.) was never saved to Firestore, so when the user logs in with the existing email account, their profile data is missing. Fix: Create a new saveUserToFirestoreOnCollision() method that: 1. Saves user data to Firestore FIRST (using the phone auth UID) 2. Then signs out the phone-only account 3. Then redirects to login with prefilled email This ensures the user's profile data is preserved even when the email linking fails due to an existing account.
📝 WalkthroughWalkthroughOTPActivity now handles email-linking collisions by persisting user data to Firestore before signing out and redirecting to LoginActivity with the conflicting email prefilled. LoginActivity receives and pre-populates that email field on open. ChangesEmail Collision Redirect with Prefill
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java (1)
224-226: 💤 Low valueHardcoded English strings break i18n.
The toast messages on lines 225-226 and 235-236 use hardcoded English strings while the rest of the codebase uses string resources (e.g.,
getString(R.string.xxx)). Consider extracting these tostrings.xmlfor consistency with the existing localization pattern.Also applies to: 235-237
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java` around lines 224 - 226, The two Toasts in OTPActivity (the Toast.makeText calls in OTPActivity.java that show "An account with this email already exists..." and the other hardcoded message around lines 235-237) use hardcoded English strings; extract both messages into string resources in strings.xml and replace the literals with getString(R.string.<name>) calls in OTPActivity (e.g., R.string.email_exists_message and R.string.other_toast_message) so they follow the app's i18n pattern and can be translated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java`:
- Around line 202-243: In saveUserToFirestoreOnCollision, the method parameters
(name, email, number, password) are reassigned and then captured by lambdas
causing a compile error because they are not effectively final; fix by replacing
the conditional reassignments with new final local variables (e.g., final String
safeName = (name == null ? "" : name), final String safeEmail = (email == null ?
"" : email), etc.) and use these final locals in the UserModel constructor and
inside the addOnSuccessListener/addOnFailureListener callbacks so the lambdas
capture effectively final variables.
- Line 213: Replace the hardcoded invalid profile URL that contains "token=***"
with the real token used elsewhere (or extract to a shared constant) so both
usages reference the same value; create a private static final String (e.g.,
DEFAULT_PROFILE_URL) and use that constant in OTPActivity where the placeholder
string appears and in the other location that currently uses the actual token to
prevent broken profile images and duplication.
---
Nitpick comments:
In `@app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java`:
- Around line 224-226: The two Toasts in OTPActivity (the Toast.makeText calls
in OTPActivity.java that show "An account with this email already exists..." and
the other hardcoded message around lines 235-237) use hardcoded English strings;
extract both messages into string resources in strings.xml and replace the
literals with getString(R.string.<name>) calls in OTPActivity (e.g.,
R.string.email_exists_message and R.string.other_toast_message) so they follow
the app's i18n pattern and can be translated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7987af6-d0d4-4192-a56a-62939eb611d6
📒 Files selected for processing (2)
app/src/main/java/com/example/updateapp/views/activites/LoginActivity.javaapp/src/main/java/com/example/updateapp/views/activites/OTPActivity.java
Comment on lines
+202
to
+243
| private void saveUserToFirestoreOnCollision(String uid, String name, String email, String number, String password) { | ||
| if (name == null) name = ""; | ||
| if (email == null) email = ""; | ||
| if (number == null) number = ""; | ||
| if (password == null) password = ""; | ||
|
|
||
| UserModel user = new UserModel( | ||
| name, | ||
| email, | ||
| number, | ||
| password, | ||
| "https://firebasestorage.googleapis.com/v0/b/earning-b8942.firebasestorage.app/o/account.png?alt=media&token=***" | ||
| ); | ||
|
|
||
| if (dialog.isShowing()) dialog.dismiss(); | ||
|
|
||
| firestore.collection("users") | ||
| .document(uid) | ||
| .set(user) | ||
| .addOnSuccessListener(unused -> { | ||
| // User data saved successfully. Now sign out and redirect to login. | ||
| auth.signOut(); | ||
| Toast.makeText(OTPActivity.this, | ||
| "An account with this email already exists. Please login with your email and password.", | ||
| Toast.LENGTH_LONG).show(); | ||
| Intent intent = new Intent(OTPActivity.this, LoginActivity.class); | ||
| intent.putExtra("prefilled_email", email); | ||
| startActivity(intent); | ||
| finish(); | ||
| }) | ||
| .addOnFailureListener(e -> { | ||
| // Failed to save - still redirect to login with email prefilled. | ||
| auth.signOut(); | ||
| Toast.makeText(OTPActivity.this, | ||
| "Failed to link email: " + e.getMessage() + ". Please login with your existing account.", | ||
| Toast.LENGTH_LONG).show(); | ||
| Intent intent = new Intent(OTPActivity.this, LoginActivity.class); | ||
| intent.putExtra("prefilled_email", email); | ||
| startActivity(intent); | ||
| finish(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Compile error: email is not effectively final.
The parameters name, email, number, and password are conditionally reassigned (lines 203-206), then email is captured in lambda callbacks (lines 228, 239). Java requires variables used in lambdas to be effectively final. This code will fail to compile.
🐛 Proposed fix using final local variables
private void saveUserToFirestoreOnCollision(String uid, String name, String email, String number, String password) {
- if (name == null) name = "";
- if (email == null) email = "";
- if (number == null) number = "";
- if (password == null) password = "";
+ final String safeName = (name == null) ? "" : name;
+ final String safeEmail = (email == null) ? "" : email;
+ final String safeNumber = (number == null) ? "" : number;
+ final String safePassword = (password == null) ? "" : password;
UserModel user = new UserModel(
- name,
- email,
- number,
- password,
+ safeName,
+ safeEmail,
+ safeNumber,
+ safePassword,
"https://firebasestorage.googleapis.com/v0/b/earning-b8942.firebasestorage.app/o/account.png?alt=media&token=***"
);
if (dialog.isShowing()) dialog.dismiss();
firestore.collection("users")
.document(uid)
.set(user)
.addOnSuccessListener(unused -> {
// User data saved successfully. Now sign out and redirect to login.
auth.signOut();
Toast.makeText(OTPActivity.this,
"An account with this email already exists. Please login with your email and password.",
Toast.LENGTH_LONG).show();
Intent intent = new Intent(OTPActivity.this, LoginActivity.class);
- intent.putExtra("prefilled_email", email);
+ intent.putExtra("prefilled_email", safeEmail);
startActivity(intent);
finish();
})
.addOnFailureListener(e -> {
// Failed to save - still redirect to login with email prefilled.
auth.signOut();
Toast.makeText(OTPActivity.this,
"Failed to link email: " + e.getMessage() + ". Please login with your existing account.",
Toast.LENGTH_LONG).show();
Intent intent = new Intent(OTPActivity.this, LoginActivity.class);
- intent.putExtra("prefilled_email", email);
+ intent.putExtra("prefilled_email", safeEmail);
startActivity(intent);
finish();
});
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java`
around lines 202 - 243, In saveUserToFirestoreOnCollision, the method parameters
(name, email, number, password) are reassigned and then captured by lambdas
causing a compile error because they are not effectively final; fix by replacing
the conditional reassignments with new final local variables (e.g., final String
safeName = (name == null ? "" : name), final String safeEmail = (email == null ?
"" : email), etc.) and use these final locals in the UserModel constructor and
inside the addOnSuccessListener/addOnFailureListener callbacks so the lambdas
capture effectively final variables.
| email, | ||
| number, | ||
| password, | ||
| "https://firebasestorage.googleapis.com/v0/b/earning-b8942.firebasestorage.app/o/account.png?alt=media&token=***" |
There was a problem hiding this comment.
Placeholder token will produce broken profile images.
The image URL uses token=*** while the existing method at line 257 uses the actual token. Users created through the collision flow will have invalid profile image URLs. Use the same token or extract the URL to a constant.
🐛 Proposed fix
- "https://firebasestorage.googleapis.com/v0/b/earning-b8942.firebasestorage.app/o/account.png?alt=media&token=***"
+ "https://firebasestorage.googleapis.com/v0/b/earning-b8942.firebasestorage.app/o/account.png?alt=media&token=0ef08dd9-6b13-4da2-a39f-500cff3cf4f0"Consider extracting this URL to a constant to avoid duplication:
private static final String DEFAULT_PROFILE_URL =
"https://firebasestorage.googleapis.com/v0/b/earning-b8942.firebasestorage.app/o/account.png?alt=media&token=0ef08dd9-6b13-4da2-a39f-500cff3cf4f0";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "https://firebasestorage.googleapis.com/v0/b/earning-b8942.firebasestorage.app/o/account.png?alt=media&token=***" | |
| "https://firebasestorage.googleapis.com/v0/b/earning-b8942.firebasestorage.app/o/account.png?alt=media&token=0ef08dd9-6b13-4da2-a39f-500cff3cf4f0" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java` at
line 213, Replace the hardcoded invalid profile URL that contains "token=***"
with the real token used elsewhere (or extract to a shared constant) so both
usages reference the same value; create a private static final String (e.g.,
DEFAULT_PROFILE_URL) and use that constant in OTPActivity where the placeholder
string appears and in the other location that currently uses the actual token to
prevent broken profile images and duplication.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix for [BOUNTY /usr/bin/bash.40] Login fails with "Invalid Email" after successful signup and OTP verification — Issue #5
Root cause
When a user signs up with phone+email and the email already exists in Firebase Auth (FirebaseAuthUserCollisionException), the original code would:
The problem: user data (name, phone, etc.) was never saved to Firestore, so when the user logs in with the existing email account, their profile data is missing and the login fails with "Invalid Email" error.
Fix
Create a new
saveUserToFirestoreOnCollision()method that:This ensures the user's profile data is preserved even when the email linking fails due to an existing account.
Changes
saveUserToFirestoreOnCollision()method in OTPActivity.javasaveUserToFirestoreAndContinue()method is unchanged for normal flowCloses #5
Summary by CodeRabbit