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

Add a private link for the current folder to the app bar #2009

Merged
merged 3 commits into from
Sep 27, 2019
Merged

Add a private link for the current folder to the app bar #2009

merged 3 commits into from
Sep 27, 2019

Conversation

VicDeo
Copy link
Member

@VicDeo VicDeo commented Sep 18, 2019 •
edited
Loading

Description

Allows to copy the private link

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

PermalinkIcon

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)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • ...

@VicDeo VicDeo added the Status:Needs-Review Needs review from a maintainer label Sep 18, 2019
@VicDeo VicDeo self-assigned this Sep 18, 2019
@CLAassistant
Copy link

CLAassistant commented Sep 18, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@PVince81
Copy link
Contributor

please add acceptance tests, given that Selenium allows for testing clipboard operations.
maybe there's already an example for copying public links

cc @individual-it

@individual-it
Copy link
Member

you could try to paste the clipboard to an input field and then check the content of the field https://stackoverflow.com/questions/49192754/nightwatch-js-how-to-paste

you can use execute to create a new dummy input field https://nightwatchjs.org/api/commands/#execute or miss-use the search-field

@VicDeo VicDeo force-pushed the feature/1988 branch 2 times, most recently from 655412c to 948f0ae Compare September 23, 2019 12:44
@ownclouders
Copy link
Contributor

💥 Acceptance tests webUILogin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/5165/

20190923-124341-374.png
20190923-124356-299.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUITrashbin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/5167/

20190923-133940-257.png
20190923-133940-486.png
20190923-134005-543.png
20190923-134005-773.png

@VicDeo
Copy link
Member Author

VicDeo commented Sep 23, 2019

@PVince81 @individual-it I made some progress spending approx 3 hours but now I wonder how it is possible to get a privateLink property of the current directory. This is an expected clipboard content.

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUILogin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/5168/

20190923-141256-543.png
20190923-141314-336.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUITrashbin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/5168/

20190923-142622-944.png
20190923-142709-442.png

@PVince81
Copy link
Contributor

@VicDeo there is a DAV property: https://github.com/owncloud/core/blob/master/apps/dav/lib/Connector/Sabre/FilesPlugin.php#L66

else you'll need to build the link by hand by appending the file id

@VicDeo VicDeo force-pushed the feature/1988 branch 2 times, most recently from 4820347 to e091d3c Compare September 25, 2019 09:04
@VicDeo
Copy link
Member Author

VicDeo commented Sep 25, 2019

@individual-it Somehow client.execute doesn't work at all.
I tried this sample nightwatchjs/nightwatch#353 (comment) and there is no any kind of output from console.log

@individual-it individual-it self-assigned this Sep 25, 2019
@individual-it
Copy link
Member

assigned myself to look at the execute() issue

@LukasHirt
Copy link
Contributor

assigned myself to look at the execute() issue

As we discussed I'm having the same issue in #2049 even after moving it directly into context file. I'll try moving it into custom commands and see. Let me pls know if you'll find solution faster.

@VicDeo
Copy link
Member Author

VicDeo commented Sep 25, 2019

@individual-it I got execute working with the last push (basically by putting it to the very top of the method), but it's still not possible to read a clipboard with this code piece
https://github.com/owncloud/phoenix/pull/2009/files#diff-2394b558df520322909de1efc5895cf4R492-R496

I appreciate your help with it

@VicDeo VicDeo force-pushed the feature/1988 branch 2 times, most recently from 9428185 to 899ad1e Compare September 25, 2019 12:49
@ownclouders
Copy link
Contributor

💥 Acceptance tests webUILogin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/5210/

20190925-141637-558.png
20190925-141656-741.png

@VicDeo
Copy link
Member Author

VicDeo commented Sep 25, 2019

@individual-it passing locally, please rereview

@VicDeo VicDeo force-pushed the feature/1988 branch 2 times, most recently from f36c318 to 4e3ffd7 Compare September 25, 2019 15:45
individual-it
individual-it previously requested changes Sep 26, 2019
View reviewed changes
Copy link
Member

@individual-it individual-it left a comment

Choose a reason for hiding this comment

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

something is not quite right with the getProperties function

TypeError: Cannot read property 'd:response' of undefined

https://drone.owncloud.com/owncloud/phoenix/5213/3/13
same happens locally

tests/acceptance/stepDefinitions/filesContext.js Outdated Show resolved Hide resolved
@VicDeo
Copy link
Member Author

VicDeo commented Sep 26, 2019

@individual-it

TypeError: Cannot read property 'd:response' of undefined
https://drone.owncloud.com/owncloud/phoenix/5213/3/13

Is skeleton with /simple-folder created automatically? I can't see it in VNC - root folder is empty and the dav response is

{ _declaration: { _attributes: { version: '1.0', encoding: 'utf-8' } },
  'd:error': 
   { _attributes: { 'xmlns:d': 'DAV:', 'xmlns:s': 'http://sabredav.org/ns' },
     's:exception': { _text: 'Sabre\\DAV\\Exception\\NotFound' },
     's:message': { _text: 'File with name simple-folder could not be located' } } }

I expect simple-folder to be created along with a user

@individual-it
Copy link
Member

@VicDeo yes, that folder should be there if the skeleton setting is set correctly in config.php of oC

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUILogin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/5222/

20190926-115238-743.png
20190926-115254-140.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUILogin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/5224/

20190926-123754-637.png
20190926-123814-747.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests webUILogin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/5225/

20190926-125541-580.png
20190926-125559-597.png

@VicDeo
Copy link
Member Author

VicDeo commented Sep 26, 2019

Well, the skeleton was not copied due to misconfigured s3 backend in OC.
Now simple-folder is created.
I also set propfind folderDepth to be zero as I don't want props from it's children

@VicDeo VicDeo force-pushed the feature/1988 branch 2 times, most recently from 6bc7c5c to 508dc15 Compare September 26, 2019 14:28
@VicDeo
Copy link
Member Author

VicDeo commented Sep 26, 2019

rebased, squashed, should pass CI now

individual-it
individual-it reviewed Sep 27, 2019
View reviewed changes
Copy link
Member

@individual-it individual-it left a comment

Choose a reason for hiding this comment

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

tests look good 👍

PVince81
PVince81 approved these changes Sep 27, 2019
View reviewed changes
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 PVince81 merged commit d3c7793 into master Sep 27, 2019
@delete-merged-branch delete-merged-branch bot deleted the feature/1988 branch September 27, 2019 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@individual-it individual-it individual-it left review comments

@PVince81 PVince81 PVince81 approved these changes

@DeepDiver1975 DeepDiver1975 Awaiting requested review from DeepDiver1975

@LukasHirt LukasHirt Awaiting requested review from LukasHirt

Assignees

@VicDeo VicDeo

@individual-it individual-it

Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Private link for current folder in app bar
6 participants
@VicDeo @CLAassistant @PVince81 @individual-it @ownclouders @LukasHirt