docs: fix information typo in simulator handler comment#955
docs: fix information typo in simulator handler comment#955magic-peach wants to merge 1 commit intoCircuitVerse:mainfrom
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. WalkthroughThis pull request modifies simulator handler pages to fix a comment typo and add user authentication handling. A misspelled comment in src/pages/simulatorHandler.vue and v1/src/pages/simulatorHandler.vue is corrected from "informaton" to "information". Additionally, a new getLoginData() function is introduced to fetch current user data from the /api/v1/me endpoint with Authorization headers. The function updates the authentication store on successful response and sets isUserLoggedIn to false on 401 responses, with error logging for failure cases. This new function is integrated into the onBeforeMount lifecycle hook's else branch when no project context is available. 🚥 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/simulatorHandler.vue (2)
94-98:⚠️ Potential issue | 🟠 Major
getLoginData()is not awaited — auth state may not be set when the component renders.
getLoginData()isasyncbut is called withoutawaiton line 96, soisLoading.value = falseon line 97 executes immediately. The component will render the simulator before the/api/v1/meresponse arrives, meaningisUserLoggedInandauthStoremay still be in their default state. This is the same fire-and-forget pattern thatcheckEditAccess()has, butcheckEditAccesssetsisLoading = falseinside its response handler, keeping the loading screen up until the response arrives.getLoginDatadoes not do this.Proposed fix: await getLoginData() or move isLoading inside the response handler
Option A — make
onBeforeMountcallback async and await:-onBeforeMount(() => { +onBeforeMount(async () => { // prioritize logixProjectId from window const windowLogixProjectId = (window as any).logixProjectId if (windowLogixProjectId && windowLogixProjectId !== '0') { ;(window as any).logixProjectId = windowLogixProjectId checkEditAccess() } else if (route.params.projectId) { ;(window as any).logixProjectId = route.params.projectId checkEditAccess() } else { // if projectId is not defined open blank simulator - getLoginData() + await getLoginData() isLoading.value = false } })Option B — set
isLoadinginsidegetLoginData(consistent withcheckEditAccess):async function getLoginData() { try { const response = await fetch('/api/v1/me', { ... }) if (response.ok) { const data = await response.json() authStore.setUserInfo(data.data) ;(window as any).isUserLoggedIn = true } else if (response.status === 401) { ;(window as any).isUserLoggedIn = false } } catch (err) { console.error(err) + } finally { + isLoading.value = false } }Then remove
isLoading.value = falsefrom theonBeforeMountelse branch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/simulatorHandler.vue` around lines 94 - 98, The else branch in onBeforeMount calls the async getLoginData() without awaiting, so isLoading.value is set to false before auth state (isUserLoggedIn/authStore) is populated; either make the onBeforeMount callback async and await getLoginData() before setting isLoading.value = false, or move the isLoading.value = false update into the success/finally path inside getLoginData() (like checkEditAccess does) so the loading state remains until the /api/v1/me response completes.
64-83:⚠️ Potential issue | 🟡 MinorPR scope exceeds its stated objective — this adds runtime behavior, not just a typo fix.
The PR title and description say this is a comment-only typo fix, but
getLoginData()introduces a new API call and modifies auth state at runtime. This should be called out in the PR description (and ideally linked to a separate issue/feature request) so reviewers understand the full scope of change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/simulatorHandler.vue` around lines 64 - 83, This PR accidentally adds runtime behavior via the new getLoginData() function (calls fetch('/api/v1/me') and mutates auth state via authStore.setUserInfo and window.isUserLoggedIn) despite claiming to be a typo-only change; either remove/revert getLoginData (and the fetch/auth mutations: authStore.setUserInfo and window.isUserLoggedIn) from this branch or move the entire function into a separate feature branch/PR and create/link an issue for it, and update the PR title/description to accurately reflect the behavioral change if you intend to keep it here.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/pages/simulatorHandler.vue`:
- Around line 94-98: The else branch in onBeforeMount calls the async
getLoginData() without awaiting, so isLoading.value is set to false before auth
state (isUserLoggedIn/authStore) is populated; either make the onBeforeMount
callback async and await getLoginData() before setting isLoading.value = false,
or move the isLoading.value = false update into the success/finally path inside
getLoginData() (like checkEditAccess does) so the loading state remains until
the /api/v1/me response completes.
- Around line 64-83: This PR accidentally adds runtime behavior via the new
getLoginData() function (calls fetch('/api/v1/me') and mutates auth state via
authStore.setUserInfo and window.isUserLoggedIn) despite claiming to be a
typo-only change; either remove/revert getLoginData (and the fetch/auth
mutations: authStore.setUserInfo and window.isUserLoggedIn) from this branch or
move the entire function into a separate feature branch/PR and create/link an
issue for it, and update the PR title/description to accurately reflect the
behavioral change if you intend to keep it here.
---
Duplicate comments:
In `@v1/src/pages/simulatorHandler.vue`:
- Around line 94-98: The code sets isLoading.value = false immediately after
calling getLoginData(), causing a race where loading ends before the async fetch
completes; update the else branch to await getLoginData() (e.g., await
getLoginData()) and handle errors (try/catch or .catch) so isLoading.value =
false only runs after getLoginData resolves or fails, keeping references to
getLoginData and isLoading.value when making the change.
magic-peach
force-pushed
the
simulator-handler-comment-typo#954
branch
from
8a8b1dd to
45379a7
Compare
1 participant
Fixes #954
Updated a typo in simulator handler comments from informaton to information in src/pages/simulatorHandler.vue and v1/src/pages/simulatorHandler.vue.
This is a comment-only correction and does not change behavior.
How to verify: open both files and confirm the comment uses information.
Summary by CodeRabbit
Bug Fixes
Improvements