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

Improve collaborators and owner columns in shared file lists #3049

Merged
merged 1 commit into from
Feb 28, 2020

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Feb 17, 2020

Description

When sharing with multiple targets, they must all be listed there instead of a single entry. This is fixed by replacing the code that make share entries unique into an aggregation that has been inspired by the one used in OC core.
Also includes group shares and link shares now.
Added avatars and icons for the displayed collaborators in the column.

Related Issue

Fixes #2924

Motivation and Context

Just doing a bugfix was too boring, but also the column contents didn't look very nice, so adding avatars there was a good move IMO.

How Has This Been Tested?

Manual test
Acceptance tests

Screenshots (if appropriate):

Shared with others

image

Shared with you

image

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:

  • test first!!!
  • add test for the group cases
  • implement the actual fix
  • changelog

@PVince81 PVince81 self-assigned this Feb 17, 2020
@update-docs
Copy link

update-docs bot commented Feb 17, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@PVince81
Copy link
Contributor Author

PVince81 commented Feb 26, 2020

@PVince81
Copy link
Contributor Author

Shared with others

image

Shared with you

image

I could go further and try and add resharer/owner but it increases complexity. I think the current result is already a big step forward.

@PVince81
Copy link
Contributor Author

This is only a stepping stone as we will consider merging "shared with me" with "shared with others" view into a single one, so it will likely evolve further from here

@PVince81
Copy link
Contributor Author

Failed tests:

  • renaming from share list: tests/acceptance/features/webUIRenameFiles/renameFiles.feature:160
  • deleting from share list: tests/acceptance/features/webUIDeleteFilesFolders/deleteFilesFolders.feature:201
  • resharing: tests/acceptance/features/webUIResharing/reshareUsers.feature:185
  • some sharing: tests/acceptance/features/webUISharingInternalGroups/shareWithGroups.feature:62
  • accept/reject: tests/acceptance/features/webUISharingAcceptShares/acceptShares.feature:13

@PVince81
Copy link
Contributor Author

data structure aligned

@PVince81 PVince81 changed the title Fix collaborators column in shared with others list Improve collaborators and owner columns in shared file lists Feb 27, 2020
@PVince81
Copy link
Contributor Author

row height issue unrelated to this PR => #3099

@PVince81
Copy link
Contributor Author

rebased and squashed.

next up will be adding/fixing tests

@PVince81
Copy link
Contributor Author

added and fixed tests, let's hope all pass now

@PVince81
Copy link
Contributor Author

raised #3101 which I discovered while trying to extend the test coverage

@PVince81
Copy link
Contributor Author

I'll adjust the tests later in the morning (not saying tomorrow as it's past midnight...)

When sharing with multiple targets, they must all be listed there
instead of a single entry. This is fixed by replacing the code that make
share entries unique into an aggregation that has been inspired by the
one used in OC core.
Also includes group shares and link shares now.

Added avatars and icons for the displayed collaborators in the column.
@PVince81
Copy link
Contributor Author

tests adjusted

background: the main reason for all these changes is that before this PR we were using share.path in the shared file list, which is always the original path. I changed it to use share.file_target which is the path in the point of view of the receiver. And that even when a share is declined, the latter stays. This is to align with the OC 10 logic. We could decide to change this later on if a different behavior makes sense.

@PVince81 PVince81 merged commit 6e0405e into master Feb 28, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-share-with-column branch February 28, 2020 07:46
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.

Shared with column in "Shared with others" should contain multiple users
2 participants