-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Cleanup and do not list current user shares in getShares too #16789
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
Cleanup and do not list current user shares in getShares too #16789
Conversation
2f9c3ab to
a4c4733
Compare
This comment has been minimized.
This comment has been minimized.
a4c4733 to
ff5d724
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ff5d724 to
75ee2b2
Compare
|
Same here everyone, please let's get this in so I can continue the sharing revamp! |
ChristophWurst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐘
3492fd9 to
bf6209c
Compare
danxuliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are several failing tests, not ready to release yet. I will take a look.
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
"ShareManager::getSharesBy()" already checks if the share provider exists before returning the shares and, if the provider does not exist, it returns an empty array. Therefore it is not needed to explicitly check if the provider exists or not. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Getting the shares of a file no longer returns shares with the current user for consistency with the results when getting the shares including subfiles. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
ce6c693 to
8a06720
Compare
|
Sorry, it took me way longer than expected :-( I have added more unit and integration tests, split the fixes and the refactorings, did some minor cleanups and fixed the pending issues (I hope :-P ). @skjnldsv I moved I have also added some integration tests in a last commit that show the current behaviour in some strange scenarios. After working on this so much time I no longer know what works as expected or not... so I just document it and I let others decide for me :-P And if the tests themselves do not make sense either just drop the last commit; I sense a bug (not from this pull request, but coming already from the previous code) lurking around in the code that returns a reduced set of shares when there are not enough resharing rights, but as much as I tried I could not find it. So maybe everything is just fine :-P |
Argh, I cannot recall! :( The issue I noticed was on the new sidebar, when listing the shares of a shared-with-me folder, I could also see the share that made the folder available to me.
That's is an incredible work!! |
that is the issue here.
So far: EDIT: fixed 🚀 |
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
8a06720 to
d0b205d
Compare
|
Failed integration test now passes. 🚀 |
I could use some help on the tests :)