feat(cloud): account toolbar control and device sign-in (3.4.0) - #709
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>
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>
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>
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>
Raise project version to 3.4.0 and sync pinned doc references. Add unit tests for CloudAccountMenuButton (auth refresh, menu actions, display-name fallbacks, signals) and CloudCredentialStore (empty token, corrupt session file, migration no-op) to improve coverage on new cloud UI code. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughVersion 3.4.0 release with Sentry observability instrumentation for cloud account menu interactions. CloudAccountMenuButton gains breadcrumb logging for menu display and action triggers. Comprehensive test suite validates initials, menu state, signal emission, and refresh behavior. CloudCredentialStore tests expanded for session persistence. MainWindow refactored to wire button signals and async sign-in polling. CMake test configuration updated. ChangesVersion 3.4.0 Release with Cloud Account Observability
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)src/CloudAccountMenuButton_test.cppIn file included from src/CloudAccountMenuButton_test.cpp:1: ... [truncated 1138 characters] ... lib/clang/18/include" src/CloudAccountMenuButton.cppIn file included from src/CloudAccountMenuButton.cpp:1: ... [truncated 1123 characters] ... tall/lib/clang/18/include" src/CloudCredentialStore_test.cppIn file included from src/CloudCredentialStore_test.cpp:1: ... [truncated 1131 characters] ... l/lib/clang/18/include"
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83eb78247c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pollTimer.start(intervalMs); | ||
| pollOnce(); | ||
| prompt.exec(); |
There was a problem hiding this comment.
If the very first pollOnce() returns anything other than authorization_pending/slow_down (for example a transient network error from QtMeshCloudClient::pollDeviceToken, or an immediate failure from the service), failSignIn() calls prompt.reject() before prompt.exec() has started. QDialog::exec() then initializes and shows the dialog anyway, with the timer already stopped, leaving users stuck in a sign-in dialog that will never poll again after the warning. Start the timer/poll from the dialog's event loop (e.g. a zero-delay QTimer::singleShot) or skip exec() when the pre-exec poll already completed.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@README.md`:
- Around line 54-57: Update the stale version strings in the README so all
examples and descriptive text match the bumped tag 3.4.0: replace occurrences of
"currently **3.1.0**" and "pin :**3.1.0**" (and any other mentions within the
ranges cited: 82-86, 89-95, 97-104, 106-113) with "3.4.0" (or "currently
**3.4.0**"/"pin :**3.4.0**" to match existing formatting) so the documented
example using uses: fernandotonon/QtMeshEditor@3.4.0 and image-tag: "3.4.0" is
consistent throughout.
In `@src/CloudAccountMenuButton.cpp`:
- Around line 238-255: The menu actions in CloudAccountMenuButton
(m_openProjectsAction, m_uploadAction, m_signOutAction, m_signInAction) must
emit their own telemetry breadcrumbs; move/insert
SentryReporter::addBreadcrumb(...) calls directly next to each
QAction::triggered connect in the CloudAccountMenuButton constructor so the
widget self-reports when openProjectsRequested, uploadFilesRequested,
signOutRequested and signInRequested are triggered, using a clear
category/message per action and ensuring the breadcrumb call runs before
emitting the signal.
In `@src/CloudCredentialStore_test.cpp`:
- Around line 74-80: The test writes a corrupt session file at sessionFilePath()
but doesn't create the parent directory first, causing file.open(...) to fail on
a clean worker; before opening the QFile at path, create the parent directory
like CloudCredentialStore::writeFallbackFile() does (use QFileInfo or QDir to
get the directory for sessionFilePath() and call QDir::mkpath(...) to ensure it
exists) so the subsequent QFile::open and write succeed and the corrupt-file
path is exercised.
In `@src/mainwindow.cpp`:
- Around line 2296-2326: The cloud toolbar entry points add breadcrumbs but
follow-on actions are missing instrumentation; update
MainWindow::setupCloudAccountStatusControl and the related flows to add
SentryReporter::addBreadcrumb calls for all follow-on user actions and failures:
add a breadcrumb on QDesktopServices::openUrl failure in the Open My Projects
handler (use a message like "Cloud toolbar: Open My Projects failed"), and
ensure signInToQtMeshCloud(), signOutOfQtMeshCloud(), and
uploadFilesToQtMeshCloud() emit breadcrumbs for their subsequent user actions
(e.g., device-code flow steps); instrument the device-code dialog callbacks
(Copy Code, Open Browser, Cancel/Close) to call SentryReporter::addBreadcrumb
with descriptive messages (e.g., "Cloud device-code: Copy Code", "Cloud
device-code: Open Browser", "Cloud device-code: Cancel") and keep the existing
m_cloudAccountControl/menu aboutToShow breadcrumb.
In `@tests/CMakeLists.txt`:
- Line 68: The tests for CloudAccountMenuButton aren't registered with CTest:
include the test source src/CloudAccountMenuButton_test.cpp in the call to
create_test_executable(...) (alongside the existing support source
CloudAccountMenuButton.cpp) and ensure that the resulting target is passed to
gtest_discover_tests(...) so the 12 tests are built and executed in CI; update
the tests/CMakeLists.txt entries that reference CloudAccountMenuButton.cpp, the
create_test_executable(...) invocation, and the gtest_discover_tests(...) call
accordingly.
🪄 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: b9092085-cf28-4f91-b22c-8caf36f978ac
⛔ Files ignored due to path filters (1)
resources/cloud_account_user.svgis excluded by!**/*.svg
📒 Files selected for processing (14)
CMakeLists.txtREADME.mdresources/resource.qrcsrc/CMakeLists.txtsrc/CloudAccountMenuButton.cppsrc/CloudAccountMenuButton.hsrc/CloudAccountMenuButton_test.cppsrc/CloudCredentialStore.cppsrc/CloudCredentialStore_test.cppsrc/mainwindow.cppsrc/mainwindow.hsrc/mainwindow_test.cpptests/CMakeLists.txtwebsite/src/hooks/useQtmeshActionRef.js
| void MainWindow::setupCloudAccountStatusControl() | ||
| { | ||
| m_cloudAccountControl = new CloudAccountMenuButton(this); | ||
|
|
||
| connect(m_cloudAccountControl, &CloudAccountMenuButton::signInRequested, this, [this]() { | ||
| SentryReporter::addBreadcrumb(QStringLiteral("ui.action"), | ||
| QStringLiteral("Cloud toolbar: Sign in")); | ||
| signInToQtMeshCloud(); | ||
| }); | ||
| connect(m_cloudAccountControl, &CloudAccountMenuButton::signOutRequested, this, [this]() { | ||
| SentryReporter::addBreadcrumb(QStringLiteral("ui.action"), | ||
| QStringLiteral("Cloud toolbar: Sign out")); | ||
| signOutOfQtMeshCloud(); | ||
| }); | ||
| connect(m_cloudAccountControl, &CloudAccountMenuButton::uploadFilesRequested, this, [this]() { | ||
| SentryReporter::addBreadcrumb(QStringLiteral("ui.action"), | ||
| QStringLiteral("Cloud toolbar: Upload Files")); | ||
| uploadFilesToQtMeshCloud(); | ||
| }); | ||
| connect(m_cloudAccountControl, &CloudAccountMenuButton::openProjectsRequested, this, [this]() { | ||
| SentryReporter::addBreadcrumb(QStringLiteral("ui.action"), | ||
| QStringLiteral("Cloud toolbar: Open My Projects")); | ||
| if (!QDesktopServices::openUrl(QUrl(QStringLiteral(QTMESH_CLOUD_WEB_URL)))) { | ||
| QMessageBox::warning(this, tr("QtMesh Cloud"), | ||
| tr("Could not open QtMesh Cloud in your browser.")); | ||
| } | ||
| }); | ||
| connect(m_cloudAccountControl->menu(), &QMenu::aboutToShow, this, []() { | ||
| SentryReporter::addBreadcrumb(QStringLiteral("ui.action"), | ||
| QStringLiteral("Cloud toolbar menu opened")); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Breadcrumb the rest of the new cloud sign-in flow.
The toolbar entry points are tracked, but the new follow-on actions are not: failed openUrl() here, Copy Code, Open Browser, and cancel/close in the device-code dialog all bypass ui.action breadcrumbs. That leaves the new cloud UX under-instrumented even though it is now a primary user flow.
As per coding guidelines, **/*.cpp: "All user-facing actions and significant operations must be tracked with SentryReporter::addBreadcrumb(category, message)."
Also applies to: 2460-2485
🤖 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/mainwindow.cpp` around lines 2296 - 2326, The cloud toolbar entry points
add breadcrumbs but follow-on actions are missing instrumentation; update
MainWindow::setupCloudAccountStatusControl and the related flows to add
SentryReporter::addBreadcrumb calls for all follow-on user actions and failures:
add a breadcrumb on QDesktopServices::openUrl failure in the Open My Projects
handler (use a message like "Cloud toolbar: Open My Projects failed"), and
ensure signInToQtMeshCloud(), signOutOfQtMeshCloud(), and
uploadFilesToQtMeshCloud() emit breadcrumbs for their subsequent user actions
(e.g., device-code flow steps); instrument the device-code dialog callbacks
(Copy Code, Open Browser, Cancel/Close) to call SentryReporter::addBreadcrumb
with descriptive messages (e.g., "Cloud device-code: Copy Code", "Cloud
device-code: Open Browser", "Cloud device-code: Cancel") and keep the existing
m_cloudAccountControl/menu aboutToShow breadcrumb.
Resolve test file conflicts keeping expanded cloud coverage; defer sign-in polling until the dialog is shown; move Sentry breadcrumbs into the widget; fix README version pins and register CloudAccountMenuButton tests in CTest. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/CMakeLists.txt (1)
566-572:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
CloudAccountMenuButton_testto the aggregate target dependencies.
run_all_materialeditorthml_testsstill doesn’t depend onCloudAccountMenuButton_test, so the new test binary may not be built beforectest --verboseruns.Suggested patch
add_custom_target(run_all_materialeditorthml_tests COMMAND ctest --verbose DEPENDS MaterialEditorQML_qml_test_runner + $<$<TARGET_EXISTS:CloudAccountMenuButton_test>:CloudAccountMenuButton_test> $<$<TARGET_EXISTS:MaterialEditorQML_test>:MaterialEditorQML_test> $<$<TARGET_EXISTS:MaterialEditorQML_qml_test>:MaterialEditorQML_qml_test> $<$<TARGET_EXISTS:MaterialEditorQML_perf_test>:MaterialEditorQML_perf_test> COMMENT "Running all MaterialEditorQML comprehensive tests" )🤖 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 `@tests/CMakeLists.txt` around lines 566 - 572, The aggregate CMake target run_all_materialeditorthml_tests is missing the new CloudAccountMenuButton_test dependency; update the add_custom_target(...) DEPENDS list for run_all_materialeditorthml_tests to include CloudAccountMenuButton_test so the test binary is built before running ctest --verbose, ensuring you add CloudAccountMenuButton_test alongside the existing dependencies (MaterialEditorQML_test, MaterialEditorQML_qml_test, MaterialEditorQML_perf_test) in the add_custom_target definition.
🤖 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.
Duplicate comments:
In `@tests/CMakeLists.txt`:
- Around line 566-572: The aggregate CMake target
run_all_materialeditorthml_tests is missing the new CloudAccountMenuButton_test
dependency; update the add_custom_target(...) DEPENDS list for
run_all_materialeditorthml_tests to include CloudAccountMenuButton_test so the
test binary is built before running ctest --verbose, ensuring you add
CloudAccountMenuButton_test alongside the existing dependencies
(MaterialEditorQML_test, MaterialEditorQML_qml_test,
MaterialEditorQML_perf_test) in the add_custom_target definition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e802da4f-21d9-4689-a4cc-ed609dabe70e
📒 Files selected for processing (6)
README.mdsrc/CloudAccountMenuButton.cppsrc/CloudAccountMenuButton_test.cppsrc/CloudCredentialStore_test.cppsrc/mainwindow.cpptests/CMakeLists.txt
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- src/CloudAccountMenuButton.cpp
- src/CloudAccountMenuButton_test.cpp
|
1 participant
Summary
CloudAccountMenuButtonat the bottom of the left objects toolbar (avatar, status badge, structured popup menu).UI / UX
CloudAccountMenuButtonwidget for future placement (e.g. top toolbar).Technical
CloudCredentialStoreuses isolated fallback-file storage whenQCoreApplication::organizationName()isQtMeshEditorTestsso unit tests do not touch the OS keychain.ui.actionbreadcrumbs on cloud toolbar actions; browser open failures surface a warning.Test plan
./build_local/bin/UnitTests --gtest_filter="MainWindowTest.CloudAccount*"./build_local/bin/UnitTests --gtest_filter="CloudAccountMenuButtonTest.*"./build_local/bin/UnitTests --gtest_filter="CloudCredentialStoreTest.*"Made with Cursor
Summary by CodeRabbit
Release Notes - Version 3.4.0
New Features
Improvements
Tests