Skip to content

Conversation

@danxuliu
Copy link
Member

When the receiver of a group share modifies it (for example, by moving it to a different folder) the original share is not modified, but a "ghost" share that keeps track of the changes made by that specific user is used instead.

By default, the method getShareById in the share provider returns the share from the point of view of the sharer, but it can be used too to get the share from the point of view of a sharee by providing the recipient parameter (and if the sharee is not found then the share is returned from the point of view of the sharer).

The ShareAPIController always formats the share from the point of view of the current user, but when getting the information of a specific share the recipient parameter was not given, so it was always returned from the point of view of the sharer, even if the current user was a sharee. Now the recipient parameter is set to the current user, and thus the information of the share is returned from the point of view of the current user, be it the sharer or a sharee.

All this can be seen in the integration tests added also in this pull request; without the fix, when getting the information of a group share as a sharee, the returned file_target is /textfile0.txt instead of /textfile0 (2).txt (because it is returned by getShareById), but the returned path is /textfile0 (2).txt with and without the fix (because it is got from the node as the current user when the share is formatted).

Note that this special behaviour of getShareById happens only with group shares; with other types of shares the share is the same for the sharer and the sharee, and thus the parameter is ignored (DefaultShareProvider, ShareByCircleProvider, ShareByMailProvider and FederatedShareProvider); it was added for them too just for consistency.

danxuliu added 2 commits June 29, 2018 08:20
When the receiver of a group share modifies it (for example, by moving
it to a different folder) the original share is not modified, but a
"ghost" share that keeps track of the changes made by that specific user
is used instead.

By default, the method "getShareById" in the share provider returns the
share from the point of view of the sharer, but it can be used too to
get the share from the point of view of a sharee by providing the
"recipient" parameter (and if the sharee is not found then the share is
returned from the point of view of the sharer).

The "ShareAPIController" always formats the share from the point of view
of the current user, but when getting the information of a specific
share the "recipient" parameter was not given, so it was always returned
from the point of view of the sharer, even if the current user was a
sharee. Now the "recipient" parameter is set to the current user, and
thus the information of the share is returned from the point of view of
the current user, be it the sharer or a sharee.

Note that this special behaviour of "getShareById" happens only with
group shares; with other types of shares the share is the same for the
sharer and the sharee, and thus the parameter is ignored; it was added
for them too just for consistency.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu added this to the Nextcloud 14 milestone Jun 29, 2018
@MorrisJobke
Copy link
Member

This one is not related, right?

1) OCA\Files_Sharing\Tests\ShareTest::testShareWithGroupUniqueName
OCP\Files\NotFoundException: Node for share not found, fileid: 2653

/drone/src/github.com/nextcloud/server/lib/private/Share20/Share.php:168
/drone/src/github.com/nextcloud/server/lib/private/Share20/Manager.php:254
/drone/src/github.com/nextcloud/server/lib/private/Share20/Manager.php:808
/drone/src/github.com/nextcloud/server/apps/files_sharing/tests/ShareTest.php:186

Or should it fix that one?

@danxuliu
Copy link
Member Author

No, it is not related; that is a different issue.

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Indeed makes sense.

@MorrisJobke MorrisJobke merged commit a788f49 into master Jun 29, 2018
@MorrisJobke MorrisJobke deleted the fix-getting-the-information-of-a-group-share-as-a-sharee branch June 29, 2018 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants