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.

Sign up for GitHub

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

Jump to bottom

Share indicator for direct and indirect shares in file list #2877

Merged
merged 2 commits into from
Jan 23, 2020
Merged

Share indicator for direct and indirect shares in file list #2877

merged 2 commits into from
Jan 23, 2020

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Jan 14, 2020 •
edited
Loading

Steps to view:

  1. Setup a new OC 10.4 server
  2. Run the script from Add integration section and location/resource picker feature product#40
  3. Open Phoenix
  4. Login as "bob" with password "test"
  5. Browse the folders and enjoy the share indicators

image

Ref: owncloud/owncloud-design-system#607
For story: https://github.com/owncloud/enterprise/issues/3701

Fixes #2060
Fixes #2894

@pmaier1 FYI

@PVince81 PVince81 self-assigned this Jan 14, 2020
@PVince81 PVince81 mentioned this pull request Jan 14, 2020
Merged
3 tasks
@PVince81 PVince81 changed the title [No merge] Live mock up share indicators [No merge] Live mock up share indicators and other Jan 16, 2020
@PVince81
Copy link
Contributor Author

Share owner appearing now in collaborators list:
IUP or IUC case:
image

IUC, OUP case:
image

note: also achieved through dirty hack, just for showcasing

@PVince81
Copy link
Contributor Author

I've already added crude via links, here's a good example with both inherited resource owner and inherited share from parent resource:
image

@PVince81
Copy link
Contributor Author

PVince81 commented Jan 17, 2020 •
edited
Loading

This was referenced Jan 20, 2020
Closed
Closed
@PVince81 PVince81 changed the title [No merge] Live mock up share indicators and other Direct and indirect share indicators, share owner and collaborators Jan 20, 2020
@PVince81
Copy link
Contributor Author

I'd like to reuse this PR and fill it with actual data once available.

@PVince81
Copy link
Contributor Author

PVince81 commented Jan 20, 2020 •
edited
Loading

Added share indicators for indirect outgoing shares.

TODOs:

  • python script issue: IUP+OLP is missing link share!

  • need to query webdav props on parents as well for oc:share-types for inheritent incoming shares

  • query incoming shares on all parents and adjust indicators to consider the data for inherited shares

  • share indicator event after creating/deleting link for direct share ? might need to reload just the row ?

  • update ODS for icon colors: Update all libraries #2901

  • enable caching:

    • only request if not in sharesTree yet
    • cache clearing

  • let's finish just the share indicators and defer (remove) the rest to separate PRs

  • tests for indicators (see OC 10 ones if reusable)

  • changelog entry

@PVince81
Copy link
Contributor Author

In OC 10 we don't load incoming shares for all parents, but only the Webdav share-types. While this might be enough to display indicators, it won't be enough for showing the inherited resource/share owners. So here in Phoenix let's go with an OCS call for incoming shares right away.

@PVince81 PVince81 mentioned this pull request Jan 21, 2020
Closed
5 tasks
@PVince81
Copy link
Contributor Author

Added requests for incoming shares and now the indirect indicators work for both inheritance scenarios.
Added caching for the shares tree with pruning algorithm when switching folders.

Next up is to clean up stuff unrelated to share indicators to bring this into mergable state.

@PVince81 PVince81 changed the title Direct and indirect share indicators, share owner and collaborators Share indicator for direct and indirect shares in file list Jan 21, 2020
@PVince81 PVince81 requested a review from kulmann January 21, 2020 13:35
@PVince81
Copy link
Contributor Author

Squashed into a single commit.

The "dummy" parts for later tasks were moved to the branch "share-indirect-info" for continuing later.

@PVince81
Copy link
Contributor Author

PVince81 commented Jan 21, 2020 •
edited
Loading

  • BUG: indicator for incoming user share on parent (IUP) should be grey. currently is absent

@PVince81
Copy link
Contributor Author

PVince81 commented Jan 21, 2020 •
edited
Loading

  • verify that share indicators and its logic does NOT run in public link page

@PVince81
Copy link
Contributor Author

PVince81 commented Jan 21, 2020 •
edited
Loading

  • share indicators into a separate vue component ? => let's do it later as part of the extension point work

@PVince81 PVince81 mentioned this pull request Jan 21, 2020
Closed
2 tasks
@PVince81
Copy link
Contributor Author

Adjusted all tests, I expect all to pass except maybe the federation ones which I couldn't run locally.

@PVince81
Copy link
Contributor Author

added a few more fixes for federated tests (squashed)

@PVince81
Copy link
Contributor Author

Fixed federation tests where a copy-paste issue made it create the share recipient users on the wrong instance.

@PVince81
Copy link
Contributor Author

PVince81 commented Jan 22, 2020 •
edited
Loading

This doesn't make sense... two failures with login in https://drone.owncloud.com/owncloud/phoenix/7770/28/20 :

  ✖ And user "user2" has logged in using the webUI # tests/acceptance/stepDefinitions/loginContext.js:60

one thing I observed earlier is that if you attempt to log in as an already logged in user, the same error happens. However, in this one case the currently logged in user should be "user1".

@PVince81
Copy link
Contributor Author

weird, I looked at VNC and see that it's stuck on the authorize page showing "User One" while trying to login as user2. I thought we had logic in place to automatically switch the user there ?

@PVince81
Copy link
Contributor Author

I managed to fix the tests by using the "relogin" step to force clearing the session.
And also fixed a random failure due to missing delay in the share autocomplete input.

I'm expecting everything to pass now.

@PVince81
Copy link
Contributor Author

All passed, @kulmann please review

Vincent Petry added 2 commits January 23, 2020 16:17
Add share indicators in the main file list
0c34903 
For direct shares:
- Use the "oc:share-types" Webdav property to find out about direct shares in the file list, and display the matching icons.
- Display an active (blue) share indicator whenever there is at least one share of the given type.

For indirect shares:
- Define a new state attribute "sharesTree" to contain the list of shares for specific directories.
- After loading the current folder data, also populate "sharesTree" with the incoming and outgoing share lists from the parent folders.
- Use "sharesTree" like a cache and only keep share entries relevant to the current path and its parents. This prevents having to reload both incoming and outgoing shares for parent entries where we already have them available.
- Aggregate the share types from the data retrieved for parent folders to decide which "passive" (grey) share indicator to display for indirect outgoing shares.

Other:
- Keep the progress bar of the file list while loading.
- Clicking a share indicator opens the matching sidebar panel, Collaborators or Links.

Tests:
- Imported, adjusted  and extended acceptance tests for share indicators from OC 10.4
- Kept the share details tests in skipped mode for later
Add delay to share autocomplete dropdown
71ee5d2 
Added 1s delay to to the share autocomplete dropdown.
This increases the speed of tests as tests are simulating typing each
letter.
This also fixes random failures in the federation tests as sometimes the
dropdown would miss the last typed letter.
kulmann
kulmann approved these changes Jan 23, 2020
View reviewed changes
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

🚀

@PVince81 PVince81 merged commit 5c055ff into master Jan 23, 2020
@delete-merged-branch delete-merged-branch bot deleted the dummy-share-indicators branch January 23, 2020 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulmann kulmann kulmann approved these changes

@DeepDiver1975 DeepDiver1975 Awaiting requested review from DeepDiver1975

Assignees

@PVince81 PVince81

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Read and cache share information from local and parent folders Sharing indicators in the file list rows
2 participants
@PVince81 @kulmann