Add new share type for federated groups #34132
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Psalm found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
smesterheide
changed the title
lib/private/Collaboration/Collaborators/FederatedGroupPlugin.php
Outdated
Show resolved
Hide resolved
|
/compile-amend / |
|
Related branch on application side: https://github.com/nextcloud/vo_federation/tree/wip/share-provider |
|
Question: |
| @@ -266,6 +269,31 @@ | |||
Check failure
Code scanning / Psalm
UndefinedDocblockClass
| protected function getFederatedGroupShareProvider() { | ||
| if ($this->federatedGroupShareProvider === null) { | ||
| /* | ||
| * Check if the app is enabled |
Check failure
Code scanning / Psalm
UndefinedDocblockClass
| } | ||
|
|
||
| return $this->federatedGroupShareProvider; | ||
| } |
Check failure
Code scanning / Psalm
NullableReturnStatement
|
|
||
| /** | ||
| * @inheritdoc | ||
| */ |
Check failure
Code scanning / Psalm
InvalidReturnStatement
|
I can't say something about the code as such, this is something @come-nc and others have to decide. But the general structure and the introduction of the share type "IShare::TYPE_FEDERATED_GROUP" looks fine to me |
| private $circlesAreNotAvailable = false; | ||
| /** @var \OCA\Talk\Share\RoomShareProvider */ | ||
| private $roomShareProvider = null; | ||
| /** @var \OCA\VO_Federation\FederatedGroupShareProvider */ |
There was a problem hiding this comment.
Usually we would use the registerProvider function here to dynamically register our provider from our Application.php
As it turns out the ProviderFactory is also required by remote.php during WebDAV share mounts where normal app bootstrapping is skipped.
| * @since 26.0.0 | ||
| */ | ||
| public function getCloudFederationProviderForFederationShare(ICloudFederationShare $share); | ||
|
|
Check failure
Code scanning / Psalm
UndefinedDocblockClass
|
Can you please run |
| $result['share_with'] = $share->getSharedWith(); | ||
| $result['share_with_displayname'] = $group !== null ? $group->getDisplayName() : $share->getSharedWith(); | ||
| try { | ||
| $result = array_merge($result, $this->getFederatedGroupShareHelper()->formatShare($share)); |
Check failure
Code scanning / Psalm
UndefinedDocblockClass
| */ | ||
| private function getFederatedGroupShareHelper() { | ||
| if (!$this->appManager->isEnabledForUser('vo_federation')) { | ||
| throw new QueryException(); |
Check failure
Code scanning / Psalm
UndefinedDocblockClass
|
|
||
| return $this->serverContainer->get('\OCA\VO_Federation\Sharing\ShareAPIHelper'); | ||
| } | ||
|
|
Check notice
Code scanning / Psalm
DeprecatedClass
Signed-off-by: Sandro Mesterheide <[email protected]>
Signed-off-by: Sandro Mesterheide <[email protected]>
Signed-off-by: Sandro Mesterheide <[email protected]>
skjnldsv
added
2. developing
|
Hi @smesterheide If you want to spark a conversation again, because it actually fit another feature, feel free to do so, we'll be happy to talk and assist you! :) |
7 participants
Summary
The PR aims to add a new share type for remote groups on multiple federated NC instances.
Changes
CloudFederationProviderare registered for a resourceType and shareType instead of a resourceType (file) alone.TODO
Checklist