fix: Fix handling of deleting share from self #2319
+3
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
/backport to stable32 |
Member
Author
|
/backport to stable31 |
Activity
|
||||||||||||||||||||||||||||
| Project |
Activity
|
| Branch Review |
fix-handling-of-deleting-share-from-self
|
| Run status |
|
| Run duration | 02m 22s |
| Commit |
|
| Committer | Daniel Calviño Sánchez |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
1
|
Skipped
|
0
|
Passing
|
9
|
| View all changes introduced in this branch ↗︎ | |
danxuliu
force-pushed
the
fix-handling-of-deleting-share-from-self
branch
from
5fe098b to
3c6b4dd
Compare
If a share is deleted from self and it is not a group share it was handled as a user share. However, other types of shares can be deleted from self (for example, circle or room shares), so user shares need to be explicitly checked. Note that as those other share types are not explicitly handled no entry is added to the activity for them, but that was already the case before (this change just prevents an exception to be thrown). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu
force-pushed
the
fix-handling-of-deleting-share-from-self
branch
from
3c6b4dd to
80afa22
Compare
Member
Author
|
psalm intermittently (which is strange) fails locally for me with that error on master, and in any case it should be unrelated to the changes in this pull request. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes a regression introduced in #2103
Before #2103
unShareSelfexplicitly handled group shares and, for other types, delegated tounShare, which explicitly checkes user, group and link shares. In practice in that caseunShareonly calledunshareFromUserfor user shares, as group shares were already handled and link shares can not be deleted from self.Since #2103
unShareSelfstill explicitly handles group shares, but for other types directly callsunshareFromUser. However, there are other types that can be deleted from self (for example, circle or room shares), so they were wrongly handled as user shares.The result was not a wrong entry in the activity, but an exception thrown in
getUserRelativePathdue to the path of the share not belonging to the user that removed it. The exception does not cause the request to fail due to nextcloud/server#54419 for Nextcloud 32 and later, and due to #2217 for Nextcloud 31.0.11 and later, but it was added nevertheless to the Nextcloud log.This fix ensures that now only user shares are hadled as user shares, but it does not handle other types of shares, so no entry is added in the activity for them (but that was already the case before).
How to test
Result with this pull request
No error is printed in Nextcloud log
Result without this pull request
Error while sending ' . leave share . ' event with message /XXX/files/debugger.css is not a path for YYY (where XXX and YYY are the user ids) is added to Nextcloud log