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
WIP: add support for public link roles assigned by apps #40289
base: master
Are you sure you want to change the base?
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
}, | ||
{ | ||
name: "allowPublicFileReadWrite", | ||
permissions: OC.PERMISSION_READ | OC.PERMISSION_UPDATE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DeepDiver1975 public file read/write cannot have OC.PERMISSION_CREATE and OC.PERMISSION_DELETE , I fixed it here. Otherwise we could allow for delete of public link file from public link , and create obviously does not make sense.
@@ -22,31 +30,33 @@ | |||
'<label class="public-link-modal--label">{{linkNameLabel}}</label>' + | |||
'<input class="public-link-modal--input" type="text" name="linkName" placeholder="{{namePlaceholder}}" value="{{name}}" maxlength="64" />' + | |||
'</div>' + | |||
'<div id="appManagedPublicLinkModelItems">' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a place there apps can register new checkboxes
@@ -277,7 +277,7 @@ protected function formatShare(IShare $share, $received = false) { | |||
|
|||
$result['attributes'] = null; | |||
if ($attributes = $share->getAttributes()) { | |||
$result['attributes'] = \json_encode($attributes->toArray()); | |||
$result['attributes'] = $attributes->toArray(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This somehow sneaked in and is a bug, but would need full ci to run to confirm
@@ -882,6 +882,7 @@ public function updateShare($id) { | |||
// legacy | |||
$newPermissions !== (Constants::PERMISSION_READ | Constants::PERMISSION_CREATE | Constants::PERMISSION_UPDATE) && | |||
// correct | |||
$newPermissions !== (Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DeepDiver1975 public link file read/write does not work without this line. It was possible to create such share, but not update.
@phil-davis I changed quite some stuff in the HTML code so I expect a lot of JS and acceptance tests fails |
💥 Acceptance tests pipeline webUIWebdavLockProt-chrome-mariadb10.2-php7.4 failed. The build has been cancelled. |
I can look at the acceptance test fails, and add a basic acceptance test for the new UI option. The test will verify that the public link does not have the ability to do an "ordinary" download or upload. Should this public link feature be able to be selected for a folder? (and thus apply to all files/folders "under" that folder?) |
@phil-davis @pmaier1 no it only works on single files (collabora formats more specifically) |
@phil-davis btw, new option is only available with richdocuments app installed, and secure view enabled. |
@phil-davis maybe in here make sense to just confirm no other feature is broken because of this one cc @pmaier1 |
yes, for acceptance tests I will just make a scenario that checks that the new UI element exists and that it can be clicked to create a link... @mrow4a are you going to make the unit tests (JS and PHP) pass? |
@phil-davis this is the point, new Preview option will NOT show unless registered by apps. In this case richdocuments with Secure View emabled |
OK - now I understand why it will be hard to test just in core CI. |
@mrow4a what should we do here now in the Server Team Board? |
@NannaBarz decide whether we want to have this feature or not.. ref. https://github.com/owncloud/enterprise/issues/5277 |
Yes, we want to have it for 10.12. |
I am highly confident that this needs documentation when merged. |
@mrow4a there is mention above about "Yes, we want to have it for 10.12" Is that still true? https://github.com/owncloud/core/tree/e5277 |
Description
Feature: Add support for public link roles assigned by apps
TODO
Related Issue
How Has This Been Tested?
Types of changes
Checklist:
Example use-case: richdocuments preview