Skip to content

Attempt to fix failed uploads #1592

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

Merged
merged 3 commits into from
Jun 7, 2018

Conversation

misaochan
Copy link
Member

@misaochan misaochan commented Jun 7, 2018

Hopefully might fix #1485 . In cases where we cannot revalidate auth token during file upload, force user to re-login. (Similar to #1433)

I cannot test this as I can't reproduce the bug, but it builds successfully.

@codecov-io
Copy link

Codecov Report

Merging #1592 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1592      +/-   ##
=========================================
- Coverage    3.87%   3.87%   -0.01%     
=========================================
  Files         150     150              
  Lines        7560    7567       +7     
  Branches      708     708              
=========================================
  Hits          293     293              
- Misses       7250    7257       +7     
  Partials       17      17
Impacted Files Coverage Δ
...java/fr/free/nrw/commons/upload/UploadService.java 0% <0%> (ø) ⬆️
.../java/fr/free/nrw/commons/auth/SessionManager.java 15.38% <0%> (-1.29%) ⬇️
...n/java/fr/free/nrw/commons/auth/LoginActivity.java 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f47d39...7e40f43. Read the comment docs.

@misaochan misaochan requested a review from maskaravivek June 7, 2018 07:58
@misaochan misaochan changed the base branch from master to 2.8-release June 7, 2018 08:08
Copy link
Member

@maskaravivek maskaravivek left a comment

Choose a reason for hiding this comment

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

These changes look good to me and would help in cases where authentication fails.

Should we also consider upload logging API failures(using logs API or keeping the logs on a wiki page), so that we know why the uploads are failing for various users.

@neslihanturan
Copy link
Collaborator

neslihanturan commented Jun 7, 2018

This should do the job @misaochan , thanks! I am also unable to reproduce the bug now but I did once, and then I reinstalled app for testing purposes and never experienced the bug again. So it is probably related with revalidation.

@neslihanturan neslihanturan merged commit b3b4567 into commons-app:2.8-release Jun 7, 2018
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.

4 participants