Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.2.4] Pause synchronization: Wrong UI in menus #5290

Closed
armaccloud opened this issue Oct 31, 2016 · 12 comments
Closed

[2.2.4] Pause synchronization: Wrong UI in menus #5290

armaccloud opened this issue Oct 31, 2016 · 12 comments
Assignees
Labels
ReadyToTest QA, please validate the fix/enhancement sev3-medium type:bug
Milestone

Comments

@armaccloud
Copy link

When the tray icon menu of the owncloud client is used to pause a synchronization, the icon changes to indicate that the client has paused the sync, however in fact the sync is still running.
Using the option to pause the sync from within the application itself does work as it should (pause the sync)

Expected behaviour

When the menu option (from the tray icon) pause synchronization is chosen, the synchronization is paused.

Actual behaviour

When the menu option (from the tray icon) pause synchronization is chosen, the synchronization continues to run.

Steps to reproduce

  1. Place a large file in a synchronized folder on the Ubuntu desktop
  2. Open nethogs and verify owncloud client is sending data
  3. Choose the option pause synchronization from the tray icon menu
  4. Verify in nethogs the data being sent doesn't drop

Server configuration

Operating system:
Unknown (ocloud.de provider)

Web server:
Unknown (ocloud.de provider)

Database:
Unknown (ocloud.de provider)

PHP version:
Unknown (ocloud.de provider)

ownCloud version:
9

Storage backend (external storage):
Unknown (ocloud.de provider)

Client configuration

Client version:
2.2.4

Operating system:
Ubuntu Desktop 16.04

OS language:
English

Installation path of client:
/usr/bin/owncloud

@guruz guruz changed the title Pause synchronization doesn't pause the synchronization [Linux] [2.2.4] Pause synchronization doesn't pause the synchronization Nov 1, 2016
@guruz guruz added the type:bug label Nov 1, 2016
@ckamm ckamm added this to the 2.3.0 milestone Nov 7, 2016
@ckamm
Copy link
Contributor

ckamm commented Nov 7, 2016

I can reproduce the problem.

@ckamm ckamm added ReadyToTest QA, please validate the fix/enhancement sev3-medium labels Nov 7, 2016
@SamuAlfageme
Copy link
Contributor

@ckamm not happening anymore (tested w/ Ubuntu 16.10 + OC 2.3.0~nightly20161110) but the menubar dropdown is still displaying the words: Syncing X MB (n minute(s) t second(s) left) it'd be NtH something pointing that synchronization is paused.

dropdown_ubuntu

@SamuAlfageme
Copy link
Contributor

And this was not only for Linux client:

sync_paused

With the fix, OS X displays Unknown status on the menubar options (see comment above regarding Linux)

cropped

@SamuAlfageme SamuAlfageme changed the title [Linux] [2.2.4] Pause synchronization doesn't pause the synchronization [2.2.4] Pause synchronization doesn't pause the synchronization Nov 10, 2016
@SamuAlfageme SamuAlfageme removed the ReadyToTest QA, please validate the fix/enhancement label Nov 10, 2016
@ckamm
Copy link
Contributor

ckamm commented Nov 14, 2016

@SamuAlfageme I'm a bit confused about how this ticket's focus changed. In the future, feel free to close and make a new ticket for a follow-up issue.

So, the remaining problem in the "syncing" or "Unknown status" message in the context menu?

@SamuAlfageme
Copy link
Contributor

@ckamm I removed the bug tag because I feel like the critical part of the issue was solved (pausing the sync now does pause the sync) and the [LINUX] part from the title as it doesn't involve only Linux OS.

And yes, the only remaining issue is to clarify in the context menu which is the state in which the client is left after pausing the sync cause it's kind of confusing. I just commented in here as I guessed it's somehow closely related to this issue.

@guruz guruz removed this from the 2.3.0 milestone Nov 15, 2016
@guruz guruz changed the title [2.2.4] Pause synchronization doesn't pause the synchronization [2.2.4] Pause synchronization: Wrong UI in menus Nov 15, 2016
ckamm added a commit that referenced this issue Nov 18, 2016
Some listeners detect whether a sync is finished by checking
for isUpdatingEstimates and completedFiles >= totalFiles. But
if a sync didn't transfer any files we never sent signal
with these values. Now we do.
@ckamm ckamm added the ReadyToTest QA, please validate the fix/enhancement label Nov 18, 2016
@ckamm
Copy link
Contributor

ckamm commented Nov 18, 2016

@SamuAlfageme I've done a small change with which I can't reproduce the problem anymore. It now says "Up to date" which is also wrong, but that state display is wrong a lot. I'll make a separate ticket about it: #5321

@SamuAlfageme
Copy link
Contributor

@ckamm I was testing this again with multiple accounts and the Pause all synchronization option and it seems to be reproducing when more than one account is being synced at the same time (as one account is "waiting" for the other to be done downloading) it just pauses the active and causes the other to start downloading.

@ckamm
Copy link
Contributor

ckamm commented Nov 22, 2016

@SamuAlfageme You're saying that you clicked "Pause all synchronization" and the current sync gets aborted and one that was in the queue gets started? That shouldn't happen, but I'll try to reproduce.

@ckamm
Copy link
Contributor

ckamm commented Nov 22, 2016

@SamuAlfageme This is an actual bug in the folder scheduling logic, great catch! :)

ckamm added a commit that referenced this issue Nov 22, 2016
Previously the last folder in the queue was scheduled, regardless
of whether it was paused or not.
@SamuAlfageme
Copy link
Contributor

@ckamm working as expected now 👌. One small detail to consider if relevant is that the account that was being synced (not the one "waiting") is left in SyncResult::Success estate when pausing the download; while the other one appears "Paused".

screen shot 2016-11-29 at 09 32 04

@ckamm
Copy link
Contributor

ckamm commented Nov 29, 2016

@SamuAlfageme I addressed that issue.

@SamuAlfageme SamuAlfageme added this to the 2.3.0 milestone Nov 29, 2016
@SamuAlfageme
Copy link
Contributor

@ckamm 👍 working like a charm! Found #5341 while testing this and could be related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReadyToTest QA, please validate the fix/enhancement sev3-medium type:bug
Projects
None yet
Development

No branches or pull requests

4 participants