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

app: prevent duplicate file downloads at the UI level #974

Merged
merged 1 commit into from
Mar 23, 2020

Conversation

redshiftzero
Copy link
Contributor

Description

Fixes #116.

This is a UI layer solution for the purpose of the beta to resolve the above issue where multiple clicks result in attempting to download a file multiple times (see prevent-duplicate-jobs for a generic approach for ensuring duplicate jobs in general cannot be added to the queue).

Test Plan

  1. Submit a file as a source.
  2. Sign into the client with debug level logging (LOGLEVEL=DEBUG ./run.sh), attempt to download the file.
  3. While it is downloading (note: for test purposes you should submit a large file that takes some time to download OR add a time.sleep(2) in the submission download endpoint server-side), keep clicking.
  4. Ensure that no additional jobs are added.

You should see the following line only once. indicating that duplicated clicks did not result in duplicate FileDownloadJobs being enqueued:

2020-03-20 20:17:25,768 - securedrop_client.queue:233(enqueue) DEBUG: Added FileDownloadJob to download queue

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

  • I have tested these changes in the appropriate Qubes environment
  • I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
  • These changes should not need testing in Qubes

If these changes add or remove files other than client code, packaging logic (e.g., the AppArmor profile) may need to be updated. Please check as applicable:

  • I have submitted a separate PR to the packaging repo
  • No update to the packaging logic (e.g., AppArmor profile) is required for these changes
  • I don't know and would appreciate guidance

rmol
rmol previously approved these changes Mar 23, 2020
Copy link
Contributor

@rmol rmol left a comment

Choose a reason for hiding this comment

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

Works. Just needs a rebase.

Copy link
Contributor

@rmol rmol left a comment

Choose a reason for hiding this comment

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

Yep, still works.

@rmol rmol merged commit 33f46ee into master Mar 23, 2020
@rmol rmol deleted the ui-level-file-widget-download branch March 23, 2020 20:50
@eloquence eloquence mentioned this pull request Jul 16, 2020
2 tasks
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.

Avoid duplicate downloads of files
2 participants