Skip to content
This repository was archived by the owner on Jul 14, 2026. It is now read-only.

Add download feedback - #1895

Merged
PVince81 merged 1 commit into
masterfrom
bugfix/download-feedback
Sep 6, 2019
Merged

Add download feedback#1895
PVince81 merged 1 commit into
masterfrom
bugfix/download-feedback

Conversation

@LukasHirt

@LukasHirt LukasHirt commented Sep 4, 2019

Copy link
Copy Markdown
Collaborator

Description

Use request in file download method, add the file that is being downloaded into progress queue and disable file actions if download action is in progress.

Related Issue

Motivation and Context

When user downloads a big file he could start triggering download multiple times since he had no feedback until the file was downloaded in the background. With request, we can track the progress and add the file into the progress queue. To also prevent triggering the download again it's better to disable actions for the file that is being currently downloaded.

How Has This Been Tested?

  • test environment: Manually
  1. Download a small file
  2. Download a big file
  3. Create public link
  4. Download a file shared with public link
  5. Create a password-protected public link
  6. Download a file shared with a password-protected link

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:

Add downloaded files into progress queue
@LukasHirt LukasHirt added Category:Enhancement Add new functionality Status:Needs-Review Needs review from a maintainer labels Sep 4, 2019
@LukasHirt
LukasHirt requested a review from PVince81 September 4, 2019 19:45
@LukasHirt LukasHirt self-assigned this Sep 4, 2019

@PVince81 PVince81 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code makes sense 👍

@PVince81

PVince81 commented Sep 5, 2019

Copy link
Copy Markdown
Contributor

In OC core I remember that we had to introduce a magic cookie to detect whenever the download had started. Not sure why we don't need this approach here any more.

Ref: https://github.com/owncloud/core/blob/master/apps/files/ajax/download.php#L56 and https://github.com/owncloud/core/blob/master/apps/files/js/files.js#L313

Maybe it was about cases where the download is opened in a separate tab/window.

In the case of Phoenix we likely cannot work with such cookies, so not much point in trying to reproduce the case.

@LukasHirt

Copy link
Copy Markdown
Collaborator Author

In OC core I remember that we had to introduce a magic cookie to detect whenever the download had started. Not sure why we don't need this approach here any more.

I'm not really sure what was the reason to implement this in old UI or how this could be beneficial to Phoenix since we don't use redirection to any download URL but directly trigger the download of a file with download attribute (apart from ie11). Can you pls clarify? @PVince81

@LukasHirt

LukasHirt commented Sep 5, 2019

Copy link
Copy Markdown
Collaborator Author

There is also content-disposition: attachment which should be able to allow the cross-origin download. Haven't yet looked more into it so can't really say if it would fit our use case.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition

@PVince81

PVince81 commented Sep 6, 2019

Copy link
Copy Markdown
Contributor

@LukasHirt I don't think this is a concern for now, let's move forward.

@PVince81
PVince81 merged commit e8189d3 into master Sep 6, 2019
@delete-merged-branch
delete-merged-branch Bot deleted the bugfix/download-feedback branch September 6, 2019 08:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Category:Enhancement Add new functionality Status:Needs-Review Needs review from a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

file download does not work on password protected public shares

2 participants