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

[Overlays] Client folder error status not shown in higher-level icon status #3634

Closed
phil-davis opened this issue Aug 17, 2015 · 18 comments
Closed

Comments

@phil-davis
Copy link
Contributor

Expected behaviour

If a folder has a problem syncing from client to server then the problem status will be shown in the icons of parent folders and in the overall sync status icon in the icon box.

Actual behaviour

If a folder has a problem syncing from client to server it (correctly) shows a little red "x" in the folder icon.
But any folders above it show green,
The overall client status icon shows green.

If the problem folder is several folders down in the folder tree then the user is unlikely to notice it and thus will not know to investigate.

It would be nice to show some non-green status for the folders above and the overall status icon.

Steps to reproduce

  1. On the client create FolderA.
  2. Let it sync up to the server.
  3. From the folders-to-sync dropdown, unselect FolderA and apply.
  4. A sync starts, FolderA is deleted on the client - good - and remains on the server - good.
  5. Create another FolderA in the same place.
    FolderA is shown with red "x" icon - good.
    Activity log says: Ignored because of the "choose what to sync" blacklist
    That is also reasonable, because there is already FolderA on the server with some (presumably different) content, and it is not being synced to this client.

But the overall sync status is green.

There is the same sort of behaviour when FolderA is inside some other folder/s. The higher level folders all show green status.

Server configuration

ownCloud version: 8.0

Client configuration

Client version: Version 2.0.0-nightly20150817 (build 5403)

Operating system: Windows 10

OS language: English (US)

Installation path of client: default

@guruz guruz added this to the 2.0 - Multi-account milestone Aug 17, 2015
@guruz guruz added the type:bug label Aug 17, 2015
@MTRichards MTRichards modified the milestones: 2.0.1-next, 2.0 - Multi-account Aug 18, 2015
@MTRichards
Copy link

@guruz are you working on this now for 2.0, or does it slide to 2.0.1?

@MTRichards MTRichards modified the milestones: 2.0 - Multi-account, 2.0.1-next Aug 18, 2015
@jturcotte
Copy link
Member

I haven't investigated it yet but unless this is a regression from 1.8 (it's not, is it?) then we shouldn't put pressure on 2.0.0. If this requires bigger changes it might have to go in 2.1.

See also #3171 for a glimpse of how error statuses aresn't preserved as long as they should. The moment that a file ends up in the black list, its status becomes invisible to the user. My knowledge of that area isn't quite good though, @ogoffart could prove me wrong if I am.

@jturcotte
Copy link
Member

Another question is: Is it correct to propagate anything else than the SYNC status to parent folders?

If I have the structure:
A
|- B
|- C

And that C has an ignored or error status, should A also be marked as ignored/error? A isn't ignored completely, A isn't completely in error either. For SYNC, however we can say that "A isn't up-to-date", implied by C not being up-to-date.

I agree that there should be a way to see the error more easily than by inspecting the activity monitor, but I'm not sure if the status of parent directories is the right place to keep that information until the user decides to fix the situation (if he even knows how to fix it). It might handicap his ability to quickly judge if other files are up-to-date. Let me know how you see it.

@MTRichards
Copy link

I thought we said that A should have a yellow, saying that it was non fatal, but there were problems in the directory below it. If the entire directory in C failed, C is red, otherwise if it is a partial fail C is yellow too, and that gets propagated up. I have to find the selective sync notes as it outlines it.

@MTRichards
Copy link

Found it:
#1818
That has the details on the design point for folders, including errors.

@phil-davis
Copy link
Contributor Author

Those requirements look good, with the addition of the "bubbling up with yellow" to parent folders that have red somewhere below but not all red, as @MTRichards has commented.

@dragotin dragotin modified the milestones: 2.1-next, 2.0 - Multi-account Aug 21, 2015
@dragotin
Copy link
Contributor

Moved to 2.1. This is not exactly how it is implemented today and as a result a bigger change.

@dragotin
Copy link
Contributor

This should be done together with #3171

@guruz guruz changed the title Client folder error status not shown in higher-level icon status [Overlays] Client folder error status not shown in higher-level icon status Oct 20, 2015
@guruz guruz changed the title [Overlays] Client folder error status not shown in higher-level icon status [Windows] [Overlays] Client folder error status not shown in higher-level icon status Nov 3, 2015
@guruz guruz modified the milestones: 2.1.2, 2.1-next Nov 3, 2015
@guruz
Copy link
Contributor

guruz commented Jan 4, 2016

Related #4026

@jturcotte jturcotte modified the milestones: 2.2-next, 2.1.1-current Jan 11, 2016
@jturcotte jturcotte changed the title [Windows] [Overlays] Client folder error status not shown in higher-level icon status [Overlays] Client folder error status not shown in higher-level icon status Mar 28, 2016
jturcotte added a commit that referenced this issue Mar 28, 2016
…t folders #3634

This also remove all smartness from the SocketApi about the status
of a file and solely use info from the current and last sync.
This simplifies the logic a lot and prevents any discrepancy between
the status shown in the activity log and the one displayed on the
overlay icon of a file.

The main benefit of the additional simplicity is that we are able
to push all new status of a file reliably (including warnings for
parent folders) to properly update the icon on overlay implementations
that don't allow us invalidating the status cache, like on OS X.

Both errors and warning from the last sync are now kept in a set,
which is used to also affect parent folders of an error.

To make sure that errors don't become warning icons on a second
sync, SyncFileItem::_hasBlacklistEntry is also interpreted as an error.
This also renames StatusIgnore to StatusWarning to match this semantic.

SyncEngine::aboutToPropagate is used in favor of SyncEngine::syncItemDiscovered
since the latter is emitted before file permission warnings are set on the
SyncFileItem. SyncEngine::finished is not used since we have all the
needed information in SyncEngine::itemCompleted.
@guruz guruz added the ReadyToTest QA, please validate the fix/enhancement label Apr 6, 2016
@guruz guruz removed the ReadyToTest QA, please validate the fix/enhancement label Apr 6, 2016
@guruz
Copy link
Contributor

guruz commented Apr 6, 2016

@phil-davis Can re-test this with a recent nightly as soon as the overlay binaries are rebuilt (#4631)

@guruz
Copy link
Contributor

guruz commented Apr 7, 2016

@guruz guruz added the ReadyToTest QA, please validate the fix/enhancement label Apr 7, 2016
@guruz
Copy link
Contributor

guruz commented Apr 19, 2016

@phil-davis Still alive? :-)
There's even newer nightlies now..

@mcastroSG
Copy link

mcastroSG commented Apr 19, 2016

Hi hi:

@guruz I have made some test about this Issue using basically the same steps that @phil-davis put:

Steps executed

  1. On the client create Folder1.
  2. Inside Folder1 create Folder2
  3. Let it sync up to the server.
  4. From the folders-to-sync dropdown, unselect Folder2 and apply.
  5. A sync starts, Folder2 is deleted on the client and still is on the server.
  6. Create another Folder2 in the same place.

Results:

Folder2 is shown with yellow icon and Activity log says: Ignored because of the "choose what to sync" blacklist, but Folder1 remains with a green icon.So it seems that sync error are not getting propagated up .

MacOS
Client Version: Version 2.2.0-nightly20160419 (build 3282)
Client OS: MacOS X El Capitan 10.11.4
Server Version: 9.0.1
Server OS: Ubuntu

Unable to check it on windows due to https://github.com/owncloud/client/issues/4701

Regards

@guruz guruz removed the ReadyToTest QA, please validate the fix/enhancement label Apr 19, 2016
@jturcotte
Copy link
Member

Warning don't get propagated up, only errors. Else the root folder for example would never be green, there is always an ignored file somewhere in the tree (for example the .csync_journal db) and the user most likely won't want to change this. Errors on the other hand will usually need to be actioned by the user.

@jturcotte
Copy link
Member

Maybe the "ignored because of the choose what to sync blacklist" message should be an error though, it's not a normal use case.

@guruz
Copy link
Contributor

guruz commented Apr 22, 2016

@mcastroSG BTW, the overlays are very OS-sensitive. So in this case you should test Windows (where @phil-davis is on).
The spec that @jturcotte mentions is https://github.com/owncloud/client/pull/4704/files ..

@guruz guruz added the ReadyToTest QA, please validate the fix/enhancement label Apr 22, 2016
@mcastroSG
Copy link

Blocked due to #4737

@mcastroSG mcastroSG removed the Waiting label Apr 27, 2016
@mcastroSG
Copy link

Hi hi:

Test Case ID Test case Steps Expected behaviour Result Related Comments
1 Overlay Icons - Folder hierarchy warning 1.- Generate a warning i.e a folder that could not be synced
Overlay icon at parent folder is a "Green check"
2 Overlay Icons - Folder hierarchy error 1.- Generate an error i.e exceed quota allowed
Overlay icon at parent folder is "Yellow warning" icon
3 Overlay Icons - Folder hierarchy error solved 1.- Generate an error i.e exceed quota allowed
2.- Wait till error is returned
3.- Solve problem deleting problematic files
Overlay icon at parent folder is updated from "Yellow warning" icon to "Green Check" icon

OSX:

Client OS: OS X El capitán 10.11.4
Client Version: Version 2.2.0-nightly20160504 (build 3331)

OSW:

Client OS: W81
Client Version: Version 2.2.0-nightly20160505 (build 6054)
OSLinux

Client OS: Ubuntu 14.04 LTE
Client Version: Version 2.2.0nightly201654 (build 1587)

@mcastroSG mcastroSG removed the ReadyToTest QA, please validate the fix/enhancement label May 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants