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

Allow upload of empty folders via drag & drop in WebUI #39285

Merged
merged 2 commits into from
Sep 27, 2021

Conversation

AlexAndBear
Copy link

@AlexAndBear AlexAndBear commented Sep 24, 2021

Description

Enhancement: Allow empty folder uploads via webUI

Before this PR, drag and drop an empty folder did not work, there was no
response in the webUI anyways.
While uploading a folder with a text file and an empty folder,
the folder with the text file was created but the empty folder wasn't.

With this PR the upper scenarios work now.

Related Issue

Motivation and Context

  • I have my music collection on my computer with dirs from interprets a-z
  • I want to upload my music collection to ownCloud
  • Some folders have not been uploaded as they are empty
  • Bad :(

How Has This Been Tested?

Tested on browsers

  • IE 11 -> Dropping folders does not work, even with content or not -> this issue was prexisting Does not work
  • Chrome/Chromium -> Works
  • Edge -> Works
  • Firefox -> Works
  • Safari -> Works

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@AlexAndBear AlexAndBear marked this pull request as ready for review September 24, 2021 13:16
@AlexAndBear AlexAndBear changed the title Allow upload of empty folders Allow upload of empty folders via WebUI Sep 24, 2021
@AlexAndBear
Copy link
Author

AlexAndBear commented Sep 24, 2021

Note, we are touching apps/files/js/jquery.fileupload.js here.

Which has been already done by our devs, e.g bf04daf#diff-ae6d415538c7caff6e1d808b2d442226a680c6ca54576ba4e9f08317bdeee387

@owncloud owncloud deleted a comment from update-docs bot Sep 24, 2021
@AlexAndBear AlexAndBear changed the title Allow upload of empty folders via WebUI Allow upload of empty folders via drag & drop in WebUI Sep 24, 2021
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Just a few suggestions for words in text. If you are sorting out test coverage, you can consider these also. IMO we should not say "This PR" in changelogs, because readers of the changelog may not know what "PR" is.

apps/files/js/file-upload.js Outdated Show resolved Hide resolved
changelog/unreleased/39285 Outdated Show resolved Hide resolved
changelog/unreleased/39285 Outdated Show resolved Hide resolved
changelog/unreleased/39285 Outdated Show resolved Hide resolved
@phil-davis
Copy link
Contributor

phil-davis commented Sep 24, 2021

Works for me - I manually tested drag-and-drop of an empty folder, folder with a file, and flder with a mix of files and folders. I dropped them into the root, or on top of (=into) an existing folder of a user. All combinations worked.

Tested on Ubuntu 20.04 with these browsers:
Chrome Version 94.0.4606.54 (Official Build) (64-bit)
Firefox 92.0
Opera Version:79.0.4143.50

@jvillafanez
Copy link
Member

I might be mistaking with a similar issue, but I think there were problems because the behavior is browser-dependent.
Please, include a list of browsers where you've tested the feature.

@AlexAndBear
Copy link
Author

AlexAndBear commented Sep 27, 2021

@jvillafanez Please reference the GitHub or Portal issue here

I suspect IE and Opera won't work here, because of the limited API https://developer.mozilla.org/en-US/docs/Web/API/DataTransferItem/webkitGetAsEntry

@phil-davis
Copy link
Contributor

I suspect IE and Opera won't work here

I tested Opera on Ubuntu 20.04 and it works.

apps/files/js/file-upload.js Outdated Show resolved Hide resolved
apps/files/js/file-upload.js Outdated Show resolved Hide resolved
@AlexAndBear
Copy link
Author

@micbar can we merge as approved, adding js tests for this case would be much overhead

Copy link
Member

@jvillafanez jvillafanez left a comment

Choose a reason for hiding this comment

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

assuming tests pass

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

LGTM. I manually tested with 3 different browsers. See comment #39285 (comment)

@sonarcloud
Copy link

sonarcloud bot commented Sep 27, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

12.5% 12.5% Coverage
0.0% 0.0% Duplication

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.

Drag and drop empty folder not working
5 participants