Skip to content

fix(auth): prevent partial authentication state on fetch user failure#539

Open
may-tas wants to merge 3 commits intoCircuitVerse:masterfrom
may-tas:fix-partial-auth-crash
Open

fix(auth): prevent partial authentication state on fetch user failure#539
may-tas wants to merge 3 commits intoCircuitVerse:masterfrom
may-tas:fix-partial-auth-crash

Conversation

@may-tas
Copy link
Copy Markdown

@may-tas may-tas commented Mar 19, 2026

Fixes #537

Describe the changes you have made in this PR -

  • Fixed a fatal UI crash spanning multiple drawer/profile views caused by a partial authentication state logic bug.
  • Re-ordered the login sequence in LoginViewModel and SignupViewModel. Previously, _storage.isLoggedIn = true was being assigned before the network call to fetch user profile data completed.
  • Now, the codebase verifies that fetchCurrentUser() has successfully fetched data and mapped the model before allowing the application state to reflect a logged-in status.

Screenshots of the changes (If any) -
N/A

Summary by CodeRabbit

  • Refactor
    • Reordered authentication operations in login and signup flows to set logged-in status after user data is successfully fetched and saved, improving initialization sequence.

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 19, 2026

Deploy Preview for cv-mobile-app-web ready!

Name Link
🔨 Latest commit f5f3b5c
🔍 Latest deploy log https://app.netlify.com/projects/cv-mobile-app-web/deploys/69bbcc7914d3f50008105b14
😎 Deploy Preview https://deploy-preview-539--cv-mobile-app-web.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 19, 2026

Warning

Rate limit exceeded

@may-tas has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 1 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0ca8484a-0f6a-4d13-b8b1-01543bfaab0d

📥 Commits

Reviewing files that changed from the base of the PR and between 5459171 and f5f3b5c.

📒 Files selected for processing (4)
  • lib/viewmodels/authentication/login_viewmodel.dart
  • lib/viewmodels/authentication/signup_viewmodel.dart
  • test/viewmodel_tests/authentication/login_viewmodel_test.dart
  • test/viewmodel_tests/authentication/signup_viewmodel_test.dart

Walkthrough

The changes reorder the authentication state assignments in LoginViewModel.login and SignupViewModel.signup. The _storage.isLoggedIn = true assignment is moved to occur after the _userApi.fetchCurrentUser() call completes, ensuring the logged-in flag is only set after the current user data is successfully fetched and stored. Both methods maintain their existing control flow and error handling while adjusting the sequence of state updates.

Suggested labels

potential-ai-slop

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: reordering authentication state updates to prevent setting isLoggedIn before fetchCurrentUser completes.
Linked Issues check ✅ Passed The PR reorders isLoggedIn assignment after fetchCurrentUser in both LoginViewModel and SignupViewModel, directly addressing issue #537's requirement to prevent partial authentication state.
Out of Scope Changes check ✅ Passed All changes are focused on reordering the isLoggedIn assignment relative to fetchCurrentUser in authentication viewmodels, staying within the scope of issue #537.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7aaaf182-76c4-4c2d-9448-0dcad78626fb

📥 Commits

Reviewing files that changed from the base of the PR and between f621531 and 5459171.

📒 Files selected for processing (2)
  • lib/viewmodels/authentication/login_viewmodel.dart
  • lib/viewmodels/authentication/signup_viewmodel.dart

Comment thread lib/viewmodels/authentication/login_viewmodel.dart
Comment thread lib/viewmodels/authentication/signup_viewmodel.dart
@may-tas
Copy link
Copy Markdown
Author

may-tas commented Mar 19, 2026

@coderabbitai please review.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 19, 2026

@may-tas Sure! I'll review the changes now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(auth): fatal UI crashes due to partial authentication state during login failures

1 participant