Skip to content

fix(tray): restore pause and resume sync action - #10529

Open
Rello wants to merge 2 commits into
masterfrom
bugfix/pauseSync
Open

fix(tray): restore pause and resume sync action#10529
Rello wants to merge 2 commits into
masterfrom
bugfix/pauseSync

Conversation

@Rello

@Rello Rello commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator
  • Restores the global Pause sync for all / Resume sync for all action lost during the tray-menu migration.
  • Adds the action to both Qt and macOS root tray menus, between Add account and Settings.
  • Centralizes availability and behavior in Systray.
  • Hides the action when no classic sync folders exist, including macOS File Provider mode.
  • Preserves the previous behavior across all configured accounts.
  • Adds regression coverage for visibility, labels, and pause/resume behavior across two accounts.

Assisted-by: Codex:GPT-5

Bildschirmfoto 2026-08-03 um 23 33 33 Bildschirmfoto 2026-08-03 um 23 33 48

@kra-mo
this was a feature in the old user menu and was forgotten during migration.
I think we need to add it. Any objections?

- Restores the global Pause sync for all / Resume sync for all action lost during the tray-menu migration.
- Adds the action to both Qt and macOS root tray menus, between Add account and Settings.
- Centralizes availability and behavior in Systray.
- Hides the action when no classic sync folders exist, including macOS File Provider mode.
- Preserves the previous behavior across all configured accounts.
- Adds regression coverage for visibility, labels, and pause/resume behavior across two accounts.

Assisted-by: Codex:GPT-5
Signed-off-by: Rello <github@scherello.de>
@Rello
Rello force-pushed the bugfix/pauseSync branch from b8238c8 to e8e7a36 Compare August 3, 2026 21:49

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8e7a36ef2

ℹ️ 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".

Comment thread test/testsystraysynccontrol.cpp Outdated
Comment on lines +124 to +125
#else
systray->toggleSyncPaused();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Test the macOS tray control through its UI action

In the macOS branch, the regression test calls toggleSyncPaused() directly and never constructs NCTrayPopup or activates the new NCActionRow, so the test still passes if the action is omitted, mislabeled, or disconnected in nctraypopup.mm. Add a test seam that verifies the macOS popup's visibility, title, and action callback instead of bypassing the platform UI wiring.

AGENTS.md reference: AGENTS.md:L205-L209

Useful? React with 👍 / 👎.

@claucambra claucambra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Production code looks good, just some small nitpicks and a more substantial comment on the test

Comment thread src/gui/macOS/trayaccountpopup/nctraypopup.mm Outdated
Comment thread test/testsystraysynccontrol.cpp Outdated
Comment thread test/testsystraysynccontrol.cpp Outdated
Comment on lines +49 to +55
#ifndef Q_OS_MACOS
static QAction *syncControlAction(QMenu &menu, Systray *systray)
{
setupQtTrayContextMenu(&menu, systray);
return menu.findChild<QAction *>(QStringLiteral("traySyncControlAction"));
}
#endif

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This test uses a lot of preprocessor conditions which makes this test difficult to read.
I think a better approach would be to split the macOS and Qt tests into two separate tests that are compiled/run conditionally depending on the host system

If there is common code in e.g. setup/teardown routines this can be extracted into a separate helper file

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@claucambra can you recheck? the tests were separated. added more codebase but might be more helpful for the future

Assisted-by: Codex:GPT-5
Signed-off-by: Rello <github@scherello.de>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Artifact containing the AppImage: nextcloud-appimage-pr-10529.zip

Digest: sha256:44a612aade22440a5f7a156d2509aedf9bdb61fc93bfe63376e170649c65dd21

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

@sonarqubecloud

sonarqubecloud Bot commented Aug 4, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
5 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@camilasan camilasan added this to the 34.0.2 milestone Aug 5, 2026
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.

3 participants