Skip to content

Conversation

@maskaravivek
Copy link
Contributor

Title (required)

Fixes part of #1545.

Description (required)

The only could that could be null in the startUpload method of UploadController is name field of CurrentAccount. Have added a null check for the same and will be redirecting the user to login if the session is null.

Tests performed (required)

Unable to reproduce the issue but this check should ideally prevent the crash and redirect the user to login.

@codecov-io
Copy link

codecov-io commented Jul 1, 2018

Codecov Report

Merging #1684 into 2.8-release will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff               @@
##           2.8-release   #1684      +/-   ##
==============================================
- Coverage         3.87%   3.86%   -0.01%     
==============================================
  Files              150     150              
  Lines             7571    7576       +5     
  Branches           710     711       +1     
==============================================
  Hits               293     293              
- Misses            7261    7266       +5     
  Partials            17      17
Impacted Files Coverage Δ
.../java/fr/free/nrw/commons/auth/SessionManager.java 15.38% <ø> (ø) ⬆️
...a/fr/free/nrw/commons/upload/UploadController.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 8859ff9...794ef83. Read the comment docs.

Copy link
Member

@misaochan misaochan left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this @maskaravivek . Looks good to me, submitted a suggestion for a string change.

<string name="menu_set_wallpaper">Set wallpaper</string>
<string name="wallpaper_set_successfully">Wallpaper set successfully!</string>

<string name="user_not_logged_in">No active session found for this user.</string>
Copy link
Member

@misaochan misaochan Jul 2, 2018

Choose a reason for hiding this comment

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

Probably more user-friendly to say something like: "Login session expired, please log in again" ?

@misaochan misaochan merged commit 306f23d into commons-app:2.8-release Jul 3, 2018
@maskaravivek maskaravivek deleted the uploadCrash branch September 12, 2018 20:25
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.

3 participants