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.

Sign up for GitHub

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

Jump to bottom

Add quota restrictions options #28009

Merged
merged 1 commit into from
Jul 30, 2021
Merged

Add quota restrictions options #28009

merged 1 commit into from
Jul 30, 2021

Conversation

julien-nc
Copy link
Member

@julien-nc julien-nc commented Jul 16, 2021 •
edited
Loading

refs #21045

2 new Files app config options:

  • allow_unlimited_quota to toggle unlimited quota
  • max_quota to set the maximum quota that can be affected to users

A few things are taken care of:

  • Do not show unlimited suggestion when adding a member, editing a member and in default quota selection
  • Do not allow to manually set unlimited quota as default quota or as user quota
  • If unlimited quota is not allowed, it has to be avoided as the default_quota fallback value (when not set). The first quota preset is used instead.

Existing quota values and default quota are not affected by these changes. If the default quota or a user's quota has been set to unlimited, switching off unlimited quotas won't change the values. Only future affectations will be denied.

Quota presets are already configurable with the quota_preset config value of the Files app.
The remaining issue's item to address is

Entering of free custom values should be allowable or deniable

Feel free to add other reviewers 😁.

@julien-nc julien-nc added this to the Nextcloud 23 milestone Jul 16, 2021
@julien-nc
Copy link
Member Author

/compile amend /

@julien-nc julien-nc force-pushed the enh/21045/quota-restrictions branch from c5f72c5 to 78c3c81 Compare July 19, 2021 09:24
@julien-nc
Copy link
Member Author

/compile amend /

juliushaertl
juliushaertl reviewed Jul 19, 2021
View reviewed changes
lib/private/User/User.php

// if unlimited quota is not allowed => avoid getting 'unlimited' as default_quota fallback value
// use the first preset instead
$allowUnlimitedQuota = $this->config->getAppValue('files', 'allow_unlimited_quota', '1') === '1';
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if that could be covered by some tests

Copy link
Member Author

Choose a reason for hiding this comment

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

@juliushaertl Thanks for the review. I made an attempt to implement some tests in tests/lib/User/UserTest.php.

I'm having trouble to run those tests locally. I've played with

./autotest.sh sqlite lib/User/UserTest.php

but it always says Cannot open file "lib/User/UserTest".
Any idea or hint?

Copy link
Member

Choose a reason for hiding this comment

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

running them with the full path should work:

NOCOVERAGE=1 ./autotest.sh sqlite tests/lib/User/UserTest.php

Otherwise you can also run server tests from within phpstorm by using the proper bootstrap file in tests/bootstrap.php

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks a lot. Still not working when using autotest.sh. Nevermind, working fine with PhpStorm.

After a little fight with PhpUnit, here are 2 small tests 😁.

@juliushaertl juliushaertl added the pending documentation This pull request needs an associated documentation update label Jul 19, 2021
@julien-nc julien-nc force-pushed the enh/21045/quota-restrictions branch 3 times, most recently from d4e6a65 to b467c0a Compare July 19, 2021 15:46
@artonge
Copy link
Contributor

artonge commented Jul 20, 2021

/compile amend /

@julien-nc
Copy link
Member Author

I guess we can wait for #28202

@julien-nc julien-nc force-pushed the enh/21045/quota-restrictions branch from 8943546 to ea77ada Compare July 29, 2021 11:06
@julien-nc
Copy link
Member Author

/compile amend /

1 similar comment
@Pytal
Copy link
Member

Pytal commented Jul 29, 2021

/compile amend /

@npmbuildbot-nextcloud
refs #21045 add app config to disable unlimited quota and to set max …
416d10f 
…quota

avoid unlimited quota as default_quota fallback value if unlimited quota is not allowed
avoid getting/setting/displaying unlimited default quota if not allowed
implement tests for unlimited quota restrictions

Signed-off-by: Julien Veyssier <eneiluj@posteo.net>
Signed-off-by: npmbuildbot-nextcloud[bot] <npmbuildbot-nextcloud[bot]@users.noreply.github.com>
@julien-nc
Copy link
Member Author

Drone CI failure is not related (missing dependencies in nodb step).

@julien-nc
Copy link
Member Author

/backport to stable22

@julien-nc julien-nc merged commit 7170c03 into master Jul 30, 2021
@julien-nc julien-nc deleted the enh/21045/quota-restrictions branch July 30, 2021 08:26
@backportbot-nextcloud
Copy link

The backport to stable22 failed. Please do this backport manually.

@szaimen

This comment has been minimized.

Sign in to view
@nextcloud nextcloud deleted a comment from backportbot-nextcloud bot Jul 30, 2021
@nextcloud nextcloud deleted a comment from backportbot-nextcloud bot Jul 30, 2021
@julien-nc julien-nc mentioned this pull request Jul 30, 2021
Merged
@Pytal Pytal mentioned this pull request Aug 6, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliushaertl juliushaertl juliushaertl approved these changes

@Pytal Pytal Pytal approved these changes

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@szaimen szaimen Awaiting requested review from szaimen

@artonge artonge Awaiting requested review from artonge

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement feature: files feature: settings pending documentation This pull request needs an associated documentation update
Projects
None yet
Milestone
Nextcloud 23
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@julien-nc @artonge @Pytal @szaimen @juliushaertl @skjnldsv