fix: resolve login state issue preventing navigation (#504)#515
fix: resolve login state issue preventing navigation (#504)#515sahu-virendra-1908 wants to merge 3 commits intoCircuitVerse:masterfrom
Conversation
✅ Deploy Preview for cv-mobile-app-web ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis pull request updates Android build dependencies and configuration while refactoring the authentication flow. The Android SDK versions are bumped (compileSdk to 36, minSdkVersion to 24, targetSdkVersion to 36), Gradle wrapper is updated from 8.10.2 to 8.11.1, and Gradle plugins are upgraded (com.android.application to 8.9.1 and Kotlin to 2.1.0). Configuration adjustments include fixing a .gitignore pattern, adding FVM cache entries, removing a synthetic-package directive from l10n.yaml, and pinning a shared_preferences dependency. The login viewmodel is refactored to store the authentication token and mark the user as logged in immediately, with current user data fetched asynchronously without blocking login success. 🚥 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 3
🧹 Nitpick comments (1)
.gitignore (1)
47-49: Consider committing.fvmrcfor team consistency.The
.fvm/directory (cache) should be ignored, but.fvmrccontains the Flutter version configuration for the project. Committing.fvmrcensures all team members use the same Flutter version.Proposed change to commit .fvmrc
# FVM Version Cache .fvm/ -.fvmrc
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.gitignoreandroid/app/build.gradleandroid/gradle/wrapper/gradle-wrapper.propertiesandroid/settings.gradlel10n.yamllib/viewmodels/authentication/login_viewmodel.dartpubspec.yaml
💤 Files with no reviewable changes (1)
- l10n.yaml
| // Step 3: Fetch current user (non-blocking) | ||
| // If this fails, user is still logged in with token | ||
| try { | ||
| _storage.currentUser = await _userApi.fetchCurrentUser(); | ||
| print("_storage.currentUser: ${_storage.currentUser}"); | ||
| } on Failure catch (e) { | ||
| // Log the error but don't fail the login | ||
| print('Warning: Failed to fetch current user: ${e.message}'); | ||
| // User will be fetched later when accessing profile or landing screen | ||
| _storage.currentUser = null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "currentUser" --type=dart -C 2 | head -100Repository: CircuitVerse/mobile-app
Length of output: 8341
🏁 Script executed:
rg -n "currentUser\s*[!=]=\s*null|currentUser\?" --type=dartRepository: CircuitVerse/mobile-app
Length of output: 543
🏁 Script executed:
rg -n "CVLandingView|LandingView" --type=dart -C 2 | head -50Repository: CircuitVerse/mobile-app
Length of output: 3560
🏁 Script executed:
cat -n lib/viewmodels/profile/user_favourites_viewmodel.dart | sed -n '50,80p'Repository: CircuitVerse/mobile-app
Length of output: 1441
🏁 Script executed:
cat -n lib/viewmodels/profile/user_projects_viewmodel.dart | sed -n '45,75p'Repository: CircuitVerse/mobile-app
Length of output: 1419
🏁 Script executed:
cat -n lib/viewmodels/cv_landing_viewmodel.dart | sed -n '55,75p'Repository: CircuitVerse/mobile-app
Length of output: 664
🏁 Script executed:
fd -n "cv_landing.*view" --type=dart lib/ui/views | head -5Repository: CircuitVerse/mobile-app
Length of output: 297
🏁 Script executed:
fd "cv_landing" --type=dart lib/ui/viewsRepository: CircuitVerse/mobile-app
Length of output: 238
🏁 Script executed:
rg -n "fetchUserFavourites|fetchUserProjects" --type=dart -B5 -A5 | head -80Repository: CircuitVerse/mobile-app
Length of output: 7540
🏁 Script executed:
cat -n lib/ui/views/cv_landing_view.dart | head -100Repository: CircuitVerse/mobile-app
Length of output: 4220
🏁 Script executed:
cat -n lib/ui/views/profile/profile_view.dart | head -80Repository: CircuitVerse/mobile-app
Length of output: 3455
Add null-safety checks for currentUser in profile-related viewmodels.
When fetchCurrentUser fails, currentUser is set to null but login succeeds. However, UserFavouritesViewModel (lines 64, 72) and UserProjectsViewModel (lines 56, 64) use non-null assertions (currentUser!.data.id) when no explicit userId is provided. This will crash when a logged-in user with null currentUser navigates to their profile.
Either guard these methods with null checks before accessing currentUser, use safe navigation (currentUser?.data.id), or implement the retry mechanism mentioned in your comment (line 33) to fetch currentUser when accessing the profile screen.
| print("_storage.currentUser: ${_storage.currentUser}"); | ||
| } on Failure catch (e) { | ||
| // Log the error but don't fail the login | ||
| print('Warning: Failed to fetch current user: ${e.message}'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug print statements before merging.
The print() calls are debug artifacts that should be removed or replaced with proper logging infrastructure. These can leak user information in production logs.
Remove debug print statements
try {
_storage.currentUser = await _userApi.fetchCurrentUser();
- print("_storage.currentUser: ${_storage.currentUser}");
} on Failure catch (e) {
// Log the error but don't fail the login
- print('Warning: Failed to fetch current user: ${e.message}');
+ // Consider using a proper logging framework here
// User will be fetched later when accessing profile or landing screen
_storage.currentUser = null;
}📝 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.
| print("_storage.currentUser: ${_storage.currentUser}"); | |
| } on Failure catch (e) { | |
| // Log the error but don't fail the login | |
| print('Warning: Failed to fetch current user: ${e.message}'); | |
| try { | |
| _storage.currentUser = await _userApi.fetchCurrentUser(); | |
| } on Failure catch (e) { | |
| // Log the error but don't fail the login | |
| // Consider using a proper logging framework here | |
| // User will be fetched later when accessing profile or landing screen | |
| _storage.currentUser = null; | |
| } |
| scroll_to_index: ^3.0.1 | ||
| share_plus: ^11.1.0 | ||
| shared_preferences: ^2.5.4 | ||
| shared_preferences: 2.5.3 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "pubspec\.yaml|\.git" | head -20Repository: CircuitVerse/mobile-app
Length of output: 538
🏁 Script executed:
cat -n pubspec.yaml | head -60Repository: CircuitVerse/mobile-app
Length of output: 2346
🏁 Script executed:
# Check if there's git history to see what changed
git log --oneline --all -20 2>/dev/null || echo "No git history available"Repository: CircuitVerse/mobile-app
Length of output: 1592
🏁 Script executed:
# Query pub.dev API for shared_preferences information
echo "=== Latest shared_preferences version ===" && \
curl -s "https://pub.dev/api/packages/shared_preferences" | jq '.latest.version' 2>/dev/null || echo "Could not fetch latest"
echo -e "\n=== Version 2.5.3 environment requirements ===" && \
curl -s "https://pub.dev/api/packages/shared_preferences/versions/2.5.3" 2>/dev/null | jq '.pubspec.environment' || echo "Could not fetch 2.5.3 info"
echo -e "\n=== Version 2.5.4 environment requirements ===" && \
curl -s "https://pub.dev/api/packages/shared_preferences/versions/2.5.4" 2>/dev/null | jq '.pubspec.environment' || echo "Could not fetch 2.5.4 info"Repository: CircuitVerse/mobile-app
Length of output: 306
🏁 Script executed:
# Check if there's a comment in pubspec.yaml explaining the shared_preferences pin
sed -n '50,60p' pubspec.yamlRepository: CircuitVerse/mobile-app
Length of output: 331
Add a comment explaining the Dart SDK constraint.
The exact pin to 2.5.3 is necessary, not discretionary. The project's minimum Dart SDK is 3.7.2, but shared_preferences 2.5.4 requires Dart 3.9.0. Version 2.5.3 is the latest compatible release for the current SDK constraint.
Add an inline comment above the dependency to document this constraint so future maintainers understand why the version is pinned:
# Pinned to 2.5.3: shared_preferences 2.5.4 requires Dart 3.9.0, but this project supports 3.7.2+
shared_preferences: 2.5.3To upgrade to 2.5.4, the project's minimum Dart SDK must be bumped to 3.9.0 or higher.
1 participant
Fixes #504
Problem
After successful login, the token was correctly received and stored in SharedPreferences.
However, the login state was incorrectly set to Error, preventing navigation to CVLandingView.
Root Cause
The login ViewModel was overriding the state to Error despite successful authentication.
Changes Made
Result
No UI changes. This is a state management fix.
Summary by CodeRabbit
Bug Fixes
Chores