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

Fix upload download problems #12794

Merged
merged 3 commits into from
Apr 4, 2024

Conversation

JonasMayerDev
Copy link
Collaborator

By Alper
This PR Fixes Following Problems

Sync icon visibility during download
Accessing after download attempt to etag throws null point exception
Remove file dialog actions order is wrong for downloaded and normal file
File download
Note

The Spotbugs count increased due to the reintroduction of old code (with improvements) in this PR.

  • Tests written, or not not needed

Copy link
Contributor

@Unpublished Unpublished left a comment

Choose a reason for hiding this comment

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

As review was requested by Alper in the original PR:
Confirming normal downloading works for me with this PR

@Unpublished Unpublished linked an issue Apr 2, 2024 that may be closed by this pull request
4 tasks
@tobiasKaminsky
Copy link
Member

Can you either suppress spotbugs, or manually increase count, so that CI is green?

@AndyScherzinger
Copy link
Member

/backport to stable-3.28

Copy link
Collaborator

@ZetaTom ZetaTom left a comment

Choose a reason for hiding this comment

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

Downloading and uploading worked flawlessly in my tests.

Please address my comment on FileDownloadHelper.

Comment on lines +53 to +54
return isJobScheduled || if (file.isFolder) {
backgroundJobManager.isStartFileDownloadJobScheduled(user, topParentId)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't isJobScheduled be joined onto line 54 (inside the if branch) instead of line 53? Was this change made on purpose?
Otherwise this could change the return value of isDownloading().

JonasMayerDev and others added 3 commits April 4, 2024 08:57
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
Signed-off-by: Jonas Mayer <jonas.a.mayer@gmx.net>
Co-authored-by: Tom <70907959+ZetaTom@users.noreply.github.com>
Signed-off-by: Alper Öztürk <67455295+alperozturk96@users.noreply.github.com>
@alperozturk96 alperozturk96 force-pushed the Bugfix/upload-and-download-problems branch from b91da26 to 11c24e6 Compare April 4, 2024 06:57
Copy link

github-actions bot commented Apr 4, 2024

Codacy

Lint

TypemasterPR
Warnings7171
Errors33

SpotBugs

CategoryBaseNew
Bad practice6767
Correctness6868
Dodgy code350350
Experimental11
Internationalization77
Malicious code vulnerability22
Multithreaded correctness66
Performance5858
Security1919
Total578578

Copy link

github-actions bot commented Apr 4, 2024

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12794.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@tobiasKaminsky tobiasKaminsky merged commit 39c80a5 into master Apr 4, 2024
19 of 20 checks passed
@delete-merged-branch delete-merged-branch bot deleted the Bugfix/upload-and-download-problems branch April 4, 2024 07:25
Copy link

github-actions bot commented Apr 4, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants