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 option to exclude groups from creating link shares #26727

Merged
merged 4 commits into from
May 21, 2021

Conversation

icewind1991
Copy link
Member

I would have hoped to solve this in a more generic "per group share settings" way but the way the code base interacts with the share settings is to messy to do that without a lot of rework (this PR includes some improvement in this area).

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Apr 23, 2021
@icewind1991 icewind1991 added this to the Nextcloud 22 milestone Apr 23, 2021
@tobiasKaminsky
Copy link
Member

How is this then shown to clients?
Is then just files_sharing -> public -> enabled: false?
(in capabilities)
So nothing needs to be done/tested on clients?

@icewind1991
Copy link
Member Author

Clients should see the same from the capabilities response as if link shares were completely disabled

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.

Does the trick for me!

@rullzer
Copy link
Member

rullzer commented Apr 29, 2021

However, linting, psalm and phpunit are not happy

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 29, 2021
@rullzer rullzer requested a review from skjnldsv April 29, 2021 19:02
@icewind1991 icewind1991 force-pushed the group-exclude-link-share branch 3 times, most recently from 253f504 to 9afb874 Compare May 12, 2021 11:01
$publicUploadEnabled = $config->getAppValue('core', 'shareapi_allow_public_upload', 'yes');
/** @var IManager $shareManager */
$shareManager = \OC::$server->get(IManager::class);
$publicUploadEnabled = $shareManager->shareApiLinkAllowPublicUpload();
Copy link
Member

Choose a reason for hiding this comment

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

I couldn't find any usage of the data attribute where this value is assigned to but from a compatibility perspective we should probably preserve the yes/no values here:

Suggested change
$publicUploadEnabled = $shareManager->shareApiLinkAllowPublicUpload();
$publicUploadEnabled = $shareManager->shareApiLinkAllowPublicUpload() ? 'yes' : 'no';

Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Small comment, otherwise code looks good and works as expected 👍

@icewind1991 icewind1991 force-pushed the group-exclude-link-share branch 2 times, most recently from 1e333bc to 094c4d5 Compare May 12, 2021 11:32
@juliusknorr
Copy link
Member

Now just the CapabilitiesTest still needs adjustments 😉

Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
@MorrisJobke MorrisJobke mentioned this pull request May 20, 2021
@MorrisJobke MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 21, 2021
@MorrisJobke
Copy link
Member

CI is happy :)

@MorrisJobke MorrisJobke merged commit f1dbabd into master May 21, 2021
@MorrisJobke MorrisJobke deleted the group-exclude-link-share branch May 21, 2021 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants