fix: Improve dashboard configuration form and monorepo scripts#25
Conversation
- Mask pairing code input with type="password" - Normalize baseUrl in payload to match gatewayBaseUrl (trim trailing slash) - Reset save status banner with a 3s timeout after successful save - Hide save status banner when any form field is edited - Conditionally include secrets in request body only if they are not empty - Add a 10s timeout to the POST request using AbortController - Ensure timeouts are cleared on component unmount - Fix typo "Requisitos minimos" to "Requisitos mínimos" in README.md - Add "test" and "test:dashboard" scripts to web monorepo root - Update README.md with the new test commands Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds request timeout handling (AbortController + 10s timer) and save-status timers to the dashboard App.vue, with cleanup via onUnmounted and a resetSaveStatus helper. Updates secret-input handling and baseUrl update behavior. Adds two npm test scripts and a small README spelling fix and new i18n timeout message. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as Dashboard App.vue
participant Timer as Request Timer (10s)
participant Server
User->>App: enter baseUrl / pairingCode (password)
App->>App: resetSaveStatus() (clear timers, set idle)
User->>App: trigger saveGatewayConfig
App->>Timer: start 10s AbortController timeout
App->>Server: fetch(save payload, signal)
Server-->>App: success response
App->>App: clear secret inputs, increment nonce, set saveStatus = success
App->>App: start 3s saveStatusTimeoutId to reset to idle
Timer-->>App: (if elapsed) abort fetch -> App sets timeoutError
note over App: onUnmounted clears both timers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ 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 |
|
Thank you for contributing to this project with this PR, welcome to the community and the amazing world of open source! |
Deploying corvus with
|
| Latest commit: |
72d8e9b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://31f31719.corvus-42x.pages.dev |
| Branch Preview URL: | https://fix-dashboard-config-improve.corvus-42x.pages.dev |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-02-18 to 2026-02-18 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
clients/web/apps/dashboard/src/App.vue (1)
34-35: Timer IDs don't need to be reactiverefs.
saveStatusTimeoutIdandrequestTimeoutIdare never read in the template or in a computed/watcher, so wrapping them inrefadds unnecessary Vue reactivity tracking.♻️ Replace with plain mutable variables
-const saveStatusTimeoutId = ref<ReturnType<typeof setTimeout> | null>(null); -const requestTimeoutId = ref<ReturnType<typeof setTimeout> | null>(null); +let saveStatusTimeoutId: ReturnType<typeof setTimeout> | null = null; +let requestTimeoutId: ReturnType<typeof setTimeout> | null = null;Update all
.valueaccesses to bare variable references throughoutresetSaveStatus,saveGatewayConfig, andonUnmounted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.vue` around lines 34 - 35, The timeout ID variables saveStatusTimeoutId and requestTimeoutId are incorrectly made reactive refs; change them to plain mutable variables (e.g., let saveStatusTimeoutId: ReturnType<typeof setTimeout> | null = null and let requestTimeoutId: ReturnType<typeof setTimeout> | null = null) and update all usages that currently access .value to use the bare variables instead; adjust references inside resetSaveStatus, saveGatewayConfig, and the onUnmounted cleanup to use the new plain variables and clearTimeout on them as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 59-65: The issue is that resetSaveStatus() can set saveStatus back
to "idle" while a save request is in-flight, leading to concurrent requests and
lost requestTimeoutId/AbortController; fix by preventing mid-flight changes:
either (A) add a guard at the top of resetSaveStatus() to return early when
saveStatus === 'saving' (so functions like captureSecretInput and the baseUrl
`@update`:model-value handler call resetSaveStatus safely), or (B) disable all
form inputs and the Save button while saving by adding :disabled="saveStatus ===
'saving'" to the inputs (bearerToken, webhookSecret, baseUrl input and any
secret capture inputs) so users cannot edit and re-enable the button during an
in-flight request; update captureSecretInput and the baseUrl handler to follow
the chosen approach.
- Around line 125-128: The current timeout fallback is ineffective because
t("form.timeoutError") returns the key string when missing; update the
localization or provide a proper default: either add the "form.timeoutError"
entry to your Spanish locale (e.g., src/locales/es.json) or change the call in
App.vue where saveErrorMessage.value is set to use the i18n default argument
form t("form.timeoutError", "Request timeout"); also make the save error lookup
consistent (t("form.saveError", "Save failed") or ensure the key exists) so
saveErrorMessage.value always receives a user-friendly string.
---
Nitpick comments:
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 34-35: The timeout ID variables saveStatusTimeoutId and
requestTimeoutId are incorrectly made reactive refs; change them to plain
mutable variables (e.g., let saveStatusTimeoutId: ReturnType<typeof setTimeout>
| null = null and let requestTimeoutId: ReturnType<typeof setTimeout> | null =
null) and update all usages that currently access .value to use the bare
variables instead; adjust references inside resetSaveStatus, saveGatewayConfig,
and the onUnmounted cleanup to use the new plain variables and clearTimeout on
them as before.
- Add guard to resetSaveStatus to prevent resetting state during in-flight requests - Change timeout variables from reactive refs to plain mutable variables for efficiency - Add "timeoutError" localization to Spanish locale - Improve error message fallback logic in saveGatewayConfig - Ensure error message keys are checked correctly with i18n - Clean up unused reactive refs for timeouts Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com>
This PR addresses several improvements and fixes for the dashboard configuration form:
PR created automatically by Jules for task 16664632965353528971 started by @yacosta738
Summary by CodeRabbit
Documentation
Bug Fixes
New Features
Chores