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

Ft/owncloud #41

Merged
merged 8 commits into from
Apr 14, 2017
Merged

Ft/owncloud #41

merged 8 commits into from
Apr 14, 2017

Conversation

dtext
Copy link
Member

@dtext dtext commented Sep 19, 2016

Maybe we should merge this before we change the storage providers. It will only work with pyocclient 0.3, which is not finished yet, but I think it would be beneficial to carry this along for the time being.
Closes #6 if we do so.

Any thoughts?

Edit: Just thought about this. As you can see, we're behind the latest changes with owncloud support again. If we carried it along, it would always be up to date and it will eventually work. We'll need to think about the requirements.txt though, as 0.3 is not out yet. I don't know if that's a problem.

Edit 2: See post below.

… updated for python 3.5 before this becomes usable.
This updates the owncloud feature branch in order to be able to easily
merge it. Since pyocclient 0.4 was released recently, we can finally
support uploading to owncloud.
@dtext
Copy link
Member Author

dtext commented Jan 27, 2017

pyocclient 0.4 was released recently, so we could merge owncloud support now. Reviews would be appreciated!

@dtext dtext self-assigned this Jan 27, 2017
Copy link
Member

@denniseffing denniseffing left a comment

Choose a reason for hiding this comment

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

Works fine now. Could be improved by sharing a direct file link instead of the ownCloud link since the ownCloud link redirects to the ownCloud UI with the embedded snapshot.

Just as a reminder: We have to change the password logic if we merge the keyring integration first.

@dtext
Copy link
Member Author

dtext commented Apr 11, 2017

Updated the owncloud implementation to use the kvstore now. I also implemented some URL transformation similar to what we have for dropbox.


def _transform_url(url: str) -> str:
Copy link
Member

Choose a reason for hiding this comment

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

_transform_url doesn't work properly. The transformed URL only shows a small thumbnail when opened and not the actual image.

Copy link
Member Author

@dtext dtext Apr 13, 2017

Choose a reason for hiding this comment

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

OwnCloud does not support direct links to files because of concerns about cross site scripting. So for images, transforming the URL would be a solution. We would need the dimensions of the image though. Looking at multimedia support further complicating things, I'm starting to think we should just leave the link as is (ditch the _transform_url function). That's how it's supposed to be shared anyways. What do you think?

Related:
owncloud/core#8180
owncloud/core#16470 (comment)

@dtext
Copy link
Member Author

dtext commented Apr 14, 2017

Reverted the direct URL thing for now, we could iterate on this in the future.

@dtext dtext merged commit 8cd0e7c into master Apr 14, 2017
@dtext dtext deleted the ft/owncloud branch April 14, 2017 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants