Add new share type for federated groups#34132
Add new share type for federated groups#34132smesterheide wants to merge 4 commits intonextcloud:masterfrom smesterheide:vo-federation-features
Conversation
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.
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 @@ protected function getRoomShareProvider() { | |||
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.
|
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 <sandro.mesterheide@extern.publicplan.de>
Signed-off-by: Sandro Mesterheide <sandro.mesterheide@extern.publicplan.de>
Signed-off-by: Sandro Mesterheide <sandro.mesterheide@extern.publicplan.de>
|
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! :) |
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