Skip to content
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

Add deck share provider support #24605

Merged
merged 17 commits into from
Dec 10, 2020
Merged

Add deck share provider support #24605

merged 17 commits into from
Dec 10, 2020

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Dec 8, 2020

This PR extends the share manager to allow apps registering their own share providers and implements support for deck shares in places where handing over the logic to the apps is yet not possible. Basically the parts where the server still needs app based logic is mainly the share api and the sharing sidebar in files.

Heavily inspired by #10255

Server part of nextcloud/deck#2638 This can already be tested together for the share provider logic, however the deck side is still lacking the user interface to interact with the shares and upload/share files from within deck.

@juliusknorr juliusknorr force-pushed the enh/share-deck branch 2 times, most recently from e5533e7 to 4ac2cbb Compare December 8, 2020 15:50
@juliusknorr juliusknorr marked this pull request as ready for review December 8, 2020 16:31
@juliusknorr juliusknorr added 3. to review Waiting for reviews enhancement labels Dec 8, 2020
@juliusknorr juliusknorr added this to the Nextcloud 21 milestone Dec 8, 2020
Comment on lines +321 to +322
} elseif ($shareType === IShare::TYPE_DECK) {
$provider = $this->getProvider('deck');
Copy link
Member Author

Choose a reason for hiding this comment

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

We could also extend the share manager to allow registering a share provider together with a shareType, then at least the provider registration could be done without having additional code in server (except for the actual shareType constant), but i kept that for later to not overcomplicate this pr.

Copy link
Member Author

Choose a reason for hiding this comment

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

'deck' is the identifier of the share provider https://github.com/nextcloud/deck/pull/2638/files#diff-bfa062609a398b64ebc3167faba3abbca19b04da0c1e3f3dd033fa6babaebbc4R100-R102 that basically implies that IShare::TYPE_DECK is used.

@nextcloud nextcloud deleted a comment from faily-bot bot Dec 8, 2020
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

🐘 looks like it could work. but didnt really test or check if all places have been adapted.

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.

Please check the comments from @nickvergessen but fine by me

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
…to have a link

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants