Skip to content

Add indeterminate state to share permission checkbox - #8390

Merged
MorrisJobke merged 3 commits into
nextcloud:masterfrom
dasisdormax:master
Feb 26, 2018
Merged

Add indeterminate state to share permission checkbox#8390
MorrisJobke merged 3 commits into
nextcloud:masterfrom
dasisdormax:master

Conversation

@dasisdormax

@dasisdormaxdasisdormax commented Feb 16, 2018

Copy link
Copy Markdown
Contributor

This PR adds the 'indeterminate' checkbox state to the general "can edit" checkbox if some, but not all of the 'create', 'update', and 'delete' prermissions are given for that share. See issue #8371

@dasisdormax
Add indeterminate state to 'can edit' share permission checkbox, seen…
d4238a5
…extcloud#8371
Signed-off-by: Maximilian Wende <dasisdormax@secure.mailbox.org>
@dasisdormax
Fix indeterminate checkbox state not showing correctly
602f50b
Also, the checkbox is updated to the correct state while a
permission change is in progress.
should fix issue nextcloud#8371
Signed-off-by: Maximilian Wende <dasisdormax@secure.mailbox.org>
@MorrisJobkeMorrisJobke added the 3. to review Waiting for reviews label Feb 16, 2018
@MorrisJobke

MorrisJobke commented Feb 16, 2018

Copy link
Copy Markdown
Member

The JSUnit tests fail: Could you have a look at https://drone.nextcloud.com/nextcloud/server/4940/216 (roughly line 900+)?

@dasisdormax
Update tests for indeterminate state, fix slashes not being escaped
7c453b2
Signed-off-by: Maximilian Wende <dasisdormax@mailbox.org>
@codecov

codecovBot commented Feb 16, 2018

Copy link
Copy Markdown

Codecov Report

Merging #8390 into master will decrease coverage by 0.41%.
The diff coverage is n/a.

@@ Coverage Diff @@## master #8390 +/- ##
===========================================
- Coverage 51.71% 51.3% -0.42% 
Complexity 25404 25404 ===========================================
Files 1600 1535 -65 Lines 95135 87332 -7803 Branches 1377 0 -1377 ===========================================
- Hits 49198 44803 -4395 + Misses 45937 42529 -3408
Impacted FilesCoverage ΔComplexity Δ
lib/private/Security/CertificateManager.php91% <0%> (-3%)39% <0%> (ø)
core/js/multiselect.js
core/js/mimetype.js
apps/systemtags/js/filesplugin.js
core/js/sharedialogview.js
apps/files_sharing/js/sharetabview.js
core/js/files/fileinfo.js
...mments/js/vendor/At.js/dist/js/jquery.atwho.min.js
apps/files_versions/js/versionstabview.js
core/js/apps.js
... and 57 more

@dasisdormax

dasisdormax commented Feb 16, 2018

Copy link
Copy Markdown
ContributorAuthor

@MorrisJobke it seems that my change in the render function revealed a bug we haven't found before (slashes in the sharee's federation URL weren't escaped).

Fixed that and updated the test for the new desired behaviour. Let's see if there's more ...

@juliusknorrjuliusknorr added enhancement design Design, UI, UX, etc. labels Feb 26, 2018
@juliusknorrjuliusknorr added this to the Nextcloud 14 milestone Feb 26, 2018
@juliusknorr

juliusknorr commented Feb 26, 2018

Copy link
Copy Markdown
Member

@dasisdormax Thanks for the pull request. That is a really nice improvement. 👍

MorrisJobke
MorrisJobke approved these changes Feb 26, 2018
View reviewed changes

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code change looks good and works nicely 👍

@MorrisJobke

MorrisJobke commented Feb 26, 2018•
edited
Loading

Copy link
Copy Markdown
Member

Swift CI failure is known -> from CI point of view it's good to merge.

@ChristophWurst@skjnldsv@danxuliu@rullzer Mind to review this one here?

@MorrisJobke
MorrisJobke merged commit 452e96e into nextcloud:masterFeb 26, 2018
@MorrisJobkeMorrisJobke mentioned this pull request Feb 26, 2018
Closed
@MorrisJobke

MorrisJobke commented Feb 26, 2018

Copy link
Copy Markdown
Member

Hey @dasisdormax we would like to invite you to the IRC channel #nextcloud-dev on freenode. Maybe also have a look at the other "good first issues": https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@danxuliudanxuliu mentioned this pull request May 10, 2018
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MorrisJobkeMorrisJobkeMorrisJobke approved these changes
@juliusknorrjuliusknorrjuliusknorr approved these changes
@ChristophWurstChristophWurstAwaiting requested review from ChristophWurst
@pixelipopixelipoAwaiting requested review from pixelipo
@skjnldsvskjnldsvAwaiting requested review from skjnldsv
@danxuliudanxuliuAwaiting requested review from danxuliu

Assignees

No one assigned

Labels

3. to reviewWaiting for reviewsdesignDesign, UI, UX, etc.enhancement

Projects

None yet

Milestone

Nextcloud 14

Development

Successfully merging this pull request may close these issues.

3 participants

@dasisdormax@MorrisJobke@juliusknorr