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

Shared with others list #1702

Merged
merged 1 commit into from
Aug 12, 2019
Merged

Shared with others list #1702

merged 1 commit into from
Aug 12, 2019

Conversation

LukasHirt
Copy link
Collaborator

@LukasHirt LukasHirt commented Aug 6, 2019

Description

Created SharedFilesList component. Moved most of file list related functionality into mixins so we can reuse it in different lists. At the moment this list doesn't contain any actions thus checkboxes and star were removed as well and file name got class uk-disabled to prevent any hover/focus on it and to allow selecting of the row by clicking on it.

Related Issue

How Has This Been Tested?

  • test environment: Manually
  1. Share file with other user
  2. Share file that has been shared with me with can share permission with some other user

Screenshots (if appropriate):

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:

@LukasHirt LukasHirt self-assigned this Aug 6, 2019
@LukasHirt LukasHirt changed the title Created shared from me list [WIP] Shared with others list [WIP] Aug 6, 2019
@@ -56,6 +56,14 @@ const navItems = [
path: `/${appInfo.id}/favorites`
}
},
{
name: 'Shared with others',
Copy link
Member

Choose a reason for hiding this comment

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

we need to find a way to get this translated ....

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess we could translate it inside the menu component in computed properties but doesn't really seem like a nice solution.

loadFolderSharedFromMe (context, { client, $gettext }) {
context.commit('UPDATE_FOLDER_LOADING', true)

client.requests.ocs({
Copy link
Member

Choose a reason for hiding this comment

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

are we missing an api method in the sdk here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found mentions of shared_with_me in the shares there but no way how to load shared with others. (tried to use the getShares function, it's possible I overlooked something or did something wrong though)

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@LukasHirt LukasHirt changed the title Shared with others list [WIP] Shared with others list Aug 12, 2019
@LukasHirt LukasHirt added Status:Needs-Review Needs review from a maintainer and removed Status:In-Progress labels Aug 12, 2019
@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@LukasHirt LukasHirt marked this pull request as ready for review August 12, 2019 11:20
@DeepDiver1975
Copy link
Member

Is it possible to add a column with the recipients?

e.g. for user shares you have this information
Screenshot from 2019-08-12 14-50-32

for link shares at least a chain icon can be displayed ...

@LukasHirt
Copy link
Collaborator Author

LukasHirt commented Aug 12, 2019

Is it possible to add a column with the recipients?

@DeepDiver1975 With the current implementation we could display only one recipient per item... I'm filtering the response to remove all duplicate files to save a little bit of the performance and not build the same file e. g. 5 times.

@DeepDiver1975
Copy link
Member

With the current implementation we could display only one recipient per item... I'm filtering the response to remove all duplicate files to save a little bit of the performance and not build the same file e. g. 5 times.

Understood - oc10 is doing the same afaik
Screenshot from 2019-08-12 15-28-23

@DeepDiver1975
Copy link
Member

and there is a conflict - please resolve - THX

@LukasHirt
Copy link
Collaborator Author

Understood - oc10 is doing the same afaik

I added at least that one user. We can try to figure this out later IMHO 🤷‍♂

and there is a conflict - please resolve - THX

Rebased 😉

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@DeepDiver1975
Copy link
Member

@LukasHirt whats wrong with the tests? mind having a look? THX

@LukasHirt
Copy link
Collaborator Author

Should be green now. Let's see

@ownclouders

This comment has been minimized.

@ownclouders

This comment has been minimized.

@LukasHirt
Copy link
Collaborator Author

Damn. Still not 🤬

@ownclouders

This comment has been minimized.

Build only unique files

Added original file id

Added shared with

Fixed for cycle for sidebars

Hide files list on trashbin page
@LukasHirt
Copy link
Collaborator Author

@DeepDiver1975 Green 🙌

@DeepDiver1975 DeepDiver1975 merged commit 6b1e77b into master Aug 12, 2019
@delete-merged-branch delete-merged-branch bot deleted the feature/shared-from-me-list branch August 12, 2019 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Enhancement Add new functionality Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants