Fix login error message for incorrect credentials#507
Open
hiraljj05 wants to merge 7 commits intoCircuitVerse:masterfrom
Open
Fix login error message for incorrect credentials#507hiraljj05 wants to merge 7 commits intoCircuitVerse:masterfrom
hiraljj05 wants to merge 7 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 the authentication system with OAuth improvements, email verification enforcement, and API contract refinements. It modifies login and signup flows to return non-nullable Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 14
🧹 Nitpick comments (6)
lib/viewmodels/authentication/auth_options_viewmodel.dart (1)
22-61: Remove the 40-line commented-out block.This dead code adds noise. The new implementation below is the active path — the old one should be deleted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/viewmodels/authentication/auth_options_viewmodel.dart` around lines 22 - 61, Remove the large commented-out legacy googleAuth implementation: delete the entire commented block that defines the old Future googleAuth(...) implementation (the lines referencing GoogleSignIn, GOOGLE_OAUTH, ViewState, _userApi, _storage, AuthType) so only the new active authentication implementation remains; ensure there are no leftover commented references to GOOGLE_OAUTH or ViewState related to that old flow.lib/utils/api_utils.dart (1)
48-49: Remove commented-out debug prints.These commented-out statements would log
statusCodeandresponse.body(which may contain auth tokens or PII) if ever uncommented. They shouldn't be committed — use a proper logging framework with configurable levels instead of leaving commented-outProposed fix
- // print("STATUS CODE: ${response.statusCode}"); - // print("RAW BODY: ${response.body}");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/utils/api_utils.dart` around lines 48 - 49, Remove the commented-out debug prints that reference response.statusCode and response.body in lib/utils/api_utils.dart; delete the lines "// print("STATUS CODE: ${response.statusCode}");" and "// print("RAW BODY: ${response.body}");" and if you need runtime diagnostics replace them with calls to the project's logging utility at an appropriate log level (never log response.body or sensitive tokens) — locate the call site where the local variable response is used and swap in logger.debug/info with sanitized output instead of leaving commented print statements.lib/config/environment_config.dart (1)
17-34: Remove commented-out code; consider usingString.fromEnvironmentfor the Google client ID for consistency.Lines 25–31 are dead commented-out code and should be removed.
The
GOOGLE_OAUTH_CLIENT_IDis hardcoded whileGITHUB_OAUTH_CLIENT_IDusesString.fromEnvironment. For consistency and flexibility (e.g., different client IDs per environment/flavor), consider the same pattern:Proposed change
- // Google OAUTH ENV VARIABLES - static const String GOOGLE_OAUTH_CLIENT_ID = - "30473033321-4v4ap4b3r16e8vq9c100d8bi0edbnbbs.apps.googleusercontent.com"; + // Google OAuth + static const String GOOGLE_OAUTH_CLIENT_ID = String.fromEnvironment( + 'GOOGLE_OAUTH_CLIENT_ID', + defaultValue: '30473033321-4v4ap4b3r16e8vq9c100d8bi0edbnbbs.apps.googleusercontent.com', + ); // GITHUB OAUTH ENV VARIABLES static const String GITHUB_OAUTH_CLIENT_ID = String.fromEnvironment( 'GITHUB_OAUTH_CLIENT_ID', ); - // static const String GITHUB_OAUTH_CLIENT_SECRET = String.fromEnvironment( - // 'GITHUB_OAUTH_CLIENT_SECRET', - // ); - // static const String GITHUB_OAUTH_REDIRECT_URI = String.fromEnvironment( - // 'GITHUB_OAUTH_REDIRECT_URI', - // defaultValue: 'circuitverse://auth/callback/github', - // );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/config/environment_config.dart` around lines 17 - 34, Remove the dead commented-out block that references GITHUB_OAUTH_CLIENT_SECRET and the commented GITHUB_OAUTH_REDIRECT_URI, and make the Google client ID follow the same pattern as GitHub by replacing the hardcoded GOOGLE_OAUTH_CLIENT_ID with a String.fromEnvironment lookup (e.g., use String.fromEnvironment('GOOGLE_OAUTH_CLIENT_ID', defaultValue: '<current-id-or-empty>')) while leaving GITHUB_OAUTH_CLIENT_ID and the active GITHUB_OAUTH_REDIRECT_URI symbol unchanged.lib/ui/views/authentication/login_view.dart (1)
133-135: Please remove stale commented-out code blocks.The old focus/snackbar snippets (Line 133, Line 143, Line 155) add noise and make this flow harder to scan.
Also applies to: 143-145, 155-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/ui/views/authentication/login_view.dart` around lines 133 - 135, Remove the stale commented-out focus/snackbar snippets inside the login view widget (e.g., the commented FocusScope.of(context).requestFocus(FocusNode()) and the old snackbar/focus blocks found near the login form in LoginView/_LoginViewState build/submit handlers); simply delete those commented lines (the blocks around the FocusScope and old SnackBar code) so the code is cleaner and easier to scan, leaving any active keyboard dismissals or snackbar logic intact if already implemented elsewhere.android/app/build.gradle (2)
136-136: Use theplugins {}block instead of the legacyapply pluginsyntax.
apply plugin: 'com.google.gms.google-services'is the legacy Groovy syntax. Sincecom.google.gms.google-servicesis already declared inandroid/settings.gradlewithapply false, the idiomatic approach is to add it to the existingplugins {}block at the top of this file.♻️ Proposed refactor
plugins { id "com.android.application" id "kotlin-android" id "dev.flutter.flutter-gradle-plugin" + id "com.google.gms.google-services" }Then remove:
-apply plugin: 'com.google.gms.google-services'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/build.gradle` at line 136, Replace the legacy Groovy "apply plugin: 'com.google.gms.google-services'" line by adding the plugin id into the existing plugins { } block at the top of the Gradle file (use id "com.google.gms.google-services") since it is declared apply false in settings.gradle, then remove the old apply plugin line; ensure the plugins block is used consistently for other plugins in the file.
129-133: The forcedandroidx.browser:browser:1.8.0is a maintenance liability.Pinning a transitive dependency to an exact version via
resolutionStrategy.forcewill block all future upgrades to that library until manually updated. If the version that brings the dependency naturally evolves, this pin will silently override it. Prefer constraining only the minimum version (prefer/strictlywith a lower bound) if the goal is to resolve a version conflict, and include a comment explaining the root cause.♻️ Suggested alternative
configurations.all { resolutionStrategy { - force "androidx.browser:browser:1.8.0" + // TODO: Remove once <dependency X> updates its own browser dependency. + // Force minimum 1.8.0 to resolve conflict with <library>. + force "androidx.browser:browser:1.8.0" } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/build.gradle` around lines 129 - 133, Replace the hard force of "androidx.browser:browser:1.8.0" in the configurations.all resolutionStrategy (the resolutionStrategy.force call) with a version constraint that only sets a minimum/preferred version (e.g., use prefer/strictly with a lower bound or an eachDependency rule that sets version if missing) so transitive upgrades are not blocked; update the resolutionStrategy usage (targeting the same dependency group:artifact "androidx.browser:browser") and add a short comment explaining the root cause of the original conflict and why the minimum version is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@android/app/build.gradle`:
- Line 54: The PR bumps compileSdk to 36 which together with a simultaneous bump
of targetSdkVersion to 36 enforces Android edge-to-edge behavior; confirm
window-insets-safe handling across your UI or avoid the behavior change by
keeping targetSdkVersion at 35 for now. Inspect the Android manifest and UI code
paths that handle system bars (window insets handling,
CoordinatorLayout/AppCompat themes, insets listeners) and ensure components
correctly consume/apply Insets before setting targetSdkVersion to 36, or change
only compileSdk to 36 while leaving targetSdkVersion at 35 until you audit and
adapt all screens.
- Line 76: The change replaced the hardcoded minSdkVersion 23 with
flutter.minSdkVersion which can resolve to API 16–19 depending on Flutter
version; to prevent lowering device support, restore a safe minimum by setting
minSdkVersion back to 23 (replace flutter.minSdkVersion with 23 in the
minSdkVersion declaration) or explicitly enforce a project-wide minSdk (e.g., in
gradle.properties or the android block) so that the Gradle symbol minSdkVersion
uses API 23 at build time.
In `@android/settings.gradle`:
- Line 23: Update the Gradle plugin declaration for the Google Services plugin
by changing the version string for id "com.google.gms.google-services" from
"4.4.0" to "4.4.4" so the project uses the latest stable release; locate the
line containing id "com.google.gms.google-services" in android/settings.gradle
and replace the version token accordingly, then re-sync/refresh Gradle to verify
no compatibility issues.
In `@l10n.yaml`:
- Line 5: Restore the explicit synthetic-package setting in l10n.yaml by
re-adding the "synthetic-package: false" entry so the generator respects the
configured output-dir (lib/gen_l10n/) and avoids writing to
.dart_tool/flutter_gen/, preventing stale/missing imports like
package:mobile_app/gen_l10n/app_localizations.dart and future incompatibility
with package:flutter_gen removal.
In `@lib/services/API/users_api.dart`:
- Around line 96-112: The signup method body (function signup) is mis-indented
and appears as a top-level function; re-indent the entire method block so its
signature and body align with other class members (match the class indentation
level used by other methods in the same file), ensuring the opening and closing
braces are inside the class scope and the try/catch and internal statements are
consistently indented; verify no stray top-level whitespace remains so signup is
clearly a class method.
- Line 104: Remove the debug print that leaks API response data by deleting the
print(response) call in users_api.dart; if you need to retain observability,
replace it with a proper logging call (e.g., use the existing app logger) and
guard it behind a debug flag or environment check so sensitive response bodies
or tokens are never printed in production — locate the print(response)
invocation in the signup/response-handling function and either remove it or swap
it for a debug-only logger call.
- Around line 233-282: Remove the stray duplicate `@override` and the leftover
commented-out blocks around the sendResetPasswordInstructions method: keep the
valid Future<bool> sendResetPasswordInstructions(String email) implementation
(with its single `@override`) and delete the earlier dangling `@override` and both
commented-out code regions above and below the method so there is no
dead/commented duplicate code or duplicate annotations.
- Around line 56-59: The user-verification check in users_api.dart is
misindented and currently throws a Failure with a human-readable string which
LoginViewModel then pattern-matches on; fix by aligning the if block with the
surrounding try/catch and replace the string-based Failure with a structured
error (either throw a dedicated EmailNotVerifiedException or throw Failure with
a stable errorCode like 'EMAIL_NOT_VERIFIED' on the
jsonResponse['user']['is_verified'] == false branch); update the throw in the
users API (search for the if block around jsonResponse['user']['is_verified']
and the Failure construction) so LoginViewModel can check f.errorCode ==
'EMAIL_NOT_VERIFIED' or catch EmailNotVerifiedException instead.
- Around line 69-76: The current catch chain in the login/auth method swallows a
previously-thrown Failure (e.g., "Please verify your email before logging in.")
because the broad `on Exception` handler catches all Exceptions; update the
catch clauses in the function that contains the
UnauthorizedException/NotFoundException handlers (the login method in
users_api.dart) by inserting an `on Failure { rethrow; }` clause before the
general `on Exception` handler so user-generated Failure instances propagate
unchanged, and add a separate `on FormatException { throw Failure("Response
format error"); }` (or other suitable message) to handle malformed JSON
explicitly instead of mapping it to "Incorrect email or password".
In `@lib/ui/views/authentication/login_view.dart`:
- Around line 163-176: Don't compare against the localized message string;
instead use a stable error code/type exposed by the model (e.g., add or use an
enum or constant like LoginError.invalidCredentials or a method like
_model.errorCodeFor(_model.LOGIN)) and check that value before calling
_formKey.currentState?.reset(); update the view to call the stable accessor
(replace the raw string equality check) and, if needed, modify the model to
return an error code or property representing the "incorrect credentials" case
so the UI resets only when that error code is present.
In `@lib/ui/views/authentication/signup_view.dart`:
- Around line 145-164: Replace the hardcoded dialog strings in the
Get.defaultDialog call with localized strings from AppLocalizations (e.g.
AppLocalizations.of(context)!.verify_email_title, verify_email_message, ok) and
remove the dead commented SnackBarUtils/Future/Get.offAllNamed block; keep the
dialog behavior intact (Get.back(), Get.offAllNamed(LoginView.id)). Add the
corresponding keys (verify_email_title, verify_email_message, ok) to the .arb
localization files and ensure any new keys are referenced via
AppLocalizations.of(context)! in signup_view.dart rather than literal strings.
In `@lib/viewmodels/authentication/auth_options_viewmodel.dart`:
- Around line 63-121: The googleAuth method body is indented at top-level,
making it appear outside the class; re-indent the entire googleAuth method so
its opening/closing braces and body are aligned with the other instance methods
(e.g., githubAuth) inside the same class in auth_options_viewmodel.dart, ensure
the method is nested within the class declaration (adjust surrounding braces if
necessary) and match the same indentation style used by githubAuth to keep the
class structure and readability consistent.
In `@lib/viewmodels/authentication/login_viewmodel.dart`:
- Around line 30-42: Remove the dead commented-out catch block, re-indent the
existing catch body so it aligns with the surrounding try block, and replace the
two hardcoded strings passed to setErrorMessageFor(LOGIN, ...) with localization
lookups (e.g. AppLocalizations.of(context).emailNotVerified and
AppLocalizations.of(context).incorrectEmailOrPassword) instead of raw literals;
also stop coupling on f.message text — check a structured property on Failure
such as f.code == 'not_verified' or a FailureType (update users_api.dart to
populate that code) and use that to decide which localized message to set.
In `@lib/viewmodels/authentication/signup_viewmodel.dart`:
- Around line 15-49: Delete the large commented-out signup block and re-indent
the remaining Future<void> signup(String name, String email, String password)
async { ... } so it is defined as a method inside the SignupViewModel class (not
a top-level function); ensure the closing braces of SignupViewModel align
correctly and remove any stray extra brace so SignupViewModel contains the
signup method and there is no dead commented code.
---
Nitpick comments:
In `@android/app/build.gradle`:
- Line 136: Replace the legacy Groovy "apply plugin:
'com.google.gms.google-services'" line by adding the plugin id into the existing
plugins { } block at the top of the Gradle file (use id
"com.google.gms.google-services") since it is declared apply false in
settings.gradle, then remove the old apply plugin line; ensure the plugins block
is used consistently for other plugins in the file.
- Around line 129-133: Replace the hard force of
"androidx.browser:browser:1.8.0" in the configurations.all resolutionStrategy
(the resolutionStrategy.force call) with a version constraint that only sets a
minimum/preferred version (e.g., use prefer/strictly with a lower bound or an
eachDependency rule that sets version if missing) so transitive upgrades are not
blocked; update the resolutionStrategy usage (targeting the same dependency
group:artifact "androidx.browser:browser") and add a short comment explaining
the root cause of the original conflict and why the minimum version is required.
In `@lib/config/environment_config.dart`:
- Around line 17-34: Remove the dead commented-out block that references
GITHUB_OAUTH_CLIENT_SECRET and the commented GITHUB_OAUTH_REDIRECT_URI, and make
the Google client ID follow the same pattern as GitHub by replacing the
hardcoded GOOGLE_OAUTH_CLIENT_ID with a String.fromEnvironment lookup (e.g., use
String.fromEnvironment('GOOGLE_OAUTH_CLIENT_ID', defaultValue:
'<current-id-or-empty>')) while leaving GITHUB_OAUTH_CLIENT_ID and the active
GITHUB_OAUTH_REDIRECT_URI symbol unchanged.
In `@lib/ui/views/authentication/login_view.dart`:
- Around line 133-135: Remove the stale commented-out focus/snackbar snippets
inside the login view widget (e.g., the commented
FocusScope.of(context).requestFocus(FocusNode()) and the old snackbar/focus
blocks found near the login form in LoginView/_LoginViewState build/submit
handlers); simply delete those commented lines (the blocks around the FocusScope
and old SnackBar code) so the code is cleaner and easier to scan, leaving any
active keyboard dismissals or snackbar logic intact if already implemented
elsewhere.
In `@lib/utils/api_utils.dart`:
- Around line 48-49: Remove the commented-out debug prints that reference
response.statusCode and response.body in lib/utils/api_utils.dart; delete the
lines "// print("STATUS CODE: ${response.statusCode}");" and "// print("RAW
BODY: ${response.body}");" and if you need runtime diagnostics replace them with
calls to the project's logging utility at an appropriate log level (never log
response.body or sensitive tokens) — locate the call site where the local
variable response is used and swap in logger.debug/info with sanitized output
instead of leaving commented print statements.
In `@lib/viewmodels/authentication/auth_options_viewmodel.dart`:
- Around line 22-61: Remove the large commented-out legacy googleAuth
implementation: delete the entire commented block that defines the old Future
googleAuth(...) implementation (the lines referencing GoogleSignIn,
GOOGLE_OAUTH, ViewState, _userApi, _storage, AuthType) so only the new active
authentication implementation remains; ensure there are no leftover commented
references to GOOGLE_OAUTH or ViewState related to that old flow.
ℹ️ 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 (15)
.gitignoreandroid/app/build.gradleandroid/settings.gradlel10n.yamllib/config/environment_config.dartlib/services/API/users_api.dartlib/ui/views/authentication/forgot_password_view.dartlib/ui/views/authentication/login_view.dartlib/ui/views/authentication/signup_view.dartlib/utils/api_utils.dartlib/viewmodels/authentication/auth_options_viewmodel.dartlib/viewmodels/authentication/login_viewmodel.dartlib/viewmodels/authentication/signup_viewmodel.dartwindows/flutter/generated_plugin_registrant.ccwindows/flutter/generated_plugins.cmake
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.
Fixes #502
Describe the changes you have made in this PR -
Added proper error handling for login with incorrect credentials.
Implemented a ScaffoldMessenger SnackBar to show a clear message like "Incorrect email or password." when login fails.
Ensured that valid logins continue to navigate to the landing page without any issues.
No other code logic or functionality has been affected; only the login feedback mechanism has been enhanced.
Form resets automatically when incorrect credentials are entered to allow the user to retry.
1.mp4
Screenshots of the changes (If any) -
Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
Release Notes
New Features
Chores