refactor: update AuthRepositoryImpl to use Firebase Firestore consistently - #1
Open
devin-ai-integration[bot] wants to merge 1 commit into
Open
Conversation
…ently - Add @Inject constructor for Hilt DI support - Use injected firestore instance via usersCollection field everywhere (replaced direct Firebase.firestore references) - Add password verification in login flow - Add account active status check in login flow - Fix isLoggedIn() logic bug (was always returning true for non-blank IDs) - Fix getCurrentUser() to handle missing documents with proper null checks - Make toUser() null-safe with fallback defaults instead of !! operators - Add safe UserRole parsing with fallback to EXECUTOR - Remove unused getUserById() method (dead code) - Clean up unused imports (FirebaseAuth, SharedPreferences, Source) Co-Authored-By: moazz.m.2021 <moazz.m.2021@gmail.com>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
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
Refactors
AuthRepositoryImplto consistently use the injectedFirebaseFirestoreinstance and fixes several bugs in the authentication data layer.Firestore consistency:
Firebase.firestorecalls ingetUserEmailByPhone()and the removedgetUserById()with the injectedfirestoreinstance via a sharedusersCollectionfield.@Inject constructorfor proper Hilt DI.FirebaseAuth,SharedPreferences,Source).Login validation (behavioral change):
passwordfield. Previously, login succeeded for any password as long as the email existed.isActiveand rejects deactivated accounts.Bug fixes:
isLoggedIn()— the oldletblock always evaluated totruefor non-blank IDs due totruebeing the last expression in the block regardless of theifcheck.getCurrentUser()rewritten for clarity; removed deadgetUserById()method.toUser()now uses?: ""fallbacks and a try-catch forUserRoleparsing instead of!!operators that would crash on missing Firestore fields.Review & Testing Checklist for Human
storedPassword != passwordas plaintext. If passwords in Firestore are hashed/encrypted, this comparison will always fail and break login. Verify thepasswordfield format in theuserscollection.isActivefield presence: The?: falsedefault means users without anisActivefield in Firestore will be locked out. Verify all user documents have this field set.toUser()null-safety fallbacks: Confirm that returning empty strings for missing fields (instead of crashing) is acceptable behavior — this could silently hide data issues.Notes
RepositoryModulestill providesAuthRepositoryImplvia@Provides; this remains compatible with the newly added@Inject constructor.Link to Devin session: https://app.devin.ai/sessions/7d4047aa4c39448eab3973ca6b983915
Requested by: @MoaazMushatha