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

Let the admin configure the default share permissions #7363

Merged
merged 7 commits into from
Feb 27, 2018
Merged

Let the admin configure the default share permissions #7363

merged 7 commits into from
Feb 27, 2018

Conversation

schiessle
Copy link
Member

@schiessle schiessle commented Dec 1, 2017 •
edited
Loading

Let the admin configure the default share permissions

Possible look:

default-share2

or (currently implemented)

default-share1

@schiessle schiessle added this to the Nextcloud 13 milestone Dec 1, 2017
@schiessle
Copy link
Member Author

schiessle commented Dec 1, 2017 •
edited
Loading

@jancborchardt please have a look at the two screenshots above. What do you prefer? Or something complete different? (It is about the "default share permissions")

@codecov
Copy link

codecov bot commented Dec 1, 2017 •
edited
Loading

Codecov Report

Merging #7363 into master will increase coverage by <.01%.
The diff coverage is 71.11%.

@@             Coverage Diff              @@
##             master    #7363      +/-   ##
============================================
+ Coverage     51.88%   51.89%   +<.01%     
- Complexity    25394    25396       +2     
============================================
  Files          1603     1603              
  Lines         95214    95249      +35     
  Branches       1379     1380       +1     
============================================
+ Hits          49403    49427      +24     
- Misses        45811    45822      +11
Impacted Files Coverage Δ Complexity Δ
lib/private/TemplateLayout.php 0% <0%> (ø) 48 <0> (ø) ⬇️
core/Controller/OCJSController.php 0% <0%> (ø) 2 <0> (ø) ⬇️
core/js/js.js 66.11% <0%> (-0.15%) 0 <0> (ø)
settings/templates/settings/admin/sharing.php 0% <0%> (ø) 0 <0> (ø) ⬇️
lib/private/Template/JSConfigHelper.php 0% <0%> (ø) 17 <0> (ø) ⬇️
core/js/shareitemmodel.js 88.14% <100%> (+0.64%) 0 <0> (ø) ⬇️
lib/private/Settings/Manager.php 65% <100%> (ø) 42 <0> (ø) ⬇️
apps/files_sharing/lib/Capabilities.php 87.8% <100%> (+0.3%) 5 <0> (ø) ⬇️
...iles_sharing/lib/Controller/ShareAPIController.php 68.34% <82.14%> (+0.65%) 156 <0> (+1) ⬆️
lib/private/Settings/Admin/Sharing.php 84% <84.21%> (-8.86%) 6 <2> (+1)
... and 3 more

@schiessle schiessle force-pushed the default-share-perms branch 5 times, most recently from 97db685 to c09d4d9 Compare December 1, 2017 15:56
@schiessle schiessle added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 1, 2017
@schiessle schiessle modified the milestones: Nextcloud 13, Nextcloud 14 Dec 4, 2017
@MorrisJobke MorrisJobke mentioned this pull request Dec 9, 2017
Merged
28 tasks
@jancborchardt
Copy link
Member

Do we still want to stick to this complicated CRUD-mechanic? I thought at some point we only do two permissions:

  • Can edit
  • Can reshare

This was referenced Feb 7, 2018
Closed
Closed
@schiessle
Copy link
Member Author

Since Nextcloud 13 is out I think now it is the right time to pick this up, looking forward to your reviews

@MorrisJobke
Copy link
Member

Since Nextcloud 13 is out I think now it is the right time to pick this up, looking forward to your reviews

First of all: looking forward for your merge conflict fixes 😜 😉

@schiessle schiessle force-pushed the default-share-perms branch 2 times, most recently from 4fe6b04 to 50e9aab Compare February 9, 2018 20:02
@schiessle
Copy link
Member Author

@MorrisJobke done... now it is your turn 😉

@jancborchardt
Copy link
Member

What about my comment above? "Can edit", "Can reshare" should be the permissions, none other.

@schiessle
Copy link
Member Author

schiessle commented Feb 13, 2018 •
edited
Loading

What about my comment above? "Can edit", "Can reshare" should be the permissions, none other.

@jancborchardt I think it is a bit out of context. I'm completely open for simplifying the permissions. But this is one is just about making the default permissions configurable.

@MorrisJobke MorrisJobke mentioned this pull request Feb 15, 2018
Closed
@schiessle
Copy link
Member Author

waiting for a second review... Would be great to get this finally in before we have to rebase it again and again. Thanks!

@MorrisJobke
Copy link
Member

@rullzer @nickvergessen @skjnldsv Review would be nice :)

schiessle and others added 7 commits February 27, 2018 12:29
@schiessle @rullzer
expose capabilities in js
7d0102b 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle @rullzer
add share permissions to settings page
1615312 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle @rullzer
improved layout of the admin settings
7741429 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle @rullzer
share api: use default permission of no permission is given
ebb1528 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle @rullzer
fix unit tests
3285bec 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@rullzer
Fix share capabilities JS tests
7466468 
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@MorrisJobke @rullzer
Fix JSDoc
20ec034 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@rullzer
Copy link
Member

rullzer commented Feb 27, 2018

I rebased to make sure it all still works. Fails somehow. I'll look into the why....

@rullzer
Copy link
Member

rullzer commented Feb 27, 2018

Ah was just cached stuff. Works like a charm.

@rullzer rullzer added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 27, 2018
@MorrisJobke MorrisJobke merged commit 7bc3c2e into master Feb 27, 2018
@MorrisJobke MorrisJobke deleted the default-share-perms branch February 27, 2018 12:44
@kimsyversen kimsyversen mentioned this pull request Jun 15, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rullzer rullzer rullzer approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@blizzz blizzz Awaiting requested review from blizzz

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@danxuliu danxuliu Awaiting requested review from danxuliu

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish feature: sharing
Projects
None yet
Milestone
Nextcloud 14
Development

Successfully merging this pull request may close these issues.
4 participants
@schiessle @jancborchardt @MorrisJobke @rullzer