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

Improve public link sharing permissions for folders #29376

Merged
merged 2 commits into from
Nov 2, 2017
Merged

Conversation

felixheidecke
Copy link
Contributor

@felixheidecke felixheidecke commented Oct 27, 2017

Description

  • Rework permissions in UX friendlier radio-button style.
  • Improve modal behavior on mobile devices.

Related Issue

Fixes: #29116
Fixes: https://github.com/owncloud/enterprise/issues/2184

Motivation and Context

Currently the meaning of the option "Allow editing" (Download/Upload/Rename/Delete/Move) essentially changes to "Allow upload" (Upload only) when the second option "Show file listing" is unticked. This is inconsistent and can be very confusing for users. Moreover we thought a lot about how to make the file drop feature better understandable.

How Has This Been Tested?

Visual test only

Screenshots (if appropriate):

bildschirmfoto vom 2017-10-27 16-30-06

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@felixheidecke felixheidecke added 3 - To Review enhancement feature:sharing p1-urgent Critical issue, need to consider hotfix with just that issue labels Oct 27, 2017
@PVince81 PVince81 added this to the development milestone Oct 27, 2017
@codecov
Copy link

codecov bot commented Oct 27, 2017

Codecov Report

Merging #29376 into master will increase coverage by 0.29%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #29376      +/-   ##
============================================
+ Coverage     60.21%   60.51%   +0.29%     
- Complexity    17188    17193       +5     
============================================
  Files          1030     1031       +1     
  Lines         57274    57288      +14     
============================================
+ Hits          34490    34667     +177     
+ Misses        22784    22621     -163
Impacted Files Coverage Δ Complexity Δ
settings/templates/panels/admin/filesharing.php 100% <0%> (ø) 0% <0%> (ø) ⬇️
apps/files_sharing/lib/Capabilities.php 100% <0%> (ø) 6% <0%> (ø) ⬇️
settings/Panels/Admin/FileSharing.php 100% <0%> (ø) 6% <0%> (ø) ⬇️
core/Migrations/Version20171026130750.php 0% <0%> (ø) 2% <0%> (?)
lib/private/Share20/Manager.php 97.25% <0%> (+0.01%) 198% <0%> (+1%) ⬆️
lib/private/Share/Share.php 66.87% <0%> (+0.09%) 502% <0%> (+2%) ⬆️
apps/files_sharing/lib/ShareBackend/File.php 24.77% <0%> (+0.91%) 35% <0%> (ø) ⬇️
lib/public/Share.php 61.81% <0%> (+3.63%) 26% <0%> (ø) ⬇️
...files_sharing/lib/Controller/ShareesController.php 89.03% <0%> (+72.73%) 93% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 214d030...6bc5e61. Read the comment docs.

@haukman
Copy link

haukman commented Oct 30, 2017

@felixheidecke

I tried to apply the patch to a clean 10.0.3 without success:

root@ubuntu:/var/www/owncloud# patch -p1 < file.patch
patching file apps/files_sharing/css/sharetabview.css
patching file core/css/jquery.ocdialog.css
Hunk #1 FAILED at 8.
1 out of 1 hunk FAILED -- saving rejects to file core/css/jquery.ocdialog.css.rej
patching file core/js/jquery.ocdialog.js
Hunk #3 succeeded at 143 (offset 16 lines).
Hunk #4 succeeded at 154 (offset 16 lines).
patching file core/js/sharedialoglinkshareview.js

Am I doing somehting wrong?

Copy link

@haukman haukman left a comment

Choose a reason for hiding this comment

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

I tested the three radio buttons and everything worked fine.

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@PVince81
Copy link
Contributor

PVince81 commented Nov 2, 2017

@felixheidecke please backport to stable10

@PVince81
Copy link
Contributor

PVince81 commented Nov 3, 2017

stable10: #29413

@felixheidecke please always post link to backport, it's easier to find and more readable and triggers a github notification.

@michaelstingl
Copy link

@felixheidecke love PR's with screenshots 😍 !!!

@lock
Copy link

lock bot commented Aug 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4 - To release enhancement feature:sharing p1-urgent Critical issue, need to consider hotfix with just that issue status/STALE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve public link sharing permissions for folders
5 participants