Upload file using SDK instead of XHR #2239

PVince81 · 2019-10-14T13:18:48Z

Description

Use owncloud-sdk for uploading files

Related Issue

None raised

Motivation and Context

As we'll add chunk upload later to the SDK, it makes sense to use the SDK for upload instead.
It will only be a matter of adding "maxChunkSize" to the options to have it using chunks.
I don't think we should implement chunking directly in Phoenix.

Also: as we use the SDK in other places we should be consistent and use it everywhere!

How Has This Been Tested?

Quick manual test with a file upload:

TEST: upload works in root 🤖
TEST: upload works in subdir 🤖
TEST: upload works in public page root 🤖
TEST: upload works in public page subdir 🤖
TEST: upload works in files drop 🤖
TEST: progress bar works
TEST: overwrite works in all above cases
TEST: upload with drag and drop in all the above cases
TEST: IE11...

Screenshots (if appropriate):

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:

REQUIRES: owncloud-sdk merge Add upload progress + headers owncloud-sdk#366 for progress events
REQUIRES: davclient.js merge Add onProgress callback davclient.js#39 for progress events
TODO: also apply and test the change for public file upload
TODO: delete FileUpload.js

PVince81 · 2019-10-14T13:19:46Z

@LukasHirt @DeepDiver1975 please have a look at this POC and also the linked dependencies in owncloud-sdk and let me know if this is ok before I continue applying the changes to the public upload.

I intend to delete FileUpload.js (but not FileUpload.vue) and later on we can implement chunking inside of owncloud-sdk: owncloud/owncloud-sdk#18

ownclouders · 2019-10-14T13:44:17Z

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

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

ownclouders · 2019-10-14T13:45:00Z

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

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

DeepDiver1975 · 2019-10-14T13:55:13Z

apps/files/src/mixins.js

@@ -1,6 +1,5 @@
 import filesize from 'filesize'
 import moment from 'moment'
-import FileUpload from './FileUpload.js'


can we delete FileUpload.js?

yes, when I'm done. See TODO list in top post

PVince81 · 2019-10-14T15:10:04Z

also we need to talk about "loadend" now (see conflict), I'm still not convinced that we need it.
especially we can't add it in davclient.js as that library has no polyfills in place and adding those there seems like overkill

PVince81 · 2019-10-14T15:13:14Z

seems loadend also fires after an error: https://humanwhocodes.com/blog/2012/05/22/working-with-files-in-javascript-part-3/

PVince81 · 2019-10-14T15:53:40Z

more trouble: uploading into subdirs doesn't work because all the upload code assumptions so far was about using the full URL as prefix. this means we need to change many places to use the current folder for its path. This changes the paradigms a bit...

change URL use for upload to using path instead

PVince81 · 2019-10-14T16:11:39Z

I managed to fix the subdir upload but I'm now blocked as I can't test public upload: need #1951 first

PVince81 · 2019-10-15T09:52:26Z

note: acceptance tests to be added as part of ticket #2222

LukasHirt · 2019-10-16T09:36:57Z

seems loadend also fires after an error: https://humanwhocodes.com/blog/2012/05/22/working-with-files-in-javascript-part-3/

Exactly, that's what I mentioned here #2225 (comment) and why I used it at all.

LukasHirt · 2019-10-16T09:38:46Z

Anyway, like a lot the approach to move it into the sdk 👍

PVince81 · 2019-11-07T18:37:01Z

I've fixed the public link upload. I find some messy stuff there like the token sometimes being embedded in the path... I'll raise a separate ticket to address this debt as I'd prefer to have more transparency there where the file list components don't need to care about public or not. This might affect how SDK API is designed.

wait for SDK update + package.json + locks update
raise ticket for public link code mess (+ bugs with createFolder not accepting password...)

PVince81 · 2019-11-12T10:02:30Z

rebase once the public upload acceptance tests are in place, as they will confirm that this works correctly
solve conflicts

PVince81 · 2019-11-14T16:13:45Z

all done. I did a quick run of some upload tests locally and they passed.

let's see if the whole suites run...

ownclouders · 2019-11-14T16:56:58Z

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

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

PVince81 · 2019-11-15T12:54:12Z

BUG: upload issues on public link page: tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature:206 and others

PVince81 · 2019-11-15T15:55:17Z

Fixed files drop case. Glad to see that this could be caught by acceptance tests!

PVince81 · 2019-11-18T08:30:13Z

@LukasHirt all green in CI, just some bug that prevented it to appear published on Github.

Can we merge ?

PVince81 · 2019-11-18T13:12:42Z

BUG: drag and drop from file manager onto the drop zone (overlay that appears when dragging) => it currently opens the file directly ?!

Use owncloud-sdk for uploading files for both the regular file view and also public page. Removed FileUpload.js as it is now obsolete. Had to switch to using paths instead of URLs in the upload handlers. There were some challenges because the public link page currently has the token embedded into the absolute path. Introduced dependency on join-path to avoid more path gymnastics in various places.

PVince81 · 2019-11-18T13:31:37Z

Fixed drag and drop bug. It turns out that vue-dropzone needs a URL even though we don't use that functionality. I've fed it a dummy URL for now. To be cleant up in #1953

LukasHirt

🚀

PVince81 · 2019-11-19T08:51:47Z

One CI job got stuck on cloning. Restart build.

PVince81 added the Status:In-Progress label Oct 14, 2019

PVince81 added this to the backlog milestone Oct 14, 2019

PVince81 requested a review from DeepDiver1975 as a code owner October 14, 2019 13:18

PVince81 self-assigned this Oct 14, 2019

DeepDiver1975 requested changes Oct 14, 2019

View reviewed changes

PVince81 force-pushed the upload-with-sdk branch 2 times, most recently from 878ef46 to 38e8c7e Compare November 7, 2019 18:35

PVince81 mentioned this pull request Nov 7, 2019

WIP public link separate token from path #2408

Closed

PVince81 mentioned this pull request Nov 14, 2019

Update dependencies 20191114 #2469

Merged

PVince81 force-pushed the upload-with-sdk branch from 38e8c7e to 54521b0 Compare November 14, 2019 16:13

PVince81 force-pushed the upload-with-sdk branch from 54521b0 to 42db901 Compare November 15, 2019 15:55

PVince81 force-pushed the upload-with-sdk branch from 42db901 to 52f644a Compare November 18, 2019 13:30

LukasHirt reviewed Nov 18, 2019

View reviewed changes

LukasHirt approved these changes Nov 18, 2019

View reviewed changes

PVince81 merged commit 324d5a4 into master Nov 19, 2019

delete-merged-branch bot deleted the upload-with-sdk branch November 19, 2019 15:58

pascalwengerter mentioned this pull request Apr 6, 2021

Remove vue2-dropzone package #1953

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upload file using SDK instead of XHR #2239

Upload file using SDK instead of XHR #2239

PVince81 commented Oct 14, 2019 •

edited

Loading

PVince81 commented Oct 14, 2019 •

edited

Loading

ownclouders commented Oct 14, 2019

ownclouders commented Oct 14, 2019

DeepDiver1975 Oct 14, 2019

PVince81 Oct 14, 2019

PVince81 commented Oct 14, 2019

PVince81 commented Oct 14, 2019

PVince81 commented Oct 14, 2019 •

edited

Loading

PVince81 commented Oct 14, 2019

PVince81 commented Oct 15, 2019

LukasHirt commented Oct 16, 2019

LukasHirt commented Oct 16, 2019

PVince81 commented Nov 7, 2019 •

edited

Loading

PVince81 commented Nov 12, 2019 •

edited

Loading

PVince81 commented Nov 14, 2019

ownclouders commented Nov 14, 2019

PVince81 commented Nov 15, 2019 •

edited

Loading

PVince81 commented Nov 15, 2019

PVince81 commented Nov 18, 2019

PVince81 commented Nov 18, 2019 •

edited

Loading

PVince81 commented Nov 18, 2019

LukasHirt left a comment

PVince81 commented Nov 19, 2019

Upload file using SDK instead of XHR #2239

Upload file using SDK instead of XHR #2239

Conversation

PVince81 commented Oct 14, 2019 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Open tasks:

PVince81 commented Oct 14, 2019 • edited Loading

ownclouders commented Oct 14, 2019

ownclouders commented Oct 14, 2019

DeepDiver1975 Oct 14, 2019

Choose a reason for hiding this comment

PVince81 Oct 14, 2019

Choose a reason for hiding this comment

PVince81 commented Oct 14, 2019

PVince81 commented Oct 14, 2019

PVince81 commented Oct 14, 2019 • edited Loading

PVince81 commented Oct 14, 2019

PVince81 commented Oct 15, 2019

LukasHirt commented Oct 16, 2019

LukasHirt commented Oct 16, 2019

PVince81 commented Nov 7, 2019 • edited Loading

PVince81 commented Nov 12, 2019 • edited Loading

PVince81 commented Nov 14, 2019

ownclouders commented Nov 14, 2019

PVince81 commented Nov 15, 2019 • edited Loading

PVince81 commented Nov 15, 2019

PVince81 commented Nov 18, 2019

PVince81 commented Nov 18, 2019 • edited Loading

PVince81 commented Nov 18, 2019

LukasHirt left a comment

Choose a reason for hiding this comment

PVince81 commented Nov 19, 2019

PVince81 commented Oct 14, 2019 •

edited

Loading

PVince81 commented Oct 14, 2019 •

edited

Loading

PVince81 commented Oct 14, 2019 •

edited

Loading

PVince81 commented Nov 7, 2019 •

edited

Loading

PVince81 commented Nov 12, 2019 •

edited

Loading

PVince81 commented Nov 15, 2019 •

edited

Loading

PVince81 commented Nov 18, 2019 •

edited

Loading