Retry failed uploads - #1556 Allow users to easily re-upload failed uploads... #2112
Conversation
…ires internet connection
Codecov Report
@@ Coverage Diff @@
## master #2112 +/- ##
=========================================
+ Coverage 5.6% 5.69% +0.08%
=========================================
Files 225 233 +8
Lines 11379 11614 +235
Branches 1065 1077 +12
=========================================
+ Hits 638 661 +23
- Misses 10689 10898 +209
- Partials 52 55 +3
Continue to review full report at Codecov.
|
| * @param cursor cursor which will be deleted | ||
| */ | ||
| public void deleteUpload(Cursor cursor) { | ||
| if (NetworkUtils.isInternetConnectionEstablished(mContext)) { |
There was a problem hiding this comment.
This deletes the thing from the local db right, do we need to have a check for Network Connection ?
There was a problem hiding this comment.
Very valid concern but yes, since we can not make sure db is up to date. If we network off during upload, then click to retry or cancel, cursor displays latest uploaded item and it says "Skipping deletion for non-failed contrib [previous contrib]" . It points to wrong contrib. For retry it was same. You can go
f3818c6 commit and see. Thanks for reviewing though:)
There was a problem hiding this comment.
Ohh, if that is the case, then yeah it is required
|
We can switch from toast to any other thing to display this solutions. If you have any idea pleas share |
|
Thanks for the PR, @neslihanturan . Unfortunately I have no failed uploads and thus cannot test this. The code looks good to me however. If @maskaravivek or @ashishkumar468 can take a look at the code and approve it as well, I think we can just merge... and let our alpha users test it. ;) |
app/src/main/res/values/strings.xml
Outdated
| <string name="display_location_permission_explanation">Ask for location permission when needed for nearby notification card view feature.</string> | ||
|
|
||
| <string name="this_function_needs_network_connection">This function requires network connection, please check your connection settings.</string> | ||
| <string name="bad_token_error_proposed_solution">Upload failed with "badtoken" error. Logging out and in again usually help to solve this issue. Meanwhile, we are working to prevent this error. </string> |
There was a problem hiding this comment.
Upload failed due to issues with edit token. Please try logging out and in again.
|
@misaochan you can test it like this: Upload something, disable network during connection. It will fail and you can test it. This is how I did during development:) |
|
String is fixed, thanks @misaochan :) |
There was a problem hiding this comment.
The changes look good to me. Even I do not have a failed upload on my device as of now. :(
We can probably merge it and let our alpha users test it.
Also, if we are suggesting users to log out for bad token error, we should persist the pending contributions otherwise he will have to again upload the same file. For him, the easiest thing to do would be to simply click on retry after re-login.
The flow could be:
try an upload > badtoken error > show a snackbar prompting him to login > take him to login page without clearing shared prefs and DBs > on relogin let him retry the upload.
We just need the user to refresh his user session, so there's no need to clear other data.
|
You can manage to create failed uploads with technique I mentioned above. Anyway, I consistently created and retry button worked for me. |
|
@neslihanturan Could you please rebase on master so that @maskaravivek 's Travis fixes are included? The Travis build is failing, probably because of that. |
|
It was just old Travis report, currently it passed. You can go ahead and merge:) |
|
@misaochan pinging:) So that I can mark this iğssue as solved on #2171 |
|
Yes you can mark the issue as solved. ;) |
Description (required)
Fixes #1556 Allow users to easily re-upload failed uploads... Currently, fail and cancel buttons moved to contrib list and they works as intended.
But we don not give any reason about fail. Because I couldn't find any table of Mediawiki API error codes. Maybe someone can help me.It gives a possible solution for badtoken error code.
Meanwhile this can be merged.
What changes did you make and why?
Tests performed (required)
Tested API 26, betaDebug
Screenshots showing what changed (optional - for UI changes)
