Added pending uploads screen #5752
Conversation
|
Looks great! 🙂 |
|
Added error fragment WhatsApp.Video.2024-06-09.at.18.19.25_8a3bde9e.mp4 |
|
Here are the few bugs/issues that I have found and currently working on, will make the PR ready for review after solving them
|
|
I started a multiupload of 71 pictures. |
|
Will look into it. |
|
I tested it with 105 uploads it seems to show me 30, when I logged the contributions in |
There was a problem hiding this comment.
Nice work! Left a few minor suggestions.
One thing I often observe is that the progress bar in the notification regresses quite frequently. Maybe we could not estimate the exact progress, but would it be possible to stop at the current progress itself if at all we have to reduce it?
app/src/main/java/fr/free/nrw/commons/contributions/ContributionDao.java
Outdated
Show resolved
Hide resolved
app/src/main/java/fr/free/nrw/commons/upload/FailedUploadsFragment.kt
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| fun deleteUploads(){ |
There was a problem hiding this comment.
How about generalising this method as well? This is being used in the failed uploads fragment too.
| fun retryUpload(contribution: Contribution) { | ||
| if (NetworkUtils.isInternetConnectionEstablished(context)) { | ||
| if (contribution.state == Contribution.STATE_PAUSED | ||
| || contribution.state == Contribution.STATE_QUEUED_LIMITED_CONNECTION_MODE |
There was a problem hiding this comment.
How are we currently supporting limited connection mode? Is this by using the pause button in the top bar?
app/src/main/java/fr/free/nrw/commons/upload/PendingUploadsPresenter.java
Outdated
Show resolved
Hide resolved
| if (CommonsApplication.cancelledUploads.contains(contribution.pageId)) { | ||
| compositeDisposable.clear() | ||
| return@forEach | ||
| }else{ |
There was a problem hiding this comment.
Unfortunately, we still don't have a linter on our repository. Would you mind making the spaces around if-else blocks more consistent? We follow the Java style guide.
| @@ -188,7 +189,7 @@ class UploadWorker(var appContext: Context, workerParams: WorkerParameters) : | |||
| .blockingGet() | |||
| //Showing initial notification for the number of uploads being processed | |||
|
|
|||
| Timber.e("Queued Contributions: " + queuedContributions.size) | |||
| Timber.tag("PRINT").e("Queued Contributions: " + queuedContributions.size) | |||
There was a problem hiding this comment.
If we're planning to club the log messages together, then I would suggest using a more specific name.
|
@RitikaPahwa4444, Thanks for reviewing it, I will try to share a newer commit asap |
|
Hi @RitikaPahwa4444, I have made the fixes in the newer commit 04c102d Needs to work on the notification progress bar though |
|
Hi @nicolas-raoul, Failure of Error received - |
Description (required)
Fixes #5583
What changes did you make and why?
Tests performed (required)
Tested prodDebug on Samsung S21 FE with API level 33.
Screenshots (for UI changes only)
WhatsApp.Video.2024-06-08.at.17.08.37_459ee7ba.mp4
⚙️To-do list -