feat(cloud): move account control to objects toolbar#700
Merged
Conversation
Replace the Cloud menu bar entry with a VS Code-style account button pinned to the bottom of the left objects toolbar, including a disabled signed-in user row in the popup menu and MainWindow unit tests. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
More reviews will be available in 9 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReplaces the top-level "Cloud" menu with a CloudAccountMenuButton avatar in the objects toolbar, adds the CloudAccountMenuButton widget and tests, integrates it via MainWindow::setupCloudAccountStatusControl(), refactors updateCloudAuthActions() to refresh the new control, and isolates credential persistence for tests. ChangesCloud Account Toolbar UI Migration
Sequence Diagram(s)sequenceDiagram
participant User
participant CloudButton as CloudAccountMenuButton
participant MainWindow
participant CredentialStore as CloudCredentialStore
User->>CloudButton: click opens dropdown
CloudButton->>MainWindow: QMenu aboutToShow -> request refresh
MainWindow->>CredentialStore: query CloudSession and QSettings
CredentialStore-->>MainWindow: CloudSession and display-name
MainWindow-->>CloudButton: update tooltip, avatar, and menu items
User->>CloudButton: select Upload / Sign in/out / Open Projects
CloudButton-->>MainWindow: emit uploadFilesRequested / signInRequested / signOutRequested / openProjectsRequested
MainWindow->>MainWindow: handle requests and add breadcrumbs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 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)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mainwindow_test.cpp`:
- Around line 378-408: The test currently calls
CloudCredentialStore::clearSession() only inside
CloudAccountMenuShowsConnectedUserAsDisabledRow, which can leave persisted
credentials if an assertion fails; move the cleanup to
MainWindowTest::TearDown() so every test always clears stored credentials after
running. Modify MainWindowTest::TearDown() to call
CloudCredentialStore::clearSession() (and any related QSettings cleanup for
AppSettingsKeys::cloudUserName() if needed) and remove or keep redundant clears
inside individual tests like CloudAccountMenuShowsConnectedUserAsDisabledRow to
ensure deterministic teardown.
In `@src/mainwindow.cpp`:
- Around line 2305-2325: The new cloud menu actions are missing Sentry
breadcrumbs and the "Open My Projects" handler ignores QDesktopServices::openUrl
failure; add SentryReporter::addBreadcrumb("ui.action", "<action name>") at the
start of each action handler (signInToQtMeshCloud, signOutOfQtMeshCloud,
uploadFilesToQtMeshCloud and the lambda used for m_cloudOpenDashboardAction) so
clicks are tracked, and change the m_cloudOpenDashboardAction lambda to capture
the QDesktopServices::openUrl(...) return value and handle false by
logging/reporting the failure (e.g. SentryReporter::addBreadcrumb or error
report and user-visible fallback) instead of failing silently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2cdf80e-7e5c-4ac5-9a22-712e92b8c877
📒 Files selected for processing (3)
src/mainwindow.cppsrc/mainwindow.hsrc/mainwindow_test.cpp
Co-authored-by: Cursor <cursoragent@cursor.com>
Clear CloudCredentialStore in MainWindowTest teardown, add ui.action Sentry breadcrumbs for cloud toolbar menu actions, and warn when the browser fails to open QtMesh Cloud. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the generic network icon with a dedicated CloudAccountMenuButton component: outline user icon when signed out, initials avatar when signed in, and a status badge for connected/offline. Restructure the popup with a non-clickable name header, main cloud actions, and account sign-in/out. Fix signed-out paint crash (nested QPainter) and remove the header from the menu when logged out so stale name text no longer bleeds through. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/CloudAccountMenuButton_test.cpp`:
- Around line 19-40: The test mutates the real CloudCredentialStore and any
QSettings-backed state; instead ensure the test uses a test-only credential
backend or a fake store and isolated QSettings namespace. Modify the test (and
code if needed) so CloudAccountMenuButton uses an injectable store: add or use
an API such as passing a FakeCloudCredentialStore into CloudAccountMenuButton
(or a CloudCredentialStore::setTestBackend / useTestNamespace) and configure
QSettings to a test-only organization/application name or prefix before calling
CloudCredentialStore::clearSession/saveSession; then call the same sequence
(show/refresh/repaint/processEvents) against the fake store so the production
credential store and real QSettings are not touched.
In `@src/CloudAccountMenuButton.cpp`:
- Around line 224-227: The subtitle text is hard-coded so the "Signed in as …"
row never appears; change initialization and updates to derive the subtitle
(m_headerSubtitleLabel) or the name label from the signed-in state and
displayName: set m_headerSubtitleLabel to either "Signed in as" (when signedIn
is true) or "QtMesh Cloud account" (when not signed in) in the constructor and
ensure updateHeader() also updates m_headerSubtitleLabel based on the
signedIn/displayName values; also apply the same change to the second header
block referenced around the other label pair (lines ~257-263) so both header
subtitle/name labels reflect the connected state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23ec3e04-38e9-48be-8df7-957e2046d6ee
⛔ Files ignored due to path filters (1)
resources/cloud_account_user.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
resources/resource.qrcsrc/CMakeLists.txtsrc/CloudAccountMenuButton.cppsrc/CloudAccountMenuButton.hsrc/CloudAccountMenuButton_test.cppsrc/mainwindow.cppsrc/mainwindow.hsrc/mainwindow_test.cpptests/CMakeLists.txt
✅ Files skipped from review due to trivial changes (1)
- resources/resource.qrc
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mainwindow_test.cpp
Comment on lines
+224
to
+227
| m_headerSubtitleLabel = new QLabel(tr("QtMesh Cloud account"), m_headerWidget); | ||
| m_headerSubtitleLabel->setObjectName(QStringLiteral("cloudAccountMenuHeaderSubtitle")); | ||
| m_headerSubtitleLabel->setStyleSheet(QStringLiteral( | ||
| "color: #9a9a9a; font-size: 11px; background: transparent;")); |
There was a problem hiding this comment.
Use signed-in header copy here.
The menu never renders the promised disabled Signed in as … row because the subtitle stays hard-coded to QtMesh Cloud account and updateHeader() only swaps the name label. Please derive one of these labels from signedIn/displayName so the connected state is explicit.
Also applies to: 257-263
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/CloudAccountMenuButton.cpp` around lines 224 - 227, The subtitle text is
hard-coded so the "Signed in as …" row never appears; change initialization and
updates to derive the subtitle (m_headerSubtitleLabel) or the name label from
the signed-in state and displayName: set m_headerSubtitleLabel to either "Signed
in as" (when signedIn is true) or "QtMesh Cloud account" (when not signed in) in
the constructor and ensure updateHeader() also updates m_headerSubtitleLabel
based on the signedIn/displayName values; also apply the same change to the
second header block referenced around the other label pair (lines ~257-263) so
both header subtitle/name labels reflect the connected state.
Route credential storage to an isolated fallback file when tests set organization to QtMeshEditorTests so unit tests do not touch the OS keychain. Use gtest fixtures with isolated QSettings in cloud tests, show "Signed in to QtMesh Cloud" in the menu header subtitle, and centralize MainWindow cloud-session cleanup in TearDown. Co-authored-by: Cursor <cursoragent@cursor.com>
Consolidate duplicate fallback branches in CloudCredentialStore, apply C++17 init-statements, const-correctness, and auto per SonarCloud rules for the account menu changes. Co-authored-by: Cursor <cursoragent@cursor.com>
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 (1)
src/CloudCredentialStore_test.cpp (1)
51-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert that
cloudTokenExpiresAt()is removed too.
CloudCredentialStore::migrateLegacySettingsIfNeeded()clears the expiry key along with token and email, but this test only checks two of the three legacy fields. A partial migration would still pass here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/CloudCredentialStore_test.cpp` around lines 51 - 67, Update the test MigratesLegacyPlaintextSettings to also verify the expiry key is removed: after calling CloudCredentialStore::migrateLegacySettingsIfNeeded() add an assertion that the legacy expiry entry (AppSettingsKeys::cloudTokenExpiresAt()) is gone (e.g. settings.value(AppSettingsKeys::cloudTokenExpiresAt()).isNull() or equivalent) so the test fails for partial migrations; reference the test name and CloudCredentialStore::migrateLegacySettingsIfNeeded() to locate where to add the check.
🧹 Nitpick comments (1)
src/CloudAccountMenuButton_test.cpp (1)
47-72: ⚡ Quick winAssert the signed-out menu cleanup too.
After Line 68 this only exercises
refresh()/repaint(). It never verifies that the account header is actually gone once the session is cleared, so the "ghost header text after sign-out" regression could come back without failing this test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/CloudAccountMenuButton_test.cpp` around lines 47 - 72, The test currently clears the session with CloudCredentialStore::clearSession() and calls button.refresh()/button.repaint() but does not verify the UI cleaned up; after calling clearSession() and refreshing/repainting the CloudAccountMenuButton, assert that the header QLabel with objectName "cloudAccountMenuHeaderSubtitle" no longer exists or is not visible—e.g. call button.findChild<QLabel*>("cloudAccountMenuHeaderSubtitle") and EXPECT_EQ(it, nullptr) or EXPECT_FALSE(subtitle->isVisible())—so the CloudAccountMenuButtonTest ensures the signed-out header is removed after CloudCredentialStore::clearSession().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/CloudCredentialStore_test.cpp`:
- Around line 51-67: Update the test MigratesLegacyPlaintextSettings to also
verify the expiry key is removed: after calling
CloudCredentialStore::migrateLegacySettingsIfNeeded() add an assertion that the
legacy expiry entry (AppSettingsKeys::cloudTokenExpiresAt()) is gone (e.g.
settings.value(AppSettingsKeys::cloudTokenExpiresAt()).isNull() or equivalent)
so the test fails for partial migrations; reference the test name and
CloudCredentialStore::migrateLegacySettingsIfNeeded() to locate where to add the
check.
---
Nitpick comments:
In `@src/CloudAccountMenuButton_test.cpp`:
- Around line 47-72: The test currently clears the session with
CloudCredentialStore::clearSession() and calls button.refresh()/button.repaint()
but does not verify the UI cleaned up; after calling clearSession() and
refreshing/repainting the CloudAccountMenuButton, assert that the header QLabel
with objectName "cloudAccountMenuHeaderSubtitle" no longer exists or is not
visible—e.g. call button.findChild<QLabel*>("cloudAccountMenuHeaderSubtitle")
and EXPECT_EQ(it, nullptr) or EXPECT_FALSE(subtitle->isVisible())—so the
CloudAccountMenuButtonTest ensures the signed-out header is removed after
CloudCredentialStore::clearSession().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3f67ded-8fef-48b2-9eea-cb01231647fd
📒 Files selected for processing (6)
src/CloudAccountMenuButton.cppsrc/CloudAccountMenuButton_test.cppsrc/CloudCredentialStore.cppsrc/CloudCredentialStore_test.cppsrc/mainwindow.cppsrc/mainwindow_test.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
- src/mainwindow_test.cpp
- src/CloudAccountMenuButton.cpp
- src/mainwindow.cpp
Poll for browser approval while the sign-in dialog stays open so copy/paste approval works without clicking Open Browser. Show status text without repeating the user code. Tests: assert legacy cloudTokenExpiresAt is cleared on migration, and that the signed-out menu no longer lists the account header widget action. Co-authored-by: Cursor <cursoragent@cursor.com>
|
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.
Summary
CloudAccountMenuButton: outline user/cloud icon when signed out, initials avatar + connected badge when signed in.QPainterin custom button paint) and ghost header text after sign-out (remove header actions from menu instead of only hiding).Test plan
./build_local/bin/UnitTests --gtest_filter="MainWindowTest.CloudAccount*"./build_local/bin/UnitTests --gtest_filter="MainWindowTest.CloudMenuIsNotOnMenuBar"./build_local/bin/UnitTests --gtest_filter="CloudAccountMenuButtonTest.*"Summary by CodeRabbit
New Features
UI Changes
Bug Fixes
Tests