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

Indirect share info now visible in favorite and other file lists #3135

Merged
merged 1 commit into from
Mar 11, 2020

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Mar 5, 2020

Description

When open the share panel of other flat file lists like the favorites,
the collaborators list and link list are now showing the same entries like
in the "All files" list, which includes indirect shares (via) that were
previously missing.

Related Issue

Fixes #3040

Motivation and Context

How Has This Been Tested?

  • TEST: favorite list: indirect collaborators appearing
  • TEST: favorite list: indirect links appearing
  • TEST: shared with other list: indirect collaborators appearing
  • TEST: shared with other list: indirect links appearing
  • TEST: shared with me list: indirect collaborators appearing => no via possible here
  • TEST: shared with me list: indirect links appearing => no via possible here
  • acceptance tests

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • TODO: fix remaining issues (see tests section)
  • TODO: add acceptance tests

@PVince81
Copy link
Contributor Author

PVince81 commented Mar 5, 2020

I had to change the logic a bit to exclude the current folder, because whenever such info was cached it would also show in "via" despite being the same folder.

Share indicators broken:

  • tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature:430
  • tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature:695
  • ...

@PVince81
Copy link
Contributor Author

PVince81 commented Mar 5, 2020

really weird. it seems that the simple act of loading the shares tree within the collaborator list is somehow breaking the automatic update of the share indicator

@PVince81
Copy link
Contributor Author

PVince81 commented Mar 6, 2020

  • ⚠️ share indicators apparently broken already on Phoenix master: in the folder "alice2bob2charlie+link (IUP, OUP, OLP)" only indirect indicators appear. When opening share panel for two of the entries, the direct indicators properly appear: https://github.com/owncloud/phoenix/issues/3160

@PVince81
Copy link
Contributor Author

PVince81 commented Mar 6, 2020

ok, I guess the failure is not related to that OC 10 bug.

@PVince81
Copy link
Contributor Author

PVince81 commented Mar 6, 2020

I've manually tested the failing case and it looks like the sharedTree always contains the old state.
For example if at first we have collaborators, the indirect icons are shown when entering the folder.
Then if for that folder I remove all collaborators then enter the folder, it still shows the old indicators.

So something is fishy regarding updating the sharesTree from the panel.

@PVince81
Copy link
Contributor Author

PVince81 commented Mar 6, 2020

There is no code for explicitly changing sharesTree... So this means the reason it worked so far on master is only because the information was not cached yet, so exploring into the folder would load the information from scratch.

Now in this PR we load it in advance and pre-cache it. So either we need to dynamically adjust it there or remove and reload the matching sharesTree entries. Forcing a reload feels a bit overkill and heavy, but so does updating the sharesTree structure...

@PVince81
Copy link
Contributor Author

PVince81 commented Mar 6, 2020

Problem solved: the new loadSharesTree was loading the tree starting with the current path but it should actually start with the parent one. This prevents caching of the current path which causes the side effects described above. With the fix in place, the existing cache invalidation already works fine for making sure the displayed info is up to date.

@PVince81
Copy link
Contributor Author

Added acceptance tests, rebased.

@PVince81 PVince81 marked this pull request as ready for review March 10, 2020 10:49
@LukasHirt LukasHirt self-requested a review March 10, 2020 10:50
@PVince81
Copy link
Contributor Author

test failure reproducible consistently locally: tests/acceptance/features/webUICreateFilesFolders/createFolderEdgeCases.feature:41

the empty message does appear, but it appears too late when the test already stopped waiting :-S

@PVince81
Copy link
Contributor Author

ah, there's a delay between the time where the progress bar disappears and the empty message appears, and the test relies on this.

@PVince81
Copy link
Contributor Author

I'll need to fix $_loadingFolder. Apparently some code outside is also relying on the global "loadingFolder" so I can't put the new logic just in AllFilesList. Have to put it on global scope.

@PVince81
Copy link
Contributor Author

I've fixed the issue by moving the "is sidebar open" condition to the getter by checking if there's a selected file. Not too nice but does the job for now until we rework the progress bars.

@PVince81 PVince81 requested a review from LukasHirt March 10, 2020 13:55
@PVince81
Copy link
Contributor Author

seems I broke other related stuff now :-(

When open the share panel of other flat file lists like the favorites,
the collaborators list and link list are now showing the same entries like
in the "All files" list, which includes indirect shares (via) that were
previously missing.
@PVince81
Copy link
Contributor Author

apparently an additional "s" has creeped in into the new highlightedFile check... fixed now and the test passes locally.

@PVince81 PVince81 merged commit 0a0b743 into master Mar 11, 2020
@delete-merged-branch delete-merged-branch bot deleted the share-owner-via-flat-lists branch March 11, 2020 08:13
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.

Indirect share info and owner missing in flat lists
2 participants