core icon indicating copy to clipboard operation
core copied to clipboard

WIP: add support for public link roles assigned by apps

Open mrow4a opened this issue 1 year ago • 16 comments

Description

Feature: Add support for public link roles assigned by apps

TODO

Related Issue

  • Related https://github.com/owncloud/enterprise/issues/5277
  • Related https://github.com/owncloud/core/pull/40264

How Has This Been Tested?

  • manually in the developer instance
  • unit/acceptance tests

Types of changes

  • [x] New feature (non-breaking change which adds functionality)

Checklist:

  • [x] Base code changes
  • [ ] Unit tests added
  • [ ] Acceptance tests added
  • [ ] Changelog item, see TEMPLATE

Example use-case: richdocuments preview

Screenshot 2022-08-15 at 21 52 53

Screenshot 2022-08-15 at 21 51 56

mrow4a avatar Aug 15 '22 19:08 mrow4a

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.

update-docs[bot] avatar Aug 15 '22 19:08 update-docs[bot]

@phil-davis I changed quite some stuff in the HTML code so I expect a lot of JS and acceptance tests fails

mrow4a avatar Aug 15 '22 20:08 mrow4a

:boom: Acceptance tests pipeline webUIWebdavLockProt-chrome-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/36574/160

ownclouders avatar Aug 15 '22 20:08 ownclouders

@phil-davis I changed quite some stuff in the HTML code so I expect a lot of JS and acceptance tests fails

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 avatar Aug 17 '22 10:08 phil-davis

@phil-davis @pmaier1 no it only works on single files (collabora formats more specifically)

mrow4a avatar Aug 17 '22 10:08 mrow4a

@phil-davis btw, new option is only available with richdocuments app installed, and secure view enabled.

mrow4a avatar Aug 18 '22 11:08 mrow4a

@phil-davis maybe in here make sense to just confirm no other feature is broken because of this one

cc @pmaier1

mrow4a avatar Aug 18 '22 11:08 mrow4a

@phil-davis maybe in here make sense to just confirm no other feature is broken because of this one

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 avatar Aug 19 '22 06:08 phil-davis

@phil-davis this is the point, new Preview option will NOT show unless registered by apps. In this case richdocuments with Secure View emabled

mrow4a avatar Aug 19 '22 06:08 mrow4a

@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.

phil-davis avatar Aug 19 '22 07:08 phil-davis

@mrow4a what should we do here now in the Server Team Board?

NannaBarz avatar Oct 10 '22 08:10 NannaBarz

@NannaBarz decide whether we want to have this feature or not.. ref. https://github.com/owncloud/enterprise/issues/5277

mrow4a avatar Oct 10 '22 09:10 mrow4a

@NannaBarz decide whether we want to have this feature or not

Yes, we want to have it for 10.12.

pmaier1 avatar Oct 10 '22 09:10 pmaier1

I am highly confident that this needs documentation when merged. Pls file an issue in docs-server before merged.

mmattel avatar Jan 19 '23 07:01 mmattel

@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 There are conflicts reported, and it needs a rebase: This branch is 3 commits ahead, 630 commits behind master.

phil-davis avatar Jan 26 '23 11:01 phil-davis

@phil-davis on-hold -> https://github.com/owncloud/enterprise/issues/5277#issuecomment-1381704115

mrow4a avatar Jan 26 '23 12:01 mrow4a